* Re: [PATCH v2 1/2] drm/i915: Cleanup instdone collection
[not found] ` <1473087178-4847-2-git-send-email-imre.deak@intel.com>
@ 2016-09-06 11:21 ` Mika Kuoppala
2016-09-16 9:56 ` Jani Nikula
0 siblings, 1 reply; 7+ messages in thread
From: Mika Kuoppala @ 2016-09-06 11:21 UTC (permalink / raw)
To: Imre Deak, intel-gfx; +Cc: Ben Widawsky
Imre Deak <imre.deak@intel.com> writes:
> From: Ben Widawsky <benjamin.widawsky@intel.com>
>
> 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@bwidawsk.net>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
> 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] drm/i915: Try to print INSTDONE bits for all slice/subslice
[not found] ` <1473087178-4847-3-git-send-email-imre.deak@intel.com>
@ 2016-09-15 14:30 ` Mika Kuoppala
2016-09-15 16:10 ` Imre Deak
2016-09-26 19:09 ` Ben Widawsky
2016-09-20 13:09 ` Mika Kuoppala
1 sibling, 2 replies; 7+ messages in thread
From: Mika Kuoppala @ 2016-09-15 14:30 UTC (permalink / raw)
To: Imre Deak, intel-gfx; +Cc: Ben Widawsky
Imre Deak <imre.deak@intel.com> writes:
> From: Ben Widawsky <benjamin.widawsky@intel.com>
>
> v2: (Imre)
> - Access only subslices that are known to exist.
> - Reset explictly the MCR selector to slice/sub-slice ID 0 after the
> readout.
> - Use the subslice INSTDONE bits for the hangcheck/subunits-stuck
> detection too.
> - Take the uncore lock for the MCR-select/subslice-readout sequence.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 14 ++++--
> drivers/gpu/drm/i915/i915_gpu_error.c | 76 ++++++++++++++++++++++++++++++---
> drivers/gpu/drm/i915/i915_irq.c | 25 ++++++++---
> drivers/gpu/drm/i915/i915_reg.h | 5 +++
> drivers/gpu/drm/i915/intel_ringbuffer.h | 23 +++++++++-
> 5 files changed, 125 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 45244f9..0f84165 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1281,6 +1281,9 @@ static void i915_instdone_info(struct drm_i915_private *dev_priv,
> struct seq_file *m,
> struct intel_instdone *instdone)
> {
> + int slice;
> + int subslice;
> +
> seq_printf(m, "\t\tINSTDONE: 0x%08x\n",
> instdone->instdone);
>
> @@ -1293,10 +1296,13 @@ static void i915_instdone_info(struct drm_i915_private *dev_priv,
> 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);
> + for_each_instdone_slice_subslice(dev_priv, slice, subslice)
> + seq_printf(m, "\t\tSAMPLER_INSTDONE[%d][%d]: 0x%08x\n",
> + slice, subslice, instdone->sampler[slice][subslice]);
> +
> + for_each_instdone_slice_subslice(dev_priv, slice, subslice)
> + seq_printf(m, "\t\tROW_INSTDONE[%d][%d]: 0x%08x\n",
> + slice, subslice, instdone->row[slice][subslice]);
> }
>
> static int i915_hangcheck_info(struct seq_file *m, void *unused)
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 80fe101..06d4309 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -231,6 +231,9 @@ static const char *hangcheck_action_to_str(enum intel_engine_hangcheck_action a)
> static void error_print_instdone(struct drm_i915_error_state_buf *m,
> struct drm_i915_error_engine *ee)
> {
> + int slice;
> + int subslice;
> +
> err_printf(m, " INSTDONE: 0x%08x\n",
> ee->instdone.instdone);
>
> @@ -243,10 +246,15 @@ static void error_print_instdone(struct drm_i915_error_state_buf *m,
> 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);
> + for_each_instdone_slice_subslice(m->i915, slice, subslice)
> + err_printf(m, " SAMPLER_INSTDONE[%d][%d]: 0x%08x\n",
> + slice, subslice,
> + ee->instdone.sampler[slice][subslice]);
> +
> + for_each_instdone_slice_subslice(m->i915, slice, subslice)
> + err_printf(m, " ROW_INSTDONE[%d][%d]: 0x%08x\n",
> + slice, subslice,
> + ee->instdone.row[slice][subslice]);
> }
>
> static void error_print_engine(struct drm_i915_error_state_buf *m,
> @@ -1534,12 +1542,52 @@ const char *i915_cache_level_str(struct drm_i915_private *i915, int type)
> }
> }
>
> +static inline uint32_t
> +read_subslice_reg(struct drm_i915_private *dev_priv, int slice,
> + int subslice, i915_reg_t reg)
> +{
> + uint32_t mcr;
> + uint32_t ret;
> + enum forcewake_domains fw_domains;
> +
> + fw_domains = intel_uncore_forcewake_for_reg(dev_priv, reg,
> + FW_REG_READ);
> + fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
> + GEN8_MCR_SELECTOR,
> + FW_REG_READ | FW_REG_WRITE);
> +
> + spin_lock_irq(&dev_priv->uncore.lock);
> + intel_uncore_forcewake_get__locked(dev_priv, fw_domains);
> +
> + mcr = I915_READ_FW(GEN8_MCR_SELECTOR);
> + /*
> + * The HW expects the slice and sublice selectors to be reset to 0
> + * after reading out the registers.
> + */
> + WARN_ON_ONCE(mcr & (GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK));
> + mcr &= ~(GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK);
> + mcr |= GEN8_MCR_SLICE(slice) | GEN8_MCR_SUBSLICE(subslice);
> + I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr);
> +
> + ret = I915_READ_FW(reg);
> +
> + mcr &= ~(GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK);
> + I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr);
> +
> + intel_uncore_forcewake_put__locked(dev_priv, fw_domains);
> + spin_unlock_irq(&dev_priv->uncore.lock);
> +
> + return ret;
> +}
> +
> /* NB: please notice the memset */
> void i915_get_engine_instdone(struct drm_i915_private *dev_priv,
> enum intel_engine_id engine_id,
> struct intel_instdone *instdone)
> {
> u32 mmio_base = dev_priv->engine[engine_id].mmio_base;
> + int slice;
> + int subslice;
>
> memset(instdone, 0, sizeof(*instdone));
>
> @@ -1551,8 +1599,24 @@ void i915_get_engine_instdone(struct drm_i915_private *dev_priv,
> break;
>
> instdone->slice_common = I915_READ(GEN7_SC_INSTDONE);
> - instdone->sampler = I915_READ(GEN7_SAMPLER_INSTDONE);
> - instdone->row = I915_READ(GEN7_ROW_INSTDONE);
> + for_each_instdone_slice_subslice(dev_priv, slice, subslice) {
> + instdone->sampler[slice][subslice] =
> + read_subslice_reg(dev_priv, slice, subslice,
> + GEN7_SAMPLER_INSTDONE);
> + instdone->row[slice][subslice] =
> + read_subslice_reg(dev_priv, slice, subslice,
> + GEN7_ROW_INSTDONE);
> + }
> + break;
> + case 7:
> + instdone->instdone = I915_READ(RING_INSTDONE(mmio_base));
> +
> + if (engine_id != RCS)
> + break;
> +
> + instdone->slice_common = I915_READ(GEN7_SC_INSTDONE);
> + instdone->sampler[0][0] = I915_READ(GEN7_SAMPLER_INSTDONE);
> + instdone->row[0][0] = I915_READ(GEN7_ROW_INSTDONE);
>
> break;
> case 6:
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 82cf823..d44f4d8 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2554,6 +2554,9 @@ static inline void
> i915_err_print_instdone(struct drm_i915_private *dev_priv,
> struct intel_instdone *instdone)
> {
> + int slice;
> + int subslice;
> +
> pr_err(" INSTDONE: 0x%08x\n", instdone->instdone);
>
> if (INTEL_GEN(dev_priv) <= 3)
> @@ -2564,8 +2567,13 @@ i915_err_print_instdone(struct drm_i915_private *dev_priv,
> 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);
> + for_each_instdone_slice_subslice(dev_priv, slice, subslice)
> + pr_err(" SAMPLER_INSTDONE[%d][%d]: 0x%08x\n",
> + slice, subslice, instdone->sampler[slice][subslice]);
> +
> + for_each_instdone_slice_subslice(dev_priv, slice, subslice)
> + pr_err(" ROW_INSTDONE[%d][%d]: 0x%08x\n",
> + slice, subslice, instdone->row[slice][subslice]);
> }
>
> static void i915_report_and_clear_eir(struct drm_i915_private *dev_priv)
> @@ -2982,6 +2990,8 @@ static bool subunits_stuck(struct intel_engine_cs *engine)
> struct intel_instdone instdone;
> struct intel_instdone *accu_instdone = &engine->hangcheck.instdone;
> bool stuck;
> + int slice;
> + int subslice;
>
> if (engine->id != RCS)
> return true;
> @@ -2997,10 +3007,13 @@ static bool subunits_stuck(struct intel_engine_cs *engine)
> &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);
> +
> + for_each_instdone_slice_subslice(dev_priv, slice, subslice) {
> + stuck &= instdone_unchanged(instdone.sampler[slice][subslice],
> + &accu_instdone->sampler[slice][subslice]);
> + stuck &= instdone_unchanged(instdone.row[slice][subslice],
> + &accu_instdone->row[slice][subslice]);
> + }
>
> return stuck;
> }
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index e84bc2e..24e741d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1708,6 +1708,11 @@ enum skl_disp_power_wells {
> #define GEN7_SC_INSTDONE _MMIO(0x7100)
> #define GEN7_SAMPLER_INSTDONE _MMIO(0xe160)
> #define GEN7_ROW_INSTDONE _MMIO(0xe164)
> +#define GEN8_MCR_SELECTOR _MMIO(0xfdc)
> +#define GEN8_MCR_SLICE(slice) (((slice) & 3) << 26)
> +#define GEN8_MCR_SLICE_MASK GEN8_MCR_SLICE(3)
> +#define GEN8_MCR_SUBSLICE(subslice) (((subslice) & 3) << 24)
There is some conflicting info about these changing on gen9 to widen
and shift the masks. So I am confused.
Can we easily test if we get sane slice/subslice states?
-Mika
> +#define GEN8_MCR_SUBSLICE_MASK GEN8_MCR_SUBSLICE(3)
> #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 afc3f1e..983d7da 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -73,12 +73,31 @@ enum intel_engine_hangcheck_action {
>
> #define HANGCHECK_SCORE_RING_HUNG 31
>
> +#define I915_MAX_SLICES 3
> +#define I915_MAX_SUBSLICES 3
> +
> +#define instdone_slice_mask(dev_priv__) \
> + (INTEL_GEN(dev_priv__) == 7 ? \
> + 1 : INTEL_INFO(dev_priv__)->sseu.slice_mask)
> +
> +#define instdone_subslice_mask(dev_priv__) \
> + (INTEL_GEN(dev_priv__) == 7 ? \
> + 1 : INTEL_INFO(dev_priv__)->sseu.subslice_mask)
> +
> +#define for_each_instdone_slice_subslice(dev_priv__, slice__, subslice__) \
> + for ((slice__) = 0, (subslice__) = 0; \
> + (slice__) < I915_MAX_SLICES; \
> + (subslice__) = ((subslice__) + 1) < I915_MAX_SUBSLICES ? (subslice__) + 1 : 0, \
> + (slice__) += ((subslice__) == 0)) \
> + for_each_if((BIT(slice__) & instdone_slice_mask(dev_priv__)) && \
> + (BIT(subslice__) & instdone_subslice_mask(dev_priv__)))
> +
> struct intel_instdone {
> u32 instdone;
> /* The following exist only in the RCS engine */
> u32 slice_common;
> - u32 sampler;
> - u32 row;
> + u32 sampler[I915_MAX_SLICES][I915_MAX_SUBSLICES];
> + u32 row[I915_MAX_SLICES][I915_MAX_SUBSLICES];
> };
>
> struct intel_engine_hangcheck {
> --
> 2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] drm/i915: Try to print INSTDONE bits for all slice/subslice
2016-09-15 14:30 ` [PATCH v2 2/2] drm/i915: Try to print INSTDONE bits for all slice/subslice Mika Kuoppala
@ 2016-09-15 16:10 ` Imre Deak
2016-09-26 19:09 ` Ben Widawsky
1 sibling, 0 replies; 7+ messages in thread
From: Imre Deak @ 2016-09-15 16:10 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx; +Cc: Ben Widawsky
On Thu, 2016-09-15 at 17:30 +0300, Mika Kuoppala wrote:
> Imre Deak <imre.deak@intel.com> writes:
>
> > From: Ben Widawsky <benjamin.widawsky@intel.com>
> >
> > v2: (Imre)
> > - Access only subslices that are known to exist.
> > - Reset explictly the MCR selector to slice/sub-slice ID 0 after the
> > readout.
> > - Use the subslice INSTDONE bits for the hangcheck/subunits-stuck
> > detection too.
> > - Take the uncore lock for the MCR-select/subslice-readout sequence.
> >
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_debugfs.c | 14 ++++--
> > drivers/gpu/drm/i915/i915_gpu_error.c | 76 ++++++++++++++++++++++++++++++---
> > drivers/gpu/drm/i915/i915_irq.c | 25 ++++++++---
> > drivers/gpu/drm/i915/i915_reg.h | 5 +++
> > drivers/gpu/drm/i915/intel_ringbuffer.h | 23 +++++++++-
> > 5 files changed, 125 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 45244f9..0f84165 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -1281,6 +1281,9 @@ static void i915_instdone_info(struct drm_i915_private *dev_priv,
> > struct seq_file *m,
> > struct intel_instdone *instdone)
> > {
> > + int slice;
> > + int subslice;
> > +
> > seq_printf(m, "\t\tINSTDONE: 0x%08x\n",
> > instdone->instdone);
> >
> > @@ -1293,10 +1296,13 @@ static void i915_instdone_info(struct drm_i915_private *dev_priv,
> > 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);
> > + for_each_instdone_slice_subslice(dev_priv, slice, subslice)
> > + seq_printf(m, "\t\tSAMPLER_INSTDONE[%d][%d]: 0x%08x\n",
> > + slice, subslice, instdone->sampler[slice][subslice]);
> > +
> > + for_each_instdone_slice_subslice(dev_priv, slice, subslice)
> > + seq_printf(m, "\t\tROW_INSTDONE[%d][%d]: 0x%08x\n",
> > + slice, subslice, instdone->row[slice][subslice]);
> > }
> >
> > static int i915_hangcheck_info(struct seq_file *m, void *unused)
> > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> > index 80fe101..06d4309 100644
> > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > @@ -231,6 +231,9 @@ static const char *hangcheck_action_to_str(enum intel_engine_hangcheck_action a)
> > static void error_print_instdone(struct drm_i915_error_state_buf *m,
> > struct drm_i915_error_engine *ee)
> > {
> > + int slice;
> > + int subslice;
> > +
> > err_printf(m, " INSTDONE: 0x%08x\n",
> > ee->instdone.instdone);
> >
> > @@ -243,10 +246,15 @@ static void error_print_instdone(struct drm_i915_error_state_buf *m,
> > 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);
> > + for_each_instdone_slice_subslice(m->i915, slice, subslice)
> > + err_printf(m, " SAMPLER_INSTDONE[%d][%d]: 0x%08x\n",
> > + slice, subslice,
> > + ee->instdone.sampler[slice][subslice]);
> > +
> > + for_each_instdone_slice_subslice(m->i915, slice, subslice)
> > + err_printf(m, " ROW_INSTDONE[%d][%d]: 0x%08x\n",
> > + slice, subslice,
> > + ee->instdone.row[slice][subslice]);
> > }
> >
> > static void error_print_engine(struct drm_i915_error_state_buf *m,
> > @@ -1534,12 +1542,52 @@ const char *i915_cache_level_str(struct drm_i915_private *i915, int type)
> > }
> > }
> >
> > +static inline uint32_t
> > +read_subslice_reg(struct drm_i915_private *dev_priv, int slice,
> > + int subslice, i915_reg_t reg)
> > +{
> > + uint32_t mcr;
> > + uint32_t ret;
> > + enum forcewake_domains fw_domains;
> > +
> > + fw_domains = intel_uncore_forcewake_for_reg(dev_priv, reg,
> > + FW_REG_READ);
> > + fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
> > + GEN8_MCR_SELECTOR,
> > + FW_REG_READ | FW_REG_WRITE);
> > +
> > + spin_lock_irq(&dev_priv->uncore.lock);
> > + intel_uncore_forcewake_get__locked(dev_priv, fw_domains);
> > +
> > + mcr = I915_READ_FW(GEN8_MCR_SELECTOR);
> > + /*
> > + * The HW expects the slice and sublice selectors to be reset to 0
> > + * after reading out the registers.
> > + */
> > + WARN_ON_ONCE(mcr & (GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK));
> > + mcr &= ~(GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK);
> > + mcr |= GEN8_MCR_SLICE(slice) | GEN8_MCR_SUBSLICE(subslice);
> > + I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr);
> > +
> > + ret = I915_READ_FW(reg);
> > +
> > + mcr &= ~(GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK);
> > + I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr);
> > +
> > + intel_uncore_forcewake_put__locked(dev_priv, fw_domains);
> > + spin_unlock_irq(&dev_priv->uncore.lock);
> > +
> > + return ret;
> > +}
> > +
> > /* NB: please notice the memset */
> > void i915_get_engine_instdone(struct drm_i915_private *dev_priv,
> > enum intel_engine_id engine_id,
> > struct intel_instdone *instdone)
> > {
> > u32 mmio_base = dev_priv->engine[engine_id].mmio_base;
> > + int slice;
> > + int subslice;
> >
> > memset(instdone, 0, sizeof(*instdone));
> >
> > @@ -1551,8 +1599,24 @@ void i915_get_engine_instdone(struct drm_i915_private *dev_priv,
> > break;
> >
> > instdone->slice_common = I915_READ(GEN7_SC_INSTDONE);
> > - instdone->sampler = I915_READ(GEN7_SAMPLER_INSTDONE);
> > - instdone->row = I915_READ(GEN7_ROW_INSTDONE);
> > + for_each_instdone_slice_subslice(dev_priv, slice, subslice) {
> > + instdone->sampler[slice][subslice] =
> > + read_subslice_reg(dev_priv, slice, subslice,
> > + GEN7_SAMPLER_INSTDONE);
> > + instdone->row[slice][subslice] =
> > + read_subslice_reg(dev_priv, slice, subslice,
> > + GEN7_ROW_INSTDONE);
> > + }
> > + break;
> > + case 7:
> > + instdone->instdone = I915_READ(RING_INSTDONE(mmio_base));
> > +
> > + if (engine_id != RCS)
> > + break;
> > +
> > + instdone->slice_common = I915_READ(GEN7_SC_INSTDONE);
> > + instdone->sampler[0][0] = I915_READ(GEN7_SAMPLER_INSTDONE);
> > + instdone->row[0][0] = I915_READ(GEN7_ROW_INSTDONE);
> >
> > break;
> > case 6:
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 82cf823..d44f4d8 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2554,6 +2554,9 @@ static inline void
> > i915_err_print_instdone(struct drm_i915_private *dev_priv,
> > struct intel_instdone *instdone)
> > {
> > + int slice;
> > + int subslice;
> > +
> > pr_err(" INSTDONE: 0x%08x\n", instdone->instdone);
> >
> > if (INTEL_GEN(dev_priv) <= 3)
> > @@ -2564,8 +2567,13 @@ i915_err_print_instdone(struct drm_i915_private *dev_priv,
> > 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);
> > + for_each_instdone_slice_subslice(dev_priv, slice, subslice)
> > + pr_err(" SAMPLER_INSTDONE[%d][%d]: 0x%08x\n",
> > + slice, subslice, instdone->sampler[slice][subslice]);
> > +
> > + for_each_instdone_slice_subslice(dev_priv, slice, subslice)
> > + pr_err(" ROW_INSTDONE[%d][%d]: 0x%08x\n",
> > + slice, subslice, instdone->row[slice][subslice]);
> > }
> >
> > static void i915_report_and_clear_eir(struct drm_i915_private *dev_priv)
> > @@ -2982,6 +2990,8 @@ static bool subunits_stuck(struct intel_engine_cs *engine)
> > struct intel_instdone instdone;
> > struct intel_instdone *accu_instdone = &engine->hangcheck.instdone;
> > bool stuck;
> > + int slice;
> > + int subslice;
> >
> > if (engine->id != RCS)
> > return true;
> > @@ -2997,10 +3007,13 @@ static bool subunits_stuck(struct intel_engine_cs *engine)
> > &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);
> > +
> > + for_each_instdone_slice_subslice(dev_priv, slice, subslice) {
> > + stuck &= instdone_unchanged(instdone.sampler[slice][subslice],
> > + &accu_instdone->sampler[slice][subslice]);
> > + stuck &= instdone_unchanged(instdone.row[slice][subslice],
> > + &accu_instdone->row[slice][subslice]);
> > + }
> >
> > return stuck;
> > }
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index e84bc2e..24e741d 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -1708,6 +1708,11 @@ enum skl_disp_power_wells {
> > #define GEN7_SC_INSTDONE _MMIO(0x7100)
> > #define GEN7_SAMPLER_INSTDONE _MMIO(0xe160)
> > #define GEN7_ROW_INSTDONE _MMIO(0xe164)
> > +#define GEN8_MCR_SELECTOR _MMIO(0xfdc)
> > +#define GEN8_MCR_SLICE(slice) (((slice) & 3) << 26)
> > +#define GEN8_MCR_SLICE_MASK GEN8_MCR_SLICE(3)
> > +#define GEN8_MCR_SUBSLICE(subslice) (((subslice) & 3) << 24)
>
> There is some conflicting info about these changing on gen9 to widen
> and shift the masks. So I am confused.
Checking again, they seem to match the GEN9 bspec.
> Can we easily test if we get sane slice/subslice states?
My smoke test on APL was to check i915_hangcheck_info and see the
subslice sampler and instdone bits changing (with differing values for
different subslices) while rendering was going on and static when idle.
Not sure about a more precise way.
--Imre
> -Mika
>
> > +#define GEN8_MCR_SUBSLICE_MASK GEN8_MCR_SUBSLICE(3)
> > #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 afc3f1e..983d7da 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > @@ -73,12 +73,31 @@ enum intel_engine_hangcheck_action {
> >
> > #define HANGCHECK_SCORE_RING_HUNG 31
> >
> > +#define I915_MAX_SLICES 3
> > +#define I915_MAX_SUBSLICES 3
> > +
> > +#define instdone_slice_mask(dev_priv__) \
> > + (INTEL_GEN(dev_priv__) == 7 ? \
> > + 1 : INTEL_INFO(dev_priv__)->sseu.slice_mask)
> > +
> > +#define instdone_subslice_mask(dev_priv__) \
> > + (INTEL_GEN(dev_priv__) == 7 ? \
> > + 1 : INTEL_INFO(dev_priv__)->sseu.subslice_mask)
> > +
> > +#define for_each_instdone_slice_subslice(dev_priv__, slice__, subslice__) \
> > + for ((slice__) = 0, (subslice__) = 0; \
> > + (slice__) < I915_MAX_SLICES; \
> > + (subslice__) = ((subslice__) + 1) < I915_MAX_SUBSLICES ? (subslice__) + 1 : 0, \
> > + (slice__) += ((subslice__) == 0)) \
> > + for_each_if((BIT(slice__) & instdone_slice_mask(dev_priv__)) && \
> > + (BIT(subslice__) & instdone_subslice_mask(dev_priv__)))
> > +
> > struct intel_instdone {
> > u32 instdone;
> > /* The following exist only in the RCS engine */
> > u32 slice_common;
> > - u32 sampler;
> > - u32 row;
> > + u32 sampler[I915_MAX_SLICES][I915_MAX_SUBSLICES];
> > + u32 row[I915_MAX_SLICES][I915_MAX_SUBSLICES];
> > };
> >
> > struct intel_engine_hangcheck {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] drm/i915: Cleanup instdone collection
2016-09-06 11:21 ` [PATCH v2 1/2] drm/i915: Cleanup instdone collection Mika Kuoppala
@ 2016-09-16 9:56 ` Jani Nikula
2016-09-20 13:44 ` Imre Deak
0 siblings, 1 reply; 7+ messages in thread
From: Jani Nikula @ 2016-09-16 9:56 UTC (permalink / raw)
To: Mika Kuoppala, Imre Deak, intel-gfx; +Cc: Ben Widawsky
On Tue, 06 Sep 2016, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> Imre Deak <imre.deak@intel.com> writes:
>
>> From: Ben Widawsky <benjamin.widawsky@intel.com>
>>
>> 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@bwidawsk.net>
>> Signed-off-by: Imre Deak <imre.deak@intel.com>
>
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
I didn't receive the original messages, and I can't find them on the
list either.
BR,
Jani.
>
>> ---
>> 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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] drm/i915: Try to print INSTDONE bits for all slice/subslice
[not found] ` <1473087178-4847-3-git-send-email-imre.deak@intel.com>
2016-09-15 14:30 ` [PATCH v2 2/2] drm/i915: Try to print INSTDONE bits for all slice/subslice Mika Kuoppala
@ 2016-09-20 13:09 ` Mika Kuoppala
1 sibling, 0 replies; 7+ messages in thread
From: Mika Kuoppala @ 2016-09-20 13:09 UTC (permalink / raw)
To: Imre Deak, intel-gfx; +Cc: Ben Widawsky
Imre Deak <imre.deak@intel.com> writes:
> From: Ben Widawsky <benjamin.widawsky@intel.com>
>
> v2: (Imre)
> - Access only subslices that are known to exist.
> - Reset explictly the MCR selector to slice/sub-slice ID 0 after the
> readout.
> - Use the subslice INSTDONE bits for the hangcheck/subunits-stuck
> detection too.
> - Take the uncore lock for the MCR-select/subslice-readout sequence.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 14 ++++--
> drivers/gpu/drm/i915/i915_gpu_error.c | 76 ++++++++++++++++++++++++++++++---
> drivers/gpu/drm/i915/i915_irq.c | 25 ++++++++---
> drivers/gpu/drm/i915/i915_reg.h | 5 +++
> drivers/gpu/drm/i915/intel_ringbuffer.h | 23 +++++++++-
> 5 files changed, 125 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 45244f9..0f84165 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1281,6 +1281,9 @@ static void i915_instdone_info(struct drm_i915_private *dev_priv,
> struct seq_file *m,
> struct intel_instdone *instdone)
> {
> + int slice;
> + int subslice;
> +
> seq_printf(m, "\t\tINSTDONE: 0x%08x\n",
> instdone->instdone);
>
> @@ -1293,10 +1296,13 @@ static void i915_instdone_info(struct drm_i915_private *dev_priv,
> 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);
> + for_each_instdone_slice_subslice(dev_priv, slice, subslice)
> + seq_printf(m, "\t\tSAMPLER_INSTDONE[%d][%d]: 0x%08x\n",
> + slice, subslice, instdone->sampler[slice][subslice]);
> +
> + for_each_instdone_slice_subslice(dev_priv, slice, subslice)
> + seq_printf(m, "\t\tROW_INSTDONE[%d][%d]: 0x%08x\n",
> + slice, subslice, instdone->row[slice][subslice]);
> }
>
> static int i915_hangcheck_info(struct seq_file *m, void *unused)
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 80fe101..06d4309 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -231,6 +231,9 @@ static const char *hangcheck_action_to_str(enum intel_engine_hangcheck_action a)
> static void error_print_instdone(struct drm_i915_error_state_buf *m,
> struct drm_i915_error_engine *ee)
> {
> + int slice;
> + int subslice;
> +
> err_printf(m, " INSTDONE: 0x%08x\n",
> ee->instdone.instdone);
>
> @@ -243,10 +246,15 @@ static void error_print_instdone(struct drm_i915_error_state_buf *m,
> 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);
> + for_each_instdone_slice_subslice(m->i915, slice, subslice)
> + err_printf(m, " SAMPLER_INSTDONE[%d][%d]: 0x%08x\n",
> + slice, subslice,
> + ee->instdone.sampler[slice][subslice]);
> +
> + for_each_instdone_slice_subslice(m->i915, slice, subslice)
> + err_printf(m, " ROW_INSTDONE[%d][%d]: 0x%08x\n",
> + slice, subslice,
> + ee->instdone.row[slice][subslice]);
> }
>
> static void error_print_engine(struct drm_i915_error_state_buf *m,
> @@ -1534,12 +1542,52 @@ const char *i915_cache_level_str(struct drm_i915_private *i915, int type)
> }
> }
>
> +static inline uint32_t
> +read_subslice_reg(struct drm_i915_private *dev_priv, int slice,
> + int subslice, i915_reg_t reg)
> +{
> + uint32_t mcr;
> + uint32_t ret;
> + enum forcewake_domains fw_domains;
> +
> + fw_domains = intel_uncore_forcewake_for_reg(dev_priv, reg,
> + FW_REG_READ);
> + fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
> + GEN8_MCR_SELECTOR,
> + FW_REG_READ | FW_REG_WRITE);
> +
> + spin_lock_irq(&dev_priv->uncore.lock);
> + intel_uncore_forcewake_get__locked(dev_priv, fw_domains);
> +
> + mcr = I915_READ_FW(GEN8_MCR_SELECTOR);
> + /*
> + * The HW expects the slice and sublice selectors to be reset to 0
> + * after reading out the registers.
> + */
> + WARN_ON_ONCE(mcr & (GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK));
> + mcr &= ~(GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK);
> + mcr |= GEN8_MCR_SLICE(slice) | GEN8_MCR_SUBSLICE(subslice);
> + I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr);
> +
> + ret = I915_READ_FW(reg);
> +
> + mcr &= ~(GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK);
> + I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr);
> +
> + intel_uncore_forcewake_put__locked(dev_priv, fw_domains);
> + spin_unlock_irq(&dev_priv->uncore.lock);
> +
> + return ret;
> +}
> +
> /* NB: please notice the memset */
> void i915_get_engine_instdone(struct drm_i915_private *dev_priv,
> enum intel_engine_id engine_id,
> struct intel_instdone *instdone)
> {
> u32 mmio_base = dev_priv->engine[engine_id].mmio_base;
> + int slice;
> + int subslice;
>
> memset(instdone, 0, sizeof(*instdone));
>
> @@ -1551,8 +1599,24 @@ void i915_get_engine_instdone(struct drm_i915_private *dev_priv,
> break;
>
> instdone->slice_common = I915_READ(GEN7_SC_INSTDONE);
> - instdone->sampler = I915_READ(GEN7_SAMPLER_INSTDONE);
> - instdone->row = I915_READ(GEN7_ROW_INSTDONE);
> + for_each_instdone_slice_subslice(dev_priv, slice, subslice) {
> + instdone->sampler[slice][subslice] =
> + read_subslice_reg(dev_priv, slice, subslice,
> + GEN7_SAMPLER_INSTDONE);
> + instdone->row[slice][subslice] =
> + read_subslice_reg(dev_priv, slice, subslice,
> + GEN7_ROW_INSTDONE);
> + }
> + break;
> + case 7:
> + instdone->instdone = I915_READ(RING_INSTDONE(mmio_base));
> +
> + if (engine_id != RCS)
> + break;
> +
> + instdone->slice_common = I915_READ(GEN7_SC_INSTDONE);
> + instdone->sampler[0][0] = I915_READ(GEN7_SAMPLER_INSTDONE);
> + instdone->row[0][0] = I915_READ(GEN7_ROW_INSTDONE);
>
> break;
> case 6:
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 82cf823..d44f4d8 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2554,6 +2554,9 @@ static inline void
> i915_err_print_instdone(struct drm_i915_private *dev_priv,
> struct intel_instdone *instdone)
> {
> + int slice;
> + int subslice;
> +
> pr_err(" INSTDONE: 0x%08x\n", instdone->instdone);
>
> if (INTEL_GEN(dev_priv) <= 3)
> @@ -2564,8 +2567,13 @@ i915_err_print_instdone(struct drm_i915_private *dev_priv,
> 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);
> + for_each_instdone_slice_subslice(dev_priv, slice, subslice)
> + pr_err(" SAMPLER_INSTDONE[%d][%d]: 0x%08x\n",
> + slice, subslice, instdone->sampler[slice][subslice]);
> +
> + for_each_instdone_slice_subslice(dev_priv, slice, subslice)
> + pr_err(" ROW_INSTDONE[%d][%d]: 0x%08x\n",
> + slice, subslice, instdone->row[slice][subslice]);
> }
>
> static void i915_report_and_clear_eir(struct drm_i915_private *dev_priv)
> @@ -2982,6 +2990,8 @@ static bool subunits_stuck(struct intel_engine_cs *engine)
> struct intel_instdone instdone;
> struct intel_instdone *accu_instdone = &engine->hangcheck.instdone;
> bool stuck;
> + int slice;
> + int subslice;
>
> if (engine->id != RCS)
> return true;
> @@ -2997,10 +3007,13 @@ static bool subunits_stuck(struct intel_engine_cs *engine)
> &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);
> +
> + for_each_instdone_slice_subslice(dev_priv, slice, subslice) {
> + stuck &= instdone_unchanged(instdone.sampler[slice][subslice],
> + &accu_instdone->sampler[slice][subslice]);
> + stuck &= instdone_unchanged(instdone.row[slice][subslice],
> + &accu_instdone->row[slice][subslice]);
> + }
>
> return stuck;
> }
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index e84bc2e..24e741d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1708,6 +1708,11 @@ enum skl_disp_power_wells {
> #define GEN7_SC_INSTDONE _MMIO(0x7100)
> #define GEN7_SAMPLER_INSTDONE _MMIO(0xe160)
> #define GEN7_ROW_INSTDONE _MMIO(0xe164)
> +#define GEN8_MCR_SELECTOR _MMIO(0xfdc)
> +#define GEN8_MCR_SLICE(slice) (((slice) & 3) << 26)
> +#define GEN8_MCR_SLICE_MASK GEN8_MCR_SLICE(3)
> +#define GEN8_MCR_SUBSLICE(subslice) (((subslice) & 3) << 24)
> +#define GEN8_MCR_SUBSLICE_MASK GEN8_MCR_SUBSLICE(3)
> #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 afc3f1e..983d7da 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -73,12 +73,31 @@ enum intel_engine_hangcheck_action {
>
> #define HANGCHECK_SCORE_RING_HUNG 31
>
> +#define I915_MAX_SLICES 3
> +#define I915_MAX_SUBSLICES 3
> +
> +#define instdone_slice_mask(dev_priv__) \
> + (INTEL_GEN(dev_priv__) == 7 ? \
> + 1 : INTEL_INFO(dev_priv__)->sseu.slice_mask)
> +
> +#define instdone_subslice_mask(dev_priv__) \
> + (INTEL_GEN(dev_priv__) == 7 ? \
> + 1 : INTEL_INFO(dev_priv__)->sseu.subslice_mask)
> +
> +#define for_each_instdone_slice_subslice(dev_priv__, slice__, subslice__) \
> + for ((slice__) = 0, (subslice__) = 0; \
> + (slice__) < I915_MAX_SLICES; \
> + (subslice__) = ((subslice__) + 1) < I915_MAX_SUBSLICES ? (subslice__) + 1 : 0, \
> + (slice__) += ((subslice__) == 0)) \
> + for_each_if((BIT(slice__) & instdone_slice_mask(dev_priv__)) && \
> + (BIT(subslice__) & instdone_subslice_mask(dev_priv__)))
> +
> struct intel_instdone {
> u32 instdone;
> /* The following exist only in the RCS engine */
> u32 slice_common;
> - u32 sampler;
> - u32 row;
> + u32 sampler[I915_MAX_SLICES][I915_MAX_SUBSLICES];
> + u32 row[I915_MAX_SLICES][I915_MAX_SUBSLICES];
> };
>
> struct intel_engine_hangcheck {
> --
> 2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] drm/i915: Cleanup instdone collection
2016-09-16 9:56 ` Jani Nikula
@ 2016-09-20 13:44 ` Imre Deak
0 siblings, 0 replies; 7+ messages in thread
From: Imre Deak @ 2016-09-20 13:44 UTC (permalink / raw)
To: Jani Nikula, Mika Kuoppala, intel-gfx; +Cc: Ben Widawsky
On pe, 2016-09-16 at 12:56 +0300, Jani Nikula wrote:
> On Tue, 06 Sep 2016, Mika Kuoppala <mika.kuoppala@linux.intel.com>
> wrote:
> > Imre Deak <imre.deak@intel.com> writes:
> >
> > > From: Ben Widawsky <benjamin.widawsky@intel.com>
> > >
> > > 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@bwidawsk.net>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> >
> > Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
>
> I didn't receive the original messages, and I can't find them on the
> list either.
Can't find it in the list archive either, not sure where it got stuck.
I'll resend it.
--Imre
>
> BR,
> Jani.
>
>
> >
> > > ---
> > > 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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] drm/i915: Try to print INSTDONE bits for all slice/subslice
2016-09-15 14:30 ` [PATCH v2 2/2] drm/i915: Try to print INSTDONE bits for all slice/subslice Mika Kuoppala
2016-09-15 16:10 ` Imre Deak
@ 2016-09-26 19:09 ` Ben Widawsky
1 sibling, 0 replies; 7+ messages in thread
From: Ben Widawsky @ 2016-09-26 19:09 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
On 16-09-15 17:30:10, Mika Kuoppala wrote:
>Imre Deak <imre.deak@intel.com> writes:
>
>> From: Ben Widawsky <benjamin.widawsky@intel.com>
>>
>> v2: (Imre)
>> - Access only subslices that are known to exist.
>> - Reset explictly the MCR selector to slice/sub-slice ID 0 after the
>> readout.
>> - Use the subslice INSTDONE bits for the hangcheck/subunits-stuck
>> detection too.
>> - Take the uncore lock for the MCR-select/subslice-readout sequence.
>>
>> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
>> Signed-off-by: Imre Deak <imre.deak@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_debugfs.c | 14 ++++--
>> drivers/gpu/drm/i915/i915_gpu_error.c | 76 ++++++++++++++++++++++++++++++---
>> drivers/gpu/drm/i915/i915_irq.c | 25 ++++++++---
>> drivers/gpu/drm/i915/i915_reg.h | 5 +++
>> drivers/gpu/drm/i915/intel_ringbuffer.h | 23 +++++++++-
>> 5 files changed, 125 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 45244f9..0f84165 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -1281,6 +1281,9 @@ static void i915_instdone_info(struct drm_i915_private *dev_priv,
>> struct seq_file *m,
>> struct intel_instdone *instdone)
>> {
>> + int slice;
>> + int subslice;
>> +
>> seq_printf(m, "\t\tINSTDONE: 0x%08x\n",
>> instdone->instdone);
>>
>> @@ -1293,10 +1296,13 @@ static void i915_instdone_info(struct drm_i915_private *dev_priv,
>> 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);
>> + for_each_instdone_slice_subslice(dev_priv, slice, subslice)
>> + seq_printf(m, "\t\tSAMPLER_INSTDONE[%d][%d]: 0x%08x\n",
>> + slice, subslice, instdone->sampler[slice][subslice]);
>> +
>> + for_each_instdone_slice_subslice(dev_priv, slice, subslice)
>> + seq_printf(m, "\t\tROW_INSTDONE[%d][%d]: 0x%08x\n",
>> + slice, subslice, instdone->row[slice][subslice]);
>> }
>>
>> static int i915_hangcheck_info(struct seq_file *m, void *unused)
>> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
>> index 80fe101..06d4309 100644
>> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
>> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
>> @@ -231,6 +231,9 @@ static const char *hangcheck_action_to_str(enum intel_engine_hangcheck_action a)
>> static void error_print_instdone(struct drm_i915_error_state_buf *m,
>> struct drm_i915_error_engine *ee)
>> {
>> + int slice;
>> + int subslice;
>> +
>> err_printf(m, " INSTDONE: 0x%08x\n",
>> ee->instdone.instdone);
>>
>> @@ -243,10 +246,15 @@ static void error_print_instdone(struct drm_i915_error_state_buf *m,
>> 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);
>> + for_each_instdone_slice_subslice(m->i915, slice, subslice)
>> + err_printf(m, " SAMPLER_INSTDONE[%d][%d]: 0x%08x\n",
>> + slice, subslice,
>> + ee->instdone.sampler[slice][subslice]);
>> +
>> + for_each_instdone_slice_subslice(m->i915, slice, subslice)
>> + err_printf(m, " ROW_INSTDONE[%d][%d]: 0x%08x\n",
>> + slice, subslice,
>> + ee->instdone.row[slice][subslice]);
>> }
>>
>> static void error_print_engine(struct drm_i915_error_state_buf *m,
>> @@ -1534,12 +1542,52 @@ const char *i915_cache_level_str(struct drm_i915_private *i915, int type)
>> }
>> }
>>
>> +static inline uint32_t
>> +read_subslice_reg(struct drm_i915_private *dev_priv, int slice,
>> + int subslice, i915_reg_t reg)
>> +{
>> + uint32_t mcr;
>> + uint32_t ret;
>> + enum forcewake_domains fw_domains;
>> +
>> + fw_domains = intel_uncore_forcewake_for_reg(dev_priv, reg,
>> + FW_REG_READ);
>> + fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
>> + GEN8_MCR_SELECTOR,
>> + FW_REG_READ | FW_REG_WRITE);
>> +
>> + spin_lock_irq(&dev_priv->uncore.lock);
>> + intel_uncore_forcewake_get__locked(dev_priv, fw_domains);
>> +
>> + mcr = I915_READ_FW(GEN8_MCR_SELECTOR);
>> + /*
>> + * The HW expects the slice and sublice selectors to be reset to 0
>> + * after reading out the registers.
>> + */
>> + WARN_ON_ONCE(mcr & (GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK));
>> + mcr &= ~(GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK);
>> + mcr |= GEN8_MCR_SLICE(slice) | GEN8_MCR_SUBSLICE(subslice);
>> + I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr);
>> +
>> + ret = I915_READ_FW(reg);
>> +
>> + mcr &= ~(GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK);
>> + I915_WRITE_FW(GEN8_MCR_SELECTOR, mcr);
>> +
>> + intel_uncore_forcewake_put__locked(dev_priv, fw_domains);
>> + spin_unlock_irq(&dev_priv->uncore.lock);
>> +
>> + return ret;
>> +}
>> +
>> /* NB: please notice the memset */
>> void i915_get_engine_instdone(struct drm_i915_private *dev_priv,
>> enum intel_engine_id engine_id,
>> struct intel_instdone *instdone)
>> {
>> u32 mmio_base = dev_priv->engine[engine_id].mmio_base;
>> + int slice;
>> + int subslice;
>>
>> memset(instdone, 0, sizeof(*instdone));
>>
>> @@ -1551,8 +1599,24 @@ void i915_get_engine_instdone(struct drm_i915_private *dev_priv,
>> break;
>>
>> instdone->slice_common = I915_READ(GEN7_SC_INSTDONE);
>> - instdone->sampler = I915_READ(GEN7_SAMPLER_INSTDONE);
>> - instdone->row = I915_READ(GEN7_ROW_INSTDONE);
>> + for_each_instdone_slice_subslice(dev_priv, slice, subslice) {
>> + instdone->sampler[slice][subslice] =
>> + read_subslice_reg(dev_priv, slice, subslice,
>> + GEN7_SAMPLER_INSTDONE);
>> + instdone->row[slice][subslice] =
>> + read_subslice_reg(dev_priv, slice, subslice,
>> + GEN7_ROW_INSTDONE);
>> + }
>> + break;
>> + case 7:
>> + instdone->instdone = I915_READ(RING_INSTDONE(mmio_base));
>> +
>> + if (engine_id != RCS)
>> + break;
>> +
>> + instdone->slice_common = I915_READ(GEN7_SC_INSTDONE);
>> + instdone->sampler[0][0] = I915_READ(GEN7_SAMPLER_INSTDONE);
>> + instdone->row[0][0] = I915_READ(GEN7_ROW_INSTDONE);
>>
>> break;
>> case 6:
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index 82cf823..d44f4d8 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -2554,6 +2554,9 @@ static inline void
>> i915_err_print_instdone(struct drm_i915_private *dev_priv,
>> struct intel_instdone *instdone)
>> {
>> + int slice;
>> + int subslice;
>> +
>> pr_err(" INSTDONE: 0x%08x\n", instdone->instdone);
>>
>> if (INTEL_GEN(dev_priv) <= 3)
>> @@ -2564,8 +2567,13 @@ i915_err_print_instdone(struct drm_i915_private *dev_priv,
>> 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);
>> + for_each_instdone_slice_subslice(dev_priv, slice, subslice)
>> + pr_err(" SAMPLER_INSTDONE[%d][%d]: 0x%08x\n",
>> + slice, subslice, instdone->sampler[slice][subslice]);
>> +
>> + for_each_instdone_slice_subslice(dev_priv, slice, subslice)
>> + pr_err(" ROW_INSTDONE[%d][%d]: 0x%08x\n",
>> + slice, subslice, instdone->row[slice][subslice]);
>> }
>>
>> static void i915_report_and_clear_eir(struct drm_i915_private *dev_priv)
>> @@ -2982,6 +2990,8 @@ static bool subunits_stuck(struct intel_engine_cs *engine)
>> struct intel_instdone instdone;
>> struct intel_instdone *accu_instdone = &engine->hangcheck.instdone;
>> bool stuck;
>> + int slice;
>> + int subslice;
>>
>> if (engine->id != RCS)
>> return true;
>> @@ -2997,10 +3007,13 @@ static bool subunits_stuck(struct intel_engine_cs *engine)
>> &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);
>> +
>> + for_each_instdone_slice_subslice(dev_priv, slice, subslice) {
>> + stuck &= instdone_unchanged(instdone.sampler[slice][subslice],
>> + &accu_instdone->sampler[slice][subslice]);
>> + stuck &= instdone_unchanged(instdone.row[slice][subslice],
>> + &accu_instdone->row[slice][subslice]);
>> + }
>>
>> return stuck;
>> }
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index e84bc2e..24e741d 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -1708,6 +1708,11 @@ enum skl_disp_power_wells {
>> #define GEN7_SC_INSTDONE _MMIO(0x7100)
>> #define GEN7_SAMPLER_INSTDONE _MMIO(0xe160)
>> #define GEN7_ROW_INSTDONE _MMIO(0xe164)
>> +#define GEN8_MCR_SELECTOR _MMIO(0xfdc)
>> +#define GEN8_MCR_SLICE(slice) (((slice) & 3) << 26)
>> +#define GEN8_MCR_SLICE_MASK GEN8_MCR_SLICE(3)
>> +#define GEN8_MCR_SUBSLICE(subslice) (((subslice) & 3) << 24)
>
>There is some conflicting info about these changing on gen9 to widen
>and shift the masks. So I am confused.
>
>Can we easily test if we get sane slice/subslice states?
>
>-Mika
>
Yeah. I guess GEN9 has up to 3 slices, so the mask would change. I don't think
we need a test though, just read the docs.
/me shrugs
>> +#define GEN8_MCR_SUBSLICE_MASK GEN8_MCR_SUBSLICE(3)
>> #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 afc3f1e..983d7da 100644
>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
>> @@ -73,12 +73,31 @@ enum intel_engine_hangcheck_action {
>>
>> #define HANGCHECK_SCORE_RING_HUNG 31
>>
>> +#define I915_MAX_SLICES 3
>> +#define I915_MAX_SUBSLICES 3
>> +
>> +#define instdone_slice_mask(dev_priv__) \
>> + (INTEL_GEN(dev_priv__) == 7 ? \
>> + 1 : INTEL_INFO(dev_priv__)->sseu.slice_mask)
>> +
>> +#define instdone_subslice_mask(dev_priv__) \
>> + (INTEL_GEN(dev_priv__) == 7 ? \
>> + 1 : INTEL_INFO(dev_priv__)->sseu.subslice_mask)
>> +
>> +#define for_each_instdone_slice_subslice(dev_priv__, slice__, subslice__) \
>> + for ((slice__) = 0, (subslice__) = 0; \
>> + (slice__) < I915_MAX_SLICES; \
>> + (subslice__) = ((subslice__) + 1) < I915_MAX_SUBSLICES ? (subslice__) + 1 : 0, \
>> + (slice__) += ((subslice__) == 0)) \
>> + for_each_if((BIT(slice__) & instdone_slice_mask(dev_priv__)) && \
>> + (BIT(subslice__) & instdone_subslice_mask(dev_priv__)))
>> +
>> struct intel_instdone {
>> u32 instdone;
>> /* The following exist only in the RCS engine */
>> u32 slice_common;
>> - u32 sampler;
>> - u32 row;
>> + u32 sampler[I915_MAX_SLICES][I915_MAX_SUBSLICES];
>> + u32 row[I915_MAX_SLICES][I915_MAX_SUBSLICES];
>> };
>>
>> struct intel_engine_hangcheck {
>> --
>> 2.5.0
--
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-09-26 19:09 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1473087178-4847-1-git-send-email-imre.deak@intel.com>
[not found] ` <1473087178-4847-2-git-send-email-imre.deak@intel.com>
2016-09-06 11:21 ` [PATCH v2 1/2] drm/i915: Cleanup instdone collection Mika Kuoppala
2016-09-16 9:56 ` Jani Nikula
2016-09-20 13:44 ` Imre Deak
[not found] ` <1473087178-4847-3-git-send-email-imre.deak@intel.com>
2016-09-15 14:30 ` [PATCH v2 2/2] drm/i915: Try to print INSTDONE bits for all slice/subslice Mika Kuoppala
2016-09-15 16:10 ` Imre Deak
2016-09-26 19:09 ` Ben Widawsky
2016-09-20 13:09 ` Mika Kuoppala
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).