Imre Deak <imre.d...@intel.com> writes:

> From: Ben Widawsky <benjamin.widaw...@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 <b...@bwidawsk.net>
> Signed-off-by: Imre Deak <imre.d...@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

Reply via email to