Re: [PATCH v5 2/2] drm/xe: expose PAT software config to debugfs

[Matt Roper <matthew.d.roper@intel.com>]

On Wed, Dec 03, 2025 at 08:30:14PM -0800, Wang, X wrote:
> 
> 
> > -----Original Message-----
> > From: Roper, Matthew D <matthew.d.roper@intel.com>
> > Sent: Wednesday, December 3, 2025 15:11
> > To: Wang, X <x.wang@intel.com>
> > Cc: intel-xe@lists.freedesktop.org; Lin, Shuicheng <shuicheng.lin@intel.com>;
> > Zuo, Alex <alex.zuo@intel.com>; Nikula, Jani <jani.nikula@intel.com>; Cavitt,
> > Jonathan <jonathan.cavitt@intel.com>
> > Subject: Re: [PATCH v5 2/2] drm/xe: expose PAT software config to debugfs
> > 
> > On Wed, Dec 03, 2025 at 10:53:55PM +0000, Xin Wang wrote:
> > > The existing "pat" debugfs node dumps the live PAT registers. Under
> > > SR-IOV the VF cannot touch those registers, so the file vanishes and
> > > users lose all PAT visibility. Add a VF-safe "pat_sw_config" entry to
> > > the VF-safe debugfs list. It prints the cached PAT table the driver
> > > programmed, rather than poking HW, so PF and VF instances present the
> > > same view.
> > >
> > > This lets IGT and other tools query the PAT configuration without
> > > carrying platform-specific tables or mirroring kernel logic.
> > >
> > > v2: (Jonathan)
> > > - Only append "(* = reserved entry)" to the PAT table header on Xe2+
> > >   platforms where it actually applies.
> > > - Deduplicate the PTA/ATS mode printing by introducing the small
> > >   drm_printf_pat_mode() helper macro.
> > >
> > > v3: (Matt)
> > > - Print IDX[XE_CACHE_NONE_COMPRESSION] on every Xe2+ platform so the
> > >   dump always reflects the value the driver might use (even if it defaults
> > >   to 0) and future IP revisions don’t need extra condition tweaks.
> > >
> > > v4:
> > > - Drop the drm_printf_pat_mode macro and introduce a real helper
> > >   xe2_pat_entry_dump(). (Jani)
> > > - Reuse the helper across all PTA/ATS/PAT dumps for xe2+ entries to keep
> > >   output format identical.
> > >
> > > v5: (Matt)
> > > - Split the original patch into two: one for refactoring helpers, one for
> > >   the new debugfs entry.
> > >
> > > CC: Jani Nikula <jani.nikula@intel.com>
> > > Suggested-by: Matt Roper <matthew.d.roper@intel.com>
> > > Signed-off-by: Xin Wang <x.wang@intel.com>
> > > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/xe_gt_debugfs.c |  1 +
> > >  drivers/gpu/drm/xe/xe_pat.c        | 59 ++++++++++++++++++++++++++++++
> > >  drivers/gpu/drm/xe/xe_pat.h        |  1 +
> > >  3 files changed, 61 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.c
> > > b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> > > index 7c3de6539044..e4f38b5150fc 100644
> > > --- a/drivers/gpu/drm/xe/xe_gt_debugfs.c
> > > +++ b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> > > @@ -209,6 +209,7 @@ static const struct drm_info_list
> > vf_safe_debugfs_list[] = {
> > >  	{ "default_lrc_vcs", .show = xe_gt_debugfs_show_with_rpm, .data =
> > vcs_default_lrc },
> > >  	{ "default_lrc_vecs", .show = xe_gt_debugfs_show_with_rpm, .data =
> > vecs_default_lrc },
> > >  	{ "hwconfig", .show = xe_gt_debugfs_show_with_rpm, .data =
> > hwconfig
> > > },
> > > +	{ "pat_sw_config", .show = xe_gt_debugfs_simple_show, .data =
> > > +xe_pat_dump_sw_config },
> > >  };
> > >
> > >  /* everything else should be added here */ diff --git
> > > a/drivers/gpu/drm/xe/xe_pat.c b/drivers/gpu/drm/xe/xe_pat.c index
> > > b8146724a60e..21f053c41899 100644
> > > --- a/drivers/gpu/drm/xe/xe_pat.c
> > > +++ b/drivers/gpu/drm/xe/xe_pat.c
> > > @@ -581,3 +581,62 @@ int xe_pat_dump(struct xe_gt *gt, struct
> > > drm_printer *p)
> > >
> > >  	return xe->pat.ops->dump(gt, p);
> > >  }
> > > +
> > > +/**
> > > + * xe_pat_dump_sw_config() - Dump the software-configured GT PAT table
> > into a drm printer.
> > > + * @gt: the &xe_gt
> > > + * @p: the &drm_printer
> > > + *
> > > + * Return: 0 on success or a negative error code on failure.
> > > + */
> > > +int xe_pat_dump_sw_config(struct xe_gt *gt, struct drm_printer *p) {
> > > +	struct xe_device *xe = gt_to_xe(gt);
> > > +	char label[PAT_LABEL_LEN];
> > > +
> > > +	if (!xe->pat.table || !xe->pat.n_entries)
> > > +		return -EOPNOTSUPP;
> > > +
> > > +	drm_printf(p, "PAT table:%s\n", GRAPHICS_VER(xe) >= 20 ? " (* =
> > reserved entry)" : "");
> > > +	for (u32 i = 0; i < xe->pat.n_entries; i++) {
> > > +		u32 pat = xe->pat.table[i].value;
> > > +
> > > +		if (GRAPHICS_VER(xe) >= 20) {
> > 
> > I think we're missing a "GRAPHICS_VERx100(xe) == 3511" check at the top of
> > this ladder that would use the xe3p_xpc dumping function.
> > 
> > For consistency with xe_pat_init_early()'s logic, you might want to make this
> > condition "GRAPHICS_VER(xe) == 30 || GRAPHICS_VER(xe) == 20" so that
> > both are using the same selection logic.
> > 
> I have made the  xe3p_xpc dumping function using the same format as the xe2_dump in the first patch, so here we could just use the  GRAPHICS_VER(xe) >= 20.

Oh, I missed that on the first patch.  That breaks our ability to
compare the output of these debugfs directly with the bspec (since now
we're printing an undocumented COMP_EN value that isn't present in the
spec.  I think it would be better to keep a separate XPC printing
function so that we stay aligned with the spec and don't confuse people.


Matt

> 
> > 
> > Matt
> > 
> > > +			xe_pat_index_label(label, sizeof(label), i);
> > > +			xe2_pat_entry_dump(p, label, pat, !xe-
> > >pat.table[i].valid);
> > > +		} else if (xe->info.platform == XE_METEORLAKE) {
> > > +			xelpg_pat_entry_dump(p, i, pat);
> > > +		} else if (xe->info.platform == XE_PVC) {
> > > +			xehpc_pat_entry_dump(p, i, pat);
> > > +		} else if (xe->info.platform == XE_DG2 ||
> > GRAPHICS_VERx100(xe) <= 1210) {
> > > +			xelp_pat_entry_dump(p, i, pat);
> > > +		} else {
> > > +			return -EOPNOTSUPP;
> > > +		}
> > > +	}
> > > +
> > > +	if (xe->pat.pat_pta) {
> > > +		u32 pat = xe->pat.pat_pta->value;
> > > +
> > > +		drm_printf(p, "Page Table Access:\n");
> > > +		xe2_pat_entry_dump(p, "PTA_MODE", pat, false);
> > > +	}
> > > +
> > > +	if (xe->pat.pat_ats) {
> > > +		u32 pat = xe->pat.pat_ats->value;
> > > +
> > > +		drm_printf(p, "PCIe ATS/PASID:\n");
> > > +		xe2_pat_entry_dump(p, "PAT_ATS ", pat, false);
> > > +	}
> > > +
> > > +	drm_printf(p, "Cache Level:\n");
> > > +	drm_printf(p, "IDX[XE_CACHE_NONE] = %d\n", xe-
> > >pat.idx[XE_CACHE_NONE]);
> > > +	drm_printf(p, "IDX[XE_CACHE_WT] = %d\n", xe-
> > >pat.idx[XE_CACHE_WT]);
> > > +	drm_printf(p, "IDX[XE_CACHE_WB] = %d\n", xe-
> > >pat.idx[XE_CACHE_WB]);
> > > +	if (GRAPHICS_VER(xe) >= 20) {
> > > +		drm_printf(p, "IDX[XE_CACHE_NONE_COMPRESSION]
> > = %d\n",
> > > +			   xe->pat.idx[XE_CACHE_NONE_COMPRESSION]);
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > diff --git a/drivers/gpu/drm/xe/xe_pat.h b/drivers/gpu/drm/xe/xe_pat.h
> > > index b8559120989e..5749a488d9a9 100644
> > > --- a/drivers/gpu/drm/xe/xe_pat.h
> > > +++ b/drivers/gpu/drm/xe/xe_pat.h
> > > @@ -49,6 +49,7 @@ void xe_pat_init_early(struct xe_device *xe);  void
> > > xe_pat_init(struct xe_gt *gt);
> > >
> > >  int xe_pat_dump(struct xe_gt *gt, struct drm_printer *p);
> > > +int xe_pat_dump_sw_config(struct xe_gt *gt, struct drm_printer *p);
> > >
> > >  /**
> > >   * xe_pat_index_get_coh_mode - Extract the coherency mode for the
> > > given
> > > --
> > > 2.43.0
> > >
> > 
> > --
> > Matt Roper
> > Graphics Software Engineer
> > Linux GPU Platform Enablement
> > Intel Corporation

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