Re: [PATCH v13 12/16] drm/xe/hwmon: Update memory controller temperature offset for CRI

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

On 14-10-2026 23:22, Rodrigo Vivi wrote:
> On Wed, Oct 14, 2026 at 05:28:02PM +0530, Karthik Poosa wrote:
>> On CRI the memory controller temperatures moved in the READ_THERMAL_DATA
>> output: they now occupy bytes 16..19 instead of starting at byte 2.
>>
>> Read and average the CRI memory controller sensors from bytes 16..19,
>> keeping the existing byte 2..count-1 range for earlier platforms. Only
>> the group(s) covering the used bytes are fetched.
>>
>> Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>
>> ---
>>
>> v2: Use the CRI memory temperature mapping only on
>>      Crescent Island. (sashiko)
>>
>>   drivers/gpu/drm/xe/xe_hwmon.c | 20 ++++++++++++++++----
>>   1 file changed, 16 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
>> index dad2397db141..eef6560569ce 100644
>> --- a/drivers/gpu/drm/xe/xe_hwmon.c
>> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
>> @@ -109,6 +109,10 @@ enum sensor_attr_power {
>>   /* Index of memory controller in READ_THERMAL_DATA output */
>>   #define TEMP_INDEX_MCTRL	2
>>   
>> +/* On CRI the memory controller temperatures are READ_THERMAL_DATA bytes 16..19. */
>> +#define CRI_TEMP_INDEX_MCTRL	16
>> +#define CRI_TEMP_MCTRL_COUNT	4
>> +
>>   /* Mailbox sentinel reported for an unavailable temperature sensor */
>>   #define TEMP_MBX_INVALID	0xFF
>>   
>> @@ -986,14 +990,22 @@ static int mbx_temp_decode(u8 value)
>>   static int get_mc_temp(struct xe_hwmon *hwmon, long *val)
>>   {
>>   	struct xe_tile *root_tile = xe_device_get_root_tile(hwmon->xe);
>> -	int ret, i, count = 0;
>> +	int ret, i, start, end, count = 0;
>>   	s32 average = 0;
>> +	u32 data = 0;
>>   
>>   	guard(mutex)(&hwmon->hwmon_lock);
>>   
>> -	for (i = 0; i < DIV_ROUND_UP(hwmon->temp.count, sizeof(u32)); i++) {
>> -		u32 data = 0;
>> +	/* CRI relocated the memory controller temperatures to READ_THERMAL_DATA bytes 16..19. */
>> +	if (hwmon->xe->info.platform == XE_CRESCENTISLAND) {
> we need to be consistent... we either use >= cri everywhere or we use ==...

I used == explicitly here, as sensor mapping can change based on the 
platform configuration.

where as MMIO definitions and mailbox definitions are expected to be 
same in future platforms.

if those can change, I shall make all places == XE_CRESCENTISLAND

>
>> +		start = CRI_TEMP_INDEX_MCTRL;
>> +		end = CRI_TEMP_INDEX_MCTRL + CRI_TEMP_MCTRL_COUNT;
>> +	} else {
>> +		start = TEMP_INDEX_MCTRL;
>> +		end = hwmon->temp.count - 1;
>> +	}
>>   
>> +	for (i = start / sizeof(u32); i < DIV_ROUND_UP(end, sizeof(u32)); i++) {
>>   		ret = xe_pcode_read(root_tile, PCODE_MBOX(PCODE_THERMAL_INFO, READ_THERMAL_DATA, i),
>>   				    &data, NULL);
>>   		if (ret)
>> @@ -1002,7 +1014,7 @@ static int get_mc_temp(struct xe_hwmon *hwmon, long *val)
>>   		hwmon->temp.dword[i] = cpu_to_le32(data);
>>   	}
>>   
>> -	for (i = TEMP_INDEX_MCTRL; i < hwmon->temp.count - 1; i++) {
>> +	for (i = start; i < end; i++) {
>>   		if (!mbx_temp_valid(hwmon, hwmon->temp.value[i]))
>>   			continue;
>>   		average += mbx_temp_decode(hwmon->temp.value[i]);
>> -- 
>> 2.25.1
>>