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

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

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.