Re: [PATCH v2 2/3] platform/x86: acer-wmi: use an ACPI bitmap to set the platform profile choices

[Armin Wolf <W_Armin@xxxxxx>]

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@xxxxxx/
> > Signed-off-by: Hridesh MG <hridesh699@xxxxxxxxx>
> > ---
> >   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);