drm/i915: Track the previous pinned context inside the request

As the contexts are accessed by the hardware until the switch is completed
to a new context, the hardware may still be writing to the context object
after the breadcrumb is visible. We must not unpin/unbind/prune that
object whilst still active and so we keep the previous context pinned until
the following request. We can generalise the tracking we already do via
the engine->last_context and move it to the request so that it works
equally for execlists and GuC.

v2: Drop the execlists double pin as that exposes a race inside the lrc
irq handler as it tries to access the context after it may be retired.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1461833819-3991-22-git-send-email-chris@chris-wilson.co.uk

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0bd9d17ff9744a04eb456e43432cc47352349edf..eb4a95b853ddfda5f668d06162027f72f9ca30b0 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2305,6 +2305,17 @@ struct drm_i915_gem_request {
        struct intel_context *ctx;
        struct intel_ringbuffer *ringbuf;
 
+       /**
+        * Context related to the previous request.
+        * As the contexts are accessed by the hardware until the switch is
+        * completed to a new context, the hardware may still be writing
+        * to the context object after the breadcrumb is visible. We must
+        * not unpin/unbind/prune that object whilst still active and so
+        * we keep the previous context pinned until the following (this)
+        * request is retired.
+        */
+       struct intel_context *previous_context;
+
        /** Batch buffer related to this request if any (used for
            error state dump only) */
        struct drm_i915_gem_object *batch_obj;

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e9bf45a3ffa2bff4e2b5104f7ea65e1f70712a58..72cd08419655910b3e1dcb3dfce041c26b250ccb 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1413,13 +1413,13 @@ 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->ctx) {
+       if (request->previous_context) {
                if (i915.enable_execlists)
-                       intel_lr_context_unpin(request->ctx, request->engine);
-
-               i915_gem_context_unreference(request->ctx);
+                       intel_lr_context_unpin(request->previous_context,
+                                              request->engine);
        }
 
+       i915_gem_context_unreference(request->ctx);
        i915_gem_request_unreference(request);
 }
 

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 3ab5caa8c165ce1b23b123f3c364761e3118fa89..b7a3e7e639378b2e4c8f2ee6b66f975ff190fd24 100644 (file)
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -771,12 +771,14 @@ intel_logical_ring_advance_and_submit(struct drm_i915_gem_request *request)
        if (intel_engine_stopped(engine))
                return 0;
 
-       if (engine->last_context != request->ctx) {
-               if (engine->last_context)
-                       intel_lr_context_unpin(engine->last_context, engine);
-               intel_lr_context_pin(request->ctx, engine);
-               engine->last_context = request->ctx;
-       }
+       /* We keep the previous context alive until we retire the following
+        * request. This ensures that any the context object is still pinned
+        * for any residual writes the HW makes into it on the context switch
+        * into the next object following the breadcrumb. Otherwise, we may
+        * retire the context too early.
+        */
+       request->previous_context = engine->last_context;
+       engine->last_context = request->ctx;
 
        if (dev_priv->guc.execbuf_client)
                i915_guc_submit(dev_priv->guc.execbuf_client, request);