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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 10939 bytes --]

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@linux.intel.com> 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@kursatabayli.dev>
> >
> > 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;
> > >  }
> > >
>