[PATCH 1/4] drm/i915: teardown default context in reverse, update comments

From: Dave Gordon <david.s.gordon@intel.com>
To: intel-gfx@lists.freedesktop.org
Subject: [PATCH 1/4] drm/i915: teardown default context in reverse, update comments
Date: Wed, 16 Dec 2015 18:36:48 +0000	[thread overview]
Message-ID: <1450291011-31486-2-git-send-email-david.s.gordon@intel.com> (raw)
In-Reply-To: <1450291011-31486-1-git-send-email-david.s.gordon@intel.com>

We set up engines in forwards order, so some things (notably the
default context) are "owned" by engine 0 (the render engine, aka "RCS").
For symmetry and to make sure such shared objects don't disappear too
early, we should generally run teardown loops in the reverse order,
so that engine 0 is processed last.

This patch changes i915_gem_context_fini() to do that, and clarifies the
comments in i915_gem_context_{init,fini}() about the refcounting of the
default {struct intel_)context: the refcount is just ONE, no matter how
many rings exist or are active, and this refcount is nominally ascribed
to the render ring (RCS), which is set up first and now torn down last.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 900ffd0..e143ea5 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -391,7 +391,13 @@ int i915_gem_context_init(struct drm_device *dev)
 	for (i = 0; i < I915_NUM_RINGS; i++) {
 		struct intel_engine_cs *ring = &dev_priv->ring[i];
 
-		/* NB: RCS will hold a ref for all rings */
+		/*
+		 * Although each engine has a pointer to the global default
+		 * context, they don't contribute to the refcount on the
+		 * context. We consider that RCS (which is set up first and
+		 * torn down last) holds this reference on behalf of all the
+		 * other engines
+		 */
 		ring->default_context = ctx;
 	}
 
@@ -431,14 +437,21 @@ void i915_gem_context_fini(struct drm_device *dev)
 		i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
 	}
 
-	for (i = 0; i < I915_NUM_RINGS; i++) {
+	for (i = I915_NUM_RINGS; --i >= 0;) {
 		struct intel_engine_cs *ring = &dev_priv->ring[i];
 
-		if (ring->last_context)
+		if (ring->last_context) {
 			i915_gem_context_unreference(ring->last_context);
+			ring->last_context = NULL;
+		}
 
+		/*
+		 * These default_context pointers don't contribute to the
+		 * refcount on the context. We consider that RCS holds its
+		 * reference on behalf of all the other engines, so there's
+		 * just a single unreference() call below.
+		 */
 		ring->default_context = NULL;
-		ring->last_context = NULL;
 	}
 
 	i915_gem_context_unreference(dctx);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
