[Intel-gfx] [PATCH 41/42] drm/i915/bdw: Pin the context backing objects to GGTT on-demand

oscar.mateo at intel.com oscar.mateo at intel.com
Fri Jul 11 17:49:33 CEST 2014


From: Oscar Mateo <oscar.mateo at intel.com>

Up until now, we have pinned every logical ring context backing object
during creation, and left it pinned until destruction. This made my life
easier, but it's a harmful thing to do, because we cause fragmentation
of the GGTT (and, eventually, we would run out of space).

This patch makes the pinning on-demand: the backing objects of the two
contexts that are written to the ELSP are pinned right before submission
and unpinned once the hardware is done with them. The only context that
is still pinned regardless is the global default one, so that the HWS can
still be accessed in the same way (ring->status_page).

There are several TODO left in this patch, so it's probably not ready for
merging yet:
- execlists_submit_context has increased chances of failing now, so we
  cannot simply BUG_ON if it does. I outlined here a possible recovery
  mechanism, but maybe it's a bit too naive.
- The debugfs entries that access the context backing object need to be
  checked and modified to pin the object accordingly.
- A recovery from reset probably needs to unpin some objects as well.
- Something is not right during module re-loading (occasional failure
  in tests/drv_module_reload).
- Lots of testing!!

Signed-off-by: Oscar Mateo <oscar.mateo at intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 74 +++++++++++++++++++++++++++++-----------
 1 file changed, 54 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 3c3fa69..244269b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -333,24 +333,41 @@ static int execlists_ctx_write_tail(struct drm_i915_gem_object *ctx_obj, u32 tai
 }
 
 static int execlists_submit_context(struct intel_engine_cs *ring,
-				    struct intel_context *to0, u32 tail0,
-				    struct intel_context *to1, u32 tail1)
+				    struct intel_ctx_submit_request *req0,
+				    struct intel_ctx_submit_request *req1)
 {
-	struct drm_i915_gem_object *ctx_obj0;
+	struct intel_context *to0 = req0->ctx;
+	struct drm_i915_gem_object *ctx_obj0 = to0->engine[ring->id].state;
 	struct drm_i915_gem_object *ctx_obj1 = NULL;
+	bool ctx0_pinned = false;
+	int ret;
 
-	ctx_obj0 = to0->engine[ring->id].state;
 	BUG_ON(!ctx_obj0);
-	BUG_ON(!i915_gem_obj_is_pinned(ctx_obj0));
+	if (to0 != ring->default_context && !req0->elsp_submitted) {
+		ret = i915_gem_obj_ggtt_pin(ctx_obj0, GEN8_LR_CONTEXT_ALIGN, 0);
+		if (ret)
+			return ret;
+		ctx0_pinned = true;
+	}
+
+	execlists_ctx_write_tail(ctx_obj0, req0->tail);
 
-	execlists_ctx_write_tail(ctx_obj0, tail0);
+	if (req1) {
+		struct intel_context *to1 = req1->ctx;
 
-	if (to1) {
 		ctx_obj1 = to1->engine[ring->id].state;
+
 		BUG_ON(!ctx_obj1);
-		BUG_ON(!i915_gem_obj_is_pinned(ctx_obj1));
+		if (to1 != ring->default_context) {
+			ret = i915_gem_obj_ggtt_pin(ctx_obj1, GEN8_LR_CONTEXT_ALIGN, 0);
+			if (ret) {
+				if (ctx0_pinned)
+					i915_gem_object_ggtt_unpin(ctx_obj0);
+				return ret;
+			}
+		}
 
-		execlists_ctx_write_tail(ctx_obj1, tail1);
+		execlists_ctx_write_tail(ctx_obj1, req1->tail);
 	}
 
 	execlists_elsp_write(ring, ctx_obj0, ctx_obj1);
@@ -388,8 +405,15 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
 
 	WARN_ON(req1 && req1->elsp_submitted);
 
-	BUG_ON(execlists_submit_context(ring, req0->ctx, req0->tail,
-			req1? req1->ctx : NULL, req1? req1->tail : 0));
+	if (WARN_ON(execlists_submit_context(ring, req0, req1))) {
+		list_del(&req0->execlist_link);
+		queue_work(dev_priv->wq, &req0->work);
+		if (req1) {
+			list_del(&req1->execlist_link);
+			queue_work(dev_priv->wq, &req1->work);
+		}
+		return;
+	}
 
 	req0->elsp_submitted++;
 	if (req1)
@@ -413,6 +437,8 @@ static bool execlists_check_remove_request(struct intel_engine_cs *ring,
 			WARN(head_req->elsp_submitted == 0,
 					"Never submitted head request\n");
 			if (--head_req->elsp_submitted <= 0) {
+				if (head_req->ctx != ring->default_context)
+					i915_gem_object_ggtt_unpin(ctx_obj);
 				list_del(&head_req->execlist_link);
 				queue_work(dev_priv->wq, &head_req->work);
 				return true;
@@ -1604,12 +1630,15 @@ void intel_lr_context_free(struct intel_context *ctx)
 
 	for (i = 0; i < I915_NUM_RINGS; i++) {
 		struct drm_i915_gem_object *ctx_obj = ctx->engine[i].state;
-		struct intel_ringbuffer *ringbuf = ctx->engine[i].ringbuf;
 
 		if (ctx_obj) {
+			struct intel_ringbuffer *ringbuf = ctx->engine[i].ringbuf;
+			struct intel_engine_cs *ring = ringbuf->ring;
+
 			intel_destroy_ringbuffer_obj(ringbuf);
 			kfree(ringbuf);
-			i915_gem_object_ggtt_unpin(ctx_obj);
+			if (ctx == ring->default_context)
+				i915_gem_object_ggtt_unpin(ctx_obj);
 			drm_gem_object_unreference(&ctx_obj->base);
 		}
 	}
@@ -1652,6 +1681,7 @@ static uint32_t get_lr_context_size(struct intel_engine_cs *ring)
 int intel_lr_context_deferred_create(struct intel_context *ctx,
 				     struct intel_engine_cs *ring)
 {
+	const bool is_global_default_ctx = (ctx == ring->default_context);
 	struct drm_device *dev = ring->dev;
 	struct drm_i915_gem_object *ctx_obj;
 	uint32_t context_size;
@@ -1671,18 +1701,21 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
 		return ret;
 	}
 
-	ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN, 0);
-	if (ret) {
-		DRM_DEBUG_DRIVER("Pin LRC backing obj failed: %d\n", ret);
-		drm_gem_object_unreference(&ctx_obj->base);
-		return ret;
+	if (is_global_default_ctx) {
+		ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN, 0);
+		if (ret) {
+			DRM_DEBUG_DRIVER("Pin LRC backing obj failed: %d\n", ret);
+			drm_gem_object_unreference(&ctx_obj->base);
+			return ret;
+		}
 	}
 
 	ringbuf = kzalloc(sizeof(*ringbuf), GFP_KERNEL);
 	if (!ringbuf) {
 		DRM_DEBUG_DRIVER("Failed to allocate ringbuffer %s\n",
 				ring->name);
-		i915_gem_object_ggtt_unpin(ctx_obj);
+		if (is_global_default_ctx)
+			i915_gem_object_ggtt_unpin(ctx_obj);
 		drm_gem_object_unreference(&ctx_obj->base);
 		ret = -ENOMEM;
 		return ret;
@@ -1738,7 +1771,8 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
 
 error:
 	kfree(ringbuf);
-	i915_gem_object_ggtt_unpin(ctx_obj);
+	if (is_global_default_ctx)
+		i915_gem_object_ggtt_unpin(ctx_obj);
 	drm_gem_object_unreference(&ctx_obj->base);
 	return ret;
 }
-- 
1.9.0




More information about the Intel-gfx mailing list