Re: [Intel-gfx] [PATCH 6/9] drm/i915: Add function to determine if a slice has a subslice
Quoting Summers, Stuart (2019-08-20 20:01:05) > On Tue, 2019-08-20 at 11:53 +0100, Chris Wilson wrote: > > Quoting Stuart Summers (2019-08-19 22:50:00) > > > Add a new function to determine whether a particular slice > > > has a given subslice. > > > > > > Signed-off-by: Stuart Summers > > > --- > > > drivers/gpu/drm/i915/gt/intel_sseu.h | 10 ++ > > > drivers/gpu/drm/i915/intel_device_info.c | 9 - > > > 2 files changed, 14 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.h > > > b/drivers/gpu/drm/i915/gt/intel_sseu.h > > > index 2261d4e7d98b..0ecc1c35a7a1 100644 > > > --- a/drivers/gpu/drm/i915/gt/intel_sseu.h > > > +++ b/drivers/gpu/drm/i915/gt/intel_sseu.h > > > @@ -66,6 +66,16 @@ 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]; > > > + > > > > Suggestion: > > > > GEM_BUG_ON(subslice >= sseu->ss_slice); > > Thanks Chris and makes sense. I think this should instead be (ss / > BITS_PER_BYTE >= ss_stride). Don't assume I put more thought into than dropping a hint to catch programmer errors that would lead to UB. :) > I'll post an update hopefully today on this. Let me know if you'd like > me to drop your R-B with the above change. If CI doesn't explode, keep the r-b ;) -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/9] drm/i915: Add function to determine if a slice has a subslice
On Tue, 2019-08-20 at 11:53 +0100, Chris Wilson wrote: > Quoting Stuart Summers (2019-08-19 22:50:00) > > Add a new function to determine whether a particular slice > > has a given subslice. > > > > Signed-off-by: Stuart Summers > > --- > > drivers/gpu/drm/i915/gt/intel_sseu.h | 10 ++ > > drivers/gpu/drm/i915/intel_device_info.c | 9 - > > 2 files changed, 14 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.h > > b/drivers/gpu/drm/i915/gt/intel_sseu.h > > index 2261d4e7d98b..0ecc1c35a7a1 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_sseu.h > > +++ b/drivers/gpu/drm/i915/gt/intel_sseu.h > > @@ -66,6 +66,16 @@ 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]; > > + > > Suggestion: > > GEM_BUG_ON(subslice >= sseu->ss_slice); Thanks Chris and makes sense. I think this should instead be (ss / BITS_PER_BYTE >= ss_stride). I'll post an update hopefully today on this. Let me know if you'd like me to drop your R-B with the above change. Thanks, Stuart > > > + return mask & BIT(subslice % BITS_PER_BYTE); > > +} > > Reviewed-by: Chris Wilson > -Chris smime.p7s Description: S/MIME cryptographic signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/9] drm/i915: Add function to determine if a slice has a subslice
Quoting Stuart Summers (2019-08-19 22:50:00) > Add a new function to determine whether a particular slice > has a given subslice. > > Signed-off-by: Stuart Summers > --- > drivers/gpu/drm/i915/gt/intel_sseu.h | 10 ++ > drivers/gpu/drm/i915/intel_device_info.c | 9 - > 2 files changed, 14 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.h > b/drivers/gpu/drm/i915/gt/intel_sseu.h > index 2261d4e7d98b..0ecc1c35a7a1 100644 > --- a/drivers/gpu/drm/i915/gt/intel_sseu.h > +++ b/drivers/gpu/drm/i915/gt/intel_sseu.h > @@ -66,6 +66,16 @@ 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]; > + Suggestion: GEM_BUG_ON(subslice >= sseu->ss_slice); > + return mask & BIT(subslice % BITS_PER_BYTE); > +} Reviewed-by: Chris Wilson -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/9] drm/i915: Add function to determine if a slice has a subslice
Add a new function to determine whether a particular slice has a given subslice. Signed-off-by: Stuart Summers --- drivers/gpu/drm/i915/gt/intel_sseu.h | 10 ++ drivers/gpu/drm/i915/intel_device_info.c | 9 - 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.h b/drivers/gpu/drm/i915/gt/intel_sseu.h index 2261d4e7d98b..0ecc1c35a7a1 100644 --- a/drivers/gpu/drm/i915/gt/intel_sseu.h +++ b/drivers/gpu/drm/i915/gt/intel_sseu.h @@ -66,6 +66,16 @@ 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); diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index 2320613a51ac..ff3d6508fd17 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -210,10 +210,9 @@ static void gen11_sseu_info_init(struct drm_i915_private *dev_priv) intel_sseu_set_subslices(sseu, s, (ss_en >> ss_idx) & ss_en_mask); - for (ss = 0; ss < sseu->max_subslices; ss++) { - if (sseu->subslice_mask[s] & BIT(ss)) + 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); @@ -395,7 +394,7 @@ static void gen9_sseu_info_init(struct drm_i915_private *dev_priv) 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; @@ -501,7 +500,7 @@ static void broadwell_sseu_info_init(struct drm_i915_private *dev_priv) 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; -- 2.22.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/9] drm/i915: Add function to determine if a slice has a subslice
Add a new function to determine whether a particular slice has a given subslice. Signed-off-by: Stuart Summers --- drivers/gpu/drm/i915/gt/intel_sseu.h | 10 ++ drivers/gpu/drm/i915/intel_device_info.c | 9 - 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.h b/drivers/gpu/drm/i915/gt/intel_sseu.h index 2261d4e7d98b..0ecc1c35a7a1 100644 --- a/drivers/gpu/drm/i915/gt/intel_sseu.h +++ b/drivers/gpu/drm/i915/gt/intel_sseu.h @@ -66,6 +66,16 @@ 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); diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index 2320613a51ac..ff3d6508fd17 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -210,10 +210,9 @@ static void gen11_sseu_info_init(struct drm_i915_private *dev_priv) intel_sseu_set_subslices(sseu, s, (ss_en >> ss_idx) & ss_en_mask); - for (ss = 0; ss < sseu->max_subslices; ss++) { - if (sseu->subslice_mask[s] & BIT(ss)) + 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); @@ -395,7 +394,7 @@ static void gen9_sseu_info_init(struct drm_i915_private *dev_priv) 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; @@ -501,7 +500,7 @@ static void broadwell_sseu_info_init(struct drm_i915_private *dev_priv) 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; -- 2.22.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/9] drm/i915: Add function to determine if a slice has a subslice
Quoting Stuart Summers (2019-08-07 17:58:29) > Add a new function to determine whether a particular slice > has a given subslice. > +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); One might ask why haven't we switched to bitmap.h by this point? -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/9] drm/i915: Add function to determine if a slice has a subslice
Add a new function to determine whether a particular slice has a given subslice. Signed-off-by: Stuart Summers --- drivers/gpu/drm/i915/gt/intel_sseu.h | 10 ++ drivers/gpu/drm/i915/intel_device_info.c | 9 - 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.h b/drivers/gpu/drm/i915/gt/intel_sseu.h index 2261d4e7d98b..0ecc1c35a7a1 100644 --- a/drivers/gpu/drm/i915/gt/intel_sseu.h +++ b/drivers/gpu/drm/i915/gt/intel_sseu.h @@ -66,6 +66,16 @@ 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); diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index 2320613a51ac..ff3d6508fd17 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -210,10 +210,9 @@ static void gen11_sseu_info_init(struct drm_i915_private *dev_priv) intel_sseu_set_subslices(sseu, s, (ss_en >> ss_idx) & ss_en_mask); - for (ss = 0; ss < sseu->max_subslices; ss++) { - if (sseu->subslice_mask[s] & BIT(ss)) + 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); @@ -395,7 +394,7 @@ static void gen9_sseu_info_init(struct drm_i915_private *dev_priv) 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; @@ -501,7 +500,7 @@ static void broadwell_sseu_info_init(struct drm_i915_private *dev_priv) 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; -- 2.21.0.5.gaeb582a983 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/9] drm/i915: Add function to determine if a slice has a subslice
Add a new function to determine whether a particular slice has a given subslice. Signed-off-by: Stuart Summers --- drivers/gpu/drm/i915/gt/intel_sseu.h | 10 ++ drivers/gpu/drm/i915/intel_device_info.c | 9 - 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.h b/drivers/gpu/drm/i915/gt/intel_sseu.h index 2261d4e7d98b..0ecc1c35a7a1 100644 --- a/drivers/gpu/drm/i915/gt/intel_sseu.h +++ b/drivers/gpu/drm/i915/gt/intel_sseu.h @@ -66,6 +66,16 @@ 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); diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index 22b59fdb31fc..723b1fde5fc4 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -210,10 +210,9 @@ static void gen11_sseu_info_init(struct drm_i915_private *dev_priv) intel_sseu_set_subslices(sseu, s, (ss_en >> ss_idx) & ss_en_mask); - for (ss = 0; ss < sseu->max_subslices; ss++) { - if (sseu->subslice_mask[s] & BIT(ss)) + 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); @@ -399,7 +398,7 @@ static void gen9_sseu_info_init(struct drm_i915_private *dev_priv) 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; @@ -505,7 +504,7 @@ static void broadwell_sseu_info_init(struct drm_i915_private *dev_priv) 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; -- 2.21.0.5.gaeb582a983 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/9] drm/i915: Add function to determine if a slice has a subslice
Add a new function to determine whether a particular slice has a given subslice. Signed-off-by: Stuart Summers --- drivers/gpu/drm/i915/gt/intel_sseu.h | 10 ++ drivers/gpu/drm/i915/intel_device_info.c | 9 - 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.h b/drivers/gpu/drm/i915/gt/intel_sseu.h index 2261d4e7d98b..0ecc1c35a7a1 100644 --- a/drivers/gpu/drm/i915/gt/intel_sseu.h +++ b/drivers/gpu/drm/i915/gt/intel_sseu.h @@ -66,6 +66,16 @@ 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); diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index 22b59fdb31fc..723b1fde5fc4 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -210,10 +210,9 @@ static void gen11_sseu_info_init(struct drm_i915_private *dev_priv) intel_sseu_set_subslices(sseu, s, (ss_en >> ss_idx) & ss_en_mask); - for (ss = 0; ss < sseu->max_subslices; ss++) { - if (sseu->subslice_mask[s] & BIT(ss)) + 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); @@ -399,7 +398,7 @@ static void gen9_sseu_info_init(struct drm_i915_private *dev_priv) 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; @@ -505,7 +504,7 @@ static void broadwell_sseu_info_init(struct drm_i915_private *dev_priv) 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; -- 2.21.0.5.gaeb582a983 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx