Re: [PATCH v2 2/9] drm/xe/hwmon: initialize fan-control backend and table cache

On 17-07-2026 09:47, Karthik Poosa wrote:
> Initialize Xe hwmon fan-control support by detecting fan count,
> reading stock fan control points and min PWM.
>
> v2:
>   - Avoid user table initialization during probe. (Badal)
>   - Move unused code to appropriate patches.
>   - Use xe helpers for dmesg logs.
>
> Signed-off-by: Karthik Poosa<[email protected]>
> Assisted-by: Codex:gpt-5-4
> ---
>   drivers/gpu/drm/xe/xe_hwmon.c     | 159 +++++++++++++++++++++++++++---
>   drivers/gpu/drm/xe/xe_pcode_api.h |   5 +
>   2 files changed, 149 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> index de3f2aeffc3f..0a416e3e7b8c 100644
> --- a/drivers/gpu/drm/xe/xe_hwmon.c
> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> @@ -20,6 +20,7 @@
>   #include "xe_pcode_api.h"
>   #include "xe_sriov.h"
>   #include "xe_pm.h"
> +#include "xe_printk.h"
>   #include "xe_vsec.h"
>   #include "regs/xe_pmt.h"
>   
> @@ -122,14 +123,50 @@ struct xe_hwmon_energy_info {
>   	long accum_energy;
>   };
>   
> +enum fan_table_type {
> +	/** @STOCK_FAN_TABLE: firmware-provided fan table */
> +	STOCK_FAN_TABLE,
> +	/** @USER_FAN_TABLE: user fan table written through sysfs */
> +	USER_FAN_TABLE,
> +	/** @FAN_TABLE_MAX: number of fan table slots tracked per fan */
> +	FAN_TABLE_MAX,
> +};
> +
> +/* Maximum number of fan control points supported by each fan */
> +#define MAX_FAN_CONTROL_POINTS	(10)
> +
> +/* Fan control point index: bit31 selects fan table type; bits30:0 control point index */
> +#define FCP_INDEX(FAN_TABLE_TYPE, POINT_NUM) (((FAN_TABLE_TYPE) << 31) | \
> +						((POINT_NUM) & REG_GENMASK(30, 0)))
> +
> +/* PCODE operations timeout for fan control commands */
> +#define XE_PCODE_FAN_CONTROL_TIMEOUT_MS	(10)
> +
>   /**
> - * struct xe_hwmon_fan_info - to cache previous fan reading
> + * struct xe_hwmon_fan_info - cached fan telemetry and control state
> + *
> + * Each fan keeps the latest tachometer sampling state along with two fan
> + * tables: the stock table discovered from firmware and the user table managed
> + * by hwmon sysfs writes.
>    */
>   struct xe_hwmon_fan_info {
>   	/** @reg_val_prev: previous fan reg val */
>   	u32 reg_val_prev;
>   	/** @time_prev: previous timestamp */
>   	u64 time_prev;
> +	/** @fan_table: fan control tables */
> +	struct fan_table {
> +		/** @fan_control_point_count: number of supported fan control points */
> +		u8 fan_control_point_count;
> +		struct fan_control_point {
> +			/** @temp: temperature in degree celsius */
> +			u8 temp;
> +			/** @speed: fan speed in percentage */
> +			u8 speed;
> +		} fcp[MAX_FAN_CONTROL_POINTS];
> +	} fan_table[FAN_TABLE_MAX];
> +	/** @min_pwm: minimum fan PWM */
> +	u32 min_pwm;
>   };
>   
>   /**
> @@ -168,6 +205,8 @@ struct xe_hwmon {
>   	int scl_shift_time;
>   	/** @ei: Energy info for energyN_input */
>   	struct xe_hwmon_energy_info ei[CHANNEL_MAX];
> +	/** @num_fans: number of fans available */
> +	u8 num_fans;
>   	/** @fi: Fan info for fanN_input */
>   	struct xe_hwmon_fan_info fi[FAN_MAX];
>   	/** @boot_power_limit_read: is boot power limits read */
> @@ -856,17 +895,100 @@ static int xe_hwmon_pcode_write_i1(const struct xe_hwmon *hwmon, u32 uval)
>   			      (uval & POWER_SETUP_I1_DATA_MASK));
>   }
>   
> -static int xe_hwmon_pcode_read_fan_control(const struct xe_hwmon *hwmon, u32 subcmd, u32 *uval)
> +static int xe_hwmon_pcode_read_fan_control(const struct xe_hwmon *hwmon, u32 subcmd, u8 fan_num,
> +					   u32 *uval)
>   {
>   	struct xe_tile *root_tile = xe_device_get_root_tile(hwmon->xe);
> +	return xe_pcode_read_timeout(root_tile, PCODE_MBOX(FAN_SPEED_CONTROL, subcmd, fan_num),
> +				     uval, NULL, XE_PCODE_FAN_CONTROL_TIMEOUT_MS);
> +}
> +
> +static int xe_hwmon_get_num_fans(const struct xe_hwmon *hwmon, u32 *num_fans)
> +{
> +	u32 fan_mask = 0;
> +	int ret;
>   
>   	/* Platforms that don't return correct value */
> -	if (hwmon->xe->info.platform == XE_DG2 && subcmd == FSC_READ_NUM_FANS) {
> -		*uval = 2;
> +	if (hwmon->xe->info.platform == XE_DG2) {
> +		*num_fans = 2;
>   		return 0;
>   	}
>   
> -	return xe_pcode_read(root_tile, PCODE_MBOX(FAN_SPEED_CONTROL, subcmd, 0), uval, NULL);
> +	ret = xe_hwmon_pcode_read_fan_control(hwmon, FSC_READ_NUM_FANS, 0, &fan_mask);
> +	if (ret) {
> +		xe_warn(hwmon->xe, "failed to read number of fans, ret=%d\n", ret);
> +		return ret;
> +	}
> +
> +	*num_fans = min_t(u32, hweight32(fan_mask), FAN_MAX);
> +
> +	return 0;
> +}
> +
> +static int xe_hwmon_read_fan_control_info(struct xe_hwmon *hwmon)
> +{
> +	struct xe_tile *root_tile = xe_device_get_root_tile(hwmon->xe);
> +	int point;
> +	int fan;
> +	int ret;
> +
> +	for (fan = 0; fan < hwmon->num_fans; fan++) {
> +		u32 stock_fcp_count = 0;
> +
> +		struct xe_hwmon_fan_info *fi = &hwmon->fi[fan];
> +
> +		ret = xe_hwmon_pcode_read_fan_control(hwmon,
> +						      FSC_READ_STOCK_FAN_CONTROL_POINTS,
> +						      fan, &stock_fcp_count);
> +		if (ret) {
> +			xe_err(hwmon->xe,
> +			       "failed to read fan %d stock control point count, ret=%d\n",
> +			       fan, ret);
> +			return ret;
> +		}
> +		fi->fan_table[STOCK_FAN_TABLE].fan_control_point_count =
> +				min_t(u8, stock_fcp_count, MAX_FAN_CONTROL_POINTS);
> +
> +		xe_dbg(hwmon->xe, "fan %d stock points %u\n", fan,
> +		       fi->fan_table[STOCK_FAN_TABLE].fan_control_point_count);
> +
> +		/* Dump the stock fan control points for debugging purposes. */
> +		for (point = 0; point < fi->fan_table[STOCK_FAN_TABLE].fan_control_point_count;
> +		     point++) {
> +			u32 fcp = 0;
> +
> +			fcp = FCP_INDEX(STOCK_FAN_TABLE, point);
> +			ret = xe_pcode_read_timeout(root_tile,
> +						    PCODE_MBOX(FAN_SPEED_CONTROL,
> +							       FSC_READ_FAN_TABLE, fan),
> +						    &fcp, NULL, XE_PCODE_FAN_CONTROL_TIMEOUT_MS);
> +			if (ret) {
> +				xe_err(hwmon->xe, "failed to read fan %d stock point %d, ret=%d\n",
> +				       fan, point, ret);
> +				continue;
> +			}
> +
> +			/* Cache the stock fan control points in local structure for later use. */
> +			fi->fan_table[STOCK_FAN_TABLE].fcp[point].temp =
> +						REG_FIELD_GET(FAN_CONTROL_POINT_TEMP_MASK, fcp);
> +			fi->fan_table[STOCK_FAN_TABLE].fcp[point].speed =
> +						REG_FIELD_GET(FAN_CONTROL_POINT_SPEED_MASK, fcp);
> +			xe_dbg(hwmon->xe, "fan %d stock point %d: temp %u C, speed %u %%\n",
> +			       fan, point, fi->fan_table[STOCK_FAN_TABLE].fcp[point].temp,
> +			       fi->fan_table[STOCK_FAN_TABLE].fcp[point].speed);
> +		}
> +
> +		/* Read minimum fan PWM */
> +		ret = xe_hwmon_pcode_read_fan_control(hwmon, FSC_READ_FAN_MIN_PWM, fan,
> +						      &fi->min_pwm);
> +		if (ret) {
> +			xe_err(hwmon->xe, "failed to read fan %d min PWM, ret=%d\n", fan, ret);
> +			continue;
> +		}
> +
> +		xe_dbg(hwmon->xe, "fan %d min PWM %u\n", fan, fi->min_pwm);
> +	}
> +	return 0;
>   }
>   
>   static int xe_hwmon_power_curr_crit_read(struct xe_hwmon *hwmon, int channel,
> @@ -1279,17 +1401,12 @@ xe_hwmon_energy_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *val)
>   static umode_t
>   xe_hwmon_fan_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
>   {
> -	u32 uval = 0;
> -
>   	if (!hwmon->xe->info.has_fan_control)
>   		return 0;
>   
>   	switch (attr) {
>   	case hwmon_fan_input:
> -		if (xe_hwmon_pcode_read_fan_control(hwmon, FSC_READ_NUM_FANS, &uval))
> -			return 0;
> -
> -		return channel < uval ? 0444 : 0;
> +		return channel < hwmon->num_fans ? 0444 : 0;
>   	default:
>   		return 0;
>   	}
> @@ -1476,6 +1593,7 @@ xe_hwmon_get_preregistration_info(struct xe_hwmon *hwmon)
>   	u64 val_sku_unit = 0;
>   	int channel;
>   	struct xe_reg pkg_power_sku_unit;
> +	u32 num_fans = 0;
>   
>   	if (hwmon->xe->info.has_mbx_power_limits) {
>   		/* Check if GPU firmware support mailbox power limits commands. */
> @@ -1531,10 +1649,21 @@ xe_hwmon_get_preregistration_info(struct xe_hwmon *hwmon)
>   		if (xe_hwmon_is_visible(hwmon, hwmon_energy, hwmon_energy_input, channel))
>   			xe_hwmon_energy_get(hwmon, channel, &energy);
>   
> -	/* Initialize 'struct xe_hwmon_fan_info' with initial fan register reading. */
> -	for (channel = 0; channel < FAN_MAX; channel++)
> -		if (xe_hwmon_is_visible(hwmon, hwmon_fan, hwmon_fan_input, channel))
> -			xe_hwmon_fan_input_read(hwmon, channel, &fan_speed);
> +	if (hwmon->xe->info.has_fan_control) {
> +		xe_hwmon_get_num_fans(hwmon, &num_fans);
> +
> +		xe_info(hwmon->xe, "Number of fans detected: %u\n", num_fans);
> +		hwmon->num_fans = num_fans;
> +
> +		/* Initialize 'struct xe_hwmon_fan_info' with initial fan register reading. */
> +		for (channel = 0; channel < hwmon->num_fans; channel++)
> +			if (xe_hwmon_is_visible(hwmon, hwmon_fan, hwmon_fan_input, channel))
> +				xe_hwmon_fan_input_read(hwmon, channel, &fan_speed);
> +
> +		/* Fan control tables initialization */
> +		if (xe_hwmon_read_fan_control_info(hwmon))
> +			xe_warn(hwmon->xe, "Fan control tables are not available\n");

The table appears to be captured before late binding. Do we need 
separate pre-/post-LB tables, or should the table be updated again after 
late binding done?

Thanks,
Badal

> +	}
>   
>   	if (hwmon->xe->info.has_mbx_thermal_info && xe_hwmon_pcode_read_thermal_info(hwmon))
>   		drm_warn(&hwmon->xe->drm, "Thermal mailbox not supported by card firmware\n");
> diff --git a/drivers/gpu/drm/xe/xe_pcode_api.h b/drivers/gpu/drm/xe/xe_pcode_api.h
> index 94575c476e3d..669010f1e2d0 100644
> --- a/drivers/gpu/drm/xe/xe_pcode_api.h
> +++ b/drivers/gpu/drm/xe/xe_pcode_api.h
> @@ -85,6 +85,11 @@
>   
>   #define   FAN_SPEED_CONTROL			0x7D
>   #define     FSC_READ_NUM_FANS			0x4
> +#define     FSC_READ_STOCK_FAN_CONTROL_POINTS	0x5
> +#define     FSC_READ_FAN_TABLE			0x7
> +#define     FAN_CONTROL_POINT_TEMP_MASK		REG_GENMASK(7, 0)
> +#define     FAN_CONTROL_POINT_SPEED_MASK	REG_GENMASK(15, 8)
> +#define     FSC_READ_FAN_MIN_PWM		0x8
>   
>   #define PCODE_SCRATCH(x)		XE_REG(0x138320 + ((x) * 4))
>   /* PCODE_SCRATCH0 */