Re: [PATCH v3 1/4] drm/xe/hwmon: Expose temperature limits

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

On 17-12-2025 22:35, Raag Jadav wrote:
> On Tue, Dec 16, 2025 at 05:10:27PM +0530, Karthik Poosa wrote:
>> Read temperature limits using pcode mailbox and
>> expose shutdown temperature limit as tempX_emergency,
>> critical temperature limit of as tempX_crit and
>> GPU average temperature limit as tempX_max.
>>
>> Update Xe hwmon documentation for these entries.
>>
>> Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>
>> ---
>>   .../ABI/testing/sysfs-driver-intel-xe-hwmon   |  40 ++++++
>>   drivers/gpu/drm/xe/xe_device_types.h          |   3 +
>>   drivers/gpu/drm/xe/xe_hwmon.c                 | 116 +++++++++++++++++-
>>   drivers/gpu/drm/xe/xe_pci.c                   |   5 +
>>   drivers/gpu/drm/xe/xe_pci_types.h             |   1 +
>>   drivers/gpu/drm/xe/xe_pcode_api.h             |   3 +
>>   6 files changed, 164 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-xe-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-xe-hwmon
>> index d9e2b17c6872..c8f211c336be 100644
>> --- a/Documentation/ABI/testing/sysfs-driver-intel-xe-hwmon
>> +++ b/Documentation/ABI/testing/sysfs-driver-intel-xe-hwmon
>> @@ -196,3 +196,43 @@ Description:	RW. Package burst power limit interval (Tau in PL2/Tau) in
>>   		milliseconds over which sustained power is averaged.
>>   
>>   		Only supported for particular Intel Xe graphics platforms.
>> +
>> +What:		/sys/bus/pci/drivers/xe/.../hwmon/hwmon<i>/temp2_emergency
>> +Date:		December 2025
>> +KernelVersion:	6.19
>> +Contact:	intel-xe@lists.freedesktop.org
>> +Description:	RO. Package shutdown temperature in millidegree Celsius.
>> +
>> +		Only supported for particular Intel Xe graphics platforms.
>> +
>> +What:		/sys/bus/pci/drivers/xe/.../hwmon/hwmon<i>/temp2_crit
>> +Date:		December 2025
>> +KernelVersion:	6.19
>> +Contact:	intel-xe@lists.freedesktop.org
>> +Description:	RO. Package critical temperature in millidegree Celsius.
>> +
>> +		Only supported for particular Intel Xe graphics platforms.
>> +
>> +What:		/sys/bus/pci/drivers/xe/.../hwmon/hwmon<i>/temp2_max
>> +Date:		December 2025
>> +KernelVersion:	6.19
>> +Contact:	intel-xe@lists.freedesktop.org
>> +Description:	RO. Package average temperature limit in millidegree Celsius.
>> +
>> +		Only supported for particular Intel Xe graphics platforms.
>> +
>> +What:		/sys/bus/pci/drivers/xe/.../hwmon/hwmon<i>/temp3_emergency
>> +Date:		December 2025
>> +KernelVersion:	6.19
>> +Contact:	intel-xe@lists.freedesktop.org
>> +Description:	RO. VRAM shutdown temperature in millidegree Celsius.
>> +
>> +		Only supported for particular Intel Xe graphics platforms.
>> +
>> +What:		/sys/bus/pci/drivers/xe/.../hwmon/hwmon<i>/temp3_crit
>> +Date:		December 2025
>> +KernelVersion:	6.19
>> +Contact:	intel-xe@lists.freedesktop.org
>> +Description:	RO. VRAM critical temperature in millidegree Celsius.
>> +
>> +		Only supported for particular Intel Xe graphics platforms.
> As per hwmon documentation all these attributes are RW so they don't
> match with ABI expectations.

1. These are read only values for our hardware, write is not supported.

2. Couple of other drivers are also supporting readonly for temp_crit.

Ex:

a) drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.c - bnxt_hwmon_is_visible

75 static umode_t bnxt_hwmon_is_visible(const void *_data, enum 
hwmon_sensor_types type,
  76                                      u32 attr, int channel)
  77 { 82
  83         switch (attr) {
  86         case hwmon_temp_max:
  87         case hwmon_temp_crit:
  88         case hwmon_temp_emergency:
  89         case hwmon_temp_max_alarm:
  90         case hwmon_temp_crit_alarm:
  91         case hwmon_temp_emergency_alarm:
  92                 if (!(bp->fw_cap & 
BNXT_FW_CAP_THRESHOLD_TEMP_SUPPORTED))
  93                         return 0;
  94                 return 0444;

b) . drivers/hwmon/intel-m10-bmc-hwmon.c -

static const struct hwmon_ops m10bmc_hwmon_ops = {
         .visible = 0444,
         .read = m10bmc_hwmon_read,

3. hwmon also doesn't report any violation for 0444 return from 
xe_hwmon_temp_is_visible.

>> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
>> index 413ba4c8b62e..8e6dd2665596 100644
>> --- a/drivers/gpu/drm/xe/xe_device_types.h
>> +++ b/drivers/gpu/drm/xe/xe_device_types.h
>> @@ -321,6 +321,9 @@ struct xe_device {
>>   		 * pcode mailbox commands.
>>   		 */
>>   		u8 has_mbx_power_limits:1;
>> +		/** @info.has_mbx_thermal_info: Device has support for thermal mailbox commands.
>> +		 */
> Can be one line.
>
>> +		u8 has_mbx_thermal_info:1;
>>   		/** @info.has_mem_copy_instr: Device supports MEM_COPY instruction */
>>   		u8 has_mem_copy_instr:1;
>>   		/** @info.has_mert: Device has standalone MERT */
>> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
>> index ff2aea52ef75..66a8c3e40027 100644
>> --- a/drivers/gpu/drm/xe/xe_hwmon.c
>> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
>> @@ -53,6 +53,14 @@ enum xe_fan_channel {
>>   	FAN_MAX,
>>   };
>>   
>> +enum xe_temp_limit {
>> +	TEMP_LIMIT_PKG_SHUTDOWN,
>> +	TEMP_LIMIT_PKG_TJMAX,
>> +	TEMP_LIMIT_MEM_SHUTDOWN,
>> +	TEMP_LIMIT_PKG_CRIT,
>> +	TEMP_LIMIT_MEM_TJMAX,
>> +};
>> +
>>   /* Attribute index for powerX_xxx_interval sysfs entries */
>>   enum sensor_attr_power {
>>   	SENSOR_INDEX_PSYS_PL1,
>> @@ -111,6 +119,18 @@ struct xe_hwmon_fan_info {
>>   	u64 time_prev;
>>   };
>>   
>> +/**
>> + * struct xe_hwmon_thermal_info - to store temperature data
>> + */
>> +struct xe_hwmon_thermal_info {
>> +	union {
>> +		/** @limits: temperatures limits */
>> +		u8 limit[8];
> What are the magic numbers? Can we have proper defines?

We can use xe_temp_limit max value and keep remaining bytes as reserved.

>
>> +		/** @data: temperature limits in dwords */
>> +		u32 data[2];
>> +	};
>> +};
>> +
>>   /**
>>    * struct xe_hwmon - xe hwmon data structure
>>    */
>> @@ -137,7 +157,8 @@ struct xe_hwmon {
>>   	u32 pl1_on_boot[CHANNEL_MAX];
>>   	/** @pl2_on_boot: power limit PL2 on boot */
>>   	u32 pl2_on_boot[CHANNEL_MAX];
>> -
>> +	/** @temp: Temperature info */
>> +	struct xe_hwmon_thermal_info temp;
>>   };
>>   
>>   static int xe_hwmon_pcode_read_power_limit(const struct xe_hwmon *hwmon, u32 attr, int channel,
>> @@ -677,8 +698,11 @@ static const struct attribute_group *hwmon_groups[] = {
>>   };
>>   
>>   static const struct hwmon_channel_info * const hwmon_info[] = {
>> -	HWMON_CHANNEL_INFO(temp, HWMON_T_LABEL, HWMON_T_INPUT | HWMON_T_LABEL,
>> -			   HWMON_T_INPUT | HWMON_T_LABEL),
>> +	HWMON_CHANNEL_INFO(temp,
>> +			   HWMON_T_LABEL,
>> +			   HWMON_T_LABEL | HWMON_T_INPUT | HWMON_T_EMERGENCY | HWMON_T_CRIT |
>> +			   HWMON_T_MAX,
>> +			   HWMON_T_LABEL | HWMON_T_INPUT | HWMON_T_EMERGENCY | HWMON_T_CRIT),
>>   	HWMON_CHANNEL_INFO(power, HWMON_P_MAX | HWMON_P_RATED_MAX | HWMON_P_LABEL | HWMON_P_CRIT |
>>   			   HWMON_P_CAP,
>>   			   HWMON_P_MAX | HWMON_P_RATED_MAX | HWMON_P_LABEL | HWMON_P_CAP),
>> @@ -689,6 +713,23 @@ static const struct hwmon_channel_info * const hwmon_info[] = {
>>   	NULL
>>   };
>>   
>> +static int xe_hwmon_pcode_read_thermal_info(struct xe_hwmon *hwmon)
>> +{
>> +	struct xe_tile *root_tile = xe_device_get_root_tile(hwmon->xe);
>> +	int ret = 0;
>> +
>> +	if (!hwmon->xe->info.has_mbx_power_limits)
>> +		return -EOPNOTSUPP;
>> +
>> +	/* Read thermal info */
>> +	ret = xe_pcode_read(root_tile, PCODE_MBOX(PCODE_THERMAL_INFO, READ_THERMAL_LIMITS, 0),
>> +			    &hwmon->temp.data[0], &hwmon->temp.data[1]);
>> +	drm_dbg(&hwmon->xe->drm, "thermal info read ret %d, val 0x%x val1 0x%x\n", ret,
>> +		hwmon->temp.data[0], hwmon->temp.data[1]);
>> +
>> +	return ret;
>> +}
>> +
>>   /* I1 is exposed as power_crit or as curr_crit depending on bit 31 */
>>   static int xe_hwmon_pcode_read_i1(const struct xe_hwmon *hwmon, u32 *uval)
>>   {
>> @@ -787,6 +828,34 @@ static umode_t
>>   xe_hwmon_temp_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
>>   {
>>   	switch (attr) {
>> +	case hwmon_temp_emergency:
>> +		switch (channel) {
>> +		case CHANNEL_PKG:
>> +			return hwmon->temp.limit[TEMP_LIMIT_PKG_SHUTDOWN] ? 0444 : 0;
>> +		case CHANNEL_VRAM:
>> +			return hwmon->temp.limit[TEMP_LIMIT_MEM_SHUTDOWN] ? 0444 : 0;
>> +		default:
>> +			return 0;
>> +		}
>> +		break;
>> +	case hwmon_temp_crit:
>> +		switch (channel) {
>> +		case CHANNEL_PKG:
>> +			return hwmon->temp.limit[TEMP_LIMIT_PKG_TJMAX] ? 0444 : 0;
>> +		case CHANNEL_VRAM:
>> +			return hwmon->temp.limit[TEMP_LIMIT_MEM_TJMAX] ? 0444 : 0;
>> +		default:
>> +			return 0;
>> +		}
>> +		break;
>> +	case hwmon_temp_max:
>> +		switch (channel) {
>> +		case CHANNEL_PKG:
>> +			return hwmon->temp.limit[TEMP_LIMIT_PKG_CRIT] ? 0444 : 0;
>> +		default:
>> +			return 0;
>> +		}
>> +		break;
>>   	case hwmon_temp_input:
>>   	case hwmon_temp_label:
>>   		return xe_reg_is_valid(xe_hwmon_get_reg(hwmon, REG_TEMP, channel)) ? 0444 : 0;
>> @@ -807,10 +876,46 @@ xe_hwmon_temp_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *val)
>>   
>>   		/* HW register value is in degrees Celsius, convert to millidegrees. */
>>   		*val = REG_FIELD_GET(TEMP_MASK, reg_val) * MILLIDEGREE_PER_DEGREE;
>> -		return 0;
>> +		break;
>> +	case hwmon_temp_emergency:
>> +		switch (channel) {
>> +		case CHANNEL_PKG:
>> +			*val = hwmon->temp.limit[TEMP_LIMIT_PKG_SHUTDOWN] * MILLIDEGREE_PER_DEGREE;
>> +			break;
>> +		case CHANNEL_VRAM:
>> +			*val = hwmon->temp.limit[TEMP_LIMIT_MEM_SHUTDOWN] * MILLIDEGREE_PER_DEGREE;
>> +			break;
>> +		default:
>> +			*val = 0;
> Do you need this?
>
>> +			return -EOPNOTSUPP;
>> +		}
>> +		break;
>> +	case hwmon_temp_crit:
>> +		switch (channel) {
>> +		case CHANNEL_PKG:
>> +			*val = hwmon->temp.limit[TEMP_LIMIT_PKG_TJMAX] * MILLIDEGREE_PER_DEGREE;
>> +			break;
>> +		case CHANNEL_VRAM:
>> +			*val = hwmon->temp.limit[TEMP_LIMIT_MEM_TJMAX] * MILLIDEGREE_PER_DEGREE;
>> +			break;
>> +		default:
>> +			*val = 0;
> Ditto.
>
>> +			return -EOPNOTSUPP;
>> +		}
>> +		break;
>> +	case hwmon_temp_max:
>> +		switch (channel) {
>> +		case CHANNEL_PKG:
>> +			*val = hwmon->temp.limit[TEMP_LIMIT_PKG_CRIT] * MILLIDEGREE_PER_DEGREE;
>> +			break;
>> +		default:
>> +			return 0;
>> +		}
>> +		break;
>>   	default:
>>   		return -EOPNOTSUPP;
>>   	}
>> +	return 0;
>>   }
>>   
>>   static umode_t
>> @@ -1263,6 +1368,9 @@ xe_hwmon_get_preregistration_info(struct xe_hwmon *hwmon)
>>   	for (channel = 0; channel < FAN_MAX; channel++)
>>   		if (xe_hwmon_is_visible(hwmon, hwmon_fan, hwmon_fan_input, channel))
>>   			xe_hwmon_fan_input_read(hwmon, channel, &fan_speed);
>> +
>> +	if (xe_hwmon_pcode_read_thermal_info(hwmon))
>> +		drm_dbg(&hwmon->xe->drm, "Thermal mailbox support not present in firmware\n");
>>   }
>>   
>>   int xe_hwmon_register(struct xe_device *xe)
>> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
>> index 673761c8623e..a47637900e84 100644
>> --- a/drivers/gpu/drm/xe/xe_pci.c
>> +++ b/drivers/gpu/drm/xe/xe_pci.c
>> @@ -311,6 +311,7 @@ static const struct xe_device_desc dg2_desc = {
>>   	.has_display = true,
>>   	.has_fan_control = true,
>>   	.has_mbx_power_limits = false,
>> +	.has_mbx_thermal_info = false,
> Why?
>
>>   };
>>   
>>   static const __maybe_unused struct xe_device_desc pvc_desc = {
>> @@ -328,6 +329,7 @@ static const __maybe_unused struct xe_device_desc pvc_desc = {
>>   	.vm_max_level = 4,
>>   	.vram_flags = XE_VRAM_FLAGS_NEED64K,
>>   	.has_mbx_power_limits = false,
>> +	.has_mbx_thermal_info = false,
> Ditto.
>
>>   };
>>   
>>   static const struct xe_device_desc mtl_desc = {
>> @@ -365,6 +367,7 @@ static const struct xe_device_desc bmg_desc = {
>>   	.has_fan_control = true,
>>   	.has_flat_ccs = 1,
>>   	.has_mbx_power_limits = true,
>> +	.has_mbx_thermal_info = true,
>>   	.has_gsc_nvm = 1,
>>   	.has_heci_cscfi = 1,
>>   	.has_i2c = true,
>> @@ -418,6 +421,7 @@ static const struct xe_device_desc cri_desc = {
>>   	.has_flat_ccs = false,
>>   	.has_i2c = true,
>>   	.has_mbx_power_limits = true,
>> +	.has_mbx_thermal_info = true,
>>   	.has_mert = true,
>>   	.has_pre_prod_wa = 1,
>>   	.has_sriov = true,
>> @@ -681,6 +685,7 @@ static int xe_info_init_early(struct xe_device *xe,
>>   	/* runtime fusing may force flat_ccs to disabled later */
>>   	xe->info.has_flat_ccs = desc->has_flat_ccs;
>>   	xe->info.has_mbx_power_limits = desc->has_mbx_power_limits;
>> +	xe->info.has_mbx_thermal_info = desc->has_mbx_thermal_info;
>>   	xe->info.has_gsc_nvm = desc->has_gsc_nvm;
>>   	xe->info.has_heci_gscfi = desc->has_heci_gscfi;
>>   	xe->info.has_heci_cscfi = desc->has_heci_cscfi;
>> diff --git a/drivers/gpu/drm/xe/xe_pci_types.h b/drivers/gpu/drm/xe/xe_pci_types.h
>> index 3bb51d155951..34d2b66188e4 100644
>> --- a/drivers/gpu/drm/xe/xe_pci_types.h
>> +++ b/drivers/gpu/drm/xe/xe_pci_types.h
>> @@ -48,6 +48,7 @@ struct xe_device_desc {
>>   	u8 has_late_bind:1;
>>   	u8 has_llc:1;
>>   	u8 has_mbx_power_limits:1;
>> +	u8 has_mbx_thermal_info:1;
>>   	u8 has_mem_copy_instr:1;
>>   	u8 has_mert:1;
>>   	u8 has_pre_prod_wa:1;
>> diff --git a/drivers/gpu/drm/xe/xe_pcode_api.h b/drivers/gpu/drm/xe/xe_pcode_api.h
>> index 975892d6b230..a3bff76a074d 100644
>> --- a/drivers/gpu/drm/xe/xe_pcode_api.h
>> +++ b/drivers/gpu/drm/xe/xe_pcode_api.h
>> @@ -65,6 +65,9 @@
>>   #define       FAN_TABLE				1
>>   #define       VR_CONFIG				2
>>   
>> +#define	PCODE_THERMAL_INFO			0x25
>> +#define	READ_THERMAL_LIMITS			0x0
> Does this match with spacing convention and ordering?
>
> Raag
>
>>   #define   PCODE_FREQUENCY_CONFIG		0x6e
>>   /* Frequency Config Sub Commands (param1) */
>>   #define     PCODE_MBOX_FC_SC_READ_FUSED_P0	0x0
>> -- 
>> 2.25.1
>>