1. Patchew
  2. linux
  3. View series

[PATCH] platform/x86: acer-wmi: improve platform profile handling

Hridesh MG posted 1 patch 1 year, 4 months ago 
drivers/platform/x86/acer-wmi.c | 220 ++++++++++++++++++++++----------
1 file changed, 154 insertions(+), 66 deletions(-)
[PATCH] platform/x86: acer-wmi: improve platform profile handling
Posted by Hridesh MG 1 year, 4 months ago 
Two improvements to the platform profile handling for laptops using
the Acer Predator interface -

1) Use WMI calls to fetch the current platform profile instead of
   directly accessing it from the EC since the EC address differs for
   certain laptops.
2) Use the supported profiles bitmap to dynamically set
   platform_profile_choices.

Link: https://lore.kernel.org/platform-driver-x86/d7be714c-3103-42ee-ad15-223a3fe67f80@gmx.de/
Co-developed-by: Armin Wolf <W_Armin@gmx.de>
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
Signed-off-by: Hridesh MG <hridesh699@gmail.com>
---
This patch has been tested on an acer nitro AN515-58 laptop, it would be
good if someone with a predator laptop could also test it
---
 drivers/platform/x86/acer-wmi.c | 220 ++++++++++++++++++++++----------
 1 file changed, 154 insertions(+), 66 deletions(-)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index b3043d78a7b3..37f629b6e3d3 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -31,6 +31,7 @@
 #include <acpi/video.h>
 #include <linux/hwmon.h>
 #include <linux/units.h>
+#include <linux/unaligned.h>
 #include <linux/bitfield.h>
 
 MODULE_AUTHOR("Carlos Corbacho");
@@ -68,8 +69,11 @@ MODULE_LICENSE("GPL");
 #define ACER_WMID_GET_GAMING_SYS_INFO_METHODID 5
 #define ACER_WMID_SET_GAMING_FAN_BEHAVIOR 14
 #define ACER_WMID_SET_GAMING_MISC_SETTING_METHODID 22
+#define ACER_WMID_GET_GAMING_MISC_SETTING_METHODID 23
 
-#define ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET 0x54
+#define ACER_GAMING_MISC_SETTING_STATUS_MASK GENMASK_ULL(7, 0)
+#define ACER_GAMING_MISC_SETTING_INDEX_MASK GENMASK_ULL(7, 0)
+#define ACER_GAMING_MISC_SETTING_VALUE_MASK GENMASK_ULL(15, 8)
 
 #define ACER_PREDATOR_V4_RETURN_STATUS_BIT_MASK GENMASK_ULL(7, 0)
 #define ACER_PREDATOR_V4_SENSOR_INDEX_BIT_MASK GENMASK_ULL(15, 8)
@@ -115,6 +119,13 @@ enum acer_wmi_predator_v4_sensor_id {
 	ACER_WMID_SENSOR_GPU_TEMPERATURE	= 0x0A,
 };
 
+enum acer_wmi_gaming_misc_setting {
+	ACER_WMID_MISC_SETTING_OC_1			= 0x0005,
+	ACER_WMID_MISC_SETTING_OC_2			= 0x0007,
+	ACER_WMID_MISC_SETTING_SUPPORTED_PROFILES	= 0x000A,
+	ACER_WMID_MISC_SETTING_PLATFORM_PROFILE		= 0x000B,
+};
+
 static const struct key_entry acer_wmi_keymap[] __initconst = {
 	{KE_KEY, 0x01, {KEY_WLAN} },     /* WiFi */
 	{KE_KEY, 0x03, {KEY_WLAN} },     /* WiFi */
@@ -751,20 +762,12 @@ static bool platform_profile_support;
  */
 static int last_non_turbo_profile;
 
-enum acer_predator_v4_thermal_profile_ec {
-	ACER_PREDATOR_V4_THERMAL_PROFILE_ECO = 0x04,
-	ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO = 0x03,
-	ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE = 0x02,
-	ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET = 0x01,
-	ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED = 0x00,
-};
-
-enum acer_predator_v4_thermal_profile_wmi {
-	ACER_PREDATOR_V4_THERMAL_PROFILE_ECO_WMI = 0x060B,
-	ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI = 0x050B,
-	ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE_WMI = 0x040B,
-	ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET_WMI = 0x0B,
-	ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI = 0x010B,
+enum acer_predator_v4_thermal_profile {
+	ACER_PREDATOR_V4_THERMAL_PROFILE_ECO = 0x06,
+	ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO = 0x05,
+	ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE = 0x04,
+	ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED = 0x01,
+	ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET = 0x00,
 };
 
 /* Find which quirks are needed for a particular vendor/ model pair */
@@ -1477,6 +1480,45 @@ WMI_gaming_execute_u64(u32 method_id, u64 in, u64 *out)
 	return status;
 }
 
+static int WMI_gaming_execute_u32_u64(u32 method_id, u32 in, u64 *out)
+{
+	struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL };
+	struct acpi_buffer input = {
+		.length = sizeof(in),
+		.pointer = &in,
+	};
+	union acpi_object *obj;
+	acpi_status status;
+	int ret = 0;
+
+	status = wmi_evaluate_method(WMID_GUID4, 0, method_id, &input, &result);
+	if (ACPI_FAILURE(status))
+		return -EIO;
+
+	obj = result.pointer;
+	if (obj && out) {
+		switch (obj->type) {
+		case ACPI_TYPE_INTEGER:
+			*out = obj->integer.value;
+			break;
+		case ACPI_TYPE_BUFFER:
+			if (obj->buffer.length < sizeof(*out))
+				ret = -ENOMSG;
+			else
+				*out = get_unaligned_le64(obj->buffer.pointer);
+
+			break;
+		default:
+			ret = -ENOMSG;
+			break;
+		}
+	}
+
+	kfree(obj);
+
+	return ret;
+}
+
 static acpi_status WMID_gaming_set_u64(u64 value, u32 cap)
 {
 	u32 method_id = 0;
@@ -1565,6 +1607,48 @@ static void WMID_gaming_set_fan_mode(u8 fan_mode)
 	WMID_gaming_set_u64(gpu_fan_config2 | gpu_fan_config1 << 16, ACER_CAP_TURBO_FAN);
 }
 
+static int WMID_gaming_set_misc_setting(enum acer_wmi_gaming_misc_setting setting, u8 value)
+{
+	acpi_status status;
+	u64 input = 0;
+	u64 result;
+
+	input |= FIELD_PREP(ACER_GAMING_MISC_SETTING_INDEX_MASK, setting);
+	input |= FIELD_PREP(ACER_GAMING_MISC_SETTING_VALUE_MASK, value);
+
+	status = WMI_gaming_execute_u64(ACER_WMID_SET_GAMING_MISC_SETTING_METHODID, input, &result);
+	if (ACPI_FAILURE(status))
+		return -EIO;
+
+	/* The return status must be zero for the operation to have succeeded */
+	if (FIELD_GET(ACER_GAMING_MISC_SETTING_STATUS_MASK, result))
+		return -EIO;
+
+	return 0;
+}
+
+static int WMID_gaming_get_misc_setting(enum acer_wmi_gaming_misc_setting setting, u8 *value)
+{
+	u64 input = 0;
+	u64 result;
+	int ret;
+
+	input |= FIELD_PREP(ACER_GAMING_MISC_SETTING_INDEX_MASK, setting);
+
+	ret = WMI_gaming_execute_u32_u64(ACER_WMID_GET_GAMING_MISC_SETTING_METHODID, input,
+					 &result);
+	if (ret < 0)
+		return ret;
+
+	/* The return status must be zero for the operation to have succeeded */
+	if (FIELD_GET(ACER_GAMING_MISC_SETTING_STATUS_MASK, result))
+		return -EIO;
+
+	*value = FIELD_GET(ACER_GAMING_MISC_SETTING_VALUE_MASK, result);
+
+	return 0;
+}
+
 /*
  * Generic Device (interface-independent)
  */
@@ -1833,9 +1917,8 @@ acer_predator_v4_platform_profile_get(struct platform_profile_handler *pprof,
 	u8 tp;
 	int err;
 
-	err = ec_read(ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET, &tp);
-
-	if (err < 0)
+	err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE, &tp);
+	if (err)
 		return err;
 
 	switch (tp) {
@@ -1865,36 +1948,33 @@ static int
 acer_predator_v4_platform_profile_set(struct platform_profile_handler *pprof,
 				      enum platform_profile_option profile)
 {
-	int tp;
-	acpi_status status;
+	int tp, err;
 
 	switch (profile) {
 	case PLATFORM_PROFILE_PERFORMANCE:
-		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
+		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
 		break;
 	case PLATFORM_PROFILE_BALANCED_PERFORMANCE:
-		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE_WMI;
+		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
 		break;
 	case PLATFORM_PROFILE_BALANCED:
-		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
+		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
 		break;
 	case PLATFORM_PROFILE_QUIET:
-		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET_WMI;
+		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
 		break;
 	case PLATFORM_PROFILE_LOW_POWER:
-		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO_WMI;
+		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
 		break;
 	default:
 		return -EOPNOTSUPP;
 	}
 
-	status = WMI_gaming_execute_u64(
-		ACER_WMID_SET_GAMING_MISC_SETTING_METHODID, tp, NULL);
-
-	if (ACPI_FAILURE(status))
-		return -EIO;
+	err = WMID_gaming_set_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE, tp);
+	if (err)
+		return err;
 
-	if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI)
+	if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO)
 		last_non_turbo_profile = tp;
 
 	return 0;
@@ -1904,6 +1984,7 @@ static int acer_platform_profile_setup(struct platform_device *device)
 {
 	if (quirks->predator_v4) {
 		int err;
+		u8 supported_profiles;
 
 		platform_profile_handler.name = "acer-wmi";
 		platform_profile_handler.dev = &device->dev;
@@ -1912,16 +1993,27 @@ static int acer_platform_profile_setup(struct platform_device *device)
 		platform_profile_handler.profile_set =
 			acer_predator_v4_platform_profile_set;
 
-		set_bit(PLATFORM_PROFILE_PERFORMANCE,
-			platform_profile_handler.choices);
-		set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE,
-			platform_profile_handler.choices);
-		set_bit(PLATFORM_PROFILE_BALANCED,
-			platform_profile_handler.choices);
-		set_bit(PLATFORM_PROFILE_QUIET,
-			platform_profile_handler.choices);
-		set_bit(PLATFORM_PROFILE_LOW_POWER,
-			platform_profile_handler.choices);
+		err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_SUPPORTED_PROFILES,
+						   &supported_profiles);
+		if (err)
+			return err;
+
+		if (supported_profiles & 1 << 0)
+			set_bit(PLATFORM_PROFILE_QUIET,
+				platform_profile_handler.choices);
+		if (supported_profiles & 1 << 1)
+			set_bit(PLATFORM_PROFILE_BALANCED,
+				platform_profile_handler.choices);
+		if (supported_profiles & 1 << 4)
+			set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE,
+				platform_profile_handler.choices);
+		if (supported_profiles & 1 << 5)
+			set_bit(PLATFORM_PROFILE_PERFORMANCE,
+				platform_profile_handler.choices);
+		if (supported_profiles & 1 << 6)
+			set_bit(PLATFORM_PROFILE_LOW_POWER,
+				platform_profile_handler.choices);
+
 
 		err = platform_profile_register(&platform_profile_handler);
 		if (err)
@@ -1931,7 +2023,7 @@ static int acer_platform_profile_setup(struct platform_device *device)
 
 		/* Set default non-turbo profile  */
 		last_non_turbo_profile =
-			ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
+			ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
 	}
 	return 0;
 }
@@ -1946,12 +2038,10 @@ static int acer_thermal_profile_change(void)
 		u8 current_tp;
 		int tp, err;
 		u64 on_AC;
-		acpi_status status;
-
-		err = ec_read(ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET,
-			      &current_tp);
 
-		if (err < 0)
+		err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE,
+						   &current_tp);
+		if (err)
 			return err;
 
 		/* Check power source */
@@ -1962,54 +2052,52 @@ static int acer_thermal_profile_change(void)
 		switch (current_tp) {
 		case ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO:
 			if (!on_AC)
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
+				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
 			else if (cycle_gaming_thermal_profile)
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO_WMI;
+				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
 			else
 				tp = last_non_turbo_profile;
 			break;
 		case ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE:
 			if (!on_AC)
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
+				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
 			else
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
+				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_WMI;
+				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
 			else if (cycle_gaming_thermal_profile)
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE_WMI;
+				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
 			else
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
+				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_WMI;
+				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
 			else if (cycle_gaming_thermal_profile)
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
+				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
 			else
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
+				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_WMI;
+				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
 			else if (cycle_gaming_thermal_profile)
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET_WMI;
+				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
 			else
-				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
+				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
 			break;
 		default:
 			return -EOPNOTSUPP;
 		}
 
-		status = WMI_gaming_execute_u64(
-			ACER_WMID_SET_GAMING_MISC_SETTING_METHODID, tp, NULL);
-
-		if (ACPI_FAILURE(status))
-			return -EIO;
+		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_WMI)
+		if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO)
 			last_non_turbo_profile = tp;
 
 		platform_profile_notify(&platform_profile_handler);
-- 
2.47.1
Re: [PATCH] platform/x86: acer-wmi: improve platform profile handling
Posted by Armin Wolf 1 year, 4 months ago 
Am 31.12.24 um 15:04 schrieb Hridesh MG:

> Two improvements to the platform profile handling for laptops using
> the Acer Predator interface -
>
> 1) Use WMI calls to fetch the current platform profile instead of
>     directly accessing it from the EC since the EC address differs for
>     certain laptops.
> 2) Use the supported profiles bitmap to dynamically set
>     platform_profile_choices.

Hi,

can you please split the second part into a separate patch? This would make review easier.

> Link: https://lore.kernel.org/platform-driver-x86/d7be714c-3103-42ee-ad15-223a3fe67f80@gmx.de/
> Co-developed-by: Armin Wolf <W_Armin@gmx.de>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> Signed-off-by: Hridesh MG <hridesh699@gmail.com>
> ---
> This patch has been tested on an acer nitro AN515-58 laptop, it would be
> good if someone with a predator laptop could also test it
> ---
>   drivers/platform/x86/acer-wmi.c | 220 ++++++++++++++++++++++----------
>   1 file changed, 154 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index b3043d78a7b3..37f629b6e3d3 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -31,6 +31,7 @@
>   #include <acpi/video.h>
>   #include <linux/hwmon.h>
>   #include <linux/units.h>
> +#include <linux/unaligned.h>
>   #include <linux/bitfield.h>
>
>   MODULE_AUTHOR("Carlos Corbacho");
> @@ -68,8 +69,11 @@ MODULE_LICENSE("GPL");
>   #define ACER_WMID_GET_GAMING_SYS_INFO_METHODID 5
>   #define ACER_WMID_SET_GAMING_FAN_BEHAVIOR 14
>   #define ACER_WMID_SET_GAMING_MISC_SETTING_METHODID 22
> +#define ACER_WMID_GET_GAMING_MISC_SETTING_METHODID 23
>
> -#define ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET 0x54
> +#define ACER_GAMING_MISC_SETTING_STATUS_MASK GENMASK_ULL(7, 0)
> +#define ACER_GAMING_MISC_SETTING_INDEX_MASK GENMASK_ULL(7, 0)
> +#define ACER_GAMING_MISC_SETTING_VALUE_MASK GENMASK_ULL(15, 8)
>
>   #define ACER_PREDATOR_V4_RETURN_STATUS_BIT_MASK GENMASK_ULL(7, 0)
>   #define ACER_PREDATOR_V4_SENSOR_INDEX_BIT_MASK GENMASK_ULL(15, 8)
> @@ -115,6 +119,13 @@ enum acer_wmi_predator_v4_sensor_id {
>   	ACER_WMID_SENSOR_GPU_TEMPERATURE	= 0x0A,
>   };
>
> +enum acer_wmi_gaming_misc_setting {
> +	ACER_WMID_MISC_SETTING_OC_1			= 0x0005,
> +	ACER_WMID_MISC_SETTING_OC_2			= 0x0007,
> +	ACER_WMID_MISC_SETTING_SUPPORTED_PROFILES	= 0x000A,
> +	ACER_WMID_MISC_SETTING_PLATFORM_PROFILE		= 0x000B,
> +};
> +
>   static const struct key_entry acer_wmi_keymap[] __initconst = {
>   	{KE_KEY, 0x01, {KEY_WLAN} },     /* WiFi */
>   	{KE_KEY, 0x03, {KEY_WLAN} },     /* WiFi */
> @@ -751,20 +762,12 @@ static bool platform_profile_support;
>    */
>   static int last_non_turbo_profile;
>
> -enum acer_predator_v4_thermal_profile_ec {
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_ECO = 0x04,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO = 0x03,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE = 0x02,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET = 0x01,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED = 0x00,
> -};
> -
> -enum acer_predator_v4_thermal_profile_wmi {
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_ECO_WMI = 0x060B,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI = 0x050B,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE_WMI = 0x040B,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET_WMI = 0x0B,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI = 0x010B,
> +enum acer_predator_v4_thermal_profile {
> +	ACER_PREDATOR_V4_THERMAL_PROFILE_ECO = 0x06,
> +	ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO = 0x05,
> +	ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE = 0x04,
> +	ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED = 0x01,
> +	ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET = 0x00,
>   };
>
>   /* Find which quirks are needed for a particular vendor/ model pair */
> @@ -1477,6 +1480,45 @@ WMI_gaming_execute_u64(u32 method_id, u64 in, u64 *out)
>   	return status;
>   }
>
> +static int WMI_gaming_execute_u32_u64(u32 method_id, u32 in, u64 *out)
> +{
> +	struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL };
> +	struct acpi_buffer input = {
> +		.length = sizeof(in),
> +		.pointer = &in,
> +	};
> +	union acpi_object *obj;
> +	acpi_status status;
> +	int ret = 0;
> +
> +	status = wmi_evaluate_method(WMID_GUID4, 0, method_id, &input, &result);
> +	if (ACPI_FAILURE(status))
> +		return -EIO;
> +
> +	obj = result.pointer;
> +	if (obj && out) {
> +		switch (obj->type) {
> +		case ACPI_TYPE_INTEGER:
> +			*out = obj->integer.value;
> +			break;
> +		case ACPI_TYPE_BUFFER:
> +			if (obj->buffer.length < sizeof(*out))
> +				ret = -ENOMSG;
> +			else
> +				*out = get_unaligned_le64(obj->buffer.pointer);
> +
> +			break;
> +		default:
> +			ret = -ENOMSG;
> +			break;
> +		}
> +	}
> +
> +	kfree(obj);
> +
> +	return ret;
> +}
> +
>   static acpi_status WMID_gaming_set_u64(u64 value, u32 cap)
>   {
>   	u32 method_id = 0;
> @@ -1565,6 +1607,48 @@ static void WMID_gaming_set_fan_mode(u8 fan_mode)
>   	WMID_gaming_set_u64(gpu_fan_config2 | gpu_fan_config1 << 16, ACER_CAP_TURBO_FAN);
>   }
>
> +static int WMID_gaming_set_misc_setting(enum acer_wmi_gaming_misc_setting setting, u8 value)
> +{
> +	acpi_status status;
> +	u64 input = 0;
> +	u64 result;
> +
> +	input |= FIELD_PREP(ACER_GAMING_MISC_SETTING_INDEX_MASK, setting);
> +	input |= FIELD_PREP(ACER_GAMING_MISC_SETTING_VALUE_MASK, value);
> +
> +	status = WMI_gaming_execute_u64(ACER_WMID_SET_GAMING_MISC_SETTING_METHODID, input, &result);
> +	if (ACPI_FAILURE(status))
> +		return -EIO;
> +
> +	/* The return status must be zero for the operation to have succeeded */
> +	if (FIELD_GET(ACER_GAMING_MISC_SETTING_STATUS_MASK, result))
> +		return -EIO;
> +
> +	return 0;
> +}
> +
> +static int WMID_gaming_get_misc_setting(enum acer_wmi_gaming_misc_setting setting, u8 *value)
> +{
> +	u64 input = 0;
> +	u64 result;
> +	int ret;
> +
> +	input |= FIELD_PREP(ACER_GAMING_MISC_SETTING_INDEX_MASK, setting);
> +
> +	ret = WMI_gaming_execute_u32_u64(ACER_WMID_GET_GAMING_MISC_SETTING_METHODID, input,
> +					 &result);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* The return status must be zero for the operation to have succeeded */
> +	if (FIELD_GET(ACER_GAMING_MISC_SETTING_STATUS_MASK, result))
> +		return -EIO;
> +
> +	*value = FIELD_GET(ACER_GAMING_MISC_SETTING_VALUE_MASK, result);
> +
> +	return 0;
> +}
> +
>   /*
>    * Generic Device (interface-independent)
>    */
> @@ -1833,9 +1917,8 @@ acer_predator_v4_platform_profile_get(struct platform_profile_handler *pprof,
>   	u8 tp;
>   	int err;
>
> -	err = ec_read(ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET, &tp);
> -
> -	if (err < 0)
> +	err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE, &tp);
> +	if (err)
>   		return err;
>
>   	switch (tp) {
> @@ -1865,36 +1948,33 @@ static int
>   acer_predator_v4_platform_profile_set(struct platform_profile_handler *pprof,
>   				      enum platform_profile_option profile)
>   {
> -	int tp;
> -	acpi_status status;
> +	int tp, err;
>
>   	switch (profile) {
>   	case PLATFORM_PROFILE_PERFORMANCE:
> -		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> +		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
>   		break;
>   	case PLATFORM_PROFILE_BALANCED_PERFORMANCE:
> -		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE_WMI;
> +		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
>   		break;
>   	case PLATFORM_PROFILE_BALANCED:
> -		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> +		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>   		break;
>   	case PLATFORM_PROFILE_QUIET:
> -		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET_WMI;
> +		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
>   		break;
>   	case PLATFORM_PROFILE_LOW_POWER:
> -		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO_WMI;
> +		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
>   		break;
>   	default:
>   		return -EOPNOTSUPP;
>   	}
>
> -	status = WMI_gaming_execute_u64(
> -		ACER_WMID_SET_GAMING_MISC_SETTING_METHODID, tp, NULL);
> -
> -	if (ACPI_FAILURE(status))
> -		return -EIO;
> +	err = WMID_gaming_set_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE, tp);
> +	if (err)
> +		return err;
>
> -	if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI)
> +	if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO)
>   		last_non_turbo_profile = tp;
>
>   	return 0;
> @@ -1904,6 +1984,7 @@ static int acer_platform_profile_setup(struct platform_device *device)
>   {
>   	if (quirks->predator_v4) {
>   		int err;
> +		u8 supported_profiles;

Please order the variables in reverse xmas tree order, which means that the long declarations come first.

>
>   		platform_profile_handler.name = "acer-wmi";
>   		platform_profile_handler.dev = &device->dev;
> @@ -1912,16 +1993,27 @@ static int acer_platform_profile_setup(struct platform_device *device)
>   		platform_profile_handler.profile_set =
>   			acer_predator_v4_platform_profile_set;
>
> -		set_bit(PLATFORM_PROFILE_PERFORMANCE,
> -			platform_profile_handler.choices);
> -		set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE,
> -			platform_profile_handler.choices);
> -		set_bit(PLATFORM_PROFILE_BALANCED,
> -			platform_profile_handler.choices);
> -		set_bit(PLATFORM_PROFILE_QUIET,
> -			platform_profile_handler.choices);
> -		set_bit(PLATFORM_PROFILE_LOW_POWER,
> -			platform_profile_handler.choices);
> +		err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_SUPPORTED_PROFILES,
> +						   &supported_profiles);
> +		if (err)
> +			return err;
> +
> +		if (supported_profiles & 1 << 0)
> +			set_bit(PLATFORM_PROFILE_QUIET,
> +				platform_profile_handler.choices);
> +		if (supported_profiles & 1 << 1)
> +			set_bit(PLATFORM_PROFILE_BALANCED,
> +				platform_profile_handler.choices);
> +		if (supported_profiles & 1 << 4)
> +			set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE,
> +				platform_profile_handler.choices);
> +		if (supported_profiles & 1 << 5)
> +			set_bit(PLATFORM_PROFILE_PERFORMANCE,
> +				platform_profile_handler.choices);
> +		if (supported_profiles & 1 << 6)
> +			set_bit(PLATFORM_PROFILE_LOW_POWER,
> +				platform_profile_handler.choices);

Please use the appropriated values from "enum acer_predator_v4_thermal_profile" instead of those
magic numbers.

> +
>
>   		err = platform_profile_register(&platform_profile_handler);
>   		if (err)
> @@ -1931,7 +2023,7 @@ static int acer_platform_profile_setup(struct platform_device *device)
>
>   		/* Set default non-turbo profile  */
>   		last_non_turbo_profile =
> -			ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> +			ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>   	}
>   	return 0;
>   }
> @@ -1946,12 +2038,10 @@ static int acer_thermal_profile_change(void)
>   		u8 current_tp;
>   		int tp, err;
>   		u64 on_AC;
> -		acpi_status status;
> -
> -		err = ec_read(ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET,
> -			      &current_tp);
>
> -		if (err < 0)
> +		err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE,
> +						   &current_tp);
> +		if (err)
>   			return err;
>
>   		/* Check power source */
> @@ -1962,54 +2052,52 @@ static int acer_thermal_profile_change(void)
>   		switch (current_tp) {
>   		case ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO:
>   			if (!on_AC)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>   			else if (cycle_gaming_thermal_profile)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
>   			else
>   				tp = last_non_turbo_profile;
>   			break;
>   		case ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE:
>   			if (!on_AC)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>   			else
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> +				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_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
>   			else if (cycle_gaming_thermal_profile)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
>   			else
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> +				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_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>   			else if (cycle_gaming_thermal_profile)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>   			else
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> +				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_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>   			else if (cycle_gaming_thermal_profile)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
>   			else
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
>   			break;
>   		default:
>   			return -EOPNOTSUPP;
>   		}

This needs a bit more work, since you might accidentally select unsupported platform profiles this way.

Using platform_profile_cycle() would make sense here, but we still need to handle "on_AC". I however wonder
if that is really necessary.

I CCed the original patch author hoping that he can give us some background information here.

Thanks,
Armin Wolf

>
> -		status = WMI_gaming_execute_u64(
> -			ACER_WMID_SET_GAMING_MISC_SETTING_METHODID, tp, NULL);
> -
> -		if (ACPI_FAILURE(status))
> -			return -EIO;
> +		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_WMI)
> +		if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO)
>   			last_non_turbo_profile = tp;
>
>   		platform_profile_notify(&platform_profile_handler);
Re: [PATCH] platform/x86: acer-wmi: improve platform profile handling
Posted by Hridesh MG 1 year, 4 months ago 
On Thu, Jan 2, 2025 at 2:28 AM Armin Wolf <W_Armin@gmx.de> wrote:
> > @@ -1946,12 +2038,10 @@ static int acer_thermal_profile_change(void)
> >               u8 current_tp;
> >               int tp, err;
> >               u64 on_AC;
> > -             acpi_status status;
> > -
> > -             err = ec_read(ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET,
> > -                           &current_tp);
> >
> > -             if (err < 0)
> > +             err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE,
> > +                                                &current_tp);
> > +             if (err)
> >                       return err;
> >
> >               /* Check power source */
> > @@ -1962,54 +2052,52 @@ static int acer_thermal_profile_change(void)
> >               switch (current_tp) {
> >               case ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO:
> >                       if (!on_AC)
> > -                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> > +                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> >                       else if (cycle_gaming_thermal_profile)
> > -                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO_WMI;
> > +                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
> >                       else
> >                               tp = last_non_turbo_profile;
> >                       break;
> >               case ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE:
> >                       if (!on_AC)
> > -                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> > +                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> >                       else
> > -                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> > +                             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_WMI;
> > +                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
> >                       else if (cycle_gaming_thermal_profile)
> > -                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE_WMI;
> > +                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
> >                       else
> > -                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> > +                             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_WMI;
> > +                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> >                       else if (cycle_gaming_thermal_profile)
> > -                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> > +                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> >                       else
> > -                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> > +                             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_WMI;
> > +                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> >                       else if (cycle_gaming_thermal_profile)
> > -                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET_WMI;
> > +                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
> >                       else
> > -                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> > +                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
> >                       break;
> >               default:
> >                       return -EOPNOTSUPP;
> >               }
>
> This needs a bit more work, since you might accidentally select unsupported platform profiles this way.
>
> Using platform_profile_cycle() would make sense here, but we still need to handle "on_AC". I however wonder
> if that is really necessary.
>
From my testing, what I've found out is that even if we don't handle
on_AC, the profile is still set but the hardware will not put the
changes into effect until we plug in AC.

I've incorporated the rest of the changes into v2, thanks for the feedback!

--
Thanks,
Hridesh MG
Re: [PATCH] platform/x86: acer-wmi: improve platform profile handling
Posted by Kurt Borja 1 year, 4 months ago 
On Thu, Jan 02, 2025 at 12:20:36PM +0530, Hridesh MG wrote:
> On Thu, Jan 2, 2025 at 2:28 AM Armin Wolf <W_Armin@gmx.de> wrote:
> > > @@ -1946,12 +2038,10 @@ static int acer_thermal_profile_change(void)
> > >               u8 current_tp;
> > >               int tp, err;
> > >               u64 on_AC;
> > > -             acpi_status status;
> > > -
> > > -             err = ec_read(ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET,
> > > -                           &current_tp);
> > >
> > > -             if (err < 0)
> > > +             err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE,
> > > +                                                &current_tp);
> > > +             if (err)
> > >                       return err;
> > >
> > >               /* Check power source */
> > > @@ -1962,54 +2052,52 @@ static int acer_thermal_profile_change(void)
> > >               switch (current_tp) {
> > >               case ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO:
> > >                       if (!on_AC)
> > > -                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> > > +                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> > >                       else if (cycle_gaming_thermal_profile)
> > > -                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO_WMI;
> > > +                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
> > >                       else
> > >                               tp = last_non_turbo_profile;
> > >                       break;
> > >               case ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE:
> > >                       if (!on_AC)
> > > -                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> > > +                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> > >                       else
> > > -                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> > > +                             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_WMI;
> > > +                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
> > >                       else if (cycle_gaming_thermal_profile)
> > > -                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE_WMI;
> > > +                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
> > >                       else
> > > -                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> > > +                             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_WMI;
> > > +                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> > >                       else if (cycle_gaming_thermal_profile)
> > > -                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> > > +                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> > >                       else
> > > -                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> > > +                             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_WMI;
> > > +                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> > >                       else if (cycle_gaming_thermal_profile)
> > > -                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET_WMI;
> > > +                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
> > >                       else
> > > -                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> > > +                             tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
> > >                       break;
> > >               default:
> > >                       return -EOPNOTSUPP;
> > >               }
> >
> > This needs a bit more work, since you might accidentally select unsupported platform profiles this way.
> >
> > Using platform_profile_cycle() would make sense here, but we still need to handle "on_AC". I however wonder
> > if that is really necessary.
> >
> >From my testing, what I've found out is that even if we don't handle
> on_AC, the profile is still set but the hardware will not put the
> changes into effect until we plug in AC.

This is interesting.

Do you perhaps know if users expect a thermal profile change on AC
connect/disconnect events on Windows? This would solve this discussion
[1].

~ Kurt

[1] https://lore.kernel.org/platform-driver-x86/20241210001657.3362-6-W_Armin@gmx.de/

> 
> I've incorporated the rest of the changes into v2, thanks for the feedback!
> 
> --
> Thanks,
> Hridesh MG
Re: [PATCH] platform/x86: acer-wmi: improve platform profile handling
Posted by Hridesh MG 1 year, 4 months ago 
> Do you perhaps know if users expect a thermal profile change on AC
> connect/disconnect events on Windows? This would solve this discussion
> [1].
Yep, there is a profile change on AC disconnect on Windows.
Specifically, it forces the balanced platform profile and returns to
the last active profile on replugging (the UI disallows changing of
profiles when disconnected but it is possible via WMI)

> From my testing, what I've found out is that even if we don't handle
> on_AC, the profile is still set but the hardware will not put the
> changes into effect until we plug in AC.
I did some more testing and I was a bit mistaken in the way it works.
While the system is unplugged it will still apply the three different
profiles (as evident by the differing fan speeds) but the CPU will
aggressively throttle under stress, placing an upper limit on the
maximum clock rate, this limit is lifted upon plugging in AC.

--
Thanks,
Hridesh MG
Re: [PATCH] platform/x86: acer-wmi: improve platform profile handling
Posted by Armin Wolf 1 year, 4 months ago 
Am 03.01.25 um 20:52 schrieb Hridesh MG:

>> Do you perhaps know if users expect a thermal profile change on AC
>> connect/disconnect events on Windows? This would solve this discussion
>> [1].
> Yep, there is a profile change on AC disconnect on Windows.
> Specifically, it forces the balanced platform profile and returns to
> the last active profile on replugging (the UI disallows changing of
> profiles when disconnected but it is possible via WMI)
>
>>  From my testing, what I've found out is that even if we don't handle
>> on_AC, the profile is still set but the hardware will not put the
>> changes into effect until we plug in AC.
> I did some more testing and I was a bit mistaken in the way it works.
> While the system is unplugged it will still apply the three different
> profiles (as evident by the differing fan speeds) but the CPU will
> aggressively throttle under stress, placing an upper limit on the
> maximum clock rate, this limit is lifted upon plugging in AC.

That is some very important information right here. The platform-profile documentation states:

	"Specifically when selecting a high performance profile the actual achieved
	 performance may be limited by various factors such as: the heat generated
	 by other components, room temperature, free air flow at the bottom of a
	 laptop, etc. It is explicitly NOT a goal of this API to let userspace know
	 about any sub-optimal conditions which are impeding reaching the requested
	 performance level."

I think the AC handling is unnecessary in this case as the hardware seems to accept different profiles
even when not running on AC. This would simplify the platform profile cycling inside the driver and
allow us to use platform_profile_cycle().

I wonder if this special behavior of the acer-wmi driver is documented somewhere. I am asking this since
this has a great potential to confuse users.

Thanks,
Armin Wolf

>
> --
> Thanks,
> Hridesh MG
Re: [PATCH] platform/x86: acer-wmi: improve platform profile handling
Posted by SungHwan Jung 1 year, 4 months ago 

On 1/4/25 06:07, Armin Wolf wrote:

> That is some very important information right here. The platform-profile
> documentation states:
> 
>     "Specifically when selecting a high performance profile the actual
> achieved
>      performance may be limited by various factors such as: the heat
> generated
>      by other components, room temperature, free air flow at the bottom
> of a
>      laptop, etc. It is explicitly NOT a goal of this API to let
> userspace know
>      about any sub-optimal conditions which are impeding reaching the
> requested
>      performance level."
> 
> I think the AC handling is unnecessary in this case as the hardware
> seems to accept different profiles
> even when not running on AC. This would simplify the platform profile
> cycling inside the driver and
> allow us to use platform_profile_cycle().
> 
> I wonder if this special behavior of the acer-wmi driver is documented
> somewhere. I am asking this since
> this has a great potential to confuse users.
The acer_thermal_profile_change function was made for predator phn16-71
and tested with some predator laptop series.

These predator series have the mode button that toggles between turbo or
non-turbo or cycles each modes. And the predator sense app restricts
these predator series to Eco and Balanced mode on battery.
So this function includes on_AC to reflect these behaviors.(mentioned on
comments)

platform_profile_cycle() is suitable for cycle mode by
cycle_gaming_thermal_profile parameter and if there is no reason to
restrict thermal profiles on battery, we don't need to handle on_AC. But
I'm not sure why the manufacturer restricts the thermal profiles on
battery on windows.

Thanks,
SungHwan Jung
> 
> Thanks,
> Armin Wolf
Re: [PATCH] platform/x86: acer-wmi: improve platform profile handling
Posted by Kurt Borja 1 year, 4 months ago 
On Tue, Dec 31, 2024 at 07:34:41PM +0530, Hridesh MG wrote:
> Two improvements to the platform profile handling for laptops using
> the Acer Predator interface -
> 
> 1) Use WMI calls to fetch the current platform profile instead of
>    directly accessing it from the EC since the EC address differs for
>    certain laptops.
> 2) Use the supported profiles bitmap to dynamically set
>    platform_profile_choices.
> 
> Link: https://lore.kernel.org/platform-driver-x86/d7be714c-3103-42ee-ad15-223a3fe67f80@gmx.de/
> Co-developed-by: Armin Wolf <W_Armin@gmx.de>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> Signed-off-by: Hridesh MG <hridesh699@gmail.com>
> ---
> This patch has been tested on an acer nitro AN515-58 laptop, it would be
> good if someone with a predator laptop could also test it
> ---
>  drivers/platform/x86/acer-wmi.c | 220 ++++++++++++++++++++++----------
>  1 file changed, 154 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index b3043d78a7b3..37f629b6e3d3 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -31,6 +31,7 @@
>  #include <acpi/video.h>
>  #include <linux/hwmon.h>
>  #include <linux/units.h>
> +#include <linux/unaligned.h>
>  #include <linux/bitfield.h>
>  
>  MODULE_AUTHOR("Carlos Corbacho");
> @@ -68,8 +69,11 @@ MODULE_LICENSE("GPL");
>  #define ACER_WMID_GET_GAMING_SYS_INFO_METHODID 5
>  #define ACER_WMID_SET_GAMING_FAN_BEHAVIOR 14
>  #define ACER_WMID_SET_GAMING_MISC_SETTING_METHODID 22
> +#define ACER_WMID_GET_GAMING_MISC_SETTING_METHODID 23
>  
> -#define ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET 0x54
> +#define ACER_GAMING_MISC_SETTING_STATUS_MASK GENMASK_ULL(7, 0)
> +#define ACER_GAMING_MISC_SETTING_INDEX_MASK GENMASK_ULL(7, 0)
> +#define ACER_GAMING_MISC_SETTING_VALUE_MASK GENMASK_ULL(15, 8)
>  
>  #define ACER_PREDATOR_V4_RETURN_STATUS_BIT_MASK GENMASK_ULL(7, 0)
>  #define ACER_PREDATOR_V4_SENSOR_INDEX_BIT_MASK GENMASK_ULL(15, 8)
> @@ -115,6 +119,13 @@ enum acer_wmi_predator_v4_sensor_id {
>  	ACER_WMID_SENSOR_GPU_TEMPERATURE	= 0x0A,
>  };
>  
> +enum acer_wmi_gaming_misc_setting {
> +	ACER_WMID_MISC_SETTING_OC_1			= 0x0005,
> +	ACER_WMID_MISC_SETTING_OC_2			= 0x0007,
> +	ACER_WMID_MISC_SETTING_SUPPORTED_PROFILES	= 0x000A,
> +	ACER_WMID_MISC_SETTING_PLATFORM_PROFILE		= 0x000B,
> +};
> +
>  static const struct key_entry acer_wmi_keymap[] __initconst = {
>  	{KE_KEY, 0x01, {KEY_WLAN} },     /* WiFi */
>  	{KE_KEY, 0x03, {KEY_WLAN} },     /* WiFi */
> @@ -751,20 +762,12 @@ static bool platform_profile_support;
>   */
>  static int last_non_turbo_profile;
>  
> -enum acer_predator_v4_thermal_profile_ec {
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_ECO = 0x04,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO = 0x03,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE = 0x02,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET = 0x01,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED = 0x00,
> -};
> -
> -enum acer_predator_v4_thermal_profile_wmi {
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_ECO_WMI = 0x060B,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI = 0x050B,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE_WMI = 0x040B,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET_WMI = 0x0B,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI = 0x010B,
> +enum acer_predator_v4_thermal_profile {
> +	ACER_PREDATOR_V4_THERMAL_PROFILE_ECO = 0x06,
> +	ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO = 0x05,
> +	ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE = 0x04,
> +	ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED = 0x01,
> +	ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET = 0x00,

I forgot to say, these should also be aligned.

~ Kurt

>  };
>  
>  /* Find which quirks are needed for a particular vendor/ model pair */
> @@ -1477,6 +1480,45 @@ WMI_gaming_execute_u64(u32 method_id, u64 in, u64 *out)
>  	return status;
>  }
>  
> +static int WMI_gaming_execute_u32_u64(u32 method_id, u32 in, u64 *out)
> +{
> +	struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL };
> +	struct acpi_buffer input = {
> +		.length = sizeof(in),
> +		.pointer = &in,
> +	};
> +	union acpi_object *obj;
> +	acpi_status status;
> +	int ret = 0;
> +
> +	status = wmi_evaluate_method(WMID_GUID4, 0, method_id, &input, &result);
> +	if (ACPI_FAILURE(status))
> +		return -EIO;
> +
> +	obj = result.pointer;
> +	if (obj && out) {
> +		switch (obj->type) {
> +		case ACPI_TYPE_INTEGER:
> +			*out = obj->integer.value;
> +			break;
> +		case ACPI_TYPE_BUFFER:
> +			if (obj->buffer.length < sizeof(*out))
> +				ret = -ENOMSG;
> +			else
> +				*out = get_unaligned_le64(obj->buffer.pointer);
> +
> +			break;
> +		default:
> +			ret = -ENOMSG;
> +			break;
> +		}
> +	}
> +
> +	kfree(obj);
> +
> +	return ret;
> +}
> +
>  static acpi_status WMID_gaming_set_u64(u64 value, u32 cap)
>  {
>  	u32 method_id = 0;
> @@ -1565,6 +1607,48 @@ static void WMID_gaming_set_fan_mode(u8 fan_mode)
>  	WMID_gaming_set_u64(gpu_fan_config2 | gpu_fan_config1 << 16, ACER_CAP_TURBO_FAN);
>  }
>  
> +static int WMID_gaming_set_misc_setting(enum acer_wmi_gaming_misc_setting setting, u8 value)
> +{
> +	acpi_status status;
> +	u64 input = 0;
> +	u64 result;
> +
> +	input |= FIELD_PREP(ACER_GAMING_MISC_SETTING_INDEX_MASK, setting);
> +	input |= FIELD_PREP(ACER_GAMING_MISC_SETTING_VALUE_MASK, value);
> +
> +	status = WMI_gaming_execute_u64(ACER_WMID_SET_GAMING_MISC_SETTING_METHODID, input, &result);
> +	if (ACPI_FAILURE(status))
> +		return -EIO;
> +
> +	/* The return status must be zero for the operation to have succeeded */
> +	if (FIELD_GET(ACER_GAMING_MISC_SETTING_STATUS_MASK, result))
> +		return -EIO;
> +
> +	return 0;
> +}
> +
> +static int WMID_gaming_get_misc_setting(enum acer_wmi_gaming_misc_setting setting, u8 *value)
> +{
> +	u64 input = 0;
> +	u64 result;
> +	int ret;
> +
> +	input |= FIELD_PREP(ACER_GAMING_MISC_SETTING_INDEX_MASK, setting);
> +
> +	ret = WMI_gaming_execute_u32_u64(ACER_WMID_GET_GAMING_MISC_SETTING_METHODID, input,
> +					 &result);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* The return status must be zero for the operation to have succeeded */
> +	if (FIELD_GET(ACER_GAMING_MISC_SETTING_STATUS_MASK, result))
> +		return -EIO;
> +
> +	*value = FIELD_GET(ACER_GAMING_MISC_SETTING_VALUE_MASK, result);
> +
> +	return 0;
> +}
> +
>  /*
>   * Generic Device (interface-independent)
>   */
> @@ -1833,9 +1917,8 @@ acer_predator_v4_platform_profile_get(struct platform_profile_handler *pprof,
>  	u8 tp;
>  	int err;
>  
> -	err = ec_read(ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET, &tp);
> -
> -	if (err < 0)
> +	err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE, &tp);
> +	if (err)
>  		return err;
>  
>  	switch (tp) {
> @@ -1865,36 +1948,33 @@ static int
>  acer_predator_v4_platform_profile_set(struct platform_profile_handler *pprof,
>  				      enum platform_profile_option profile)
>  {
> -	int tp;
> -	acpi_status status;
> +	int tp, err;
>  
>  	switch (profile) {
>  	case PLATFORM_PROFILE_PERFORMANCE:
> -		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> +		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
>  		break;
>  	case PLATFORM_PROFILE_BALANCED_PERFORMANCE:
> -		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE_WMI;
> +		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
>  		break;
>  	case PLATFORM_PROFILE_BALANCED:
> -		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> +		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>  		break;
>  	case PLATFORM_PROFILE_QUIET:
> -		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET_WMI;
> +		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
>  		break;
>  	case PLATFORM_PROFILE_LOW_POWER:
> -		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO_WMI;
> +		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
>  		break;
>  	default:
>  		return -EOPNOTSUPP;
>  	}
>  
> -	status = WMI_gaming_execute_u64(
> -		ACER_WMID_SET_GAMING_MISC_SETTING_METHODID, tp, NULL);
> -
> -	if (ACPI_FAILURE(status))
> -		return -EIO;
> +	err = WMID_gaming_set_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE, tp);
> +	if (err)
> +		return err;
>  
> -	if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI)
> +	if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO)
>  		last_non_turbo_profile = tp;
>  
>  	return 0;
> @@ -1904,6 +1984,7 @@ static int acer_platform_profile_setup(struct platform_device *device)
>  {
>  	if (quirks->predator_v4) {
>  		int err;
> +		u8 supported_profiles;
>  
>  		platform_profile_handler.name = "acer-wmi";
>  		platform_profile_handler.dev = &device->dev;
> @@ -1912,16 +1993,27 @@ static int acer_platform_profile_setup(struct platform_device *device)
>  		platform_profile_handler.profile_set =
>  			acer_predator_v4_platform_profile_set;
>  
> -		set_bit(PLATFORM_PROFILE_PERFORMANCE,
> -			platform_profile_handler.choices);
> -		set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE,
> -			platform_profile_handler.choices);
> -		set_bit(PLATFORM_PROFILE_BALANCED,
> -			platform_profile_handler.choices);
> -		set_bit(PLATFORM_PROFILE_QUIET,
> -			platform_profile_handler.choices);
> -		set_bit(PLATFORM_PROFILE_LOW_POWER,
> -			platform_profile_handler.choices);
> +		err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_SUPPORTED_PROFILES,
> +						   &supported_profiles);
> +		if (err)
> +			return err;
> +
> +		if (supported_profiles & 1 << 0)
> +			set_bit(PLATFORM_PROFILE_QUIET,
> +				platform_profile_handler.choices);
> +		if (supported_profiles & 1 << 1)
> +			set_bit(PLATFORM_PROFILE_BALANCED,
> +				platform_profile_handler.choices);
> +		if (supported_profiles & 1 << 4)
> +			set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE,
> +				platform_profile_handler.choices);
> +		if (supported_profiles & 1 << 5)
> +			set_bit(PLATFORM_PROFILE_PERFORMANCE,
> +				platform_profile_handler.choices);
> +		if (supported_profiles & 1 << 6)
> +			set_bit(PLATFORM_PROFILE_LOW_POWER,
> +				platform_profile_handler.choices);
> +
>  
>  		err = platform_profile_register(&platform_profile_handler);
>  		if (err)
> @@ -1931,7 +2023,7 @@ static int acer_platform_profile_setup(struct platform_device *device)
>  
>  		/* Set default non-turbo profile  */
>  		last_non_turbo_profile =
> -			ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> +			ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>  	}
>  	return 0;
>  }
> @@ -1946,12 +2038,10 @@ static int acer_thermal_profile_change(void)
>  		u8 current_tp;
>  		int tp, err;
>  		u64 on_AC;
> -		acpi_status status;
> -
> -		err = ec_read(ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET,
> -			      &current_tp);
>  
> -		if (err < 0)
> +		err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE,
> +						   &current_tp);
> +		if (err)
>  			return err;
>  
>  		/* Check power source */
> @@ -1962,54 +2052,52 @@ static int acer_thermal_profile_change(void)
>  		switch (current_tp) {
>  		case ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO:
>  			if (!on_AC)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>  			else if (cycle_gaming_thermal_profile)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
>  			else
>  				tp = last_non_turbo_profile;
>  			break;
>  		case ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE:
>  			if (!on_AC)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>  			else
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> +				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_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
>  			else if (cycle_gaming_thermal_profile)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
>  			else
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> +				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_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>  			else if (cycle_gaming_thermal_profile)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>  			else
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> +				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_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>  			else if (cycle_gaming_thermal_profile)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
>  			else
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
>  			break;
>  		default:
>  			return -EOPNOTSUPP;
>  		}
>  
> -		status = WMI_gaming_execute_u64(
> -			ACER_WMID_SET_GAMING_MISC_SETTING_METHODID, tp, NULL);
> -
> -		if (ACPI_FAILURE(status))
> -			return -EIO;
> +		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_WMI)
> +		if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO)
>  			last_non_turbo_profile = tp;
>  
>  		platform_profile_notify(&platform_profile_handler);
Re: [PATCH] platform/x86: acer-wmi: improve platform profile handling
Posted by Kurt Borja 1 year, 4 months ago 
On Tue, Dec 31, 2024 at 07:34:41PM +0530, Hridesh MG wrote:
> Two improvements to the platform profile handling for laptops using
> the Acer Predator interface -
> 
> 1) Use WMI calls to fetch the current platform profile instead of
>    directly accessing it from the EC since the EC address differs for
>    certain laptops.
> 2) Use the supported profiles bitmap to dynamically set
>    platform_profile_choices.

Hi Hridesh,

> 
> Link: https://lore.kernel.org/platform-driver-x86/d7be714c-3103-42ee-ad15-223a3fe67f80@gmx.de/
> Co-developed-by: Armin Wolf <W_Armin@gmx.de>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> Signed-off-by: Hridesh MG <hridesh699@gmail.com>
> ---
> This patch has been tested on an acer nitro AN515-58 laptop, it would be
> good if someone with a predator laptop could also test it
> ---
>  drivers/platform/x86/acer-wmi.c | 220 ++++++++++++++++++++++----------
>  1 file changed, 154 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index b3043d78a7b3..37f629b6e3d3 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -31,6 +31,7 @@
>  #include <acpi/video.h>
>  #include <linux/hwmon.h>
>  #include <linux/units.h>
> +#include <linux/unaligned.h>
>  #include <linux/bitfield.h>
>  
>  MODULE_AUTHOR("Carlos Corbacho");
> @@ -68,8 +69,11 @@ MODULE_LICENSE("GPL");
>  #define ACER_WMID_GET_GAMING_SYS_INFO_METHODID 5
>  #define ACER_WMID_SET_GAMING_FAN_BEHAVIOR 14
>  #define ACER_WMID_SET_GAMING_MISC_SETTING_METHODID 22
> +#define ACER_WMID_GET_GAMING_MISC_SETTING_METHODID 23
>  
> -#define ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET 0x54
> +#define ACER_GAMING_MISC_SETTING_STATUS_MASK GENMASK_ULL(7, 0)
> +#define ACER_GAMING_MISC_SETTING_INDEX_MASK GENMASK_ULL(7, 0)
> +#define ACER_GAMING_MISC_SETTING_VALUE_MASK GENMASK_ULL(15, 8)
>  
>  #define ACER_PREDATOR_V4_RETURN_STATUS_BIT_MASK GENMASK_ULL(7, 0)
>  #define ACER_PREDATOR_V4_SENSOR_INDEX_BIT_MASK GENMASK_ULL(15, 8)
> @@ -115,6 +119,13 @@ enum acer_wmi_predator_v4_sensor_id {
>  	ACER_WMID_SENSOR_GPU_TEMPERATURE	= 0x0A,
>  };
>  
> +enum acer_wmi_gaming_misc_setting {
> +	ACER_WMID_MISC_SETTING_OC_1			= 0x0005,
> +	ACER_WMID_MISC_SETTING_OC_2			= 0x0007,
> +	ACER_WMID_MISC_SETTING_SUPPORTED_PROFILES	= 0x000A,

Should this be GETTING?

> +	ACER_WMID_MISC_SETTING_PLATFORM_PROFILE		= 0x000B,
> +};
> +
>  static const struct key_entry acer_wmi_keymap[] __initconst = {
>  	{KE_KEY, 0x01, {KEY_WLAN} },     /* WiFi */
>  	{KE_KEY, 0x03, {KEY_WLAN} },     /* WiFi */
> @@ -751,20 +762,12 @@ static bool platform_profile_support;
>   */
>  static int last_non_turbo_profile;
>  
> -enum acer_predator_v4_thermal_profile_ec {
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_ECO = 0x04,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO = 0x03,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE = 0x02,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET = 0x01,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED = 0x00,
> -};
> -
> -enum acer_predator_v4_thermal_profile_wmi {
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_ECO_WMI = 0x060B,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI = 0x050B,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE_WMI = 0x040B,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET_WMI = 0x0B,
> -	ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI = 0x010B,
> +enum acer_predator_v4_thermal_profile {
> +	ACER_PREDATOR_V4_THERMAL_PROFILE_ECO = 0x06,
> +	ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO = 0x05,
> +	ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE = 0x04,
> +	ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED = 0x01,
> +	ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET = 0x00,

I think it's better if you sort these from least to greatest.

>  };
>  
>  /* Find which quirks are needed for a particular vendor/ model pair */
> @@ -1477,6 +1480,45 @@ WMI_gaming_execute_u64(u32 method_id, u64 in, u64 *out)
>  	return status;
>  }
>  
> +static int WMI_gaming_execute_u32_u64(u32 method_id, u32 in, u64 *out)
> +{
> +	struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL };
> +	struct acpi_buffer input = {
> +		.length = sizeof(in),
> +		.pointer = &in,
> +	};
> +	union acpi_object *obj;
> +	acpi_status status;
> +	int ret = 0;
> +
> +	status = wmi_evaluate_method(WMID_GUID4, 0, method_id, &input, &result);
> +	if (ACPI_FAILURE(status))
> +		return -EIO;
> +
> +	obj = result.pointer;
> +	if (obj && out) {
> +		switch (obj->type) {
> +		case ACPI_TYPE_INTEGER:
> +			*out = obj->integer.value;
> +			break;
> +		case ACPI_TYPE_BUFFER:
> +			if (obj->buffer.length < sizeof(*out))
> +				ret = -ENOMSG;
> +			else
> +				*out = get_unaligned_le64(obj->buffer.pointer);
> +
> +			break;
> +		default:
> +			ret = -ENOMSG;
> +			break;
> +		}
> +	}
> +
> +	kfree(obj);
> +
> +	return ret;
> +}
> +
>  static acpi_status WMID_gaming_set_u64(u64 value, u32 cap)
>  {
>  	u32 method_id = 0;
> @@ -1565,6 +1607,48 @@ static void WMID_gaming_set_fan_mode(u8 fan_mode)
>  	WMID_gaming_set_u64(gpu_fan_config2 | gpu_fan_config1 << 16, ACER_CAP_TURBO_FAN);
>  }
>  
> +static int WMID_gaming_set_misc_setting(enum acer_wmi_gaming_misc_setting setting, u8 value)
> +{
> +	acpi_status status;
> +	u64 input = 0;
> +	u64 result;
> +
> +	input |= FIELD_PREP(ACER_GAMING_MISC_SETTING_INDEX_MASK, setting);
> +	input |= FIELD_PREP(ACER_GAMING_MISC_SETTING_VALUE_MASK, value);
> +
> +	status = WMI_gaming_execute_u64(ACER_WMID_SET_GAMING_MISC_SETTING_METHODID, input, &result);
> +	if (ACPI_FAILURE(status))
> +		return -EIO;
> +
> +	/* The return status must be zero for the operation to have succeeded */
> +	if (FIELD_GET(ACER_GAMING_MISC_SETTING_STATUS_MASK, result))
> +		return -EIO;
> +
> +	return 0;
> +}
> +
> +static int WMID_gaming_get_misc_setting(enum acer_wmi_gaming_misc_setting setting, u8 *value)
> +{
> +	u64 input = 0;
> +	u64 result;
> +	int ret;
> +
> +	input |= FIELD_PREP(ACER_GAMING_MISC_SETTING_INDEX_MASK, setting);
> +
> +	ret = WMI_gaming_execute_u32_u64(ACER_WMID_GET_GAMING_MISC_SETTING_METHODID, input,
> +					 &result);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* The return status must be zero for the operation to have succeeded */
> +	if (FIELD_GET(ACER_GAMING_MISC_SETTING_STATUS_MASK, result))
> +		return -EIO;
> +
> +	*value = FIELD_GET(ACER_GAMING_MISC_SETTING_VALUE_MASK, result);
> +
> +	return 0;
> +}
> +
>  /*
>   * Generic Device (interface-independent)
>   */
> @@ -1833,9 +1917,8 @@ acer_predator_v4_platform_profile_get(struct platform_profile_handler *pprof,
>  	u8 tp;
>  	int err;
>  
> -	err = ec_read(ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET, &tp);
> -
> -	if (err < 0)
> +	err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE, &tp);
> +	if (err)
>  		return err;
>  
>  	switch (tp) {
> @@ -1865,36 +1948,33 @@ static int
>  acer_predator_v4_platform_profile_set(struct platform_profile_handler *pprof,
>  				      enum platform_profile_option profile)
>  {
> -	int tp;
> -	acpi_status status;
> +	int tp, err;
>  
>  	switch (profile) {
>  	case PLATFORM_PROFILE_PERFORMANCE:
> -		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> +		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
>  		break;
>  	case PLATFORM_PROFILE_BALANCED_PERFORMANCE:
> -		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE_WMI;
> +		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
>  		break;
>  	case PLATFORM_PROFILE_BALANCED:
> -		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> +		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>  		break;
>  	case PLATFORM_PROFILE_QUIET:
> -		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET_WMI;
> +		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
>  		break;
>  	case PLATFORM_PROFILE_LOW_POWER:
> -		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO_WMI;
> +		tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
>  		break;
>  	default:
>  		return -EOPNOTSUPP;
>  	}
>  
> -	status = WMI_gaming_execute_u64(
> -		ACER_WMID_SET_GAMING_MISC_SETTING_METHODID, tp, NULL);
> -
> -	if (ACPI_FAILURE(status))
> -		return -EIO;
> +	err = WMID_gaming_set_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE, tp);
> +	if (err)
> +		return err;
>  
> -	if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI)
> +	if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO)
>  		last_non_turbo_profile = tp;
>  
>  	return 0;
> @@ -1904,6 +1984,7 @@ static int acer_platform_profile_setup(struct platform_device *device)
>  {
>  	if (quirks->predator_v4) {
>  		int err;
> +		u8 supported_profiles;
>  
>  		platform_profile_handler.name = "acer-wmi";
>  		platform_profile_handler.dev = &device->dev;
> @@ -1912,16 +1993,27 @@ static int acer_platform_profile_setup(struct platform_device *device)
>  		platform_profile_handler.profile_set =
>  			acer_predator_v4_platform_profile_set;
>  
> -		set_bit(PLATFORM_PROFILE_PERFORMANCE,
> -			platform_profile_handler.choices);
> -		set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE,
> -			platform_profile_handler.choices);
> -		set_bit(PLATFORM_PROFILE_BALANCED,
> -			platform_profile_handler.choices);
> -		set_bit(PLATFORM_PROFILE_QUIET,
> -			platform_profile_handler.choices);
> -		set_bit(PLATFORM_PROFILE_LOW_POWER,
> -			platform_profile_handler.choices);
> +		err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_SUPPORTED_PROFILES,
> +						   &supported_profiles);
> +		if (err)
> +			return err;
> +
> +		if (supported_profiles & 1 << 0)
> +			set_bit(PLATFORM_PROFILE_QUIET,
> +				platform_profile_handler.choices);
> +		if (supported_profiles & 1 << 1)
> +			set_bit(PLATFORM_PROFILE_BALANCED,
> +				platform_profile_handler.choices);
> +		if (supported_profiles & 1 << 4)
> +			set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE,
> +				platform_profile_handler.choices);
> +		if (supported_profiles & 1 << 5)
> +			set_bit(PLATFORM_PROFILE_PERFORMANCE,
> +				platform_profile_handler.choices);
> +		if (supported_profiles & 1 << 6)

Please, use test_bit() from <linux/bitops.h> in all of these conditions.

~ Kurt

> +			set_bit(PLATFORM_PROFILE_LOW_POWER,
> +				platform_profile_handler.choices);
> +
>  
>  		err = platform_profile_register(&platform_profile_handler);
>  		if (err)
> @@ -1931,7 +2023,7 @@ static int acer_platform_profile_setup(struct platform_device *device)
>  
>  		/* Set default non-turbo profile  */
>  		last_non_turbo_profile =
> -			ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> +			ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>  	}
>  	return 0;
>  }
> @@ -1946,12 +2038,10 @@ static int acer_thermal_profile_change(void)
>  		u8 current_tp;
>  		int tp, err;
>  		u64 on_AC;
> -		acpi_status status;
> -
> -		err = ec_read(ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET,
> -			      &current_tp);
>  
> -		if (err < 0)
> +		err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE,
> +						   &current_tp);
> +		if (err)
>  			return err;
>  
>  		/* Check power source */
> @@ -1962,54 +2052,52 @@ static int acer_thermal_profile_change(void)
>  		switch (current_tp) {
>  		case ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO:
>  			if (!on_AC)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>  			else if (cycle_gaming_thermal_profile)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
>  			else
>  				tp = last_non_turbo_profile;
>  			break;
>  		case ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE:
>  			if (!on_AC)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>  			else
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> +				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_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
>  			else if (cycle_gaming_thermal_profile)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
>  			else
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> +				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_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>  			else if (cycle_gaming_thermal_profile)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>  			else
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> +				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_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
>  			else if (cycle_gaming_thermal_profile)
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
>  			else
> -				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI;
> +				tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
>  			break;
>  		default:
>  			return -EOPNOTSUPP;
>  		}
>  
> -		status = WMI_gaming_execute_u64(
> -			ACER_WMID_SET_GAMING_MISC_SETTING_METHODID, tp, NULL);
> -
> -		if (ACPI_FAILURE(status))
> -			return -EIO;
> +		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_WMI)
> +		if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO)
>  			last_non_turbo_profile = tp;
>  
>  		platform_profile_notify(&platform_profile_handler);
Re: [PATCH] platform/x86: acer-wmi: improve platform profile handling
Posted by Hridesh MG 1 year, 4 months ago 
On Tue, Dec 31, 2024 at 10:36 PM Kurt Borja <kuurtb@gmail.com> wrote:
>
> > +enum acer_wmi_gaming_misc_setting {
> > +     ACER_WMID_MISC_SETTING_OC_1                     = 0x0005,
> > +     ACER_WMID_MISC_SETTING_OC_2                     = 0x0007,
> > +     ACER_WMID_MISC_SETTING_SUPPORTED_PROFILES       = 0x000A,
>
> Should this be GETTING?
No, the SETTING comes from the name of the WMI call (GamingMiscSetting)

>
> > +     ACER_WMID_MISC_SETTING_PLATFORM_PROFILE         = 0x000B,
> > +};
> > +
> >  static const struct key_entry acer_wmi_keymap[] __initconst = {
> >       {KE_KEY, 0x01, {KEY_WLAN} },     /* WiFi */
> >       {KE_KEY, 0x03, {KEY_WLAN} },     /* WiFi */
> > @@ -751,20 +762,12 @@ static bool platform_profile_support;
> >   */
> >  static int last_non_turbo_profile;
> >
> > -enum acer_predator_v4_thermal_profile_ec {
> > -     ACER_PREDATOR_V4_THERMAL_PROFILE_ECO = 0x04,
> > -     ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO = 0x03,
> > -     ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE = 0x02,
> > -     ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET = 0x01,
> > -     ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED = 0x00,
> > -};
> > -
> > -enum acer_predator_v4_thermal_profile_wmi {
> > -     ACER_PREDATOR_V4_THERMAL_PROFILE_ECO_WMI = 0x060B,
> > -     ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO_WMI = 0x050B,
> > -     ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE_WMI = 0x040B,
> > -     ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET_WMI = 0x0B,
> > -     ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED_WMI = 0x010B,
> > +enum acer_predator_v4_thermal_profile {
> > +     ACER_PREDATOR_V4_THERMAL_PROFILE_ECO = 0x06,
> > +     ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO = 0x05,
> > +     ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE = 0x04,
> > +     ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED = 0x01,
> > +     ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET = 0x00,
>
> I think it's better if you sort these from least to greatest.
Alright, makes sense.

> > @@ -1904,6 +1984,7 @@ static int acer_platform_profile_setup(struct platform_device *device)
> >  {
> >       if (quirks->predator_v4) {
> >               int err;
> > +             u8 supported_profiles;
> >
> >               platform_profile_handler.name = "acer-wmi";
> >               platform_profile_handler.dev = &device->dev;
> > @@ -1912,16 +1993,27 @@ static int acer_platform_profile_setup(struct platform_device *device)
> >               platform_profile_handler.profile_set =
> >                       acer_predator_v4_platform_profile_set;
> >
> > -             set_bit(PLATFORM_PROFILE_PERFORMANCE,
> > -                     platform_profile_handler.choices);
> > -             set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE,
> > -                     platform_profile_handler.choices);
> > -             set_bit(PLATFORM_PROFILE_BALANCED,
> > -                     platform_profile_handler.choices);
> > -             set_bit(PLATFORM_PROFILE_QUIET,
> > -                     platform_profile_handler.choices);
> > -             set_bit(PLATFORM_PROFILE_LOW_POWER,
> > -                     platform_profile_handler.choices);
> > +             err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_SUPPORTED_PROFILES,
> > +                                                &supported_profiles);
> > +             if (err)
> > +                     return err;
> > +
> > +             if (supported_profiles & 1 << 0)
> > +                     set_bit(PLATFORM_PROFILE_QUIET,
> > +                             platform_profile_handler.choices);
> > +             if (supported_profiles & 1 << 1)
> > +                     set_bit(PLATFORM_PROFILE_BALANCED,
> > +                             platform_profile_handler.choices);
> > +             if (supported_profiles & 1 << 4)
> > +                     set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE,
> > +                             platform_profile_handler.choices);
> > +             if (supported_profiles & 1 << 5)
> > +                     set_bit(PLATFORM_PROFILE_PERFORMANCE,
> > +                             platform_profile_handler.choices);
> > +             if (supported_profiles & 1 << 6)
>
> Please, use test_bit() from <linux/bitops.h> in all of these conditions.
Will do, thanks for the feedback. I will make a v2 with the changes
(including the alignment one) and submit it in a few days.

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.