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

[Souza, Jose]

On Mon, 2023-04-24 at 15:59 -0700, Matt Roper wrote:
> 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.

Cc: Rodrigo

Okay will send the first patch with comments addressed.

Then if decided to not program this in KMD it needs to be documented publicly.
Together with all other small behavior changes from i915, MI_BATCH_BUFFER_START is other case where behavior changed.

This will save a lot of debug time in the other UMDs.

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