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

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

On Fri, Sep 25, 2026 at 02:24:20AM +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_mbx_temp_sentinel, enabled on CRI currently.
> 
> Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_device_types.h |  2 ++
>  drivers/gpu/drm/xe/xe_hwmon.c        | 16 ++++++++++++++++
>  drivers/gpu/drm/xe/xe_pci.c          |  2 ++
>  drivers/gpu/drm/xe/xe_pci_types.h    |  1 +
>  4 files changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index 4661bfce2f4e..87554605fa41 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -202,6 +202,8 @@ struct xe_device {
>  		 * pcode mailbox commands.
>  		 */
>  		u8 has_mbx_power_limits:1;
> +		/** @info.has_mbx_temp_sentinel: Device has temperature-unavailable sentinels */
> +		u8 has_mbx_temp_sentinel:1;
>  		/** @info.has_mbx_thermal_info: Device supports thermal mailbox commands */
>  		u8 has_mbx_thermal_info:1;
>  		/** @info.has_mem_copy_instr: Device supports MEM_COPY instruction */
> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> index c743867a2ebf..8a1dbe708828 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)
> @@ -822,6 +825,19 @@ 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)
> +{
> +	return hwmon->xe->info.has_mbx_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_mbx_temp_sentinel ? value != TEMP_MBX_INVALID
> +						     : value != 0;
> +}

When reviewing the patch 7, I noticed a small change that took me back to the
temp valid...

What if temperature is 0 ?!
Why isn't it valid?

> +
>  /* 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 d66d5a0339ae..e656fc012902 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -476,6 +476,7 @@ static const struct xe_device_desc cri_desc = {
>  	.has_gsc_nvm = 1,
>  	.has_i2c = true,
>  	.has_mbx_power_limits = true,
> +	.has_mbx_temp_sentinel = true,
>  	.has_mbx_thermal_info = true,
>  	.has_mert = true,
>  	.has_pre_prod_wa = 1,
> @@ -796,6 +797,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_temp_sentinel = desc->has_mbx_temp_sentinel;
>  	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;
> diff --git a/drivers/gpu/drm/xe/xe_pci_types.h b/drivers/gpu/drm/xe/xe_pci_types.h
> index fed509ff601e..0041ec5676d3 100644
> --- a/drivers/gpu/drm/xe/xe_pci_types.h
> +++ b/drivers/gpu/drm/xe/xe_pci_types.h
> @@ -50,6 +50,7 @@ struct xe_device_desc {
>  	u8 has_late_bind:1;
>  	u8 has_llc:1;
>  	u8 has_mbx_power_limits:1;
> +	u8 has_mbx_temp_sentinel:1;
>  	u8 has_mbx_thermal_info:1;
>  	u8 has_mert:1;
>  	u8 has_pre_prod_wa:1;
> -- 
> 2.25.1
>