[Intel-gfx] [PATCH] Avoid i915 flip unpin/HPD event handler deadlock.

[Stuart Abercrombie]

Both of these were taking the mode_config mutex but executed from the
same work queue.  If intel_crtc_page_flip happened to flush a work queue
containing an HPD event handler work item, deadlock resulted, since the
mutex required by the HPD handler was taken before the flush.  Instead
use a separate work queue for the flip unpin work.

Signed-off-by: sabercrombie at chromium.org
Reviewed-by: marcheu at chromium.org

---
 drivers/gpu/drm/i915/i915_dma.c      | 21 ++++++++++++++++++++-
 drivers/gpu/drm/i915/i915_drv.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c |  4 ++--
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 4f129bb..9215360 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1558,6 +1558,22 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 		goto out_mtrrfree;
 	}
 
+	/* intel_crtc_page_flip runs with the mode_config mutex having been
+	 * taken in the DRM layer.  It synchronously waits for pending unpin
+	 * work items while holding this mutex.  Therefore this queue cannot
+	 * contain work items that take this mutex, such as HPD event
+	 * handling, or we deadlock.  There is also no reason for flipping to
+	 * wait on such events.  Therefore put flip unpinning in its own
+	 * work queue.
+	 */
+	dev_priv->flip_unpin_wq = alloc_ordered_workqueue("i915", 0);
+	if (dev_priv->flip_unpin_wq == NULL) {
+		DRM_ERROR("Failed to create flip unpin workqueue.\n");
+		destroy_workqueue(dev_priv->wq);
+		ret = -ENOMEM;
+		goto out_mtrrfree;
+	}
+
 	/* This must be called before any calls to HAS_PCH_* */
 	intel_detect_pch(dev);
 
@@ -1628,6 +1644,7 @@ out_gem_unload:
 	intel_teardown_gmbus(dev);
 	intel_teardown_mchbar(dev);
 	destroy_workqueue(dev_priv->wq);
+	destroy_workqueue(dev_priv->flip_unpin_wq);
 out_mtrrfree:
 	if (dev_priv->mm.gtt_mtrr >= 0) {
 		mtrr_del(dev_priv->mm.gtt_mtrr,
@@ -1709,7 +1726,8 @@ int i915_driver_unload(struct drm_device *dev)
 	intel_opregion_fini(dev);
 
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
-		/* Flush any outstanding unpin_work. */
+		/* Flush any outstanding unpin, HPD, etc. work. */
+		flush_workqueue(dev_priv->flip_unpin_wq);
 		flush_workqueue(dev_priv->wq);
 
 		mutex_lock(&dev->struct_mutex);
@@ -1734,6 +1752,7 @@ int i915_driver_unload(struct drm_device *dev)
 	intel_teardown_mchbar(dev);
 
 	destroy_workqueue(dev_priv->wq);
+	destroy_workqueue(dev_priv->flip_unpin_wq);
 
 	pci_dev_put(dev_priv->bridge_dev);
 	kfree(dev->dev_private);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4130e6d..7eb4619 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -766,6 +766,7 @@ typedef struct drm_i915_private {
 	struct work_struct error_work;
 	struct completion error_completion;
 	struct workqueue_struct *wq;
+	struct workqueue_struct *flip_unpin_wq;
 
 	/* Display functions */
 	struct drm_i915_display_funcs display;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e204913..acd07dd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6937,7 +6937,7 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
 			  &obj->pending_flip.counter);
 	wake_up(&dev_priv->pending_flip_queue);
 
-	queue_work(dev_priv->wq, &work->work);
+	queue_work(dev_priv->flip_unpin_wq, &work->work);
 
 	trace_i915_flip_complete(intel_crtc->plane, work->pending_flip_obj);
 }
@@ -7305,7 +7305,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 
 	if (atomic_read(&intel_crtc->unpin_work_count) >= 2)
-		flush_workqueue(dev_priv->wq);
+		flush_workqueue(dev_priv->flip_unpin_wq);
 
 	ret = i915_mutex_lock_interruptible(dev);
 	if (ret)
-- 
1.8.4