drm/i915: Only reset the pinned kernel contexts on resume
authorChris Wilson <chris@chris-wilson.co.uk>
Wed, 10 Apr 2019 19:01:20 +0000 (20:01 +0100)
committerChris Wilson <chris@chris-wilson.co.uk>
Wed, 10 Apr 2019 20:18:11 +0000 (21:18 +0100)
On resume, we know that the only pinned contexts in danger of seeing
corruption are the kernel context, and so we do not need to walk the
list of all GEM contexts as we tracked them on each engine.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190410190120.830-1-chris@chris-wilson.co.uk
drivers/gpu/drm/i915/i915_drv.h
drivers/gpu/drm/i915/i915_gem.c
drivers/gpu/drm/i915/intel_context_types.h
drivers/gpu/drm/i915/intel_engine_cs.c
drivers/gpu/drm/i915/intel_lrc.c
drivers/gpu/drm/i915/intel_lrc.h
drivers/gpu/drm/i915/intel_ringbuffer.c
drivers/gpu/drm/i915/intel_ringbuffer.h

index 63eca3061d103755d82e7fdd540fbd5c19309d42..35d0782c077ece37de2f92f8889a44fabc7720ac 100644 (file)
@@ -1995,7 +1995,6 @@ struct drm_i915_private {
 
        /* Abstract the submission mechanism (legacy ringbuffer or execlists) away */
        struct {
-               void (*resume)(struct drm_i915_private *);
                void (*cleanup_engine)(struct intel_engine_cs *engine);
 
                struct i915_gt_timelines {
index bf3d12f94365ca2e6f26127be730b2909035b617..0a818a60ad31f8a192d3ad8313426038dfb50a91 100644 (file)
@@ -4513,7 +4513,7 @@ void i915_gem_resume(struct drm_i915_private *i915)
         * guarantee that the context image is complete. So let's just reset
         * it and start again.
         */
-       i915->gt.resume(i915);
+       intel_gt_resume(i915);
 
        if (i915_gem_init_hw(i915))
                goto err_wedged;
@@ -4853,13 +4853,10 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
 
        dev_priv->mm.unordered_timeline = dma_fence_context_alloc(1);
 
-       if (HAS_LOGICAL_RING_CONTEXTS(dev_priv)) {
-               dev_priv->gt.resume = intel_lr_context_resume;
+       if (HAS_LOGICAL_RING_CONTEXTS(dev_priv))
                dev_priv->gt.cleanup_engine = intel_logical_ring_cleanup;
-       } else {
-               dev_priv->gt.resume = intel_legacy_submission_resume;
+       else
                dev_priv->gt.cleanup_engine = intel_engine_cleanup;
-       }
 
        i915_timelines_init(dev_priv);
 
index 624729a3587514d3198b7f86c7b65cf5950dd7de..68b4ca1611e0cb68d3a88e7ecf848f32ea70fdc0 100644 (file)
@@ -24,6 +24,7 @@ struct intel_context_ops {
        int (*pin)(struct intel_context *ce);
        void (*unpin)(struct intel_context *ce);
 
+       void (*reset)(struct intel_context *ce);
        void (*destroy)(struct kref *kref);
 };
 
index d0427c2e39972482628137e4415ea00a3456ae9a..f29a667cad52a4608342ec9411c2bd877031af26 100644 (file)
@@ -753,6 +753,30 @@ err_unpin:
        return ret;
 }
 
+void intel_gt_resume(struct drm_i915_private *i915)
+{
+       struct intel_engine_cs *engine;
+       enum intel_engine_id id;
+
+       /*
+        * After resume, we may need to poke into the pinned kernel
+        * contexts to paper over any damage caused by the sudden suspend.
+        * Only the kernel contexts should remain pinned over suspend,
+        * allowing us to fixup the user contexts on their first pin.
+        */
+       for_each_engine(engine, i915, id) {
+               struct intel_context *ce;
+
+               ce = engine->kernel_context;
+               if (ce)
+                       ce->ops->reset(ce);
+
+               ce = engine->preempt_context;
+               if (ce)
+                       ce->ops->reset(ce);
+       }
+}
+
 /**
  * intel_engines_cleanup_common - cleans up the engine state created by
  *                                the common initiailizers.
index 6931dbb2888c46450f115adbeea727851376adaf..b54dbf36197eb276a911c8fbe84e8eb5a9bb5ccb 100644 (file)
@@ -1379,9 +1379,33 @@ static int execlists_context_pin(struct intel_context *ce)
        return __execlists_context_pin(ce, ce->engine);
 }
 
+static void execlists_context_reset(struct intel_context *ce)
+{
+       /*
+        * Because we emit WA_TAIL_DWORDS there may be a disparity
+        * between our bookkeeping in ce->ring->head and ce->ring->tail and
+        * that stored in context. As we only write new commands from
+        * ce->ring->tail onwards, everything before that is junk. If the GPU
+        * starts reading from its RING_HEAD from the context, it may try to
+        * execute that junk and die.
+        *
+        * The contexts that are stilled pinned on resume belong to the
+        * kernel, and are local to each engine. All other contexts will
+        * have their head/tail sanitized upon pinning before use, so they
+        * will never see garbage,
+        *
+        * So to avoid that we reset the context images upon resume. For
+        * simplicity, we just zero everything out.
+        */
+       intel_ring_reset(ce->ring, 0);
+       __execlists_update_reg_state(ce, ce->engine);
+}
+
 static const struct intel_context_ops execlists_context_ops = {
        .pin = execlists_context_pin,
        .unpin = execlists_context_unpin,
+
+       .reset = execlists_context_reset,
        .destroy = execlists_context_destroy,
 };
 
@@ -2895,31 +2919,6 @@ error_deref_obj:
        return ret;
 }
 
-void intel_lr_context_resume(struct drm_i915_private *i915)
-{
-       struct i915_gem_context *ctx;
-       struct intel_context *ce;
-
-       /*
-        * Because we emit WA_TAIL_DWORDS there may be a disparity
-        * between our bookkeeping in ce->ring->head and ce->ring->tail and
-        * that stored in context. As we only write new commands from
-        * ce->ring->tail onwards, everything before that is junk. If the GPU
-        * starts reading from its RING_HEAD from the context, it may try to
-        * execute that junk and die.
-        *
-        * So to avoid that we reset the context images upon resume. For
-        * simplicity, we just zero everything out.
-        */
-       list_for_each_entry(ctx, &i915->contexts.list, link) {
-               list_for_each_entry(ce, &ctx->active_engines, active_link) {
-                       GEM_BUG_ON(!ce->ring);
-                       intel_ring_reset(ce->ring, 0);
-                       __execlists_update_reg_state(ce, ce->engine);
-               }
-       }
-}
-
 void intel_execlists_show_requests(struct intel_engine_cs *engine,
                                   struct drm_printer *m,
                                   void (*show_request)(struct drm_printer *m,
index 92642ab914722ce67eebf4748af21f94fb6108de..4d0b7736cb6d7a5b4deebbcc010ff924c4679d87 100644 (file)
@@ -102,7 +102,6 @@ struct drm_printer;
 struct drm_i915_private;
 struct i915_gem_context;
 
-void intel_lr_context_resume(struct drm_i915_private *dev_priv);
 void intel_execlists_set_default_submission(struct intel_engine_cs *engine);
 
 void intel_execlists_show_requests(struct intel_engine_cs *engine,
index 8a19eee9c5d47181fa8515cf5dd905da2449241f..af35f99c59403e997b448b4f1301a894358079ab 100644 (file)
@@ -1508,9 +1508,16 @@ err_unpin:
        return err;
 }
 
+static void ring_context_reset(struct intel_context *ce)
+{
+       intel_ring_reset(ce->ring, 0);
+}
+
 static const struct intel_context_ops ring_context_ops = {
        .pin = ring_context_pin,
        .unpin = ring_context_unpin,
+
+       .reset = ring_context_reset,
        .destroy = ring_context_destroy,
 };
 
@@ -1581,16 +1588,6 @@ void intel_engine_cleanup(struct intel_engine_cs *engine)
        kfree(engine);
 }
 
-void intel_legacy_submission_resume(struct drm_i915_private *dev_priv)
-{
-       struct intel_engine_cs *engine;
-       enum intel_engine_id id;
-
-       /* Restart from the beginning of the rings for convenience */
-       for_each_engine(engine, dev_priv, id)
-               intel_ring_reset(engine->buffer, 0);
-}
-
 static int load_pd_dir(struct i915_request *rq,
                       const struct i915_hw_ppgtt *ppgtt)
 {
index e58d6f04177b7bc23fd6f74c183df6500b93f568..0dea6c7fd4384a976bc7d21cbeab3c5e5f828aff 100644 (file)
@@ -268,8 +268,6 @@ static inline void intel_ring_put(struct intel_ring *ring)
 void intel_engine_stop(struct intel_engine_cs *engine);
 void intel_engine_cleanup(struct intel_engine_cs *engine);
 
-void intel_legacy_submission_resume(struct drm_i915_private *dev_priv);
-
 int __must_check intel_ring_cacheline_align(struct i915_request *rq);
 
 u32 __must_check *intel_ring_begin(struct i915_request *rq, unsigned int n);
@@ -463,6 +461,7 @@ static inline void intel_engine_reset(struct intel_engine_cs *engine,
 }
 
 void intel_engines_sanitize(struct drm_i915_private *i915, bool force);
+void intel_gt_resume(struct drm_i915_private *i915);
 
 bool intel_engine_is_idle(struct intel_engine_cs *engine);
 bool intel_engines_are_idle(struct drm_i915_private *dev_priv);