RE: [PATCH] drm/xe/xe3: Apply Wa_16029380221 to media

[Gustavo Sousa <gustavo.sousa@intel.com>]

"Gote, Nitin R" <nitin.r.gote@intel.com> writes:

> Hi,
>
>> -----Original Message-----
>> From: Sousa, Gustavo <gustavo.sousa@intel.com>
>> Sent: Friday, May 29, 2026 6:15 PM
>> To: Gote, Nitin R <nitin.r.gote@intel.com>; intel-xe@lists.freedesktop.org
>> Cc: Gote, Nitin R <nitin.r.gote@intel.com>; Roper, Matthew D
>> <matthew.d.roper@intel.com>
>> Subject: Re: [PATCH] drm/xe/xe3: Apply Wa_16029380221 to media
>> 
>> Nitin Gote <nitin.r.gote@intel.com> writes:
>> 
>> > Apply Wa_16029380221 to Xe3p_LPM.
>> >
>> > The Xe3p_LPM media page walker is hard-wired NonCoherent and cannot
>> > observe CPU:WB cached page table data. Force page tables to CPU:WC by
>> > clearing has_cached_pt when MEDIA_VERSION(3500) is detected.
>> >
>> > Cc: Matt Roper <matthew.d.roper@intel.com>
>> > Signed-off-by: Nitin Gote <nitin.r.gote@intel.com>
>> > ---
>> >  drivers/gpu/drm/xe/xe_device.c     | 9 +++++++++
>> >  drivers/gpu/drm/xe/xe_wa_oob.rules | 1 +
>> >  2 files changed, 10 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/xe/xe_device.c
>> > b/drivers/gpu/drm/xe/xe_device.c index d224861b6f6f..8a6f494e11d5
>> > 100644
>> > --- a/drivers/gpu/drm/xe/xe_device.c
>> > +++ b/drivers/gpu/drm/xe/xe_device.c
>> > @@ -948,6 +948,15 @@ int xe_device_probe(struct xe_device *xe)
>> >  			return err;
>> >  	}
>> >
>> > +	/*
>> > +	 * Wa_16029380221: Xe3p_LPM media page walker is hard-wired
>> > +	 * NonCoherent, so page tables must use CPU:WC (uncached) to
>> > +	 * be visible to the media engine regardless of stepping.

The info about what IP and stepping Wa_16029380221 applies to is already
present in xe_wa_oob.rules, so repeating it here is a bit redundant.  I
would drop this comment entirely, but if you still would like to keep
it, I would only say something along the lines of

  Wa_16029380221: The affected GT will always use non-coherent access to
  page tables, so we must do uncached writes from the CPU.

>> > +	 */
>> > +	for_each_gt(gt, xe, id)
>> > +		if (XE_GT_WA(gt, 16029380221))
>> > +			xe->info.has_cached_pt = false;
>> > +
>> 
>> This will also disable the feature for the primary GT. Do we really want/need that?
>
> Yes. On NVL-P both GTs are on the same tile (max_gt_per_tile = 2) and share the same page
> Table, xe_lrc_set_ppgtt() programs vm->pt_root[tile->id] into the LRC for both GTs. 
> Looks like a single BO can only have one caching mode, so WC must win for the NonCoherent media walker.
>
>> 
>> I wonder if we should change this setting to be a per-GT one instead of global to
>> the device.  Perhaps one problem would be page tables shared between media
>> and primary GT.  Is that a possibility?
>> 
>
> Page tables are allocated per-tile (xe_pt_create(vm, tile, ...)), not per-GT. 
> As of now, I see there's no mechanism to give the same PT BO different caching per GT.
> And Yes, they are always shared on the same tile.

Thanks for the explanation!

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

>
> - Nitin
>
>> --
>> Gustavo Sousa
>> 
>> >  	for_each_tile(tile, xe, id) {
>> >  		err = xe_ggtt_init_early(tile->mem.ggtt);
>> >  		if (err)
>> > diff --git a/drivers/gpu/drm/xe/xe_wa_oob.rules
>> > b/drivers/gpu/drm/xe/xe_wa_oob.rules
>> > index f8a185103b80..9027365f0043 100644
>> > --- a/drivers/gpu/drm/xe/xe_wa_oob.rules
>> > +++ b/drivers/gpu/drm/xe/xe_wa_oob.rules
>> > @@ -65,3 +65,4 @@
>> >
>> >  14025883347	MEDIA_VERSION_RANGE(1301, 3503)
>> >  		GRAPHICS_VERSION_RANGE(2004, 3005)
>> > +16029380221	MEDIA_VERSION(3500)
>> > --
>> > 2.50.1