Re: [PATCH v5 0/6] i915: SSEU handling updates

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
> 



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux