Commit f4e2dece authored by Tvrtko Ursulin's avatar Tvrtko Ursulin

drm/i915: Fix premature LRC unpin in GuC mode

In GuC mode LRC pinning lifetime depends exclusively on the
request liftime. Since that is terminated by the seqno update
that opens up a race condition between GPU finishing writing
out the context image and the driver unpinning the LRC.

To extend the LRC lifetime we will employ a similar approach
to what legacy ringbuffer submission does.

We will start tracking the last submitted context per engine
and keep it pinned until it is replaced by another one.

Note that the driver unload path is a bit fragile and could
benefit greatly from efforts to unify the legacy and exec
list submission code paths.

At the moment i915_gem_context_fini has special casing for the
two which are potentialy not needed, and also depends on
i915_gem_cleanup_ringbuffer running before itself.

v2:
 * Move pinning into engine->emit_request and actually fix
   the reference/unreference logic. (Chris Wilson)

 * ring->dev can be NULL on driver unload so use a different
   route towards it.

v3:
 * Rebase.
 * Handle the reset path. (Chris Wilson)
 * Exclude default context from the pinning - it is impossible
   to get it right before default context special casing in
   general is eliminated.

v4:
 * Rebased & moved context tracking to
   intel_logical_ring_advance_and_submit.
Signed-off-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Issue: VIZ-4277
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Nick Hoath <nicholas.hoath@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1453976997-25424-1-git-send-email-tvrtko.ursulin@linux.intel.com
parent a0b4a6a8
...@@ -324,9 +324,13 @@ i915_gem_create_context(struct drm_device *dev, ...@@ -324,9 +324,13 @@ i915_gem_create_context(struct drm_device *dev,
static void i915_gem_context_unpin(struct intel_context *ctx, static void i915_gem_context_unpin(struct intel_context *ctx,
struct intel_engine_cs *engine) struct intel_engine_cs *engine)
{ {
if (engine->id == RCS && ctx->legacy_hw_ctx.rcs_state) if (i915.enable_execlists) {
i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state); intel_lr_context_unpin(ctx, engine);
i915_gem_context_unreference(ctx); } else {
if (engine->id == RCS && ctx->legacy_hw_ctx.rcs_state)
i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
i915_gem_context_unreference(ctx);
}
} }
void i915_gem_context_reset(struct drm_device *dev) void i915_gem_context_reset(struct drm_device *dev)
......
...@@ -765,6 +765,7 @@ intel_logical_ring_advance_and_submit(struct drm_i915_gem_request *request) ...@@ -765,6 +765,7 @@ intel_logical_ring_advance_and_submit(struct drm_i915_gem_request *request)
{ {
struct intel_ringbuffer *ringbuf = request->ringbuf; struct intel_ringbuffer *ringbuf = request->ringbuf;
struct drm_i915_private *dev_priv = request->i915; struct drm_i915_private *dev_priv = request->i915;
struct intel_engine_cs *engine = request->ring;
intel_logical_ring_advance(ringbuf); intel_logical_ring_advance(ringbuf);
request->tail = ringbuf->tail; request->tail = ringbuf->tail;
...@@ -779,9 +780,20 @@ intel_logical_ring_advance_and_submit(struct drm_i915_gem_request *request) ...@@ -779,9 +780,20 @@ intel_logical_ring_advance_and_submit(struct drm_i915_gem_request *request)
intel_logical_ring_emit(ringbuf, MI_NOOP); intel_logical_ring_emit(ringbuf, MI_NOOP);
intel_logical_ring_advance(ringbuf); intel_logical_ring_advance(ringbuf);
if (intel_ring_stopped(request->ring)) if (intel_ring_stopped(engine))
return 0; return 0;
if (engine->last_context != request->ctx) {
if (engine->last_context)
intel_lr_context_unpin(engine->last_context, engine);
if (request->ctx != request->i915->kernel_context) {
intel_lr_context_pin(request->ctx, engine);
engine->last_context = request->ctx;
} else {
engine->last_context = NULL;
}
}
if (dev_priv->guc.execbuf_client) if (dev_priv->guc.execbuf_client)
i915_guc_submit(dev_priv->guc.execbuf_client, request); i915_guc_submit(dev_priv->guc.execbuf_client, request);
else else
...@@ -1131,7 +1143,7 @@ void intel_lr_context_unpin(struct intel_context *ctx, ...@@ -1131,7 +1143,7 @@ void intel_lr_context_unpin(struct intel_context *ctx,
{ {
struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state; struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state;
WARN_ON(!mutex_is_locked(&engine->dev->struct_mutex)); WARN_ON(!mutex_is_locked(&ctx->i915->dev->struct_mutex));
if (WARN_ON_ONCE(!ctx_obj)) if (WARN_ON_ONCE(!ctx_obj))
return; return;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment