[Intel-xe] [PATCH 1/2] drm/xe/pvc: Force even num engines to use 64B

[Matt Roper]

On Thu, Jul 20, 2023 at 01:10:37AM -0700, Niranjana Vishwanathapura wrote:
> Wa_16017236439 requires that we update BCS_SWCTRL
> (via indirect context batch buffer) to set 64B
> transfers when running on an even-numbered BCS
> engine and 256B on an odd-numbered BCS engine.

The workaround description says that we need to either use odd engines
for copies into system memory, or that we need to set 64B mode on even
engines if they might be used with SMEM destinations.  From the kernel
we don't really know what userspace will decide to do with these
engines, so it looks like the implementation here is setting 64B to
cover all bases, even though that unnecessarily lowers the performance
of those engines if they wind up being used for LMEM only.

However BCS_SWCTRL is listed on bspec 45546 which means that this access
can be updated by userspace from non-privileged batchbuffers.  Given
that, it seems like this workaround can and should probably be
implemented fully on the UMD side.  They can either choose option 1 (use
the odd engines for operations targetting SMEM) or option 2 (update the
BCS_SWCTRL context register if they want to use even engines with SMEM
destinations).  And since BCS_SWCTRL is part of the engine context, the
setting chosen by one userspace client will not impact the behavior for
other clients.

> 
> Signed-off-by: Tejas Upadhyay <tejas.upadhyay at intel.com>
> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura at intel.com>

Nitpick:  The first s-o-b line should generally match the author of the
patch.  It looks like you're the author here, but Tejas is listed first.

> ---
>  drivers/gpu/drm/xe/regs/xe_engine_regs.h | 3 +++
>  drivers/gpu/drm/xe/xe_wa.c               | 7 +++++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/regs/xe_engine_regs.h b/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> index 79873bf64e8d..8e488cbe332b 100644
> --- a/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> +++ b/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> @@ -59,6 +59,9 @@
>  #define RING_BBADDR(base)			XE_REG((base) + 0x140)
>  #define RING_BBADDR_UDW(base)			XE_REG((base) + 0x168)
>  
> +#define BCS_ENGINE_SWCTL(base)                 XE_REG((base) + 0x200, XE_REG_OPTION_MASKED)
> +#define BCS_ENGINE_SWCTL_DISABLE_256B          REG_BIT(2)
> +
>  /* Handling MOCS value in BLIT_CCTL like it was done CMD_CCTL */
>  #define BLIT_CCTL(base)				XE_REG((base) + 0x204)
>  #define   BLIT_CCTL_DST_MOCS_MASK		REG_GENMASK(14, 9)
> diff --git a/drivers/gpu/drm/xe/xe_wa.c b/drivers/gpu/drm/xe/xe_wa.c
> index 21087f7a4609..b90f64ad301c 100644
> --- a/drivers/gpu/drm/xe/xe_wa.c
> +++ b/drivers/gpu/drm/xe/xe_wa.c
> @@ -518,6 +518,13 @@ static const struct xe_rtp_entry_sr engine_was[] = {
>  		       GRAPHICS_STEP(B0, C0)),
>  	  XE_RTP_ACTIONS(SET(CACHE_MODE_SS, DISABLE_ECC))
>  	},
> +	{ XE_RTP_NAME("16017236439"),
> +	  XE_RTP_RULES(PLATFORM(PVC), ENGINE_CLASS(COPY),
> +		       FUNC(xe_rtp_match_even_instance)),
> +	  XE_RTP_ACTIONS(SET(BCS_ENGINE_SWCTL(0),

According to bspec 53978, BCS_SWCTRL is part of the context image for
the blitter engine.  If we do wind up implementing this workaround in
the kernel, it should be in lrc_was[] so that it becomes part of the
golden context image.


Matt

> +			     BCS_ENGINE_SWCTL_DISABLE_256B,
> +			     XE_RTP_ACTION_FLAG(ENGINE_BASE))),
> +	},
>  
>  	/* Xe_LPG */
>  	{ XE_RTP_NAME("14017856879"),
> -- 
> 2.21.0.rc0.32.g243a4c7e27
> 

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