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

["Poosa, Karthik" <karthik.poosa@intel.com>]

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


On 24-09-2026 16:57, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] drm/xe/hwmon: incorrect BMG register used for CRI per-VRAM channel temperatures
> --
>
> commit 8d77f2708804f58e62723e17d986cd5eb1877de7
> Author: Karthik Poosa<karthik.poosa@intel.com>
>
> drm/xe/hwmon: use CRI-specific package and VRAM temperature registers
>
> This commit updates the hardware monitoring register accesses to use
> Crescent Island (CRI) specific MMIO offsets for package and VRAM
> temperatures, as they differ from Battlemage offsets.
>
>> 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: Medium]
> Does Crescent Island actually use the Battlemage register offset for
> per-channel VRAM temperatures?
>
> If a monitoring daemon reads a sysfs temperature attribute corresponding
> to a specific VRAM channel (e.g., temp3_input mapping to CHANNEL_VRAM_N)
> on a Crescent Island device, xe_hwmon_get_reg() will return the
> BMG_VRAM_TEMPERATURE_N macro in the XE_CRESCENTISLAND branch.
>
> The commit message explicitly notes that Crescent Island uses different MMIO
> offsets for package and VRAM temperatures. While the aggregate CHANNEL_PKG
> and CHANNEL_VRAM cases were updated to use the new CRI_ macros, the
> CHANNEL_VRAM_N fallback still uses the BMG_VRAM_TEMPERATURE_N macro.
>
> Will this cause hardware monitoring tools to read incorrect temperature data
> for individual VRAM channels on Crescent Island since it accesses the
> Battlemage MMIO offset?
>
>
CRI VRAM individual temperature offset is different, I shall at 
CRI_VRAM_TEMPERATURE_N for it.

[-- Attachment #2: Type: text/html, Size: 3249 bytes --]