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@xxxxxxxxxxxxxxx> > Cc: Balasubramani Vivekanandan <balasubramani.vivekanandan@xxxxxxxxx> 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@xxxxxxxxx> > > 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 >