Re: [PATCH i-g-t v4 10/13] drm/xe/hwmon: use CRI-specific package and VRAM temperature registers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] xe_hwmon: VRAM temperature register offset collision on Crescent Island
--

commit fd97330baada16c38f4cebd065bec94e97f517b7
Author: Karthik Poosa <karthik.poosa@intel.com>

drm/xe/hwmon: use CRI-specific package and VRAM temperature registers

Crescent Island uses different MMIO offsets for package and VRAM temperature
than Battlemage. Add CRI-specific register definitions and select them in
xe_hwmon_get_reg() for REG_TEMP handling.

> diff --git a/drivers/gpu/drm/xe/regs/xe_pcode_regs.h b/drivers/gpu/drm/xe/regs/xe_pcode_regs.h
> index 11916b4f7cfc0..a6f13d7c39894 100644
> --- a/drivers/gpu/drm/xe/regs/xe_pcode_regs.h
> +++ b/drivers/gpu/drm/xe/regs/xe_pcode_regs.h
[ ... ]
>  #define CRI_PACKAGE_ENERGY_STATUS		XE_REG(0x138120)
> -#define CRI_PLATFORM_ENERGY_STATUS		XE_REG(0x138458)
>  #define CRI_MSU_VRAM_ENABLE             XE_REG(0x138340)
> +#define CRI_PACKAGE_TEMPERATURE			XE_REG(0x138344)
> +#define CRI_VRAM_TEMPERATURE			XE_REG(0x138348)
> +#define CRI_PLATFORM_ENERGY_STATUS		XE_REG(0x138458)
>  
>  #endif /* _XE_PCODE_REGS_H_ */

> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> index 1b587bd9487e1..e1cff5700e7f9 100644
> --- a/drivers/gpu/drm/xe/xe_hwmon.c
> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> @@ -293,7 +293,15 @@ static struct xe_reg xe_hwmon_get_reg(struct xe_hwmon *hwmon, enum xe_hwmon_reg
>  
>  	switch (hwmon_reg) {
>  	case REG_TEMP:
> -		if (xe->info.platform == XE_BATTLEMAGE || xe->info.platform == XE_CRESCENTISLAND) {
> +		if (xe->info.platform == XE_CRESCENTISLAND) {
> +			if (channel == CHANNEL_PKG)
> +				return CRI_PACKAGE_TEMPERATURE;
> +			else if (channel == CHANNEL_VRAM)
> +				return CRI_VRAM_TEMPERATURE;
> +			else if (in_range(channel, CHANNEL_VRAM_N, hwmon->temp.vram_count) &&
> +				 xe_hwmon_vram_channel_enabled(hwmon, channel - CHANNEL_VRAM_N))
> +				return BMG_VRAM_TEMPERATURE_N(channel - CHANNEL_VRAM_N);

[Severity: High]
Will using the BMG_VRAM_TEMPERATURE_N() macro for Crescent Island cause
register offset collisions for higher VRAM channels?

The BMG_VRAM_TEMPERATURE_N(n) macro computes offsets as 0x138260 + (n * 4).
When n >= 56, the resulting offsets overlap with the newly added CRI
registers:

- n = 56 gives 0x138340 (collides with CRI_MSU_VRAM_ENABLE)
- n = 57 gives 0x138344 (collides with CRI_PACKAGE_TEMPERATURE)
- n = 58 gives 0x138348 (collides with CRI_VRAM_TEMPERATURE)

Since Crescent Island allows up to 80 VRAM channels, reading the sysfs
hwmon tempX_input attributes for channels 56 and above would return
data from these unrelated registers instead of VRAM temperature telemetry.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924105000.2627929-1-karthik.poosa@intel.com?part=10