Re: [PATCH 2/3] drm/xe/hwmon: Use VRAM temperature sensor count from thermal config on CRI

[Raag Jadav <raag.jadav@intel.com>]

On Tue, Aug 25, 2026 at 12:11:32AM +0530, Karthik Poosa wrote:
> Read the number of VRAM temperature sensor channels from the second byte
> of READ_THERMAL_CONFIG on CRI platforms. Use the reported count to avoid
> exposing hwmon attributes for unavailable VRAM temperature sensors, while
> retaining the maximum supported channel count on non-CRI platforms.
> 
> Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_hwmon.c     | 35 ++++++++++++++++++++++++++-----
>  drivers/gpu/drm/xe/xe_pcode_api.h |  1 +
>  2 files changed, 31 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> index 2c4eba4b8f8f..6e7cb250e628 100644
> --- a/drivers/gpu/drm/xe/xe_hwmon.c
> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> @@ -39,7 +39,8 @@ enum xe_hwmon_reg_operation {
>  	REG_READ64,
>  };
>  
> -#define MAX_VRAM_CHANNELS      (16)
> +/* Maximum number of VRAM channels supported by Xe */
> +#define MAX_VRAM_CHANNELS      (80)

Why?

>  enum xe_hwmon_channel {
>  	CHANNEL_CARD,
> @@ -48,6 +49,7 @@ enum xe_hwmon_channel {
>  	CHANNEL_MCTRL,
>  	CHANNEL_PCIE,
>  	CHANNEL_VRAM_N,
> +	/* Compile-time upper bound; actual channel count is hwmon->temp.vram_count */
>  	CHANNEL_VRAM_N_MAX = CHANNEL_VRAM_N + MAX_VRAM_CHANNELS - 1,
>  	CHANNEL_MAX,
>  };
> @@ -144,10 +146,12 @@ struct xe_hwmon_thermal_info {
>  	};
>  	/** @count: no of temperature sensors available for the platform */

So now this can be "total number of temperature sensors"?

>  	u8 count;
> +	/** @vram_count: number of VRAM temperature sensors available for the platform */
> +	u8 vram_count;
>  	/** @value: signed value from each sensor */
>  	s8 value[U8_MAX];
> -	/** @vram_label: vram label names */
> -	char vram_label[MAX_VRAM_CHANNELS][MAX_LABEL_SIZE];
> +	/** @vram_label: vram label names, dynamically allocated based on vram_count */
> +	char (*vram_label)[MAX_LABEL_SIZE];
>  };
>  
>  /**
> @@ -271,7 +275,7 @@ static struct xe_reg xe_hwmon_get_reg(struct xe_hwmon *hwmon, enum xe_hwmon_reg
>  				return BMG_PACKAGE_TEMPERATURE;
>  			else if (channel == CHANNEL_VRAM)
>  				return BMG_VRAM_TEMPERATURE;
> -			else if (in_range(channel, CHANNEL_VRAM_N, MAX_VRAM_CHANNELS))
> +			else if (in_range(channel, CHANNEL_VRAM_N, hwmon->temp.vram_count))
>  				return BMG_VRAM_TEMPERATURE_N(channel - CHANNEL_VRAM_N);
>  		} else if (xe->info.platform == XE_DG2) {
>  			if (channel == CHANNEL_PKG)
> @@ -810,6 +814,17 @@ static int xe_hwmon_pcode_read_thermal_info(struct xe_hwmon *hwmon)
>  	drm_dbg(&hwmon->xe->drm, "thermal config count 0x%x\n", config);
>  	hwmon->temp.count = REG_FIELD_GET(TEMP_MASK, config);
>  
> +	if (hwmon->xe->info.platform >= XE_CRESCENTISLAND) {

Same as last patch. Don't solve problems that don't exist.

> +		hwmon->temp.vram_count = REG_FIELD_GET(VRAM_COUNT_MASK, config);
> +		if (hwmon->temp.vram_count > MAX_VRAM_CHANNELS && hwmon->temp.vram_count) {

Isn't the first condition sufficient? What am I missing?

> +			drm_warn(&hwmon->xe->drm, "VRAM channel count %d exceeds max %d, clamping\n",

Can this be invalid? i.e. 0xff? And should we clamp it in that case?

> +				 hwmon->temp.vram_count, MAX_VRAM_CHANNELS);
> +			hwmon->temp.vram_count = MAX_VRAM_CHANNELS;

So perhaps CRI_MAX_VRAM_CHANNELS?

> +		}
> +	} else {
> +		hwmon->temp.vram_count = 16; /* For older platforms, max is 16 VRAM channels */

BMG_MAX_VRAM_CHANNELS?

> +	}
> +
>  	return ret;
>  }
>  
> @@ -988,6 +1003,9 @@ static inline bool is_vram_ch_available(struct xe_hwmon *hwmon, int channel)
>  	u32 reg_val;
>  	u8 temp;
>  
> +	if (vram_id >= hwmon->temp.vram_count)
> +		return false;
> +
>  	vram_reg = xe_hwmon_get_reg(hwmon, REG_TEMP, channel);
>  	if (!xe_reg_is_valid(vram_reg))
>  		return false;
> @@ -1516,7 +1534,7 @@ static int xe_hwmon_read_label(struct device *dev,
>  			*str = "mctrl";
>  		else if (channel == CHANNEL_PCIE)
>  			*str = "pcie";
> -		else if (in_range(channel, CHANNEL_VRAM_N, MAX_VRAM_CHANNELS))
> +		else if (in_range(channel, CHANNEL_VRAM_N, hwmon->temp.vram_count))
>  			*str = hwmon->temp.vram_label[channel - CHANNEL_VRAM_N];
>  		return 0;
>  	case hwmon_power:
> @@ -1645,6 +1663,13 @@ int xe_hwmon_register(struct xe_device *xe)
>  
>  	xe_hwmon_get_preregistration_info(hwmon);
>  
> +	hwmon->temp.vram_label = devm_kcalloc(dev, hwmon->temp.vram_count,

What if vram_count is 0?

Raag

> +					      MAX_LABEL_SIZE, GFP_KERNEL);
> +	if (!hwmon->temp.vram_label) {
> +		xe->hwmon = NULL;
> +		return -ENOMEM;
> +	}
> +
>  	drm_dbg(&xe->drm, "Register xe hwmon interface\n");
>  
>  	/*  hwmon_dev points to device hwmon<i> */
> diff --git a/drivers/gpu/drm/xe/xe_pcode_api.h b/drivers/gpu/drm/xe/xe_pcode_api.h
> index 94575c476e3d..e1079eff72c6 100644
> --- a/drivers/gpu/drm/xe/xe_pcode_api.h
> +++ b/drivers/gpu/drm/xe/xe_pcode_api.h
> @@ -57,6 +57,7 @@
>  #define   PCODE_THERMAL_INFO			0x25
>  #define     READ_THERMAL_LIMITS			0x0
>  #define     READ_THERMAL_CONFIG			0x1
> +#define       VRAM_COUNT_MASK			REG_GENMASK(15, 8)
>  #define     READ_THERMAL_DATA			0x2
>  #define       PCIE_SENSOR_GROUP_ID		0x2
>  #define       PCIE_SENSOR_MASK			REG_GENMASK(31, 16)
> -- 
> 2.25.1
>