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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALiyAon3r=VXFNZw7is4VWZoRnFFbrUUvLB9XeW+xkMLjXDyRA@mail.gmail.com>
Date: Thu, 2 Jan 2025 12:20:36 +0530
From: Hridesh MG <hridesh699@...il.com>
To: Armin Wolf <W_Armin@....de>
Cc: "Lee, Chun-Yi" <jlee@...e.com>, Hans de Goede <hdegoede@...hat.com>, 
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>, 
	platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Shuah Khan <skhan@...uxfoundation.org>, onenowy@...il.com
Subject: Re: [PATCH] platform/x86: acer-wmi: improve platform profile handling

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

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

--
Thanks,
Hridesh MG