Re: [PATCH v2 4/9] drm/xe/hwmon: expose pwm[1-3]

On 24-07-2026 12:32, Purkait, Soham wrote:
> Hi Karthik,
>
> On 17-07-2026 09:47, Karthik Poosa wrote:
>> Expose pwm[1-3] fan duty attributes through hwmon.
>>
>> This enables reading and writing user PWM values (0..255) for each
>> available fan channel.
>>
>> Update Xe hwmon ABI documentation for pwm[1-3].
>>
>> This sysfs node can be used for manual control of fan speed,
>> irrespective of fan curve.
>>
>> v2:
>>   - Use xe helpers for dmesg logs.
>>   - Avoid use hwmon->num_fans to get fan count in 
>> xe_hwmon_pwm_is_visible().
>>
>> Signed-off-by: Karthik Poosa <[email protected]>
>> Assisted-by: Codex:gpt-5-4
>> ---
>>   .../ABI/testing/sysfs-driver-intel-xe-hwmon   |   9 +
>>   drivers/gpu/drm/xe/xe_hwmon.c                 | 230 ++++++++++++++++++
>>   drivers/gpu/drm/xe/xe_pcode_api.h             |   3 +
>>   3 files changed, 242 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-xe-hwmon 
>> b/Documentation/ABI/testing/sysfs-driver-intel-xe-hwmon
>> index ec0b94d76e22..7383898890aa 100644
>> --- a/Documentation/ABI/testing/sysfs-driver-intel-xe-hwmon
>> +++ b/Documentation/ABI/testing/sysfs-driver-intel-xe-hwmon
>> @@ -321,3 +321,12 @@ Description:    RW. Package burst power limit 
>> interval (Tau in PL2/Tau) in
>>           milliseconds over which sustained power is averaged.
>>             Only supported for particular Intel Xe graphics platforms.
>> +
>> +What: /sys/bus/pci/drivers/xe/.../hwmon/hwmon<i>/pwm[1-3]
>> +Date:           July 2026
>> +KernelVersion:  7.2
>> +Contact: [email protected]
>> +Description:    RW. Target fan PWM duty value in the range 0..255. 0 
>> is zero
>> +                fan speed and 255 is 100% fan speed.
>> +
>> +                Only supported for particular Intel Xe graphics 
>> platforms.
>> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c 
>> b/drivers/gpu/drm/xe/xe_hwmon.c
>> index 769f4d1da83e..d3379727b1a9 100644
>> --- a/drivers/gpu/drm/xe/xe_hwmon.c
>> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
>> @@ -803,6 +803,7 @@ static const struct hwmon_channel_info * const 
>> hwmon_info[] = {
>>       HWMON_CHANNEL_INFO(energy, HWMON_E_INPUT | HWMON_E_LABEL, 
>> HWMON_E_INPUT | HWMON_E_LABEL),
>>       HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT | HWMON_F_MAX, 
>> HWMON_F_INPUT | HWMON_F_MAX,
>>                  HWMON_F_INPUT | HWMON_F_MAX),
>> +    HWMON_CHANNEL_INFO(pwm, HWMON_PWM_INPUT, HWMON_PWM_INPUT, 
>> HWMON_PWM_INPUT),
>>       NULL
>>   };
>>   @@ -928,6 +929,158 @@ static int xe_hwmon_get_num_fans(const struct 
>> xe_hwmon *hwmon, u32 *num_fans)
>>       return 0;
>>   }
>>   +static int xe_hwmon_get_fan_point_count(struct xe_hwmon *hwmon, u8 
>> fan, u32 *point_count,
>> +                    int table_type)
>> +{
>> +    int ret;
>> +
>> +    ret = xe_hwmon_pcode_read_fan_control(hwmon,
>> +                          (table_type == USER_FAN_TABLE) ?
>> +                          FSC_READ_USER_FAN_CONTROL_POINTS :
>> +                          FSC_READ_STOCK_FAN_CONTROL_POINTS,
>> +                          fan, point_count);
>> +    if (ret) {
>> +        xe_err(hwmon->xe, "failed to read fan %d %s point count, 
>> ret=%d\n", fan,
>> +               (table_type == USER_FAN_TABLE) ? "user" : "stock", ret);
>> +        return ret;
>> +    }
>> +
>> +    xe_dbg(hwmon->xe, "fan %d %s point count read as %u\n", fan,
>> +           (table_type == USER_FAN_TABLE) ? "user" : "stock", 
>> *point_count);
>> +
>> +    return 0;
>> +}
>> +
>> +static int xe_hwmon_write_user_fan_point(struct xe_hwmon *hwmon, u8 
>> fan, u8 point, u8 temp,
>> +                     u8 speed)
>> +{
>> +    struct xe_tile *root_tile = xe_device_get_root_tile(hwmon->xe);
>> +    u16 user_fcp_raw = FIELD_PREP(FAN_CONTROL_POINT_TEMP_MASK, temp) |
>> +             FIELD_PREP(FAN_CONTROL_POINT_SPEED_MASK, speed);
>> +    int ret;
>> +
>> +    ret = xe_pcode_write_timeout(root_tile,
>> +                     PCODE_MBOX(FAN_SPEED_CONTROL, 
>> FSC_WRITE_FAN_TABLE, fan),
>> +                    (u32)user_fcp_raw, 
>> XE_PCODE_FAN_CONTROL_TIMEOUT_MS);
>> +    if (ret)
>> +        xe_dbg(hwmon->xe,
>> +               "failed to write fan %d user point %d temp %u, speed 
>> %u %%, ret=%d\n",
>> +               fan, point, temp, speed, ret);
>> +
>> +    return ret;
>> +}
>> +
>> +static int xe_hwmon_activate_user_fan_table(struct xe_hwmon *hwmon, 
>> u8 fan,
>> +                        bool is_full_speed, enum fan_table_type 
>> table_source)
>> +{
>> +    struct xe_tile *root_tile = xe_device_get_root_tile(hwmon->xe);
>> +    struct xe_hwmon_fan_info *fi = &hwmon->fi[fan];
>> +    u32 point_count;
>> +    int ret;
>> +    int point;
>> +    struct fan_table *source_table = &fi->fan_table[table_source];
>> +
>> +    xe_dbg(hwmon->xe, "activating fan %d user table from %s 
>> table\n", fan,
>> +           (table_source == USER_FAN_TABLE) ? "user" : "stock");
>> +
>> +    point_count = source_table->fan_control_point_count;
>> +
>> +    if (!point_count) {
>> +        xe_err(hwmon->xe,
>> +               "fan %d %s table point count is 0, cannot activate 
>> user table\n", fan,
>> +               (table_source == USER_FAN_TABLE) ? "user" : "stock");
>> +        return -ENODATA;
>> +    }
>> +
>> +    for (point = 0; point < point_count; point++) {
>> +        u8 temp = source_table->fcp[point].temp;
>> +        u8 speed = is_full_speed ? 100 : max_t(u8, 
>> source_table->fcp[point].speed,
>> +                        min_t(u32, fi->min_pwm, U8_MAX));
>> +
>> +        ret = xe_hwmon_write_user_fan_point(hwmon, fan, point, temp, 
>> speed);
>> +        if (ret)
>> +            return ret;
>> +
>> +        if (table_source == STOCK_FAN_TABLE) {
>> +            /* Update user table cache with stock table values */
>> +            fi->fan_table[USER_FAN_TABLE].fcp[point].temp = temp;
>> +            fi->fan_table[USER_FAN_TABLE].fcp[point].speed = speed;
>> +        }
>> +        if (is_full_speed)
>> +            fi->fan_table[USER_FAN_TABLE].fcp[point].speed = speed;
>> +    }
>> +
>> +    ret = xe_pcode_write_timeout(root_tile,
>> +                     PCODE_MBOX(FAN_SPEED_CONTROL,
>> +                        FSC_WRITE_NUM_FAN_CONTROL_POINTS, fan),
>> +                     point_count, XE_PCODE_FAN_CONTROL_TIMEOUT_MS);
>> +    if (ret) {
>> +        xe_dbg(hwmon->xe, "failed to write fan %d user table, 
>> ret=%d\n", fan, ret);
>> +        return ret;
>> +    }
>> +
>> +    /* Verify if all the user points are set */
>> +    ret = xe_hwmon_get_fan_point_count(hwmon, fan, &point_count, 
>> USER_FAN_TABLE);
>> +    if (ret) {
>> +        xe_err(hwmon->xe,
>> +               "failed to read fan %d user table point count, 
>> ret=%d\n", fan, ret);
>> +        return ret;
>> +    }
>> +    if (point_count != source_table->fan_control_point_count) {
>> +        xe_err(hwmon->xe, "fan %d user table point count mismatch, 
>> expected %u, got %u\n",
>> +               fan, source_table->fan_control_point_count, 
>> point_count);
>> +        return -EIO;
>> +    }
>> +
>> +    fi->fan_table[USER_FAN_TABLE].fan_control_point_count = 
>> point_count;
>> +
>> +    return 0;
>> +}
>> +
>> +static int xe_hwmon_set_user_fan_pwm(struct xe_hwmon *hwmon, u8 fan, 
>> u8 pwm)
>> +{
>> +    struct xe_tile *root_tile = xe_device_get_root_tile(hwmon->xe);
>> +    struct xe_hwmon_fan_info *fi = &hwmon->fi[fan];
>> +    u32 point_count = 
>> fi->fan_table[USER_FAN_TABLE].fan_control_point_count;
>> +    u8 clamped_pwm;
>> +    int ret;
>> +    int point;
>> +
>> +    /* Read user table fan point count, if it is not set, activate 
>> the user table */
>> +    ret = xe_hwmon_get_fan_point_count(hwmon, fan, &point_count, 
>> USER_FAN_TABLE);
>> +    if (ret)
>> +        return ret;
>> +
>> +    if (!point_count) {
>> +        xe_dbg(hwmon->xe, "fan %d user table not set, activating 
>> it\n", fan);
>> +        ret = xe_hwmon_activate_user_fan_table(hwmon, fan, false, 
>> STOCK_FAN_TABLE);
>> +        if (ret)
>> +            return ret;
>> +    }
>> +
>> +    pwm = DIV_ROUND_CLOSEST(pwm * 100, U8_MAX);
>> +    clamped_pwm = max_t(u8, pwm, min_t(u32, fi->min_pwm, U8_MAX));
> What is the use of this clamping logic? as the pwm is in percentage 
> already.

Clamping ensures that the PWM value requested by the user is not lower 
than the minimum PWM supported by the hardware, preventing fan buzzing 
or stalling.

>> +
>> +    for (point = 0; point < point_count; point++) {
>> +        u8 temp = fi->fan_table[USER_FAN_TABLE].fcp[point].temp;
>> +
>> +        ret = xe_hwmon_write_user_fan_point(hwmon, fan, point, temp, 
>> clamped_pwm);
>> +        if (ret)
>> +            return ret;
>> +
>> +        fi->fan_table[USER_FAN_TABLE].fcp[point].speed = clamped_pwm;
>> +    }
>> +
>> +    ret = xe_pcode_write(root_tile,
>> +                 PCODE_MBOX(FAN_SPEED_CONTROL, 
>> FSC_WRITE_NUM_FAN_CONTROL_POINTS, fan),
>> +                 point_count);
>> +    if (ret) {
>> +        xe_dbg(hwmon->xe, "failed to update fan %d user table count, 
>> ret=%d\n", fan, ret);
>> +        return ret;
>> +    }
>> +    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);
>> @@ -1424,6 +1577,20 @@ xe_hwmon_fan_is_visible(struct xe_hwmon 
>> *hwmon, u32 attr, int channel)
>>       }
>>   }
>>   +static umode_t
>> +xe_hwmon_pwm_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
>> +{
>> +    if (!hwmon->xe->info.has_fan_control || channel >= hwmon->num_fans)
>> +        return 0;
>> +
>> +    switch (attr) {
>> +    case hwmon_pwm_input:
>> +        return 0644;
>> +    default:
>> +        return 0;
>> +    }
>> +}
>> +
>>   static int
>>   xe_hwmon_fan_input_read(struct xe_hwmon *hwmon, int channel, long 
>> *val)
>>   {
>> @@ -1478,6 +1645,62 @@ xe_hwmon_fan_read(struct xe_hwmon *hwmon, u32 
>> attr, int channel, long *val)
>>       }
>>   }
>>   +static int
>> +xe_hwmon_pwm_read(struct xe_hwmon *hwmon, u32 attr, int channel, 
>> long *val)
>> +{
>> +    struct xe_hwmon_fan_info *fi;
>> +    int ret = 0;
>> +
>> +    if (channel < 0 || channel >= FAN_MAX)
>> +        return -EINVAL;
>> +
>> +    fi = &hwmon->fi[channel];
>> +
>> +    switch (attr) {
>> +    case hwmon_pwm_input:
>> +        /* Check if user fan table is set else activate it.*/
>> +        if (!fi->fan_table[USER_FAN_TABLE].fan_control_point_count) {
>> +            xe_dbg(hwmon->xe, "fan %d user table not set, activating 
>> it\n", channel);
>> +            ret = xe_hwmon_activate_user_fan_table(hwmon, channel, 
>> false,
>> +                                   STOCK_FAN_TABLE);
>> +            if (ret)
>> +                return ret;
>> +        }
>> +        *val = 
>> DIV_ROUND_CLOSEST(fi->fan_table[USER_FAN_TABLE].fcp[0].speed *
>> +                        U8_MAX,    100);
>> +        return 0;
>> +    default:
>> +        return -EOPNOTSUPP;
>> +    }
>> +}
>> +
>> +static int
>> +xe_hwmon_pwm_write(struct xe_hwmon *hwmon, u32 attr, int channel, 
>> long val)
>> +{
>> +    int ret = 0;
>> +
>> +    if (channel < 0 || channel >= FAN_MAX)
>> +        return -EINVAL;
>> +
>> +    mutex_lock(&hwmon->hwmon_lock);
>> +
>> +    switch (attr) {
>> +    case hwmon_pwm_input:
>> +        if (val < 0 || val > U8_MAX) {
>> +            ret = -EINVAL;
>> +            break;
>> +        }
>> +        ret = xe_hwmon_set_user_fan_pwm(hwmon, channel, (u8)val);
>> +        break;
>> +    default:
>> +        ret = -EOPNOTSUPP;
>> +        break;
>> +    }
>> +    mutex_unlock(&hwmon->hwmon_lock);
>> +
>> +    return ret;
>> +}
>> +
>>   static umode_t
>>   xe_hwmon_is_visible(const void *drvdata, enum hwmon_sensor_types type,
>>               u32 attr, int channel)
>> @@ -1504,6 +1727,9 @@ xe_hwmon_is_visible(const void *drvdata, enum 
>> hwmon_sensor_types type,
>>       case hwmon_fan:
>>           ret = xe_hwmon_fan_is_visible(hwmon, attr, channel);
>>           break;
>> +    case hwmon_pwm:
>> +        ret = xe_hwmon_pwm_is_visible(hwmon, attr, channel);
>> +        break;
>>       default:
>>           ret = 0;
>>           break;
>> @@ -1533,6 +1759,8 @@ xe_hwmon_read(struct device *dev, enum 
>> hwmon_sensor_types type, u32 attr,
>>           return xe_hwmon_energy_read(hwmon, attr, channel, val);
>>       case hwmon_fan:
>>           return xe_hwmon_fan_read(hwmon, attr, channel, val);
>> +    case hwmon_pwm:
>> +        return xe_hwmon_pwm_read(hwmon, attr, channel, val);
>>       default:
>>           return -EOPNOTSUPP;
>>       }
>> @@ -1551,6 +1779,8 @@ xe_hwmon_write(struct device *dev, enum 
>> hwmon_sensor_types type, u32 attr,
>>           return xe_hwmon_power_write(hwmon, attr, channel, val);
>>       case hwmon_curr:
>>           return xe_hwmon_curr_write(hwmon, attr, channel, val);
>> +    case hwmon_pwm:
>> +        return xe_hwmon_pwm_write(hwmon, attr, channel, val);
>>       default:
>>           return -EOPNOTSUPP;
>>       }
>> diff --git a/drivers/gpu/drm/xe/xe_pcode_api.h 
>> b/drivers/gpu/drm/xe/xe_pcode_api.h
>> index 419ab4f416fa..5c5fdc650a97 100644
>> --- a/drivers/gpu/drm/xe/xe_pcode_api.h
>> +++ b/drivers/gpu/drm/xe/xe_pcode_api.h
>> @@ -84,9 +84,12 @@
>>   #define     PCODE_MBOX_DOMAIN_HBM        0x2
>>     #define   FAN_SPEED_CONTROL            0x7D
>> +#define     FSC_WRITE_NUM_FAN_CONTROL_POINTS    0x0
>> +#define     FSC_WRITE_FAN_TABLE            0x1
>>   #define     FSC_READ_MAX_FAN_RPS        0x3
>>   #define     FSC_READ_NUM_FANS            0x4
>>   #define     FSC_READ_STOCK_FAN_CONTROL_POINTS    0x5
>> +#define     FSC_READ_USER_FAN_CONTROL_POINTS    0x6
>>   #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)