Re: [PATCH] platform/x86: asus-wmi: Re-enable custom fan curves after setting throttle_thermal_policy

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


On Mon, Jan 15 2024 at 13:38:16 +01:00:00, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:

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;

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

  Powered by Linux