linux-kernel - Re: [PATCH v5] platform/x86: acer-wmi: Add Turbo Mode support for Acer PH315-53

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: <CAMW3L+3WUCXx60fALT=2J8TjK_BO3jOTb+8+JQQrKA1_F9pLTQ@mail.gmail.com>
Date:   Sat, 14 Aug 2021 16:10:39 +0430
From:   Jafar Akhondali <jafar.akhoondali@...il.com>
To:     Hans de Goede <hdegoede@...hat.com>
Cc:     jlee@...e.com, linux-kernel@...r.kernel.org,
        platform-driver-x86@...r.kernel.org, mgross@...ux.intel.com
Subject: Re: [PATCH v5] platform/x86: acer-wmi: Add Turbo Mode support for
 Acer PH315-53

 Hi Hans,
Thank you for your efforts on the patch, I've sure learned a lot from
your comments as this
was my first contribution to Linux kernel.

Just an extra question, for the next patch I'm gonna send turbo mode support for
14 other Acer Predator laptops. The only required change is that I should add
"quirk_entry" for GPU and CPU fan count, and match product names to their quirk.

12 of these Predator laptops have exactly 1 fan for CPU and 1 fan for GPU,
but two of them have 2 fans for GPU and 1 for CPU. So my question is should
I add the quirks per product (which will end up to have 14 quirks,
same as current patch) like:

static struct quirk_entry quirk_acer_predator_ph315_53 = {
      .turbo = 1,
      .cpu_fans = 1,
      .gpu_fans = 1,
};

or should I specify the quirk per fan count like this one:

static struct quirk_entry quirk_acer_predator_gpu_fan_one_cpu_fan_one = {
      .turbo = 1,
      .cpu_fans = 1,
      .gpu_fans = 1,
};

and then set different matched DMI product names to the above quirk?

The first approach is more verbose, the second uses less code.
If possible, I would like to know your thoughts on this.

Thanks in advance,

Best,

Jafar

On Fri, Aug 13, 2021 at 3:52 PM Hans de Goede <hdegoede@...hat.com> wrote:
>
> Hi,
>
> On 8/12/21 2:53 PM, JafarAkhondali wrote:
> > Hi,
> >
> > The Acer Predator Helios series (usually denoted by PHxxx-yy) features
> > a particular key above the keyboard named "TURBO".
> > The turbo key does 3 things:
> > 1. Set all fan's speeds to TURBO mode
> > 2. Overclocks the CPU and GPU in the safe range
> > 3. Turn on an LED just below the turbo button
> >
> > All the above actions are operating using WMI function calls,
> > and there is no custom OC level for turbo. It acts as a flag
> > for enabling turbo mode instead of telling processors to use
> > a specific multiply of power (e.g. 1.3x of power).
> >
> > I've run some benchmark tests and it worked fine:
> >
> > GpuTest 0.7.0
> > http://www.geeks3d.com
> >
> > Module: FurMark
> > Normal mode Score: 7289 points (FPS: 121)
> > Turbo mode Score: 7675 points (FPS: 127)
> > Settings:
> > - 1920x1080 fullscreen
> > - antialiasing: Off
> > - duration: 60000 ms
> >
> > Renderer:
> > - GeForce RTX 2060/PCIe/SSE2
> > - OpenGL: 4.6.0 NVIDIA 460.32.03
> >
> > This feature is presented by Acer officially and should not harm
> > hardware in any case.
> >
> > A challenging part of implementing this feature is that calling
> > overclock function requires knowing the exact count of fans
> > for CPU and GPU of each model, which to the best of my
> > knowledge is not available in the kernel.
> >
> > So after checking the official PredatorSense application methods, it
> > turned out they have provided the software the list of fans in each model.
> > I have access to the mentioned list, and all similar PH-iii-jj can be
> > added easily by matching "DMI_PRODUCT_NAME".
> >
> > Creating a specific file for the Acer gaming features is not possible
> > because the current in use WMI event GUID is required for the turbo button
> > and it's not possible to register multiple listeners on a single WMI event.
> >
> >
> > Signed-off-by: JafarAkhondali <jafar.akhoondali@...il.com>
>
> Thank you for your patch, I've applied this patch to my review-hans
> branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
>
> Note it will show up in my review-hans branch once I've pushed my
> local branch there, which might take a while.
>
> Once I've run some tests on this branch the patches there will be
> added to the platform-drivers-x86/for-next branch and eventually
> will be included in the pdx86 pull-request to Linus for the next
> merge-window.
>
> Regards,
>
> Hans
>