drm/i915: fix hpd work vs. flush_work in the pageflip code deadlock

commit 645416f5adc87c8fae44289cdba7562f3ade8f5c upstream.

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.

v2: Add a comment to the declaration of dev_priv->wq to warn readers
about the tricky implications of using it. Suggested by Chris Wilson.

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
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
[danvet: Squash in a comment at the place where we schedule the work.
Requested after-the-fact by Chris on irc since the hpd work isn't the
only place we botch this.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1929bffc1c77f645a920c9b4037f0b5eb646a145..2f09e8004e4c225a2c4a97e323f5953df42be040 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1091,6 +1091,13 @@ typedef struct drm_i915_private {
 
        unsigned int fsb_freq, mem_freq, is_ddr3;
 
+       /**
+        * wq - Driver workqueue for GEM.
+        *
+        * NOTE: Work items scheduled here are not allowed to grab any modeset
+        * locks, for otherwise the flushing done in the pageflip code will
+        * result in deadlocks.
+        */
        struct workqueue_struct *wq;
 
        /* Display functions */

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 3d92a7cef1541bd20b2c4016cdc2447791991c56..6eadcfa96b8814b89ce611c720d0a0bf54df1c2f 100644 (file)
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -910,8 +910,13 @@ static inline void intel_hpd_irq_handler(struct drm_device *dev,
                dev_priv->display.hpd_irq_setup(dev);
        spin_unlock(&dev_priv->irq_lock);
 
-       queue_work(dev_priv->wq,
-                  &dev_priv->hotplug_work);
+       /*
+        * Our hotplug handler can grab modeset locks (by calling down into the
+        * fb helpers). Hence it must not be run on our own dev-priv->wq work
+        * queue for otherwise the flush_work in the pageflip code will
+        * deadlock.
+        */
+       schedule_work(&dev_priv->hotplug_work);
 }
 
 static void gmbus_irq_handler(struct drm_device *dev)