On 08/04/16 15:37, Chris Wilson wrote:
On Fri, Apr 08, 2016 at 02:54:56PM +0100, Tvrtko Ursulin wrote:From: Tvrtko Ursulin <tvrtko@xxxxxxxxxxx> 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 contexts remain pinned in all cases until the are fully saved by the GPU. v2: Only track contexts. Signed-off-by: Tvrtko Ursulin <tvrtko@xxxxxxxxxxx> Cc: Alex Dai <yu.dai@xxxxxxxxx> Cc: Dave Gordon <david.s.gordon@xxxxxxxxx> Cc: Nick Hoath <nicholas.hoath@xxxxxxxxx> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> --- drivers/gpu/drm/i915/i915_drv.h | 4 +++- drivers/gpu/drm/i915/i915_gem.c | 5 +++++ drivers/gpu/drm/i915/intel_lrc.c | 17 +++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index fa199b7e51b5..216b2297ae3e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2283,6 +2283,9 @@ struct drm_i915_gem_request { struct intel_context *ctx; struct intel_ringbuffer *ringbuf; + /** Context preceding this one on the same engine. Can be NULL. */ + struct intel_context *prev_ctx; + /** Batch buffer related to this request if any (used for error state dump only) */ struct drm_i915_gem_object *batch_obj; @@ -2318,7 +2321,6 @@ struct drm_i915_gem_request { /** Execlists no. of times this request has been sent to the ELSP */ int elsp_submitted; - }; 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 c1df696f9002..118911ce7231 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1445,6 +1445,11 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request) */ request->ringbuf->last_retired_head = request->postfix; + if (request->prev_ctx) { + intel_lr_context_unpin(request->prev_ctx, request->engine); + request->prev_ctx = NULL; + } + __i915_gem_request_release(request); } diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 31445aa3429b..3d08dac0a9b0 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -798,6 +798,23 @@ intel_logical_ring_advance_and_submit(struct drm_i915_gem_request *request) if (intel_engine_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 (engine->last_context) { + if (engine->last_context != request->i915->kernel_context) { + intel_lr_context_pin(engine->last_context, engine); + request->prev_ctx = engine->last_context; + } + } + + /* + * 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);We can use pinned_context to reduce the code complexity, not make it worse. If you first apply the execlists default context fix, adding pinned_context is a reduction is code.
Okay I'll pull that patch in this series. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
