diff mbox

drm/i915/ringbuffer: Make context pin/unpin symmetric

Message ID 20180605085348.3018-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson June 5, 2018, 8:53 a.m. UTC
Currently, we have a special routine for pinning the context state at
the start of activity tracking, but lack the complementary unpin
routine. Create it to to ease later patches that want to do partial
teardown on error, and, not least, to improve the readability of the
code.

Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 67 ++++++++++++++++---------
 1 file changed, 43 insertions(+), 24 deletions(-)

Comments

Joonas Lahtinen June 5, 2018, 9:09 a.m. UTC | #1
Quoting Chris Wilson (2018-06-05 11:53:48)
> Currently, we have a special routine for pinning the context state at
> the start of activity tracking, but lack the complementary unpin
> routine. Create it to to ease later patches that want to do partial
> teardown on error, and, not least, to improve the readability of the
> code.
> 
> Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Matthew Auld <matthew.william.auld@gmail.com

Matt's e-mail seems damaged here.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 97b38bbb7ce2..63342e3ed068 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1195,20 +1195,14 @@  static void intel_ring_context_destroy(struct intel_context *ce)
 		__i915_gem_object_release_unless_active(ce->state->obj);
 }
 
-static void intel_ring_context_unpin(struct intel_context *ce)
-{
-	if (ce->state) {
-		ce->state->obj->pin_global--;
-		i915_vma_unpin(ce->state);
-	}
-
-	i915_gem_context_put(ce->gem_context);
-}
-
 static int __context_pin(struct intel_context *ce)
 {
-	struct i915_vma *vma = ce->state;
-	int ret;
+	struct i915_vma *vma;
+	int err;
+
+	vma = ce->state;
+	if (!vma)
+		return 0;
 
 	/*
 	 * Clear this page out of any CPU caches for coherent swap-in/out.
@@ -1216,13 +1210,42 @@  static int __context_pin(struct intel_context *ce)
 	 * on an active context (which by nature is already on the GPU).
 	 */
 	if (!(vma->flags & I915_VMA_GLOBAL_BIND)) {
-		ret = i915_gem_object_set_to_gtt_domain(vma->obj, true);
-		if (ret)
-			return ret;
+		err = i915_gem_object_set_to_gtt_domain(vma->obj, true);
+		if (err)
+			return err;
 	}
 
-	return i915_vma_pin(vma, 0, I915_GTT_MIN_ALIGNMENT,
-			    PIN_GLOBAL | PIN_HIGH);
+	err = i915_vma_pin(vma, 0, I915_GTT_MIN_ALIGNMENT,
+			   PIN_GLOBAL | PIN_HIGH);
+	if (err)
+		return err;
+
+	/*
+	 * And mark is as a globally pinned object to let the shrinker now
+	 * it cannot reclaim the object until we release it.
+	 */
+	vma->obj->pin_global++;
+
+	return 0;
+}
+
+static void __context_unpin(struct intel_context *ce)
+{
+	struct i915_vma *vma;
+
+	vma = ce->state;
+	if (!vma)
+		return;
+
+	vma->obj->pin_global--;
+	i915_vma_unpin(vma);
+}
+
+static void intel_ring_context_unpin(struct intel_context *ce)
+{
+	__context_unpin(ce);
+
+	i915_gem_context_put(ce->gem_context);
 }
 
 static struct i915_vma *
@@ -1313,13 +1336,9 @@  __ring_context_pin(struct intel_engine_cs *engine,
 		ce->state = vma;
 	}
 
-	if (ce->state) {
-		err = __context_pin(ce);
-		if (err)
-			goto err;
-
-		ce->state->obj->pin_global++;
-	}
+	err = __context_pin(ce);
+	if (err)
+		goto err;
 
 	i915_gem_context_get(ctx);