[PATCH] drm/xe/xe3: Extend wa_15015404425 for xe3

[Matt Roper]

On Thu, Feb 06, 2025 at 08:56:32PM -0800, Upadhyay, Tejas wrote:
> 
> 
> > -----Original Message-----
> > From: De Marchi, Lucas <lucas.demarchi at intel.com>
> > Sent: Friday, February 7, 2025 5:03 AM
> > To: Atwood, Matthew S <matthew.s.atwood at intel.com>
> > Cc: Roper, Matthew D <matthew.d.roper at intel.com>; intel-
> > xe at lists.freedesktop.org; Upadhyay, Tejas <tejas.upadhyay at intel.com>
> > Subject: Re: [PATCH] drm/xe/xe3: Extend wa_15015404425 for xe3
> > 
> > On Thu, Feb 06, 2025 at 09:12:49AM -0800, Matt Atwood wrote:
> > >On Thu, Jan 30, 2025 at 09:30:30AM -0800, Matt Roper wrote:
> > >> On Tue, Jan 28, 2025 at 04:55:51PM -0800, Matt Atwood wrote:
> > >> > From: Tejas Upadhyay <tejas.upadhyay at intel.com>
> > >> >
> > >> > wa_15015404425 applies to xe3 A0 step as well.
> > >>
> > >> We really need a better commit message explaining how this workaround
> > >> is different from our usual ones and why it needs to be handled in a
> > >> special way that's different from all of our other workarounds,
> > >> especially since someone who just quickly skims the workaround
> > >> database without digging into the details is going to incorrectly
> > >> think this just applies to Xe2 media and isn't relevant to PTL.
> > >Ack.
> > >>
> > >> I was kind of hoping we'd add a more well-defined infrastructure for
> > >> handling device / SoC workarounds.  Adding quick hacks like this is
> > >> going to get out of hand quickly if more of these start showing up.
> > >Any particular ideas here?
> > 
> > at the very least, use XE_WA(). You may need to extend the matches it knows
> > about to implement what you need here.
> 
> As far as I remember, init of wa_oob was little late so we could not use XE_WA. I am not sure if init wa_oob moved elsewhere and we are good here!
> 
> I was seeing below assert and thus we decided to go with what we have here,
> 
> It asserts,
> [  778.229096] xe 0000:00:02.0: [drm] [drm] Assertion `(gt)->wa_active.oob_initialized` failed!
>                platform: LUNARLAKE subplatform: 1
>                graphics: (null) 0.00 step **
>                media: (null) 0.00 step **
>                tile: 0 VRAM 0 B
>                GT: 0 type 0
> [  778.229121] WARNING: CPU: 1 PID: 1567 at drivers/gpu/drm/xe/xe_mmio.c:133 mmio_flush_pending_writes+0x33d/0x350 [xe]

You can't use XE_WA unmodified since it's currently GT-centric and
relies on various stuff in the GT already being initialized, as you see
above.  However you can either extend it to also support device-level
workarounds, or at least create a similar XE_DEVICE_WA() framework that
works in a similar way.  Device workarounds ultimately depend on the
platform and an SoC stepping derived from the PCI revid, and that
information is immediately available on probe without needing to bring
up the GTs.


Matt

> 
> Tejas
> > 
> > Lucas De Marchi
> > 
> > >+Lucas
> > >>
> > >>
> > >> Matt
> > >>
> > >> >
> > >> > v2: query based off SOC stepping
> > >> >
> > >> > Signed-off-by: Tejas Upadhyay <tejas.upadhyay at intel.com>
> > >> > Signed-off-by: Matt Atwood <matthew.s.atwood at intel.com>
> > >> > ---
> > >> >  drivers/gpu/drm/xe/xe_mmio.c | 13 +++++++------
> > >> > drivers/gpu/drm/xe/xe_step.h |  2 ++
> > >> >  2 files changed, 9 insertions(+), 6 deletions(-)
> > >> >
> > >> > diff --git a/drivers/gpu/drm/xe/xe_mmio.c
> > >> > b/drivers/gpu/drm/xe/xe_mmio.c index d321a21aacf0..98d068ad33bb
> > >> > 100644
> > >> > --- a/drivers/gpu/drm/xe/xe_mmio.c
> > >> > +++ b/drivers/gpu/drm/xe/xe_mmio.c
> > >> > @@ -150,14 +150,15 @@ int xe_mmio_init(struct xe_device *xe)
> > >> > static void mmio_flush_pending_writes(struct xe_mmio *mmio)  {
> > >> >  #define DUMMY_REG_OFFSET	0x130030
> > >> > +	struct xe_device *xe = tile_to_xe(mmio->tile);
> > >> >  	int i;
> > >> >
> > >> > -	if (mmio->tile->xe->info.platform != XE_LUNARLAKE)
> > >> > -		return;
> > >> > -
> > >> > -	/* 4 dummy writes */
> > >> > -	for (i = 0; i < 4; i++)
> > >> > -		writel(0, mmio->regs + DUMMY_REG_OFFSET);
> > >> > +	if (xe->info.platform == XE_LUNARLAKE ||
> > >> > +	    (xe->info.platform == XE_PANTHERLAKE &&
> > >> > +	     xe->info.revid == PTL_SOC_STEP_A0))
> > >> > +		/* 4 dummy writes */
> > >> > +		for (i = 0; i < 4; i++)
> > >> > +			writel(0, mmio->regs + DUMMY_REG_OFFSET);
> > >> >  }
> > >> >
> > >> >  u8 xe_mmio_read8(struct xe_mmio *mmio, struct xe_reg reg) diff
> > >> > --git a/drivers/gpu/drm/xe/xe_step.h b/drivers/gpu/drm/xe/xe_step.h
> > >> > index 686cb59200c2..879486a818e9 100644
> > >> > --- a/drivers/gpu/drm/xe/xe_step.h
> > >> > +++ b/drivers/gpu/drm/xe/xe_step.h
> > >> > @@ -20,4 +20,6 @@ static inline u32 xe_step_to_gmdid(enum xe_step
> > >> > step) { return step - STEP_A0; }
> > >> >
> > >> >  const char *xe_step_name(enum xe_step step);
> > >> >
> > >> > +#define PTL_SOC_STEP_A0  0x0
> > >> > +
> > >> >  #endif
> > >> > --
> > >> > 2.45.0
> > >> >
> > >>
> > >> --
> > >> Matt Roper
> > >> Graphics Software Engineer
> > >> Linux GPU Platform Enablement
> > >> Intel Corporation

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