[PATCH 5/5] drm/xe: disable wa_15015404425 for PTL B0

[Matt Atwood]

On Tue, Jun 24, 2025 at 03:25:37PM -0400, Rodrigo Vivi wrote:
> On Mon, Jun 23, 2025 at 04:31:30PM -0700, Matt Roper wrote:
> > On Mon, Jun 23, 2025 at 05:12:05PM -0400, Rodrigo Vivi wrote:
> > > On Fri, Jun 20, 2025 at 02:49:20PM -0700, Matt Atwood wrote:
> > > > This workaround only applies to PTL Compute Die A0. However, this
> > > > information cannot be determined until after the GT is brought up. This
> > > > means that we will assume that it is required for the initial bring up of
> > > > the gt. After GT init, the oob workarounds are enabled for the GT. Use
> > > > this flag to then manually set the bit in the soc oob bit field to 0
> > > > which will help performance after device bring up.
> > > > 
> > > > Signed-off-by: Matt Atwood <matthew.s.atwood at intel.com>
> > > > ---
> > > >  drivers/gpu/drm/xe/xe_pci.c        | 6 ++++++
> > > >  drivers/gpu/drm/xe/xe_wa_oob.rules | 1 +
> > > >  2 files changed, 7 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> > > > index ded0f3dc8d73..a624c3fb9498 100644
> > > > --- a/drivers/gpu/drm/xe/xe_pci.c
> > > > +++ b/drivers/gpu/drm/xe/xe_pci.c
> > > > @@ -34,6 +34,9 @@
> > > >  #include "xe_tile.h"
> > > >  #include "xe_wa.h"
> > > >  
> > > > +#include "generated/xe_wa_oob.h"
> > > > +#include "generated/xe_soc_wa_oob.h"
> > > > +
> > > >  enum toggle_d3cold {
> > > >  	D3COLD_DISABLE,
> > > >  	D3COLD_ENABLE,
> > > > @@ -890,6 +893,9 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > > >  	drm_dbg(&xe->drm, "d3cold: capable=%s\n",
> > > >  		str_yes_no(xe->d3cold.capable));
> > > >  
> > > > +	if (XE_WA(xe->tiles->media_gt, 15015404425_disable))
> > > > +		xe->oob[XE_SOC_WA_OOB_15015404425] = 0;
> > > 
> > > We are discussing this offline, but I need to make it very clear here
> > > that we should not move forward with this as is.
> > > 
> > > Two unnaceptable points in here:
> > > \
> > > 1. _disable. We either enable or we don't. If we need to wait for the gmdid,
> > > let it be and enable the workaround after that. GMDID should be one of the
> > > first things and I confirmed this workaround is so rare that in an a0 situation
> > > you could wait to only enable after you read the gmd-id and confirm the
> > > media is A0.
> > 
> > This is exactly what we *can't* do for this workaround.  The requirement
> > is that on any impacted platform, every single register read must be
> > preceded by four extra dummy writes.  There's no such thing as "early
> > enough to ignore" ---
> 
> What I got from the Architects on this is that it would be safe enough to
> read the GMDID without this workaround since the condition for the issue
> to manifest is not on a single read like this, although the protection is
> global.
> 
> But okay, let's work with the assumption that it is better to protect
> anyway.
> 
> > it's specifically noted that even pre-OS firmware
> > and such needs to be careful about doing this as well.  So this causes a
> > bit of a chicken-and-egg issue:  we cannot read the GMD_ID register
> > without having the workaround active on impacted platforms, but we
> > cannot figure out whether the platform is impacted until after we've
> > read that register[*].  We also do a bunch of other register reads
> > during early driver probe before the xe_gt is initialized enough to be
> > able to service workaround lookup queries.  So there are a few options
> > here:
> > 
> >  * Mark the platform as always being active on PTL, then come back and
> >    disable it later if/when we confirm that we're on a stepping that
> >    isn't impacted by the issue.  This is pretty simple conceptually, and
> >    is quite likely something we'll need in the future for other
> >    workarounds.  That's the approach MattA has taken here.
> 
> ack.
> 
> > 
> >  * Add two separate workarounds in the driver: 15015404425_early and
> >    15015404425.  15015404425_early is an SoC workaround that applies
> >    unconditionally on PTL, and 15015404425 is a GT workaround that
> >    applies only on specific media steppings.  Both do exactly the same
> >    thing (4 dummy writes before any register read), but
> >    15015404425_early is checked before all early MMIO accesses and
> >    15015404425 is checked on all others.  The downside of this approach
> >    is that we'd need to use a completely different set of MMIO
> >    operations for early driver boot (i.e., no xe_mmio_read32 and such).
> > 
> >  * Ignore this workaround completely if we can confirm that it only
> >    impacts pre-production steppings.  I don't think we have confirmation
> >    yet that A-step hardware is preprod-only, so we can't take this easy
> >    path yet.
> > 
> > > 
> > > 2. Don't mix SoC with Media. If the Bug is SoC don't wait of the media stepping
> > > and check directly for the SoC that needs this. So, don't create an infra that
> > > already has an exception in it. And if possible, avoid the infra at all. This
> > > might bring even more confusion to the w/a handling.
> > > 
> > > This w/a in specific here is soc, but it is getting mapped to our media-ip,
> > > so let's use the media check...
> > 
> > I think more explanation needs to be added to the patches to clarify
> > exactly what's going on since it's still a bit non-obvious.  The reality
> > is that our modern platforms aren't actually "SOCs" at all anymore;
> > they're technically MCP's --- Multi Chip Packages with logic (and
> > sometimes hardware issues) spread across the various dies.  The hardware
> > teams still refer certain things to be "SoC" logic for historical
> > reasons, even though that logic is technically distributed across
> > multiple chips these days.
> 
> Well, I still see the glue of all the chips as the SoC. It is easier to
> understand.
> 
> > 
> > We absolutely need some kind of "device" workaround framework that isn't
> > tied to the GT and that can be used before GTs are even up; we have some
> > non-GT workarounds today that we're not really handling properly, and we
> > know there are more coming.  I think when this workaround first came up
> > I suggested a "device workaround" infrastructure and using the same
> > general XE_WA() calls, with a _Generic() implementation to lookup the
> > status of a workaround in either the xe_device or the xe_gt depending on
> > which is passed (with the xe_device workaround table being initialized
> > much earlier in the probe sequence).
> 
> If we split in device_wa and gt_wa that would make much more sense with
> our Xe design indeed.
Is the ask here to change the name from XE_SOC_WA -> XE_DEVICE_WA?
> 
> > 
> > Issues outside of the GT can live in different places:  the GCD die, the
> > compute (aka CPU) die, the IO die, etc.  Each of these have their own
> > steppings, and the MCP itself has a stepping too.  In some of these
> > cases, the proper way to determine a stepping is to map PCI revid into a
> > die stepping.  In other cases the proper approach is to "fingerprint" a
> > die's stepping by inspecting some other IP that lives on the same die
> > (similar to how we used to fingerprint the PCH back in the day by
> > inspecting the ISA bus device).  For this workaround the stepping we
> > care about is the compute (CPU) die stepping, and since standalone media
> > happens to live on that same die, the bspec tells us how to figure out
> > compute die stepping from media stepping (A-step compute die <=> A-step
> > media IP in this case, although that isn't something that's guaranteed
> > to always be true).
> > 
> > 
> > [*] An alternative to fingerprinting compute die based on media stepping
> >     would probably be to check the CPU stepping through whatever
> >     mechanism is used to print the stepping in /proc/cpuinfo.  We'd have
> >     to find separate documentation on how to map the numeric value there
> >     into somthing like "B0;" I don't know off the top of my head where
> >     that documentation would be but the core kernel guys could probably
> >     point us in the right direction.
> 
> Yeap, in this case it would be the combination of the pci id + cpu-revid.
> This makes much more sense for this w/a, but it is hard to generalize indeed.
> 
> But also, any device_wa is hard to generalize anyway, since we won't have
> a device stepping or anything like that.
> 
> So, okay, 15015404425 in xe_device_wa enables it and
> 15015404425_disable in xe_gt_wa disables it for MEDIA_STEP(B0, FOREVER)
> 
> > 
> > 
> > Matt
> > 
> > > 
> > > > +
> > > >  	return 0;
> > > >  
> > > >  err_driver_cleanup:
> > > > diff --git a/drivers/gpu/drm/xe/xe_wa_oob.rules b/drivers/gpu/drm/xe/xe_wa_oob.rules
> > > > index 8c2aa48cb33a..822cbff13819 100644
> > > > --- a/drivers/gpu/drm/xe/xe_wa_oob.rules
> > > > +++ b/drivers/gpu/drm/xe/xe_wa_oob.rules
> > > > @@ -71,3 +71,4 @@ no_media_l3	MEDIA_VERSION(3000)
> > > >  # primary GT GMDID
> > > >  14022085890	GRAPHICS_VERSION(2001)
> > > >  16026007364 	MEDIA_VERSION(3000)
> > > > +15015404425_disable	PLATFORM(PANTHERLAKE), MEDIA_STEP(B0, FOREVER)
> > > > -- 
> > > > 2.49.0
> > > > 
> > 
> > -- 
> > Matt Roper
> > Graphics Software Engineer
> > Linux GPU Platform Enablement
> > Intel Corporation
MattA