[v2] platform/x86 acer-wmi: Improve platform profile handling

[PATCH v2 3/3] platform/x86: acer-wmi: simplify platform profile cycling

Posted by Hridesh MG 1 year, 1 month ago

Make use of platform_profile_cycle() to simplify the logic used for
cycling through the different platform profiles. Also remove the
handling for AC power as the hardware will accept the different profiles
regardless of whether or not AC is plugged in.

Link: https://lore.kernel.org/platform-driver-x86/20e3ac66-b040-49a9-ab00-0adcfdaed2ff@gmx.de/
Signed-off-by: Hridesh MG <hridesh699@gmail.com>
---
 drivers/platform/x86/acer-wmi.c | 87 +++++++++++------------------------------
 1 file changed, 23 insertions(+), 64 deletions(-)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index f6c47deb4c452fc193f22c479c730aecb1e69e44..9c73f78eb302323299e03bf9dbeb2c68bb422938 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -34,6 +34,7 @@
 #include <linux/unaligned.h>
 #include <linux/bitfield.h>
 #include <linux/bitops.h>
+#include "linux/bitmap.h"
 
 MODULE_AUTHOR("Carlos Corbacho");
 MODULE_DESCRIPTION("Acer Laptop WMI Extras Driver");
@@ -1975,9 +1976,6 @@ acer_predator_v4_platform_profile_set(struct platform_profile_handler *pprof,
 	if (err)
 		return err;
 
-	if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO)
-		last_non_turbo_profile = tp;
-
 	return 0;
 }
 
@@ -2036,76 +2034,37 @@ static int acer_platform_profile_setup(struct platform_device *device)
 static int acer_thermal_profile_change(void)
 {
 	/*
-	 * This mode key can rotate each mode or toggle turbo mode.
-	 * On battery, only ECO and BALANCED mode are available.
+	 * This mode key will either cycle through each mode or toggle the performance profile.
 	 */
 	if (quirks->predator_v4) {
 		u8 current_tp;
-		int tp, err;
-		u64 on_AC;
+		int max_perf, tp, err;
 
-		err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE,
-						   &current_tp);
-		if (err)
-			return err;
+		if (cycle_gaming_thermal_profile) {
+			platform_profile_cycle();
+		} else {
+			err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE,
+							   &current_tp);
+			if (err)
+				return err;
 
-		/* Check power source */
-		err = WMID_gaming_get_sys_info(ACER_WMID_CMD_GET_PREDATOR_V4_BAT_STATUS, &on_AC);
-		if (err < 0)
-			return err;
+			max_perf = find_last_bit(platform_profile_handler.choices,
+						 PLATFORM_PROFILE_LAST);
 
-		switch (current_tp) {
-		case ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO:
-			if (!on_AC)
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
-			else if (cycle_gaming_thermal_profile)
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
-			else
+			if (current_tp == max_perf) {
 				tp = last_non_turbo_profile;
-			break;
-		case ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE:
-			if (!on_AC)
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
-			else
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
-			break;
-		case ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED:
-			if (!on_AC)
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
-			else if (cycle_gaming_thermal_profile)
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
-			else
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
-			break;
-		case ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET:
-			if (!on_AC)
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
-			else if (cycle_gaming_thermal_profile)
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
-			else
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
-			break;
-		case ACER_PREDATOR_V4_THERMAL_PROFILE_ECO:
-			if (!on_AC)
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
-			else if (cycle_gaming_thermal_profile)
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
-			else
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
-			break;
-		default:
-			return -EOPNOTSUPP;
-		}
-
-		err = WMID_gaming_set_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE, tp);
-		if (err)
-			return err;
+			} else {
+				last_non_turbo_profile = current_tp;
+				tp = max_perf;
+			}
 
-		/* Store non-turbo profile for turbo mode toggle*/
-		if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO)
-			last_non_turbo_profile = tp;
+			err = WMID_gaming_set_misc_setting(
+				ACER_WMID_MISC_SETTING_PLATFORM_PROFILE, tp);
+			if (err)
+				return err;
 
-		platform_profile_notify(&platform_profile_handler);
+			platform_profile_notify(&platform_profile_handler);
+		}
 	}
 
 	return 0;

-- 
2.47.1

Re: [PATCH v2 3/3] platform/x86: acer-wmi: simplify platform profile cycling

Posted by Kurt Borja 1 year, 1 month ago

On Sat, Jan 04, 2025 at 08:59:22PM +0530, Hridesh MG wrote:
> Make use of platform_profile_cycle() to simplify the logic used for
> cycling through the different platform profiles. Also remove the
> handling for AC power as the hardware will accept the different profiles
> regardless of whether or not AC is plugged in.
> 
> Link: https://lore.kernel.org/platform-driver-x86/20e3ac66-b040-49a9-ab00-0adcfdaed2ff@gmx.de/
> Signed-off-by: Hridesh MG <hridesh699@gmail.com>
> ---
>  drivers/platform/x86/acer-wmi.c | 87 +++++++++++------------------------------
>  1 file changed, 23 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index f6c47deb4c452fc193f22c479c730aecb1e69e44..9c73f78eb302323299e03bf9dbeb2c68bb422938 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -34,6 +34,7 @@
>  #include <linux/unaligned.h>
>  #include <linux/bitfield.h>
>  #include <linux/bitops.h>
> +#include "linux/bitmap.h"
>  
>  MODULE_AUTHOR("Carlos Corbacho");
>  MODULE_DESCRIPTION("Acer Laptop WMI Extras Driver");
> @@ -1975,9 +1976,6 @@ acer_predator_v4_platform_profile_set(struct platform_profile_handler *pprof,
>  	if (err)
>  		return err;
>  
> -	if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO)
> -		last_non_turbo_profile = tp;
> -

I think this should be kept. If the user changes profile manually this
may not reflect the actual last_non_turbo_profile.

>  	return 0;
>  }
>  
> @@ -2036,76 +2034,37 @@ static int acer_platform_profile_setup(struct platform_device *device)
>  static int acer_thermal_profile_change(void)

I'm Cc'ing SungHwan Jung as they were the author of patch that added
this function. 

~ Kurt

>  {
>  	/*
> -	 * This mode key can rotate each mode or toggle turbo mode.
> -	 * On battery, only ECO and BALANCED mode are available.
> +	 * This mode key will either cycle through each mode or toggle the performance profile.
>  	 */
>  	if (quirks->predator_v4) {
>  		u8 current_tp;
> -		int tp, err;
> -		u64 on_AC;
> +		int max_perf, tp, err;
>  
> -		err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE,
> -						   &current_tp);
> -		if (err)
> -			return err;
> +		if (cycle_gaming_thermal_profile) {
> +			platform_profile_cycle();
> +		} else {
> +			err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE,
> +							   &current_tp);
> +			if (err)
> +				return err;
>  
> -		/* Check power source */
> -		err = WMID_gaming_get_sys_info(ACER_WMID_CMD_GET_PREDATOR_V4_BAT_STATUS, &on_AC);
> -		if (err < 0)
> -			return err;
> +			max_perf = find_last_bit(platform_profile_handler.choices,
> +						 PLATFORM_PROFILE_LAST);
>  
> -		switch (current_tp) {
> -		case ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO:
> -			if (!on_AC)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> -			else if (cycle_gaming_thermal_profile)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
> -			else
> +			if (current_tp == max_perf) {
>  				tp = last_non_turbo_profile;
> -			break;
> -		case ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE:
> -			if (!on_AC)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> -			else
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
> -			break;
> -		case ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED:
> -			if (!on_AC)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
> -			else if (cycle_gaming_thermal_profile)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
> -			else
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
> -			break;
> -		case ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET:
> -			if (!on_AC)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> -			else if (cycle_gaming_thermal_profile)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> -			else
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
> -			break;
> -		case ACER_PREDATOR_V4_THERMAL_PROFILE_ECO:
> -			if (!on_AC)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> -			else if (cycle_gaming_thermal_profile)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
> -			else
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
> -			break;
> -		default:
> -			return -EOPNOTSUPP;
> -		}
> -
> -		err = WMID_gaming_set_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE, tp);
> -		if (err)
> -			return err;
> +			} else {
> +				last_non_turbo_profile = current_tp;
> +				tp = max_perf;
> +			}
>  
> -		/* Store non-turbo profile for turbo mode toggle*/
> -		if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO)
> -			last_non_turbo_profile = tp;
> +			err = WMID_gaming_set_misc_setting(
> +				ACER_WMID_MISC_SETTING_PLATFORM_PROFILE, tp);
> +			if (err)
> +				return err;
>  
> -		platform_profile_notify(&platform_profile_handler);
> +			platform_profile_notify(&platform_profile_handler);
> +		}
>  	}
>  
>  	return 0;

Re: [PATCH v2 3/3] platform/x86: acer-wmi: simplify platform profile cycling

Posted by Hridesh MG 1 year, 1 month ago

On Sat, Jan 4, 2025 at 11:23 PM Kurt Borja <kuurtb@gmail.com> wrote:
>
> On Sat, Jan 04, 2025 at 08:59:22PM +0530, Hridesh MG wrote:
> > Make use of platform_profile_cycle() to simplify the logic used for
> > cycling through the different platform profiles. Also remove the
> > handling for AC power as the hardware will accept the different profiles
> > regardless of whether or not AC is plugged in.
> >
> > Link: https://lore.kernel.org/platform-driver-x86/20e3ac66-b040-49a9-ab00-0adcfdaed2ff@gmx.de/
> > Signed-off-by: Hridesh MG <hridesh699@gmail.com>
> > ---
> >  drivers/platform/x86/acer-wmi.c | 87 +++++++++++------------------------------
> >  1 file changed, 23 insertions(+), 64 deletions(-)
> >
> > diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> > index f6c47deb4c452fc193f22c479c730aecb1e69e44..9c73f78eb302323299e03bf9dbeb2c68bb422938 100644
> > --- a/drivers/platform/x86/acer-wmi.c
> > +++ b/drivers/platform/x86/acer-wmi.c
> > @@ -34,6 +34,7 @@
> >  #include <linux/unaligned.h>
> >  #include <linux/bitfield.h>
> >  #include <linux/bitops.h>
> > +#include "linux/bitmap.h"
> >
> >  MODULE_AUTHOR("Carlos Corbacho");
> >  MODULE_DESCRIPTION("Acer Laptop WMI Extras Driver");
> > @@ -1975,9 +1976,6 @@ acer_predator_v4_platform_profile_set(struct platform_profile_handler *pprof,
> >       if (err)
> >               return err;
> >
> > -     if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO)
> > -             last_non_turbo_profile = tp;
> > -
>
> I think this should be kept. If the user changes profile manually this
> may not reflect the actual last_non_turbo_profile.
I thought that the purpose of last_non_turbo_profile was for
acer_thermal_profile_change() to store the profile just before
toggling turbo so that the system can return to it later on (as
mentioned in the comments). I don't see a valid use case for this
variable outside of that specific context, which is why I decided to
remove its update during manual profile changes.

Thanks,
Hridesh MG

Re: [PATCH v2 3/3] platform/x86: acer-wmi: simplify platform profile cycling

Posted by SungHwan Jung 1 year, 1 month ago


On 1/5/25 03:19, Hridesh MG wrote:
>> I think this should be kept. If the user changes profile manually this
>> may not reflect the actual last_non_turbo_profile.
> I thought that the purpose of last_non_turbo_profile was for
> acer_thermal_profile_change() to store the profile just before
> toggling turbo so that the system can return to it later on (as
> mentioned in the comments). I don't see a valid use case for this
> variable outside of that specific context, which is why I decided to
> remove its update during manual profile changes.
> 
I think last_non_turbo_profile is still needed in
acer_predator_v4_platform_profile_set for returning from turbo mode set
by user space application in toggle mode.

Without this, when users change profiles and set turbo mode using
applications or scripts (like predator sense GUI on windows) then use
the mode button to return from turbo mode, it returns to default or the
last value by the button, not the actual last profile.

Thanks,
SungHwan Jung

> Thanks,
> Hridesh MG

Re: [PATCH v2 3/3] platform/x86: acer-wmi: simplify platform profile cycling

Posted by Hridesh MG 1 year, 1 month ago

On Sun, Jan 5, 2025 at 9:31 AM SungHwan Jung <onenowy@gmail.com> wrote:
> On 1/5/25 03:19, Hridesh MG wrote:
> >> I think this should be kept. If the user changes profile manually this
> >> may not reflect the actual last_non_turbo_profile.
> > I thought that the purpose of last_non_turbo_profile was for
> > acer_thermal_profile_change() to store the profile just before
> > toggling turbo so that the system can return to it later on (as
> > mentioned in the comments). I don't see a valid use case for this
> > variable outside of that specific context, which is why I decided to
> > remove its update during manual profile changes.
> >
> I think last_non_turbo_profile is still needed in
> acer_predator_v4_platform_profile_set for returning from turbo mode set
> by user space application in toggle mode.
>
> Without this, when users change profiles and set turbo mode using
> applications or scripts (like predator sense GUI on windows) then use
> the mode button to return from turbo mode, it returns to default or the
> last value by the button, not the actual last profile.
>
Ah, I see now, that case seems to have slipped my mind. Thanks for
pointing it out.


--
Thanks,
Hridesh MG