Re: [PATCH 3/6] drm/xe/xe3: Add initial set of workarounds

["Chauhan, Shekhar" <shekhar.chauhan@intel.com>]

On 10/8/2024 7:05, Matt Atwood wrote:
> From: Gustavo Sousa <gustavo.sousa@intel.com>
>
> Implement the initial set of workarounds for Xe3 IPs.
>
> v2: correct steppings for 14021490052, 22019338487
>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
>   drivers/gpu/drm/xe/regs/xe_engine_regs.h |  1 +
>   drivers/gpu/drm/xe/regs/xe_gt_regs.h     |  3 ++
>   drivers/gpu/drm/xe/xe_wa.c               | 52 ++++++++++++++++++++++--
>   drivers/gpu/drm/xe/xe_wa_oob.rules       |  1 +
>   4 files changed, 54 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/regs/xe_engine_regs.h b/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> index 81b71903675e..7c78496e6213 100644
> --- a/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> +++ b/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> @@ -186,6 +186,7 @@
>   
>   #define VDBOX_CGCTL3F10(base)			XE_REG((base) + 0x3f10)
>   #define   IECPUNIT_CLKGATE_DIS			REG_BIT(22)
> +#define   RAMDFTUNIT_CLKGATE_DIS		REG_BIT(9)
>   
>   #define VDBOX_CGCTL3F18(base)			XE_REG((base) + 0x3f18)
>   #define   ALNUNIT_CLKGATE_DIS			REG_BIT(13)
> diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> index fb80042cbe0d..ac5a9ae94eb1 100644
> --- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> +++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> @@ -286,6 +286,9 @@
>   #define   GAMTLBVEBOX0_CLKGATE_DIS		REG_BIT(16)
>   #define   LTCDD_CLKGATE_DIS			REG_BIT(10)
>   
> +#define UNSLCGCTL9454				XE_REG(0x9454)
> +#define   LSCFE_CLKGATE_DIS			REG_BIT(4)
> +
>   #define XEHP_SLICE_UNIT_LEVEL_CLKGATE		XE_REG_MCR(0x94d4)
>   #define   L3_CR2X_CLKGATE_DIS			REG_BIT(17)
>   #define   L3_CLKGATE_DIS			REG_BIT(16)
> diff --git a/drivers/gpu/drm/xe/xe_wa.c b/drivers/gpu/drm/xe/xe_wa.c
> index 94ea76b098ed..a47f43166651 100644
> --- a/drivers/gpu/drm/xe/xe_wa.c
> +++ b/drivers/gpu/drm/xe/xe_wa.c
> @@ -252,6 +252,34 @@ static const struct xe_rtp_entry_sr gt_was[] = {
>   	  XE_RTP_ENTRY_FLAG(FOREACH_ENGINE),
>   	},
>   
> +	/* Xe3_LPG */
> +
> +	{ XE_RTP_NAME("14021871409"),
> +	  XE_RTP_RULES(GRAPHICS_VERSION(3000), GRAPHICS_STEP(A0, B0)),
> +	  XE_RTP_ACTIONS(SET(UNSLCGCTL9454, LSCFE_CLKGATE_DIS))
> +	},
> +
> +	/* Xe3_LPM */
> +
> +	{ XE_RTP_NAME("16021867713"),
> +	  XE_RTP_RULES(MEDIA_VERSION(3000),
> +		       ENGINE_CLASS(VIDEO_DECODE)),
> +	  XE_RTP_ACTIONS(SET(VDBOX_CGCTL3F1C(0), MFXPIPE_CLKGATE_DIS)),
> +	  XE_RTP_ENTRY_FLAG(FOREACH_ENGINE),
> +	},
> +	{ XE_RTP_NAME("16021865536"),
> +	  XE_RTP_RULES(MEDIA_VERSION(3000),
> +		       ENGINE_CLASS(VIDEO_DECODE)),
> +	  XE_RTP_ACTIONS(SET(VDBOX_CGCTL3F10(0), IECPUNIT_CLKGATE_DIS)),
> +	  XE_RTP_ENTRY_FLAG(FOREACH_ENGINE),
> +	},
> +	{ XE_RTP_NAME("14021486841"),
> +	  XE_RTP_RULES(MEDIA_VERSION(3000), MEDIA_STEP(A0, B0),
> +		       ENGINE_CLASS(VIDEO_DECODE)),
> +	  XE_RTP_ACTIONS(SET(VDBOX_CGCTL3F10(0), RAMDFTUNIT_CLKGATE_DIS)),
> +	  XE_RTP_ENTRY_FLAG(FOREACH_ENGINE),
> +	},
> +
>   	{}
>   };
>   
> @@ -568,6 +596,13 @@ static const struct xe_rtp_entry_sr engine_was[] = {
>   			     XE_RTP_ACTION_FLAG(ENGINE_BASE)))
>   	},
>   
> +	/* Xe3_LPG */
> +
> +	{ XE_RTP_NAME("14021402888"),
> +	  XE_RTP_RULES(GRAPHICS_VERSION_RANGE(3000, 3001), FUNC(xe_rtp_match_first_render_or_compute)),
> +	  XE_RTP_ACTIONS(SET(HALF_SLICE_CHICKEN7, CLEAR_OPTIMIZATION_DISABLE))
> +	},
> +
>   	{}
>   };
>   
> @@ -656,8 +691,7 @@ static const struct xe_rtp_entry_sr lrc_was[] = {
>   	/* Xe2_LPG */
>   
>   	{ XE_RTP_NAME("16020518922"),
> -	  XE_RTP_RULES(GRAPHICS_VERSION(2004), GRAPHICS_STEP(A0, B0),
> -		       ENGINE_CLASS(RENDER)),
> +	  XE_RTP_RULES(GRAPHICS_VERSION(2004), ENGINE_CLASS(RENDER)),
I think you got confused between 16020518922 and 14021490052 (right 
below). 16020518922 applies to steppings A0 to B0 and 14021490052 
applies to all the steppings. Having said that, I believe we shouldn't 
be touching workarounds targeting GRAPHICS_VERSION(2004) at all, since 
the patch stands for Xe3. The original code was correct, maybe some 
confusion resulted in this, if you could revert these two w/a changes, 
the patch looks good to go.
>   	  XE_RTP_ACTIONS(SET(FF_MODE,
>   			     DIS_TE_AUTOSTRIP |
>   			     DIS_MESH_PARTIAL_AUTOSTRIP |
> @@ -703,7 +737,7 @@ static const struct xe_rtp_entry_sr lrc_was[] = {
>   	  XE_RTP_ACTIONS(SET(WM_CHICKEN3, HIZ_PLANE_COMPRESSION_DIS))
>   	},
>   	{ XE_RTP_NAME("14021490052"),
> -	  XE_RTP_RULES(GRAPHICS_VERSION(2004), ENGINE_CLASS(RENDER)),
> +	  XE_RTP_RULES(GRAPHICS_VERSION(2004), ENGINE_CLASS(RENDER), GRAPHICS_STEP(A0, B0)),
>   	  XE_RTP_ACTIONS(SET(FF_MODE,
>   			     DIS_MESH_PARTIAL_AUTOSTRIP |
>   			     DIS_MESH_AUTOSTRIP),
> @@ -739,6 +773,18 @@ static const struct xe_rtp_entry_sr lrc_was[] = {
>   	  XE_RTP_ACTIONS(SET(CHICKEN_RASTER_1, DIS_CLIP_NEGATIVE_BOUNDING_BOX))
>   	},
>   
> +	/* Xe3_LPG */
> +	{ XE_RTP_NAME("14021490052"),
> +	  XE_RTP_RULES(GRAPHICS_VERSION(3000), GRAPHICS_STEP(A0, B0),
> +		       ENGINE_CLASS(RENDER)),
> +	  XE_RTP_ACTIONS(SET(FF_MODE,
> +			     DIS_MESH_PARTIAL_AUTOSTRIP |
> +			     DIS_MESH_AUTOSTRIP),
> +			 SET(VFLSKPD,
> +			     DIS_PARTIAL_AUTOSTRIP |
> +			     DIS_AUTOSTRIP))
> +	},
> +
>   	{}
>   };
>   
> diff --git a/drivers/gpu/drm/xe/xe_wa_oob.rules b/drivers/gpu/drm/xe/xe_wa_oob.rules
> index 920ca5060146..51d7208d7681 100644
> --- a/drivers/gpu/drm/xe/xe_wa_oob.rules
> +++ b/drivers/gpu/drm/xe/xe_wa_oob.rules
> @@ -33,6 +33,7 @@
>   		GRAPHICS_VERSION(2004)
>   22019338487	MEDIA_VERSION(2000)
>   		GRAPHICS_VERSION(2001)
> +		MEDIA_VERSION(3000), GRAPHICS_STEP(A0, B0)
>   22019338487_display	PLATFORM(LUNARLAKE)
>   16023588340	GRAPHICS_VERSION(2001)
>   14019789679	GRAPHICS_VERSION(1255)

-- 
-shekhar