Re: [PATCH v3 3/4] drm/xe/hwmon: Expose GPU pcie temperature

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

On 19-12-2025 13:53, Raag Jadav wrote:
> On Tue, Dec 16, 2025 at 05:10:29PM +0530, Karthik Poosa wrote:
>> Expose GPU PCIe average temperature and its limits via hwmon
> Use consistent upper/lower cases in subject.
ok
>
>> sysfs temp5_xxx.
>> Update Xe hwmon sysfs documentation for this.
>>
>> v2: Update kernel version in Xe hwmon documentation. (Raag)
>>
>> Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>
>> ---
>>   .../ABI/testing/sysfs-driver-intel-xe-hwmon   | 24 +++++++++++++
>>   drivers/gpu/drm/xe/xe_hwmon.c                 | 36 +++++++++++++++++++
>>   2 files changed, 60 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-xe-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-xe-hwmon
>> index 81f9b5d58850..51a35fcfb393 100644
>> --- a/Documentation/ABI/testing/sysfs-driver-intel-xe-hwmon
>> +++ b/Documentation/ABI/testing/sysfs-driver-intel-xe-hwmon
>> @@ -260,3 +260,27 @@ Contact:	intel-xe@lists.freedesktop.org
>>   Description:	RO. Memory controller critical temperature in millidegree Celsius.
>>   
>>   		Only supported for particular Intel Xe graphics platforms.
>> +
>> +What:		/sys/bus/pci/drivers/xe/.../hwmon/hwmon<i>/temp5_input
>> +Date:		December 2025
>> +KernelVersion:	6.19
>> +Contact:	intel-xe@lists.freedesktop.org
>> +Description:	RO. GPU PCIe temperature in millidegree Celsius.
>> +
>> +		Only supported for particular Intel Xe graphics platforms.
>> +
>> +What:		/sys/bus/pci/drivers/xe/.../hwmon/hwmon<i>/temp5_emergency
>> +Date:		December 2025
>> +KernelVersion:	6.19
>> +Contact:	intel-xe@lists.freedesktop.org
>> +Description:	RO. GPU PCIe shutdown temperature in millidegree Celsius.
>> +
>> +		Only supported for particular Intel Xe graphics platforms.
>> +
>> +What:		/sys/bus/pci/drivers/xe/.../hwmon/hwmon<i>/temp5_crit
>> +Date:		December 2025
>> +KernelVersion:	6.19
>> +Contact:	intel-xe@lists.freedesktop.org
>> +Description:	RO. GPU PCIe critical temperature in millidegree Celsius.
>> +
>> +		Only supported for particular Intel Xe graphics platforms.
> Same as patch 1. These attributes defer from ABI definition so not my call.
>
>> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
>> index 6d31ad74cd0e..b8519c734b4e 100644
>> --- a/drivers/gpu/drm/xe/xe_hwmon.c
>> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
>> @@ -44,6 +44,7 @@ enum xe_hwmon_channel {
>>   	CHANNEL_PKG,
>>   	CHANNEL_VRAM,
>>   	CHANNEL_MCTRL,
>> +	CHANNEL_PCIE,
>>   	CHANNEL_MAX,
>>   };
>>   
>> @@ -713,6 +714,7 @@ static const struct hwmon_channel_info * const hwmon_info[] = {
>>   			   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_T_LABEL | HWMON_T_INPUT | HWMON_T_EMERGENCY | HWMON_T_CRIT,
>>   			   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,
>> @@ -787,6 +789,28 @@ static int get_mc_temp(struct xe_hwmon *hwmon, long *val)
>>   	return 0;
>>   }
>>   
>> +static int get_pcie_temp(struct xe_hwmon *hwmon, long *val)
>> +{
>> +	struct xe_tile *root_tile = xe_device_get_root_tile(hwmon->xe);
>> +	int ret = 0;
>> +	u32 data = 0;
>> +
>> +	ret = xe_pcode_read(root_tile, PCODE_MBOX(PCODE_THERMAL_INFO, READ_THERMAL_DATA, 2),
>> +			    &data, NULL);
>> +	drm_dbg(&hwmon->xe->drm, "thermal data for pcie ret %d, val 0x%x\n", ret, data);
> Same comments as last patch (and also in all other places where applicable).
we can remove this.
>
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (hwmon->xe->info.subplatform != XE_SUBPLATFORM_BATTLEMAGE_G21)
>> +		data >>= 8;
> I'm a bit lost here. How is data format different per subplatform?
> Shouldn't this be a pcode bug?

Format is same, only offset of sensor is different for G21 is different 
as it has less sensors.

>
>> +	*val = (data & TEMP_MASK_MAILBOX) * MILLIDEGREE_PER_DEGREE;
>> +
>> +	if (data & 0x80)
>> +		*val = *val * -1;
>> +
>> +	return 0;
>> +}
>> +
>>   /* 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)
>>   {
>> @@ -895,6 +919,8 @@ xe_hwmon_temp_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
>>   			return hwmon->temp.limit[TEMP_LIMIT_MEM_SHUTDOWN] ? 0444 : 0;
>>   		case CHANNEL_MCTRL:
>>   			return (!get_mc_temp(hwmon, &val)) ? 0444 : 0;
>> +		case CHANNEL_PCIE:
>> +			return (!get_pcie_temp(hwmon, &val)) ? 0444 : 0;
> Same comments as last patch.
>
>>   		default:
>>   			return 0;
>>   		}
>> @@ -907,6 +933,8 @@ xe_hwmon_temp_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
>>   			return hwmon->temp.limit[TEMP_LIMIT_MEM_TJMAX] ? 0444 : 0;
>>   		case CHANNEL_MCTRL:
>>   			return (!get_mc_temp(hwmon, &val)) ? 0444 : 0;
>> +		case CHANNEL_PCIE:
>> +			return (!get_pcie_temp(hwmon, &val)) ? 0444 : 0;
> Ditto.
>
>>   		default:
>>   			return 0;
>>   		}
>> @@ -928,6 +956,8 @@ xe_hwmon_temp_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
>>   					       channel)) ? 0444 : 0;
>>   		case CHANNEL_MCTRL:
>>   			return (!get_mc_temp(hwmon, &val)) ? 0444 : 0;
>> +		case CHANNEL_PCIE:
>> +			return (!get_pcie_temp(hwmon, &val)) ? 0444 : 0;
> Ditto.
>
> Raag
>
>>   		default:
>>   			return 0;
>>   		}
>> @@ -954,6 +984,8 @@ xe_hwmon_temp_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *val)
>>   			break;
>>   		case CHANNEL_MCTRL:
>>   			return get_mc_temp(hwmon, val);
>> +		case CHANNEL_PCIE:
>> +			return get_pcie_temp(hwmon, val);
>>   		default:
>>   			*val = 0;
>>   			return -EOPNOTSUPP;
>> @@ -962,6 +994,7 @@ xe_hwmon_temp_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *val)
>>   	case hwmon_temp_emergency:
>>   		switch (channel) {
>>   		case CHANNEL_PKG:
>> +		case CHANNEL_PCIE:
>>   			*val = hwmon->temp.limit[TEMP_LIMIT_PKG_SHUTDOWN] * MILLIDEGREE_PER_DEGREE;
>>   			break;
>>   		case CHANNEL_VRAM:
>> @@ -976,6 +1009,7 @@ xe_hwmon_temp_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *val)
>>   	case hwmon_temp_crit:
>>   		switch (channel) {
>>   		case CHANNEL_PKG:
>> +		case CHANNEL_PCIE:
>>   			*val = hwmon->temp.limit[TEMP_LIMIT_PKG_TJMAX] * MILLIDEGREE_PER_DEGREE;
>>   			break;
>>   		case CHANNEL_VRAM:
>> @@ -1360,6 +1394,8 @@ static int xe_hwmon_read_label(struct device *dev,
>>   			*str = "vram";
>>   		else if (channel == CHANNEL_MCTRL)
>>   			*str = "mctrl_avg";
>> +		else if (channel == CHANNEL_PCIE)
>> +			*str = "pcie";
>>   		return 0;
>>   	case hwmon_power:
>>   	case hwmon_energy:
>> -- 
>> 2.25.1
>>