[PATCH 0/2] platform/x86: acer-wmi: Last minute fixes

[PATCH 0/2] platform/x86: acer-wmi: Last minute fixes

[Armin Wolf]

This patch series contains some last minute fixes to the acer-wmi
driver.

The first patch was already part of a previous patch series but got
rejected due to the fact that the acer-wmi driver was using the AC
status at the time to cycle platform profiles. Since this is not the
case anymore the patch is now being resubmitted.

The second patch fixed a small issue during platform profile
initialization.

The first patch was tested on a real machine while the second patch is
compile-tested only.

Armin Wolf (2):
  platform/x86: acer-wmi: Ignore AC events
  platform/x86: acer-wmi: Fix initialization of last_non_turbo_profile

 drivers/platform/x86/acer-wmi.c | 45 ++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 15 deletions(-)

--
2.39.5

[PATCH 1/2] platform/x86: acer-wmi: Ignore AC events

[Armin Wolf]

On the Acer Swift SFG14-41, the events 8 - 1 and 8 - 0 are printed on
AC connect/disconnect. Ignore those events to avoid spamming the
kernel log with error messages.

Reported-by: Farhan Anwar <farhan.anwar8@gmail.com>
Closes: https://lore.kernel.org/platform-driver-x86/2ffb529d-e7c8-4026-a3b8-120c8e7afec8@gmail.com
Tested-by: Rayan Margham <rayanmargham4@gmail.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/acer-wmi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index ae2a7c93ab72..a85c881d1f24 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -104,6 +104,7 @@ enum acer_wmi_event_ids {
 	WMID_HOTKEY_EVENT = 0x1,
 	WMID_ACCEL_OR_KBD_DOCK_EVENT = 0x5,
 	WMID_GAMING_TURBO_KEY_EVENT = 0x7,
+	WMID_AC_EVENT = 0x8,
 };

 enum acer_wmi_predator_v4_sys_info_command {
@@ -2414,6 +2415,9 @@ static void acer_wmi_notify(union acpi_object *obj, void *context)
 		if (return_value.key_num == 0x5 && has_cap(ACER_CAP_PLATFORM_PROFILE))
 			acer_thermal_profile_change();
 		break;
+	case WMID_AC_EVENT:
+		/* We ignore AC events here */
+		break;
 	default:
 		pr_warn("Unknown function number - %d - %d\n",
 			return_value.function, return_value.key_num);
--
2.39.5

[PATCH 2/2] platform/x86: acer-wmi: Fix initialization of last_non_turbo_profile

[Armin Wolf]

On machines that do not support the balanced profile the value of
last_non_turbo_profile is invalid after initialization which might
cause the driver to switch to an unsupported platform profile later.

Fix this by only setting last_non_turbo_profile to supported platform
profile values.

Fixes: 191e21f1a4c3 ("platform/x86: acer-wmi: use an ACPI bitmap to set the platform profile choices")
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/acer-wmi.c | 41 +++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index a85c881d1f24..69336bd778ee 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -792,7 +792,7 @@ static bool platform_profile_support;
  * The profile used before turbo mode. This variable is needed for
  * returning from turbo mode when the mode key is in toggle mode.
  */
-static int last_non_turbo_profile;
+static int last_non_turbo_profile = INT_MIN;

 /* The most performant supported profile */
 static int acer_predator_v4_max_perf;
@@ -2034,32 +2034,43 @@ acer_predator_v4_platform_profile_probe(void *drvdata, unsigned long *choices)
 	/* Iterate through supported profiles in order of increasing performance */
 	if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_ECO, &supported_profiles)) {
 		set_bit(PLATFORM_PROFILE_LOW_POWER, choices);
-		acer_predator_v4_max_perf =
-			ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
+		acer_predator_v4_max_perf = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
+		last_non_turbo_profile = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
 	}

 	if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET, &supported_profiles)) {
 		set_bit(PLATFORM_PROFILE_QUIET, choices);
-		acer_predator_v4_max_perf =
-			ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
+		acer_predator_v4_max_perf = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
+		last_non_turbo_profile = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
 	}

 	if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED, &supported_profiles)) {
 		set_bit(PLATFORM_PROFILE_BALANCED, choices);
-		acer_predator_v4_max_perf =
-			ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
+		acer_predator_v4_max_perf = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
+		last_non_turbo_profile = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
 	}

 	if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE, &supported_profiles)) {
 		set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE, choices);
-		acer_predator_v4_max_perf =
-			ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
+		acer_predator_v4_max_perf = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
+
+		/* We only use this profile as a fallback option in case no prior
+		 * profile is supported.
+		 */
+		if (last_non_turbo_profile < 0)
+			last_non_turbo_profile = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
 	}

 	if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO, &supported_profiles)) {
 		set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
-		acer_predator_v4_max_perf =
-			ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
+		acer_predator_v4_max_perf = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
+
+		/* We need to handle the hypothetical case where only the turbo profile
+		 * is supported. In this case the turbo toggle will essentially be a
+		 * no-op.
+		 */
+		if (last_non_turbo_profile < 0)
+			last_non_turbo_profile = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
 	}

 	return 0;
@@ -2080,10 +2091,6 @@ static int acer_platform_profile_setup(struct platform_device *device)
 			return PTR_ERR(platform_profile_device);

 		platform_profile_support = true;
-
-		/* Set default non-turbo profile  */
-		last_non_turbo_profile =
-			ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
 	}
 	return 0;
 }
@@ -2101,6 +2108,10 @@ static int acer_thermal_profile_change(void)
 		if (cycle_gaming_thermal_profile) {
 			platform_profile_cycle();
 		} else {
+			/* Do nothing if no suitable platform profiles where found */
+			if (last_non_turbo_profile < 0)
+				return 0;
+
 			err = WMID_gaming_get_misc_setting(
 				ACER_WMID_MISC_SETTING_PLATFORM_PROFILE, &current_tp);
 			if (err)
--
2.39.5

Re: [PATCH 2/2] platform/x86: acer-wmi: Fix initialization of last_non_turbo_profile

[Hridesh MG]

On Mon, Jan 20, 2025 at 1:47 AM Armin Wolf <W_Armin@gmx.de> wrote:
> On machines that do not support the balanced profile the value of
> last_non_turbo_profile is invalid after initialization which might
> cause the driver to switch to an unsupported platform profile later.
Hello Armin, while I think it's unlikely that Acer would release a
gaming laptop which has support for quiet and eco but not balanced
mode, more robust error handling is always nice to have.

> Fix this by only setting last_non_turbo_profile to supported platform
> profile values.
>
> Fixes: 191e21f1a4c3 ("platform/x86: acer-wmi: use an ACPI bitmap to set the platform profile choices")
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
Tested-by: Hridesh MG <hridesh699@gmail.com>
Reviewed-by: Hridesh MG <hridesh699@gmail.com>

> ---
>  drivers/platform/x86/acer-wmi.c | 41 +++++++++++++++++++++------------
>  1 file changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index a85c881d1f24..69336bd778ee 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -792,7 +792,7 @@ static bool platform_profile_support;
>   * The profile used before turbo mode. This variable is needed for
>   * returning from turbo mode when the mode key is in toggle mode.
>   */
> -static int last_non_turbo_profile;
> +static int last_non_turbo_profile = INT_MIN;
>
>  /* The most performant supported profile */
>  static int acer_predator_v4_max_perf;
> @@ -2034,32 +2034,43 @@ acer_predator_v4_platform_profile_probe(void *drvdata, unsigned long *choices)
>         /* Iterate through supported profiles in order of increasing performance */
>         if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_ECO, &supported_profiles)) {
>                 set_bit(PLATFORM_PROFILE_LOW_POWER, choices);
> -               acer_predator_v4_max_perf =
> -                       ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
> +               acer_predator_v4_max_perf = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
> +               last_non_turbo_profile = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
>         }
>
>         if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET, &supported_profiles)) {
>                 set_bit(PLATFORM_PROFILE_QUIET, choices);
> -               acer_predator_v4_max_perf =
> -                       ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
> +               acer_predator_v4_max_perf = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
> +               last_non_turbo_profile = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
>         }
>
>         if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED, &supported_profiles)) {
>                 set_bit(PLATFORM_PROFILE_BALANCED, choices);
> -               acer_predator_v4_max_perf =
> -                       ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> +               acer_predator_v4_max_perf = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> +               last_non_turbo_profile = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>         }
>
>         if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE, &supported_profiles)) {
>                 set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE, choices);
> -               acer_predator_v4_max_perf =
> -                       ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
> +               acer_predator_v4_max_perf = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
> +
> +               /* We only use this profile as a fallback option in case no prior
> +                * profile is supported.
> +                */
> +               if (last_non_turbo_profile < 0)
> +                       last_non_turbo_profile = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
>         }
>
>         if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO, &supported_profiles)) {
>                 set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
> -               acer_predator_v4_max_perf =
> -                       ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
> +               acer_predator_v4_max_perf = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
> +
> +               /* We need to handle the hypothetical case where only the turbo profile
> +                * is supported. In this case the turbo toggle will essentially be a
> +                * no-op.
> +                */
> +               if (last_non_turbo_profile < 0)
> +                       last_non_turbo_profile = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
>         }
>
>         return 0;
> @@ -2080,10 +2091,6 @@ static int acer_platform_profile_setup(struct platform_device *device)
>                         return PTR_ERR(platform_profile_device);
>
>                 platform_profile_support = true;
> -
> -               /* Set default non-turbo profile  */
> -               last_non_turbo_profile =
> -                       ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>         }
>         return 0;
>  }
> @@ -2101,6 +2108,10 @@ static int acer_thermal_profile_change(void)
>                 if (cycle_gaming_thermal_profile) {
>                         platform_profile_cycle();
>                 } else {
> +                       /* Do nothing if no suitable platform profiles where found */
> +                       if (last_non_turbo_profile < 0)
> +                               return 0;
> +
>                         err = WMID_gaming_get_misc_setting(
>                                 ACER_WMID_MISC_SETTING_PLATFORM_PROFILE, &current_tp);
>                         if (err)
> --
> 2.39.5
>


--
Thanks,
Hridesh MG

Re: [PATCH 0/2] platform/x86: acer-wmi: Last minute fixes

[Ilpo Järvinen]

On Sun, 19 Jan 2025 21:17:21 +0100, Armin Wolf wrote:

> This patch series contains some last minute fixes to the acer-wmi
> driver.
> 
> The first patch was already part of a previous patch series but got
> rejected due to the fact that the acer-wmi driver was using the AC
> status at the time to cycle platform profiles. Since this is not the
> case anymore the patch is now being resubmitted.
> 
> [...]


Thank you for your contribution, it has been applied to my local
review-ilpo-next branch. Note it will show up in the public
platform-drivers-x86/review-ilpo-next branch only once I've pushed my
local branch there, which might take a while.

The list of commits applied:
[1/2] platform/x86: acer-wmi: Ignore AC events
      commit: f6bfa25c6665f8721421ea94fe506cc22f1d4b43
[2/2] platform/x86: acer-wmi: Fix initialization of last_non_turbo_profile
      commit: 3ef4ea516c2f8a2f1ff8ca59b156fb2e4472c2a4

--
 i.