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 | 55 +++++++++++++++++++++++++++++------------
1 file changed, 39 insertions(+), 16 deletions(-)
diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index 7968fe21507b1cf28fdc575139057c795e6a873b..6c98c1bb3bdce6a7c6559f6da4ff3c6ce56b60e3 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/bitmap.h>
MODULE_AUTHOR("Carlos Corbacho");
MODULE_DESCRIPTION("Acer Laptop WMI Extras Driver");
@@ -127,6 +128,7 @@ enum acer_wmi_predator_v4_oc {
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,
};
@@ -1957,7 +1959,7 @@ static int
acer_predator_v4_platform_profile_set(struct platform_profile_handler *pprof,
enum platform_profile_option profile)
{
- int err, tp;
+ int max_perf, err, tp;
switch (profile) {
case PLATFORM_PROFILE_PERFORMANCE:
@@ -1983,7 +1985,10 @@ acer_predator_v4_platform_profile_set(struct platform_profile_handler *pprof,
if (err)
return err;
- if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO)
+ max_perf = find_last_bit(platform_profile_handler.choices,
+ PLATFORM_PROFILE_LAST);
+
+ if (tp != max_perf)
last_non_turbo_profile = tp;
return 0;
@@ -1992,6 +1997,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";
@@ -2001,16 +2007,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);
if (err)
@@ -2028,11 +2048,11 @@ static int acer_platform_profile_setup(struct platform_device *device)
static int acer_thermal_profile_change(void)
{
/*
- * This mode key will either cycle through each mode or toggle the turbo profile.
+ * This mode key will either cycle through each mode or toggle the most performant profile.
*/
if (quirks->predator_v4) {
u8 current_tp;
- int err, tp;
+ int max_perf, err, tp;
if (cycle_gaming_thermal_profile) {
platform_profile_cycle();
@@ -2042,11 +2062,14 @@ static int acer_thermal_profile_change(void)
if (err)
return err;
- if (current_tp == ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO) {
+ max_perf = find_last_bit(platform_profile_handler.choices,
+ PLATFORM_PROFILE_LAST);
+
+ if (current_tp == max_perf) {
tp = last_non_turbo_profile;
} else {
last_non_turbo_profile = current_tp;
- tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
+ tp = max_perf;
}
err = WMID_gaming_set_misc_setting(
--
2.47.1On Wed, Jan 08, 2025 at 02:15:26PM +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 | 55 +++++++++++++++++++++++++++++------------
> 1 file changed, 39 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index 7968fe21507b1cf28fdc575139057c795e6a873b..6c98c1bb3bdce6a7c6559f6da4ff3c6ce56b60e3 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/bitmap.h>
>
> MODULE_AUTHOR("Carlos Corbacho");
> MODULE_DESCRIPTION("Acer Laptop WMI Extras Driver");
> @@ -127,6 +128,7 @@ enum acer_wmi_predator_v4_oc {
> 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,
> };
>
> @@ -1957,7 +1959,7 @@ static int
> acer_predator_v4_platform_profile_set(struct platform_profile_handler *pprof,
> enum platform_profile_option profile)
> {
> - int err, tp;
> + int max_perf, err, tp;
>
> switch (profile) {
> case PLATFORM_PROFILE_PERFORMANCE:
> @@ -1983,7 +1985,10 @@ acer_predator_v4_platform_profile_set(struct platform_profile_handler *pprof,
> if (err)
> return err;
>
> - if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO)
> + max_perf = find_last_bit(platform_profile_handler.choices,
> + PLATFORM_PROFILE_LAST);
> +
> + if (tp != max_perf)
You can't directly compare `tp` and `max_perf`. ACER_PREDATOR_V4 values
may not match PLATFORM_PROFILE ones.
It does in the case of PERFORMANCE and TURBO, but it does not in the
case of QUIET and BALANCED.
I suggest you store the actual ACER_PREDATOR_V4 max_perf when setting up
the platform_profile.
> last_non_turbo_profile = tp;
>
> return 0;
> @@ -1992,6 +1997,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";
> @@ -2001,16 +2007,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);
> if (err)
> @@ -2028,11 +2048,11 @@ static int acer_platform_profile_setup(struct platform_device *device)
> static int acer_thermal_profile_change(void)
> {
> /*
> - * This mode key will either cycle through each mode or toggle the turbo profile.
> + * This mode key will either cycle through each mode or toggle the most performant profile.
> */
> if (quirks->predator_v4) {
> u8 current_tp;
> - int err, tp;
> + int max_perf, err, tp;
>
> if (cycle_gaming_thermal_profile) {
> platform_profile_cycle();
> @@ -2042,11 +2062,14 @@ static int acer_thermal_profile_change(void)
> if (err)
> return err;
>
> - if (current_tp == ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO) {
> + max_perf = find_last_bit(platform_profile_handler.choices,
> + PLATFORM_PROFILE_LAST);
> +
> + if (current_tp == max_perf) {
Same as above.
~ Kurt
> tp = last_non_turbo_profile;
> } else {
> last_non_turbo_profile = current_tp;
> - tp = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
> + tp = max_perf;
> }
>
> err = WMID_gaming_set_misc_setting(
On Wed, Jan 8, 2025 at 7:21 PM Kurt Borja <kuurtb@gmail.com> wrote:
>
> On Wed, Jan 08, 2025 at 02:15:26PM +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 | 55 +++++++++++++++++++++++++++++------------
> > 1 file changed, 39 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> > index 7968fe21507b1cf28fdc575139057c795e6a873b..6c98c1bb3bdce6a7c6559f6da4ff3c6ce56b60e3 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/bitmap.h>
> >
> > MODULE_AUTHOR("Carlos Corbacho");
> > MODULE_DESCRIPTION("Acer Laptop WMI Extras Driver");
> > @@ -127,6 +128,7 @@ enum acer_wmi_predator_v4_oc {
> > 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,
> > };
> >
> > @@ -1957,7 +1959,7 @@ static int
> > acer_predator_v4_platform_profile_set(struct platform_profile_handler *pprof,
> > enum platform_profile_option profile)
> > {
> > - int err, tp;
> > + int max_perf, err, tp;
> >
> > switch (profile) {
> > case PLATFORM_PROFILE_PERFORMANCE:
> > @@ -1983,7 +1985,10 @@ acer_predator_v4_platform_profile_set(struct platform_profile_handler *pprof,
> > if (err)
> > return err;
> >
> > - if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO)
> > + max_perf = find_last_bit(platform_profile_handler.choices,
> > + PLATFORM_PROFILE_LAST);
> > +
> > + if (tp != max_perf)
>
> You can't directly compare `tp` and `max_perf`. ACER_PREDATOR_V4 values
> may not match PLATFORM_PROFILE ones.
>
> It does in the case of PERFORMANCE and TURBO, but it does not in the
> case of QUIET and BALANCED.
>
> I suggest you store the actual ACER_PREDATOR_V4 max_perf when setting up
> the platform_profile.
Ah this was quite a stupid mistake. I'm not sure why I even assumed
both were equivalent. I have one doubt though, if i set it during
profile setup, the code becomes quite verbose -
/* Iterate through supported profiles in order of increasing
performance */
if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_ECO,
&supported_profiles)) {
set_bit(PLATFORM_PROFILE_LOW_POWER,
platform_profile_handler.choices);
max_perf = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
}
if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET,
&supported_profiles)) {
set_bit(PLATFORM_PROFILE_QUIET,
platform_profile_handler.choices);
max_perf = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
}
if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED,
&supported_profiles)) {
set_bit(PLATFORM_PROFILE_BALANCED,
platform_profile_handler.choices);
max_perf = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
}
if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE,
&supported_profiles)) {
set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE,
platform_profile_handler.choices);
max_perf = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
}
if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO,
&supported_profiles)) {
set_bit(PLATFORM_PROFILE_PERFORMANCE,
platform_profile_handler.choices);
max_perf = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
}
Is this fine? Maybe for readability's sake, I could lift it up into a
different function, like what you did in the RFC patch. Btw, thanks a lot
for the detailed reviews so far—they’ve been very helpful!
--
Thanks,
Hridesh MG
On Thu, Jan 09, 2025 at 04:40:14PM +0530, Hridesh MG wrote:
> On Wed, Jan 8, 2025 at 7:21 PM Kurt Borja <kuurtb@gmail.com> wrote:
> >
> > On Wed, Jan 08, 2025 at 02:15:26PM +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 | 55 +++++++++++++++++++++++++++++------------
> > > 1 file changed, 39 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> > > index 7968fe21507b1cf28fdc575139057c795e6a873b..6c98c1bb3bdce6a7c6559f6da4ff3c6ce56b60e3 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/bitmap.h>
> > >
> > > MODULE_AUTHOR("Carlos Corbacho");
> > > MODULE_DESCRIPTION("Acer Laptop WMI Extras Driver");
> > > @@ -127,6 +128,7 @@ enum acer_wmi_predator_v4_oc {
> > > 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,
> > > };
> > >
> > > @@ -1957,7 +1959,7 @@ static int
> > > acer_predator_v4_platform_profile_set(struct platform_profile_handler *pprof,
> > > enum platform_profile_option profile)
> > > {
> > > - int err, tp;
> > > + int max_perf, err, tp;
> > >
> > > switch (profile) {
> > > case PLATFORM_PROFILE_PERFORMANCE:
> > > @@ -1983,7 +1985,10 @@ acer_predator_v4_platform_profile_set(struct platform_profile_handler *pprof,
> > > if (err)
> > > return err;
> > >
> > > - if (tp != ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO)
> > > + max_perf = find_last_bit(platform_profile_handler.choices,
> > > + PLATFORM_PROFILE_LAST);
> > > +
> > > + if (tp != max_perf)
> >
> > You can't directly compare `tp` and `max_perf`. ACER_PREDATOR_V4 values
> > may not match PLATFORM_PROFILE ones.
> >
> > It does in the case of PERFORMANCE and TURBO, but it does not in the
> > case of QUIET and BALANCED.
> >
> > I suggest you store the actual ACER_PREDATOR_V4 max_perf when setting up
> > the platform_profile.
> Ah this was quite a stupid mistake. I'm not sure why I even assumed
> both were equivalent. I have one doubt though, if i set it during
> profile setup, the code becomes quite verbose -
>
> /* Iterate through supported profiles in order of increasing
> performance */
> if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_ECO,
> &supported_profiles)) {
> set_bit(PLATFORM_PROFILE_LOW_POWER,
> platform_profile_handler.choices);
> max_perf = ACER_PREDATOR_V4_THERMAL_PROFILE_ECO;
> }
>
> if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET,
> &supported_profiles)) {
> set_bit(PLATFORM_PROFILE_QUIET,
> platform_profile_handler.choices);
> max_perf = ACER_PREDATOR_V4_THERMAL_PROFILE_QUIET;
> }
>
> if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED,
> &supported_profiles)) {
> set_bit(PLATFORM_PROFILE_BALANCED,
> platform_profile_handler.choices);
> max_perf = ACER_PREDATOR_V4_THERMAL_PROFILE_BALANCED;
> }
>
> if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE,
> &supported_profiles)) {
> set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE,
> platform_profile_handler.choices);
> max_perf = ACER_PREDATOR_V4_THERMAL_PROFILE_PERFORMANCE;
> }
>
> if (test_bit(ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO,
> &supported_profiles)) {
> set_bit(PLATFORM_PROFILE_PERFORMANCE,
> platform_profile_handler.choices);
> max_perf = ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO;
> }
Hi Hridesh,
It looks a bit verbose, but for me it's fine, it even caches the value.
If max_perf is a global variable now, rename it to something like
acer_predator_v4_max_perf.
>
> Is this fine? Maybe for readability's sake, I could lift it up into a
> different function, like what you did in the RFC patch. Btw, thanks a lot
> for the detailed reviews so far—they’ve been very helpful!
Glad I can help :)
~ Kurt
>
> --
> Thanks,
> Hridesh MG
© 2016 - 2025 Red Hat, Inc.