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

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Thu, Oct 15, 2026 at 01:03:36AM +0530, Poosa, Karthik wrote:
> 
> 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.

I'm afraid we cannot assure that. I just want us to be consistent,
that's all.

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

probably better.
with that:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

and keep anywhere that is modified

> 
> > 
> > > +		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
> > >