[Intel-xe] [PATCH V2] drm/xe/xe2: Add workaround 14019449301

[Matt Roper]

On Thu, Nov 16, 2023 at 06:33:21PM +0530, Tejas Upadhyay wrote:
> This workaround applies to Xe2_LPM
> 
> V2(MattR):
>   - Change name of register
>   - Loop for all engines
>   - Driver permanent WA, applies to all steps
> 
> Signed-off-by: Tejas Upadhyay <tejas.upadhyay at intel.com>
> ---
>  drivers/gpu/drm/xe/regs/xe_engine_regs.h | 3 +++
>  drivers/gpu/drm/xe/xe_wa.c               | 8 ++++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/regs/xe_engine_regs.h b/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> index b57dec17eb2d..ff6b0207c78e 100644
> --- a/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> +++ b/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> @@ -121,4 +121,7 @@
>  #define VDBOX_CGCTL3F1C(base)			XE_REG((base) + 0x3f1c)
>  #define   MFXPIPE_CLKGATE_DIS			REG_BIT(3)
>  
> +#define VDBOX_CGCTL3F08				XE_REG(0x3f08)

This is misleading; there's no such register at offset 0x3F08.  0x3F08
is a relative offset from a given engine's base (e.g., for the vd
engines it will be located at 0x1cf308, 0x1c7f08, 0x1df308, etc.).
So this should be defined as a parameterized macro that takes the
engine's base offset as a parameter and adds it to the 0x3f08.

Then when you use it in the gt_was[] table below, you'll just pass 0 as
the base since the FOREACH_ENGINE flag takes care of providing the
various engine bases.

Also note that this register should also be slightly higher in this
header since registers are supposed to be sorted by offset.

> +#define CG3DDISHRS_CLKGATE_DIS			REG_BIT(5)

This line is missing two extra spaces after the 'define;' you should
make the style match the rest of the register bit definitions.

> +
>  #endif
> diff --git a/drivers/gpu/drm/xe/xe_wa.c b/drivers/gpu/drm/xe/xe_wa.c
> index d03e6674519f..aff92a7efa07 100644
> --- a/drivers/gpu/drm/xe/xe_wa.c
> +++ b/drivers/gpu/drm/xe/xe_wa.c
> @@ -259,6 +259,14 @@ static const struct xe_rtp_entry_sr gt_was[] = {
>  	  XE_RTP_ACTIONS(SET(XELPMP_SQCNT1, ENFORCE_RAR))
>  	},
>  
> +	/* Xe2_LPM */

For consistency with how things were handled on Xe1, you might want to
place the block of Xe2_LPM media workarounds after the block of Xe2_LPG
graphics workarounds.


Matt

> +
> +	{ XE_RTP_NAME("14019449301"),
> +	  XE_RTP_RULES(MEDIA_VERSION(2000), ENGINE_CLASS(VIDEO_DECODE)),
> +	  XE_RTP_ACTIONS(SET(VDBOX_CGCTL3F08, CG3DDISHRS_CLKGATE_DIS)),
> +	  XE_RTP_ENTRY_FLAG(FOREACH_ENGINE),
> +	},
> +
>  	/* Xe2_LPG */
>  
>  	{ XE_RTP_NAME("16020975621"),
> -- 
> 2.25.1
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation