[RFC] drm/i915/guc: Keep the previous context pinned until the next one has been completed

* [RFC] drm/i915/guc: Keep the previous context pinned until the next one has been completed
@ 2016-02-11 10:39 Tvrtko Ursulin
  2016-02-15 15:08 ` ✗ Fi.CI.BAT: failure for " Patchwork
  0 siblings, 1 reply; 2+ messages in thread
From: Tvrtko Ursulin @ 2016-02-11 10:39 UTC (permalink / raw)
  To: Intel-gfx; +Cc: Tvrtko Ursulin

From: Tvrtko Ursulin <tvrtko@ursulin.net>

In GuC mode submitting requests is only putting them into the
GuC queue with the actual submission to hardware following at
some future point. This makes the per engine last context
tracking insufficient for closing the premature context unpin
race.

Instead we need to make requests track and pin the previous
context on the same engine, and only unpin them when they
themselves are retired.

This will ensure context remain pinned until the are fully
saved by the GPU in all cases.

This patch adds overhead to the request retire path which
could be avoided by refactoring LRC pinning to make it track
the VMA and cache the kmap in the VMA as suggested by Chris
Wilson.

It is also not tested in GuC mode at all so just speculative.

Signed-off-by: Tvrtko Ursulin <tvrtko@ursulin.net>
Cc: Alex Dai <yu.dai@intel.com>
Cc: Dave Gordon <david.s.gordon@intel.com>
Cc: Nick Hoath <nicholas.hoath@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h  |  6 ++++++
 drivers/gpu/drm/i915/i915_gem.c  |  5 +++++
 drivers/gpu/drm/i915/intel_lrc.c | 15 +++++++++++++++
 3 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e11eef1e5134..090172de81a0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2313,6 +2313,12 @@ struct drm_i915_gem_request {
 	/** Execlists no. of times this request has been sent to the ELSP */
 	int elsp_submitted;
 
+	/**
+	 * Context of the request precedeing this one on the same engine.
+	 * Can be NULL.
+	 */
+	struct intel_context *previous_ctx;
+
 };
 
 struct drm_i915_gem_request * __must_check
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ef53bb314639..c63c3072a8a9 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1428,6 +1428,11 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
 	list_del_init(&request->list);
 	i915_gem_request_remove_from_client(request);
 
+	if (request->previous_ctx) {
+		intel_lr_context_unpin(request->previous_ctx, request->ring);
+		request->previous_ctx = NULL;
+	}
+
 	i915_gem_request_unreference(request);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 3a03646e343d..89eb892df4ae 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -783,6 +783,21 @@ intel_logical_ring_advance_and_submit(struct drm_i915_gem_request *request)
 	if (intel_ring_stopped(engine))
 		return 0;
 
+	/*
+	 * Pin the previous context on this engine to ensure it is not
+	 * prematurely unpinned in GuC mode.
+	 * Previous context will be unpinned when this request is retired,
+	 * ensuring the GPU has switched out from the previous context and into
+	 * a new context at that point.
+	 */
+	if (i915.enable_guc_submission && engine->last_context) {
+		request->previous_ctx = engine->last_context;
+		intel_lr_context_pin(request->previous_ctx, engine);
+	}
+
+	/*
+	 * Track and pin the last context submitted on an engine.
+	 */
 	if (engine->last_context != request->ctx) {
 		if (engine->last_context)
 			intel_lr_context_unpin(engine->last_context, engine);
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 2+ messages in thread