Re: [PATCH] drm/i915: fix hpd work vs. flush_work in the pageflip code deadlock

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Sep 02, 2013 at 12:52:05PM +0100, Chris Wilson wrote:
> On Mon, Sep 02, 2013 at 08:49:01AM +0200, Daniel Vetter wrote:
> > Historically we've run our own driver hotplug handling in our own
> > work-queue, which then launched the drm core hotplug handling in the
> > system workqueue. This is important since we flush our own driver
> > workqueue in the pageflip code while hodling modeset locks, and only
> > the drm hotplug code grabbed these locks. But with
> > 
> > commit 69787f7da6b2adc4054357a661aaa1701a9ca76f
> > Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Date:   Tue Oct 23 18:23:34 2012 +0000
> > 
> >     drm: run the hpd irq event code directly
> > 
> > this was changed and now we could deadlock in our flip handler if
> > there's a hotplug work blocking the progress of the crucial unpin
> > works. So this broke the careful deadlock avoidance implemented in
> > 
> > commit b4a98e57fc27854b5938fc8b08b68e5e68b91e1f
> > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > Date:   Thu Nov 1 09:26:26 2012 +0000
> > 
> >     drm/i915: Flush outstanding unpin tasks before pageflipping
> > 
> > Since the rule thus far has been that work items on our own workqueue
> > may never grab modeset locks simply restore that rule again.
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Stuart Abercrombie <sabercrombie@chromium.org>
> > Reported-by: Stuart Abercrombie <sabercrombie@chromium.org>
> > References: http://permalink.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/26239
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> That wins for simplicity, and it is indeed the only caller that requires
> mode_config.lock, so
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Bonus would a reminder in i915_drv.h to say that we cannot put items
> that require mode_config.lock onto the wq, and that they should go onto
> the global workqueue instead. 

I've merged the updated version, thanks for your review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch