On pe, 2016-09-16 at 12:56 +0300, Jani Nikula wrote: > On Tue, 06 Sep 2016, Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx> > wrote: > > Imre Deak <imre.deak@xxxxxxxxx> writes: > > > > > From: Ben Widawsky <benjamin.widawsky@xxxxxxxxx> > > > > > > Consolidate the instdone logic so we can get a bit fancier. This > > > patch also > > > removes the duplicated print of INSTDONE[0]. > > > > > > v2: (Imre) > > > - Rebased on top of hangcheck INSTDONE changes. > > > - Move all INSTDONE registers into a single struct, store it > > > within the > > > engine error struct during error capturing. > > > > > > Signed-off-by: Ben Widawsky <ben@xxxxxxxxxxxx> > > > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx> > > > > Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx> > > 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@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx