| Message ID | 20190524154022.13575-6-stuart.summers@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Refactor to expand subslice mask | expand |
Hi Stuart, On Fri, May 24, 2019 at 08:40:22AM -0700, Stuart Summers wrote: > 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 spacing in set_sseu_info args > use set_sseu_info to initialize sseu data when building > device status in debugfs > rename variables in intel_engine_types.h to avoid checkpatch > warnings > v3: update headers in intel_sseu.h > v4: add const to some sseu_dev_info variables > use sseu->eu_stride for EU stride calculations > v5: address review comments from Tvrtko and Daniele > v6: remove extra space in intel_sseu_get_subslices > return the correct subslice enable in for_each_instdone > add GEM_BUG_ON to ensure user doesn't pass invalid ss_mask size > use printk formatted string for subslice mask > v7: remove string.h header and rebase > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Signed-off-by: Stuart Summers <stuart.summers@intel.com> > --- <snip> > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c > index 97f742530fa1..3625f777f3a3 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.c > +++ b/drivers/gpu/drm/i915/intel_device_info.c > @@ -92,9 +92,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); > @@ -117,10 +117,9 @@ void intel_device_info_dump_runtime(const struct intel_runtime_info *info, > static int sseu_eu_idx(const struct sseu_dev_info *sseu, int slice, > int subslice) > { > - int subslice_stride = GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); > - int slice_stride = sseu->max_subslices * subslice_stride; > + int slice_stride = sseu->max_subslices * sseu->eu_stride; > > - return slice * slice_stride + subslice * subslice_stride; > + return slice * slice_stride + subslice * sseu->eu_stride; > } > > static u16 sseu_get_eus(const struct sseu_dev_info *sseu, int slice, > @@ -129,7 +128,7 @@ static u16 sseu_get_eus(const struct sseu_dev_info *sseu, int slice, > int i, offset = sseu_eu_idx(sseu, slice, subslice); > u16 eu_mask = 0; > > - for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); i++) { > + for (i = 0; i < sseu->eu_stride; i++) { > eu_mask |= ((u16)sseu->eu_mask[offset + i]) << > (i * BITS_PER_BYTE); > } > @@ -142,7 +141,7 @@ static void sseu_set_eus(struct sseu_dev_info *sseu, int slice, int subslice, > { > int i, offset = sseu_eu_idx(sseu, slice, subslice); > > - for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); i++) { > + for (i = 0; i < sseu->eu_stride; i++) { > sseu->eu_mask[offset + i] = > (eu_mask >> (BITS_PER_BYTE * i)) & 0xff; > } > @@ -159,9 +158,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); > @@ -190,15 +189,10 @@ static void gen11_sseu_info_init(struct drm_i915_private *dev_priv) > u8 eu_en; > int s; > > - if (IS_ELKHARTLAKE(dev_priv)) { > - sseu->max_slices = 1; > - sseu->max_subslices = 4; > - sseu->max_eus_per_subslice = 8; > - } else { > - sseu->max_slices = 1; > - sseu->max_subslices = 8; > - sseu->max_eus_per_subslice = 8; > - } > + if (IS_ELKHARTLAKE(dev_priv)) > + intel_sseu_set_info(sseu, 1, 4, 8); > + else > + intel_sseu_set_info(sseu, 1, 8, 8); > > s_en = I915_READ(GEN11_GT_SLICE_ENABLE) & GEN11_GT_S_ENA_MASK; > ss_en = ~I915_READ(GEN11_GT_SUBSLICE_DISABLE); > @@ -207,15 +201,15 @@ static void gen11_sseu_info_init(struct drm_i915_private *dev_priv) > > for (s = 0; s < sseu->max_slices; s++) { > if (s_en & BIT(s)) { > - int ss_idx = sseu->max_subslices * s; > int ss; > > sseu->slice_mask |= BIT(s); > - sseu->subslice_mask[s] = (ss_en >> ss_idx) & ss_en_mask; > - for (ss = 0; ss < sseu->max_subslices; ss++) { > - if (sseu->subslice_mask[s] & BIT(ss)) > + > + intel_sseu_set_subslices(sseu, s, ss_en_mask); > + > + for (ss = 0; ss < sseu->max_subslices; ss++) > + if (intel_sseu_has_subslice(sseu, s, ss)) > sseu_set_eus(sseu, s, ss, eu_en); > - } > } > } > sseu->eu_per_subslice = hweight8(eu_en); > @@ -235,23 +229,10 @@ static void gen10_sseu_info_init(struct drm_i915_private *dev_priv) > const int eu_mask = 0xff; > u32 subslice_mask, eu_en; > > + intel_sseu_set_info(sseu, 6, 4, 8); > + > sseu->slice_mask = (fuse2 & GEN10_F2_S_ENA_MASK) >> > GEN10_F2_S_ENA_SHIFT; > - sseu->max_slices = 6; > - sseu->max_subslices = 4; > - sseu->max_eus_per_subslice = 8; > - > - subslice_mask = (1 << 4) - 1; > - subslice_mask &= ~((fuse2 & GEN10_F2_SS_DIS_MASK) >> > - GEN10_F2_SS_DIS_SHIFT); > - > - /* > - * Slice0 can have up to 3 subslices, but there are only 2 in > - * slice1/2. > - */ > - sseu->subslice_mask[0] = subslice_mask; > - for (s = 1; s < sseu->max_slices; s++) > - sseu->subslice_mask[s] = subslice_mask & 0x3; > > /* Slice0 */ > eu_en = ~I915_READ(GEN8_EU_DISABLE0); > @@ -276,14 +257,22 @@ static void gen10_sseu_info_init(struct drm_i915_private *dev_priv) > eu_en = ~I915_READ(GEN10_EU_DISABLE3); > sseu_set_eus(sseu, 5, 1, eu_en & eu_mask); > > - /* Do a second pass where we mark the subslices disabled if all their > - * eus are off. > - */ > + subslice_mask = (1 << 4) - 1; > + subslice_mask &= ~((fuse2 & GEN10_F2_SS_DIS_MASK) >> > + GEN10_F2_SS_DIS_SHIFT); > + > for (s = 0; s < sseu->max_slices; s++) { > for (ss = 0; ss < sseu->max_subslices; ss++) { > if (sseu_get_eus(sseu, s, ss) == 0) > - sseu->subslice_mask[s] &= ~BIT(ss); > + subslice_mask &= ~BIT(ss); > } > + > + /* > + * Slice0 can have up to 3 subslices, but there are only 2 in > + * slice1/2. > + */ > + intel_sseu_set_subslices(sseu, s, s == 0 ? subslice_mask : > + subslice_mask & 0x3); > } > > sseu->eu_total = compute_eu_total(sseu); > @@ -309,13 +298,12 @@ static void cherryview_sseu_info_init(struct drm_i915_private *dev_priv) > { > struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu; > u32 fuse; > + u8 subslice_mask; > > fuse = I915_READ(CHV_FUSE_GT); > > sseu->slice_mask = BIT(0); > - sseu->max_slices = 1; > - sseu->max_subslices = 2; > - sseu->max_eus_per_subslice = 8; > + intel_sseu_set_info(sseu, 1, 2, 8); > > if (!(fuse & CHV_FGT_DISABLE_SS0)) { > u8 disabled_mask = > @@ -324,7 +312,7 @@ static void cherryview_sseu_info_init(struct drm_i915_private *dev_priv) > (((fuse & CHV_FGT_EU_DIS_SS0_R1_MASK) >> > CHV_FGT_EU_DIS_SS0_R1_SHIFT) << 4); > > - sseu->subslice_mask[0] |= BIT(0); > + subslice_mask |= BIT(0); When building with -Wuninitialized, clang warns: drivers/gpu/drm/i915/intel_device_info.c:315:3: warning: variable 'subslice_mask' is uninitialized when used here [-Wuninitialized] subslice_mask |= BIT(0); ^~~~~~~~~~~~~ drivers/gpu/drm/i915/intel_device_info.c:301:18: note: initialize the variable 'subslice_mask' to silence this warning u8 subslice_mask; ^ = '\0' I assume that it should be initialized to zero but maybe you intended something different? Thanks, Nathan
On Wed, 29 May 2019, Nathan Chancellor <natechancellor@gmail.com> wrote: > Hi Stuart, > > On Fri, May 24, 2019 at 08:40:22AM -0700, Stuart Summers wrote: >> 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 spacing in set_sseu_info args >> use set_sseu_info to initialize sseu data when building >> device status in debugfs >> rename variables in intel_engine_types.h to avoid checkpatch >> warnings >> v3: update headers in intel_sseu.h >> v4: add const to some sseu_dev_info variables >> use sseu->eu_stride for EU stride calculations >> v5: address review comments from Tvrtko and Daniele >> v6: remove extra space in intel_sseu_get_subslices >> return the correct subslice enable in for_each_instdone >> add GEM_BUG_ON to ensure user doesn't pass invalid ss_mask size >> use printk formatted string for subslice mask >> v7: remove string.h header and rebase >> >> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >> Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> Signed-off-by: Stuart Summers <stuart.summers@intel.com> >> --- > > <snip> > >> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c >> index 97f742530fa1..3625f777f3a3 100644 >> --- a/drivers/gpu/drm/i915/intel_device_info.c >> +++ b/drivers/gpu/drm/i915/intel_device_info.c >> @@ -92,9 +92,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); >> @@ -117,10 +117,9 @@ void intel_device_info_dump_runtime(const struct intel_runtime_info *info, >> static int sseu_eu_idx(const struct sseu_dev_info *sseu, int slice, >> int subslice) >> { >> - int subslice_stride = GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); >> - int slice_stride = sseu->max_subslices * subslice_stride; >> + int slice_stride = sseu->max_subslices * sseu->eu_stride; >> >> - return slice * slice_stride + subslice * subslice_stride; >> + return slice * slice_stride + subslice * sseu->eu_stride; >> } >> >> static u16 sseu_get_eus(const struct sseu_dev_info *sseu, int slice, >> @@ -129,7 +128,7 @@ static u16 sseu_get_eus(const struct sseu_dev_info *sseu, int slice, >> int i, offset = sseu_eu_idx(sseu, slice, subslice); >> u16 eu_mask = 0; >> >> - for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); i++) { >> + for (i = 0; i < sseu->eu_stride; i++) { >> eu_mask |= ((u16)sseu->eu_mask[offset + i]) << >> (i * BITS_PER_BYTE); >> } >> @@ -142,7 +141,7 @@ static void sseu_set_eus(struct sseu_dev_info *sseu, int slice, int subslice, >> { >> int i, offset = sseu_eu_idx(sseu, slice, subslice); >> >> - for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); i++) { >> + for (i = 0; i < sseu->eu_stride; i++) { >> sseu->eu_mask[offset + i] = >> (eu_mask >> (BITS_PER_BYTE * i)) & 0xff; >> } >> @@ -159,9 +158,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); >> @@ -190,15 +189,10 @@ static void gen11_sseu_info_init(struct drm_i915_private *dev_priv) >> u8 eu_en; >> int s; >> >> - if (IS_ELKHARTLAKE(dev_priv)) { >> - sseu->max_slices = 1; >> - sseu->max_subslices = 4; >> - sseu->max_eus_per_subslice = 8; >> - } else { >> - sseu->max_slices = 1; >> - sseu->max_subslices = 8; >> - sseu->max_eus_per_subslice = 8; >> - } >> + if (IS_ELKHARTLAKE(dev_priv)) >> + intel_sseu_set_info(sseu, 1, 4, 8); >> + else >> + intel_sseu_set_info(sseu, 1, 8, 8); >> >> s_en = I915_READ(GEN11_GT_SLICE_ENABLE) & GEN11_GT_S_ENA_MASK; >> ss_en = ~I915_READ(GEN11_GT_SUBSLICE_DISABLE); >> @@ -207,15 +201,15 @@ static void gen11_sseu_info_init(struct drm_i915_private *dev_priv) >> >> for (s = 0; s < sseu->max_slices; s++) { >> if (s_en & BIT(s)) { >> - int ss_idx = sseu->max_subslices * s; >> int ss; >> >> sseu->slice_mask |= BIT(s); >> - sseu->subslice_mask[s] = (ss_en >> ss_idx) & ss_en_mask; >> - for (ss = 0; ss < sseu->max_subslices; ss++) { >> - if (sseu->subslice_mask[s] & BIT(ss)) >> + >> + intel_sseu_set_subslices(sseu, s, ss_en_mask); >> + >> + for (ss = 0; ss < sseu->max_subslices; ss++) >> + if (intel_sseu_has_subslice(sseu, s, ss)) >> sseu_set_eus(sseu, s, ss, eu_en); >> - } >> } >> } >> sseu->eu_per_subslice = hweight8(eu_en); >> @@ -235,23 +229,10 @@ static void gen10_sseu_info_init(struct drm_i915_private *dev_priv) >> const int eu_mask = 0xff; >> u32 subslice_mask, eu_en; >> >> + intel_sseu_set_info(sseu, 6, 4, 8); >> + >> sseu->slice_mask = (fuse2 & GEN10_F2_S_ENA_MASK) >> >> GEN10_F2_S_ENA_SHIFT; >> - sseu->max_slices = 6; >> - sseu->max_subslices = 4; >> - sseu->max_eus_per_subslice = 8; >> - >> - subslice_mask = (1 << 4) - 1; >> - subslice_mask &= ~((fuse2 & GEN10_F2_SS_DIS_MASK) >> >> - GEN10_F2_SS_DIS_SHIFT); >> - >> - /* >> - * Slice0 can have up to 3 subslices, but there are only 2 in >> - * slice1/2. >> - */ >> - sseu->subslice_mask[0] = subslice_mask; >> - for (s = 1; s < sseu->max_slices; s++) >> - sseu->subslice_mask[s] = subslice_mask & 0x3; >> >> /* Slice0 */ >> eu_en = ~I915_READ(GEN8_EU_DISABLE0); >> @@ -276,14 +257,22 @@ static void gen10_sseu_info_init(struct drm_i915_private *dev_priv) >> eu_en = ~I915_READ(GEN10_EU_DISABLE3); >> sseu_set_eus(sseu, 5, 1, eu_en & eu_mask); >> >> - /* Do a second pass where we mark the subslices disabled if all their >> - * eus are off. >> - */ >> + subslice_mask = (1 << 4) - 1; >> + subslice_mask &= ~((fuse2 & GEN10_F2_SS_DIS_MASK) >> >> + GEN10_F2_SS_DIS_SHIFT); >> + >> for (s = 0; s < sseu->max_slices; s++) { >> for (ss = 0; ss < sseu->max_subslices; ss++) { >> if (sseu_get_eus(sseu, s, ss) == 0) >> - sseu->subslice_mask[s] &= ~BIT(ss); >> + subslice_mask &= ~BIT(ss); >> } >> + >> + /* >> + * Slice0 can have up to 3 subslices, but there are only 2 in >> + * slice1/2. >> + */ >> + intel_sseu_set_subslices(sseu, s, s == 0 ? subslice_mask : >> + subslice_mask & 0x3); >> } >> >> sseu->eu_total = compute_eu_total(sseu); >> @@ -309,13 +298,12 @@ static void cherryview_sseu_info_init(struct drm_i915_private *dev_priv) >> { >> struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu; >> u32 fuse; >> + u8 subslice_mask; >> >> fuse = I915_READ(CHV_FUSE_GT); >> >> sseu->slice_mask = BIT(0); >> - sseu->max_slices = 1; >> - sseu->max_subslices = 2; >> - sseu->max_eus_per_subslice = 8; >> + intel_sseu_set_info(sseu, 1, 2, 8); >> >> if (!(fuse & CHV_FGT_DISABLE_SS0)) { >> u8 disabled_mask = >> @@ -324,7 +312,7 @@ static void cherryview_sseu_info_init(struct drm_i915_private *dev_priv) >> (((fuse & CHV_FGT_EU_DIS_SS0_R1_MASK) >> >> CHV_FGT_EU_DIS_SS0_R1_SHIFT) << 4); >> >> - sseu->subslice_mask[0] |= BIT(0); >> + subslice_mask |= BIT(0); > > When building with -Wuninitialized, clang warns: > > drivers/gpu/drm/i915/intel_device_info.c:315:3: warning: variable 'subslice_mask' is uninitialized when used here [-Wuninitialized] > subslice_mask |= BIT(0); > ^~~~~~~~~~~~~ > drivers/gpu/drm/i915/intel_device_info.c:301:18: note: initialize the variable 'subslice_mask' to silence this warning > u8 subslice_mask; > ^ > = '\0' > > I assume that it should be initialized to zero but maybe you intended > something different? As it happens, the commit has been reverted for other reasons, and we (the royal we, I really mean Stuart) can fix this while at it. ;) BR, Jani.
On Fri, 24 May 2019, Stuart Summers <stuart.summers@intel.com> wrote: > 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 spacing in set_sseu_info args > use set_sseu_info to initialize sseu data when building > device status in debugfs > rename variables in intel_engine_types.h to avoid checkpatch > warnings > v3: update headers in intel_sseu.h > v4: add const to some sseu_dev_info variables > use sseu->eu_stride for EU stride calculations > v5: address review comments from Tvrtko and Daniele > v6: remove extra space in intel_sseu_get_subslices > return the correct subslice enable in for_each_instdone > add GEM_BUG_ON to ensure user doesn't pass invalid ss_mask size > use printk formatted string for subslice mask > v7: remove string.h header and rebase > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Signed-off-by: Stuart Summers <stuart.summers@intel.com> As this patch uncovered a latent issue in 1e40d4aea57b ("drm/i915/cnl: Implement WaProgramMgsrForCorrectSliceSpecificMmioReads") and got reverted, I'll take the opportunity to comment. I acknowledge the revert is shooting the messenger a bit, and this will smell like maintainer bikeshedding. Now, the first reaction looking at the commit was, it does not fare well on the "if a bisect landed on this commit, how happy would I be" scale. While it's mostly refactoring, it could be chopped up to several logical and obvious steps. For example, add intel_sseu_set_info() first with no other changes. Add ss_stride and eu_stride to struct sseu_dev_info separately. Add intel_sseu_get_subslices() but don't expand yet, make it just sseu->subslice_mask[s] first. And so on, you get the idea, a series of small non-functional changes followed by patches with functional changes that stand out. Indeed patches 1-4 did this fine. It's easy on the reviewer, it's easy on whoever git blames years down the line. Trust me, we will. And it would be the commit adding intel_sseu_get_subslices(), or the one adding the GEM_BUG_ON()s into it, that would blow up 1e40d4aea57b ("drm/i915/cnl: Implement WaProgramMgsrForCorrectSliceSpecificMmioReads"). One more note below. > @@ -461,7 +461,9 @@ static int i915_getparam_ioctl(struct drm_device *dev, void *data, > return -ENODEV; > break; > case I915_PARAM_SUBSLICE_MASK: > - value = sseu->subslice_mask[0]; > + /* Only copy bits from the first slice */ > + memcpy(&value, sseu->subslice_mask, > + min(sseu->ss_stride, (u8)sizeof(value))); Frankly I'd rather see this written in self-evident code without the comment. BR, Jani.
On Wed, 2019-05-29 at 17:33 +0300, Jani Nikula wrote: > On Wed, 29 May 2019, Nathan Chancellor <natechancellor@gmail.com> > wrote: > > Hi Stuart, > > > > On Fri, May 24, 2019 at 08:40:22AM -0700, Stuart Summers wrote: > > > 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 spacing in set_sseu_info args > > > use set_sseu_info to initialize sseu data when building > > > device status in debugfs > > > rename variables in intel_engine_types.h to avoid checkpatch > > > warnings > > > v3: update headers in intel_sseu.h > > > v4: add const to some sseu_dev_info variables > > > use sseu->eu_stride for EU stride calculations > > > v5: address review comments from Tvrtko and Daniele > > > v6: remove extra space in intel_sseu_get_subslices > > > return the correct subslice enable in for_each_instdone > > > add GEM_BUG_ON to ensure user doesn't pass invalid ss_mask > > > size > > > use printk formatted string for subslice mask > > > v7: remove string.h header and rebase > > > > > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > > > Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > > > Reviewed-by: Daniele Ceraolo Spurio < > > > daniele.ceraolospurio@intel.com> > > > Signed-off-by: Stuart Summers <stuart.summers@intel.com> > > > --- > > > > <snip> > > > > > diff --git a/drivers/gpu/drm/i915/intel_device_info.c > > > b/drivers/gpu/drm/i915/intel_device_info.c > > > index 97f742530fa1..3625f777f3a3 100644 > > > --- a/drivers/gpu/drm/i915/intel_device_info.c > > > +++ b/drivers/gpu/drm/i915/intel_device_info.c > > > @@ -92,9 +92,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); > > > @@ -117,10 +117,9 @@ void intel_device_info_dump_runtime(const > > > struct intel_runtime_info *info, > > > static int sseu_eu_idx(const struct sseu_dev_info *sseu, int > > > slice, > > > int subslice) > > > { > > > - int subslice_stride = GEN_SSEU_STRIDE(sseu- > > > >max_eus_per_subslice); > > > - int slice_stride = sseu->max_subslices * subslice_stride; > > > + int slice_stride = sseu->max_subslices * sseu->eu_stride; > > > > > > - return slice * slice_stride + subslice * subslice_stride; > > > + return slice * slice_stride + subslice * sseu->eu_stride; > > > } > > > > > > static u16 sseu_get_eus(const struct sseu_dev_info *sseu, int > > > slice, > > > @@ -129,7 +128,7 @@ static u16 sseu_get_eus(const struct > > > sseu_dev_info *sseu, int slice, > > > int i, offset = sseu_eu_idx(sseu, slice, subslice); > > > u16 eu_mask = 0; > > > > > > - for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); > > > i++) { > > > + for (i = 0; i < sseu->eu_stride; i++) { > > > eu_mask |= ((u16)sseu->eu_mask[offset + i]) << > > > (i * BITS_PER_BYTE); > > > } > > > @@ -142,7 +141,7 @@ static void sseu_set_eus(struct sseu_dev_info > > > *sseu, int slice, int subslice, > > > { > > > int i, offset = sseu_eu_idx(sseu, slice, subslice); > > > > > > - for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); > > > i++) { > > > + for (i = 0; i < sseu->eu_stride; i++) { > > > sseu->eu_mask[offset + i] = > > > (eu_mask >> (BITS_PER_BYTE * i)) & 0xff; > > > } > > > @@ -159,9 +158,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); > > > @@ -190,15 +189,10 @@ static void gen11_sseu_info_init(struct > > > drm_i915_private *dev_priv) > > > u8 eu_en; > > > int s; > > > > > > - if (IS_ELKHARTLAKE(dev_priv)) { > > > - sseu->max_slices = 1; > > > - sseu->max_subslices = 4; > > > - sseu->max_eus_per_subslice = 8; > > > - } else { > > > - sseu->max_slices = 1; > > > - sseu->max_subslices = 8; > > > - sseu->max_eus_per_subslice = 8; > > > - } > > > + if (IS_ELKHARTLAKE(dev_priv)) > > > + intel_sseu_set_info(sseu, 1, 4, 8); > > > + else > > > + intel_sseu_set_info(sseu, 1, 8, 8); > > > > > > s_en = I915_READ(GEN11_GT_SLICE_ENABLE) & GEN11_GT_S_ENA_MASK; > > > ss_en = ~I915_READ(GEN11_GT_SUBSLICE_DISABLE); > > > @@ -207,15 +201,15 @@ static void gen11_sseu_info_init(struct > > > drm_i915_private *dev_priv) > > > > > > for (s = 0; s < sseu->max_slices; s++) { > > > if (s_en & BIT(s)) { > > > - int ss_idx = sseu->max_subslices * s; > > > int ss; > > > > > > sseu->slice_mask |= BIT(s); > > > - sseu->subslice_mask[s] = (ss_en >> ss_idx) & > > > ss_en_mask; > > > - for (ss = 0; ss < sseu->max_subslices; ss++) { > > > - if (sseu->subslice_mask[s] & BIT(ss)) > > > + > > > + intel_sseu_set_subslices(sseu, s, ss_en_mask); > > > + > > > + for (ss = 0; ss < sseu->max_subslices; ss++) > > > + if (intel_sseu_has_subslice(sseu, s, > > > ss)) > > > sseu_set_eus(sseu, s, ss, > > > eu_en); > > > - } > > > } > > > } > > > sseu->eu_per_subslice = hweight8(eu_en); > > > @@ -235,23 +229,10 @@ static void gen10_sseu_info_init(struct > > > drm_i915_private *dev_priv) > > > const int eu_mask = 0xff; > > > u32 subslice_mask, eu_en; > > > > > > + intel_sseu_set_info(sseu, 6, 4, 8); > > > + > > > sseu->slice_mask = (fuse2 & GEN10_F2_S_ENA_MASK) >> > > > GEN10_F2_S_ENA_SHIFT; > > > - sseu->max_slices = 6; > > > - sseu->max_subslices = 4; > > > - sseu->max_eus_per_subslice = 8; > > > - > > > - subslice_mask = (1 << 4) - 1; > > > - subslice_mask &= ~((fuse2 & GEN10_F2_SS_DIS_MASK) >> > > > - GEN10_F2_SS_DIS_SHIFT); > > > - > > > - /* > > > - * Slice0 can have up to 3 subslices, but there are only 2 in > > > - * slice1/2. > > > - */ > > > - sseu->subslice_mask[0] = subslice_mask; > > > - for (s = 1; s < sseu->max_slices; s++) > > > - sseu->subslice_mask[s] = subslice_mask & 0x3; > > > > > > /* Slice0 */ > > > eu_en = ~I915_READ(GEN8_EU_DISABLE0); > > > @@ -276,14 +257,22 @@ static void gen10_sseu_info_init(struct > > > drm_i915_private *dev_priv) > > > eu_en = ~I915_READ(GEN10_EU_DISABLE3); > > > sseu_set_eus(sseu, 5, 1, eu_en & eu_mask); > > > > > > - /* Do a second pass where we mark the subslices disabled if all > > > their > > > - * eus are off. > > > - */ > > > + subslice_mask = (1 << 4) - 1; > > > + subslice_mask &= ~((fuse2 & GEN10_F2_SS_DIS_MASK) >> > > > + GEN10_F2_SS_DIS_SHIFT); > > > + > > > for (s = 0; s < sseu->max_slices; s++) { > > > for (ss = 0; ss < sseu->max_subslices; ss++) { > > > if (sseu_get_eus(sseu, s, ss) == 0) > > > - sseu->subslice_mask[s] &= ~BIT(ss); > > > + subslice_mask &= ~BIT(ss); > > > } > > > + > > > + /* > > > + * Slice0 can have up to 3 subslices, but there are > > > only 2 in > > > + * slice1/2. > > > + */ > > > + intel_sseu_set_subslices(sseu, s, s == 0 ? > > > subslice_mask : > > > + subslice_mas > > > k & 0x3); > > > } > > > > > > sseu->eu_total = compute_eu_total(sseu); > > > @@ -309,13 +298,12 @@ static void > > > cherryview_sseu_info_init(struct drm_i915_private *dev_priv) > > > { > > > struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu; > > > u32 fuse; > > > + u8 subslice_mask; > > > > > > fuse = I915_READ(CHV_FUSE_GT); > > > > > > sseu->slice_mask = BIT(0); > > > - sseu->max_slices = 1; > > > - sseu->max_subslices = 2; > > > - sseu->max_eus_per_subslice = 8; > > > + intel_sseu_set_info(sseu, 1, 2, 8); > > > > > > if (!(fuse & CHV_FGT_DISABLE_SS0)) { > > > u8 disabled_mask = > > > @@ -324,7 +312,7 @@ static void cherryview_sseu_info_init(struct > > > drm_i915_private *dev_priv) > > > (((fuse & CHV_FGT_EU_DIS_SS0_R1_MASK) >> > > > CHV_FGT_EU_DIS_SS0_R1_SHIFT) << 4); > > > > > > - sseu->subslice_mask[0] |= BIT(0); > > > + subslice_mask |= BIT(0); > > > > When building with -Wuninitialized, clang warns: > > > > drivers/gpu/drm/i915/intel_device_info.c:315:3: warning: variable > > 'subslice_mask' is uninitialized when used here [-Wuninitialized] > > subslice_mask |= BIT(0); > > ^~~~~~~~~~~~~ > > drivers/gpu/drm/i915/intel_device_info.c:301:18: note: initialize > > the variable 'subslice_mask' to silence this warning > > u8 subslice_mask; > > ^ > > = '\0' > > > > I assume that it should be initialized to zero but maybe you > > intended > > something different? > > As it happens, the commit has been reverted for other reasons, and we > (the royal we, I really mean Stuart) can fix this while at it. ;) I'll accept the royal 'we' descriptor in this instance and address the issue. Thanks for the comment! -Stuart > > BR, > Jani. > >
On Wed, 2019-05-29 at 17:58 +0300, Jani Nikula wrote: > On Fri, 24 May 2019, Stuart Summers <stuart.summers@intel.com> wrote: > > 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 spacing in set_sseu_info args > > use set_sseu_info to initialize sseu data when building > > device status in debugfs > > rename variables in intel_engine_types.h to avoid checkpatch > > warnings > > v3: update headers in intel_sseu.h > > v4: add const to some sseu_dev_info variables > > use sseu->eu_stride for EU stride calculations > > v5: address review comments from Tvrtko and Daniele > > v6: remove extra space in intel_sseu_get_subslices > > return the correct subslice enable in for_each_instdone > > add GEM_BUG_ON to ensure user doesn't pass invalid ss_mask size > > use printk formatted string for subslice mask > > v7: remove string.h header and rebase > > > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > > Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > > Reviewed-by: Daniele Ceraolo Spurio < > > daniele.ceraolospurio@intel.com> > > Signed-off-by: Stuart Summers <stuart.summers@intel.com> > > As this patch uncovered a latent issue in 1e40d4aea57b > ("drm/i915/cnl: > Implement WaProgramMgsrForCorrectSliceSpecificMmioReads") and got > reverted, I'll take the opportunity to comment. I acknowledge the > revert > is shooting the messenger a bit, and this will smell like maintainer > bikeshedding. I have no problem reworking regressions my series caused :) I just wasn't aware there was an issue when merging. > > Now, the first reaction looking at the commit was, it does not fare > well > on the "if a bisect landed on this commit, how happy would I be" > scale. > > While it's mostly refactoring, it could be chopped up to several > logical > and obvious steps. For example, add intel_sseu_set_info() first with > no > other changes. Add ss_stride and eu_stride to struct sseu_dev_info > separately. Add intel_sseu_get_subslices() but don't expand yet, make > it > just sseu->subslice_mask[s] first. And so on, you get the idea, a > series > of small non-functional changes followed by patches with functional > changes that stand out. Indeed patches 1-4 did this fine. This is good general feedback, thanks. I'll split this up when reposting. > > It's easy on the reviewer, it's easy on whoever git blames years down > the line. Trust me, we will. > > And it would be the commit adding intel_sseu_get_subslices(), or the > one > adding the GEM_BUG_ON()s into it, that would blow up 1e40d4aea57b > ("drm/i915/cnl: Implement > WaProgramMgsrForCorrectSliceSpecificMmioReads"). > > One more note below. > > > @@ -461,7 +461,9 @@ static int i915_getparam_ioctl(struct > > drm_device *dev, void *data, > > return -ENODEV; > > break; > > case I915_PARAM_SUBSLICE_MASK: > > - value = sseu->subslice_mask[0]; > > + /* Only copy bits from the first slice */ > > + memcpy(&value, sseu->subslice_mask, > > + min(sseu->ss_stride, (u8)sizeof(value))); > > Frankly I'd rather see this written in self-evident code without the > comment. Sure, I'll take a look. Thanks for the comments! Stuart > > BR, > Jani. > >
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index 2590f5904b67..509f02a65408 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -951,12 +951,30 @@ const char *i915_cache_level_str(struct drm_i915_private *i915, int type) } } +static inline u32 +intel_sseu_fls_subslice(const struct sseu_dev_info *sseu, u32 slice) +{ + u32 subslice; + int i; + + for (i = sseu->ss_stride - 1; i >= 0; i--) { + subslice = fls(sseu->subslice_mask[slice * sseu->ss_stride + + i]); + if (subslice) { + subslice += i * BITS_PER_BYTE; + break; + } + } + + return subslice; +} + u32 intel_calculate_mcr_s_ss_select(struct drm_i915_private *dev_priv) { const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu; u32 mcr_s_ss_select; u32 slice = fls(sseu->slice_mask); - u32 subslice = fls(sseu->subslice_mask[slice]); + u32 subslice = intel_sseu_fls_subslice(sseu, slice); if (IS_GEN(dev_priv, 10)) mcr_s_ss_select = GEN8_MCR_SLICE(slice) | @@ -1032,6 +1050,7 @@ void intel_engine_get_instdone(struct intel_engine_cs *engine, struct intel_instdone *instdone) { struct drm_i915_private *dev_priv = engine->i915; + const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu; struct intel_uncore *uncore = engine->uncore; u32 mmio_base = engine->mmio_base; int slice; @@ -1049,7 +1068,8 @@ 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(dev_priv, slice, subslice) { + for_each_instdone_slice_subslice(dev_priv, sseu, slice, + subslice) { instdone->sampler[slice][subslice] = read_subslice_reg(dev_priv, 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 40e774acc2cd..41c257f6a560 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h @@ -551,20 +551,20 @@ 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__, slice__, 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 3a4d09b80fa0..b19cd4cdcb5c 100644 --- a/drivers/gpu/drm/i915/gt/intel_hangcheck.c +++ b/drivers/gpu/drm/i915/gt/intel_hangcheck.c @@ -51,6 +51,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; @@ -72,7 +73,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/gt/intel_sseu.c b/drivers/gpu/drm/i915/gt/intel_sseu.c index a0756f006f5f..763b811f2c9d 100644 --- a/drivers/gpu/drm/i915/gt/intel_sseu.c +++ b/drivers/gpu/drm/i915/gt/intel_sseu.c @@ -8,6 +8,17 @@ #include "intel_lrc_reg.h" #include "intel_sseu.h" +void intel_sseu_set_info(struct sseu_dev_info *sseu, u8 max_slices, + u8 max_subslices, u8 max_eus_per_subslice) +{ + sseu->max_slices = max_slices; + sseu->max_subslices = max_subslices; + sseu->max_eus_per_subslice = max_eus_per_subslice; + + sseu->ss_stride = GEN_SSEU_STRIDE(sseu->max_subslices); + sseu->eu_stride = GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); +} + unsigned int intel_sseu_subslice_total(const struct sseu_dev_info *sseu) { @@ -19,10 +30,44 @@ intel_sseu_subslice_total(const struct sseu_dev_info *sseu) return total; } +void intel_sseu_copy_subslices(const struct sseu_dev_info *sseu, int slice, + u8 *to_mask) +{ + int offset = slice * sseu->ss_stride; + + memcpy(&to_mask[offset], &sseu->subslice_mask[offset], sseu->ss_stride); +} + +u32 intel_sseu_get_subslices(const struct sseu_dev_info *sseu, u8 slice) +{ + int i, offset = slice * sseu->ss_stride; + u32 mask; + + GEM_BUG_ON(slice >= sseu->max_slices); + + GEM_BUG_ON(sseu->ss_stride > sizeof(mask)); + + 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) +{ + int i, offset = slice * sseu->ss_stride; + + for (i = 0; i < sseu->ss_stride; i++) + sseu->subslice_mask[offset + i] = + (ss_mask >> (BITS_PER_BYTE * i)) & 0xff; +} + 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 b50d0401a4e2..eee21d9f320e 100644 --- a/drivers/gpu/drm/i915/gt/intel_sseu.h +++ b/drivers/gpu/drm/i915/gt/intel_sseu.h @@ -15,10 +15,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; @@ -33,6 +34,9 @@ struct sseu_dev_info { u8 max_subslices; u8 max_eus_per_subslice; + u8 ss_stride; + u8 eu_stride; + /* We don't have more than 8 eus per subslice at the moment and as we * store eus enabled using bits, no need to multiply by eus per * subslice. @@ -63,12 +67,33 @@ intel_sseu_from_device_info(const struct sseu_dev_info *sseu) return value; } +static inline bool +intel_sseu_has_subslice(const struct sseu_dev_info *sseu, int slice, + int subslice) +{ + u8 mask = sseu->subslice_mask[slice * sseu->ss_stride + + subslice / BITS_PER_BYTE]; + + return mask & BIT(subslice % BITS_PER_BYTE); +} + +void intel_sseu_set_info(struct sseu_dev_info *sseu, u8 max_slices, + u8 max_subslices, u8 max_eus_per_subslice); + unsigned int 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); +void intel_sseu_copy_subslices(const struct sseu_dev_info *sseu, int slice, + u8 *to_mask); + +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); + u32 intel_sseu_make_rpcs(struct drm_i915_private *i915, const struct intel_sseu *req_sseu); diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index ce4bcca3f83c..0ff58d6bb01f 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -783,7 +783,7 @@ wa_init_mcr(struct drm_i915_private *i915, struct i915_wa_list *wal) u32 slice = fls(sseu->slice_mask); u32 fuse3 = intel_uncore_read(&i915->uncore, GEN10_MIRROR_FUSE3); - u8 ss_mask = sseu->subslice_mask[slice]; + u32 ss_mask = intel_sseu_get_subslices(sseu, slice); u8 enabled_mask = (ss_mask | ss_mask >> GEN10_L3BANK_PAIR_COUNT) & GEN10_L3BANK_MASK; diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 7556a205f677..3f9f833007dd 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1271,6 +1271,7 @@ static void i915_instdone_info(struct drm_i915_private *dev_priv, struct seq_file *m, struct intel_instdone *instdone) { + struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu; int slice; int subslice; @@ -1286,11 +1287,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]); } @@ -4084,7 +4085,7 @@ static void gen10_sseu_device_status(struct drm_i915_private *dev_priv, continue; sseu->slice_mask |= BIT(s); - sseu->subslice_mask[s] = info->sseu.subslice_mask[s]; + intel_sseu_copy_subslices(&info->sseu, s, sseu->subslice_mask); for (ss = 0; ss < info->sseu.max_subslices; ss++) { unsigned int eu_cnt; @@ -4135,18 +4136,21 @@ static void gen9_sseu_device_status(struct drm_i915_private *dev_priv, sseu->slice_mask |= BIT(s); if (IS_GEN9_BC(dev_priv)) - sseu->subslice_mask[s] = - RUNTIME_INFO(dev_priv)->sseu.subslice_mask[s]; + intel_sseu_copy_subslices(&info->sseu, s, + sseu->subslice_mask); 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] & @@ -4163,25 +4167,23 @@ static void gen9_sseu_device_status(struct drm_i915_private *dev_priv, static void broadwell_sseu_device_status(struct drm_i915_private *dev_priv, struct sseu_dev_info *sseu) { + struct intel_runtime_info *info = RUNTIME_INFO(dev_priv); u32 slice_info = I915_READ(GEN8_GT_SLICE_INFO); int s; sseu->slice_mask = slice_info & GEN8_LSLICESTAT_MASK; if (sseu->slice_mask) { - sseu->eu_per_subslice = - RUNTIME_INFO(dev_priv)->sseu.eu_per_subslice; - for (s = 0; s < fls(sseu->slice_mask); s++) { - sseu->subslice_mask[s] = - RUNTIME_INFO(dev_priv)->sseu.subslice_mask[s]; - } + sseu->eu_per_subslice = info->sseu.eu_per_subslice; + for (s = 0; s < fls(sseu->slice_mask); s++) + intel_sseu_copy_subslices(&info->sseu, s, + sseu->subslice_mask); sseu->eu_total = sseu->eu_per_subslice * intel_sseu_subslice_total(sseu); /* subtract fused off EU(s) from enabled slice(s) */ for (s = 0; s < fls(sseu->slice_mask); s++) { - u8 subslice_7eu = - RUNTIME_INFO(dev_priv)->sseu.subslice_7eu[s]; + u8 subslice_7eu = info->sseu.subslice_7eu[s]; sseu->eu_total -= hweight8(subslice_7eu); } @@ -4228,6 +4230,7 @@ static void i915_print_sseu_info(struct seq_file *m, bool is_available_info, static int i915_sseu_status(struct seq_file *m, void *unused) { struct drm_i915_private *dev_priv = node_to_i915(m->private); + const struct intel_runtime_info *info = RUNTIME_INFO(dev_priv); struct sseu_dev_info sseu; intel_wakeref_t wakeref; @@ -4235,14 +4238,13 @@ static int i915_sseu_status(struct seq_file *m, void *unused) return -ENODEV; seq_puts(m, "SSEU Device Info\n"); - i915_print_sseu_info(m, true, &RUNTIME_INFO(dev_priv)->sseu); + i915_print_sseu_info(m, true, &info->sseu); seq_puts(m, "SSEU Device Status\n"); memset(&sseu, 0, sizeof(sseu)); - sseu.max_slices = RUNTIME_INFO(dev_priv)->sseu.max_slices; - sseu.max_subslices = RUNTIME_INFO(dev_priv)->sseu.max_subslices; - sseu.max_eus_per_subslice = - RUNTIME_INFO(dev_priv)->sseu.max_eus_per_subslice; + intel_sseu_set_info(&sseu, info->sseu.max_slices, + info->sseu.max_subslices, + info->sseu.max_eus_per_subslice); with_intel_runtime_pm(dev_priv, wakeref) { if (IS_CHERRYVIEW(dev_priv)) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index f3eea5676123..0bac9bdde1e4 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -331,7 +331,7 @@ static int i915_getparam_ioctl(struct drm_device *dev, void *data, struct pci_dev *pdev = dev_priv->drm.pdev; const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu; drm_i915_getparam_t *param = data; - int value; + int value = 0; switch (param->param) { case I915_PARAM_IRQ_ACTIVE: @@ -461,7 +461,9 @@ static int i915_getparam_ioctl(struct drm_device *dev, void *data, return -ENODEV; break; case I915_PARAM_SUBSLICE_MASK: - value = sseu->subslice_mask[0]; + /* Only copy bits from the first slice */ + memcpy(&value, sseu->subslice_mask, + min(sseu->ss_stride, (u8)sizeof(value))); if (!value) return -ENODEV; break; diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 4f85cbdddb0d..c760cc5b3388 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -408,6 +408,7 @@ static void print_error_buffers(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) { + struct sseu_dev_info *sseu = &RUNTIME_INFO(m->i915)->sseu; int slice; int subslice; @@ -423,12 +424,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]); diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c index 7b7016171057..ac8ac59c4860 100644 --- a/drivers/gpu/drm/i915/i915_query.c +++ b/drivers/gpu/drm/i915/i915_query.c @@ -37,8 +37,6 @@ static int query_topology_info(struct drm_i915_private *dev_priv, const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu; struct drm_i915_query_topology_info topo; u32 slice_length, subslice_length, eu_length, total_length; - u8 subslice_stride = GEN_SSEU_STRIDE(sseu->max_subslices); - u8 eu_stride = GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); int ret; if (query_item->flags != 0) @@ -50,8 +48,8 @@ static int query_topology_info(struct drm_i915_private *dev_priv, BUILD_BUG_ON(sizeof(u8) != sizeof(sseu->slice_mask)); slice_length = sizeof(sseu->slice_mask); - subslice_length = sseu->max_slices * subslice_stride; - eu_length = sseu->max_slices * sseu->max_subslices * eu_stride; + subslice_length = sseu->max_slices * sseu->ss_stride; + eu_length = sseu->max_slices * sseu->max_subslices * sseu->eu_stride; total_length = sizeof(topo) + slice_length + subslice_length + eu_length; @@ -69,9 +67,9 @@ static int query_topology_info(struct drm_i915_private *dev_priv, topo.max_eus_per_subslice = sseu->max_eus_per_subslice; topo.subslice_offset = slice_length; - topo.subslice_stride = subslice_stride; + topo.subslice_stride = sseu->ss_stride; topo.eu_offset = slice_length + subslice_length; - topo.eu_stride = eu_stride; + topo.eu_stride = sseu->eu_stride; if (__copy_to_user(u64_to_user_ptr(query_item->data_ptr), &topo, sizeof(topo))) diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index 97f742530fa1..3625f777f3a3 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -92,9 +92,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); @@ -117,10 +117,9 @@ void intel_device_info_dump_runtime(const struct intel_runtime_info *info, static int sseu_eu_idx(const struct sseu_dev_info *sseu, int slice, int subslice) { - int subslice_stride = GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); - int slice_stride = sseu->max_subslices * subslice_stride; + int slice_stride = sseu->max_subslices * sseu->eu_stride; - return slice * slice_stride + subslice * subslice_stride; + return slice * slice_stride + subslice * sseu->eu_stride; } static u16 sseu_get_eus(const struct sseu_dev_info *sseu, int slice, @@ -129,7 +128,7 @@ static u16 sseu_get_eus(const struct sseu_dev_info *sseu, int slice, int i, offset = sseu_eu_idx(sseu, slice, subslice); u16 eu_mask = 0; - for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); i++) { + for (i = 0; i < sseu->eu_stride; i++) { eu_mask |= ((u16)sseu->eu_mask[offset + i]) << (i * BITS_PER_BYTE); } @@ -142,7 +141,7 @@ static void sseu_set_eus(struct sseu_dev_info *sseu, int slice, int subslice, { int i, offset = sseu_eu_idx(sseu, slice, subslice); - for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); i++) { + for (i = 0; i < sseu->eu_stride; i++) { sseu->eu_mask[offset + i] = (eu_mask >> (BITS_PER_BYTE * i)) & 0xff; } @@ -159,9 +158,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); @@ -190,15 +189,10 @@ static void gen11_sseu_info_init(struct drm_i915_private *dev_priv) u8 eu_en; int s; - if (IS_ELKHARTLAKE(dev_priv)) { - sseu->max_slices = 1; - sseu->max_subslices = 4; - sseu->max_eus_per_subslice = 8; - } else { - sseu->max_slices = 1; - sseu->max_subslices = 8; - sseu->max_eus_per_subslice = 8; - } + if (IS_ELKHARTLAKE(dev_priv)) + intel_sseu_set_info(sseu, 1, 4, 8); + else + intel_sseu_set_info(sseu, 1, 8, 8); s_en = I915_READ(GEN11_GT_SLICE_ENABLE) & GEN11_GT_S_ENA_MASK; ss_en = ~I915_READ(GEN11_GT_SUBSLICE_DISABLE); @@ -207,15 +201,15 @@ static void gen11_sseu_info_init(struct drm_i915_private *dev_priv) for (s = 0; s < sseu->max_slices; s++) { if (s_en & BIT(s)) { - int ss_idx = sseu->max_subslices * s; int ss; sseu->slice_mask |= BIT(s); - sseu->subslice_mask[s] = (ss_en >> ss_idx) & ss_en_mask; - for (ss = 0; ss < sseu->max_subslices; ss++) { - if (sseu->subslice_mask[s] & BIT(ss)) + + intel_sseu_set_subslices(sseu, s, ss_en_mask); + + for (ss = 0; ss < sseu->max_subslices; ss++) + if (intel_sseu_has_subslice(sseu, s, ss)) sseu_set_eus(sseu, s, ss, eu_en); - } } } sseu->eu_per_subslice = hweight8(eu_en); @@ -235,23 +229,10 @@ static void gen10_sseu_info_init(struct drm_i915_private *dev_priv) const int eu_mask = 0xff; u32 subslice_mask, eu_en; + intel_sseu_set_info(sseu, 6, 4, 8); + sseu->slice_mask = (fuse2 & GEN10_F2_S_ENA_MASK) >> GEN10_F2_S_ENA_SHIFT; - sseu->max_slices = 6; - sseu->max_subslices = 4; - sseu->max_eus_per_subslice = 8; - - subslice_mask = (1 << 4) - 1; - subslice_mask &= ~((fuse2 & GEN10_F2_SS_DIS_MASK) >> - GEN10_F2_SS_DIS_SHIFT); - - /* - * Slice0 can have up to 3 subslices, but there are only 2 in - * slice1/2. - */ - sseu->subslice_mask[0] = subslice_mask; - for (s = 1; s < sseu->max_slices; s++) - sseu->subslice_mask[s] = subslice_mask & 0x3; /* Slice0 */ eu_en = ~I915_READ(GEN8_EU_DISABLE0); @@ -276,14 +257,22 @@ static void gen10_sseu_info_init(struct drm_i915_private *dev_priv) eu_en = ~I915_READ(GEN10_EU_DISABLE3); sseu_set_eus(sseu, 5, 1, eu_en & eu_mask); - /* Do a second pass where we mark the subslices disabled if all their - * eus are off. - */ + subslice_mask = (1 << 4) - 1; + subslice_mask &= ~((fuse2 & GEN10_F2_SS_DIS_MASK) >> + GEN10_F2_SS_DIS_SHIFT); + for (s = 0; s < sseu->max_slices; s++) { for (ss = 0; ss < sseu->max_subslices; ss++) { if (sseu_get_eus(sseu, s, ss) == 0) - sseu->subslice_mask[s] &= ~BIT(ss); + subslice_mask &= ~BIT(ss); } + + /* + * Slice0 can have up to 3 subslices, but there are only 2 in + * slice1/2. + */ + intel_sseu_set_subslices(sseu, s, s == 0 ? subslice_mask : + subslice_mask & 0x3); } sseu->eu_total = compute_eu_total(sseu); @@ -309,13 +298,12 @@ static void cherryview_sseu_info_init(struct drm_i915_private *dev_priv) { struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu; u32 fuse; + u8 subslice_mask; fuse = I915_READ(CHV_FUSE_GT); sseu->slice_mask = BIT(0); - sseu->max_slices = 1; - sseu->max_subslices = 2; - sseu->max_eus_per_subslice = 8; + intel_sseu_set_info(sseu, 1, 2, 8); if (!(fuse & CHV_FGT_DISABLE_SS0)) { u8 disabled_mask = @@ -324,7 +312,7 @@ static void cherryview_sseu_info_init(struct drm_i915_private *dev_priv) (((fuse & CHV_FGT_EU_DIS_SS0_R1_MASK) >> CHV_FGT_EU_DIS_SS0_R1_SHIFT) << 4); - sseu->subslice_mask[0] |= BIT(0); + subslice_mask |= BIT(0); sseu_set_eus(sseu, 0, 0, ~disabled_mask); } @@ -335,10 +323,12 @@ static void cherryview_sseu_info_init(struct drm_i915_private *dev_priv) (((fuse & CHV_FGT_EU_DIS_SS1_R1_MASK) >> CHV_FGT_EU_DIS_SS1_R1_SHIFT) << 4); - sseu->subslice_mask[0] |= BIT(1); + subslice_mask |= BIT(1); sseu_set_eus(sseu, 0, 1, ~disabled_mask); } + intel_sseu_set_subslices(sseu, 0, subslice_mask); + sseu->eu_total = compute_eu_total(sseu); /* @@ -371,9 +361,8 @@ static void gen9_sseu_info_init(struct drm_i915_private *dev_priv) sseu->slice_mask = (fuse2 & GEN8_F2_S_ENA_MASK) >> GEN8_F2_S_ENA_SHIFT; /* BXT has a single slice and at most 3 subslices. */ - sseu->max_slices = IS_GEN9_LP(dev_priv) ? 1 : 3; - sseu->max_subslices = IS_GEN9_LP(dev_priv) ? 3 : 4; - sseu->max_eus_per_subslice = 8; + intel_sseu_set_info(sseu, IS_GEN9_LP(dev_priv) ? 1 : 3, + IS_GEN9_LP(dev_priv) ? 3 : 4, 8); /* * The subslice disable field is global, i.e. it applies @@ -392,14 +381,14 @@ static void gen9_sseu_info_init(struct drm_i915_private *dev_priv) /* skip disabled slice */ continue; - sseu->subslice_mask[s] = subslice_mask; + intel_sseu_set_subslices(sseu, s, subslice_mask); eu_disable = I915_READ(GEN9_EU_DISABLE(s)); for (ss = 0; ss < sseu->max_subslices; ss++) { int eu_per_ss; u8 eu_disabled_mask; - if (!(sseu->subslice_mask[s] & BIT(ss))) + if (!intel_sseu_has_subslice(sseu, s, ss)) /* skip disabled subslice */ continue; @@ -472,9 +461,7 @@ static void broadwell_sseu_info_init(struct drm_i915_private *dev_priv) fuse2 = I915_READ(GEN8_FUSE2); sseu->slice_mask = (fuse2 & GEN8_F2_S_ENA_MASK) >> GEN8_F2_S_ENA_SHIFT; - sseu->max_slices = 3; - sseu->max_subslices = 3; - sseu->max_eus_per_subslice = 8; + intel_sseu_set_info(sseu, 3, 3, 8); /* * The subslice disable field is global, i.e. it applies @@ -501,18 +488,19 @@ static void broadwell_sseu_info_init(struct drm_i915_private *dev_priv) /* skip disabled slice */ continue; - sseu->subslice_mask[s] = subslice_mask; + intel_sseu_set_subslices(sseu, s, subslice_mask); for (ss = 0; ss < sseu->max_subslices; ss++) { u8 eu_disabled_mask; u32 n_disabled; - if (!(sseu->subslice_mask[s] & BIT(ss))) + if (!intel_sseu_has_subslice(sseu, s, ss)) /* skip disabled subslice */ continue; eu_disabled_mask = - eu_disable[s] >> (ss * sseu->max_eus_per_subslice); + eu_disable[s] >> + (ss * sseu->max_eus_per_subslice); sseu_set_eus(sseu, s, ss, ~eu_disabled_mask); @@ -552,6 +540,7 @@ static void haswell_sseu_info_init(struct drm_i915_private *dev_priv) struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu; u32 fuse1; int s, ss; + u32 subslice_mask; /* * There isn't a register to tell us how many slices/subslices. We @@ -563,22 +552,18 @@ static void haswell_sseu_info_init(struct drm_i915_private *dev_priv) /* fall through */ case 1: sseu->slice_mask = BIT(0); - sseu->subslice_mask[0] = BIT(0); + subslice_mask = BIT(0); break; case 2: sseu->slice_mask = BIT(0); - sseu->subslice_mask[0] = BIT(0) | BIT(1); + subslice_mask = BIT(0) | BIT(1); break; case 3: sseu->slice_mask = BIT(0) | BIT(1); - sseu->subslice_mask[0] = BIT(0) | BIT(1); - sseu->subslice_mask[1] = BIT(0) | BIT(1); + subslice_mask = BIT(0) | BIT(1); break; } - sseu->max_slices = hweight8(sseu->slice_mask); - sseu->max_subslices = hweight8(sseu->subslice_mask[0]); - fuse1 = I915_READ(HSW_PAVP_FUSE1); switch ((fuse1 & HSW_F1_EU_DIS_MASK) >> HSW_F1_EU_DIS_SHIFT) { default: @@ -595,9 +580,14 @@ static void haswell_sseu_info_init(struct drm_i915_private *dev_priv) sseu->eu_per_subslice = 6; break; } - sseu->max_eus_per_subslice = sseu->eu_per_subslice; + + intel_sseu_set_info(sseu, hweight8(sseu->slice_mask), + hweight8(subslice_mask), + sseu->eu_per_subslice); for (s = 0; s < sseu->max_slices; s++) { + intel_sseu_set_subslices(sseu, s, subslice_mask); + for (ss = 0; ss < sseu->max_subslices; ss++) { sseu_set_eus(sseu, s, ss, (1UL << sseu->eu_per_subslice) - 1);
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 spacing in set_sseu_info args use set_sseu_info to initialize sseu data when building device status in debugfs rename variables in intel_engine_types.h to avoid checkpatch warnings v3: update headers in intel_sseu.h v4: add const to some sseu_dev_info variables use sseu->eu_stride for EU stride calculations v5: address review comments from Tvrtko and Daniele v6: remove extra space in intel_sseu_get_subslices return the correct subslice enable in for_each_instdone add GEM_BUG_ON to ensure user doesn't pass invalid ss_mask size use printk formatted string for subslice mask v7: remove string.h header and rebase Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Signed-off-by: Stuart Summers <stuart.summers@intel.com> --- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 24 +++- drivers/gpu/drm/i915/gt/intel_engine_types.h | 30 ++--- drivers/gpu/drm/i915/gt/intel_hangcheck.c | 3 +- drivers/gpu/drm/i915/gt/intel_sseu.c | 47 ++++++- drivers/gpu/drm/i915/gt/intel_sseu.h | 27 +++- drivers/gpu/drm/i915/gt/intel_workarounds.c | 2 +- drivers/gpu/drm/i915/i915_debugfs.c | 40 +++--- drivers/gpu/drm/i915/i915_drv.c | 6 +- drivers/gpu/drm/i915/i915_gpu_error.c | 5 +- drivers/gpu/drm/i915/i915_query.c | 10 +- drivers/gpu/drm/i915/intel_device_info.c | 122 +++++++++---------- 11 files changed, 200 insertions(+), 116 deletions(-)