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

[Radhey Kalra]

Yes, please!

On Wed, Jul 1, 2026 at 4:10 PM Ilpo Järvinen
<ilpo.jarvinen@xxxxxxxxxxxxxxx> wrote:
>
> On Thu, 11 Jun 2026, Radhey Kalra wrote:
>
> > 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 for testing. Do you want to give your Tested-by tag to be credited
> for you testing efforts?
>
> (Up to you, not mandatory, of course. The tag will become permanent public
> record in the commit.)
>
> --
>  i.
>
> > 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;
> > > >  }
> > > >
> >