On Fri, Feb 28, 2025, at 2:44 PM, Mario Limonciello wrote: > 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@xxxxxxx> >>> >>> 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@xxxxxx/T/#t >>> Link: >>> https://lore.kernel.org/platform-driver-x86/CAGwozwF-WVEgiAbWbRCiUaXf=BVa3KqmMJfs06trdMQHpTGmjQ@xxxxxxxxxxxxxx/T/#m2f3929e2d4f73cc0eedd14738170dad45232fd18 >>> Cc: Antheas Kapenekakis <lkml@xxxxxxxxxxx> >>> Cc: "Luke D. Jones" <luke@xxxxxxxxxx> >>> >>> 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. Ah - I guess you get platforms without profile support where amd-pmf is the only thing. I hadn't considered that. FWIW, I believe we (Lenovo) have both low power and quiet on Windows...and they really don't make much difference (which is why the thermal team didn't do them both on Linux). I don't know if Windows users are more or less confused (or maybe they've just abandoned all hope and are migrating to Linux...) You have a better feeling as to how many issues you'll get raised if they behave the same, and have to support a wider ecosystem, so I'm happy to be over-ruled. I just wanted to wave my flag that I think the driver is getting too complicated. I'm slightly dreading having to debug customer issues at this point. > > 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. > Guaranteed if you do that some vendor will have something that differentiates. I can see having a 'use as much power as possible without needing fans' for quiet, and 'make the battery last as long as humanely possible whilst keeping the system usable' mode for low-power. Don't think anyone has done it...but they could. > 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. I really have to go play with this on the Legion-Go. I need a time-machine..... Mark
