On Thu, 1 Sept 2022 at 13:16, Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > > Hi, > > On 8/23/22 12:29, Shyam Sundar S K wrote: > > In this series, support for following features has been added. > > - "Cool n Quiet Framework (CnQF)" is an extension to the static slider, > > where the system power can be boosted or throttled independent > > of the selected slider position. > > - On the fly, the CnQF can be turned on/off via a sysfs knob. > > Thank you. I think that before doing a more in detail review > we first need to agree on the userspace interactions here. > > I've added Bastien, the power-profiles-daemon maintainer > to the Cc for this. > > From a quick peek at the patches I see that currently they do > the following: > > Probe time: > ----------- > > 1. If static slider (classic /sys/firmware/acpi/platform_profile) > is available register as a platform_profile provider > > 2. Query if the BIOS tells us that CnQF should be enable by > default if yes then unregister the platform_profile provider > and enable CnQF > > > Run time: > --------- > > Allow turning CnQF on/off by writing a sysfs attribute for this. > > 1. When CnQF gets enabled unregister the platform_profile provider > > 2. When CnQF gets disabled restore the last set profile and > register the platform_profile provider > > > Questions/remarks: > > 1. If you look at 1. and 2. under "Probe time", you will see that > when the BIOS requests to have CnQF enabled by default that > userspace will then still shortly see a platform_profile > provider. This must be fixed IMHO by checking whether to do > CnQF by default or not before the initial register call. > > 2. What about low-power scenarios ? Currently power-profiles-daemon > will always advertise a low-power mode even when there is no > platform-profile support, since this is also a hint for other > parts of the system to try and conserve power. But when this > mode is enabled we really want the system to also behave as > if the old static slider mode is active and set to low-power. > > Some ideas: > a) maybe still have the amd-pmf code register a (different) > platform_profile provider whn in CnQF mode and have it only > advertise low-power > > b) teach power-profiles-daemon about CnQF and have it > disable CnQF when entering low-power mode? > > c) make the CnQF code in PMF take the charge level into > account and have it not go "full throttle" when the chare > is below say 25% ? > > 3. Bastien, can power-profiles-daemon deal with > /sys/firmware/acpi/platform_profile disappearing or > appearing while it is running? No, it doesn't. It expects the platform_profile file to be available on startup, at worse with the choices not yet filled in. It doesn't handle the platform_profile file going away, it doesn't handle the platform_profile_choices file changing after it's been initially filled in, and it doesn't support less than one power profile being made available, and only supports hiding the performance profile if the platform doesn't support it. Some of those things we could change/fix, some other things will not. If the platform_profile_choices file only contained a single item, then power-profiles-daemon would just export the "low-power" and "balanced" profiles to user-space, as it does on unsupported hardware. The profiles in power-profiles-daemon are supposed to show the user intent, which having a single setting would effectively nullify. It's unclear to me how CnQF takes user intent into account (it's also unclear to me how that's a low-power setting rather than a combination of the existing cool and quiet settings).
