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

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

On 26-08-2026 23:49, Raag Jadav wrote:
> On Tue, Aug 25, 2026 at 12:11:32AM +0530, Karthik Poosa wrote:
>> 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.
>>
>> Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_hwmon.c     | 35 ++++++++++++++++++++++++++-----
>>   drivers/gpu/drm/xe/xe_pcode_api.h |  1 +
>>   2 files changed, 31 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
>> index 2c4eba4b8f8f..6e7cb250e628 100644
>> --- a/drivers/gpu/drm/xe/xe_hwmon.c
>> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
>> @@ -39,7 +39,8 @@ enum xe_hwmon_reg_operation {
>>   	REG_READ64,
>>   };
>>   
>> -#define MAX_VRAM_CHANNELS      (16)
>> +/* Maximum number of VRAM channels supported by Xe */
>> +#define MAX_VRAM_CHANNELS      (80)
> Why?

We are using this in enum xe_hwmon_channel.

VRAM channels supported count has increased from BMG to CRI.

>
>>   enum xe_hwmon_channel {
>>   	CHANNEL_CARD,
>> @@ -48,6 +49,7 @@ enum xe_hwmon_channel {
>>   	CHANNEL_MCTRL,
>>   	CHANNEL_PCIE,
>>   	CHANNEL_VRAM_N,
>> +	/* Compile-time upper bound; actual channel count is hwmon->temp.vram_count */
>>   	CHANNEL_VRAM_N_MAX = CHANNEL_VRAM_N + MAX_VRAM_CHANNELS - 1,
>>   	CHANNEL_MAX,
>>   };
>> @@ -144,10 +146,12 @@ struct xe_hwmon_thermal_info {
>>   	};
>>   	/** @count: no of temperature sensors available for the platform */
> So now this can be "total number of temperature sensors"?

actually this is the count got from READ_THERMAL_CONFIG commands, it is 
excluding the VRAM individual temperature sensors count

I shall rephrase this to

/** @count: temperature sensors count from READ_THERMAL_CONFIG */

>
>>   	u8 count;
>> +	/** @vram_count: number of VRAM temperature sensors available for the platform */
>> +	u8 vram_count;
>>   	/** @value: signed value from each sensor */
>>   	s8 value[U8_MAX];
>> -	/** @vram_label: vram label names */
>> -	char vram_label[MAX_VRAM_CHANNELS][MAX_LABEL_SIZE];
>> +	/** @vram_label: vram label names, dynamically allocated based on vram_count */
>> +	char (*vram_label)[MAX_LABEL_SIZE];
>>   };
>>   
>>   /**
>> @@ -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)
>> @@ -810,6 +814,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) {
> Same as last patch. Don't solve problems that don't exist.
Platform from CRI would have this, so this would be needed.
>
>> +		hwmon->temp.vram_count = REG_FIELD_GET(VRAM_COUNT_MASK, config);
>> +		if (hwmon->temp.vram_count > MAX_VRAM_CHANNELS && hwmon->temp.vram_count) {
> Isn't the first condition sufficient? What am I missing?
First one is enough
>
>> +			drm_warn(&hwmon->xe->drm, "VRAM channel count %d exceeds max %d, clamping\n",
> Can this be invalid? i.e. 0xff? And should we clamp it in that case?

as per mailbox spec no there is no invalid value for this.

this expected to give the count. 0 itself will be invalid

>
>> +				 hwmon->temp.vram_count, MAX_VRAM_CHANNELS);
>> +			hwmon->temp.vram_count = MAX_VRAM_CHANNELS;
> So perhaps CRI_MAX_VRAM_CHANNELS?
I'd like use existing MAX_VRAM_CHANNELS renamed as XE_MAX_VRAM_CHANNELS
>
>> +		}
>> +	} else {
>> +		hwmon->temp.vram_count = 16; /* For older platforms, max is 16 VRAM channels */
> BMG_MAX_VRAM_CHANNELS?
yes, I'll add it BMG_MAX_VRAM_CHANNELS
>
>> +	}
>> +
>>   	return ret;
>>   }
>>   
>> @@ -988,6 +1003,9 @@ static inline bool is_vram_ch_available(struct xe_hwmon *hwmon, int channel)
>>   	u32 reg_val;
>>   	u8 temp;
>>   
>> +	if (vram_id >= hwmon->temp.vram_count)
>> +		return false;
>> +
>>   	vram_reg = xe_hwmon_get_reg(hwmon, REG_TEMP, channel);
>>   	if (!xe_reg_is_valid(vram_reg))
>>   		return false;
>> @@ -1516,7 +1534,7 @@ static int xe_hwmon_read_label(struct device *dev,
>>   			*str = "mctrl";
>>   		else if (channel == CHANNEL_PCIE)
>>   			*str = "pcie";
>> -		else if (in_range(channel, CHANNEL_VRAM_N, MAX_VRAM_CHANNELS))
>> +		else if (in_range(channel, CHANNEL_VRAM_N, hwmon->temp.vram_count))
>>   			*str = hwmon->temp.vram_label[channel - CHANNEL_VRAM_N];
>>   		return 0;
>>   	case hwmon_power:
>> @@ -1645,6 +1663,13 @@ int xe_hwmon_register(struct xe_device *xe)
>>   
>>   	xe_hwmon_get_preregistration_info(hwmon);
>>   
>> +	hwmon->temp.vram_label = devm_kcalloc(dev, hwmon->temp.vram_count,
> What if vram_count is 0?
>
> Raag
we shouldn't alloc this labels, I'll add this check.
>
>> +					      MAX_LABEL_SIZE, GFP_KERNEL);
>> +	if (!hwmon->temp.vram_label) {
>> +		xe->hwmon = NULL;
>> +		return -ENOMEM;
>> +	}
>> +
>>   	drm_dbg(&xe->drm, "Register xe hwmon interface\n");
>>   
>>   	/*  hwmon_dev points to device hwmon<i> */
>> diff --git a/drivers/gpu/drm/xe/xe_pcode_api.h b/drivers/gpu/drm/xe/xe_pcode_api.h
>> index 94575c476e3d..e1079eff72c6 100644
>> --- a/drivers/gpu/drm/xe/xe_pcode_api.h
>> +++ b/drivers/gpu/drm/xe/xe_pcode_api.h
>> @@ -57,6 +57,7 @@
>>   #define   PCODE_THERMAL_INFO			0x25
>>   #define     READ_THERMAL_LIMITS			0x0
>>   #define     READ_THERMAL_CONFIG			0x1
>> +#define       VRAM_COUNT_MASK			REG_GENMASK(15, 8)
>>   #define     READ_THERMAL_DATA			0x2
>>   #define       PCIE_SENSOR_GROUP_ID		0x2
>>   #define       PCIE_SENSOR_MASK			REG_GENMASK(31, 16)
>> -- 
>> 2.25.1
>>