Re: [PATCH 1/4] drm/xe: Add has_sem64 bit to graphics/media IP descriptors

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] drm/xe: missing has_sem64 in graphics_xe3p_xpc disables sem64
  on 35.11 and 35.03
- [Low] drm/xe: commit message mismatch with has_sem64 resolution in
  xe_info_init()
--

commit aa9a045209e3c606e5aa5ffac66c1d9a9153a146
Author: Tangudu Tilak Tirumalesh <tilak.tirumalesh.tangudu@intel.com>

drm/xe: Add has_sem64 bit to graphics/media IP descriptors

This commit adds a has_sem64 capability bit to the graphics and media IP
descriptors and populates gt->info.has_sem64 for Xe3p platforms.

>     The bit indicates whether the GT's command streamer supports the
>     MI_SEMAPHORE_WAIT_64 instruction. A platform's CS must not mix
>     MI_SEMAPHORE_WAIT and MI_SEMAPHORE_WAIT_64 within a context or across
>     context submissions, so both the primary and media GT must agree before
>     either one enables sem64; resolve has_sem64 once in xe_info_init() and
>     hand the single value to both GT allocators.

[Severity: Low]
Does the commit message match how has_sem64 is resolved?
alloc_primary_gt() and alloc_media_gt() each compute gt->info.has_sem64
independently from their IP descriptor and stepping, and xe_info_init()
afterwards clears both fields if either GT does not support it, rather
than resolving a single value in xe_info_init() and passing it to the GT
allocators.

> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 7fba5295c33e6..5a147a577a098 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -127,6 +127,7 @@ static const struct xe_graphics_desc graphics_xe3p_lpg = {
>  	.multi_queue_engine_class_mask = BIT(XE_ENGINE_CLASS_COPY) | BIT(XE_ENGINE_CLASS_COMPUTE),
>  	.num_geometry_xecore_fuse_regs = 3,
>  	.num_compute_xecore_fuse_regs = 3,
> +	.has_sem64 = 1,
>  };
>  
>  static const struct xe_graphics_desc graphics_xe3p_xpc = {
> @@ -153,11 +154,20 @@ static const struct xe_media_desc media_xelpmp = {
>  	.hw_engine_mask =
>  		GENMASK(XE_HW_ENGINE_VCS7, XE_HW_ENGINE_VCS0) |
>  		GENMASK(XE_HW_ENGINE_VECS3, XE_HW_ENGINE_VECS0) |
> -		BIT(XE_HW_ENGINE_GSCCS0)
> +		BIT(XE_HW_ENGINE_GSCCS0),
> +};
> +
> +static const struct xe_media_desc media_xe3p_lpm = {
> +	.hw_engine_mask =
> +		GENMASK(XE_HW_ENGINE_VCS7, XE_HW_ENGINE_VCS0) |
> +		GENMASK(XE_HW_ENGINE_VECS3, XE_HW_ENGINE_VECS0) |
> +		BIT(XE_HW_ENGINE_GSCCS0),
> +	.has_sem64 = 1,
>  };
>  
>  static const struct xe_media_desc media_xe3p_hpm = {
>  	.has_uncorrectable_error_reporting = 1,
> +	.has_sem64 = 1,

[Severity: Medium]
Should graphics_xe3p_xpc also set .has_sem64 = 1?

alloc_primary_gt() scopes the stepping B0 check to GRAPHICS_VERx100(xe) ==
3510 (graphics_xe3p_lpg), and media_xe3p_hpm (35.03) enables has_sem64.
Because graphics_xe3p_xpc (35.11) omits .has_sem64 = 1,
tile->primary_gt->info.has_sem64 is false on Xe3p_XPC platforms, and the
cross-GT check in xe_info_init() then clears
tile->media_gt->info.has_sem64 on the paired Xe3p_HPM media GT as well,
leaving sem64 disabled on both GTs.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113155734.4164230-1-tilak.tirumalesh.tangudu@intel.com?part=1