Re: [PATCH v3] platform/x86: hp-wmi: Add dual-channel PWM fan control for Victus S

[Radhey Kalra]

Hi Ilpo,

I tested Kürşat's v3 patch on my 8A3D machine:

  board_name: 8A3D
  product_name: Victus by HP Gaming Laptop 15-fb0xxx
  kernel: 7.0.11-1-cachyos-bore-lto

For the test I used current platform-drivers-x86/for-next plus the v3 fan
patch, with a temporary 8A3D DMI match added locally because 8A3D is not in
for-next yet.

The patched module exposed the expected interface:

  fan1_input=3800
  fan2_input=3000
  pwm1_enable=2
  pwm1=158
  pwm2=123

Manual mode also worked:

  write pwm1_enable=1
  fan1_input=3300
  fan2_input=2500
  pwm1_enable=1
  pwm1=145
  pwm2=109

Writing pwm1 and pwm2 separately affected the two fans independently:

  write pwm1=140
  fan1_input=3000
  fan2_input=2500
  pwm1=131
  pwm2=109

  write pwm2=160
  fan1_input=3000
  fan2_input=3500
  pwm1=131
  pwm2=153

Brief max mode and return to auto also worked:

  write pwm1_enable=0
  fan1_input=3900
  fan2_input=4500
  pwm1_enable=0

  write pwm1_enable=2
  fan1_input=3300
  fan2_input=3700
  pwm1_enable=2

I did not see a fan-control failure during the test.

Thanks,
Radhey

On Wed, Jun 10, 2026 at 3:00 PM Ilpo Järvinen
<ilpo.jarvinen@xxxxxxxxxxxxxxx> wrote:
>
> + Radhey
>
> On Sun, 31 May 2026, Kürşat Abaylı wrote:
>
> > Currently, manual fan control on supported Victus S models uses a single
> > PWM value for both CPU and GPU fans, linking their speeds via a hardcoded
> > gpu_delta offset. This prevents userspace tools from managing the thermal
> > profiles of the CPU and GPU independently.
> >
> > Refactor the hwmon implementation to support independent dual-channel
> > PWM control:
> > - Split the single 'pwm' state into 'cpu_pwm' and 'gpu_pwm'.
> > - Expose a second PWM channel ('pwm2') to userspace via hwmon_channel_info.
> > - Remove the gpu_delta mechanism entirely.
> >
> > The 'pwm1_enable' mode remains shared, as the underlying hardware does
> > not support per-fan modes. When switching to manual mode, both fans are
> > smoothly initialized to their current RPMs. Additionally, ensure that
> > the HP_FAN_SPEED_AUTOMATIC flag is isolated from rpm_to_pwm mathematical
> > interpolations during mode resets to prevent unintended fan states.
> >
> > Tested on: HP Victus 16-s0xxx
> >
> > Signed-off-by: Kürşat Abaylı <hello@xxxxxxxxxxxxxxxx>
>
> Has Radhey perhaps had an opportunity to test this patch?
>
> Radhey's series initially had the gpu delta init patch which was dropped
> because this change was anticipated. It would be nice to confirm if this
> patch successfully avoids the gpu delta init problem without introducing
> some other problems?
>
> --
>  i.
>
> > ---
> > Changes in v3:
> > - Addressed review feedback from Krishna Chomal:
> >   - Expanded ternary operators in hp_wmi_fan_speed_set to standard if-else blocks.
> >   - Replaced hardcoded channel numbers with CPU_FAN and GPU_FAN macros.
> >
> > Changes in v2:
> > - Made RPM readings atomic during the transition to manual mode.
> > - Fixed a variable scoping issue in the pwm_input block.
> > ---
> > ---
> >  drivers/platform/x86/hp/hp-wmi.c | 60 +++++++++++++++++---------------
> >  1 file changed, 32 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> > index f63bc00d9a9b..cef0c4436bd0 100644
> > --- a/drivers/platform/x86/hp/hp-wmi.c
> > +++ b/drivers/platform/x86/hp/hp-wmi.c
> > @@ -488,9 +488,9 @@ struct hp_wmi_hwmon_priv {
> >       struct mutex lock;      /* protects mode, pwm */
> >       u8 min_rpm;
> >       u8 max_rpm;
> > -     int gpu_delta;
> >       u8 mode;
> > -     u8 pwm;
> > +     u8 cpu_pwm;
> > +     u8 gpu_pwm;
> >       struct delayed_work keep_alive_dwork;
> >  };
> >
> > @@ -817,24 +817,20 @@ static int hp_wmi_fan_speed_max_set(int enabled)
> >       return enabled;
> >  }
> >
> > -static int hp_wmi_fan_speed_set(struct hp_wmi_hwmon_priv *priv, u8 speed)
> > +static int hp_wmi_fan_speed_set(struct hp_wmi_hwmon_priv *priv)
> >  {
> >       u8 fan_speed[2];
> > -     int gpu_speed, ret;
> > +     int ret;
> >
> > -     fan_speed[CPU_FAN] = speed;
> > -     fan_speed[GPU_FAN] = speed;
> > +     if (priv->cpu_pwm == HP_FAN_SPEED_AUTOMATIC)
> > +             fan_speed[CPU_FAN] = HP_FAN_SPEED_AUTOMATIC;
> > +     else
> > +             fan_speed[CPU_FAN] = pwm_to_rpm(priv->cpu_pwm, priv);
> >
> > -     /*
> > -      * GPU fan speed is always a little higher than CPU fan speed, we fetch
> > -      * this delta value from the fan table during hwmon init.
> > -      * Exception: Speed is set to HP_FAN_SPEED_AUTOMATIC, to revert to
> > -      * automatic mode.
> > -      */
> > -     if (speed != HP_FAN_SPEED_AUTOMATIC) {
> > -             gpu_speed = speed + priv->gpu_delta;
> > -             fan_speed[GPU_FAN] = clamp_val(gpu_speed, 0, U8_MAX);
> > -     }
> > +     if (priv->gpu_pwm == HP_FAN_SPEED_AUTOMATIC)
> > +             fan_speed[GPU_FAN] = HP_FAN_SPEED_AUTOMATIC;
> > +     else
> > +             fan_speed[GPU_FAN] = pwm_to_rpm(priv->gpu_pwm, priv);
> >
> >       ret = hp_wmi_get_fan_count_userdefine_trigger();
> >       if (ret < 0)
> > @@ -851,7 +847,9 @@ static int hp_wmi_fan_speed_set(struct hp_wmi_hwmon_priv *priv, u8 speed)
> >
> >  static int hp_wmi_fan_speed_reset(struct hp_wmi_hwmon_priv *priv)
> >  {
> > -     return hp_wmi_fan_speed_set(priv, HP_FAN_SPEED_AUTOMATIC);
> > +     priv->cpu_pwm = HP_FAN_SPEED_AUTOMATIC;
> > +     priv->gpu_pwm = HP_FAN_SPEED_AUTOMATIC;
> > +     return hp_wmi_fan_speed_set(priv);
> >  }
> >
> >  static int hp_wmi_fan_speed_max_reset(struct hp_wmi_hwmon_priv *priv)
> > @@ -2402,7 +2400,7 @@ static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
> >       case PWM_MODE_MANUAL:
> >               if (!is_victus_s_thermal_profile())
> >                       return -EOPNOTSUPP;
> > -             ret = hp_wmi_fan_speed_set(priv, pwm_to_rpm(priv->pwm, priv));
> > +             ret = hp_wmi_fan_speed_set(priv);
> >               if (ret < 0)
> >                       return ret;
> >               mod_delayed_work(system_wq, &priv->keep_alive_dwork,
> > @@ -2502,13 +2500,14 @@ static int hp_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
> >                             u32 attr, int channel, long val)
> >  {
> >       struct hp_wmi_hwmon_priv *priv;
> > -     int rpm;
> > +     int cpu_rpm, gpu_rpm;
> >
> >       priv = dev_get_drvdata(dev);
> >       guard(mutex)(&priv->lock);
> >       switch (type) {
> >       case hwmon_pwm:
> >               if (attr == hwmon_pwm_input) {
> > +                     int rpm;
> >                       if (!is_victus_s_thermal_profile())
> >                               return -EOPNOTSUPP;
> >                       /* PWM input is invalid when not in manual mode */
> > @@ -2518,7 +2517,10 @@ static int hp_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
> >                       /* ensure PWM input is within valid fan speeds */
> >                       rpm = pwm_to_rpm(val, priv);
> >                       rpm = clamp_val(rpm, priv->min_rpm, priv->max_rpm);
> > -                     priv->pwm = rpm_to_pwm(rpm, priv);
> > +                     if (channel == CPU_FAN)
> > +                             priv->cpu_pwm = rpm_to_pwm(rpm, priv);
> > +                     else if (channel == GPU_FAN)
> > +                             priv->gpu_pwm = rpm_to_pwm(rpm, priv);
> >                       return hp_wmi_apply_fan_settings(priv);
> >               }
> >               switch (val) {
> > @@ -2532,10 +2534,14 @@ static int hp_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
> >                        * When switching to manual mode, set fan speed to
> >                        * current RPM values to ensure a smooth transition.
> >                        */
> > -                     rpm = hp_wmi_get_fan_speed_victus_s(channel);
> > -                     if (rpm < 0)
> > -                             return rpm;
> > -                     priv->pwm = rpm_to_pwm(rpm / 100, priv);
> > +                     cpu_rpm = hp_wmi_get_fan_speed_victus_s(CPU_FAN);
> > +                     if (cpu_rpm < 0)
> > +                             return cpu_rpm;
> > +                     gpu_rpm = hp_wmi_get_fan_speed_victus_s(GPU_FAN);
> > +                     if (gpu_rpm < 0)
> > +                             return gpu_rpm;
> > +                     priv->cpu_pwm = rpm_to_pwm(cpu_rpm / 100, priv);
> > +                     priv->gpu_pwm = rpm_to_pwm(gpu_rpm / 100, priv);
> >                       priv->mode = PWM_MODE_MANUAL;
> >                       return hp_wmi_apply_fan_settings(priv);
> >               case PWM_MODE_AUTO:
> > @@ -2551,7 +2557,7 @@ static int hp_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
> >
> >  static const struct hwmon_channel_info * const info[] = {
> >       HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT, HWMON_F_INPUT),
> > -     HWMON_CHANNEL_INFO(pwm, HWMON_PWM_ENABLE | HWMON_PWM_INPUT),
> > +     HWMON_CHANNEL_INFO(pwm, HWMON_PWM_ENABLE | HWMON_PWM_INPUT, HWMON_PWM_INPUT),
> >       NULL
> >  };
> >
> > @@ -2592,7 +2598,7 @@ static int hp_wmi_setup_fan_settings(struct hp_wmi_hwmon_priv *priv)
> >       struct victus_s_fan_table *fan_table;
> >       u8 min_rpm, max_rpm;
> >       u8 cpu_rpm, gpu_rpm, noise_db;
> > -     int gpu_delta, i, num_entries, ret;
> > +     int i, num_entries, ret;
> >       size_t header_size, entry_size;
> >
> >       /* Default behaviour on hwmon init is automatic mode */
> > @@ -2638,10 +2644,8 @@ static int hp_wmi_setup_fan_settings(struct hp_wmi_hwmon_priv *priv)
> >       if (min_rpm == U8_MAX || max_rpm == 0)
> >               return -EINVAL;
> >
> > -     gpu_delta = fan_table->entries[0].gpu_rpm - fan_table->entries[0].cpu_rpm;
> >       priv->min_rpm = min_rpm;
> >       priv->max_rpm = max_rpm;
> > -     priv->gpu_delta = gpu_delta;
> >
> >       return 0;
> >  }
> >