| Message ID | 20190820230544.170010-10-stuart.summers@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Refactor to expand subslice mask (rev 2) | expand |
Quoting Stuart Summers (2019-08-21 00:05:42) > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h > index a82cea95c2f2..99bee06cdbdb 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h > +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h > @@ -576,20 +576,19 @@ intel_engine_is_virtual(const struct intel_engine_cs *engine) > return engine->flags & I915_ENGINE_IS_VIRTUAL; > } > > -#define instdone_slice_mask(dev_priv__) \ > - (IS_GEN(dev_priv__, 7) ? \ > - 1 : RUNTIME_INFO(dev_priv__)->sseu.slice_mask) > - > -#define instdone_subslice_mask(dev_priv__) \ > - (IS_GEN(dev_priv__, 7) ? \ > - 1 : RUNTIME_INFO(dev_priv__)->sseu.subslice_mask[0]) > - > -#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__))) > - > +#define instdone_has_slice(dev_priv___, sseu___, slice___) \ > + ((IS_GEN(dev_priv___, 7) ? 1 : ((sseu___)->slice_mask)) & \ > + BIT(slice___)) ((IS_GEN(dev_priv___, 7) ? 1 : \ ((sseu___)->slice_mask)) & BIT(slice___)) That split is marginally easier to read So much for hoping the gen7 special case just disappears. > +#define instdone_has_subslice(dev_priv__, sseu__, slice__, subslice__) \ > + (IS_GEN(dev_priv__, 7) ? (1 & BIT(subslice__)) : \ > + intel_sseu_has_subslice(sseu__, 0, subslice__)) > + > +#define for_each_instdone_slice_subslice(dev_priv_, sseu_, slice_, subslice_) \ > + for ((slice_) = 0, (subslice_) = 0; (slice_) < I915_MAX_SLICES; \ > + (subslice_) = ((subslice_) + 1) % I915_MAX_SUBSLICES, \ > + (slice_) += ((subslice_) == 0)) \ > + for_each_if((instdone_has_slice(dev_priv_, sseu_, slice_)) && \ > + (instdone_has_subslice(dev_priv_, sseu_, slice_, \ > + subslice_))) That was less convoluted than I was expecting from previous skims. Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Wed, 2019-08-21 at 23:56 +0100, Chris Wilson wrote: > Quoting Stuart Summers (2019-08-21 00:05:42) > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h > > b/drivers/gpu/drm/i915/gt/intel_engine_types.h > > index a82cea95c2f2..99bee06cdbdb 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h > > @@ -576,20 +576,19 @@ intel_engine_is_virtual(const struct > > intel_engine_cs *engine) > > return engine->flags & I915_ENGINE_IS_VIRTUAL; > > } > > > > -#define instdone_slice_mask(dev_priv__) \ > > - (IS_GEN(dev_priv__, 7) ? \ > > - 1 : RUNTIME_INFO(dev_priv__)->sseu.slice_mask) > > - > > -#define instdone_subslice_mask(dev_priv__) \ > > - (IS_GEN(dev_priv__, 7) ? \ > > - 1 : RUNTIME_INFO(dev_priv__)->sseu.subslice_mask[0]) > > - > > -#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__))) > > - > > +#define instdone_has_slice(dev_priv___, sseu___, slice___) \ > > + ((IS_GEN(dev_priv___, 7) ? 1 : ((sseu___)->slice_mask)) & \ > > + BIT(slice___)) > > ((IS_GEN(dev_priv___, 7) ? 1 : \ > ((sseu___)->slice_mask)) & BIT(slice___)) > > That split is marginally easier to read Makes sense, and I agree what I have is a little ugly... I'll change in the next revision. > > So much for hoping the gen7 special case just disappears. > > > +#define instdone_has_subslice(dev_priv__, sseu__, slice__, > > subslice__) \ > > + (IS_GEN(dev_priv__, 7) ? (1 & BIT(subslice__)) : \ > > + intel_sseu_has_subslice(sseu__, 0, subslice__)) > > + > > +#define for_each_instdone_slice_subslice(dev_priv_, sseu_, slice_, > > subslice_) \ > > + for ((slice_) = 0, (subslice_) = 0; (slice_) < > > I915_MAX_SLICES; \ > > + (subslice_) = ((subslice_) + 1) % I915_MAX_SUBSLICES, > > \ > > + (slice_) += ((subslice_) == 0)) \ > > + for_each_if((instdone_has_slice(dev_priv_, sseu_, > > slice_)) && \ > > + (instdone_has_subslice(dev_priv_, > > sseu_, slice_, \ > > + subslice_))) > > That was less convoluted than I was expecting from previous skims. > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Thanks! -Stuart > -Chris
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index 82630db0394b..17006d50b63f 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -948,6 +948,7 @@ void intel_engine_get_instdone(struct intel_engine_cs *engine, struct intel_instdone *instdone) { struct drm_i915_private *i915 = engine->i915; + const struct sseu_dev_info *sseu = &RUNTIME_INFO(i915)->sseu; struct intel_uncore *uncore = engine->uncore; u32 mmio_base = engine->mmio_base; int slice; @@ -965,7 +966,7 @@ void intel_engine_get_instdone(struct intel_engine_cs *engine, instdone->slice_common = intel_uncore_read(uncore, GEN7_SC_INSTDONE); - for_each_instdone_slice_subslice(i915, slice, subslice) { + for_each_instdone_slice_subslice(i915, sseu, slice, subslice) { instdone->sampler[slice][subslice] = read_subslice_reg(engine, slice, subslice, GEN7_SAMPLER_INSTDONE); diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h index a82cea95c2f2..99bee06cdbdb 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h @@ -576,20 +576,19 @@ intel_engine_is_virtual(const struct intel_engine_cs *engine) return engine->flags & I915_ENGINE_IS_VIRTUAL; } -#define instdone_slice_mask(dev_priv__) \ - (IS_GEN(dev_priv__, 7) ? \ - 1 : RUNTIME_INFO(dev_priv__)->sseu.slice_mask) - -#define instdone_subslice_mask(dev_priv__) \ - (IS_GEN(dev_priv__, 7) ? \ - 1 : RUNTIME_INFO(dev_priv__)->sseu.subslice_mask[0]) - -#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__))) - +#define instdone_has_slice(dev_priv___, sseu___, slice___) \ + ((IS_GEN(dev_priv___, 7) ? 1 : ((sseu___)->slice_mask)) & \ + BIT(slice___)) + +#define instdone_has_subslice(dev_priv__, sseu__, slice__, subslice__) \ + (IS_GEN(dev_priv__, 7) ? (1 & BIT(subslice__)) : \ + intel_sseu_has_subslice(sseu__, 0, subslice__)) + +#define for_each_instdone_slice_subslice(dev_priv_, sseu_, slice_, subslice_) \ + for ((slice_) = 0, (subslice_) = 0; (slice_) < I915_MAX_SLICES; \ + (subslice_) = ((subslice_) + 1) % I915_MAX_SUBSLICES, \ + (slice_) += ((subslice_) == 0)) \ + for_each_if((instdone_has_slice(dev_priv_, sseu_, slice_)) && \ + (instdone_has_subslice(dev_priv_, sseu_, slice_, \ + subslice_))) #endif /* __INTEL_ENGINE_TYPES_H__ */ diff --git a/drivers/gpu/drm/i915/gt/intel_hangcheck.c b/drivers/gpu/drm/i915/gt/intel_hangcheck.c index 05d042cdefe2..40f62f780be5 100644 --- a/drivers/gpu/drm/i915/gt/intel_hangcheck.c +++ b/drivers/gpu/drm/i915/gt/intel_hangcheck.c @@ -53,6 +53,7 @@ static bool instdone_unchanged(u32 current_instdone, u32 *old_instdone) static bool subunits_stuck(struct intel_engine_cs *engine) { struct drm_i915_private *dev_priv = engine->i915; + const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu; struct intel_instdone instdone; struct intel_instdone *accu_instdone = &engine->hangcheck.instdone; bool stuck; @@ -71,7 +72,7 @@ static bool subunits_stuck(struct intel_engine_cs *engine) stuck &= instdone_unchanged(instdone.slice_common, &accu_instdone->slice_common); - for_each_instdone_slice_subslice(dev_priv, slice, subslice) { + for_each_instdone_slice_subslice(dev_priv, sseu, slice, subslice) { stuck &= instdone_unchanged(instdone.sampler[slice][subslice], &accu_instdone->sampler[slice][subslice]); stuck &= instdone_unchanged(instdone.row[slice][subslice], diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index abf0f6ba213c..8c5eca72ff5e 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -995,6 +995,7 @@ static void i915_instdone_info(struct drm_i915_private *dev_priv, struct seq_file *m, struct intel_instdone *instdone) { + const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu; int slice; int subslice; @@ -1010,11 +1011,11 @@ static void i915_instdone_info(struct drm_i915_private *dev_priv, if (INTEL_GEN(dev_priv) <= 6) return; - for_each_instdone_slice_subslice(dev_priv, slice, subslice) + for_each_instdone_slice_subslice(dev_priv, sseu, 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) + for_each_instdone_slice_subslice(dev_priv, sseu, slice, subslice) seq_printf(m, "\t\tROW_INSTDONE[%d][%d]: 0x%08x\n", slice, subslice, instdone->row[slice][subslice]); } diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index e284bd76fa86..4aff342b8944 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -421,6 +421,7 @@ static void err_compression_marker(struct drm_i915_error_state_buf *m) static void error_print_instdone(struct drm_i915_error_state_buf *m, const struct drm_i915_error_engine *ee) { + const struct sseu_dev_info *sseu = &RUNTIME_INFO(m->i915)->sseu; int slice; int subslice; @@ -436,12 +437,12 @@ static void error_print_instdone(struct drm_i915_error_state_buf *m, if (INTEL_GEN(m->i915) <= 6) return; - for_each_instdone_slice_subslice(m->i915, slice, subslice) + for_each_instdone_slice_subslice(m->i915, sseu, 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) + for_each_instdone_slice_subslice(m->i915, sseu, slice, subslice) err_printf(m, " ROW_INSTDONE[%d][%d]: 0x%08x\n", slice, subslice, ee->instdone.row[slice][subslice]);
Refactor instdone loops to use the new intel_sseu_has_subslice function. Signed-off-by: Stuart Summers <stuart.summers@intel.com> --- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 3 +- drivers/gpu/drm/i915/gt/intel_engine_types.h | 31 ++++++++++---------- drivers/gpu/drm/i915/gt/intel_hangcheck.c | 3 +- drivers/gpu/drm/i915/i915_debugfs.c | 5 ++-- drivers/gpu/drm/i915/i915_gpu_error.c | 5 ++-- 5 files changed, 25 insertions(+), 22 deletions(-)