From: Ben Widawsky <benjamin.widawsky@intel.com>
To: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v2 2/2] drm/i915: Try to print INSTDONE bits for all slice/subslice
Date: Mon, 26 Sep 2016 12:09:18 -0700 [thread overview]
Message-ID: <20160926190918.GB8835@intel.com> (raw)
In-Reply-To: <871t0l6xsd.fsf@gaia.fi.intel.com>
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
next prev parent reply other threads:[~2016-09-26 19:09 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
[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 [this message]
2016-09-20 13:09 ` Mika Kuoppala
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160926190918.GB8835@intel.com \
--to=benjamin.widawsky@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=mika.kuoppala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.