Re: [PATCH v13 11/16] drm/xe/hwmon: Decode CRI temperature registers as IEEE-754

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

On Wed, Oct 14, 2026 at 05:28:01PM +0530, Karthik Poosa wrote:
> CRI temperature registers encode package and VRAM temperatures as
> IEEE-754 floating-point values. Decode these values before exposing
> them through hwmon.
>  
> Use integer-based IEEE-754 decoding since kernel code does not use
> floating-point operations and hwmon reports integer temperatures.
> 
> Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>
> Assisted-by: LLM
> ---
> 
> v2: Add missing description for parameter 'temperature' in
>     xe_hwmon_temperature_decode().
> 
> v3: Add missing mmio_temp_valid() check in xe_hwmon_temp_read() for MMIO
>     temperature register reads.

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

> 
>  drivers/gpu/drm/xe/xe_hwmon.c | 99 ++++++++++++++++++++++++++++++-----
>  1 file changed, 85 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> index afcc80e34bfd..dad2397db141 100644
> --- a/drivers/gpu/drm/xe/xe_hwmon.c
> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> @@ -1266,6 +1266,70 @@ xe_hwmon_temp_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
>  	}
>  }
>  
> +#define IEEE754_SIGN		REG_BIT(31)
> +#define IEEE754_EXP_MASK	REG_GENMASK(30, 23)
> +#define IEEE754_MANTISSA_MASK	REG_GENMASK(22, 0)
> +#define IEEE754_EXP_BIAS	127 /* Exponent bias to obtain the actual exponent */
> +#define IEEE754_MANTISSA_BITS	23 /* Mantissa width and implicit leading-1 bit position */
> +#define IEEE754_EXP_SPECIAL	0xff /* reserved exponent value encoding infinity or NaN */
> +
> +/**
> + * xe_hwmon_temperature_decode - Decode an IEEE-754 temperature register value
> + * @reg_val: The raw register value to decode
> + * @temperature: Pointer to store the decoded temperature in millidegrees Celsius.
> + *
> + * IEEE-754 single-precision format:
> + *
> + *  31 30                         23 22                         0
> + * +--+-----------------------------+----------------------------+
> + * | S|          exponent           |          fraction          |
> + * +--+-----------------------------+----------------------------+
> + *
> + * value = (-1)^S * 1.fraction * 2^(exponent - 127)
> + *
> + * Return: 0 on success or an error if @reg_val is non-finite or cannot be
> + * represented in millidegrees Celsius.
> + */
> +static int xe_hwmon_temperature_decode(u32 reg_val, long *temperature)
> +{
> +	u32 exponent = REG_FIELD_GET(IEEE754_EXP_MASK, reg_val);
> +	u32 mantissa = REG_FIELD_GET(IEEE754_MANTISSA_MASK, reg_val);
> +	s64 value;
> +	int shift;
> +
> +	/* Exponent 0xff encodes infinity or NaN. */
> +	if (exponent == IEEE754_EXP_SPECIAL)
> +		return -EINVAL;
> +
> +	/* Subnormal values are smaller than one millidegree Celsius. */
> +	if (!exponent) {
> +		*temperature = 0;
> +		return 0;
> +	}
> +
> +	value = (s64)(mantissa | BIT(IEEE754_MANTISSA_BITS)) * MILLIDEGREE_PER_DEGREE;
> +	shift = exponent - (IEEE754_EXP_BIAS + IEEE754_MANTISSA_BITS);
> +	if (shift >= 0) {
> +		if (shift >= BITS_PER_LONG || value > (LONG_MAX >> shift))
> +			return -ERANGE;
> +		value <<= shift;
> +	} else if (shift > -64) {
> +		value >>= -shift;
> +	} else {
> +		value = 0;
> +	}
> +
> +	/* Ensure the magnitude fits in long, which is 32-bit on some arches. */
> +	if (value > LONG_MAX)
> +		return -ERANGE;
> +
> +	if (reg_val & IEEE754_SIGN)
> +		value = -value;
> +
> +	*temperature = value;
> +	return 0;
> +}
> +
>  static int
>  xe_hwmon_temp_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *val)
>  {
> @@ -1275,26 +1339,33 @@ xe_hwmon_temp_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *val)
>  	switch (attr) {
>  	case hwmon_temp_input:
>  		switch (channel) {
> -		case CHANNEL_PKG:
> -		case CHANNEL_VRAM:
> -			reg_val = xe_mmio_read32(mmio, xe_hwmon_get_reg(hwmon, REG_TEMP, channel));
> -
> -			/* HW register value is in degrees Celsius, convert to millidegrees. */
> -			*val = REG_FIELD_GET(TEMP_MASK, reg_val) * MILLIDEGREE_PER_DEGREE;
> -			return 0;
>  		case CHANNEL_MCTRL:
>  			return get_mc_temp(hwmon, val);
>  		case CHANNEL_PCIE:
>  			return get_pcie_temp(hwmon, val);
> +		case CHANNEL_PKG:
> +		case CHANNEL_VRAM:
>  		case CHANNEL_VRAM_N...CHANNEL_VRAM_N_MAX:
>  			reg_val = xe_mmio_read32(mmio, xe_hwmon_get_reg(hwmon, REG_TEMP, channel));
> -			/*
> -			 * This temperature format is 24 bit [31:8] signed integer and 8 bit
> -			 * [7:0] fraction.
> -			 */
> -			*val = (s32)(REG_FIELD_GET(TEMP_MASK_VRAM_N, reg_val)) *
> -				(REG_FIELD_GET(TEMP_SIGN_MASK, reg_val) ? -1 : 1) *
> -				 MILLIDEGREE_PER_DEGREE;
> +			if (!mmio_temp_valid(hwmon, reg_val)) {
> +				drm_dbg(&hwmon->xe->drm,
> +					"channel %d temperature invalid, val 0x%llx\n",
> +					channel, reg_val);
> +				return -ENXIO;
> +			}
> +			if (hwmon->xe->info.platform >= XE_CRESCENTISLAND) {
> +				return xe_hwmon_temperature_decode((u32)reg_val, val);
> +			} else if (channel >= CHANNEL_VRAM_N) {
> +				/*
> +				 * This temperature format is 24 bit [31:8] signed integer and 8 bit
> +				 * [7:0] fraction for platforms before CRI.
> +				 */
> +				*val = (s32)REG_FIELD_GET(TEMP_MASK_VRAM_N, reg_val) *
> +					(REG_FIELD_GET(TEMP_SIGN_MASK, reg_val) ? -1 : 1) *
> +					MILLIDEGREE_PER_DEGREE;
> +			} else {
> +				*val = REG_FIELD_GET(TEMP_MASK, reg_val) * MILLIDEGREE_PER_DEGREE;
> +			}
>  			return 0;
>  		default:
>  			return -EOPNOTSUPP;
> -- 
> 2.25.1
>