Re: [CI] drm/xe/guc: Enable w/a 14022293748 and 22019794406

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

On 7/19/2024 4:05 PM, Julia Filipchuk wrote:
> Enable workarounds for HW bug where render engine reset fails.

This needs a comment that the WA is only available from GuC 70.29

>
> Signed-off-by: Julia Filipchuk <julia.filipchuk@intel.com>
> ---
> This patch is for testing KLV workarrounds.
>
>   drivers/gpu/drm/xe/abi/guc_klvs_abi.h | 1 +
>   drivers/gpu/drm/xe/xe_guc_ads.c       | 5 +++++
>   drivers/gpu/drm/xe/xe_uc_fw.c         | 4 ++--
>   drivers/gpu/drm/xe/xe_wa_oob.rules    | 2 ++
>   4 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/abi/guc_klvs_abi.h b/drivers/gpu/drm/xe/abi/guc_klvs_abi.h
> index 8f9f60b28306..6b30743a2f6c 100644
> --- a/drivers/gpu/drm/xe/abi/guc_klvs_abi.h
> +++ b/drivers/gpu/drm/xe/abi/guc_klvs_abi.h
> @@ -351,6 +351,7 @@ enum xe_guc_klv_ids {
>   	GUC_WORKAROUND_KLV_ID_GAM_PFQ_SHADOW_TAIL_POLLING				= 0x9005,
>   	GUC_WORKAROUND_KLV_ID_DISABLE_MTP_DURING_ASYNC_COMPUTE				= 0x9007,
>   	GUC_WA_KLV_NP_RD_WRITE_TO_CLEAR_RCSM_AT_CGP_LATE_RESTORE			= 0x9008,
> +	GUC_WORKAROUND_KLV_ID_BACK_TO_BACK_RCS_ENGINE_RESET				= 0x9009,
>   };
>   
>   #endif
> diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c
> index 1c60b685dbc6..3190bb4ce04e 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ads.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ads.c
> @@ -367,6 +367,11 @@ static void guc_waklv_init(struct xe_guc_ads *ads)
>   					  0xC40,
>   					  &offset, &remain);
>   
> +	if (XE_WA(gt, 14022293748) && XE_WA(gt, 22019794406))

You need a check on the GuC version here. More comment on the version below.

> +		guc_waklv_enable_simple(ads,
> +					GUC_WORKAROUND_KLV_ID_BACK_TO_BACK_RCS_ENGINE_RESET,
> +					&offset, &remain);
> +
>   	size = guc_ads_waklv_size(ads) - remain;
>   	if (!size)
>   		return;
> diff --git a/drivers/gpu/drm/xe/xe_uc_fw.c b/drivers/gpu/drm/xe/xe_uc_fw.c
> index c902f4a10f44..d145ce77008e 100644
> --- a/drivers/gpu/drm/xe/xe_uc_fw.c
> +++ b/drivers/gpu/drm/xe/xe_uc_fw.c
> @@ -105,8 +105,8 @@ struct fw_blobs_by_type {
>   };
>   
>   #define XE_GUC_FIRMWARE_DEFS(fw_def, mmp_ver, major_ver)			\
> -	fw_def(BATTLEMAGE,	major_ver(intel-ci/xe,	guc,	bmg,	70, 24, 0))	\
> -	fw_def(LUNARLAKE,	major_ver(intel-ci/xe,	guc,	lnl,	70, 24, 0))	\
> +	fw_def(BATTLEMAGE,	major_ver(intel-ci/xe,	guc,	bmg,	70, 29, 0))	\
> +	fw_def(LUNARLAKE,	major_ver(intel-ci/xe,	guc,	lnl,	70, 29, 0))	\

A couple of notes here:

- The LNL and BMG guc defines are CI-only patches for now, so this 
update can't be done as part of a normal patch meant for the full tree. 
We're planning to move to the official patches very soon, so my 
recommendation would be to drop this change here and just add the 
official patch directly with 70.29

- The version in this list is the recommended version, but the driver 
will still load if an older GuC is found, so setting it here will not 
guarantee that 70.29 will be loaded, so you still need to check when 
setting the WA above. However,  we didn't distribute anything below 
70.29 for LNL and BMG, so we could indeed assume we always have at least 
70.29; if you want to go this way, please add a comment above the WA to 
make it clear why the version check is being omitted.


>   	fw_def(METEORLAKE,	major_ver(i915,	guc,	mtl,	70, 19, 2))	\
>   	fw_def(PVC,		mmp_ver(intel-ci/xe, guc, pvc,	70, 24, 0))	\
>   	fw_def(DG2,		major_ver(i915,	guc,	dg2,	70, 19, 2))	\
> diff --git a/drivers/gpu/drm/xe/xe_wa_oob.rules b/drivers/gpu/drm/xe/xe_wa_oob.rules
> index 540d38603f32..b0d7d4114967 100644
> --- a/drivers/gpu/drm/xe/xe_wa_oob.rules
> +++ b/drivers/gpu/drm/xe/xe_wa_oob.rules
> @@ -27,6 +27,8 @@
>   16022287689	GRAPHICS_VERSION(2001)
>   		GRAPHICS_VERSION(2004)
>   13011645652	GRAPHICS_VERSION(2004)
> +14022293748	GRAPHICS_VERSION(2004)
> +22019794406	GRAPHICS_VERSION(2004)

2004 is LNL, but this also applies to BMG (which is 2001)

Daniele

>   22019338487	MEDIA_VERSION(2000)
>   		GRAPHICS_VERSION(2001)
>   22019338487_display	PLATFORM(LUNARLAKE)