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

["Poosa, Karthik" <karthik.poosa@intel.com>]

On 11-08-2026 18:41, Nilawar, Badal wrote:
>
> 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 <karthik.poosa@intel.com>
>> 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);
>
> Is stock fan table going to change after late bind? If yes, then I 
> will suggest to maintain one more table.
> Or Update the current one once late binding is done.
>
> Thanks,
> Badal

Yes, late binding overwrites the default stock table, and the updated 
table remains active until the device loses power.

I think updating the existing cached table after late binding completes 
is the better approach, rather than maintaining separate pre-/post-LB 
tables.

>
>> +        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");
>> +    }
>>         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 */