Re: [Intel-gfx] [PATCH v5 5/6] drm/i915/sseu: Disassociate internal subslice mask representation from uapi
On Wed, Jun 01, 2022 at 01:48:56PM +0530, Balasubramani Vivekanandan wrote:
> On 23.05.2022 13:45, Matt Roper wrote:
> > As with EU masks, it's easier to store subslice/DSS masks internally in
> > a format that's more natural for the driver to work with, and then only
> > covert into the u8[] uapi form when the query ioctl is invoked. Since
> > the hardware design changed significantly with Xe_HP, we'll use a union
> > to choose between the old "hsw-style" subslice masks or the newer xehp
> > mask. HSW-style masks will be stored in an array of u8's, indexed by
> > slice (there's never more than 6 subslices per slice on older
> > platforms). For Xe_HP and beyond where slices no longer exist, we only
> > need a single bitmask. However we already know that this mask is
> > eventually going to grow too large for a simple u64 to hold, so we'll
> > represent it in a manner that can be operated on by the utilities in
> > linux/bitmap.h.
> >
> > v2:
> > - Fix typo: BIT(s) -> BIT(ss) in gen9_sseu_device_status()
> >
> > v3:
> > - Eliminate sseu->ss_stride and just calculate the stride while
> >specifically handling uapi. (Tvrtko)
> > - Use BITMAP_BITS() macro to refer to size of masks rather than
> >passing I915_MAX_SS_FUSE_BITS directly. (Tvrtko)
> > - Report compute/geometry DSS masks separately when dumping Xe_HP SSEU
> >info. (Tvrtko)
> > - Restore dropped range checks to intel_sseu_has_subslice(). (Tvrtko)
> >
> > v4:
> > - Make the bitmap size macro check the size of the .xehp field rather
> >than the containing union. (Tvrtko)
> > - Don't add GEM_BUG_ON() intel_sseu_has_subslice()'s check for whether
> >slice or subslice ID exceed sseu->max_[sub]slices; various loops
> >in the driver are expected to exceed these, so we should just
> >silently return 'false.'
> >
> > Cc: Tvrtko Ursulin
> > Signed-off-by: Matt Roper
> > ---
> > drivers/gpu/drm/i915/gem/i915_gem_context.c | 5 +-
> > drivers/gpu/drm/i915/gt/intel_engine_cs.c| 4 +-
> > drivers/gpu/drm/i915/gt/intel_gt.c | 12 +-
> > drivers/gpu/drm/i915/gt/intel_sseu.c | 261 +++
> > drivers/gpu/drm/i915/gt/intel_sseu.h | 76 --
> > drivers/gpu/drm/i915/gt/intel_sseu_debugfs.c | 30 +--
> > drivers/gpu/drm/i915/gt/intel_workarounds.c | 24 +-
> > drivers/gpu/drm/i915/i915_getparam.c | 3 +-
> > drivers/gpu/drm/i915/i915_query.c| 13 +-
> > 9 files changed, 241 insertions(+), 187 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > index ab4c5ab28e4d..a3bb73f5d53b 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > @@ -1875,6 +1875,7 @@ i915_gem_user_to_context_sseu(struct intel_gt *gt,
> > {
> > const struct sseu_dev_info *device = >->info.sseu;
> > struct drm_i915_private *i915 = gt->i915;
> > + unsigned int dev_subslice_mask = intel_sseu_get_hsw_subslices(device,
> > 0);
> >
> > /* No zeros in any field. */
> > if (!user->slice_mask || !user->subslice_mask ||
> > @@ -1901,7 +1902,7 @@ i915_gem_user_to_context_sseu(struct intel_gt *gt,
> > if (user->slice_mask & ~device->slice_mask)
> > return -EINVAL;
> >
> > - if (user->subslice_mask & ~device->subslice_mask[0])
> > + if (user->subslice_mask & ~dev_subslice_mask)
> > return -EINVAL;
> >
> > if (user->max_eus_per_subslice > device->max_eus_per_subslice)
> > @@ -1915,7 +1916,7 @@ i915_gem_user_to_context_sseu(struct intel_gt *gt,
> > /* Part specific restrictions. */
> > if (GRAPHICS_VER(i915) == 11) {
> > unsigned int hw_s = hweight8(device->slice_mask);
> > - unsigned int hw_ss_per_s = hweight8(device->subslice_mask[0]);
> > + unsigned int hw_ss_per_s = hweight8(dev_subslice_mask);
> > unsigned int req_s = hweight8(context->slice_mask);
> > unsigned int req_ss = hweight8(context->subslice_mask);
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > index 1adbf34c3632..f0acf8518a51 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > @@ -674,8 +674,8 @@ static void engine_mask_apply_compute_fuses(struct
> > intel_gt *gt)
> > if (GRAPHICS_VER_FULL(i915) < IP_VER(12, 50))
> > return;
> >
> > - ccs_mask =
> > intel_slicemask_from_dssmask(intel_sseu_get_compute_subslices(&info->sseu),
> > - ss_per_ccs);
> > + ccs_mask =
> > intel_slicemask_from_xehp_dssmask(info->sseu.compute_subslice_mask,
> > +ss_per_ccs);
> > /*
> > * If all DSS in a quadrant are fused off, the corresponding CCS
> > * engine is not available for use.
> > diff --git a/drivers/gpu/drm/i915/gt/in
Re: [Intel-gfx] [PATCH v5 5/6] drm/i915/sseu: Disassociate internal subslice mask representation from uapi
On 23.05.2022 13:45, Matt Roper wrote:
> As with EU masks, it's easier to store subslice/DSS masks internally in
> a format that's more natural for the driver to work with, and then only
> covert into the u8[] uapi form when the query ioctl is invoked. Since
> the hardware design changed significantly with Xe_HP, we'll use a union
> to choose between the old "hsw-style" subslice masks or the newer xehp
> mask. HSW-style masks will be stored in an array of u8's, indexed by
> slice (there's never more than 6 subslices per slice on older
> platforms). For Xe_HP and beyond where slices no longer exist, we only
> need a single bitmask. However we already know that this mask is
> eventually going to grow too large for a simple u64 to hold, so we'll
> represent it in a manner that can be operated on by the utilities in
> linux/bitmap.h.
>
> v2:
> - Fix typo: BIT(s) -> BIT(ss) in gen9_sseu_device_status()
>
> v3:
> - Eliminate sseu->ss_stride and just calculate the stride while
>specifically handling uapi. (Tvrtko)
> - Use BITMAP_BITS() macro to refer to size of masks rather than
>passing I915_MAX_SS_FUSE_BITS directly. (Tvrtko)
> - Report compute/geometry DSS masks separately when dumping Xe_HP SSEU
>info. (Tvrtko)
> - Restore dropped range checks to intel_sseu_has_subslice(). (Tvrtko)
>
> v4:
> - Make the bitmap size macro check the size of the .xehp field rather
>than the containing union. (Tvrtko)
> - Don't add GEM_BUG_ON() intel_sseu_has_subslice()'s check for whether
>slice or subslice ID exceed sseu->max_[sub]slices; various loops
>in the driver are expected to exceed these, so we should just
>silently return 'false.'
>
> Cc: Tvrtko Ursulin
> Signed-off-by: Matt Roper
> ---
> drivers/gpu/drm/i915/gem/i915_gem_context.c | 5 +-
> drivers/gpu/drm/i915/gt/intel_engine_cs.c| 4 +-
> drivers/gpu/drm/i915/gt/intel_gt.c | 12 +-
> drivers/gpu/drm/i915/gt/intel_sseu.c | 261 +++
> drivers/gpu/drm/i915/gt/intel_sseu.h | 76 --
> drivers/gpu/drm/i915/gt/intel_sseu_debugfs.c | 30 +--
> drivers/gpu/drm/i915/gt/intel_workarounds.c | 24 +-
> drivers/gpu/drm/i915/i915_getparam.c | 3 +-
> drivers/gpu/drm/i915/i915_query.c| 13 +-
> 9 files changed, 241 insertions(+), 187 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index ab4c5ab28e4d..a3bb73f5d53b 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -1875,6 +1875,7 @@ i915_gem_user_to_context_sseu(struct intel_gt *gt,
> {
> const struct sseu_dev_info *device = >->info.sseu;
> struct drm_i915_private *i915 = gt->i915;
> + unsigned int dev_subslice_mask = intel_sseu_get_hsw_subslices(device,
> 0);
>
> /* No zeros in any field. */
> if (!user->slice_mask || !user->subslice_mask ||
> @@ -1901,7 +1902,7 @@ i915_gem_user_to_context_sseu(struct intel_gt *gt,
> if (user->slice_mask & ~device->slice_mask)
> return -EINVAL;
>
> - if (user->subslice_mask & ~device->subslice_mask[0])
> + if (user->subslice_mask & ~dev_subslice_mask)
> return -EINVAL;
>
> if (user->max_eus_per_subslice > device->max_eus_per_subslice)
> @@ -1915,7 +1916,7 @@ i915_gem_user_to_context_sseu(struct intel_gt *gt,
> /* Part specific restrictions. */
> if (GRAPHICS_VER(i915) == 11) {
> unsigned int hw_s = hweight8(device->slice_mask);
> - unsigned int hw_ss_per_s = hweight8(device->subslice_mask[0]);
> + unsigned int hw_ss_per_s = hweight8(dev_subslice_mask);
> unsigned int req_s = hweight8(context->slice_mask);
> unsigned int req_ss = hweight8(context->subslice_mask);
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 1adbf34c3632..f0acf8518a51 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -674,8 +674,8 @@ static void engine_mask_apply_compute_fuses(struct
> intel_gt *gt)
> if (GRAPHICS_VER_FULL(i915) < IP_VER(12, 50))
> return;
>
> - ccs_mask =
> intel_slicemask_from_dssmask(intel_sseu_get_compute_subslices(&info->sseu),
> - ss_per_ccs);
> + ccs_mask =
> intel_slicemask_from_xehp_dssmask(info->sseu.compute_subslice_mask,
> + ss_per_ccs);
> /*
>* If all DSS in a quadrant are fused off, the corresponding CCS
>* engine is not available for use.
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
> b/drivers/gpu/drm/i915/gt/intel_gt.c
> index 034182f85501..2921f510642f 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -133,13 +133,6 @@ static const struct in
