[Intel-xe] [PATCH 2/2] drm/xe: Set default MOCS value for copy cs instructions

[Matt Roper]

On Fri, Apr 14, 2023 at 09:09:06PM -0700, José Roberto de Souza wrote:
> copy cs instructions that dont have a explict MOCS field will use this
> default MOCS value.
> 
> This was mainly copied from i915 source code.
> 
> BSpec: 45807
> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> ---
>  drivers/gpu/drm/xe/regs/xe_engine_regs.h |  9 +++++++++
>  drivers/gpu/drm/xe/xe_wa.c               | 13 +++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/regs/xe_engine_regs.h b/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> index cc874348f76bc..dbc4b9f15b4e8 100644
> --- a/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> +++ b/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> @@ -105,6 +105,15 @@
>  		(REG_FIELD_PREP(CMD_CCTL_WRITE_OVERRIDE_MASK, (write) << 1) | \
>  		 REG_FIELD_PREP(CMD_CCTL_READ_OVERRIDE_MASK, (read) << 1))
>  
> +#define BLIT_CCTL(base)				_MMIO((base) + 0x204)
> +#define   BLIT_CCTL_DST_MOCS_MASK		REG_GENMASK(14, 8)
> +#define   BLIT_CCTL_SRC_MOCS_MASK		REG_GENMASK(6, 0)
> +#define   BLIT_CCTL_MASK (BLIT_CCTL_DST_MOCS_MASK | \
> +			  BLIT_CCTL_SRC_MOCS_MASK)
> +#define   BLIT_CCTL_MOCS(dst, src)				       \
> +		(REG_FIELD_PREP(BLIT_CCTL_DST_MOCS_MASK, (dst) << 1) | \
> +		 REG_FIELD_PREP(BLIT_CCTL_SRC_MOCS_MASK, (src) << 1))

Same general comments from last patch about file position, mask
definition, and potentially dropping last two macros apply to this patch
as well.

> +
>  #define VDBOX_CGCTL3F10(base)			_MMIO((base) + 0x3f10)
>  #define   IECPUNIT_CLKGATE_DIS			REG_BIT(22)
>  
> diff --git a/drivers/gpu/drm/xe/xe_wa.c b/drivers/gpu/drm/xe/xe_wa.c
> index 5ef3561101466..0c12700bedde5 100644
> --- a/drivers/gpu/drm/xe/xe_wa.c
> +++ b/drivers/gpu/drm/xe/xe_wa.c
> @@ -660,6 +660,19 @@ xe_wa_process_runtime_was_engine(struct xe_reg_sr *sr,
>  					   CMD_CCTL_MOCS_OVERRIDE(mocs_w, mocs_r),
>  					   XE_RTP_ACTION_FLAG(MASKED_REG, ENGINE_BASE))),
>  		},
> +		/*
> +		 * Some blitter commands do not have a field for MOCS, those
> +		 * commands will use MOCS index pointed by BLIT_CCTL.
> +		 * BLIT_CCTL registers are needed to be programmed to un-cached.
> +		 */
> +		{ XE_RTP_NAME("BLIT_CCTL_default_MOCS"),
> +		  XE_RTP_RULES(GRAPHICS_VERSION_RANGE(1200, XE_RTP_END_VERSION_UNDEFINED),
> +			       ENGINE_CLASS(COPY)),
> +		  XE_RTP_ACTIONS(FIELD_SET(BLIT_CCTL(0),
> +				 BLIT_CCTL_MASK,
> +				 BLIT_CCTL_MOCS(mocs_w, mocs_r),
> +				 XE_RTP_ACTION_FLAG(MASKED_REG, ENGINE_BASE)))

Even though BLIT_CCTL looks a lot like CMD_CCTL on the surface, there
are some important differences between the two that need to be noted
here:

 - BLIT_CCTL is not a masked register.
 - BLIT_CCTL is part of an engine context (e.g., bspec 45585), so it
   would need to eventually wind up on an LRC list, not a HWE list.

The inconsistency in hardware design here is frustrating...

Also note that BLIT_CCTL is directly writable by userspace (bspec 45546)
so I question whether it's even our responsibility to do this in Xe.
It's too late to fix i915 now (at least for current platforms), but I'd
suggest just dropping this second patch for Xe and leaving it to
userspace to program whatever value they wish for their own context.


Matt

> +		},
>  		{}
>  	};
>  
> -- 
> 2.40.0
> 

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