Re: [PATCH v3 10/10] drm/xe/vsec: Update PMT internal access for CRI

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Mon, 24 Aug 2026, Michael J. Ruhl wrote:

> Xe access the PMT infrastructure directly. The current usage
> is supported ONLY by BMG devices.
> 
> CRI has further requirements for access.
> 
> Add a new API to access the GUID based on the platform.
> Use the API get the GUID for each device.
> 
> Minor cleanup for a newly unused parameter (mmio).
> 
> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> ---
>  drivers/gpu/drm/xe/regs/xe_pmt.h     |  4 ++-
>  drivers/gpu/drm/xe/xe_debugfs.c      | 44 ++++++++++++-------------
>  drivers/gpu/drm/xe/xe_device_types.h |  2 ++
>  drivers/gpu/drm/xe/xe_hwmon.c        | 10 ++++--
>  drivers/gpu/drm/xe/xe_pcode.c        | 10 ++++--
>  drivers/gpu/drm/xe/xe_vsec.c         | 48 ++++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_vsec.h         |  1 +
>  7 files changed, 92 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/regs/xe_pmt.h b/drivers/gpu/drm/xe/regs/xe_pmt.h
> index fc9c9cb6a830..683bf401dc9d 100644
> --- a/drivers/gpu/drm/xe/regs/xe_pmt.h
> +++ b/drivers/gpu/drm/xe/regs/xe_pmt.h
> @@ -10,7 +10,7 @@
>  #define BMG_PMT_BASE_OFFSET		0xDB000
>  #define BMG_DISCOVERY_OFFSET		(SOC_BASE + BMG_PMT_BASE_OFFSET)
>  
> -#define PUNIT_TELEMETRY_GUID		XE_REG(BMG_DISCOVERY_OFFSET + 0x4)
> +#define BMG_PUNIT_TELEMETRY_GUID	XE_REG(BMG_DISCOVERY_OFFSET + 0x4)
>  #define BMG_ENERGY_STATUS_PMT_OFFSET	(0x30)
>  #define	ENERGY_PKG			REG_GENMASK64(31, 0)
>  #define	ENERGY_CARD			REG_GENMASK64(63, 32)
> @@ -25,6 +25,8 @@
>  #define CRI_DISCOVERY_OFFSET		(SOC_BASE + CRI_TELEMETRY_BASE_OFFSET)
>  #define CRI_TELEMETRY_OFFSET		(SOC_BASE + CRI_TELEMETRY_BASE_OFFSET)
>  
> +#define CRI_PUNIT_TELEMETRY_GUID	XE_REG(CRI_DISCOVERY_OFFSET + 0x4)
> +
>  #define BMG_MODS_RESIDENCY_OFFSET		(0x4D0)
>  #define BMG_G2_RESIDENCY_OFFSET		(0x530)
>  #define BMG_G6_RESIDENCY_OFFSET		(0x538)
> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
> index 28135f84e286..12476549164d 100644
> --- a/drivers/gpu/drm/xe/xe_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
> @@ -21,7 +21,6 @@
>  #include "xe_gt_printk.h"
>  #include "xe_guc_ads.h"
>  #include "xe_hw_engine.h"
> -#include "xe_mmio.h"
>  #include "xe_pagefault.h"
>  #include "xe_pcode.h"
>  #include "xe_pm.h"
> @@ -97,15 +96,20 @@ static void xe_fault_inject_debugfs_register(struct xe_device *xe,
>  	}
>  }
>  
> -static void read_residency_counter(struct xe_device *xe, struct xe_mmio *mmio,
> -				   u32 offset, const char *name, struct drm_printer *p)
> +static void read_residency_counter(struct xe_device *xe, u32 offset, const char *name,
> +				   struct drm_printer *p)
>  {
>  	u64 residency = 0;
> +	u32 guid;
>  	int ret;
>  
> -	ret = xe_pmt_telem_read(xe->drm.dev,
> -				xe_mmio_read32(mmio, PUNIT_TELEMETRY_GUID),
> -				&residency, offset, sizeof(residency));
> +	guid = xe_vsec_get_guid(xe);
> +	if (!guid) {
> +		drm_warn(&xe->drm, "PMT device is not powered\n");
> +		return;
> +	}
> +
> +	ret = xe_pmt_telem_read(xe->drm.dev, guid, &residency, offset, sizeof(residency));
>  	if (ret != sizeof(residency)) {
>  		drm_warn(&xe->drm, "%s counter failed to read, ret %d\n", name, ret);
>  		return;
> @@ -244,13 +248,12 @@ static int pcode_info(struct seq_file *m, void *data)
>  static int dgfx_pkg_residencies_show(struct seq_file *m, void *data)
>  {
>  	struct xe_device *xe;
> -	struct xe_mmio *mmio;
>  	struct drm_printer p;
>  
>  	xe = node_to_xe(m->private);
>  	p = drm_seq_file_printer(m);
>  	guard(xe_pm_runtime)(xe);
> -	mmio = xe_root_tile_mmio(xe);
> +
>  	static const struct {
>  		u32 offset;
>  		const char *name;
> @@ -264,7 +267,7 @@ static int dgfx_pkg_residencies_show(struct seq_file *m, void *data)
>  	};
>  
>  	for (int i = 0; i < ARRAY_SIZE(residencies); i++)
> -		read_residency_counter(xe, mmio, residencies[i].offset, residencies[i].name, &p);
> +		read_residency_counter(xe, residencies[i].offset, residencies[i].name, &p);
>  
>  	return 0;
>  }
> @@ -272,13 +275,11 @@ static int dgfx_pkg_residencies_show(struct seq_file *m, void *data)
>  static int dgfx_pcie_link_residencies_show(struct seq_file *m, void *data)
>  {
>  	struct xe_device *xe;
> -	struct xe_mmio *mmio;
>  	struct drm_printer p;
>  
>  	xe = node_to_xe(m->private);
>  	p = drm_seq_file_printer(m);
>  	guard(xe_pm_runtime)(xe);
> -	mmio = xe_root_tile_mmio(xe);
>  
>  	static const struct {
>  		u32 offset;
> @@ -290,7 +291,7 @@ static int dgfx_pcie_link_residencies_show(struct seq_file *m, void *data)
>  	};
>  
>  	for (int i = 0; i < ARRAY_SIZE(residencies); i++)
> -		read_residency_counter(xe, mmio, residencies[i].offset, residencies[i].name, &p);
> +		read_residency_counter(xe, residencies[i].offset, residencies[i].name, &p);
>  
>  	return 0;
>  }
> @@ -710,23 +711,22 @@ void xe_debugfs_register(struct xe_device *xe)
>  				 ARRAY_SIZE(debugfs_list),
>  				 root, minor);
>  
> -	if (xe->info.platform == XE_BATTLEMAGE && !IS_SRIOV_VF(xe)) {
> -		drm_debugfs_create_files(debugfs_residencies,
> -					 ARRAY_SIZE(debugfs_residencies),
> -					 root, minor);
> -	}
> -
>  	/*
> -	 * Pcode version read from PMT is currently only supported on CRI and BMG platforms in PF
> -	 * mode, as both platforms support the necessary telemetry read mechanism and have a fixed
> -	 * PUNIT_VERSION_OFFSET.
> +	 * Residencies and Pcode version read from PMT is currently only supported on CRI and BMG
> +	 * platforms in PF mode.  Both platforms support the necessary telemetry read mechanism
> +	 * and have a fixed offsets for the required data.
>  	 * Attempting this access on other platforms must be verified before enabling support.
>  	 */
>  	if (!IS_SRIOV_VF(xe) &&
> -	    (xe->info.platform == XE_CRESCENTISLAND || xe->info.platform == XE_BATTLEMAGE))
> +	    (xe->info.platform == XE_CRESCENTISLAND || xe->info.platform == XE_BATTLEMAGE)) {
> +		drm_debugfs_create_files(debugfs_residencies,
> +					 ARRAY_SIZE(debugfs_residencies),

ARRAY_SIZE() has also own include which is missing from this file (looks 
it's already used in this file and if there's no special include policy in 
xe, it would be time to finally add the include).

> +					 root, minor);
> +
>  		drm_debugfs_create_files(pcode_info_debugfs,
>  					 ARRAY_SIZE(pcode_info_debugfs),
>  					 root, minor);
> +	}
>  
>  	debugfs_create_file("forcewake_all", 0400, root, xe,
>  			    &forcewake_all_fops);
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index 5d9e6e66c665..7acc57f6d4fd 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -472,6 +472,8 @@ struct xe_device {
>  		struct delayed_work work;
>  		/** @pmt.retry_count: late-bind probe retry */
>  		u32 retry_count;
> +		/** @pmt.punit_guid_cache: cache of the PUINT GUID */
> +		u32 punit_guid_cache;
>  	} pmt;
>  
>  	/** @soc_remapper: SoC remapper object */
> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> index 5284cab6703d..c4b44eb4b220 100644
> --- a/drivers/gpu/drm/xe/xe_hwmon.c
> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> @@ -517,9 +517,15 @@ xe_hwmon_energy_get(struct xe_hwmon *hwmon, int channel, long *energy)
>  
>  	if (hwmon->xe->info.platform == XE_BATTLEMAGE) {
>  		u64 pmt_val;
> +		u32 guid;
>  
> -		ret = xe_pmt_telem_read(hwmon->xe->drm.dev,
> -					xe_mmio_read32(mmio, PUNIT_TELEMETRY_GUID),
> +		guid = xe_vsec_get_guid(hwmon->xe);
> +		if (!guid) {
> +			drm_warn(&hwmon->xe->drm, "PMT device is not powered\n");
> +			*energy = 0;
> +			return;
> +		}
> +		ret = xe_pmt_telem_read(hwmon->xe->drm.dev, guid,
>  					&pmt_val, BMG_ENERGY_STATUS_PMT_OFFSET,	sizeof(pmt_val));
>  		if (ret != sizeof(pmt_val)) {
>  			drm_warn(&hwmon->xe->drm, "energy read from pmt failed, ret %d\n", ret);
> diff --git a/drivers/gpu/drm/xe/xe_pcode.c b/drivers/gpu/drm/xe/xe_pcode.c
> index e1b8062541a9..7831dd597794 100644
> --- a/drivers/gpu/drm/xe/xe_pcode.c
> +++ b/drivers/gpu/drm/xe/xe_pcode.c
> @@ -366,11 +366,17 @@ ALLOW_ERROR_INJECTION(xe_pcode_probe_early, ERRNO); /* See xe_pci_probe */
>  int xe_get_pcode_version(struct xe_device *xe, struct xe_pcode_version *version)
>  {
>  	int ret = 0;
> +	u32 guid;
>  
>  	guard(xe_pm_runtime)(xe);
>  
> -	ret = xe_pmt_telem_read(xe->drm.dev,
> -				xe_mmio_read32(xe_root_tile_mmio(xe), PUNIT_TELEMETRY_GUID),
> +	guid = xe_vsec_get_guid(xe);
> +	if (!guid) {
> +		xe_warn(xe, "PMT device is not powered\n");
> +		return -ENODATA;
> +	}
> +
> +	ret = xe_pmt_telem_read(xe->drm.dev, guid,
>  				(u64 *)version, PUNIT_VERSION_OFFSET, sizeof(*version));
>  	if (ret != sizeof(*version)) {
>  		xe_warn(xe, "pcode version read from PMT failed, ret %pe\n", ERR_PTR(ret));
> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
> index edc20c24137e..5ebb4ee9ef08 100644
> --- a/drivers/gpu/drm/xe/xe_vsec.c
> +++ b/drivers/gpu/drm/xe/xe_vsec.c
> @@ -564,6 +564,54 @@ static void vsec_disable_late_bind_work(void *arg)
>  		xe_pm_runtime_put(xe);
>  }
>  
> +u32 xe_vsec_get_guid(struct xe_device *xe)
> +{
> +	struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> +	u32 guid;
> +
> +	/*
> +	 * Both supported platforms (BMG, CRI) require the remapper callback to
> +	 * access data. CRI needs it for the GUID.
> +	 */
> +	if (!xe->soc_remapper.set_telem_region)
> +		return 0;
> +
> +	/* caller must ensure correct power state */
> +	if (!xe_pm_runtime_get_if_active(xe))

The comments sounds like it's a coding error somewhere if we're not 
active at this point, which would be easier to catch if there would be 
WARN_ON_ONCE() instead of silently hiding the problem.

(I don't know much about xe, so please keep it in mind when considering 
my comments on xe side code.)

-- 
 i.

> +		return 0;
> +
> +	mutex_lock(&xe->pmt.lock);
> +
> +	if (xe->pmt.punit_guid_cache) {
> +		guid = xe->pmt.punit_guid_cache;
> +		goto unlock;
> +	}
> +
> +	switch (xe->info.platform) {
> +	case XE_BATTLEMAGE:
> +		guid = xe_mmio_read32(mmio, BMG_PUNIT_TELEMETRY_GUID);
> +		break;
> +
> +	case XE_CRESCENTISLAND:
> +		xe->soc_remapper.set_telem_region(xe, CRI_IDX_TELEM_DISCOVERY);
> +		guid = xe_mmio_read32(mmio, CRI_PUNIT_TELEMETRY_GUID);
> +		break;
> +
> +	default:
> +		guid = 0;
> +		drm_err(&xe->drm, "Unsupported platform: %u\n", xe->info.platform);
> +		break;
> +	}
> +
> +	xe->pmt.punit_guid_cache = guid;
> +
> +unlock:
> +	mutex_unlock(&xe->pmt.lock);
> +	xe_pm_runtime_put(xe);
> +
> +	return guid;
> +}
> +
>  /**
>   * xe_vsec_init - Initialize resources and add intel_vsec auxiliary
>   * interface
> diff --git a/drivers/gpu/drm/xe/xe_vsec.h b/drivers/gpu/drm/xe/xe_vsec.h
> index c4a1e2fc67d8..43c3e9f227c5 100644
> --- a/drivers/gpu/drm/xe/xe_vsec.h
> +++ b/drivers/gpu/drm/xe/xe_vsec.h
> @@ -10,6 +10,7 @@ struct device;
>  struct xe_device;
>  
>  int xe_vsec_init(struct xe_device *xe);
> +u32 xe_vsec_get_guid(struct xe_device *xe);
>  int xe_pmt_telem_read(struct device *dev, u32 guid, u64 *data, loff_t user_offset, u32 count);
>  
>  #endif
>