Commit 0f36a85c3bd5 ("drm/i915: Flush pending interrupt following a GPU
reset") got confused and only applied the flush to the set-wedge path
(which itself is proving troublesome), but we also need the
serialisation on the regular reset path. Oops.

Move the interrupt into reset_irq() and make it common to the reset and
final set-wedge.

References: 0f36a85c3bd5 ("drm/i915: Flush pending interrupt following a GPU 
reset")
Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuopp...@linux.intel.com>
Cc: Michel Thierry <michel.thie...@intel.com>
Cc: Michał Winiarski <michal.winiar...@intel.com>
Cc: Jeff McGee <jeff.mc...@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 105 ++++++++++++++++++++-------------------
 1 file changed, 53 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index ce09c5ad334f..2e53882b826b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -740,6 +740,57 @@ execlists_cancel_port_requests(struct 
intel_engine_execlists * const execlists)
        }
 }
 
+static void clear_gtiir(struct intel_engine_cs *engine)
+{
+       static const u8 gtiir[] = {
+               [RCS] = 0,
+               [BCS] = 0,
+               [VCS] = 1,
+               [VCS2] = 1,
+               [VECS] = 3,
+       };
+       struct drm_i915_private *dev_priv = engine->i915;
+       int i;
+
+       /* TODO: correctly reset irqs for gen11 */
+       if (WARN_ON_ONCE(INTEL_GEN(engine->i915) >= 11))
+               return;
+
+       GEM_BUG_ON(engine->id >= ARRAY_SIZE(gtiir));
+
+       /*
+        * Clear any pending interrupt state.
+        *
+        * We do it twice out of paranoia that some of the IIR are
+        * double buffered, and so if we only reset it once there may
+        * still be an interrupt pending.
+        */
+       for (i = 0; i < 2; i++) {
+               I915_WRITE(GEN8_GT_IIR(gtiir[engine->id]),
+                          engine->irq_keep_mask);
+               POSTING_READ(GEN8_GT_IIR(gtiir[engine->id]));
+       }
+       GEM_BUG_ON(I915_READ(GEN8_GT_IIR(gtiir[engine->id])) &
+                  engine->irq_keep_mask);
+}
+
+static void reset_irq(struct intel_engine_cs *engine)
+{
+       /* Mark all CS interrupts as complete */
+       smp_store_mb(engine->execlists.active, 0);
+       synchronize_hardirq(engine->i915->drm.irq);
+
+       clear_gtiir(engine);
+
+       /*
+        * The port is checked prior to scheduling a tasklet, but
+        * just in case we have suspended the tasklet to do the
+        * wedging make sure that when it wakes, it decides there
+        * is no work to do by clearing the irq_posted bit.
+        */
+       clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
+}
+
 static void execlists_cancel_requests(struct intel_engine_cs *engine)
 {
        struct intel_engine_execlists * const execlists = &engine->execlists;
@@ -765,6 +816,8 @@ static void execlists_cancel_requests(struct 
intel_engine_cs *engine)
         */
        local_irq_save(flags);
 
+       reset_irq(engine);
+
        /* Cancel the requests on the HW and clear the ELSP tracker. */
        execlists_cancel_port_requests(execlists);
 
@@ -805,18 +858,6 @@ static void execlists_cancel_requests(struct 
intel_engine_cs *engine)
 
        spin_unlock(&engine->timeline->lock);
 
-       /* Mark all CS interrupts as complete */
-       smp_store_mb(execlists->active, 0);
-       synchronize_hardirq(engine->i915->drm.irq);
-
-       /*
-        * The port is checked prior to scheduling a tasklet, but
-        * just in case we have suspended the tasklet to do the
-        * wedging make sure that when it wakes, it decides there
-        * is no work to do by clearing the irq_posted bit.
-        */
-       clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
-
        local_irq_restore(flags);
 }
 
@@ -1566,14 +1607,6 @@ static int intel_init_workaround_bb(struct 
intel_engine_cs *engine)
        return ret;
 }
 
-static u8 gtiir[] = {
-       [RCS] = 0,
-       [BCS] = 0,
-       [VCS] = 1,
-       [VCS2] = 1,
-       [VECS] = 3,
-};
-
 static void enable_execlists(struct intel_engine_cs *engine)
 {
        struct drm_i915_private *dev_priv = engine->i915;
@@ -1657,35 +1690,6 @@ static int gen9_init_render_ring(struct intel_engine_cs 
*engine)
        return init_workarounds_ring(engine);
 }
 
-static void reset_irq(struct intel_engine_cs *engine)
-{
-       struct drm_i915_private *dev_priv = engine->i915;
-       int i;
-
-       /* TODO: correctly reset irqs for gen11 */
-       if (WARN_ON_ONCE(INTEL_GEN(engine->i915) >= 11))
-               return;
-
-       GEM_BUG_ON(engine->id >= ARRAY_SIZE(gtiir));
-
-       /*
-        * Clear any pending interrupt state.
-        *
-        * We do it twice out of paranoia that some of the IIR are double
-        * buffered, and if we only reset it once there may still be
-        * an interrupt pending.
-        */
-       for (i = 0; i < 2; i++) {
-               I915_WRITE(GEN8_GT_IIR(gtiir[engine->id]),
-                          engine->irq_keep_mask);
-               POSTING_READ(GEN8_GT_IIR(gtiir[engine->id]));
-       }
-       GEM_BUG_ON(I915_READ(GEN8_GT_IIR(gtiir[engine->id])) &
-                  engine->irq_keep_mask);
-
-       clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
-}
-
 static void reset_common_ring(struct intel_engine_cs *engine,
                              struct i915_request *request)
 {
@@ -1717,9 +1721,6 @@ static void reset_common_ring(struct intel_engine_cs 
*engine,
        __unwind_incomplete_requests(engine);
        spin_unlock(&engine->timeline->lock);
 
-       /* Mark all CS interrupts as complete */
-       execlists->active = 0;
-
        local_irq_restore(flags);
 
        /*
-- 
2.16.2

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

Reply via email to