Re: [PATCH v2 2/5] drm/xe/hwmon: Use VRAM temperature sensor count from thermal config on CRI

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

On 02-09-2026 23:40, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] VRAM temperature sensors will not be exposed on Crescent Island (CRI) platforms because `xe_hwmon_get_reg()` lacks `REG_TEMP` support for `XE_CRESCENTISLAND`.
> - [Medium] On Battlemage (BMG) platforms, all VRAM temperature sensors will be silently hidden if the thermal PCODE mailbox initialization fails.
> --
>
> commit 426a936975a4587d5003554dc21e461abf340940
> Author: Karthik Poosa <karthik.poosa@intel.com>
>
> drm/xe/hwmon: Use VRAM temperature sensor count from thermal config on CRI
>      
> Read the number of VRAM temperature sensor channels from the second byte
> of READ_THERMAL_CONFIG on CRI platforms. Use the reported count to avoid
> exposing hwmon attributes for unavailable VRAM temperature sensors, while
> retaining the maximum supported channel count on non-CRI platforms.
>
>> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
>> index 768f6d68b2916..51a6664eec485 100644
>> --- a/drivers/gpu/drm/xe/xe_hwmon.c
>> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> [ ... ]
>> @@ -271,7 +275,7 @@ static struct xe_reg xe_hwmon_get_reg(struct xe_hwmon *hwmon, enum xe_hwmon_reg
>>   				return BMG_PACKAGE_TEMPERATURE;
>>   			else if (channel == CHANNEL_VRAM)
>>   				return BMG_VRAM_TEMPERATURE;
>> -			else if (in_range(channel, CHANNEL_VRAM_N, MAX_VRAM_CHANNELS))
>> +			else if (in_range(channel, CHANNEL_VRAM_N, hwmon->temp.vram_count))
>>   				return BMG_VRAM_TEMPERATURE_N(channel - CHANNEL_VRAM_N);
>>   		} else if (xe->info.platform == XE_DG2) {
>>   			if (channel == CHANNEL_PKG)
> [Severity: Medium]
> Will VRAM temperature sensors be exposed on XE_CRESCENTISLAND platforms?
>
> When checking sysfs attribute visibility, is_temp_available() queries
> xe_hwmon_get_reg() for REG_TEMP. Since xe_hwmon_get_reg() only has branches
> for XE_BATTLEMAGE and XE_DG2, it returns an invalid register (XE_REG(0)) for
> XE_CRESCENTISLAND platforms.
>
> This causes is_temp_available() to unconditionally return false. Does this
> code need a branch for XE_CRESCENTISLAND to return the correct VRAM
> temperature registers?

which repo are you checking Sashiko, I see XE_CRESCENTISLAND check is there.

I have also mentioned the same this in comment of previous patch also

See:

https://gitlab.freedesktop.org/drm/tip/-/blob/drm-tip/drivers/gpu/drm/xe/xe_hwmon.c?ref_type=heads#L269

static struct xe_reg xe_hwmon_get_reg(struct xe_hwmon *hwmon, enum 
xe_hwmon_reg hwmon_reg,
                       int channel)
{
     struct xe_device *xe = hwmon->xe;

     switch (hwmon_reg) {
     case REG_TEMP:
         if (xe->info.platform == XE_BATTLEMAGE || xe->info.platform == 
XE_CRESCENTISLAND) {
             if (channel == CHANNEL_PKG)
                 return BMG_PACKAGE_TEMPERATURE;
             else if (channel == CHANNEL_VRAM)
                 return BMG_VRAM_TEMPERATURE;
             else if (in_range(channel, CHANNEL_VRAM_N, MAX_VRAM_CHANNELS))
>
> [ ... ]
>> @@ -786,6 +854,17 @@ static int xe_hwmon_pcode_read_thermal_info(struct xe_hwmon *hwmon)
>>   	drm_dbg(&hwmon->xe->drm, "thermal config count 0x%x\n", config);
>>   	hwmon->temp.count = REG_FIELD_GET(TEMP_MASK, config);
>>   
>> +	if (hwmon->xe->info.platform >= XE_CRESCENTISLAND) {
>> +		hwmon->temp.vram_count = REG_FIELD_GET(VRAM_COUNT_MASK, config);
>> +		if (hwmon->temp.vram_count > XE_MAX_VRAM_CHANNELS) {
>> +			drm_warn(&hwmon->xe->drm, "VRAM channel count %d exceeds max %d, clamping\n",
>> +				 hwmon->temp.vram_count, XE_MAX_VRAM_CHANNELS);
>> +			hwmon->temp.vram_count = XE_MAX_VRAM_CHANNELS;
>> +		}
>> +	} else {
>> +		hwmon->temp.vram_count = BMG_MAX_VRAM_CHANNELS;
>> +	}
>> +
>>   	return ret;
>>   }
> [Severity: Medium]
> Will this silently hide all VRAM temperature sensors on Battlemage (BMG)
> platforms if the thermal PCODE mailbox initialization fails?
>
> If xe_hwmon_pcode_read_thermal_info() encounters a failure (such as an error
> reading READ_THERMAL_LIMITS or READ_THERMAL_CONFIG earlier in the function),
> it returns early. This skips the fallback assignment of hwmon->temp.vram_count
> at the end of the function.
>
> Because hwmon->temp.vram_count is 0-initialized, the in_range() check will
> subsequently evaluate to false in xe_hwmon_get_reg(), returning an invalid
> register and hiding the sensors. Previously, this path fell back to a
> hardcoded MAX_VRAM_CHANNELS value, preserving visibility.
Yes we can assign the BMG default up front, before the PCODE reads, so 
an early return still preserves sensor visibility.