On 01.06.2022 08:07, Matt Roper wrote:
> This series reworks i915's internal handling of slice/subslice/EU (SSEU)
> data to represent platforms like Xe_HP in a more natural manner and to
> prepare for future platforms where the masks will need to grow in size.
> One key idea of this series is that although we have a fixed ABI to
> convey SSEU data to userspace (i.e., multiple u8[] arrays with data
> stored at different strides), we don't need to use this cumbersome
> format for the driver's own internal storage. As long as we can convert
> into the uapi form properly when responding to the I915_QUERY ioctl,
> it's preferable to use an internal storage format that's easier for the
> driver to work with.
>
> Another key point here is that we're reaching the point where subslice
> (DSS) masks will soon not fit within simple u32/u64 integer values.
> Xe_HP SDV and DG2 platforms today have subslice (DSS) masks that are 32
> bits, which maxes out the current storage of a u32. With PVC the masks
> are represented by a pair of 32-bit registers, requiring a bump up to at
> least 64-bits of storage internally. We could switch to u64 for that in
> the short term, but since we already know that upcoming architectures
> intend to provide DSS fuse bits via three or more registers it's best to
> switch to a representation that's more future-proof but still easy to
> work with in the driver code. To accomodate this, we start storing our
> subslice mask for Xe_HP and beyond in a new typedef that can be
> processed by the linux/bitmap.h operations.
>
> Finally, since no userspace for Xe_HP or beyond is using the legacy
> I915_GETPARAM ioctl lookups for I915_PARAM_SLICE_MASK and
> I915_PARAM_SUBSLICE_MASK (since they've migrated to the more flexible
> I915_QUERY ioctl that can return more than a simple u32 value), we take
> the opportunity to officially drop support for those GETPARAM lookups on
> modern platforms. Maintaining support for these GETPARAM lookups don't
> make sense for a number of reasons:
>
> * Traditional slices no longer exist, and newer ideas like gslices,
> cslices, mslices, etc. aren't something userspace needs to query
> since it can be inferred from other information.
> * The GETPARAM ioctl doesn't have a way to distinguish between geometry
> subslice masks and compute subslice masks, which are distinct on
> Xe_HP and beyond.
> * The I915_GETPARAM ioctl is limited to returning a 32-bit value, so
> when subslice masks begin to exceed 32-bits (on PVC), it simply can't
> return the entire mask.
> * The GETPARAM ioctl doesn't have a way to give sensible information
> for multi-tile devices.
>
> v2:
> - Switch to union of hsw/xehp formats to keep the representation in a
> natural format for different types of hardware.
> - Avoid accessing internals of intel_sseu_ss_mask_t directly outside of
> intel_sseu.[ch].
> - Include PVC SSEU which needs the larger SS mask storage enabled by
> this series.
>
> v3:
> - Correct a BIT(s) typo that should have been BIT(ss), causing
> incorrect handling on gen9 platforms.
>
> v4:
> - Eliminate sseu->{ss,eu}_stride fields and just calculate the proper
> value in the UAPI code that needs them.
> - Handle unwanted ~u8 sign extension at the callsite instead of inside
> sseu_set_eus.
> - Use BITMAP_BITS() macro rather than passing I915_MAX_SS_FUSE_BITS
> around directly to bitmap operations.
> - Improved debugfs / dmesg reporting for Xe_HP dumps
> - Various assertion check improvements.
>
> v5:
> - Rebase to latest drm-tip (resolve trivial conflicts)
> - Move XEHP_BITMAP_BITS() to the header so that we can also replace a usage of
> I915_MAX_SS_FUSE_BITS in one of the inline functions. (Bala)
> - Change the local variable in intel_slicemask_from_xehp_dssmask() from u16 to
> 'unsigned long' to make it a bit more future-proof.
> - Incorporate ack's received from Tvrtko and Lionel.
>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
> Cc: Balasubramani Vivekanandan <balasubramani.vivekanandan at intel.com>
Patch looks good to me. I do not have any comments except for the
request to please check the checkpatch warnings.
Reviewed-by: Balasubramani Vivekanandan <balasubramani.vivekanandan at intel.com>
>
> Matt Roper (6):
> drm/i915/xehp: Use separate sseu init function
> drm/i915/xehp: Drop GETPARAM lookups of I915_PARAM_[SUB]SLICE_MASK
> drm/i915/sseu: Simplify gen11+ SSEU handling
> drm/i915/sseu: Don't try to store EU mask internally in UAPI format
> drm/i915/sseu: Disassociate internal subslice mask representation from
> uapi
> drm/i915/pvc: Add SSEU changes
>
> 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_gt_regs.h | 1 +
> drivers/gpu/drm/i915/gt/intel_sseu.c | 450 ++++++++++++-------
> drivers/gpu/drm/i915/gt/intel_sseu.h | 92 ++--
> drivers/gpu/drm/i915/gt/intel_sseu_debugfs.c | 30 +-
> drivers/gpu/drm/i915/gt/intel_workarounds.c | 24 +-
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> drivers/gpu/drm/i915/i915_getparam.c | 11 +-
> drivers/gpu/drm/i915/i915_pci.c | 3 +-
> drivers/gpu/drm/i915/i915_query.c | 26 +-
> drivers/gpu/drm/i915/intel_device_info.h | 1 +
> 13 files changed, 397 insertions(+), 264 deletions(-)
>
> --
> 2.35.3
>