drm/i915/hangcheck: Replace hangcheck.seqno with RING_HEAD

After realising we need to sample RING_START to detect context switches
from preemption events that do not allow for the seqno to advance, we
can also realise that the seqno itself is just a distance along the ring
and so can be replaced by sampling RING_HEAD.

v2: Bonus comment for the mystery separate CS_STALL before MI_USER_INTERRUPT

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190508080704.24223-1-chris@chris-wilson.co.uk

diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
index 06d7855..9359b3a 100644 (file)
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -215,8 +215,6 @@ intel_write_status_page(struct intel_engine_cs *engine, int reg, u32 value)
  */
 #define I915_GEM_HWS_PREEMPT           0x32
 #define I915_GEM_HWS_PREEMPT_ADDR      (I915_GEM_HWS_PREEMPT * sizeof(u32))
-#define I915_GEM_HWS_HANGCHECK         0x34
-#define I915_GEM_HWS_HANGCHECK_ADDR    (I915_GEM_HWS_HANGCHECK * sizeof(u32))
 #define I915_GEM_HWS_SEQNO             0x40
 #define I915_GEM_HWS_SEQNO_ADDR                (I915_GEM_HWS_SEQNO * sizeof(u32))
 #define I915_GEM_HWS_SCRATCH           0x80
@@ -548,17 +546,4 @@ static inline bool inject_preempt_hang(struct intel_engine_execlists *execlists)
 
 #endif
 
-static inline u32
-intel_engine_next_hangcheck_seqno(struct intel_engine_cs *engine)
-{
-       return engine->hangcheck.next_seqno =
-               next_pseudo_random32(engine->hangcheck.next_seqno);
-}
-
-static inline u32
-intel_engine_get_hangcheck_seqno(struct intel_engine_cs *engine)
-{
-       return intel_read_status_page(engine, I915_GEM_HWS_HANGCHECK);
-}
-
 #endif /* _INTEL_RINGBUFFER_H_ */

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 416d7e2..4c3753c 100644 (file)
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -721,6 +721,7 @@ static int measure_breadcrumb_dw(struct intel_engine_cs *engine)
                goto out_timeline;
 
        dw = engine->emit_fini_breadcrumb(&frame->rq, frame->cs) - frame->cs;
+       GEM_BUG_ON(dw & 1); /* RING_TAIL must be qword aligned */
 
        i915_timeline_unpin(&frame->timeline);
 
@@ -1444,9 +1445,7 @@ void intel_engine_dump(struct intel_engine_cs *engine,
                drm_printf(m, "*** WEDGED ***\n");
 
        drm_printf(m, "\tAwake? %d\n", atomic_read(&engine->wakeref.count));
-       drm_printf(m, "\tHangcheck %x:%x [%d ms]\n",
-                  engine->hangcheck.last_seqno,
-                  engine->hangcheck.next_seqno,
+       drm_printf(m, "\tHangcheck: %d ms ago\n",
                   jiffies_to_msecs(jiffies - engine->hangcheck.action_timestamp));
        drm_printf(m, "\tReset count: %d (global %d)\n",
                   i915_reset_engine_count(error, engine),

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index c0ab11b..e381c1c 100644 (file)
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -54,8 +54,7 @@ struct intel_instdone {
 struct intel_engine_hangcheck {
        u64 acthd;
        u32 last_ring;
-       u32 last_seqno;
-       u32 next_seqno;
+       u32 last_head;
        unsigned long action_timestamp;
        struct intel_instdone instdone;
 };

diff --git a/drivers/gpu/drm/i915/gt/intel_hangcheck.c b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
index 721ab74..3a4d09b 100644 (file)
--- a/drivers/gpu/drm/i915/gt/intel_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
@@ -28,7 +28,7 @@
 struct hangcheck {
        u64 acthd;
        u32 ring;
-       u32 seqno;
+       u32 head;
        enum intel_engine_hangcheck_action action;
        unsigned long action_timestamp;
        int deadlock;
@@ -134,16 +134,16 @@ static void hangcheck_load_sample(struct intel_engine_cs *engine,
                                  struct hangcheck *hc)
 {
        hc->acthd = intel_engine_get_active_head(engine);
-       hc->seqno = intel_engine_get_hangcheck_seqno(engine);
        hc->ring = ENGINE_READ(engine, RING_START);
+       hc->head = ENGINE_READ(engine, RING_HEAD);
 }
 
 static void hangcheck_store_sample(struct intel_engine_cs *engine,
                                   const struct hangcheck *hc)
 {
        engine->hangcheck.acthd = hc->acthd;
-       engine->hangcheck.last_seqno = hc->seqno;
        engine->hangcheck.last_ring = hc->ring;
+       engine->hangcheck.last_head = hc->head;
 }
 
 static enum intel_engine_hangcheck_action
@@ -156,7 +156,7 @@ hangcheck_get_action(struct intel_engine_cs *engine,
        if (engine->hangcheck.last_ring != hc->ring)
                return ENGINE_ACTIVE_SEQNO;
 
-       if (engine->hangcheck.last_seqno != hc->seqno)
+       if (engine->hangcheck.last_head != hc->head)
                return ENGINE_ACTIVE_SEQNO;
 
        return engine_stuck(engine, hc->acthd);

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index d1a54d2..e18623d 100644 (file)
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -2275,12 +2275,6 @@ static u32 *gen8_emit_fini_breadcrumb(struct i915_request *request, u32 *cs)
                                  request->timeline->hwsp_offset,
                                  0);
 
-       cs = gen8_emit_ggtt_write(cs,
-                                 intel_engine_next_hangcheck_seqno(request->engine),
-                                 I915_GEM_HWS_HANGCHECK_ADDR,
-                                 MI_FLUSH_DW_STORE_INDEX);
-
-
        *cs++ = MI_USER_INTERRUPT;
        *cs++ = MI_ARB_ON_OFF | MI_ARB_ENABLE;
 
@@ -2292,19 +2286,17 @@ static u32 *gen8_emit_fini_breadcrumb(struct i915_request *request, u32 *cs)
 
 static u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *request, u32 *cs)
 {
+       /* XXX flush+write+CS_STALL all in one upsets gem_concurrent_blt:kbl */
        cs = gen8_emit_ggtt_write_rcs(cs,
                                      request->fence.seqno,
                                      request->timeline->hwsp_offset,
                                      PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
                                      PIPE_CONTROL_DEPTH_CACHE_FLUSH |
-                                     PIPE_CONTROL_DC_FLUSH_ENABLE |
-                                     PIPE_CONTROL_FLUSH_ENABLE |
-                                     PIPE_CONTROL_CS_STALL);
-
-       cs = gen8_emit_ggtt_write_rcs(cs,
-                                     intel_engine_next_hangcheck_seqno(request->engine),
-                                     I915_GEM_HWS_HANGCHECK_ADDR,
-                                     PIPE_CONTROL_STORE_DATA_INDEX);
+                                     PIPE_CONTROL_DC_FLUSH_ENABLE);
+       cs = gen8_emit_pipe_control(cs,
+                                   PIPE_CONTROL_FLUSH_ENABLE |
+                                   PIPE_CONTROL_CS_STALL,
+                                   0);
 
        *cs++ = MI_USER_INTERRUPT;
        *cs++ = MI_ARB_ON_OFF | MI_ARB_ENABLE;

diff --git a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
index 6fbc5dd..f0d60af 100644 (file)
--- a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
@@ -309,11 +309,6 @@ static u32 *gen6_rcs_emit_breadcrumb(struct i915_request *rq, u32 *cs)
        *cs++ = rq->timeline->hwsp_offset | PIPE_CONTROL_GLOBAL_GTT;
        *cs++ = rq->fence.seqno;
 
-       *cs++ = GFX_OP_PIPE_CONTROL(4);
-       *cs++ = PIPE_CONTROL_QW_WRITE | PIPE_CONTROL_STORE_DATA_INDEX;
-       *cs++ = I915_GEM_HWS_HANGCHECK_ADDR | PIPE_CONTROL_GLOBAL_GTT;
-       *cs++ = intel_engine_next_hangcheck_seqno(rq->engine);
-
        *cs++ = MI_USER_INTERRUPT;
        *cs++ = MI_NOOP;
 
@@ -415,13 +410,6 @@ static u32 *gen7_rcs_emit_breadcrumb(struct i915_request *rq, u32 *cs)
        *cs++ = rq->timeline->hwsp_offset;
        *cs++ = rq->fence.seqno;
 
-       *cs++ = GFX_OP_PIPE_CONTROL(4);
-       *cs++ = (PIPE_CONTROL_QW_WRITE |
-                PIPE_CONTROL_STORE_DATA_INDEX |
-                PIPE_CONTROL_GLOBAL_GTT_IVB);
-       *cs++ = I915_GEM_HWS_HANGCHECK_ADDR;
-       *cs++ = intel_engine_next_hangcheck_seqno(rq->engine);
-
        *cs++ = MI_USER_INTERRUPT;
        *cs++ = MI_NOOP;
 
@@ -440,12 +428,7 @@ static u32 *gen6_xcs_emit_breadcrumb(struct i915_request *rq, u32 *cs)
        *cs++ = I915_GEM_HWS_SEQNO_ADDR | MI_FLUSH_DW_USE_GTT;
        *cs++ = rq->fence.seqno;
 
-       *cs++ = MI_FLUSH_DW | MI_FLUSH_DW_OP_STOREDW | MI_FLUSH_DW_STORE_INDEX;
-       *cs++ = I915_GEM_HWS_HANGCHECK_ADDR | MI_FLUSH_DW_USE_GTT;
-       *cs++ = intel_engine_next_hangcheck_seqno(rq->engine);
-
        *cs++ = MI_USER_INTERRUPT;
-       *cs++ = MI_NOOP;
 
        rq->tail = intel_ring_offset(rq, cs);
        assert_ring_tail_valid(rq->ring, rq->tail);
@@ -465,10 +448,6 @@ static u32 *gen7_xcs_emit_breadcrumb(struct i915_request *rq, u32 *cs)
        *cs++ = I915_GEM_HWS_SEQNO_ADDR | MI_FLUSH_DW_USE_GTT;
        *cs++ = rq->fence.seqno;
 
-       *cs++ = MI_FLUSH_DW | MI_FLUSH_DW_OP_STOREDW | MI_FLUSH_DW_STORE_INDEX;
-       *cs++ = I915_GEM_HWS_HANGCHECK_ADDR | MI_FLUSH_DW_USE_GTT;
-       *cs++ = intel_engine_next_hangcheck_seqno(rq->engine);
-
        for (i = 0; i < GEN7_XCS_WA; i++) {
                *cs++ = MI_STORE_DWORD_INDEX;
                *cs++ = I915_GEM_HWS_SEQNO_ADDR;
@@ -480,6 +459,7 @@ static u32 *gen7_xcs_emit_breadcrumb(struct i915_request *rq, u32 *cs)
        *cs++ = 0;
 
        *cs++ = MI_USER_INTERRUPT;
+       *cs++ = MI_NOOP;
 
        rq->tail = intel_ring_offset(rq, cs);
        assert_ring_tail_valid(rq->ring, rq->tail);
@@ -928,11 +908,8 @@ static u32 *i9xx_emit_breadcrumb(struct i915_request *rq, u32 *cs)
        *cs++ = I915_GEM_HWS_SEQNO_ADDR;
        *cs++ = rq->fence.seqno;
 
-       *cs++ = MI_STORE_DWORD_INDEX;
-       *cs++ = I915_GEM_HWS_HANGCHECK_ADDR;
-       *cs++ = intel_engine_next_hangcheck_seqno(rq->engine);
-
        *cs++ = MI_USER_INTERRUPT;
+       *cs++ = MI_NOOP;
 
        rq->tail = intel_ring_offset(rq, cs);
        assert_ring_tail_valid(rq->ring, rq->tail);
@@ -950,10 +927,6 @@ static u32 *gen5_emit_breadcrumb(struct i915_request *rq, u32 *cs)
 
        *cs++ = MI_FLUSH;
 
-       *cs++ = MI_STORE_DWORD_INDEX;
-       *cs++ = I915_GEM_HWS_HANGCHECK_ADDR;
-       *cs++ = intel_engine_next_hangcheck_seqno(rq->engine);
-
        BUILD_BUG_ON(GEN5_WA_STORES < 1);
        for (i = 0; i < GEN5_WA_STORES; i++) {
                *cs++ = MI_STORE_DWORD_INDEX;
@@ -962,7 +935,6 @@ static u32 *gen5_emit_breadcrumb(struct i915_request *rq, u32 *cs)
        }
 
        *cs++ = MI_USER_INTERRUPT;
-       *cs++ = MI_NOOP;
 
        rq->tail = intel_ring_offset(rq, cs);
        assert_ring_tail_valid(rq->ring, rq->tail);

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index b609406..cb9b56f 100644 (file)
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1288,7 +1288,6 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused)
        struct drm_i915_private *dev_priv = node_to_i915(m->private);
        struct intel_engine_cs *engine;
        u64 acthd[I915_NUM_ENGINES];
-       u32 seqno[I915_NUM_ENGINES];
        struct intel_instdone instdone;
        intel_wakeref_t wakeref;
        enum intel_engine_id id;
@@ -1305,10 +1304,8 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused)
        }
 
        with_intel_runtime_pm(dev_priv, wakeref) {
-               for_each_engine(engine, dev_priv, id) {
+               for_each_engine(engine, dev_priv, id)
                        acthd[id] = intel_engine_get_active_head(engine);
-                       seqno[id] = intel_engine_get_hangcheck_seqno(engine);
-               }
 
                intel_engine_get_instdone(dev_priv->engine[RCS0], &instdone);
        }
@@ -1325,11 +1322,8 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused)
        seq_printf(m, "GT active? %s\n", yesno(dev_priv->gt.awake));
 
        for_each_engine(engine, dev_priv, id) {
-               seq_printf(m, "%s:\n", engine->name);
-               seq_printf(m, "\tseqno = %x [current %x, last %x], %dms ago\n",
-                          engine->hangcheck.last_seqno,
-                          seqno[id],
-                          engine->hangcheck.next_seqno,
+               seq_printf(m, "%s: %d ms ago\n",
+                          engine->name,
                           jiffies_to_msecs(jiffies -
                                            engine->hangcheck.action_timestamp));