[Intel-gfx] [PATCH] drm/i915: Avoid double mutex lock applying pipe A quirk during sanitize_crtc()

[Daniel Vetter]

On Tue, Jun 10, 2014 at 11:53:51AM +0300, Ville Syrjälä wrote:
> On Tue, Jun 10, 2014 at 09:02:07AM +0200, Daniel Vetter wrote:
> > On Mon, Jun 09, 2014 at 11:30:26AM +0300, Ville Syrjälä wrote:
> > > On Mon, Jun 09, 2014 at 07:47:10AM +0100, Chris Wilson wrote:
> > > > Thomas found that his machine would deadlock reloading the i915.ko
> > > > module after resume. He identified that this was caused by the
> > > > reacquisition of the connection mutex inside intel_enable_pipe_a()
> > > > during the CRTC sanitization routine. This will only affect machines
> > > > that quirk PIPE A, i.e. the original 830m chipsets.
> > > > 
> > > > This patch move the locking into a wrapper function so that
> > > > intel_enable_pipe_a() can bypass the locking knowing that it already
> > > > holds the correct locks.
> > > 
> > > It can still try to grab crtc->mutex twice. Looks like Danial undid my
> > > fix to not take all the modeset locks around
> > > intel_modeset_setup_hw_state().
> > 
> > Hm, I didn't find anything in git logs and we've had places where we used
> > modeset_lock_all since a long time. And I don't see how Chris' patch
> > wouldn't address this here. Can you please explain?
> 
> I added the modeset_all locking here:
>  commit 027476642811f8559cbe00ef6cc54db230e48a20
>  Author: Ville Syrjälä <ville.syrjala at linux.intel.com>
>  Date:   Mon Dec 2 11:08:06 2013 +0200
> 
>     drm/i915: Take modeset locks around intel_modeset_setup_hw_state()
> 
> and then had to reduce it to just the mode_config.mutex precisely due to
> the pipe A quirk here:
>  commit 7ad228b11ec26a820291c9f5a1168d6176580dc1
>  Author: Ville Syrjälä <ville.syrjala at linux.intel.com>
>  Date:   Tue Jan 7 16:15:36 2014 +0200
> 
>     drm/i915: Don't grab crtc mutexes in intel_modeset_gem_init()

Hm, indeed missed this since we have a bunch of other places that still do
the full modeset_lock_all, but probably not possible to hit the pipe A
quirk there. The ww mutex code makes this a bit more annoying so I guess
Chris' patch with having a lock-less pipe A quirk is the way to go.

But that one needs to be updated for latest drm-next/-nightly.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch