Chris Wilson <ch...@chris-wilson.co.uk> writes:
> 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.
>
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuopp...@linux.intel.com>
> ---
> 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 | 19 +++---------
> drivers/gpu/drm/i915/gt/intel_ringbuffer.c | 32 ++------------------
> drivers/gpu/drm/i915/i915_debugfs.c | 12 ++------
> 7 files changed, 17 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h
> b/drivers/gpu/drm/i915/gt/intel_engine.h
> index 06d785533502..9359b3a7ad9c 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 416d7e2e6f8c..4c3753c1b573 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 c0ab11b12e14..e381c1c73902 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 721ab74a382f..3a4d09b80fa0 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;
Change the enum also?
>
> 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 d1a54d2c3d5d..f1d62746e066 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;
>
> @@ -2297,14 +2291,11 @@ static u32 *gen8_emit_fini_breadcrumb_rcs(struct
> i915_request *request, u32 *cs)
> 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);
???
-Mika
> -
> - 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 6fbc5ddbc896..f0d60affdba3 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 b6094063be9b..cb9b56fb6a8a 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.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
