* [PATCH v2 0/3] platform/x86 acer-wmi: Improve platform profile handling
@ 2025-01-04 15:29 Hridesh MG
2025-01-04 15:29 ` [PATCH v2 1/3] platform/x86: acer-wmi: use WMI calls for " Hridesh MG
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Hridesh MG @ 2025-01-04 15:29 UTC (permalink / raw)
To: Hans de Goede, Ilpo Järvinen, Armin Wolf
Cc: platform-driver-x86, linux-kernel, Shuah Khan, Hridesh MG
This patch improves the platform profile handling for laptops using the
Acer Predator interface by making the following changes -
1) Using WMI calls to fetch the current platform profile instead of
directly accessing it from the EC.
2) Using an ACPI bitmap to dynamically set platform_profile_choices.
3) Simplifying the cycling of platform profiles by making use of
platform_profile_cycle()
v1->v2:
[1]
- Fixed enum member alignment and reordered them
[2]
- Made use of test_bit to check bitmap values
- Replaced magic numbers with proper variables
Link to v1: https://lore.kernel.org/platform-driver-x86/20241231140442.10076-1-hridesh699@gmail.com/
Signed-off-by: Hridesh MG <hridesh699@gmail.com>
---
Hridesh MG (3):
platform/x86: acer-wmi: use WMI calls for platform profile handling
platform/x86: acer-wmi: use an ACPI bitmap to set the platform profile choices
platform/x86: acer-wmi: simplify platform profile cycling
drivers/platform/x86/acer-wmi.c | 270 ++++++++++++++++++++++++----------------
1 file changed, 161 insertions(+), 109 deletions(-)
---
base-commit: 8155b4ef3466f0e289e8fcc9e6e62f3f4dceeac2
change-id: 20250102-platform_profile-fc1e0aaf2900
Best regards,
--
Hridesh MG <hridesh699@gmail.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/3] platform/x86: acer-wmi: use WMI calls for platform profile handling
2025-01-04 15:29 [PATCH v2 0/3] platform/x86 acer-wmi: Improve platform profile handling Hridesh MG
@ 2025-01-04 15:29 ` Hridesh MG
2025-01-04 17:13 ` Kurt Borja
2025-01-04 15:29 ` [PATCH v2 2/3] platform/x86: acer-wmi: use an ACPI bitmap to set the platform profile choices Hridesh MG
2025-01-04 15:29 ` [PATCH v2 3/3] platform/x86: acer-wmi: simplify platform profile cycling Hridesh MG
2 siblings, 1 reply; 13+ messages in thread
From: Hridesh MG @ 2025-01-04 15:29 UTC (permalink / raw)
To: Hans de Goede, Ilpo Järvinen, Armin Wolf
Cc: platform-driver-x86, linux-kernel, Shuah Khan, Hridesh MG
Improve the platform profile handling by using WMI calls to fetch the
current platform profile instead of directly accessing it from the EC.
This is beneficial because the EC address differs for certain laptops.
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>
---
drivers/platform/x86/acer-wmi.c | 189 ++++++++++++++++++++++++++++------------
1 file changed, 133 insertions(+), 56 deletions(-)
diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index b3043d78a7b38a7b773da5ecd4846ca11e8595f5..5370056fb2d03a768162f2f1643ef27dc6deafa8 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_QUIET = 0x00,
+ ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED = 0x01,
+ ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE = 0x04,
+ ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO = 0x05,
+ ACER_PREDATOR_V4_THERMAL_PROFILE_ECO = 0x06,
};
/* 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;
@@ -1923,6 +2003,7 @@ static int acer_platform_profile_setup(struct platform_device *device)
set_bit(PLATFORM_PROFILE_LOW_POWER,
platform_profile_handler.choices);
+
err = platform_profile_register(&platform_profile_handler);
if (err)
return err;
@@ -1931,7 +2012,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 +2027,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,
- ¤t_tp);
- if (err < 0)
+ err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE,
+ ¤t_tp);
+ if (err)
return err;
/* Check power source */
@@ -1962,54 +2041,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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/3] platform/x86: acer-wmi: use an ACPI bitmap to set the platform profile choices
2025-01-04 15:29 [PATCH v2 0/3] platform/x86 acer-wmi: Improve platform profile handling Hridesh MG
2025-01-04 15:29 ` [PATCH v2 1/3] platform/x86: acer-wmi: use WMI calls for " Hridesh MG
@ 2025-01-04 15:29 ` Hridesh MG
2025-01-04 17:18 ` Kurt Borja
2025-01-04 15:29 ` [PATCH v2 3/3] platform/x86: acer-wmi: simplify platform profile cycling Hridesh MG
2 siblings, 1 reply; 13+ messages in thread
From: Hridesh MG @ 2025-01-04 15:29 UTC (permalink / raw)
To: Hans de Goede, Ilpo Järvinen, Armin Wolf
Cc: platform-driver-x86, linux-kernel, Shuah Khan, Hridesh MG
Currently the choices for the platform profile are hardcoded. There is
an ACPI bitmap accessible via WMI that specifies the supported profiles,
use this bitmap to dynamically set the choices for the platform profile.
Link: https://lore.kernel.org/platform-driver-x86/ecb60ee5-3df7-4d7e-8ebf-8c162b339ade@gmx.de/
Signed-off-by: Hridesh MG <hridesh699@gmail.com>
---
drivers/platform/x86/acer-wmi.c | 36 ++++++++++++++++++++++++++----------
1 file changed, 26 insertions(+), 10 deletions(-)
diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index 5370056fb2d03a768162f2f1643ef27dc6deafa8..f6c47deb4c452fc193f22c479c730aecb1e69e44 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -33,6 +33,7 @@
#include <linux/units.h>
#include <linux/unaligned.h>
#include <linux/bitfield.h>
+#include <linux/bitops.h>
MODULE_AUTHOR("Carlos Corbacho");
MODULE_DESCRIPTION("Acer Laptop WMI Extras Driver");
@@ -1983,6 +1984,7 @@ acer_predator_v4_platform_profile_set(struct platform_profile_handler *pprof,
static int acer_platform_profile_setup(struct platform_device *device)
{
if (quirks->predator_v4) {
+ unsigned long supported_profiles;
int err;
platform_profile_handler.name = "acer-wmi";
@@ -1992,16 +1994,30 @@ 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,
+ (u8 *)&supported_profiles);
+ if (err)
+ return err;
+
+ if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET, &supported_profiles))
+ set_bit(PLATFORM_PROFILE_QUIET,
+ platform_profile_handler.choices);
+
+ if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED, &supported_profiles))
+ set_bit(PLATFORM_PROFILE_BALANCED,
+ platform_profile_handler.choices);
+
+ if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE, &supported_profiles))
+ set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE,
+ platform_profile_handler.choices);
+
+ if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO, &supported_profiles))
+ set_bit(PLATFORM_PROFILE_PERFORMANCE,
+ platform_profile_handler.choices);
+
+ if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_ECO, &supported_profiles))
+ set_bit(PLATFORM_PROFILE_LOW_POWER,
+ platform_profile_handler.choices);
err = platform_profile_register(&platform_profile_handler);
--
2.47.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/3] platform/x86: acer-wmi: simplify platform profile cycling
2025-01-04 15:29 [PATCH v2 0/3] platform/x86 acer-wmi: Improve platform profile handling Hridesh MG
2025-01-04 15:29 ` [PATCH v2 1/3] platform/x86: acer-wmi: use WMI calls for " Hridesh MG
2025-01-04 15:29 ` [PATCH v2 2/3] platform/x86: acer-wmi: use an ACPI bitmap to set the platform profile choices Hridesh MG
@ 2025-01-04 15:29 ` Hridesh MG
2025-01-04 17:53 ` Kurt Borja
2 siblings, 1 reply; 13+ messages in thread
From: Hridesh MG @ 2025-01-04 15:29 UTC (permalink / raw)
To: Hans de Goede, Ilpo Järvinen, Armin Wolf
Cc: platform-driver-x86, linux-kernel, Shuah Khan, Hridesh MG
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,
- ¤t_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,
+ ¤t_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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3] platform/x86: acer-wmi: use WMI calls for platform profile handling
2025-01-04 15:29 ` [PATCH v2 1/3] platform/x86: acer-wmi: use WMI calls for " Hridesh MG
@ 2025-01-04 17:13 ` Kurt Borja
2025-01-05 11:19 ` Hridesh MG
0 siblings, 1 reply; 13+ messages in thread
From: Kurt Borja @ 2025-01-04 17:13 UTC (permalink / raw)
To: Hridesh MG
Cc: Hans de Goede, Ilpo Järvinen, Armin Wolf,
platform-driver-x86, linux-kernel, Shuah Khan
On Sat, Jan 04, 2025 at 08:59:20PM +0530, Hridesh MG wrote:
> Improve the platform profile handling by using WMI calls to fetch the
> current platform profile instead of directly accessing it from the EC.
> This is beneficial because the EC address differs for certain laptops.
>
> 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>
Hi Hridesh,
> ---
> drivers/platform/x86/acer-wmi.c | 189 ++++++++++++++++++++++++++++------------
> 1 file changed, 133 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index b3043d78a7b38a7b773da5ecd4846ca11e8595f5..5370056fb2d03a768162f2f1643ef27dc6deafa8 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,
These OC settings should be added only if you add support for them.
I noticed acer_toggle_turbo() uses these settings. For consistency, I
think it should be refactored to use WMID_gaming_set_misc_setting()
instead of WMID_gaming_set_u64().
> + ACER_WMID_MISC_SETTING_SUPPORTED_PROFILES = 0x000A,
This should be added in patch [2/3].
> + 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_QUIET = 0x00,
> + ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED = 0x01,
> + ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE = 0x04,
> + ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO = 0x05,
> + ACER_PREDATOR_V4_THERMAL_PROFILE_ECO = 0x06,
> };
>
> /* 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;
> @@ -1923,6 +2003,7 @@ static int acer_platform_profile_setup(struct platform_device *device)
> set_bit(PLATFORM_PROFILE_LOW_POWER,
> platform_profile_handler.choices);
>
> +
Please, drop this extra line.
~ Kurt
> err = platform_profile_register(&platform_profile_handler);
> if (err)
> return err;
> @@ -1931,7 +2012,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 +2027,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,
> - ¤t_tp);
>
> - if (err < 0)
> + err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE,
> + ¤t_tp);
> + if (err)
> return err;
>
> /* Check power source */
> @@ -1962,54 +2041,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);
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/3] platform/x86: acer-wmi: use an ACPI bitmap to set the platform profile choices
2025-01-04 15:29 ` [PATCH v2 2/3] platform/x86: acer-wmi: use an ACPI bitmap to set the platform profile choices Hridesh MG
@ 2025-01-04 17:18 ` Kurt Borja
2025-01-05 13:06 ` Armin Wolf
0 siblings, 1 reply; 13+ messages in thread
From: Kurt Borja @ 2025-01-04 17:18 UTC (permalink / raw)
To: Hridesh MG
Cc: Hans de Goede, Ilpo Järvinen, Armin Wolf,
platform-driver-x86, linux-kernel, Shuah Khan
On Sat, Jan 04, 2025 at 08:59:21PM +0530, Hridesh MG wrote:
> Currently the choices for the platform profile are hardcoded. There is
> an ACPI bitmap accessible via WMI that specifies the supported profiles,
> use this bitmap to dynamically set the choices for the platform profile.
>
> Link: https://lore.kernel.org/platform-driver-x86/ecb60ee5-3df7-4d7e-8ebf-8c162b339ade@gmx.de/
> Signed-off-by: Hridesh MG <hridesh699@gmail.com>
> ---
> drivers/platform/x86/acer-wmi.c | 36 ++++++++++++++++++++++++++----------
> 1 file changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index 5370056fb2d03a768162f2f1643ef27dc6deafa8..f6c47deb4c452fc193f22c479c730aecb1e69e44 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -33,6 +33,7 @@
> #include <linux/units.h>
> #include <linux/unaligned.h>
> #include <linux/bitfield.h>
> +#include <linux/bitops.h>
>
> MODULE_AUTHOR("Carlos Corbacho");
> MODULE_DESCRIPTION("Acer Laptop WMI Extras Driver");
> @@ -1983,6 +1984,7 @@ acer_predator_v4_platform_profile_set(struct platform_profile_handler *pprof,
> static int acer_platform_profile_setup(struct platform_device *device)
> {
> if (quirks->predator_v4) {
> + unsigned long supported_profiles;
> int err;
>
> platform_profile_handler.name = "acer-wmi";
> @@ -1992,16 +1994,30 @@ 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,
> + (u8 *)&supported_profiles);
> + if (err)
> + return err;
> +
> + if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET, &supported_profiles))
> + set_bit(PLATFORM_PROFILE_QUIET,
> + platform_profile_handler.choices);
> +
> + if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED, &supported_profiles))
> + set_bit(PLATFORM_PROFILE_BALANCED,
> + platform_profile_handler.choices);
> +
> + if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE, &supported_profiles))
> + set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE,
> + platform_profile_handler.choices);
> +
> + if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO, &supported_profiles))
> + set_bit(PLATFORM_PROFILE_PERFORMANCE,
> + platform_profile_handler.choices);
> +
> + if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_ECO, &supported_profiles))
> + set_bit(PLATFORM_PROFILE_LOW_POWER,
> + platform_profile_handler.choices);
As Armin mentioned, with this approach you may still select unsupported
profiles in acer_thermal_profile_change(). You should either handle that
in this patch or move this patch to the end of the series.
~ Kurt
>
>
> err = platform_profile_register(&platform_profile_handler);
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/3] platform/x86: acer-wmi: simplify platform profile cycling
2025-01-04 15:29 ` [PATCH v2 3/3] platform/x86: acer-wmi: simplify platform profile cycling Hridesh MG
@ 2025-01-04 17:53 ` Kurt Borja
2025-01-04 18:19 ` Hridesh MG
0 siblings, 1 reply; 13+ messages in thread
From: Kurt Borja @ 2025-01-04 17:53 UTC (permalink / raw)
To: Hridesh MG
Cc: Hans de Goede, Ilpo Järvinen, Armin Wolf,
platform-driver-x86, linux-kernel, Shuah Khan, SungHwan Jung
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,
> - ¤t_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,
> + ¤t_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;
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/3] platform/x86: acer-wmi: simplify platform profile cycling
2025-01-04 17:53 ` Kurt Borja
@ 2025-01-04 18:19 ` Hridesh MG
2025-01-05 4:01 ` SungHwan Jung
0 siblings, 1 reply; 13+ messages in thread
From: Hridesh MG @ 2025-01-04 18:19 UTC (permalink / raw)
To: Kurt Borja
Cc: Hans de Goede, Ilpo Järvinen, Armin Wolf,
platform-driver-x86, linux-kernel, Shuah Khan, SungHwan Jung
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/3] platform/x86: acer-wmi: simplify platform profile cycling
2025-01-04 18:19 ` Hridesh MG
@ 2025-01-05 4:01 ` SungHwan Jung
2025-01-05 4:16 ` Hridesh MG
0 siblings, 1 reply; 13+ messages in thread
From: SungHwan Jung @ 2025-01-05 4:01 UTC (permalink / raw)
To: Hridesh MG, Kurt Borja
Cc: Hans de Goede, Ilpo Järvinen, Armin Wolf,
platform-driver-x86, linux-kernel, Shuah Khan
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/3] platform/x86: acer-wmi: simplify platform profile cycling
2025-01-05 4:01 ` SungHwan Jung
@ 2025-01-05 4:16 ` Hridesh MG
0 siblings, 0 replies; 13+ messages in thread
From: Hridesh MG @ 2025-01-05 4:16 UTC (permalink / raw)
To: SungHwan Jung
Cc: Kurt Borja, Hans de Goede, Ilpo Järvinen, Armin Wolf,
platform-driver-x86, linux-kernel, Shuah Khan
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3] platform/x86: acer-wmi: use WMI calls for platform profile handling
2025-01-04 17:13 ` Kurt Borja
@ 2025-01-05 11:19 ` Hridesh MG
2025-01-05 13:02 ` Armin Wolf
0 siblings, 1 reply; 13+ messages in thread
From: Hridesh MG @ 2025-01-05 11:19 UTC (permalink / raw)
To: Kurt Borja
Cc: Hans de Goede, Ilpo Järvinen, Armin Wolf,
platform-driver-x86, linux-kernel, Shuah Khan
On Sat, Jan 4, 2025 at 10:43 PM Kurt Borja <kuurtb@gmail.com> wrote:
>
> On Sat, Jan 04, 2025 at 08:59:20PM +0530, Hridesh MG wrote:
> > Improve the platform profile handling by using WMI calls to fetch the
> > current platform profile instead of directly accessing it from the EC.
> > This is beneficial because the EC address differs for certain laptops.
> >
> > 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>
>
> Hi Hridesh,
>
> > ---
> > drivers/platform/x86/acer-wmi.c | 189 ++++++++++++++++++++++++++++------------
> > 1 file changed, 133 insertions(+), 56 deletions(-)
> >
> > diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> > index b3043d78a7b38a7b773da5ecd4846ca11e8595f5..5370056fb2d03a768162f2f1643ef27dc6deafa8 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,
>
> These OC settings should be added only if you add support for them.
>
> I noticed acer_toggle_turbo() uses these settings. For consistency, I
> think it should be refactored to use WMID_gaming_set_misc_setting()
> instead of WMID_gaming_set_u64().
Yeah I agree. Actually, now that we have this function, this
particular case in WMID_gaming_set_u64() is redundant, so can I remove
it? (sorry if this is a dumb question)
switch (cap) {
case ACER_CAP_TURBO_OC:
method_id = ACER_WMID_SET_GAMING_MISC_SETTING_METHODID;
break;
}
> > + ACER_WMID_MISC_SETTING_SUPPORTED_PROFILES = 0x000A,
>
> This should be added in patch [2/3].
>
> > + 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_QUIET = 0x00,
> > + ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED = 0x01,
> > + ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE = 0x04,
> > + ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO = 0x05,
> > + ACER_PREDATOR_V4_THERMAL_PROFILE_ECO = 0x06,
> > };
> >
> > /* 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;
> > @@ -1923,6 +2003,7 @@ static int acer_platform_profile_setup(struct platform_device *device)
> > set_bit(PLATFORM_PROFILE_LOW_POWER,
> > platform_profile_handler.choices);
> >
> > +
>
> Please, drop this extra line.
>
> ~ Kurt
>
> > err = platform_profile_register(&platform_profile_handler);
> > if (err)
> > return err;
> > @@ -1931,7 +2012,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 +2027,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,
> > - ¤t_tp);
> >
> > - if (err < 0)
> > + err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE,
> > + ¤t_tp);
> > + if (err)
> > return err;
> >
> > /* Check power source */
> > @@ -1962,54 +2041,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);
--
Thanks,
Hridesh MG
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3] platform/x86: acer-wmi: use WMI calls for platform profile handling
2025-01-05 11:19 ` Hridesh MG
@ 2025-01-05 13:02 ` Armin Wolf
0 siblings, 0 replies; 13+ messages in thread
From: Armin Wolf @ 2025-01-05 13:02 UTC (permalink / raw)
To: Hridesh MG, Kurt Borja
Cc: Hans de Goede, Ilpo Järvinen, platform-driver-x86,
linux-kernel, Shuah Khan
Am 05.01.25 um 12:19 schrieb Hridesh MG:
> On Sat, Jan 4, 2025 at 10:43 PM Kurt Borja <kuurtb@gmail.com> wrote:
>> On Sat, Jan 04, 2025 at 08:59:20PM +0530, Hridesh MG wrote:
>>> Improve the platform profile handling by using WMI calls to fetch the
>>> current platform profile instead of directly accessing it from the EC.
>>> This is beneficial because the EC address differs for certain laptops.
>>>
>>> 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>
>> Hi Hridesh,
>>
>>> ---
>>> drivers/platform/x86/acer-wmi.c | 189 ++++++++++++++++++++++++++++------------
>>> 1 file changed, 133 insertions(+), 56 deletions(-)
>>>
>>> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
>>> index b3043d78a7b38a7b773da5ecd4846ca11e8595f5..5370056fb2d03a768162f2f1643ef27dc6deafa8 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,
>> These OC settings should be added only if you add support for them.
>>
>> I noticed acer_toggle_turbo() uses these settings. For consistency, I
>> think it should be refactored to use WMID_gaming_set_misc_setting()
>> instead of WMID_gaming_set_u64().
> Yeah I agree. Actually, now that we have this function, this
> particular case in WMID_gaming_set_u64() is redundant, so can I remove
> it? (sorry if this is a dumb question)
>
> switch (cap) {
> case ACER_CAP_TURBO_OC:
> method_id = ACER_WMID_SET_GAMING_MISC_SETTING_METHODID;
> break;
> }
Yes i think so. Please do the conversion of the OC settings in a separate patch so that
it is easier to review.
Keep in mind that you emulate the behavior of WMID_gaming_set_u64() and check if the
interface has ACER_CAP_TURBO_OC before calling WMID_gaming_set_misc_settings() to change
the OC settings.
Thanks,
Armin Wolf
>>> + ACER_WMID_MISC_SETTING_SUPPORTED_PROFILES = 0x000A,
>> This should be added in patch [2/3].
>>
>>> + 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_QUIET = 0x00,
>>> + ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED = 0x01,
>>> + ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE = 0x04,
>>> + ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO = 0x05,
>>> + ACER_PREDATOR_V4_THERMAL_PROFILE_ECO = 0x06,
>>> };
>>>
>>> /* 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;
>>> @@ -1923,6 +2003,7 @@ static int acer_platform_profile_setup(struct platform_device *device)
>>> set_bit(PLATFORM_PROFILE_LOW_POWER,
>>> platform_profile_handler.choices);
>>>
>>> +
>> Please, drop this extra line.
>>
>> ~ Kurt
>>
>>> err = platform_profile_register(&platform_profile_handler);
>>> if (err)
>>> return err;
>>> @@ -1931,7 +2012,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 +2027,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,
>>> - ¤t_tp);
>>>
>>> - if (err < 0)
>>> + err = WMID_gaming_get_misc_setting(ACER_WMID_MISC_SETTING_PLATFORM_PROFILE,
>>> + ¤t_tp);
>>> + if (err)
>>> return err;
>>>
>>> /* Check power source */
>>> @@ -1962,54 +2041,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);
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/3] platform/x86: acer-wmi: use an ACPI bitmap to set the platform profile choices
2025-01-04 17:18 ` Kurt Borja
@ 2025-01-05 13:06 ` Armin Wolf
0 siblings, 0 replies; 13+ messages in thread
From: Armin Wolf @ 2025-01-05 13:06 UTC (permalink / raw)
To: Kurt Borja, Hridesh MG
Cc: Hans de Goede, Ilpo Järvinen, platform-driver-x86,
linux-kernel, Shuah Khan
Am 04.01.25 um 18:18 schrieb Kurt Borja:
> On Sat, Jan 04, 2025 at 08:59:21PM +0530, Hridesh MG wrote:
>> Currently the choices for the platform profile are hardcoded. There is
>> an ACPI bitmap accessible via WMI that specifies the supported profiles,
>> use this bitmap to dynamically set the choices for the platform profile.
>>
>> Link: https://lore.kernel.org/platform-driver-x86/ecb60ee5-3df7-4d7e-8ebf-8c162b339ade@gmx.de/
>> Signed-off-by: Hridesh MG <hridesh699@gmail.com>
>> ---
>> drivers/platform/x86/acer-wmi.c | 36 ++++++++++++++++++++++++++----------
>> 1 file changed, 26 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
>> index 5370056fb2d03a768162f2f1643ef27dc6deafa8..f6c47deb4c452fc193f22c479c730aecb1e69e44 100644
>> --- a/drivers/platform/x86/acer-wmi.c
>> +++ b/drivers/platform/x86/acer-wmi.c
>> @@ -33,6 +33,7 @@
>> #include <linux/units.h>
>> #include <linux/unaligned.h>
>> #include <linux/bitfield.h>
>> +#include <linux/bitops.h>
>>
>> MODULE_AUTHOR("Carlos Corbacho");
>> MODULE_DESCRIPTION("Acer Laptop WMI Extras Driver");
>> @@ -1983,6 +1984,7 @@ acer_predator_v4_platform_profile_set(struct platform_profile_handler *pprof,
>> static int acer_platform_profile_setup(struct platform_device *device)
>> {
>> if (quirks->predator_v4) {
>> + unsigned long supported_profiles;
>> int err;
>>
>> platform_profile_handler.name = "acer-wmi";
>> @@ -1992,16 +1994,30 @@ 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,
>> + (u8 *)&supported_profiles);
>> + if (err)
>> + return err;
>> +
>> + if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET, &supported_profiles))
>> + set_bit(PLATFORM_PROFILE_QUIET,
>> + platform_profile_handler.choices);
>> +
>> + if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED, &supported_profiles))
>> + set_bit(PLATFORM_PROFILE_BALANCED,
>> + platform_profile_handler.choices);
>> +
>> + if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE, &supported_profiles))
>> + set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE,
>> + platform_profile_handler.choices);
>> +
>> + if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO, &supported_profiles))
>> + set_bit(PLATFORM_PROFILE_PERFORMANCE,
>> + platform_profile_handler.choices);
>> +
>> + if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_ECO, &supported_profiles))
>> + set_bit(PLATFORM_PROFILE_LOW_POWER,
>> + platform_profile_handler.choices);
> As Armin mentioned, with this approach you may still select unsupported
> profiles in acer_thermal_profile_change(). You should either handle that
> in this patch or move this patch to the end of the series.
>
> ~ Kurt
Correct.
I suggest that you simply reorder the patches so that this patch comes after the platform_profile_cycle() patch.
Thanks,
Armin Wolf
>>
>>
>> err = platform_profile_register(&platform_profile_handler);
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-01-05 13:06 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-04 15:29 [PATCH v2 0/3] platform/x86 acer-wmi: Improve platform profile handling Hridesh MG
2025-01-04 15:29 ` [PATCH v2 1/3] platform/x86: acer-wmi: use WMI calls for " Hridesh MG
2025-01-04 17:13 ` Kurt Borja
2025-01-05 11:19 ` Hridesh MG
2025-01-05 13:02 ` Armin Wolf
2025-01-04 15:29 ` [PATCH v2 2/3] platform/x86: acer-wmi: use an ACPI bitmap to set the platform profile choices Hridesh MG
2025-01-04 17:18 ` Kurt Borja
2025-01-05 13:06 ` Armin Wolf
2025-01-04 15:29 ` [PATCH v2 3/3] platform/x86: acer-wmi: simplify platform profile cycling Hridesh MG
2025-01-04 17:53 ` Kurt Borja
2025-01-04 18:19 ` Hridesh MG
2025-01-05 4:01 ` SungHwan Jung
2025-01-05 4:16 ` Hridesh MG
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox