[PATCH 0/5] Crescent Island PMT support

[PATCH 0/5] Crescent Island PMT support

[Michael J. Ruhl]

The Crescent Island (CRI) GPU supports PMT from the via
Xe driver registration.

The CRI PMT MMIO mapping is shared for each PMT feature and it
is necessary to set an index register value to access the correct
memory space.

The PMT driver has a callback infrastructure to access data areas
with driver specific access. It is does not support the PMT
control access.

CRI discovery/control spaces are part of the access window so a
driver callback is necessary to allow for the correct window to
be exposed.

Add control path callback support to the PMT driver.

Add CRI PMT discovery information.

Update the Xe driver to support the CRI PMT access.

The crashlog access needs to be done with power enabled (telem is
only available when the device is powered).
Determine which access is requested, and do the appropriate power
request.


Michael J. Ruhl (5):
  pmt: Add register access callbacks
  drm/xe/vsec: Use correct pm state get
  drm/xe/vsec: Support Crescent Island PMT
  drm/xe/vsec: Crescent Island PMT decode
  drm/xe/vsec: Crescent Island PMT callbacks

 drivers/gpu/drm/xe/regs/xe_pmt.h          |   5 +
 drivers/gpu/drm/xe/xe_vsec.c              | 257 +++++++++++++++++++---
 drivers/platform/x86/intel/pmt/crashlog.c |  39 +++-
 include/linux/intel_vsec.h                |  26 ++-
 4 files changed, 291 insertions(+), 36 deletions(-)

-- 
2.52.0

[PATCH 1/5] pmt: Add register access callbacks

[Michael J. Ruhl]

Some HW does not have the explicit access via MMIO.  Allow
for parent drivers to control access to the status/control
path.

Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
---
 drivers/platform/x86/intel/pmt/crashlog.c | 39 +++++++++++++++++++++--
 include/linux/intel_vsec.h                | 26 +++++++++++----
 2 files changed, 55 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/intel/pmt/crashlog.c b/drivers/platform/x86/intel/pmt/crashlog.c
index b0393c9c5b4b..978b35d56888 100644
--- a/drivers/platform/x86/intel/pmt/crashlog.c
+++ b/drivers/platform/x86/intel/pmt/crashlog.c
@@ -129,7 +129,19 @@ static void pmt_crashlog_rmw(struct crashlog_entry *crashlog, u32 bit, bool set)
 {
 	const struct crashlog_control *control = &crashlog->info->control;
 	struct intel_pmt_entry *entry = &crashlog->entry;
-	u32 reg = readl(entry->disc_table + control->offset);
+	u32 guid = entry->header.guid;
+	u32 reg;
+	int err;
+
+	if (entry->cb->read_reg) {
+		err = entry->cb->read_reg(entry->pcidev, guid, &reg, control->offset);
+		if (err) {
+			pr_err("%s: failed to read reg: %d\n", __func__, err);
+			return;
+		}
+	} else {
+		reg = readl(entry->disc_table + control->offset);
+	}
 
 	reg &= ~control->trigger_mask;
 
@@ -138,14 +150,35 @@ static void pmt_crashlog_rmw(struct crashlog_entry *crashlog, u32 bit, bool set)
 	else
 		reg &= ~bit;
 
-	writel(reg, entry->disc_table + control->offset);
+	if (entry->cb->write_reg) {
+		err = entry->cb->write_reg(entry->pcidev, guid, reg, control->offset);
+		if (err) {
+			pr_err("%s: failed to write reg: %d\n", __func__, err);
+			return;
+		}
+	} else {
+		writel(reg, entry->disc_table + control->offset);
+	}
 }
 
 /* Read the status register and see if the specified @bit is set */
 static bool pmt_crashlog_rc(struct crashlog_entry *crashlog, u32 bit)
 {
 	const struct crashlog_status *status = &crashlog->info->status;
-	u32 reg = readl(crashlog->entry.disc_table + status->offset);
+	struct intel_pmt_entry *entry = &crashlog->entry;
+	u32 guid = entry->header.guid;
+	u32 reg;
+	int err;
+
+	if (entry->cb->read_reg) {
+		err = entry->cb->read_reg(entry->pcidev, guid, &reg, status->offset);
+		if (err) {
+			pr_err("%s: failed to read reg: %d\n", __func__, err);
+			return false;
+		}
+	} else {
+		reg = readl(crashlog->entry.disc_table + status->offset);
+	}
 
 	return !!(reg & bit);
 }
diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h
index 1a0f357c2427..5416f84aca40 100644
--- a/include/linux/intel_vsec.h
+++ b/include/linux/intel_vsec.h
@@ -80,16 +80,28 @@ enum intel_vsec_quirks {
 
 /**
  * struct pmt_callbacks - Callback infrastructure for PMT devices
- * @read_telem: when specified, called by client driver to access PMT
- * data (instead of direct copy).
- * * pdev:  PCI device reference for the callback's use
- * * guid:  ID of data to acccss
- * * data:  buffer for the data to be copied
- * * off:   offset into the requested buffer
- * * count: size of buffer
+ * ->read_telem() when specified, called by client driver to access PMT data (instead
+ * of direct copy).
+ * @pdev:  PCI device reference for the callback's use
+ * @guid:  ID of data to access
+ * @data:  buffer for the data to be copied
+ * @off:   offset into the requested buffer
+ * @count: size of buffer
+ * ->read_reg() when specified called by client driver to read PMT state
+ * @pdev: PCI device reference for the callback's use
+ * @guid: ID of data to access
+ * @data: buffer for the register data to be read
+ * @offset: offset of control register to access
+ * ->write_reg() when specified called by client driver to write PMT state
+ * @pdev: PCI device reference for the callback's use
+ * @guid: ID of data to access
+ * @data: buffer data to be written to the register
+ * @offset: offset of control register to access
  */
 struct pmt_callbacks {
 	int (*read_telem)(struct pci_dev *pdev, u32 guid, u64 *data, loff_t off, u32 count);
+	int (*read_reg)(struct pci_dev *pdev, u32 guid, u32 *data, u32 offset);
+	int (*write_reg)(struct pci_dev *pdev, u32 guid, u32 data, u32 offset);
 };
 
 struct vsec_feature_dependency {
-- 
2.52.0

Re: [PATCH 1/5] pmt: Add register access callbacks

[Ilpo Järvinen]

On Tue, 27 Jan 2026, Michael J. Ruhl wrote:

> Some HW does not have the explicit access via MMIO.  Allow

One space is enough after a stop.

Please reflow to 72 chars.

> for parent drivers to control access to the status/control
> path.

This sounds quite vague compared with what you're doing (adding ability to 
provide custom read/write accessors).

> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> ---
>  drivers/platform/x86/intel/pmt/crashlog.c | 39 +++++++++++++++++++++--
>  include/linux/intel_vsec.h                | 26 +++++++++++----
>  2 files changed, 55 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/pmt/crashlog.c b/drivers/platform/x86/intel/pmt/crashlog.c
> index b0393c9c5b4b..978b35d56888 100644
> --- a/drivers/platform/x86/intel/pmt/crashlog.c
> +++ b/drivers/platform/x86/intel/pmt/crashlog.c
> @@ -129,7 +129,19 @@ static void pmt_crashlog_rmw(struct crashlog_entry *crashlog, u32 bit, bool set)
>  {
>  	const struct crashlog_control *control = &crashlog->info->control;
>  	struct intel_pmt_entry *entry = &crashlog->entry;
> -	u32 reg = readl(entry->disc_table + control->offset);
> +	u32 guid = entry->header.guid;
> +	u32 reg;
> +	int err;
> +
> +	if (entry->cb->read_reg) {
> +		err = entry->cb->read_reg(entry->pcidev, guid, &reg, control->offset);
> +		if (err) {
> +			pr_err("%s: failed to read reg: %d\n", __func__, err);

Please don't ever use __func__ in prints intended to be visible for normal 
users.

> +			return;
> +		}
> +	} else {
> +		reg = readl(entry->disc_table + control->offset);
> +	}
>  
>  	reg &= ~control->trigger_mask;
>  
> @@ -138,14 +150,35 @@ static void pmt_crashlog_rmw(struct crashlog_entry *crashlog, u32 bit, bool set)
>  	else
>  		reg &= ~bit;
>  
> -	writel(reg, entry->disc_table + control->offset);
> +	if (entry->cb->write_reg) {
> +		err = entry->cb->write_reg(entry->pcidev, guid, reg, control->offset);
> +		if (err) {
> +			pr_err("%s: failed to write reg: %d\n", __func__, err);
> +			return;
> +		}
> +	} else {
> +		writel(reg, entry->disc_table + control->offset);
> +	}
>  }
>  
>  /* Read the status register and see if the specified @bit is set */
>  static bool pmt_crashlog_rc(struct crashlog_entry *crashlog, u32 bit)
>  {
>  	const struct crashlog_status *status = &crashlog->info->status;
> -	u32 reg = readl(crashlog->entry.disc_table + status->offset);
> +	struct intel_pmt_entry *entry = &crashlog->entry;
> +	u32 guid = entry->header.guid;
> +	u32 reg;
> +	int err;
> +
> +	if (entry->cb->read_reg) {
> +		err = entry->cb->read_reg(entry->pcidev, guid, &reg, status->offset);
> +		if (err) {
> +			pr_err("%s: failed to read reg: %d\n", __func__, err);
> +			return false;
> +		}
> +	} else {
> +		reg = readl(crashlog->entry.disc_table + status->offset);
> +	}
>  
>  	return !!(reg & bit);
>  }
> diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h
> index 1a0f357c2427..5416f84aca40 100644
> --- a/include/linux/intel_vsec.h
> +++ b/include/linux/intel_vsec.h
> @@ -80,16 +80,28 @@ enum intel_vsec_quirks {
>  
>  /**
>   * struct pmt_callbacks - Callback infrastructure for PMT devices
> - * @read_telem: when specified, called by client driver to access PMT
> - * data (instead of direct copy).
> - * * pdev:  PCI device reference for the callback's use
> - * * guid:  ID of data to acccss
> - * * data:  buffer for the data to be copied
> - * * off:   offset into the requested buffer
> - * * count: size of buffer
> + * ->read_telem() when specified, called by client driver to access PMT data (instead
> + * of direct copy).
> + * @pdev:  PCI device reference for the callback's use
> + * @guid:  ID of data to access
> + * @data:  buffer for the data to be copied
> + * @off:   offset into the requested buffer
> + * @count: size of buffer
> + * ->read_reg() when specified called by client driver to read PMT state
> + * @pdev: PCI device reference for the callback's use
> + * @guid: ID of data to access
> + * @data: buffer for the register data to be read
> + * @offset: offset of control register to access
> + * ->write_reg() when specified called by client driver to write PMT state
> + * @pdev: PCI device reference for the callback's use
> + * @guid: ID of data to access
> + * @data: buffer data to be written to the register
> + * @offset: offset of control register to access

Have you checked that these don't generate warnings and how they look 
visually?

>   */
>  struct pmt_callbacks {
>  	int (*read_telem)(struct pci_dev *pdev, u32 guid, u64 *data, loff_t off, u32 count);
> +	int (*read_reg)(struct pci_dev *pdev, u32 guid, u32 *data, u32 offset);
> +	int (*write_reg)(struct pci_dev *pdev, u32 guid, u32 data, u32 offset);
>  };
>  
>  struct vsec_feature_dependency {
> 

-- 
 i.

RE: [PATCH 1/5] pmt: Add register access callbacks

[Ruhl, Michael J]

>-----Original Message-----
>From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>Sent: Wednesday, January 28, 2026 7:27 AM
>To: Ruhl, Michael J <michael.j.ruhl@intel.com>
>Cc: platform-driver-x86@vger.kernel.org; intel-xe@lists.freedesktop.org; Hans
>de Goede <hansg@kernel.org>; Brost, Matthew <matthew.brost@intel.com>;
>Vivi, Rodrigo <rodrigo.vivi@intel.com>; thomas.hellstrom@linux.intel.com;
>airlied@gmail.com; simona@ffwll.ch; david.e.box@linux.intel.com
>Subject: Re: [PATCH 1/5] pmt: Add register access callbacks
>
>On Tue, 27 Jan 2026, Michael J. Ruhl wrote:
>
>> Some HW does not have the explicit access via MMIO.  Allow
>
>One space is enough after a stop.
>
>Please reflow to 72 chars.

Will fix.

>> for parent drivers to control access to the status/control
>> path.
>
>This sounds quite vague compared with what you're doing (adding ability to
>provide custom read/write accessors).

I will reword.

>> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
>> ---
>>  drivers/platform/x86/intel/pmt/crashlog.c | 39 +++++++++++++++++++++-
>-
>>  include/linux/intel_vsec.h                | 26 +++++++++++----
>>  2 files changed, 55 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/platform/x86/intel/pmt/crashlog.c
>b/drivers/platform/x86/intel/pmt/crashlog.c
>> index b0393c9c5b4b..978b35d56888 100644
>> --- a/drivers/platform/x86/intel/pmt/crashlog.c
>> +++ b/drivers/platform/x86/intel/pmt/crashlog.c
>> @@ -129,7 +129,19 @@ static void pmt_crashlog_rmw(struct
>crashlog_entry *crashlog, u32 bit, bool set)
>>  {
>>  	const struct crashlog_control *control = &crashlog->info->control;
>>  	struct intel_pmt_entry *entry = &crashlog->entry;
>> -	u32 reg = readl(entry->disc_table + control->offset);
>> +	u32 guid = entry->header.guid;
>> +	u32 reg;
>> +	int err;
>> +
>> +	if (entry->cb->read_reg) {
>> +		err = entry->cb->read_reg(entry->pcidev, guid, &reg, control-
>>offset);
>> +		if (err) {
>> +			pr_err("%s: failed to read reg: %d\n", __func__, err);
>
>Please don't ever use __func__ in prints intended to be visible for normal
>users.

Missed that on my clean up path.  I have removed the __func__ usage.

>> +			return;
>> +		}
>> +	} else {
>> +		reg = readl(entry->disc_table + control->offset);
>> +	}
>>
>>  	reg &= ~control->trigger_mask;
>>
>> @@ -138,14 +150,35 @@ static void pmt_crashlog_rmw(struct
>crashlog_entry *crashlog, u32 bit, bool set)
>>  	else
>>  		reg &= ~bit;
>>
>> -	writel(reg, entry->disc_table + control->offset);
>> +	if (entry->cb->write_reg) {
>> +		err = entry->cb->write_reg(entry->pcidev, guid, reg, control-
>>offset);
>> +		if (err) {
>> +			pr_err("%s: failed to write reg: %d\n", __func__, err);
>> +			return;
>> +		}
>> +	} else {
>> +		writel(reg, entry->disc_table + control->offset);
>> +	}
>>  }
>>
>>  /* Read the status register and see if the specified @bit is set */
>>  static bool pmt_crashlog_rc(struct crashlog_entry *crashlog, u32 bit)
>>  {
>>  	const struct crashlog_status *status = &crashlog->info->status;
>> -	u32 reg = readl(crashlog->entry.disc_table + status->offset);
>> +	struct intel_pmt_entry *entry = &crashlog->entry;
>> +	u32 guid = entry->header.guid;
>> +	u32 reg;
>> +	int err;
>> +
>> +	if (entry->cb->read_reg) {
>> +		err = entry->cb->read_reg(entry->pcidev, guid, &reg, status-
>>offset);
>> +		if (err) {
>> +			pr_err("%s: failed to read reg: %d\n", __func__, err);
>> +			return false;
>> +		}
>> +	} else {
>> +		reg = readl(crashlog->entry.disc_table + status->offset);
>> +	}
>>
>>  	return !!(reg & bit);
>>  }
>> diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h
>> index 1a0f357c2427..5416f84aca40 100644
>> --- a/include/linux/intel_vsec.h
>> +++ b/include/linux/intel_vsec.h
>> @@ -80,16 +80,28 @@ enum intel_vsec_quirks {
>>
>>  /**
>>   * struct pmt_callbacks - Callback infrastructure for PMT devices
>> - * @read_telem: when specified, called by client driver to access PMT
>> - * data (instead of direct copy).
>> - * * pdev:  PCI device reference for the callback's use
>> - * * guid:  ID of data to acccss
>> - * * data:  buffer for the data to be copied
>> - * * off:   offset into the requested buffer
>> - * * count: size of buffer
>> + * ->read_telem() when specified, called by client driver to access PMT data
>(instead
>> + * of direct copy).
>> + * @pdev:  PCI device reference for the callback's use
>> + * @guid:  ID of data to access
>> + * @data:  buffer for the data to be copied
>> + * @off:   offset into the requested buffer
>> + * @count: size of buffer
>> + * ->read_reg() when specified called by client driver to read PMT state
>> + * @pdev: PCI device reference for the callback's use
>> + * @guid: ID of data to access
>> + * @data: buffer for the register data to be read
>> + * @offset: offset of control register to access
>> + * ->write_reg() when specified called by client driver to write PMT state
>> + * @pdev: PCI device reference for the callback's use
>> + * @guid: ID of data to access
>> + * @data: buffer data to be written to the register
>> + * @offset: offset of control register to access
>
>Have you checked that these don't generate warnings and how they look
>visually?

Are you referring to the kernel-doc usage?

The read_telem(), read_reg() and write_reg() do give an error with .  I have cleaned them up so there
is no more kernel-doc errors...  Is there something else to look at?

I used:

./scripts/kernel-doc -v --none include/linux/intel_vsec.h

And cleaned up the reported errors.

Thanks!

Mike

>>   */
>>  struct pmt_callbacks {
>>  	int (*read_telem)(struct pci_dev *pdev, u32 guid, u64 *data, loff_t off,
>u32 count);
>> +	int (*read_reg)(struct pci_dev *pdev, u32 guid, u32 *data, u32 offset);
>> +	int (*write_reg)(struct pci_dev *pdev, u32 guid, u32 data, u32 offset);
>>  };
>>
>>  struct vsec_feature_dependency {
>>
>
>--
> i.

Re: [PATCH 1/5] pmt: Add register access callbacks

[Jani Nikula]

On Tue, 27 Jan 2026, "Michael J. Ruhl" <michael.j.ruhl@intel.com> wrote:
> Some HW does not have the explicit access via MMIO.  Allow
> for parent drivers to control access to the status/control
> path.

I think this is quite vague, and could use some copy-paste from the
cover letter, for posterity.

BR,
Jani.

>
> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> ---
>  drivers/platform/x86/intel/pmt/crashlog.c | 39 +++++++++++++++++++++--
>  include/linux/intel_vsec.h                | 26 +++++++++++----
>  2 files changed, 55 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/pmt/crashlog.c b/drivers/platform/x86/intel/pmt/crashlog.c
> index b0393c9c5b4b..978b35d56888 100644
> --- a/drivers/platform/x86/intel/pmt/crashlog.c
> +++ b/drivers/platform/x86/intel/pmt/crashlog.c
> @@ -129,7 +129,19 @@ static void pmt_crashlog_rmw(struct crashlog_entry *crashlog, u32 bit, bool set)
>  {
>  	const struct crashlog_control *control = &crashlog->info->control;
>  	struct intel_pmt_entry *entry = &crashlog->entry;
> -	u32 reg = readl(entry->disc_table + control->offset);
> +	u32 guid = entry->header.guid;
> +	u32 reg;
> +	int err;
> +
> +	if (entry->cb->read_reg) {
> +		err = entry->cb->read_reg(entry->pcidev, guid, &reg, control->offset);
> +		if (err) {
> +			pr_err("%s: failed to read reg: %d\n", __func__, err);
> +			return;
> +		}
> +	} else {
> +		reg = readl(entry->disc_table + control->offset);
> +	}
>  
>  	reg &= ~control->trigger_mask;
>  
> @@ -138,14 +150,35 @@ static void pmt_crashlog_rmw(struct crashlog_entry *crashlog, u32 bit, bool set)
>  	else
>  		reg &= ~bit;
>  
> -	writel(reg, entry->disc_table + control->offset);
> +	if (entry->cb->write_reg) {
> +		err = entry->cb->write_reg(entry->pcidev, guid, reg, control->offset);
> +		if (err) {
> +			pr_err("%s: failed to write reg: %d\n", __func__, err);
> +			return;
> +		}
> +	} else {
> +		writel(reg, entry->disc_table + control->offset);
> +	}
>  }
>  
>  /* Read the status register and see if the specified @bit is set */
>  static bool pmt_crashlog_rc(struct crashlog_entry *crashlog, u32 bit)
>  {
>  	const struct crashlog_status *status = &crashlog->info->status;
> -	u32 reg = readl(crashlog->entry.disc_table + status->offset);
> +	struct intel_pmt_entry *entry = &crashlog->entry;
> +	u32 guid = entry->header.guid;
> +	u32 reg;
> +	int err;
> +
> +	if (entry->cb->read_reg) {
> +		err = entry->cb->read_reg(entry->pcidev, guid, &reg, status->offset);
> +		if (err) {
> +			pr_err("%s: failed to read reg: %d\n", __func__, err);
> +			return false;
> +		}
> +	} else {
> +		reg = readl(crashlog->entry.disc_table + status->offset);
> +	}
>  
>  	return !!(reg & bit);
>  }
> diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h
> index 1a0f357c2427..5416f84aca40 100644
> --- a/include/linux/intel_vsec.h
> +++ b/include/linux/intel_vsec.h
> @@ -80,16 +80,28 @@ enum intel_vsec_quirks {
>  
>  /**
>   * struct pmt_callbacks - Callback infrastructure for PMT devices
> - * @read_telem: when specified, called by client driver to access PMT
> - * data (instead of direct copy).
> - * * pdev:  PCI device reference for the callback's use
> - * * guid:  ID of data to acccss
> - * * data:  buffer for the data to be copied
> - * * off:   offset into the requested buffer
> - * * count: size of buffer
> + * ->read_telem() when specified, called by client driver to access PMT data (instead
> + * of direct copy).
> + * @pdev:  PCI device reference for the callback's use
> + * @guid:  ID of data to access
> + * @data:  buffer for the data to be copied
> + * @off:   offset into the requested buffer
> + * @count: size of buffer
> + * ->read_reg() when specified called by client driver to read PMT state
> + * @pdev: PCI device reference for the callback's use
> + * @guid: ID of data to access
> + * @data: buffer for the register data to be read
> + * @offset: offset of control register to access
> + * ->write_reg() when specified called by client driver to write PMT state
> + * @pdev: PCI device reference for the callback's use
> + * @guid: ID of data to access
> + * @data: buffer data to be written to the register
> + * @offset: offset of control register to access
>   */
>  struct pmt_callbacks {
>  	int (*read_telem)(struct pci_dev *pdev, u32 guid, u64 *data, loff_t off, u32 count);
> +	int (*read_reg)(struct pci_dev *pdev, u32 guid, u32 *data, u32 offset);
> +	int (*write_reg)(struct pci_dev *pdev, u32 guid, u32 data, u32 offset);
>  };
>  
>  struct vsec_feature_dependency {

-- 
Jani Nikula, Intel

[PATCH 2/5] drm/xe/vsec: Use correct pm state get

[Michael J. Ruhl]

Crashlog needs to be collected at all times.  The current pm
check assumes telemetry only.

Update read path to enable device for crashlog instances.

Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
---
 drivers/gpu/drm/xe/xe_vsec.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
index 4ebb4dbe1c9b..44607f1eaa88 100644
--- a/drivers/gpu/drm/xe/xe_vsec.c
+++ b/drivers/gpu/drm/xe/xe_vsec.c
@@ -145,6 +145,7 @@ int xe_pmt_telem_read(struct pci_dev *pdev, u32 guid, u64 *data, loff_t user_off
 {
 	struct xe_device *xe = pdev_to_xe_device(pdev);
 	void __iomem *telem_addr = xe->mmio.regs + BMG_TELEMETRY_OFFSET;
+	u32 cap_type = FIELD_GET(GUID_CAP_TYPE, guid);
 	u32 mem_region;
 	u32 offset;
 	int ret;
@@ -160,9 +161,18 @@ int xe_pmt_telem_read(struct pci_dev *pdev, u32 guid, u64 *data, loff_t user_off
 	if (!xe->soc_remapper.set_telem_region)
 		return -ENODEV;
 
-	/* indicate that we are not at an appropriate power level */
-	if (!xe_pm_runtime_get_if_active(xe))
-		return -ENODATA;
+	/* make sure that we are not at an inappropriate power level */
+	switch (cap_type) {
+	case CRASHLOG:
+		xe_pm_runtime_get(xe);
+		break;
+	case TELEMETRY:
+		if (!xe_pm_runtime_get_if_active(xe))
+			return -ENODATA;
+		break;
+	case WATCHER:
+		return -EINVAL;
+	}
 
 	/* set SoC re-mapper index register based on GUID memory region */
 	xe->soc_remapper.set_telem_region(xe, mem_region);
-- 
2.52.0

Re: [PATCH 2/5] drm/xe/vsec: Use correct pm state get

[Ilpo Järvinen]

On Tue, 27 Jan 2026, Michael J. Ruhl wrote:

> Crashlog needs to be collected at all times.  The current pm
> check assumes telemetry only.
> 
> Update read path to enable device for crashlog instances.
> 
> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_vsec.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
> index 4ebb4dbe1c9b..44607f1eaa88 100644
> --- a/drivers/gpu/drm/xe/xe_vsec.c
> +++ b/drivers/gpu/drm/xe/xe_vsec.c
> @@ -145,6 +145,7 @@ int xe_pmt_telem_read(struct pci_dev *pdev, u32 guid, u64 *data, loff_t user_off
>  {
>  	struct xe_device *xe = pdev_to_xe_device(pdev);
>  	void __iomem *telem_addr = xe->mmio.regs + BMG_TELEMETRY_OFFSET;
> +	u32 cap_type = FIELD_GET(GUID_CAP_TYPE, guid);
>  	u32 mem_region;
>  	u32 offset;
>  	int ret;
> @@ -160,9 +161,18 @@ int xe_pmt_telem_read(struct pci_dev *pdev, u32 guid, u64 *data, loff_t user_off
>  	if (!xe->soc_remapper.set_telem_region)
>  		return -ENODEV;
>  
> -	/* indicate that we are not at an appropriate power level */
> -	if (!xe_pm_runtime_get_if_active(xe))
> -		return -ENODATA;
> +	/* make sure that we are not at an inappropriate power level */
> +	switch (cap_type) {
> +	case CRASHLOG:
> +		xe_pm_runtime_get(xe);

While this is xe code so I don't know much about it, it feels odd to me to 
just add a get without put or explanation in the changelog why this change 
doesn't result in imbalance.

-- 
 i.


> +		break;
> +	case TELEMETRY:
> +		if (!xe_pm_runtime_get_if_active(xe))
> +			return -ENODATA;
> +		break;
> +	case WATCHER:
> +		return -EINVAL;
> +	}
>  
>  	/* set SoC re-mapper index register based on GUID memory region */
>  	xe->soc_remapper.set_telem_region(xe, mem_region);
> 

RE: [PATCH 2/5] drm/xe/vsec: Use correct pm state get

[Ruhl, Michael J]

>-----Original Message-----
>From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>Sent: Wednesday, January 28, 2026 7:42 AM
>To: Ruhl, Michael J <michael.j.ruhl@intel.com>
>Cc: platform-driver-x86@vger.kernel.org; intel-xe@lists.freedesktop.org; Hans
>de Goede <hansg@kernel.org>; Brost, Matthew <matthew.brost@intel.com>;
>Vivi, Rodrigo <rodrigo.vivi@intel.com>; thomas.hellstrom@linux.intel.com;
>airlied@gmail.com; simona@ffwll.ch; david.e.box@linux.intel.com
>Subject: Re: [PATCH 2/5] drm/xe/vsec: Use correct pm state get
>
>On Tue, 27 Jan 2026, Michael J. Ruhl wrote:
>
>> Crashlog needs to be collected at all times.  The current pm
>> check assumes telemetry only.
>>
>> Update read path to enable device for crashlog instances.
>>
>> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_vsec.c | 16 +++++++++++++---
>>  1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
>> index 4ebb4dbe1c9b..44607f1eaa88 100644
>> --- a/drivers/gpu/drm/xe/xe_vsec.c
>> +++ b/drivers/gpu/drm/xe/xe_vsec.c
>> @@ -145,6 +145,7 @@ int xe_pmt_telem_read(struct pci_dev *pdev, u32
>guid, u64 *data, loff_t user_off
>>  {
>>  	struct xe_device *xe = pdev_to_xe_device(pdev);
>>  	void __iomem *telem_addr = xe->mmio.regs +
>BMG_TELEMETRY_OFFSET;
>> +	u32 cap_type = FIELD_GET(GUID_CAP_TYPE, guid);
>>  	u32 mem_region;
>>  	u32 offset;
>>  	int ret;
>> @@ -160,9 +161,18 @@ int xe_pmt_telem_read(struct pci_dev *pdev, u32
>guid, u64 *data, loff_t user_off
>>  	if (!xe->soc_remapper.set_telem_region)
>>  		return -ENODEV;
>>
>> -	/* indicate that we are not at an appropriate power level */
>> -	if (!xe_pm_runtime_get_if_active(xe))
>> -		return -ENODATA;
>> +	/* make sure that we are not at an inappropriate power level */
>> +	switch (cap_type) {
>> +	case CRASHLOG:
>> +		xe_pm_runtime_get(xe);
>
>While this is xe code so I don't know much about it, it feels odd to me to
>just add a get without put or explanation in the changelog why this change
>doesn't result in imbalance.

There is an existing xe_pm_runtime_put() before this function exits.  (original
line ~170, new line ~181).  Is this what you are looking for?

The TELEM entry will not take the reference if the device is not powered,
so a put is unnecessary (exit on not enabled).

CRASLOG requires power, so the _get() will power the device and take
reference.

Thanks,

Mike
>--
> i.
>
>
>> +		break;
>> +	case TELEMETRY:
>> +		if (!xe_pm_runtime_get_if_active(xe))
>> +			return -ENODATA;
>> +		break;
>> +	case WATCHER:
>> +		return -EINVAL;
>> +	}
>>
>>  	/* set SoC re-mapper index register based on GUID memory region */
>>  	xe->soc_remapper.set_telem_region(xe, mem_region);
>>

RE: [PATCH 2/5] drm/xe/vsec: Use correct pm state get

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2783 bytes --]

On Wed, 28 Jan 2026, Ruhl, Michael J wrote:

> >-----Original Message-----
> >From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> >Sent: Wednesday, January 28, 2026 7:42 AM
> >To: Ruhl, Michael J <michael.j.ruhl@intel.com>
> >Cc: platform-driver-x86@vger.kernel.org; intel-xe@lists.freedesktop.org; Hans
> >de Goede <hansg@kernel.org>; Brost, Matthew <matthew.brost@intel.com>;
> >Vivi, Rodrigo <rodrigo.vivi@intel.com>; thomas.hellstrom@linux.intel.com;
> >airlied@gmail.com; simona@ffwll.ch; david.e.box@linux.intel.com
> >Subject: Re: [PATCH 2/5] drm/xe/vsec: Use correct pm state get
> >
> >On Tue, 27 Jan 2026, Michael J. Ruhl wrote:
> >
> >> Crashlog needs to be collected at all times.  The current pm
> >> check assumes telemetry only.
> >>
> >> Update read path to enable device for crashlog instances.
> >>
> >> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> >> ---
> >>  drivers/gpu/drm/xe/xe_vsec.c | 16 +++++++++++++---
> >>  1 file changed, 13 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
> >> index 4ebb4dbe1c9b..44607f1eaa88 100644
> >> --- a/drivers/gpu/drm/xe/xe_vsec.c
> >> +++ b/drivers/gpu/drm/xe/xe_vsec.c
> >> @@ -145,6 +145,7 @@ int xe_pmt_telem_read(struct pci_dev *pdev, u32
> >guid, u64 *data, loff_t user_off
> >>  {
> >>  	struct xe_device *xe = pdev_to_xe_device(pdev);
> >>  	void __iomem *telem_addr = xe->mmio.regs +
> >BMG_TELEMETRY_OFFSET;
> >> +	u32 cap_type = FIELD_GET(GUID_CAP_TYPE, guid);
> >>  	u32 mem_region;
> >>  	u32 offset;
> >>  	int ret;
> >> @@ -160,9 +161,18 @@ int xe_pmt_telem_read(struct pci_dev *pdev, u32
> >guid, u64 *data, loff_t user_off
> >>  	if (!xe->soc_remapper.set_telem_region)
> >>  		return -ENODEV;
> >>
> >> -	/* indicate that we are not at an appropriate power level */
> >> -	if (!xe_pm_runtime_get_if_active(xe))
> >> -		return -ENODATA;
> >> +	/* make sure that we are not at an inappropriate power level */
> >> +	switch (cap_type) {
> >> +	case CRASHLOG:
> >> +		xe_pm_runtime_get(xe);
> >
> >While this is xe code so I don't know much about it, it feels odd to me to
> >just add a get without put or explanation in the changelog why this change
> >doesn't result in imbalance.
> 
> There is an existing xe_pm_runtime_put() before this function exits.  (original
> line ~170, new line ~181).  Is this what you are looking for?
> 
> The TELEM entry will not take the reference if the device is not powered,
> so a put is unnecessary (exit on not enabled).
> 
> CRASLOG requires power, so the _get() will power the device and take
> reference.

I see, I didn't look up the function but just read the patch and found it 
odd. I guess it's fine as is.

-- 
 i.

[PATCH 3/5] drm/xe/vsec: Support Crescent Island PMT

[Michael J. Ruhl]

Crescent Island (CRI) supports PMT telemetry and crashlog.

Add Crescent Island (CRI) discovery structure (DVSEC)
information to allow for Xe registration.

Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
---
 drivers/gpu/drm/xe/regs/xe_pmt.h |  5 ++++
 drivers/gpu/drm/xe/xe_vsec.c     | 43 ++++++++++++++++++++++++++++++--
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/xe/regs/xe_pmt.h b/drivers/gpu/drm/xe/regs/xe_pmt.h
index 240d57993ea6..874f44b79780 100644
--- a/drivers/gpu/drm/xe/regs/xe_pmt.h
+++ b/drivers/gpu/drm/xe/regs/xe_pmt.h
@@ -18,6 +18,11 @@
 #define BMG_TELEMETRY_BASE_OFFSET	0xE0000
 #define BMG_TELEMETRY_OFFSET		(SOC_BASE + BMG_TELEMETRY_BASE_OFFSET)
 
+#define CRI_TELEMETRY_BASE_OFFSET	0xE0000
+/* for CRI discovery and telemetry are in an indexed window */
+#define CRI_DISCOVERY_OFFSET		(SOC_BASE + CRI_TELEMETRY_BASE_OFFSET)
+#define CRI_TELEMETRY_OFFSET		(SOC_BASE + CRI_TELEMETRY_BASE_OFFSET)
+
 #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_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
index 44607f1eaa88..254f7ebca6eb 100644
--- a/drivers/gpu/drm/xe/xe_vsec.c
+++ b/drivers/gpu/drm/xe/xe_vsec.c
@@ -19,8 +19,16 @@
 
 #include "regs/xe_pmt.h"
 
-/* PMT GUID value for BMG devices.  NOTE: this is NOT a PCI id */
+/* PMT GUID value for BMG and CRI devices.  NOTE: this is NOT a PCI id */
 #define BMG_DEVICE_ID 0xE2F8
+#define CRI_DEVICE_ID 0xE2F9
+
+/*
+ * sizeof(Crashlog Type1 Version2) = 0x18 (24) bytes
+ * For BMG and CRI crashlogs are consecutive and start at 0x60.
+ */
+#define PUNIT_DISC_OFFSET 0x60
+#define OOBMSM_DISC_OFFSET (PUNIT_DISC_OFFSET + 0x18)
 
 static struct intel_vsec_header bmg_telemetry = {
 	.rev = 1,
@@ -39,7 +47,7 @@ static struct intel_vsec_header bmg_crashlog = {
 	.num_entries = 2,
 	.entry_size = 6,
 	.tbir = 0,
-	.offset = BMG_DISCOVERY_OFFSET + 0x60,
+	.offset = BMG_DISCOVERY_OFFSET + PUNIT_DISC_OFFSET,
 };
 
 static struct intel_vsec_header *bmg_capabilities[] = {
@@ -48,9 +56,36 @@ static struct intel_vsec_header *bmg_capabilities[] = {
 	NULL
 };
 
+static struct intel_vsec_header cri_telemetry = {
+	.rev = 1,
+	.length = 0x10,
+	.id = VSEC_ID_TELEMETRY,
+	.num_entries = 3,
+	.entry_size = 4,
+	.tbir = 0,
+	.offset = CRI_DISCOVERY_OFFSET,
+};
+
+static struct intel_vsec_header cri_crashlog = {
+	.rev = 1,
+	.length = 0x10,
+	.id = VSEC_ID_CRASHLOG,
+	.num_entries = 2,
+	.entry_size = 6,
+	.tbir = 0,
+	.offset = CRI_DISCOVERY_OFFSET + PUNIT_DISC_OFFSET,
+};
+
+static struct intel_vsec_header *cri_capabilities[] = {
+	&cri_telemetry,
+	&cri_crashlog,
+	NULL
+};
+
 enum xe_vsec {
 	XE_VSEC_UNKNOWN = 0,
 	XE_VSEC_BMG,
+	XE_VSEC_CRI,
 };
 
 static struct intel_vsec_platform_info xe_vsec_info[] = {
@@ -58,6 +93,10 @@ static struct intel_vsec_platform_info xe_vsec_info[] = {
 		.caps = VSEC_CAP_TELEMETRY | VSEC_CAP_CRASHLOG,
 		.headers = bmg_capabilities,
 	},
+	[XE_VSEC_CRI] = {
+		.caps = VSEC_CAP_TELEMETRY | VSEC_CAP_CRASHLOG,
+		.headers = cri_capabilities,
+	},
 	{ }
 };
 
-- 
2.52.0

[PATCH 4/5] drm/xe/vsec: Crescent Island PMT decode

[Michael J. Ruhl]

Crescent Island (CRI) has different index and offset values for
accessing the PMT data area.

Update the decode path to support the CRI device.

Update the data read callback so to support the CRI usage.

Define several magic numbers.

Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
---
 drivers/gpu/drm/xe/xe_vsec.c | 114 +++++++++++++++++++++++++++++------
 1 file changed, 96 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
index 254f7ebca6eb..4bddc22d86c7 100644
--- a/drivers/gpu/drm/xe/xe_vsec.c
+++ b/drivers/gpu/drm/xe/xe_vsec.c
@@ -116,10 +116,27 @@ static struct intel_vsec_platform_info xe_vsec_info[] = {
 #define GUID_CAP_TYPE		GENMASK(29, 28)
 #define GUID_RECORD_ID		GENMASK(31, 30)
 
-#define PUNIT_TELEMETRY_OFFSET		0x0200
-#define PUNIT_WATCHER_OFFSET		0x14A0
-#define OOBMSM_0_WATCHER_OFFSET		0x18D8
-#define OOBMSM_1_TELEMETRY_OFFSET	0x1000
+#define BMG_IDX_TELEM_PUNIT		0x00
+#define BMG_IDX_TELEM_OOBMSM		0x01
+#define BMG_IDX_CRASHLOG_PUNIT		0x02
+#define BMG_IDX_CRASHLOG_OOBMSM		0x04
+
+#define BMG_PUNIT_TELEMETRY_OFFSET	0x0200
+#define BMG_PUNIT_WATCHER_OFFSET	0x14A0
+#define BMG_OOBMSM_0_WATCHER_OFFSET	0x18D8
+#define BMG_OOBMSM_1_TELEMETRY_OFFSET	0x1000
+
+#define CRI_IDX_TELEM_DISCOVERY		0x00
+#define CRI_IDX_TELEM_PUNIT		0x01
+#define CRI_IDX_TELEM_OOBMSM		0x02
+#define CRI_IDX_CRASHLOG_PUNIT		0x03
+#define CRI_IDX_CRASHLOG_OOBMSM		0x04
+
+#define CRI_PUNIT_TELEMETRY_OFFSET	0x0200
+#define CRI_PUNIT_WATCHER_OFFSET	0x00A0
+#define CRI_OOBMSM_0_WATCHER_OFFSET	0x04F8
+#define CRI_OOBMSM_1_TELEMETRY_OFFSET	0x1800
+#define CRI_PUNIT_CRASHLOG_OFFSET	0x0660
 
 enum record_id {
 	PUNIT,
@@ -133,44 +150,81 @@ enum capability {
 	WATCHER,
 };
 
-static int xe_guid_decode(u32 guid, int *index, u32 *offset)
+static int bmg_guid_decode(u32 guid, int *index, u32 *offset)
 {
 	u32 record_id = FIELD_GET(GUID_RECORD_ID, guid);
 	u32 cap_type  = FIELD_GET(GUID_CAP_TYPE, guid);
-	u32 device_id = FIELD_GET(GUID_DEVICE_ID, guid);
 
-	if (device_id != BMG_DEVICE_ID)
-		return -ENODEV;
+	*offset = 0;
 
-	if (cap_type > WATCHER)
+	if (cap_type == CRASHLOG) {
+		*index = record_id == PUNIT ? BMG_IDX_CRASHLOG_PUNIT : BMG_IDX_CRASHLOG_OOBMSM;
+		return 0;
+	}
+
+	switch (record_id) {
+	case PUNIT:
+		*index = BMG_IDX_TELEM_PUNIT;
+		if (cap_type == TELEMETRY)
+			*offset = BMG_PUNIT_TELEMETRY_OFFSET;
+		else
+			*offset = BMG_PUNIT_WATCHER_OFFSET;
+		break;
+
+	case OOBMSM_0:
+		*index = BMG_IDX_TELEM_OOBMSM;
+		if (cap_type == WATCHER)
+			*offset = BMG_OOBMSM_0_WATCHER_OFFSET;
+		break;
+
+	case OOBMSM_1:
+		*index = BMG_IDX_TELEM_OOBMSM;
+		if (cap_type == TELEMETRY)
+			*offset = BMG_OOBMSM_1_TELEMETRY_OFFSET;
+		break;
+	default:
 		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int cri_guid_decode(u32 guid, int *index, u32 *offset)
+{
+	u32 record_id = FIELD_GET(GUID_RECORD_ID, guid);
+	u32 cap_type  = FIELD_GET(GUID_CAP_TYPE, guid);
 
 	*offset = 0;
 
 	if (cap_type == CRASHLOG) {
-		*index = record_id == PUNIT ? 2 : 4;
+		if (record_id == PUNIT) {
+			*index = CRI_IDX_CRASHLOG_PUNIT;
+			*offset = CRI_PUNIT_CRASHLOG_OFFSET;
+		} else {
+			*index = CRI_IDX_CRASHLOG_OOBMSM;
+		}
 		return 0;
 	}
 
 	switch (record_id) {
 	case PUNIT:
-		*index = 0;
+		*index = CRI_IDX_TELEM_PUNIT;
 		if (cap_type == TELEMETRY)
-			*offset = PUNIT_TELEMETRY_OFFSET;
+			*offset = CRI_PUNIT_TELEMETRY_OFFSET;
 		else
-			*offset = PUNIT_WATCHER_OFFSET;
+			*offset = CRI_PUNIT_WATCHER_OFFSET;
 		break;
 
 	case OOBMSM_0:
-		*index = 1;
+		*index = CRI_IDX_TELEM_OOBMSM;
 		if (cap_type == WATCHER)
-			*offset = OOBMSM_0_WATCHER_OFFSET;
+			*offset = CRI_OOBMSM_0_WATCHER_OFFSET;
 		break;
 
 	case OOBMSM_1:
-		*index = 1;
+		*index = CRI_IDX_TELEM_OOBMSM;
 		if (cap_type == TELEMETRY)
-			*offset = OOBMSM_1_TELEMETRY_OFFSET;
+			*offset = CRI_OOBMSM_1_TELEMETRY_OFFSET;
 		break;
 	default:
 		return -EINVAL;
@@ -179,12 +233,30 @@ static int xe_guid_decode(u32 guid, int *index, u32 *offset)
 	return 0;
 }
 
+static int xe_guid_decode(u32 guid, int *index, u32 *offset)
+{
+	u32 cap_type  = FIELD_GET(GUID_CAP_TYPE, guid);
+	u32 device_id = FIELD_GET(GUID_DEVICE_ID, guid);
+
+	if (cap_type > WATCHER)
+		return -EINVAL;
+
+	if (device_id == BMG_DEVICE_ID)
+		return bmg_guid_decode(guid, index, offset);
+
+	if (device_id == CRI_DEVICE_ID)
+		return cri_guid_decode(guid, index, offset);
+
+	return -ENODEV;
+}
+
 int xe_pmt_telem_read(struct pci_dev *pdev, u32 guid, u64 *data, loff_t user_offset,
 		      u32 count)
 {
 	struct xe_device *xe = pdev_to_xe_device(pdev);
-	void __iomem *telem_addr = xe->mmio.regs + BMG_TELEMETRY_OFFSET;
 	u32 cap_type = FIELD_GET(GUID_CAP_TYPE, guid);
+	u32 device_id = FIELD_GET(GUID_DEVICE_ID, guid);
+	void __iomem *telem_addr = xe->mmio.regs;
 	u32 mem_region;
 	u32 offset;
 	int ret;
@@ -193,6 +265,11 @@ int xe_pmt_telem_read(struct pci_dev *pdev, u32 guid, u64 *data, loff_t user_off
 	if (ret)
 		return ret;
 
+	if (device_id == BMG_DEVICE_ID)
+		telem_addr += BMG_TELEMETRY_OFFSET;
+	else
+		telem_addr += CRI_TELEMETRY_OFFSET;
+
 	telem_addr += offset + user_offset;
 
 	guard(mutex)(&xe->pmt.lock);
@@ -217,6 +294,7 @@ int xe_pmt_telem_read(struct pci_dev *pdev, u32 guid, u64 *data, loff_t user_off
 	xe->soc_remapper.set_telem_region(xe, mem_region);
 
 	memcpy_fromio(data, telem_addr, count);
+
 	xe_pm_runtime_put(xe);
 
 	return count;
-- 
2.52.0

Re: [PATCH 4/5] drm/xe/vsec: Crescent Island PMT decode

[Ilpo Järvinen]

On Tue, 27 Jan 2026, Michael J. Ruhl wrote:

> Crescent Island (CRI) has different index and offset values for
> accessing the PMT data area.
> 
> Update the decode path to support the CRI device.
> 
> Update the data read callback so to support the CRI usage.
> 
> Define several magic numbers.
> 
> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_vsec.c | 114 +++++++++++++++++++++++++++++------
>  1 file changed, 96 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
> index 254f7ebca6eb..4bddc22d86c7 100644
> --- a/drivers/gpu/drm/xe/xe_vsec.c
> +++ b/drivers/gpu/drm/xe/xe_vsec.c
> @@ -116,10 +116,27 @@ static struct intel_vsec_platform_info xe_vsec_info[] = {
>  #define GUID_CAP_TYPE		GENMASK(29, 28)
>  #define GUID_RECORD_ID		GENMASK(31, 30)
>  
> -#define PUNIT_TELEMETRY_OFFSET		0x0200
> -#define PUNIT_WATCHER_OFFSET		0x14A0
> -#define OOBMSM_0_WATCHER_OFFSET		0x18D8
> -#define OOBMSM_1_TELEMETRY_OFFSET	0x1000
> +#define BMG_IDX_TELEM_PUNIT		0x00
> +#define BMG_IDX_TELEM_OOBMSM		0x01
> +#define BMG_IDX_CRASHLOG_PUNIT		0x02
> +#define BMG_IDX_CRASHLOG_OOBMSM		0x04
> +
> +#define BMG_PUNIT_TELEMETRY_OFFSET	0x0200
> +#define BMG_PUNIT_WATCHER_OFFSET	0x14A0
> +#define BMG_OOBMSM_0_WATCHER_OFFSET	0x18D8
> +#define BMG_OOBMSM_1_TELEMETRY_OFFSET	0x1000
> +
> +#define CRI_IDX_TELEM_DISCOVERY		0x00
> +#define CRI_IDX_TELEM_PUNIT		0x01
> +#define CRI_IDX_TELEM_OOBMSM		0x02
> +#define CRI_IDX_CRASHLOG_PUNIT		0x03
> +#define CRI_IDX_CRASHLOG_OOBMSM		0x04
> +
> +#define CRI_PUNIT_TELEMETRY_OFFSET	0x0200
> +#define CRI_PUNIT_WATCHER_OFFSET	0x00A0
> +#define CRI_OOBMSM_0_WATCHER_OFFSET	0x04F8
> +#define CRI_OOBMSM_1_TELEMETRY_OFFSET	0x1800
> +#define CRI_PUNIT_CRASHLOG_OFFSET	0x0660
>  
>  enum record_id {
>  	PUNIT,
> @@ -133,44 +150,81 @@ enum capability {
>  	WATCHER,
>  };
>  
> -static int xe_guid_decode(u32 guid, int *index, u32 *offset)
> +static int bmg_guid_decode(u32 guid, int *index, u32 *offset)
>  {
>  	u32 record_id = FIELD_GET(GUID_RECORD_ID, guid);
>  	u32 cap_type  = FIELD_GET(GUID_CAP_TYPE, guid);
> -	u32 device_id = FIELD_GET(GUID_DEVICE_ID, guid);
>  
> -	if (device_id != BMG_DEVICE_ID)
> -		return -ENODEV;
> +	*offset = 0;
>  
> -	if (cap_type > WATCHER)
> +	if (cap_type == CRASHLOG) {
> +		*index = record_id == PUNIT ? BMG_IDX_CRASHLOG_PUNIT : BMG_IDX_CRASHLOG_OOBMSM;
> +		return 0;
> +	}
> +
> +	switch (record_id) {
> +	case PUNIT:
> +		*index = BMG_IDX_TELEM_PUNIT;
> +		if (cap_type == TELEMETRY)
> +			*offset = BMG_PUNIT_TELEMETRY_OFFSET;
> +		else
> +			*offset = BMG_PUNIT_WATCHER_OFFSET;
> +		break;
> +
> +	case OOBMSM_0:
> +		*index = BMG_IDX_TELEM_OOBMSM;
> +		if (cap_type == WATCHER)
> +			*offset = BMG_OOBMSM_0_WATCHER_OFFSET;
> +		break;
> +
> +	case OOBMSM_1:
> +		*index = BMG_IDX_TELEM_OOBMSM;
> +		if (cap_type == TELEMETRY)
> +			*offset = BMG_OOBMSM_1_TELEMETRY_OFFSET;
> +		break;
> +	default:
>  		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int cri_guid_decode(u32 guid, int *index, u32 *offset)
> +{
> +	u32 record_id = FIELD_GET(GUID_RECORD_ID, guid);
> +	u32 cap_type  = FIELD_GET(GUID_CAP_TYPE, guid);
>  
>  	*offset = 0;
>  
>  	if (cap_type == CRASHLOG) {
> -		*index = record_id == PUNIT ? 2 : 4;
> +		if (record_id == PUNIT) {
> +			*index = CRI_IDX_CRASHLOG_PUNIT;
> +			*offset = CRI_PUNIT_CRASHLOG_OFFSET;
> +		} else {
> +			*index = CRI_IDX_CRASHLOG_OOBMSM;
> +		}
>  		return 0;
>  	}
>  
>  	switch (record_id) {
>  	case PUNIT:
> -		*index = 0;
> +		*index = CRI_IDX_TELEM_PUNIT;
>  		if (cap_type == TELEMETRY)
> -			*offset = PUNIT_TELEMETRY_OFFSET;
> +			*offset = CRI_PUNIT_TELEMETRY_OFFSET;
>  		else
> -			*offset = PUNIT_WATCHER_OFFSET;
> +			*offset = CRI_PUNIT_WATCHER_OFFSET;
>  		break;
>  
>  	case OOBMSM_0:
> -		*index = 1;
> +		*index = CRI_IDX_TELEM_OOBMSM;
>  		if (cap_type == WATCHER)
> -			*offset = OOBMSM_0_WATCHER_OFFSET;
> +			*offset = CRI_OOBMSM_0_WATCHER_OFFSET;
>  		break;
>  
>  	case OOBMSM_1:
> -		*index = 1;
> +		*index = CRI_IDX_TELEM_OOBMSM;
>  		if (cap_type == TELEMETRY)
> -			*offset = OOBMSM_1_TELEMETRY_OFFSET;
> +			*offset = CRI_OOBMSM_1_TELEMETRY_OFFSET;
>  		break;
>  	default:
>  		return -EINVAL;
> @@ -179,12 +233,30 @@ static int xe_guid_decode(u32 guid, int *index, u32 *offset)
>  	return 0;
>  }
>  
> +static int xe_guid_decode(u32 guid, int *index, u32 *offset)
> +{
> +	u32 cap_type  = FIELD_GET(GUID_CAP_TYPE, guid);
> +	u32 device_id = FIELD_GET(GUID_DEVICE_ID, guid);
> +
> +	if (cap_type > WATCHER)
> +		return -EINVAL;
> +
> +	if (device_id == BMG_DEVICE_ID)
> +		return bmg_guid_decode(guid, index, offset);
> +
> +	if (device_id == CRI_DEVICE_ID)
> +		return cri_guid_decode(guid, index, offset);
> +
> +	return -ENODEV;
> +}
> +
>  int xe_pmt_telem_read(struct pci_dev *pdev, u32 guid, u64 *data, loff_t user_offset,
>  		      u32 count)
>  {
>  	struct xe_device *xe = pdev_to_xe_device(pdev);
> -	void __iomem *telem_addr = xe->mmio.regs + BMG_TELEMETRY_OFFSET;
>  	u32 cap_type = FIELD_GET(GUID_CAP_TYPE, guid);
> +	u32 device_id = FIELD_GET(GUID_DEVICE_ID, guid);
> +	void __iomem *telem_addr = xe->mmio.regs;
>  	u32 mem_region;
>  	u32 offset;
>  	int ret;
> @@ -193,6 +265,11 @@ int xe_pmt_telem_read(struct pci_dev *pdev, u32 guid, u64 *data, loff_t user_off
>  	if (ret)
>  		return ret;
>  
> +	if (device_id == BMG_DEVICE_ID)
> +		telem_addr += BMG_TELEMETRY_OFFSET;
> +	else
> +		telem_addr += CRI_TELEMETRY_OFFSET;
> +
>  	telem_addr += offset + user_offset;
>  
>  	guard(mutex)(&xe->pmt.lock);
> @@ -217,6 +294,7 @@ int xe_pmt_telem_read(struct pci_dev *pdev, u32 guid, u64 *data, loff_t user_off
>  	xe->soc_remapper.set_telem_region(xe, mem_region);
>  
>  	memcpy_fromio(data, telem_addr, count);
> +

A stray change.

>  	xe_pm_runtime_put(xe);
>  
>  	return count;
> 

-- 
 i.

[PATCH 5/5] drm/xe/vsec: Crescent Island PMT callbacks

[Michael J. Ruhl]

CRI PMT support requires callbacks to access the discovery status
and control areas.  Access is a common MMIO area that requires an
index to be set before access is allowed.

Introduce the necessary callbacks to get the status and control
information for CRI PMT usage.

Add the glue logic to register the CRI PMT functionality.

Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
---
 drivers/gpu/drm/xe/xe_vsec.c | 84 ++++++++++++++++++++++++++++++++++--
 1 file changed, 81 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
index 4bddc22d86c7..c5b0c16f23be 100644
--- a/drivers/gpu/drm/xe/xe_vsec.c
+++ b/drivers/gpu/drm/xe/xe_vsec.c
@@ -300,17 +300,89 @@ int xe_pmt_telem_read(struct pci_dev *pdev, u32 guid, u64 *data, loff_t user_off
 	return count;
 }
 
-static struct pmt_callbacks xe_pmt_cb = {
+/**
+ * xe_pmt_read_reg() - read a crashlog register
+ * @pdev: the pcie device that register the callback
+ * @guid: PMT guid of the crashlog instance
+ * @reg: data read from the PMT data structure
+ * @offset: which data to read from the PMT data structure
+ *
+ * Read the requested PMT register based on the pcie device and guid.  The
+ * supported struct is the Crashlog Type1 Version2.
+ *
+ * Currently this is for CRI only.
+ *
+ */
+static int xe_pmt_read_reg(struct pci_dev *pdev, u32 guid, u32 *reg, u32 offset)
+{
+	struct xe_device *xe = pdev_to_xe_device(pdev);
+	void __iomem *disc_addr = xe->mmio.regs;
+	u32 inst;
+
+	if (FIELD_GET(GUID_DEVICE_ID, guid) != CRI_DEVICE_ID ||
+	    FIELD_GET(GUID_CAP_TYPE, guid) != CRASHLOG)
+		return -EINVAL;
+
+	inst = FIELD_GET(GUID_RECORD_ID, guid) == PUNIT ? PUNIT_DISC_OFFSET : OOBMSM_DISC_OFFSET;
+	disc_addr += CRI_DISCOVERY_OFFSET + inst + offset;
+
+	guard(mutex)(&xe->pmt.lock);
+
+	xe_pm_runtime_get(xe);
+
+	xe->soc_remapper.set_telem_region(xe, CRI_IDX_TELEM_DISCOVERY);
+
+	memcpy_fromio(reg, disc_addr, sizeof(*reg));
+
+	xe_pm_runtime_put(xe);
+
+	return 0;
+}
+
+static int xe_pmt_write_reg(struct pci_dev *pdev, u32 guid, u32 reg, u32 offset)
+{
+	struct xe_device *xe = pdev_to_xe_device(pdev);
+	void __iomem *disc_addr = xe->mmio.regs;
+	u32 inst;
+
+	if (FIELD_GET(GUID_DEVICE_ID, guid) != CRI_DEVICE_ID ||
+	    FIELD_GET(GUID_CAP_TYPE, guid) != CRASHLOG)
+		return -EINVAL;
+
+	inst = FIELD_GET(GUID_RECORD_ID, guid) == PUNIT ? PUNIT_DISC_OFFSET : OOBMSM_DISC_OFFSET;
+	disc_addr += CRI_DISCOVERY_OFFSET + inst + offset;
+
+	guard(mutex)(&xe->pmt.lock);
+
+	xe_pm_runtime_get(xe);
+
+	xe->soc_remapper.set_telem_region(xe, CRI_IDX_TELEM_DISCOVERY);
+
+	memcpy_toio(disc_addr, &reg, sizeof(reg));
+
+	xe_pm_runtime_put(xe);
+
+	return 0;
+}
+
+static struct pmt_callbacks xe_bmg_pmt_cb = {
+	.read_telem = xe_pmt_telem_read,
+};
+
+static struct pmt_callbacks xe_cri_pmt_cb = {
 	.read_telem = xe_pmt_telem_read,
+	.read_reg = xe_pmt_read_reg,
+	.write_reg = xe_pmt_write_reg,
 };
 
 static const int vsec_platforms[] = {
 	[XE_BATTLEMAGE] = XE_VSEC_BMG,
+	[XE_CRESCENTISLAND] = XE_VSEC_CRI,
 };
 
 static enum xe_vsec get_platform_info(struct xe_device *xe)
 {
-	if (xe->info.platform > XE_BATTLEMAGE)
+	if (xe->info.platform > XE_CRESCENTISLAND)
 		return XE_VSEC_UNKNOWN;
 
 	return vsec_platforms[xe->info.platform];
@@ -338,8 +410,14 @@ void xe_vsec_init(struct xe_device *xe)
 
 	switch (platform) {
 	case XE_VSEC_BMG:
-		info->priv_data = &xe_pmt_cb;
+		info->priv_data = &xe_bmg_pmt_cb;
 		break;
+
+	case XE_VSEC_CRI:
+		info->priv_data = &xe_cri_pmt_cb;
+		xe->soc_remapper.set_telem_region(xe, CRI_IDX_TELEM_DISCOVERY);
+		break;
+
 	default:
 		break;
 	}
-- 
2.52.0

Re: [PATCH 5/5] drm/xe/vsec: Crescent Island PMT callbacks

[Ilpo Järvinen]

On Tue, 27 Jan 2026, Michael J. Ruhl wrote:

> CRI PMT support requires callbacks to access the discovery status
> and control areas.  Access is a common MMIO area that requires an
> index to be set before access is allowed.
> 
> Introduce the necessary callbacks to get the status and control
> information for CRI PMT usage.
> 
> Add the glue logic to register the CRI PMT functionality.
> 
> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_vsec.c | 84 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 81 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
> index 4bddc22d86c7..c5b0c16f23be 100644
> --- a/drivers/gpu/drm/xe/xe_vsec.c
> +++ b/drivers/gpu/drm/xe/xe_vsec.c
> @@ -300,17 +300,89 @@ int xe_pmt_telem_read(struct pci_dev *pdev, u32 guid, u64 *data, loff_t user_off
>  	return count;
>  }
>  
> -static struct pmt_callbacks xe_pmt_cb = {
> +/**
> + * xe_pmt_read_reg() - read a crashlog register
> + * @pdev: the pcie device that register the callback
> + * @guid: PMT guid of the crashlog instance
> + * @reg: data read from the PMT data structure
> + * @offset: which data to read from the PMT data structure
> + *
> + * Read the requested PMT register based on the pcie device and guid.  The
> + * supported struct is the Crashlog Type1 Version2.
> + *
> + * Currently this is for CRI only.
> + *

An extra empty line.

-- 
 i.

> + */
> +static int xe_pmt_read_reg(struct pci_dev *pdev, u32 guid, u32 *reg, u32 offset)
> +{
> +	struct xe_device *xe = pdev_to_xe_device(pdev);
> +	void __iomem *disc_addr = xe->mmio.regs;
> +	u32 inst;
> +
> +	if (FIELD_GET(GUID_DEVICE_ID, guid) != CRI_DEVICE_ID ||
> +	    FIELD_GET(GUID_CAP_TYPE, guid) != CRASHLOG)
> +		return -EINVAL;
> +
> +	inst = FIELD_GET(GUID_RECORD_ID, guid) == PUNIT ? PUNIT_DISC_OFFSET : OOBMSM_DISC_OFFSET;
> +	disc_addr += CRI_DISCOVERY_OFFSET + inst + offset;
> +
> +	guard(mutex)(&xe->pmt.lock);
> +
> +	xe_pm_runtime_get(xe);
> +
> +	xe->soc_remapper.set_telem_region(xe, CRI_IDX_TELEM_DISCOVERY);
> +
> +	memcpy_fromio(reg, disc_addr, sizeof(*reg));
> +
> +	xe_pm_runtime_put(xe);
> +
> +	return 0;
> +}
> +
> +static int xe_pmt_write_reg(struct pci_dev *pdev, u32 guid, u32 reg, u32 offset)
> +{
> +	struct xe_device *xe = pdev_to_xe_device(pdev);
> +	void __iomem *disc_addr = xe->mmio.regs;
> +	u32 inst;
> +
> +	if (FIELD_GET(GUID_DEVICE_ID, guid) != CRI_DEVICE_ID ||
> +	    FIELD_GET(GUID_CAP_TYPE, guid) != CRASHLOG)
> +		return -EINVAL;
> +
> +	inst = FIELD_GET(GUID_RECORD_ID, guid) == PUNIT ? PUNIT_DISC_OFFSET : OOBMSM_DISC_OFFSET;
> +	disc_addr += CRI_DISCOVERY_OFFSET + inst + offset;
> +
> +	guard(mutex)(&xe->pmt.lock);
> +
> +	xe_pm_runtime_get(xe);
> +
> +	xe->soc_remapper.set_telem_region(xe, CRI_IDX_TELEM_DISCOVERY);
> +
> +	memcpy_toio(disc_addr, &reg, sizeof(reg));
> +
> +	xe_pm_runtime_put(xe);
> +
> +	return 0;
> +}
> +
> +static struct pmt_callbacks xe_bmg_pmt_cb = {
> +	.read_telem = xe_pmt_telem_read,
> +};
> +
> +static struct pmt_callbacks xe_cri_pmt_cb = {
>  	.read_telem = xe_pmt_telem_read,
> +	.read_reg = xe_pmt_read_reg,
> +	.write_reg = xe_pmt_write_reg,
>  };
>  
>  static const int vsec_platforms[] = {
>  	[XE_BATTLEMAGE] = XE_VSEC_BMG,
> +	[XE_CRESCENTISLAND] = XE_VSEC_CRI,
>  };
>  
>  static enum xe_vsec get_platform_info(struct xe_device *xe)
>  {
> -	if (xe->info.platform > XE_BATTLEMAGE)
> +	if (xe->info.platform > XE_CRESCENTISLAND)
>  		return XE_VSEC_UNKNOWN;
>  
>  	return vsec_platforms[xe->info.platform];
> @@ -338,8 +410,14 @@ void xe_vsec_init(struct xe_device *xe)
>  
>  	switch (platform) {
>  	case XE_VSEC_BMG:
> -		info->priv_data = &xe_pmt_cb;
> +		info->priv_data = &xe_bmg_pmt_cb;
>  		break;
> +
> +	case XE_VSEC_CRI:
> +		info->priv_data = &xe_cri_pmt_cb;
> +		xe->soc_remapper.set_telem_region(xe, CRI_IDX_TELEM_DISCOVERY);
> +		break;
> +
>  	default:
>  		break;
>  	}
>