Message-ID: <5cff4286-2800-4bc0-b243-5244d19a64b7@gmx.de>
Date: Tue, 25 Feb 2025 16:56:27 +0100
From: Armin Wolf <W_Armin@....de>
To: Antheas Kapenekakis <lkml@...heas.dev>, Luke Jones <luke@...nes.dev>
Cc: Mark Pearson <mpearson-lenovo@...ebb.ca>,
 "Limonciello, Mario" <mario.limonciello@....com>,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
 Len Brown <lenb@...nel.org>,
 "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
 linux-kernel@...r.kernel.org,
 "platform-driver-x86@...r.kernel.org" <platform-driver-x86@...r.kernel.org>,
 "Rafael J. Wysocki" <rafael@...nel.org>, Hans de Goede
 <hdegoede@...hat.com>, me@...egospodneti.ch
Subject: Re: [PATCH 0/3] ACPI: platform_profile: fix legacy sysfs with
 multiple handlers

Am 25.02.25 um 03:26 schrieb Antheas Kapenekakis:

>> If these "scripts" use `platform_profile_choices` to get their
>> selections and verify they are available then there should be zero
>> breakage. If they don't then they should be updated to be correct.
> Yeah, if any Asus users wrote scripts for their laptops to e.g., "echo
> quiet | sudo tee /sys/firmware/acpi/platform_profile" or used TLP let
> them spend a few days finding out why kernel 6.14 does not work. They
> should have written a 300 line bash script instead.

Hi,

using "echo quiet | sudo tee /sys/firmware/acpi/platform_profile" is quite
brittle, as some hardware will populate the available profiles dynamically.

Still breaking userspace is indeed not an option here, so we have to think
of something else.

>> In any case I am in the process of finalising an update to use the new
>> platform_profile API including "custom". Please don't begin trying to
>> break things just to be "first". My work has been ongoing for this
>   drivers/acpi/platform_profile.c    | 57 +++++++++++++++++++++++++-----
>   drivers/platform/x86/amd/pmf/spc.c |  3 ++
>   drivers/platform/x86/amd/pmf/sps.c |  8 +++++
>   include/linux/platform_profile.h   |  7 ++++
>
> I do not see the name Asus here. This is a compatibility patch. You
> should try it before commenting on it further. Looking at my ACPI
> database, there are at least a few Ayaneos, GPDs, and Legion laptops
> that have the ACPI bindings for pmf, this is not an Asus issue.
>
> By the way, I have merged your patch series on Bazzite (well... a
> cleaned up version that does not happen to crash your own software...)
> and it happens to work fine with this patch (I know you said platform
> profiles are not in yet). I still use the asus-wmi APIs personally.
>
> sudo fwupdmgr get-bios-setting
> Authenticating…          [ -                                     ]
> ppt_pl3_fppt:
>    Setting type:         Integer
>    Current Value:        80
>    Description:          Set the CPU slow package limit
>    Read Only:            False
>    Minimum value:        5
> ...
>
>> in my spare time for months.
> Let me comment on this a bit further. Hobbies are good to have and it
> is nice you found one you like. However, a lot of people are spending
> a lot of money on their Asus laptops and are actually starting to
> depend on Linux. If they cannot depend on you or your hobby for
> support, you should at least make sure to not interfere with parallel
> efforts for that support, if not try to be synergistic.

This whole driver was likely written by someone as a hobby, so you already
depend on a hobby here.

That being said, i agree that fixes have a priority over new features, and
i think everyone agrees on that.

> I did not make this patch to one up you or rush it. This issue is a
> blocker for deploying our 6.13 kernel. Since this kernel needs to work
> for the Z13 and pmf quirks are dead ends now (I also got annoyed by
> asus users complaining about their fan curves being wrong because pmf
> blew up), I pulled in Mario's platform profile series early, only to
> find this issue. Botching the asus-wmi platform handler did not meet
> my standards, so I had to make this series. Also, since I could not
> pull in Kurk's series, and his changes were extensive, I had to make
> this series twice, and test it twice.
>
> Good news is this series works and the kernel is on its way to be
> deployed in a few days. Flatpak fix came in clutch today too with
> 6.13.4.
>
> Antheas
>
Maybe the current strategy of the legacy platform-profile interface can be extended
without introducing the "secondary handler" concept.

The current strategy only advertises platform profiles supported by all handlers, and
as you pointed out this causes problems for users on certain devices.

I was thinking that be can change this strategy to advertise all platform profiles supported
by at least one handler can then do something like this:

  - handler 1: supports low_power, balanced and performance

  - handler 2: supports quiet, balanced and balanced-performance

-> legacy interface advertises low_power, quiet, balanced, balanced-performance and performance

When setting low_power, the closes equivalent is picked for handlers which do not support low_power:

  - handler 1: setting low_power

  - handler 2: setting quiet

When setting quiet, the same happens:

  - handler 1: setting balanced

  - handler 2: setting quiet

Basically all profiles get treated like a range:

low_power <- lower end of the performance range
cool,
quiet,
balanced,
balanced-performance,
performance <- upper end of the performance range

The only problem will be that getting the current platform profile would be more difficult, as
the legacy handler has to determine the lowest currently selected platform profile.

Would this approach be OK?

Thanks,
Armin Wolf

Powered by blists - more mailing lists