On Mon, Jan 15 2024 at 13:38:16 +01:00:00, Hans de Goede
<hdegoede@xxxxxxxxxx> wrote:
Hi,
On 1/15/24 13:22, Andrei Sabalenka wrote:
When changing throttle_thermal_policy, all the custom fan curves
are getting disabled. This patch re-enables all the custom fan
curves that were enabled before changing throttle_thermal_policy.
I believe it makes asus-wmi sysfs interface more convenient, as it
allows userspace to manage fan curves independently from
platform_profile and throttle_thermal_policy. At the kernel level,
custom fan curves should not be tied to "power profiles" scheme in
any way, as it gives the user less freedom of controlling them.
Setting a high performance power-profile typically also involves
ramping up
the fans harder. So I don't think this patch is a good idea.
If you really want this behavior then you can always re-enable the
custom
curve after changing the profile.
Luke, do you have any opinion on this?
I see some misconceptions that should be addressed:
1. ASUS themselves set separate fan curves per "platform profile", both
standard and custom
2. fan curves are not tied to platform profiles, they are tied to the
throttle_thermal_policy, and this is actually done in the acpi - so the
code here is a mirror of that
3. platform-profiles are tied to throttle_thermal_policy
There is no lack of user control at all, a decent tool (like asusctl)
can set fan curves without issues but it's perhaps not convenient for
manually setting via a script etc.
The main reason that a curve is disabled for the policy being switched
to is for safety. It was a paranoid choice I made at the time. The
kernel (and acpi) can't guarantee that a user set a reasonable default
for that policy so the safest thing is to force an explicit re-enable
of it.
Having said that: I know that the curve was previously set for that
profile/policy and in theory should be fine plus it is already used by
the user, it is also not possible to set a curve for a different
profile to the one a user is currently in - this is forced in ACPI as
you can set only the curve for the profile you are in (the kernel code
also mirrors this).
So this patch should be fine.
Signed-off-by: Luke D. Jones <luke@xxxxxxxxxx>
Signed-off-by: Andrei Sabalenka <mechakotik@xxxxxxxxx>
---
drivers/platform/x86/asus-wmi.c | 29 ++++++++++++++++++++++-------
1 file changed, 22 insertions(+), 7 deletions(-)
diff --git a/drivers/platform/x86/asus-wmi.c
b/drivers/platform/x86/asus-wmi.c
index 18be35fdb..c2e38f6d8 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -3441,13 +3441,28 @@ static int
throttle_thermal_policy_write(struct asus_wmi *asus)
return -EIO;
}
- /* Must set to disabled if mode is toggled */
- if (asus->cpu_fan_curve_available)
- asus->custom_fan_curves[FAN_CURVE_DEV_CPU].enabled = false;
- if (asus->gpu_fan_curve_available)
- asus->custom_fan_curves[FAN_CURVE_DEV_GPU].enabled = false;
- if (asus->mid_fan_curve_available)
- asus->custom_fan_curves[FAN_CURVE_DEV_MID].enabled = false;
+ /* Re-enable fan curves after profile change */
+ if (asus->cpu_fan_curve_available &&
asus->custom_fan_curves[FAN_CURVE_DEV_CPU].enabled) {
+ err = fan_curve_write(asus,
&asus->custom_fan_curves[FAN_CURVE_DEV_CPU]);
+ if (err) {
+ pr_warn("Failed to re-enable CPU fan curve: %d\n", err);
+ return err;
+ }
+ }
+ if (asus->gpu_fan_curve_available &&
asus->custom_fan_curves[FAN_CURVE_DEV_GPU].enabled) {
+ err = fan_curve_write(asus,
&asus->custom_fan_curves[FAN_CURVE_DEV_GPU]);
+ if (err) {
+ pr_warn("Failed to re-enable GPU fan curve: %d\n", err);
+ return err;
+ }
+ }
+ if (asus->mid_fan_curve_available &&
asus->custom_fan_curves[FAN_CURVE_DEV_MID].enabled) {
+ err = fan_curve_write(asus,
&asus->custom_fan_curves[FAN_CURVE_DEV_MID]);
+ if (err) {
+ pr_warn("Failed to re-enable MID fan curve: %d\n", err);
+ return err;
+ }
+ }
return 0;
}
