[Intel-gfx] [PATCH v2 2/2] drm/i915: Try to print INSTDONE bits for all slice/subslice
Ben Widawsky
benjamin.widawsky at intel.com
Mon Sep 26 19:09:18 UTC 2016
On 16-09-15 17:30:10, Mika Kuoppala wrote:
>Imre Deak <imre.deak at intel.com> writes:
>
>> From: Ben Widawsky <benjamin.widawsky at 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 at bwidawsk.net>
>> Signed-off-by: Imre Deak <imre.deak at 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
More information about the Intel-gfx
mailing list