Re: [RFC PATCH 1/3] drm/xe: Add has_sem64 bit for MI_SEMAPHORE_WAIT_64 support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/xe: missing has_sem64 flag in graphics_xe3p_xpc for Graphics IP 35.11
--

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

drm/xe: Add has_sem64 bit for MI_SEMAPHORE_WAIT_64 support

This commit adds the MI_SEMAPHORE_WAIT_64 instruction definition and a
has_sem64 flag to the graphics and media IP descriptors for Xe3p platforms
as part of Wa_16029281913. It also filters out steppings prior to B0 on
Graphics version 35.10 and Media version 35.00 when resolving
gt->info.has_sem64.

> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 5da35d36526fe..ee561ce93835f 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,
>  	.hw_engine_mask =
>  		GENMASK(XE_HW_ENGINE_VCS7, XE_HW_ENGINE_VCS0) |
>  		GENMASK(XE_HW_ENGINE_VECS3, XE_HW_ENGINE_VECS0) |
> @@ -198,7 +208,7 @@ static const struct xe_ip media_ips[] = {
>  	{ 2000, "Xe2_LPM", &media_xelpmp },
>  	{ 3000, "Xe3_LPM", &media_xelpmp },
>  	{ 3002, "Xe3_LPM", &media_xelpmp },
> -	{ 3500, "Xe3p_LPM", &media_xelpmp },
> +	{ 3500, "Xe3p_LPM", &media_xe3p_lpm },
>  	{ 3503, "Xe3p_HPM", &media_xe3p_hpm },
>  };
>  
> @@ -899,6 +909,9 @@ static struct xe_gt *alloc_primary_gt(struct xe_tile *tile,
>  	gt->info.has_indirect_ring_state = graphics_desc->has_indirect_ring_state;
>  	gt->info.has_uncorrectable_error_reporting =
>  		graphics_desc->has_uncorrectable_error_reporting;
> +	/* excludes configs that don't support sem64 */
> +	gt->info.has_sem64 = graphics_desc->has_sem64 &&
> +		!(GRAPHICS_VERx100(xe) == 3510 && xe->info.step.graphics < STEP_B0);

[Severity: Medium]
Should graphics_xe3p_xpc (Graphics IP 35.11, Xe3p_XPC) also have
.has_sem64 = 1 set in drivers/gpu/drm/xe/xe_pci.c?

On the media side, both Xe3p media descriptors (media_xe3p_lpm for 35.00
and media_xe3p_hpm for 35.03) set .has_sem64 = 1, and alloc_media_gt()
scopes the stepping B0 check to MEDIA_VERx100(xe) == 3500 so that 35.03
enables has_sem64 on all steppings.

In alloc_primary_gt(), the stepping B0 check is similarly scoped to
GRAPHICS_VERx100(xe) == 3510, which suggests other Xe3p graphics versions
are expected to have graphics_desc->has_sem64 set without the stepping
restriction. However, in graphics_ips[]:

drivers/gpu/drm/xe/xe_pci.c:
	{ 3510, "Xe3p_LPG", &graphics_xe3p_lpg },
	{ 3511, "Xe3p_XPC", &graphics_xe3p_xpc },

only graphics_xe3p_lpg (used exclusively by 3510) sets .has_sem64 = 1,
while graphics_xe3p_xpc omits it. As a result, when xe_info_init() calls
alloc_primary_gt() on Graphics IP 35.11, gt->info.has_sem64 remains 0 on
the primary GT even though its paired media GT (35.03, media_xe3p_hpm) sets
gt->info.has_sem64 = 1.

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