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

Daniel Vetter
2014-06-10T09:22:43Z

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@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@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