[PATCH V3] drm/xe/xe2: Add Wa_15015404425

[Matt Roper]

On Fri, Jul 05, 2024 at 06:33:24PM +0530, Tejas Upadhyay wrote:
> Wa_15015404425 asks us to perform four "dummy" writes to a
> non-existent register offset before every real register read.
> Although the specific offset of the writes doesn't directly
> matter, the workaround suggests offset 0x130030 as a good target
> so that these writes will be easy to recognize and filter out in
> debugging traces.
> 
> V3(MattR):
>   - Define dummy reg local to function
>   - Avoid tracing dummy writes
>   - Update commit message
> V2:
>   - Add WA to 8/16/32bit reads also - MattR
>   - Corrected dummy reg address - MattR
>   - Use for loop to avoid mental pause - JaniN
> 
> Signed-off-by: Tejas Upadhyay <tejas.upadhyay at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_mmio.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
> index f92faad4b96d..66bfa93a976d 100644
> --- a/drivers/gpu/drm/xe/xe_mmio.c
> +++ b/drivers/gpu/drm/xe/xe_mmio.c
> @@ -121,12 +121,33 @@ int xe_mmio_init(struct xe_device *xe)
>  	return devm_add_action_or_reset(xe->drm.dev, mmio_fini, xe);
>  }
>  
> +static void mmio_flush_pending_writes(struct xe_gt *gt)
> +{
> +#define DUMMY_REG	XE_REG(0x130030)
> +	struct xe_tile *tile = gt_to_tile(gt);
> +	struct xe_device *xe = gt_to_xe(gt);
> +	struct xe_reg reg = DUMMY_REG;

Since we're doing writel()'s, there's no need to define an xe_reg
structure or use the XE_REG() initializer.  A simple

        #define DUMMY_REG_OFFSET  0x130030

would suffice.

> +	u32 addr;
> +	int i;
> +
> +	if (!(xe->info.platform == XE_LUNARLAKE))
> +		return;
> +
> +	addr = xe_mmio_adjusted_addr(gt, reg.addr);

We're intentionally writing to a specific, known address (and it isn't a
media GT offset), we don't want to perform any kind of adjustment.

> +	/* 4 dummy writes */
> +	for (i = 0; i < 4; i++)
> +		writel(0, (reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + addr);

The ternary condition here is also unnecessary; we're accessing a
specific offset and know it isn't in the "mmio_ext" space.  All we
actually need here is:

        writel(0, tile->mmio.regs + DUMMY_REG_OFFSET);


Matt

> +}
> +
>  u8 xe_mmio_read8(struct xe_gt *gt, struct xe_reg reg)
>  {
>  	struct xe_tile *tile = gt_to_tile(gt);
>  	u32 addr = xe_mmio_adjusted_addr(gt, reg.addr);
>  	u8 val;
>  
> +	/* Wa_15015404425 */
> +	mmio_flush_pending_writes(gt);
> +
>  	val = readb((reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + addr);
>  	trace_xe_reg_rw(gt, false, addr, val, sizeof(val));
>  
> @@ -139,6 +160,9 @@ u16 xe_mmio_read16(struct xe_gt *gt, struct xe_reg reg)
>  	u32 addr = xe_mmio_adjusted_addr(gt, reg.addr);
>  	u16 val;
>  
> +	/* Wa_15015404425 */
> +	mmio_flush_pending_writes(gt);
> +
>  	val = readw((reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + addr);
>  	trace_xe_reg_rw(gt, false, addr, val, sizeof(val));
>  
> @@ -160,6 +184,9 @@ u32 xe_mmio_read32(struct xe_gt *gt, struct xe_reg reg)
>  	u32 addr = xe_mmio_adjusted_addr(gt, reg.addr);
>  	u32 val;
>  
> +	/* Wa_15015404425 */
> +	mmio_flush_pending_writes(gt);
> +
>  	if (!reg.vf && IS_SRIOV_VF(gt_to_xe(gt)))
>  		val = xe_gt_sriov_vf_read32(gt, reg);
>  	else
> -- 
> 2.25.1
> 

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