Re: [PATCH v11] asus-wmi: Add support for custom fan curves

[Luke Jones]

Hmm,
A change via /sys/firmware/acpi/platform_profile does disable the 
fan-curve until a user re-enables it.
It doesn't wipe the actual curve setting though, I thought that would 
be bad UX, but yes the curve is definitely disabled on profile change 
and will remain disabled until turned on again. At which point another 
profile change will disable it again.

And as stated in previous reply use of 
/sys/devices/platform/asus-nb-wmi/hwmon/hwmon<N>/pwm<N>_enable to check 
the status is stabilised (1 = manual fan).

Looking at it with fresh eyes I just spotted some things I can clean up 
further. Very sorry, there'll be a v15 :(

Many thanks,
Luke.


On Tue, Sep 28 2021 at 13:44:32 +0200, Hans de Goede 
<hdegoede@xxxxxxxxxx> wrote:
> Hi,
>
> On 9/28/21 1:36 PM, Bastien Nocera wrote:
> >  On Wed, 2021-09-08 at 11:22 +1200, Luke D. Jones wrote:
> > >  Add support for custom fan curves found on some ASUS ROG laptops.
> > >
> > >  These laptops have the ability to set a custom curve for the CPU
> > >  and GPU fans via two ACPI methods.
> > >
> > >  This patch adds two pwm<N> attributes to the hwmon sysfs,
> > >  pwm1 for CPU fan, pwm2 for GPU fan. Both are under the hwmon of the
> > >  name `asus_custom_fan_curve`. There is no safety check of the set
> > >  fan curves - this must be done in userspace.
> > >
> > >  The fans have settings [1,2,3] under pwm<N>_enable:
> > >  1. Enable and write settings out
> > >  2. Disable and use factory fan mode
> > >  3. Same as 2, additionally restoring default factory curve.
> > >
> > >  Use of 2 means that the curve the user has set is still stored and
> > >  won't be erased, but the laptop will be using its default auto-fan
> > >  mode. Re-enabling the manual mode then activates the curves again.
> > >
> > >  Notes:
> > >  - pwm<N>_enable = 0 is an invalid setting.
> > >  - pwm is actually a percentage and is scaled on writing to device.
> >
> >  I was trying to update:
> >  
> > https://gitlab.freedesktop.org/hadess/power-profiles-daemon/-/merge_requests/80
> >  but I don't understand what files I need to check for what values to
> >  detect whether custom fan curves were used.
> >
> >  Can you help me out here?
>
> How to deal with this is actually one of my remaining questions too.
>
> I've not looked at the new code closely yet, but if I understand
> things correctly, the now code basically only allows to set 1
> custom profile and setting that profile overrides the last
> profile set through /sys/firmware/acpi/platform_profile.
>
> And any write to /sys/firmware/acpi/platform_profile will
> overwrite / replace the last custom set profile (if any) with
> the one matching the requested platform-profile.
>
> So basically users of custom fan profiles are expected to
> disable power-profiles-daemon or at least to refrain from
> making any platform_profile changes.
>
> And if power-profile-daemon is actually active and
> makes a change then any custom settings will be thrown away,
> IOW p-p-d will always win. So I believe that it no longer needs
> to check for custom profiles, since any time it requests a
> standard profile that will overwrite any custom profile
> which may be present.
>
> Luke, do I have that right ?
>
> >  Also, was this patch accepted in the pdx86 tree?
>
> No, I still need to find/make some time to review it and
> I still have the same question as you :)
>
> Regards,
>
> Hans