Re: [PATCH v6 02/13] drm/xe/hwmon: Decode mailbox temperature as sign-magnitude

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

On Fri, Sep 25, 2026 at 02:24:18AM +0530, Karthik Poosa wrote:
> The mailbox temperature is an 8-bit sign-magnitude value (bit 7 sign,
> bits 6:0 magnitude, range -127..127), not two's-complement. Casting the
> raw byte to s8 decodes negative readings incorrectly.
> 
> Add mbx_temp_decode(), store the raw byte as u8, and use it in
> get_mc_temp() and get_pcie_temp() so temperatures are decoded correctly.
> 
> Fixes: 3a0cb885e111 ("drm/xe/hwmon: Expose memory controller temperature")
> Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>


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


> ---
>  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 be3fb6733b6f..57199ea00a60 100644
> --- a/drivers/gpu/drm/xe/xe_hwmon.c
> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> @@ -104,6 +104,10 @@ enum sensor_attr_power {
>  /* Index of memory controller in READ_THERMAL_DATA output */
>  #define TEMP_INDEX_MCTRL	2
>  
> +/* 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)
> +
>  /* Maximum characters in hwmon label name */
>  #define MAX_LABEL_SIZE		16
>  
> @@ -139,8 +143,8 @@ struct xe_hwmon_thermal_info {
>  	};
>  	/** @count: no of temperature sensors available for the platform */
>  	u8 count;
> -	/** @value: signed value from each sensor */
> -	s8 value[U8_MAX];
> +	/** @value: per-sensor raw mailbox temperature; bit7=sign, bits6:0=magnitude */
> +	u8 value[U8_MAX];
>  	/** @vram_label: vram label names */
>  	char vram_label[MAX_VRAM_CHANNELS][MAX_LABEL_SIZE];
>  };
> @@ -814,6 +818,14 @@ static int xe_hwmon_pcode_read_thermal_info(struct xe_hwmon *hwmon)
>  	return ret;
>  }
>  
> +/* Decode an 8-bit sign-magnitude mailbox temperature to degrees Celsius. */
> +static int mbx_temp_decode(u8 value)
> +{
> +	int temp = value & TEMP_MBX_MAG;
> +
> +	return (value & TEMP_MBX_SIGN) ? -temp : temp;
> +}
> +
>  static int get_mc_temp(struct xe_hwmon *hwmon, long *val)
>  {
>  	struct xe_tile *root_tile = xe_device_get_root_tile(hwmon->xe);
> @@ -830,7 +842,7 @@ static int get_mc_temp(struct xe_hwmon *hwmon, long *val)
>  	}
>  
>  	for (i = TEMP_INDEX_MCTRL; i < hwmon->temp.count - 1; i++)
> -		average += hwmon->temp.value[i];
> +		average += mbx_temp_decode(hwmon->temp.value[i]);
>  
>  	average /= (hwmon->temp.count - TEMP_INDEX_MCTRL - 1);
>  	*val = average * MILLIDEGREE_PER_DEGREE;
> @@ -853,7 +865,7 @@ static int get_pcie_temp(struct xe_hwmon *hwmon, long *val)
>  		data = REG_FIELD_GET(PCIE_SENSOR_MASK, data);
>  
>  	data = REG_FIELD_GET(TEMP_MASK, data);
> -	*val = (s8)data * MILLIDEGREE_PER_DEGREE;
> +	*val = mbx_temp_decode(data) * MILLIDEGREE_PER_DEGREE;
>  
>  	return 0;
>  }
> -- 
> 2.25.1
>