Re: [PATCH v8 04/15] drm/xe/hwmon: Add helpers to validate thermal sensor readings

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

On Tue, Oct 06, 2026 at 09:05:23AM +0530, Karthik Poosa wrote:
> Add mmio_temp_valid() and mbx_temp_valid() to check whether a temperature
> sensor reading is valid. An MMIO value of 0xFFFFFFFF or a mailbox value of
> 0xFF indicates the sensor is unavailable.
> 
> Gate these checks behind has_temp_sentinel, enabled on CRI currently.
> 
> Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> 
> v2:
>  - Rename info.has_mbx_temp_sentinel to has_temp_sentinel. (Rodrigo)
>  - Return true from mmio_temp_valid() and mbx_temp_valid() for platforms
>    not having support for temperature sentinels. (Rodrigo)
> 
> v3:
>  - Revert the unconditional validation of temperature readings on
>    non-sentinel platforms.
>    This avoids creating spurious VRAM temperature channels when the
>    corresponding sensors are unavailable.

my rv-b remains

> 
>  drivers/gpu/drm/xe/xe_device_types.h |  2 ++
>  drivers/gpu/drm/xe/xe_hwmon.c        | 20 ++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_pci.c          |  2 ++
>  drivers/gpu/drm/xe/xe_pci_types.h    |  1 +
>  4 files changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index 4e0e1bc040c1..beb0ceaabb3f 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -227,6 +227,8 @@ struct xe_device {
>  		u8 has_sriov:1;
>  		/** @info.has_sysctrl: Supports System Controller */
>  		u8 has_sysctrl:1;
> +		/** @info.has_temp_sentinel: Device has temperature-unavailable sentinels */
> +		u8 has_temp_sentinel:1;
>  		/** @info.has_usm: Device has unified shared memory support */
>  		u8 has_usm:1;
>  		/** @info.has_64bit_timestamp: Device supports 64-bit timestamps */
> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> index 33d0ca7d86e4..5bbcb81529db 100644
> --- a/drivers/gpu/drm/xe/xe_hwmon.c
> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> @@ -104,6 +104,9 @@ enum sensor_attr_power {
>  /* Index of memory controller in READ_THERMAL_DATA output */
>  #define TEMP_INDEX_MCTRL	2
>  
> +/* Mailbox sentinel reported for an unavailable temperature sensor */
> +#define TEMP_MBX_INVALID	0xFF
> +
>  /* Mailbox temperature is sign-magnitude: bit 7 sign, bits 6:0 magnitude. */
>  #define TEMP_MBX_SIGN		BIT(7)
>  #define TEMP_MBX_MAG		GENMASK(6, 0)
> @@ -824,6 +827,23 @@ static int xe_hwmon_pcode_read_thermal_info(struct xe_hwmon *hwmon)
>  	return ret;
>  }
>  
> +static inline bool mmio_temp_valid(const struct xe_hwmon *hwmon, u32 value)
> +{
> +	/*
> +	 * On platforms lacking temperature sentinel support, a value of 0 is treated
> +	 * as invalid. While 0°C is theoretically possible, it is highly unlikely for
> +	 * a discrete GPU in normal environments, making it a reasonable fallback
> +	 * indicator for invalid or uninitialized temperature readings.
> +	 */
> +	return hwmon->xe->info.has_temp_sentinel ? value != U32_MAX : value != 0;
> +}
> +
> +/* Mailbox temperature is sign-magnitude (bit 7 sign, bits 6:0 magnitude). */
> +static inline bool mbx_temp_valid(const struct xe_hwmon *hwmon, u8 value)
> +{
> +	return hwmon->xe->info.has_temp_sentinel ? value != TEMP_MBX_INVALID : value != 0;
> +}
> +
>  /* Decode an 8-bit sign-magnitude mailbox temperature to degrees Celsius. */
>  static int mbx_temp_decode(u8 value)
>  {
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index e44b74b9aa27..111d0aa45b10 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -486,6 +486,7 @@ static const struct xe_device_desc cri_desc = {
>  	.has_soc_remapper_telem = true,
>  	.has_sriov = true,
>  	.has_sysctrl = true,
> +	.has_temp_sentinel = true,
>  	.max_gt_per_tile = 2,
>  	MULTI_LRC_MASK,
>  	.require_force_probe = true,
> @@ -817,6 +818,7 @@ static int xe_info_init_early(struct xe_device *xe,
>  	xe->info.has_sriov = xe_configfs_primary_gt_allowed(to_pci_dev(xe->drm.dev)) &&
>  		desc->has_sriov;
>  	xe->info.has_sysctrl = desc->has_sysctrl;
> +	xe->info.has_temp_sentinel = desc->has_temp_sentinel;
>  	xe->info.skip_guc_pc = desc->skip_guc_pc;
>  	xe->info.skip_pcode = desc->skip_pcode;
>  	xe->info.needs_scratch = desc->needs_scratch;
> diff --git a/drivers/gpu/drm/xe/xe_pci_types.h b/drivers/gpu/drm/xe/xe_pci_types.h
> index 448bbe5ba1d7..15eedac8467a 100644
> --- a/drivers/gpu/drm/xe/xe_pci_types.h
> +++ b/drivers/gpu/drm/xe/xe_pci_types.h
> @@ -61,6 +61,7 @@ struct xe_device_desc {
>  	u8 has_soc_remapper_telem:1;
>  	u8 has_sriov:1;
>  	u8 has_sysctrl:1;
> +	u8 has_temp_sentinel:1;
>  	u8 needs_scratch:1;
>  	u8 skip_guc_pc:1;
>  	u8 skip_pcode:1;
> -- 
> 2.25.1
>