[Intel-gfx] [PATCH] drm/i915: Fix recursive calls to unmap

[Ben Widawsky]

After the ILK vt-d workaround patches it became clear that we had
introduced a bug.  Chris tracked down the issue to recursive calls to
unmap. This happens because we try to optimize waiting on requests by
calling retire requests after the wait, which may drop the last
reference on an object and end up freeing the object (and then unmap the
object from the gtt).

The solution here is to make a somewhat generic flag to tell our code
not to try to unmap objects from the GTT.

Kudos to Chris for tracking this one down.

Cc: Keith Packard <keithp at keithp.com>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
Reported-by: guang.a.yang at intel.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42180
Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_drv.h     |    9 +++++++++
 drivers/gpu/drm/i915/i915_gem.c     |    5 ++++-
 drivers/gpu/drm/i915/i915_gem_gtt.c |    6 +++++-
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 06a37f4..bddf19a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -654,6 +654,15 @@ typedef struct drm_i915_private {
 		 */
 		atomic_t wedged;
 
+		/**
+		 * Flag if GTT ptes shouldn't be modified.
+		 *
+		 * This is set when graphics virtual address space
+		 * should not be changed. It's currently only useful for
+		 * a workaround which requires that we serialize unmaps.
+		 */
+		bool freeze_ptes;
+
 		/** Bit 6 swizzling required for X tiling */
 		uint32_t bit_6_swizzle_x;
 		/** Bit 6 swizzling required for Y tiling */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6651c36..b5cf0e3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2026,8 +2026,11 @@ i915_wait_request(struct intel_ring_buffer *ring,
 	 * to handle this, the waiter on a request often wants an associated
 	 * buffer to have made it to the inactive list, and we would need
 	 * a separate wait queue to handle that.
+	 *
+	 * The caller may have not wanted us to actually retire commands
+	 * as that may lead to GTT modifications.
 	 */
-	if (ret == 0)
+	if (ret == 0 && !dev_priv->mm.freeze_ptes)
 		i915_gem_retire_requests_ring(ring);
 
 	return ret;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 6042c5e..2f4b6c7 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -55,6 +55,8 @@ static bool do_idling(struct drm_i915_private *dev_priv)
 
 	if (unlikely(dev_priv->mm.gtt->do_idle_maps)) {
 		dev_priv->mm.interruptible = false;
+		WARN_ON(dev_priv->mm.freeze_ptes == true);
+		dev_priv->mm.freeze_ptes = true;
 		if (i915_gpu_idle(dev_priv->dev)) {
 			DRM_ERROR("Couldn't idle GPU\n");
 			/* Wait a bit, in hopes it avoids the hang */
@@ -67,8 +69,10 @@ static bool do_idling(struct drm_i915_private *dev_priv)
 
 static void undo_idling(struct drm_i915_private *dev_priv, bool interruptible)
 {
-	if (unlikely(dev_priv->mm.gtt->do_idle_maps))
+	if (unlikely(dev_priv->mm.gtt->do_idle_maps)) {
+		dev_priv->mm.freeze_ptes = false;
 		dev_priv->mm.interruptible = interruptible;
+	}
 }
 
 void i915_gem_restore_gtt_mappings(struct drm_device *dev)
-- 
1.7.7.1