On Wed, 29 May 2019, Nathan Chancellor <[email protected]> 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 <[email protected]> >> Cc: Lionel Landwerlin <[email protected]> >> Acked-by: Lionel Landwerlin <[email protected]> >> Reviewed-by: Daniele Ceraolo Spurio <[email protected]> >> Signed-off-by: Stuart Summers <[email protected]> >> --- > > <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. -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
