[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7d76a774-9dad-4c94-b4df-7c040e9dbc47@kernel.org>
Date: Fri, 28 Feb 2025 13:44:49 -0600
From: Mario Limonciello <superm1@...nel.org>
To: Mark Pearson <mpearson-lenovo@...ebb.ca>,
Shyam Sundar S K <Shyam-sundar.S-k@....com>,
"Rafael J. Wysocki" <rafael@...nel.org>, Hans de Goede
<hdegoede@...hat.com>, Ilpo Järvinen
<ilpo.jarvinen@...ux.intel.com>, "Luke D . Jones" <luke@...nes.dev>
Cc: "platform-driver-x86@...r.kernel.org"
<platform-driver-x86@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
"Derek J . Clark" <derekjohn.clark@...il.com>,
Antheas Kapenekakis <lkml@...heas.dev>, me@...egospodneti.ch,
Denis Benato <benato.denis96@...il.com>,
"Limonciello, Mario" <mario.limonciello@....com>
Subject: Re: [PATCH 0/3] Add support for hidden choices to platform_profile
On 2/28/2025 13:39, Mark Pearson wrote:
> Hi Mario,
>
> On Fri, Feb 28, 2025, at 12:01 PM, Mario Limonciello wrote:
>> From: Mario Limonciello <mario.limonciello@....com>
>>
>> When two drivers provide platform profile handlers but use different
>> strings to mean (essentially) the same thing the legacy interface won't
>> export them because it only shows profiles common to multiple drivers.
>>
>> This causes an unexpected behavior to people who have upgraded from an
>> earlier kernel because if multiple drivers have bound platform profile
>> handlers they might not be able to access profiles they were expecting.
>>
>> Introduce a concept of a "hidden choice" that drivers can register and
>> the platform profile handler code will utilize when using the legacy
>> interface.
>>
>> There have been some other attempts at solving this issue in other ways.
>> This serves as an alternative to those attempts.
>>
>> Link:
>> https://lore.kernel.org/platform-driver-x86/e64b771e-3255-42ad-9257-5b8fc6c24ac9@gmx.de/T/#t
>> Link:
>> https://lore.kernel.org/platform-driver-x86/CAGwozwF-WVEgiAbWbRCiUaXf=BVa3KqmMJfs06trdMQHpTGmjQ@mail.gmail.com/T/#m2f3929e2d4f73cc0eedd14738170dad45232fd18
>> Cc: Antheas Kapenekakis <lkml@...heas.dev>
>> Cc: "Luke D. Jones" <luke@...nes.dev>
>>
>> Mario Limonciello (3):
>> ACPI: platform_profile: Add support for hidden choices
>> platform/x86/amd: pmf: Add 'quiet' to hidden choices
>> platform/x86/amd: pmf: Add balanced-performance to hidden choices
>>
>> drivers/acpi/platform_profile.c | 94 +++++++++++++++++++++++-------
>> drivers/platform/x86/amd/pmf/sps.c | 11 ++++
>> include/linux/platform_profile.h | 3 +
>> 3 files changed, 87 insertions(+), 21 deletions(-)
>>
>> --
>> 2.43.0
>
> The patches are all good - but my question is do we really need the whole hidden implementation bit?
>
> If the options are not hidden, and someone chooses quiet or balanced-performance for the amd-pmf driver - does it really matter that it's going to do the same as low-power or performance?
>
> So, same feedback as I had for Antheas's patches. I understand why this is being proposed but for me it is making things unnecessarily complicated.
>
> My personal vote remains that the amd_pmf driver carries the superset to keep everyone happy (sorry - it sucks to be the CPU vendor that has to play nice with everyone).
>
> Mark
Well so the problem with having all of them is specifically what happens
when "only" amd-pmf is bound?
If you advertise both "low power" and "quiet" it's really confusing to
userspace what the difference is.
The fact that it's actually 100% the same brings me to my personal
opinion on all of this. Although I spent time writing up this series to
do it this way my "preference" is that we permanently alias "low power"
and "quiet" to one another and update all drivers to use "low power"
instead.
Granted that doesn't help the case of balance-performance being hidden
that Antheas mentioned for acer-wmi and legion-wmi but I don't know
serious of a problem that actually is.
Powered by blists - more mailing lists