Message-Id: <1c0c988b-8fe6-4857-9556-6ac6880b76ff@app.fastmail.com>
Date: Mon, 24 Feb 2025 15:52:17 -0500
From: "Mark Pearson" <mpearson-lenovo@...ebb.ca>
To: "Antheas Kapenekakis" <lkml@...heas.dev>,
 "Limonciello, Mario" <mario.limonciello@....com>
Cc: 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

Hi Antheas,

On Mon, Feb 24, 2025, at 2:50 PM, Antheas Kapenekakis wrote:
> On the Asus Z13 (2025), a device that would need the amd-pmf quirk that
> was removed on the platform_profile refactor, we see the following output
> from the sysfs platform profile:
>
> $ cat /sys/firmware/acpi/platform_profile_choices
> balanced performance
>
> I.e., the quiet profile is missing. Which is a major regression in terms of
> power efficiency and affects both tuned, and ppd (it also affected my
> software but I fixed that on Saturday). This would affect any laptop that
> loads both amd-pmf and asus-wmi (around 15 models give or take?).
>
> The problem stems from the fact that asus-wmi uses quiet, and amd-pmf uses
> low-power. While it is not clear to me what the amd-pmf module is supposed
> to do here, and perhaps some autodetection should be done and make it bail,
> if we assume it should be kept, then there is a small refactor that is
> needed to maintain the existing ABI interface.
>
> This is the subject of this patch series.
>
> Essentially, we introduce the concept of a "secondary" handler. Secondary
> handlers work exactly the same, except for the fact they are able to
> receive all profile names through the sysfs interface. The expectation
> here would be that the handlers choose the closest appropriate profile
> they have, and this is what I did for the amd-pmf handler.
>
> In their own platform_profile namespace, these handlers still work normally
> and only accept the profiles from their probe functions, with -ENOSUP for
> the rest.
>
> In the absence of a primary handler, the options of all secondary handlers
> are unioned in the legacy sysfs, which prevents them from hiding each
> other's options.
>
> With this patch series applied, the sysfs interface will look like this:
>
> $ cat /sys/firmware/acpi/platform_profile_choices
> quiet balanced performance
>
> And writing quiet to it results in the profile being applied to both
> platform profile handlers.
>
> $ echo low-power > /sys/firmware/acpi/platform_profile
> bash: echo: write error: Operation not supported
> $ echo quiet > /sys/firmware/acpi/platform_profile
> $ cat /sys/class/platform-profile/platform-profile-*/{name,profile}
> asus-wmi
> amd-pmf
> quiet
> quiet
>
> Agreed ABI still works:
> $ echo quiet > /sys/class/platform-profile/platform-profile-0/profile
> $ echo quiet > /sys/class/platform-profile/platform-profile-1/profile
> bash: echo: write error: Operation not supported
> $ echo low-power > /sys/class/platform-profile/platform-profile-0/profile
> bash: echo: write error: Operation not supported
> $ echo low-power > /sys/class/platform-profile/platform-profile-1/profile
>

I understand where you're coming from with this implementation but my concern is this is making profiles more complicated - and they're already becoming hard to understand (and debug) for users.

I'm not a huge fan of multiple profile handlers, but can see why some people might want them and that they're a valid tool to have (especially given some of the limitations of what platform vendors themselves implement).

In patch #3 it states that 'It is the expectation that secondary handlers will pick the closest profile they have to what was sent'. I'm not convinced that is true, or desired.

e.g. Quiet and low-power are different things and can have different implementations. One is giving you as much power as possible with the fans running below a certain audible level; and one is giving you a system with as low-power consumption as possible, but still be usable. They're admittedly not very different in practice - but they can be different.

Would it be better here to ask AMD to implement a quiet profile (maybe it can be based on low-power, at least initially)?
I think that would solve the ASUS issue and not introduce another layer of complexity.

Mark

Powered by blists - more mailing lists