On Mon, 2025-02-24 at 15:52 -0500, Mark Pearson wrote: > 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 Hi Mark, I've supported over 80 different ASUS laptops in the last 6 years or so, I can offer some insight. Across the entire range (TUF, ROG, Vivobook, Zen) which implements some form of "thermal throttle" as it is called in asus-wmi (which is what is used by platform_profile) the difference between low-power and quiet is very much nil - the "quiet" profile is only a name, and the TDP is limited along with fans to match - so the result is "low-power". As Mario suggests in his reply perhaps an alias would be best, or, as I was going to do, simply rename the "quiet" profile in asus-wmi to "low- power" as I already did but have not submitted yet due to a large train of patches in progress. It's a single line change and nullifies the entire issue and this series. In any case asus handling of platform profile is something I have been steadily working on for the last few months for both laptops and handhelds and I will have a new patch series coming soon (version 7 of previously submitted dealing with this). This submitted series is a NACK from me. Cheers, Luke.