[PATCH] platform/x86: asus-wmi: change quiet to low-power

[PATCH] platform/x86: asus-wmi: change quiet to low-power

[Luke Jones]

From: "Luke D. Jones" <luke@ljones.dev>

Change the profile name "quiet" to "low-power" to match the AMD name. The
primary reason for this is to match AMD naming for platform_profiles and
allow both to match. It does not affect Intel machines.

The quiet profile is essentially a low-power profile which tweaks
both TDP and fans - this applies to 80+ ASUS laptops.

Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
 drivers/platform/x86/asus-wmi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index d22748f1e154..de19c3b3d8fb 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -3945,7 +3945,7 @@ static int asus_wmi_platform_profile_get(struct device *dev,
 		*profile = PLATFORM_PROFILE_PERFORMANCE;
 		break;
 	case ASUS_THROTTLE_THERMAL_POLICY_SILENT:
-		*profile = PLATFORM_PROFILE_QUIET;
+		*profile = PLATFORM_PROFILE_LOW_POWER;
 		break;
 	default:
 		return -EINVAL;
@@ -3969,7 +3969,7 @@ static int asus_wmi_platform_profile_set(struct device *dev,
 	case PLATFORM_PROFILE_BALANCED:
 		tp = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT;
 		break;
-	case PLATFORM_PROFILE_QUIET:
+	case PLATFORM_PROFILE_LOW_POWER:
 		tp = ASUS_THROTTLE_THERMAL_POLICY_SILENT;
 		break;
 	default:
@@ -3982,7 +3982,7 @@ static int asus_wmi_platform_profile_set(struct device *dev,
 
 static int asus_wmi_platform_profile_probe(void *drvdata, unsigned long *choices)
 {
-	set_bit(PLATFORM_PROFILE_QUIET, choices);
+	set_bit(PLATFORM_PROFILE_LOW_POWER, choices);
 	set_bit(PLATFORM_PROFILE_BALANCED, choices);
 	set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
 
-- 
2.48.1

Re: [PATCH] platform/x86: asus-wmi: change quiet to low-power

[Mario Limonciello]

On 2/24/2025 16:35, Luke Jones wrote:
> From: "Luke D. Jones" <luke@ljones.dev>
> 
> Change the profile name "quiet" to "low-power" to match the AMD name. The
> primary reason for this is to match AMD naming for platform_profiles and
> allow both to match. It does not affect Intel machines.
> 
> The quiet profile is essentially a low-power profile which tweaks
> both TDP and fans - this applies to 80+ ASUS laptops.
> 
> Signed-off-by: Luke D. Jones <luke@ljones.dev>

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

IMO - this should have a fixes tag since this should probably go in the 
6.14 cycle too.

Fixes: 688834743d67 ("ACPI: platform_profile: Allow multiple handlers")

> ---
>   drivers/platform/x86/asus-wmi.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index d22748f1e154..de19c3b3d8fb 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -3945,7 +3945,7 @@ static int asus_wmi_platform_profile_get(struct device *dev,
>   		*profile = PLATFORM_PROFILE_PERFORMANCE;
>   		break;
>   	case ASUS_THROTTLE_THERMAL_POLICY_SILENT:
> -		*profile = PLATFORM_PROFILE_QUIET;
> +		*profile = PLATFORM_PROFILE_LOW_POWER;
>   		break;
>   	default:
>   		return -EINVAL;
> @@ -3969,7 +3969,7 @@ static int asus_wmi_platform_profile_set(struct device *dev,
>   	case PLATFORM_PROFILE_BALANCED:
>   		tp = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT;
>   		break;
> -	case PLATFORM_PROFILE_QUIET:
> +	case PLATFORM_PROFILE_LOW_POWER:
>   		tp = ASUS_THROTTLE_THERMAL_POLICY_SILENT;
>   		break;
>   	default:
> @@ -3982,7 +3982,7 @@ static int asus_wmi_platform_profile_set(struct device *dev,
>   
>   static int asus_wmi_platform_profile_probe(void *drvdata, unsigned long *choices)
>   {
> -	set_bit(PLATFORM_PROFILE_QUIET, choices);
> +	set_bit(PLATFORM_PROFILE_LOW_POWER, choices);
>   	set_bit(PLATFORM_PROFILE_BALANCED, choices);
>   	set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
>   

Re: [PATCH] platform/x86: asus-wmi: change quiet to low-power

[Luke Jones]

On Mon, 2025-02-24 at 18:39 -0800, Mario Limonciello wrote:
> On 2/24/2025 16:35, Luke Jones wrote:
> > From: "Luke D. Jones" <luke@ljones.dev>
> > 
> > Change the profile name "quiet" to "low-power" to match the AMD
> > name. The
> > primary reason for this is to match AMD naming for
> > platform_profiles and
> > allow both to match. It does not affect Intel machines.
> > 
> > The quiet profile is essentially a low-power profile which tweaks
> > both TDP and fans - this applies to 80+ ASUS laptops.
> > 
> > Signed-off-by: Luke D. Jones <luke@ljones.dev>
> 
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> 
> IMO - this should have a fixes tag since this should probably go in
> the 
> 6.14 cycle too.
> 
> Fixes: 688834743d67 ("ACPI: platform_profile: Allow multiple
> handlers")
> 

Good point, thanks. I assume when pulled in this can be added?

> > ---
> >   drivers/platform/x86/asus-wmi.c | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/asus-wmi.c
> > b/drivers/platform/x86/asus-wmi.c
> > index d22748f1e154..de19c3b3d8fb 100644
> > --- a/drivers/platform/x86/asus-wmi.c
> > +++ b/drivers/platform/x86/asus-wmi.c
> > @@ -3945,7 +3945,7 @@ static int
> > asus_wmi_platform_profile_get(struct device *dev,
> >   		*profile = PLATFORM_PROFILE_PERFORMANCE;
> >   		break;
> >   	case ASUS_THROTTLE_THERMAL_POLICY_SILENT:
> > -		*profile = PLATFORM_PROFILE_QUIET;
> > +		*profile = PLATFORM_PROFILE_LOW_POWER;
> >   		break;
> >   	default:
> >   		return -EINVAL;
> > @@ -3969,7 +3969,7 @@ static int
> > asus_wmi_platform_profile_set(struct device *dev,
> >   	case PLATFORM_PROFILE_BALANCED:
> >   		tp = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT;
> >   		break;
> > -	case PLATFORM_PROFILE_QUIET:
> > +	case PLATFORM_PROFILE_LOW_POWER:
> >   		tp = ASUS_THROTTLE_THERMAL_POLICY_SILENT;
> >   		break;
> >   	default:
> > @@ -3982,7 +3982,7 @@ static int
> > asus_wmi_platform_profile_set(struct device *dev,
> >   
> >   static int asus_wmi_platform_profile_probe(void *drvdata,
> > unsigned long *choices)
> >   {
> > -	set_bit(PLATFORM_PROFILE_QUIET, choices);
> > +	set_bit(PLATFORM_PROFILE_LOW_POWER, choices);
> >   	set_bit(PLATFORM_PROFILE_BALANCED, choices);
> >   	set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
> >   
> 

RE: [PATCH] platform/x86: asus-wmi: change quiet to low-power

[Antheas Kapenekakis]

Hi Luke,
please add appropriate attribution.

Closes: https://lore.kernel.org/all/20250224195059.10185-1-lkml@antheas.dev/
Reported-by: Antheas Kapenekakis <lkml@antheas.dev>

For me, this patch series plus the multi-platform profile one constitute
a double ABI break. Not only does the legacy sysfs for platform profile
regress when there is a second profile handler for a device, but all
hardcoded scripts for Asus devices will have to be updated.

While I would personally like to avoid this, I am ok with it, given
appropriate attribution, since I did go through the effort of reporting it
and providing a mitigation.

@Mario: you added Reviewed-by to a patch without proper attribution. Let's
not rehash our discussion from few days ago. Please try to do better when
it comes to attributions in the future.

Antheas

Re: [PATCH] platform/x86: asus-wmi: change quiet to low-power

[Mario Limonciello]

On 2/25/2025 06:25, Antheas Kapenekakis wrote:
> Hi Luke,
> please add appropriate attribution.
> 
> Closes: https://lore.kernel.org/all/20250224195059.10185-1-lkml@antheas.dev/
> Reported-by: Antheas Kapenekakis <lkml@antheas.dev>

Good call on adding these tags.

> 
> For me, this patch series plus the multi-platform profile one constitute
> a double ABI break. Not only does the legacy sysfs for platform profile
> regress when there is a second profile handler for a device, but all
> hardcoded scripts for Asus devices will have to be updated.

The documentation says to look at platform_profile_choices.  To 
determine what is supported.  FWIW this is exactly what 
power-profiles-daemon does.

> 
> While I would personally like to avoid this, I am ok with it, given
> appropriate attribution, since I did go through the effort of reporting it
> and providing a mitigation.
> 
> @Mario: you added Reviewed-by to a patch without proper attribution. Let's
> not rehash our discussion from few days ago. Please try to do better when
> it comes to attributions in the future.
> 
> Antheas

It's an oversight, no malice intended.  b4 (which most maintainers use) 
scans the whole thread for tags.  Adding them inline as a response is 
totally fine.

If Luke needs to spin a v2 for some reason before this is committed then 
he can add them as well to the v2.

Re: [PATCH] platform/x86: asus-wmi: change quiet to low-power

[Antheas Kapenekakis]

> The documentation says to look at platform_profile_choices.  To
> determine what is supported.  FWIW this is exactly what
> power-profiles-daemon does.

Yeah, this one is minor all things considered. Since there is a
justification. Even if user error is not the best of justifications.

But as I maintain a userspace TDP tool, I can tell you that if the
other issue is not fixed, /sys/firmware/acpi/platform_profile is no
longer trustworthy due to being able to occlude platform profiles. So
a mitigation will be needed for all userspace power utilities. But
there is actually very little use currently for
/sys/class/platform-profile as AMD pmf is mostly a NOOP, so I think
this would be premature and should be avoided.

While I do not have a large portfolio of ACPI collections for
Thinkpads, HP, Alienware, and Dell that have WMI drivers, I can see
amd-pmf popping up on a lot of the late 2024 models of GPD, Ayaneo,
Lenovo, and Asus, and going into 2025. So it is worthwhile fixing this
now once and for all.

Antheas

Re: [PATCH] platform/x86: asus-wmi: change quiet to low-power

[Armin Wolf]

Am 25.02.25 um 07:13 schrieb Luke Jones:

> On Mon, 2025-02-24 at 18:39 -0800, Mario Limonciello wrote:
>> On 2/24/2025 16:35, Luke Jones wrote:
>>> From: "Luke D. Jones" <luke@ljones.dev>
>>>
>>> Change the profile name "quiet" to "low-power" to match the AMD
>>> name. The
>>> primary reason for this is to match AMD naming for
>>> platform_profiles and
>>> allow both to match. It does not affect Intel machines.
>>>
>>> The quiet profile is essentially a low-power profile which tweaks
>>> both TDP and fans - this applies to 80+ ASUS laptops.
>>>
>>> Signed-off-by: Luke D. Jones <luke@ljones.dev>
>> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
>>
>> IMO - this should have a fixes tag since this should probably go in
>> the
>> 6.14 cycle too.
>>
>> Fixes: 688834743d67 ("ACPI: platform_profile: Allow multiple
>> handlers")
>>
> Good point, thanks. I assume when pulled in this can be added?

Antheas is concerned that this patch might break brittle userspace scripts
like "echo quiet | sudo tee /sys/firmware/acpi/platform_profile".

Maybe we should instead change the strategy used by the legacy platform-profile
handler when selecting supported profiles?

Thanks,
Armin Wolff

>>> ---
>>>    drivers/platform/x86/asus-wmi.c | 6 +++---
>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/platform/x86/asus-wmi.c
>>> b/drivers/platform/x86/asus-wmi.c
>>> index d22748f1e154..de19c3b3d8fb 100644
>>> --- a/drivers/platform/x86/asus-wmi.c
>>> +++ b/drivers/platform/x86/asus-wmi.c
>>> @@ -3945,7 +3945,7 @@ static int
>>> asus_wmi_platform_profile_get(struct device *dev,
>>>    		*profile = PLATFORM_PROFILE_PERFORMANCE;
>>>    		break;
>>>    	case ASUS_THROTTLE_THERMAL_POLICY_SILENT:
>>> -		*profile = PLATFORM_PROFILE_QUIET;
>>> +		*profile = PLATFORM_PROFILE_LOW_POWER;
>>>    		break;
>>>    	default:
>>>    		return -EINVAL;
>>> @@ -3969,7 +3969,7 @@ static int
>>> asus_wmi_platform_profile_set(struct device *dev,
>>>    	case PLATFORM_PROFILE_BALANCED:
>>>    		tp = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT;
>>>    		break;
>>> -	case PLATFORM_PROFILE_QUIET:
>>> +	case PLATFORM_PROFILE_LOW_POWER:
>>>    		tp = ASUS_THROTTLE_THERMAL_POLICY_SILENT;
>>>    		break;
>>>    	default:
>>> @@ -3982,7 +3982,7 @@ static int
>>> asus_wmi_platform_profile_set(struct device *dev,
>>>
>>>    static int asus_wmi_platform_profile_probe(void *drvdata,
>>> unsigned long *choices)
>>>    {
>>> -	set_bit(PLATFORM_PROFILE_QUIET, choices);
>>> +	set_bit(PLATFORM_PROFILE_LOW_POWER, choices);
>>>    	set_bit(PLATFORM_PROFILE_BALANCED, choices);
>>>    	set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
>>>
>