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

[Hridesh MG]

On Tue, Dec 31, 2024 at 10:36 PM Kurt Borja <kuurtb@xxxxxxxxx> wrote:
>
> > +enum acer_wmi_gaming_misc_setting {
> > +     ACER_WMID_MISC_SETTING_OC_1                     = 0x0005,
> > +     ACER_WMID_MISC_SETTING_OC_2                     = 0x0007,
> > +     ACER_WMID_MISC_SETTING_SUPPORTED_PROFILES       = 0x000A,
>
> Should this be GETTING?
No, the SETTING comes from the name of the WMI call (GamingMiscSetting)

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

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