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

Kurt Borja
2025-01-08T13:45:08Z

On Wed, Jan 08, 2025 at 02:15:25PM +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
> unnecessary handling for AC power, as the hardware accepts different
> profiles regardless of whether 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 | 80 ++++++++++-------------------------------
> 1 file changed, 19 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index f662e3740408f70e1e921a90fe75ce441fd239d0..7968fe21507b1cf28fdc575139057c795e6a873b 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -2028,76 +2028,34 @@ 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 turbo profile.
> */
> if (quirks->predator_v4) {
> u8 current_tp;
> - int tp, err;
> - u64 on_AC;
> + int err, tp;
>
> - 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;
> + if (cycle_gaming_thermal_profile) {
> + platform_profile_cycle();
> + } else {
> + err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE,

nit: Break this line like you did bellow.

> + &current_tp);
> + if (err)
> + return err;
>
> - 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 == ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO) {
> tp = last_non_turbo_profile;
> - break;
> - case ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE:
> - if (!on_AC)
> - tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> - else
> + } else {
> + last_non_turbo_profile = current_tp;

WMID_gaming_set_misc_setting() may fail, so I think you should set
last_non_turbo_profile after call has succeeded.

~ Kurt

> 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;
> + }
>
> - /* 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;