Imre Deak <imre.deak@xxxxxxxxx> writes:
> From: Ben Widawsky <benjamin.widawsky@xxxxxxxxx>
>
> Consolidate the instdone logic so we can get a bit fancier. This patch also
> removes the duplicated print of INSTDONE[0].
>
> v2: (Imre)
> - Rebased on top of hangcheck INSTDONE changes.
> - Move all INSTDONE registers into a single struct, store it within the
> engine error struct during error capturing.
>
> Signed-off-by: Ben Widawsky <ben@xxxxxxxxxxxx>
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 41 +++++++++++-----
> drivers/gpu/drm/i915/i915_drv.h | 8 ++--
> drivers/gpu/drm/i915/i915_gpu_error.c | 84 +++++++++++++++++++++++----------
> drivers/gpu/drm/i915/i915_irq.c | 69 ++++++++++++++++++---------
> drivers/gpu/drm/i915/i915_reg.h | 1 -
> drivers/gpu/drm/i915/intel_ringbuffer.h | 10 +++-
> 6 files changed, 151 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 3fde507..45244f9 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1277,15 +1277,36 @@ out:
> return ret;
> }
>
> +static void i915_instdone_info(struct drm_i915_private *dev_priv,
> + struct seq_file *m,
> + struct intel_instdone *instdone)
> +{
> + seq_printf(m, "\t\tINSTDONE: 0x%08x\n",
> + instdone->instdone);
> +
> + if (INTEL_GEN(dev_priv) <= 3)
> + return;
> +
> + seq_printf(m, "\t\tSC_INSTDONE: 0x%08x\n",
> + instdone->slice_common);
> +
> + if (INTEL_GEN(dev_priv) <= 6)
> + return;
> +
> + seq_printf(m, "\t\tSAMPLER_INSTDONE: 0x%08x\n",
> + instdone->sampler);
> + seq_printf(m, "\t\tROW_INSTDONE: 0x%08x\n",
> + instdone->row);
> +}
> +
> 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];
> - u32 instdone[I915_NUM_INSTDONE_REG];
> + struct intel_instdone instdone;
> enum intel_engine_id id;
> - int j;
>
> if (!i915.enable_hangcheck) {
> seq_printf(m, "Hangcheck disabled\n");
> @@ -1299,7 +1320,7 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused)
> seqno[id] = intel_engine_get_seqno(engine);
> }
>
> - i915_get_extra_instdone(dev_priv, instdone);
> + i915_get_engine_instdone(dev_priv, RCS, &instdone);
>
> intel_runtime_pm_put(dev_priv);
>
> @@ -1327,18 +1348,14 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused)
> seq_printf(m, "\taction = %d\n", engine->hangcheck.action);
>
> if (engine->id == RCS) {
> - seq_puts(m, "\tinstdone read =");
> + seq_puts(m, "\tinstdone read =\n");
>
> - for (j = 0; j < I915_NUM_INSTDONE_REG; j++)
> - seq_printf(m, " 0x%08x", instdone[j]);
> + i915_instdone_info(dev_priv, m, &instdone);
>
> - seq_puts(m, "\n\tinstdone accu =");
> + seq_puts(m, "\tinstdone accu =\n");
>
> - for (j = 0; j < I915_NUM_INSTDONE_REG; j++)
> - seq_printf(m, " 0x%08x",
> - engine->hangcheck.instdone[j]);
> -
> - seq_puts(m, "\n");
> + i915_instdone_info(dev_priv, m,
> + &engine->hangcheck.instdone);
> }
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 757b1d1..20bf72e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -747,7 +747,7 @@ struct drm_i915_error_state {
> u32 gam_ecochk;
> u32 gab_ctl;
> u32 gfx_mode;
> - u32 extra_instdone[I915_NUM_INSTDONE_REG];
> +
> u64 fence[I915_MAX_NUM_FENCES];
> struct intel_overlay_error_state *overlay;
> struct intel_display_error_state *display;
> @@ -779,7 +779,6 @@ struct drm_i915_error_state {
> u32 hws;
> u32 ipeir;
> u32 ipehr;
> - u32 instdone;
> u32 bbstate;
> u32 instpm;
> u32 instps;
> @@ -790,6 +789,7 @@ struct drm_i915_error_state {
> u64 faddr;
> u32 rc_psmi; /* sleep state */
> u32 semaphore_mboxes[I915_NUM_ENGINES - 1];
> + struct intel_instdone instdone;
>
> struct drm_i915_error_object {
> int page_count;
> @@ -3560,7 +3560,9 @@ void i915_error_state_get(struct drm_device *dev,
> void i915_error_state_put(struct i915_error_state_file_priv *error_priv);
> void i915_destroy_error_state(struct drm_device *dev);
>
> -void i915_get_extra_instdone(struct drm_i915_private *dev_priv, uint32_t *instdone);
> +void i915_get_engine_instdone(struct drm_i915_private *dev_priv,
> + enum intel_engine_id engine_id,
> + struct intel_instdone *instdone);
> const char *i915_cache_level_str(struct drm_i915_private *i915, int type);
>
> /* i915_cmd_parser.c */
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index aed55e4..80fe101 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -228,6 +228,27 @@ static const char *hangcheck_action_to_str(enum intel_engine_hangcheck_action a)
> return "unknown";
> }
>
> +static void error_print_instdone(struct drm_i915_error_state_buf *m,
> + struct drm_i915_error_engine *ee)
> +{
> + err_printf(m, " INSTDONE: 0x%08x\n",
> + ee->instdone.instdone);
> +
> + if (ee->engine_id != RCS || INTEL_GEN(m->i915) <= 3)
> + return;
> +
> + err_printf(m, " SC_INSTDONE: 0x%08x\n",
> + ee->instdone.slice_common);
> +
> + if (INTEL_GEN(m->i915) <= 6)
> + return;
> +
> + err_printf(m, " SAMPLER_INSTDONE: 0x%08x\n",
> + ee->instdone.sampler);
> + err_printf(m, " ROW_INSTDONE: 0x%08x\n",
> + ee->instdone.row);
> +}
> +
> static void error_print_engine(struct drm_i915_error_state_buf *m,
> struct drm_i915_error_engine *ee)
> {
> @@ -242,7 +263,9 @@ static void error_print_engine(struct drm_i915_error_state_buf *m,
> (u32)(ee->acthd>>32), (u32)ee->acthd);
> err_printf(m, " IPEIR: 0x%08x\n", ee->ipeir);
> err_printf(m, " IPEHR: 0x%08x\n", ee->ipehr);
> - err_printf(m, " INSTDONE: 0x%08x\n", ee->instdone);
> +
> + error_print_instdone(m, ee);
> +
> if (ee->batchbuffer) {
> u64 start = ee->batchbuffer->gtt_offset;
> u64 end = start + ee->batchbuffer->gtt_size;
> @@ -402,10 +425,6 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
> for (i = 0; i < dev_priv->num_fence_regs; i++)
> err_printf(m, " fence[%d] = %08llx\n", i, error->fence[i]);
>
> - for (i = 0; i < ARRAY_SIZE(error->extra_instdone); i++)
> - err_printf(m, " INSTDONE_%d: 0x%08x\n", i,
> - error->extra_instdone[i]);
> -
> if (INTEL_INFO(dev)->gen >= 6) {
> err_printf(m, "ERROR: 0x%08x\n", error->error);
>
> @@ -851,7 +870,8 @@ static uint32_t i915_error_generate_code(struct drm_i915_private *dev_priv,
> if (engine_id)
> *engine_id = i;
>
> - return error->engine[i].ipehr ^ error->engine[i].instdone;
> + return error->engine[i].ipehr ^
> + error->engine[i].instdone.instdone;
> }
> }
>
> @@ -983,7 +1003,6 @@ static void error_record_engine_registers(struct drm_i915_error_state *error,
> ee->faddr = I915_READ(RING_DMA_FADD(engine->mmio_base));
> ee->ipeir = I915_READ(RING_IPEIR(engine->mmio_base));
> ee->ipehr = I915_READ(RING_IPEHR(engine->mmio_base));
> - ee->instdone = I915_READ(RING_INSTDONE(engine->mmio_base));
> ee->instps = I915_READ(RING_INSTPS(engine->mmio_base));
> ee->bbaddr = I915_READ(RING_BBADDR(engine->mmio_base));
> if (INTEL_GEN(dev_priv) >= 8) {
> @@ -995,9 +1014,10 @@ static void error_record_engine_registers(struct drm_i915_error_state *error,
> ee->faddr = I915_READ(DMA_FADD_I8XX);
> ee->ipeir = I915_READ(IPEIR);
> ee->ipehr = I915_READ(IPEHR);
> - ee->instdone = I915_READ(GEN2_INSTDONE);
> }
>
> + i915_get_engine_instdone(dev_priv, engine->id, &ee->instdone);
> +
> ee->waiting = intel_engine_has_waiter(engine);
> ee->instpm = I915_READ(RING_INSTPM(engine->mmio_base));
> ee->acthd = intel_engine_get_active_head(engine);
> @@ -1357,8 +1377,6 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
> }
> error->eir = I915_READ(EIR);
> error->pgtbl_er = I915_READ(PGTBL_ER);
> -
> - i915_get_extra_instdone(dev_priv, error->extra_instdone);
> }
>
> static void i915_error_capture_msg(struct drm_i915_private *dev_priv,
> @@ -1517,20 +1535,38 @@ const char *i915_cache_level_str(struct drm_i915_private *i915, int type)
> }
>
> /* NB: please notice the memset */
> -void i915_get_extra_instdone(struct drm_i915_private *dev_priv,
> - uint32_t *instdone)
> +void i915_get_engine_instdone(struct drm_i915_private *dev_priv,
> + enum intel_engine_id engine_id,
> + struct intel_instdone *instdone)
> {
> - memset(instdone, 0, sizeof(*instdone) * I915_NUM_INSTDONE_REG);
> -
> - if (IS_GEN2(dev_priv) || IS_GEN3(dev_priv))
> - instdone[0] = I915_READ(GEN2_INSTDONE);
> - else if (IS_GEN4(dev_priv) || IS_GEN5(dev_priv) || IS_GEN6(dev_priv)) {
> - instdone[0] = I915_READ(RING_INSTDONE(RENDER_RING_BASE));
> - instdone[1] = I915_READ(GEN4_INSTDONE1);
> - } else if (INTEL_GEN(dev_priv) >= 7) {
> - instdone[0] = I915_READ(RING_INSTDONE(RENDER_RING_BASE));
> - instdone[1] = I915_READ(GEN7_SC_INSTDONE);
> - instdone[2] = I915_READ(GEN7_SAMPLER_INSTDONE);
> - instdone[3] = I915_READ(GEN7_ROW_INSTDONE);
> + u32 mmio_base = dev_priv->engine[engine_id].mmio_base;
> +
> + memset(instdone, 0, sizeof(*instdone));
> +
> + switch (INTEL_GEN(dev_priv)) {
> + default:
> + instdone->instdone = I915_READ(RING_INSTDONE(mmio_base));
> +
> + if (engine_id != RCS)
> + break;
> +
> + instdone->slice_common = I915_READ(GEN7_SC_INSTDONE);
> + instdone->sampler = I915_READ(GEN7_SAMPLER_INSTDONE);
> + instdone->row = I915_READ(GEN7_ROW_INSTDONE);
> +
> + break;
> + case 6:
> + case 5:
> + case 4:
> + instdone->instdone = I915_READ(RING_INSTDONE(mmio_base));
> +
> + if (engine_id == RCS)
> + /* HACK: Using the wrong struct member */
> + instdone->slice_common = I915_READ(GEN4_INSTDONE1);
> + break;
> + case 3:
> + case 2:
> + instdone->instdone = I915_READ(GEN2_INSTDONE);
> + break;
> }
> }
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 82358d4..82cf823 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2550,18 +2550,36 @@ static void i915_reset_and_wakeup(struct drm_i915_private *dev_priv)
> }
> }
>
> +static inline void
> +i915_err_print_instdone(struct drm_i915_private *dev_priv,
> + struct intel_instdone *instdone)
> +{
> + pr_err(" INSTDONE: 0x%08x\n", instdone->instdone);
> +
> + if (INTEL_GEN(dev_priv) <= 3)
> + return;
> +
> + pr_err(" SC_INSTDONE: 0x%08x\n", instdone->slice_common);
> +
> + if (INTEL_GEN(dev_priv) <= 6)
> + return;
> +
> + pr_err(" SAMPLER_INSTDONE: 0x%08x\n", instdone->sampler);
> + pr_err(" ROW_INSTDONE: 0x%08x\n", instdone->row);
> +}
> +
> static void i915_report_and_clear_eir(struct drm_i915_private *dev_priv)
> {
> - uint32_t instdone[I915_NUM_INSTDONE_REG];
> + struct intel_instdone instdone;
> u32 eir = I915_READ(EIR);
> - int pipe, i;
> + int pipe;
>
> if (!eir)
> return;
>
> pr_err("render error detected, EIR: 0x%08x\n", eir);
>
> - i915_get_extra_instdone(dev_priv, instdone);
> + i915_get_engine_instdone(dev_priv, RCS, &instdone);
>
> if (IS_G4X(dev_priv)) {
> if (eir & (GM45_ERROR_MEM_PRIV | GM45_ERROR_CP_PRIV)) {
> @@ -2569,8 +2587,7 @@ static void i915_report_and_clear_eir(struct drm_i915_private *dev_priv)
>
> pr_err(" IPEIR: 0x%08x\n", I915_READ(IPEIR_I965));
> pr_err(" IPEHR: 0x%08x\n", I915_READ(IPEHR_I965));
> - for (i = 0; i < ARRAY_SIZE(instdone); i++)
> - pr_err(" INSTDONE_%d: 0x%08x\n", i, instdone[i]);
> + i915_err_print_instdone(dev_priv, &instdone);
> pr_err(" INSTPS: 0x%08x\n", I915_READ(INSTPS));
> pr_err(" ACTHD: 0x%08x\n", I915_READ(ACTHD_I965));
> I915_WRITE(IPEIR_I965, ipeir);
> @@ -2605,8 +2622,7 @@ static void i915_report_and_clear_eir(struct drm_i915_private *dev_priv)
> if (eir & I915_ERROR_INSTRUCTION) {
> pr_err("instruction error\n");
> pr_err(" INSTPM: 0x%08x\n", I915_READ(INSTPM));
> - for (i = 0; i < ARRAY_SIZE(instdone); i++)
> - pr_err(" INSTDONE_%d: 0x%08x\n", i, instdone[i]);
> + i915_err_print_instdone(dev_priv, &instdone);
> if (INTEL_GEN(dev_priv) < 4) {
> u32 ipeir = I915_READ(IPEIR);
>
> @@ -2949,31 +2965,42 @@ static void semaphore_clear_deadlocks(struct drm_i915_private *dev_priv)
> engine->hangcheck.deadlock = 0;
> }
>
> +static bool instdone_unchanged(u32 current_instdone, u32 *old_instdone)
> +{
> + u32 tmp = current_instdone | *old_instdone;
> + bool unchanged;
> +
> + unchanged = tmp == *old_instdone;
> + *old_instdone |= tmp;
> +
> + return unchanged;
> +}
> +
> static bool subunits_stuck(struct intel_engine_cs *engine)
> {
> - u32 instdone[I915_NUM_INSTDONE_REG];
> + struct drm_i915_private *dev_priv = engine->i915;
> + struct intel_instdone instdone;
> + struct intel_instdone *accu_instdone = &engine->hangcheck.instdone;
> bool stuck;
> - int i;
>
> if (engine->id != RCS)
> return true;
>
> - i915_get_extra_instdone(engine->i915, instdone);
> + i915_get_engine_instdone(dev_priv, RCS, &instdone);
>
> /* There might be unstable subunit states even when
> * actual head is not moving. Filter out the unstable ones by
> * accumulating the undone -> done transitions and only
> * consider those as progress.
> */
> - stuck = true;
> - for (i = 0; i < I915_NUM_INSTDONE_REG; i++) {
> - const u32 tmp = instdone[i] | engine->hangcheck.instdone[i];
> -
> - if (tmp != engine->hangcheck.instdone[i])
> - stuck = false;
> -
> - engine->hangcheck.instdone[i] |= tmp;
> - }
> + stuck = instdone_unchanged(instdone.instdone,
> + &accu_instdone->instdone);
> + stuck &= instdone_unchanged(instdone.slice_common,
> + &accu_instdone->slice_common);
> + stuck &= instdone_unchanged(instdone.sampler,
> + &accu_instdone->sampler);
> + stuck &= instdone_unchanged(instdone.row,
> + &accu_instdone->row);
>
> return stuck;
> }
> @@ -2984,7 +3011,7 @@ head_stuck(struct intel_engine_cs *engine, u64 acthd)
> if (acthd != engine->hangcheck.acthd) {
>
> /* Clear subunit states on head movement */
> - memset(engine->hangcheck.instdone, 0,
> + memset(&engine->hangcheck.instdone, 0,
> sizeof(engine->hangcheck.instdone));
>
> return HANGCHECK_ACTIVE;
> @@ -3158,7 +3185,7 @@ static void i915_hangcheck_elapsed(struct work_struct *work)
> /* Clear head and subunit states on seqno movement */
> acthd = 0;
>
> - memset(engine->hangcheck.instdone, 0,
> + memset(&engine->hangcheck.instdone, 0,
> sizeof(engine->hangcheck.instdone));
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a29d707..e84bc2e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1708,7 +1708,6 @@ enum skl_disp_power_wells {
> #define GEN7_SC_INSTDONE _MMIO(0x7100)
> #define GEN7_SAMPLER_INSTDONE _MMIO(0xe160)
> #define GEN7_ROW_INSTDONE _MMIO(0xe164)
> -#define I915_NUM_INSTDONE_REG 4
> #define RING_IPEIR(base) _MMIO((base)+0x64)
> #define RING_IPEHR(base) _MMIO((base)+0x68)
> /*
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 84aea54..afc3f1e 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -73,13 +73,21 @@ enum intel_engine_hangcheck_action {
>
> #define HANGCHECK_SCORE_RING_HUNG 31
>
> +struct intel_instdone {
> + u32 instdone;
> + /* The following exist only in the RCS engine */
> + u32 slice_common;
> + u32 sampler;
> + u32 row;
> +};
> +
> struct intel_engine_hangcheck {
> u64 acthd;
> u32 seqno;
> int score;
> enum intel_engine_hangcheck_action action;
> int deadlock;
> - u32 instdone[I915_NUM_INSTDONE_REG];
> + struct intel_instdone instdone;
> };
>
> struct intel_ring {
> --
> 2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
