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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On Tue, Sep 28 2021 at 14:03:54 +0200, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
Hi,

On 9/28/21 1:59 PM, Luke Jones wrote:


On Tue, Sep 28 2021 at 13:56:05 +0200, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
 Hi,

 On 9/28/21 1:43 PM, Luke Jones wrote:
Sure, the path is similar to /sys/devices/platform/asus-nb-wmi/hwmon/hwmon4/pwm1_enable where hwmon4 will likely be different depending on init, and pwm2_enable is the second fan if it exists. The values are 1,2,3 - where 1 = fan curve enabled/manual, 2 = auto. 3 here is custom extra that writes default curve back then defaults to 2.

As I understand it, this should be adhering to the accepted kernel standard, so if you use this for ASUS laptops, then it should carry over to other brands that implement it also.

 Ah, so this is a bit different then how I thought this would work
 (this is probably better though).

 <snip>

   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.

 Quoting Documentation/hwmon/sysfs-interface.rst

 `pwm[1-*]_enable`
                 Fan speed control method:

                 - 0: no fan speed control (i.e. fan at full speed)
- 1: manual fan speed control enabled (using `pwm[1-*]`)
                 - 2+: automatic fan speed control enabled

1 normally means the fans runs at a fixed speed, but you are using it
 for the custom/manual profile, which is still a temp<->pwm table,
 right?

 I guess this make sense since full manual control is not supported
 and this keeps "2" aka auto as being the normal factory auto
 setting which is good.

 Bastien is this interface usable for p-p-d ?

 I guess that it is a bit annoying that you need to figure out
 the # in the hwmon# part of the path, but there will be only
 one hwmon child.

 You could also go through /sys/class/hwmon but then you really
 have no idea which one to use. Ideally we would have some way
 to indicate that there is a hmwon class-dev associated with
 /sys/firmware/acpi/platform_profile but as we mentioned before
 we should defer coming up with a generic solution for this
 until we have more then 1 user, so that we hopefully get the
 generic solution right in one go.

If it's at all helpful, I named the interface as "asus_custom_fan_curve". I use this to verify I have the correct hwmon for asusctl. Open to suggestions on that.

Ah yes, that means the interface could be looked up through /sys/class/hwmon
too, that is probably the safest route to go then, as the
/sys/devices/platform/asus-nb-wmi/ path might change if we e.g. ever
convert the asus-wmi code to use the new wmi bus interface.

Oh man... can you link me to relevant bits? I'll stick this on my to-do for future. There will be more patches from me over time so this might be good to have done?


Now that you have confirmed that any writes to
/sys/firmware/acpi/platform_profile override any custom profiles
I wonder if p-p-d needs to take this into account at all though.

The easiest way to deal with this in p-p-d, is just to not deal
with it at all...    If that turns out to be a bad idea, we
can always reconsider and add some special handling to p-p-d for
this later.

I believe Bastiens concern here will be that manual control can still be enabled and ppd won't be aware of it without a check. For example the user may switch profile then re-enable the fan-curve. If some problem arises due to manual control of fan then there is no way for ppd to determine if manual was enabled without actually looking.

This does mean the pwm name here is set in stone. But also means it's special-cased I guess. Perhaps a check for pwm<N>_enable, then check for the pairs of pwm<N>_auto_<xX> that come with it?

Ciao,
Luke.


Regards,

Hans






[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux