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

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 9/28/21 1:56 PM, Luke Jones wrote:
> Hmm,
> A change via /sys/firmware/acpi/platform_profile does disable the fan-curve until a user re-enables it.

Ah ok, so did get that part right :)

So basically any write to /sys/firmware/acpi/platform_profile
will reset the pwm1_enable to "2" if it was not "2" already.

> It doesn't wipe the actual curve setting though, I thought that would be bad UX,

Ok that is fine.

> 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 :(

No worries, maybe wait a bit with posting v15 till Bastien has a chance
to way in on this discussion though.

Regards,

Hans



> On Tue, Sep 28 2021 at 13:44:32 +0200, Hans de Goede <hdegoede@redhat.com> 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
>>
> 
>