From 519a019491b833209b2920f0fe9db4aa22da6bbe Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 8 May 2019 09:06:25 +0100 Subject: [PATCH] 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 Cc: Mika Kuoppala Reviewed-by: Mika Kuoppala Link: https://patchwork.freedesktop.org/patch/msgid/20190508080704.24223-1-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/gt/intel_engine.h | 15 ------------- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 5 ++--- drivers/gpu/drm/i915/gt/intel_engine_types.h | 3 +-- drivers/gpu/drm/i915/gt/intel_hangcheck.c | 8 +++---- drivers/gpu/drm/i915/gt/intel_lrc.c | 20 ++++++----------- drivers/gpu/drm/i915/gt/intel_ringbuffer.c | 32 ++-------------------------- drivers/gpu/drm/i915/i915_debugfs.c | 12 +++-------- 7 files changed, 18 insertions(+), 77 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h index 06d7855..9359b3a 100644 --- 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 --- 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 --- 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 721ab74a..3a4d09b 100644 --- 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 --- 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 --- 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 --- 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)); -- 2.7.4