Re: [PATCH 0/4] platform/x86/amd/pmf: Introduce CnQF feature for AMD PMF

[Hans de Goede <hdegoede@redhat.com>]

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? 

Regards,

Hans



> 
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> 
> Shyam Sundar S K (4):
>   platform/x86/amd/pmf: Add support for CnQF
>   platform/x86/amd/pmf: Add sysfs to toggle CnQF
>   Documentation/ABI/testing/sysfs-amd-pmf: Add ABI doc for AMD PMF
>   MAINTAINERS: Update ABI doc path for AMD PMF
> 
>  Documentation/ABI/testing/sysfs-amd-pmf |  11 +
>  MAINTAINERS                             |   1 +
>  drivers/platform/x86/amd/pmf/Makefile   |   2 +-
>  drivers/platform/x86/amd/pmf/acpi.c     |  10 +
>  drivers/platform/x86/amd/pmf/cnqf.c     | 374 ++++++++++++++++++++++++
>  drivers/platform/x86/amd/pmf/core.c     |  16 +-
>  drivers/platform/x86/amd/pmf/pmf.h      | 100 +++++++
>  7 files changed, 512 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-amd-pmf
>  create mode 100644 drivers/platform/x86/amd/pmf/cnqf.c
>