| Message ID | 20190820230544.170010-12-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:44) > Currently, the subslice_mask runtime parameter is stored as an > array of subslices per slice. Expand the subslice mask array to > better match what is presented to userspace through the > I915_QUERY_TOPOLOGY_INFO ioctl. The index into this array is > then calculated: > slice * subslice stride + subslice index / 8 > > v2: Fix 32-bit build > v3: Use new helper function in SSEU workaround warning message > > Signed-off-by: Stuart Summers <stuart.summers@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_sseu.c | 27 ++++++++++++++++++++- > drivers/gpu/drm/i915/gt/intel_sseu.h | 5 +++- > drivers/gpu/drm/i915/gt/intel_workarounds.c | 5 ++-- > drivers/gpu/drm/i915/i915_debugfs.c | 5 +++- > drivers/gpu/drm/i915/intel_device_info.c | 8 +++--- > 5 files changed, 40 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.c b/drivers/gpu/drm/i915/gt/intel_sseu.c > index 5d537ec97fcc..eae1c7b20688 100644 > --- a/drivers/gpu/drm/i915/gt/intel_sseu.c > +++ b/drivers/gpu/drm/i915/gt/intel_sseu.c > @@ -30,6 +30,31 @@ intel_sseu_subslice_total(const struct sseu_dev_info *sseu) > return total; > } > > +u32 intel_sseu_get_subslices(const struct sseu_dev_info *sseu, u8 slice) > +{ > + int i, offset = slice * sseu->ss_stride; > + u32 mask = 0; > + > + if (slice >= sseu->max_slices) { > + DRM_ERROR("%s: invalid slice %d, max: %d\n", > + __func__, slice, sseu->max_slices); > + return 0; > + } That's a programmer error, we only need it during development. I am a proponent of making it explode as early as possible and make people fix it rather than ignore it. > + if (sseu->ss_stride > sizeof(mask)) { > + DRM_ERROR("%s: invalid subslice stride %d, max: %u\n", > + __func__, sseu->ss_stride, > + (unsigned int)sizeof(mask)); > + return 0; > + } Put the assertion in the constructor (set_sseu_info or whatever it was called) > + > + for (i = 0; i < sseu->ss_stride; i++) > + mask |= (u32)sseu->subslice_mask[offset + i] << > + i * BITS_PER_BYTE; Otherwise, the actual conversions look fairly straightforward, Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Wed, 2019-08-21 at 23:49 +0100, Chris Wilson wrote: > Quoting Stuart Summers (2019-08-21 00:05:44) > > Currently, the subslice_mask runtime parameter is stored as an > > array of subslices per slice. Expand the subslice mask array to > > better match what is presented to userspace through the > > I915_QUERY_TOPOLOGY_INFO ioctl. The index into this array is > > then calculated: > > slice * subslice stride + subslice index / 8 > > > > v2: Fix 32-bit build > > v3: Use new helper function in SSEU workaround warning message > > > > Signed-off-by: Stuart Summers <stuart.summers@intel.com> > > --- > > drivers/gpu/drm/i915/gt/intel_sseu.c | 27 > > ++++++++++++++++++++- > > drivers/gpu/drm/i915/gt/intel_sseu.h | 5 +++- > > drivers/gpu/drm/i915/gt/intel_workarounds.c | 5 ++-- > > drivers/gpu/drm/i915/i915_debugfs.c | 5 +++- > > drivers/gpu/drm/i915/intel_device_info.c | 8 +++--- > > 5 files changed, 40 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.c > > b/drivers/gpu/drm/i915/gt/intel_sseu.c > > index 5d537ec97fcc..eae1c7b20688 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_sseu.c > > +++ b/drivers/gpu/drm/i915/gt/intel_sseu.c > > @@ -30,6 +30,31 @@ intel_sseu_subslice_total(const struct > > sseu_dev_info *sseu) > > return total; > > } > > > > +u32 intel_sseu_get_subslices(const struct sseu_dev_info *sseu, u8 > > slice) > > +{ > > + int i, offset = slice * sseu->ss_stride; > > + u32 mask = 0; > > + > > + if (slice >= sseu->max_slices) { > > + DRM_ERROR("%s: invalid slice %d, max: %d\n", > > + __func__, slice, sseu->max_slices); > > + return 0; > > + } > > That's a programmer error, we only need it during development. I am a > proponent of making it explode as early as possible and make people > fix > it rather than ignore it. I guess this was trigger shyness on my part after bringing some of the ICL machines down with my first post of this series :) I agree this seems like the right thing to do though. I'll make the change in the next revision. > > > + if (sseu->ss_stride > sizeof(mask)) { > > + DRM_ERROR("%s: invalid subslice stride %d, max: > > %u\n", > > + __func__, sseu->ss_stride, > > + (unsigned int)sizeof(mask)); > > + return 0; > > + } > > Put the assertion in the constructor (set_sseu_info or whatever it > was > called) Makes sense. > > > + > > + for (i = 0; i < sseu->ss_stride; i++) > > + mask |= (u32)sseu->subslice_mask[offset + i] << > > + i * BITS_PER_BYTE; > > Otherwise, the actual conversions look fairly straightforward, > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Thanks! -Stuart > -Chris
diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.c b/drivers/gpu/drm/i915/gt/intel_sseu.c index 5d537ec97fcc..eae1c7b20688 100644 --- a/drivers/gpu/drm/i915/gt/intel_sseu.c +++ b/drivers/gpu/drm/i915/gt/intel_sseu.c @@ -30,6 +30,31 @@ intel_sseu_subslice_total(const struct sseu_dev_info *sseu) return total; } +u32 intel_sseu_get_subslices(const struct sseu_dev_info *sseu, u8 slice) +{ + int i, offset = slice * sseu->ss_stride; + u32 mask = 0; + + if (slice >= sseu->max_slices) { + DRM_ERROR("%s: invalid slice %d, max: %d\n", + __func__, slice, sseu->max_slices); + return 0; + } + + if (sseu->ss_stride > sizeof(mask)) { + DRM_ERROR("%s: invalid subslice stride %d, max: %u\n", + __func__, sseu->ss_stride, + (unsigned int)sizeof(mask)); + return 0; + } + + for (i = 0; i < sseu->ss_stride; i++) + mask |= (u32)sseu->subslice_mask[offset + i] << + i * BITS_PER_BYTE; + + return mask; +} + void intel_sseu_set_subslices(struct sseu_dev_info *sseu, int slice, u32 ss_mask) { @@ -47,7 +72,7 @@ void intel_sseu_set_subslices(struct sseu_dev_info *sseu, int slice, unsigned int intel_sseu_subslices_per_slice(const struct sseu_dev_info *sseu, u8 slice) { - return hweight8(sseu->subslice_mask[slice]); + return hweight32(intel_sseu_get_subslices(sseu, slice)); } u32 intel_sseu_make_rpcs(struct drm_i915_private *i915, diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.h b/drivers/gpu/drm/i915/gt/intel_sseu.h index 3a146a33a22e..edf77f07ffb3 100644 --- a/drivers/gpu/drm/i915/gt/intel_sseu.h +++ b/drivers/gpu/drm/i915/gt/intel_sseu.h @@ -17,10 +17,11 @@ struct drm_i915_private; #define GEN_MAX_SLICES (6) /* CNL upper bound */ #define GEN_MAX_SUBSLICES (8) /* ICL upper bound */ #define GEN_SSEU_STRIDE(max_entries) DIV_ROUND_UP(max_entries, BITS_PER_BYTE) +#define GEN_MAX_SUBSLICE_STRIDE GEN_SSEU_STRIDE(GEN_MAX_SUBSLICES) struct sseu_dev_info { u8 slice_mask; - u8 subslice_mask[GEN_MAX_SLICES]; + u8 subslice_mask[GEN_MAX_SLICES * GEN_MAX_SUBSLICE_STRIDE]; u16 eu_total; u8 eu_per_subslice; u8 min_eu_in_pool; @@ -91,6 +92,8 @@ intel_sseu_subslice_total(const struct sseu_dev_info *sseu); unsigned int intel_sseu_subslices_per_slice(const struct sseu_dev_info *sseu, u8 slice); +u32 intel_sseu_get_subslices(const struct sseu_dev_info *sseu, u8 slice); + void intel_sseu_set_subslices(struct sseu_dev_info *sseu, int slice, u32 ss_mask); diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index 126ab3667919..ad2261e0cba8 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -801,11 +801,10 @@ wa_init_mcr(struct drm_i915_private *i915, struct i915_wa_list *wal) } slice = fls(sseu->slice_mask) - 1; - GEM_BUG_ON(slice >= ARRAY_SIZE(sseu->subslice_mask)); - subslice = fls(l3_en & sseu->subslice_mask[slice]); + subslice = fls(l3_en & intel_sseu_get_subslices(sseu, slice)); if (!subslice) { DRM_WARN("No common index found between subslice mask %x and L3 bank mask %x!\n", - sseu->subslice_mask[slice], l3_en); + intel_sseu_get_subslices(sseu, slice), l3_en); subslice = fls(l3_en); WARN_ON(!subslice); } diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index e975cad03e39..112c8a305971 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -3859,13 +3859,16 @@ static void gen9_sseu_device_status(struct drm_i915_private *dev_priv, for (ss = 0; ss < info->sseu.max_subslices; ss++) { unsigned int eu_cnt; + u8 ss_idx = s * info->sseu.ss_stride + + ss / BITS_PER_BYTE; if (IS_GEN9_LP(dev_priv)) { if (!(s_reg[s] & (GEN9_PGCTL_SS_ACK(ss)))) /* skip disabled subslice */ continue; - sseu->subslice_mask[s] |= BIT(ss); + sseu->subslice_mask[ss_idx] |= + BIT(ss % BITS_PER_BYTE); } eu_cnt = 2 * hweight32(eu_reg[2*s + ss/2] & diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index c20f74ee5f22..d9b5baaef5d0 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -93,9 +93,9 @@ static void sseu_dump(const struct sseu_dev_info *sseu, struct drm_printer *p) hweight8(sseu->slice_mask), sseu->slice_mask); drm_printf(p, "subslice total: %u\n", intel_sseu_subslice_total(sseu)); for (s = 0; s < sseu->max_slices; s++) { - drm_printf(p, "slice%d: %u subslices, mask=%04x\n", + drm_printf(p, "slice%d: %u subslices, mask=%08x\n", s, intel_sseu_subslices_per_slice(sseu, s), - sseu->subslice_mask[s]); + intel_sseu_get_subslices(sseu, s)); } drm_printf(p, "EU total: %u\n", sseu->eu_total); drm_printf(p, "EU per subslice: %u\n", sseu->eu_per_subslice); @@ -159,9 +159,9 @@ void intel_device_info_dump_topology(const struct sseu_dev_info *sseu, } for (s = 0; s < sseu->max_slices; s++) { - drm_printf(p, "slice%d: %u subslice(s) (0x%hhx):\n", + drm_printf(p, "slice%d: %u subslice(s) (0x%08x):\n", s, intel_sseu_subslices_per_slice(sseu, s), - sseu->subslice_mask[s]); + intel_sseu_get_subslices(sseu, s)); for (ss = 0; ss < sseu->max_subslices; ss++) { u16 enabled_eus = sseu_get_eus(sseu, s, ss);
Currently, the subslice_mask runtime parameter is stored as an array of subslices per slice. Expand the subslice mask array to better match what is presented to userspace through the I915_QUERY_TOPOLOGY_INFO ioctl. The index into this array is then calculated: slice * subslice stride + subslice index / 8 v2: Fix 32-bit build v3: Use new helper function in SSEU workaround warning message Signed-off-by: Stuart Summers <stuart.summers@intel.com> --- drivers/gpu/drm/i915/gt/intel_sseu.c | 27 ++++++++++++++++++++- drivers/gpu/drm/i915/gt/intel_sseu.h | 5 +++- drivers/gpu/drm/i915/gt/intel_workarounds.c | 5 ++-- drivers/gpu/drm/i915/i915_debugfs.c | 5 +++- drivers/gpu/drm/i915/intel_device_info.c | 8 +++--- 5 files changed, 40 insertions(+), 10 deletions(-)