[PATCH] drm/xe/xe2: Enable Priority Mem Read

[Mishra, Pallavi]

> -----Original Message-----
> From: Roper, Matthew D <matthew.d.roper at intel.com>
> Sent: Tuesday, May 28, 2024 9:35 AM
> To: Mishra, Pallavi <pallavi.mishra at intel.com>
> Cc: intel-xe at lists.freedesktop.org
> Subject: Re: [PATCH] drm/xe/xe2: Enable Priority Mem Read
> 
> On Sat, May 25, 2024 at 06:19:33AM +0530, Pallavi Mishra wrote:
> > Enable feature to allow memory reads to take a priority memory path.
> > This will reduce latency on the read path, but may introduce RAW
> > hazards.
> 
> You might want to elaborate on what "may introduce RAW hazards" means.
> I.e., it's worth noting that both kernel *and* userspace-submitted batch
> buffers may now be subject to read-after-write hazards if they don't use
> MI_MEM_FENCE in cases where there are assumptions about operations
> happening in-order, but where a read could potentially leapfrog a preceding
> write now that we've enabled this.
> 
> Presumably since there aren't any other changes in this patch you've already
> confirmed that there's nowhere in the KMD itself that needs extra fences
> added to avoid read-after-write hazards...you might want to mention that in
> the commit message too.
> 
> This could expose previously-harmless bugs in userspace drivers and/or IGT,
> so it will be important to make sure we land this before lifting force_probe on
> these platforms, otherwise the appearance of those bugs would be seen as a
> KMD regression, even though the bugs themselves live in userspace.
> 
> > Bspec: 45845, 60237, 60187, 60188
> 
> 45845 is a page for pre-Xe2 platforms, so I don't think that's the one you want
> here.  I think you want 60298 for the Xe2 equivalent page that documents the
> register bit.
> 
> >
> > Signed-off-by: Pallavi Mishra <pallavi.mishra at intel.com>
> > ---
> >  drivers/gpu/drm/xe/regs/xe_engine_regs.h | 1 +
> >  drivers/gpu/drm/xe/xe_hw_engine.c        | 7 +++++++
> >  2 files changed, 8 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> > b/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> > index 263ffc7bc2ef..4e8f9a61f0bf 100644
> > --- a/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> > +++ b/drivers/gpu/drm/xe/regs/xe_engine_regs.h
> > @@ -104,6 +104,7 @@
> >  #define CSFE_CHICKEN1(base)			XE_REG((base) +
> 0xd4, XE_REG_OPTION_MASKED)
> >  #define   GHWSP_CSB_REPORT_DIS			REG_BIT(15)
> >  #define   PPHWSP_CSB_AND_TIMESTAMP_REPORT_DIS	REG_BIT(14)
> > +#define   CS_PRIORITY_MEM_READ			REG_BIT(7)
> >
> >  #define FF_SLICE_CS_CHICKEN1(base)		XE_REG((base) + 0xe0,
> XE_REG_OPTION_MASKED)
> >  #define   FFSC_PERCTX_PREEMPT_CTRL		REG_BIT(14)
> > diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c
> > b/drivers/gpu/drm/xe/xe_hw_engine.c
> > index de1aefaa2335..34aa8b5270b0 100644
> > --- a/drivers/gpu/drm/xe/xe_hw_engine.c
> > +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
> > @@ -424,6 +424,13 @@ hw_engine_setup_default_state(struct
> xe_hw_engine *hwe)
> >  					   0xA,
> >
> XE_RTP_ACTION_FLAG(ENGINE_BASE)))
> >  		},
> > +		/* Enable Priority Mem Read */
> > +		{ XE_RTP_NAME("Priority_Mem_Read"),
> > +		  XE_RTP_RULES(GRAPHICS_VERSION_RANGE(2000, 2004)),
> 
> The current platforms that currently support this are BMG (graphics version
> 20.01) and LNL (20.04), so the starting point of this range should be 2001
> rather than 2000.  Also, since this is a new feature rather than a workaround,
> the expectation is that it will (probably) continue to exist on future platforms
> as well, so it's best to just use XE_RTP_END_VERSION_UNDEFINED as the end
> point so that we don't need to keep coming back here and updating this for
> every new platform in the future.
> 
> Also, RTP rules using GRAPHICS_VERSION[_RANGE] only get applied on the
> primary GT.  If this feature is also available on the media engines (I think it is
> since I don't see anything in the bspec implying it wouldn't be supported),
> then we'll also want a second table entry for those (i.e., pretty much a copy of
> what you have here, but using a MEDIA_VERSION_RANGE(1301,
> XE_RTP_END_VERSION_UNDEFINED) to capture the media IP.

Thanks for the review. 

Just to update you, I see below failure 
https://intel-gfx-ci.01.org/tree/intel-xe/xe-pw-134038v1/bat-lnl-1/igt@xe_module_load@load.html

Working on resolving this now. Will update once this is fixed and patch with comments incorporated
is ready for review.

> 
> 
> Matt
> 
> > +		  XE_RTP_ACTIONS(SET(CSFE_CHICKEN1(0),
> > +				     CS_PRIORITY_MEM_READ)),
> > +		  XE_RTP_ACTION_FLAG(ENGINE_BASE),
> > +		},
> >  		{}
> >  	};
> >
> > --
> > 2.25.1
> >
> 
> --
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation