All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 0/1] Documentation: Add documentation for new performance_profile sysfs class
@ 2020-10-03 13:19 Hans de Goede
  2020-10-03 13:19 ` [RFC] " Hans de Goede
  2020-10-03 13:39 ` [RFC 0/1] " Hans de Goede
  0 siblings, 2 replies; 26+ messages in thread
From: Hans de Goede @ 2020-10-03 13:19 UTC (permalink / raw)
  To: Darren Hart, Andy Shevchenko, Mark Gross
  Cc: Hans de Goede, Mario Limonciello, Mark Pearson, Elia Devito,
	Bastien Nocera, Benjamin Berg, linux-pm, linux-acpi,
	platform-driver-x86, linux-kernel

Hi All,

Recently 2 different patches have been submitted for drivers under
drivers/platform/x86 to configure the performance-profile of
modern laptops (see the actual RFC patch for what I mean with
a performance-profile). One for the thinkpad_acpi driver and
one for the hp-wmi driver.

Since I don't want each pdx86 driver to invent its own userspace API
for this I have started a dicussion about coming up with a standardized /
common sysfs class / API for this on the pdx86 list:
https://www.spinics.net/lists/platform-driver-x86/msg22794.html

The sysfs API proposal which I'm sending out as RFC in this email
thread is the result of me trying to distill that discussion into
a concrete proposal.

I have Cc-ed the linux-pm and linux-acpi lists because even though
the trigger for doing this is 2 different pdx86 drivers, the resulting
API should (must even) also be suitable for other platforms. I can
e.g. see various modern ARM platforms also having similar functionality
which they may want to export to userspace and the ideally the userspace
code for allowing the end-user to configure/select a profile would be
the same under ARM and x86.

Talking about userspace I've also Cc-ed Bastien and Benjamin who are
working on the userspace side of this.

Regards,

Hans


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-03 13:19 [RFC 0/1] Documentation: Add documentation for new performance_profile sysfs class Hans de Goede
@ 2020-10-03 13:19 ` Hans de Goede
  2020-10-04  1:33   ` [External] " Mark Pearson
                     ` (3 more replies)
  2020-10-03 13:39 ` [RFC 0/1] " Hans de Goede
  1 sibling, 4 replies; 26+ messages in thread
From: Hans de Goede @ 2020-10-03 13:19 UTC (permalink / raw)
  To: Darren Hart, Andy Shevchenko, Mark Gross
  Cc: Hans de Goede, Mario Limonciello, Mark Pearson, Elia Devito,
	Bastien Nocera, Benjamin Berg, linux-pm, linux-acpi,
	platform-driver-x86, linux-kernel, Mark Pearson

On modern systems CPU/GPU/... performance is often dynamically configurable
in the form of e.g. variable clock-speeds and TPD. The performance is often
automatically adjusted to the load by some automatic-mechanism (which may
very well live outside the kernel).

These auto performance-adjustment mechanisms often can be configured with
one of several performance-profiles, with either a bias towards low-power
consumption (and cool and quiet) or towards performance (and higher power
consumption and thermals).

Introduce a new performance_profile class/sysfs API which offers a generic
API for selecting the performance-profile of these automatic-mechanisms.

Cc: Mark Pearson <markpearson@lenovo.com>
Cc: Elia Devito <eliadevito@gmail.com>
Cc: Bastien Nocera <hadess@hadess.net>
Cc: Benjamin Berg <bberg@redhat.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-acpi@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../testing/sysfs-class-performance_profile   | 104 ++++++++++++++++++
 1 file changed, 104 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-performance_profile

diff --git a/Documentation/ABI/testing/sysfs-class-performance_profile b/Documentation/ABI/testing/sysfs-class-performance_profile
new file mode 100644
index 000000000000..9c67cae39600
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-performance_profile
@@ -0,0 +1,104 @@
+Performance-profile selection (e.g. /sys/class/performance_profile/thinkpad_acpi/)
+
+On modern systems CPU/GPU/... performance is often dynamically configurable
+in the form of e.g. variable clock-speeds and TPD. The performance is often
+automatically adjusted to the load by some automatic-mechanism (which may
+very well live outside the kernel).
+
+These auto performance-adjustment mechanisms often can be configured with
+one of several performance-profiles, with either a bias towards low-power
+consumption (and cool and quiet) or towards performance (and higher power
+consumption and thermals).
+
+The purpose of the performance_profile class is to offer a generic sysfs
+API for selecting the performance-profile of these automatic-mechanisms.
+
+Note that this API is only for selecting the performance-profile, it is
+NOT a goal of this API to allow monitoring the resulting performance
+characteristics. Monitoring performance is best done with device/vendor
+specific tools such as e.g. turbostat.
+
+Specifically when selecting a high-performance profile the actual achieved
+performance may be limited by various factors such as: the heat generated by
+other components, room temperature, free air flow at the bottom of a laptop,
+etc. It is explicitly NOT a goal of this API to let userspace know about
+any sub-optimal conditions which are impeding reaching the requested
+performance level.
+
+Since numbers are a rather meaningless way to describe performance-profiles
+this API uses strings to describe the various profiles. To make sure that
+userspace gets a consistent experience when using this API this API document
+defines a fixed set of profile-names. Drivers *must* map their internal
+profile representation/names onto this fixed set.
+
+If for some reason there is no good match when mapping then a new profile-name
+may be added. Drivers which wish to introduce new profile-names must:
+1. Have very good reasons to do so.
+2. Add the new profile-name to this document, so that future drivers which also
+   have a similar problem can use the same new. Usually new profile-names will
+   be added to the "extra profile-names" section of this document. But in some
+   cases the set of standard profile-names may be extended.
+
+What:		/sys/class/performance_profile/<device>/available_profiles
+Date:		October 2020
+Contact:	Hans de Goede <hdegoede@redhat.com>
+Description:
+		Reading this file gives a space separated list of profiles
+		supported for this device.
+
+		Drivers must use the following standard profile-names whenever
+		possible:
+
+		low-power:		Emphasises low power consumption
+					(and also cool and quiet)
+		balanced-low-power:	Balances between low power consumption
+					and performance with a slight bias
+					towards low power
+		balanced:		Balance between low power consumption
+					and performance
+		balanced-performance:	Balances between performance and low
+					power consumption with a slight bias
+					towards performance
+		performance:		Emphasises performance (and may lead to
+					higher temperatures and fan speeds)
+
+		Userspace may expect drivers to offer at least several of these
+		standard profile-names! If none of the above are a good match
+		for some of the drivers profiles, then drivers may use one of
+		these extra profile-names:
+		<reserved for future use>
+
+What:		/sys/class/performance_profile/<device>/current_profile
+Date:		October 2020
+Contact:	Hans de Goede <hdegoede@redhat.com>
+Description:
+		Reading this file gives the current selected profile for this
+		device. Writing this file with one of the strings from
+		available_profiles changes the profile to the new value.
+
+		Reading this file may also return "custom". This is intended for
+		drivers which have and export multiple knobs influencing
+		performance. Such drivers may very well still want to offer a
+		set of profiles for easy of use and to be able to offer a
+		consistent standard API (this API) to userspace for configuring
+		their performance. The "custom" value is intended for when a
+		user has directly configured the knobs (through e.g. some
+		advanced control-panel for a GPU) and the knob values do not
+		match any of the presets represented by the
+		performance-profiles. In this case writing this file will
+		override the modifications and restore the selected presets.
+
+What:		/sys/class/performance_profile/<device>/type
+Date:		October 2020
+Contact:	Hans de Goede <hdegoede@redhat.com>
+Description:
+		Performance-profiles may be system-wide, or for a specific
+		device (e.g. CPU / GPU). System-wide profiles are typically
+		used on devices where where a single cooling solution is
+		shared between all components, such as laptops and NUCs.
+
+		Reading this file indicates the type of the device for which
+		the thermal-profile is being configured.
+
+		Valid values: "system"
+		Reserved for future use values: "cpu", "gpu"
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [RFC 0/1] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-03 13:19 [RFC 0/1] Documentation: Add documentation for new performance_profile sysfs class Hans de Goede
  2020-10-03 13:19 ` [RFC] " Hans de Goede
@ 2020-10-03 13:39 ` Hans de Goede
  1 sibling, 0 replies; 26+ messages in thread
From: Hans de Goede @ 2020-10-03 13:39 UTC (permalink / raw)
  To: Darren Hart, Andy Shevchenko, Mark Gross
  Cc: Mario Limonciello, Mark Pearson, Elia Devito, Bastien Nocera,
	Benjamin Berg, linux-pm, linux-acpi, platform-driver-x86,
	linux-kernel

Hi,

On 10/3/20 3:19 PM, Hans de Goede wrote:
> Hi All,
> 
> Recently 2 different patches have been submitted for drivers under
> drivers/platform/x86 to configure the performance-profile of
> modern laptops (see the actual RFC patch for what I mean with
> a performance-profile). One for the thinkpad_acpi driver and
> one for the hp-wmi driver.
> 
> Since I don't want each pdx86 driver to invent its own userspace API
> for this I have started a dicussion about coming up with a standardized /
> common sysfs class / API for this on the pdx86 list:
> https://www.spinics.net/lists/platform-driver-x86/msg22794.html
> 
> The sysfs API proposal which I'm sending out as RFC in this email
> thread is the result of me trying to distill that discussion into
> a concrete proposal.
> 
> I have Cc-ed the linux-pm and linux-acpi lists because even though
> the trigger for doing this is 2 different pdx86 drivers, the resulting
> API should (must even) also be suitable for other platforms. I can
> e.g. see various modern ARM platforms also having similar functionality
> which they may want to export to userspace and the ideally the userspace
> code for allowing the end-user to configure/select a profile would be
> the same under ARM and x86.
> 
> Talking about userspace I've also Cc-ed Bastien and Benjamin who are
> working on the userspace side of this.

p.s.

About the type part of the proposed sysfs API for this, the idea
here is that e.g. the Intel pstate driver could also export a
performance-profile interface, mirroring the custom interface it
currently has for this.

The performance-profile-daemon (p-p-d) Bastien is working on already
talks to the pstate driver in some cases.  Currently it does this using
the pstate drivers own/custom API, but that does not really scale. If
ARM or AMD chips get similar functionality in the future then ideally
these would export a /sys/class/performance-profile with a type
of "cpu" and then p-p-d would just need to talk to the performance-profile
API, instead of needing to have its own internal HAL to deal with
different CPU vendors.

Regards,

Hans


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [External] [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-03 13:19 ` [RFC] " Hans de Goede
@ 2020-10-04  1:33   ` Mark Pearson
  2020-10-04 22:29   ` Elia Devito
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 26+ messages in thread
From: Mark Pearson @ 2020-10-04  1:33 UTC (permalink / raw)
  To: Hans de Goede, Darren Hart, Andy Shevchenko, Mark Gross
  Cc: Mario Limonciello, Mark Pearson, Elia Devito, Bastien Nocera,
	Benjamin Berg, linux-pm, linux-acpi, platform-driver-x86,
	linux-kernel

Hi Hans,

On 2020-10-03 9:19 a.m., Hans de Goede wrote:
> On modern systems CPU/GPU/... performance is often dynamically configurable
> in the form of e.g. variable clock-speeds and TPD. The performance is often
> automatically adjusted to the load by some automatic-mechanism (which may
> very well live outside the kernel).
> 
> These auto performance-adjustment mechanisms often can be configured with
> one of several performance-profiles, with either a bias towards low-power
> consumption (and cool and quiet) or towards performance (and higher power
> consumption and thermals).
> 
> Introduce a new performance_profile class/sysfs API which offers a generic
> API for selecting the performance-profile of these automatic-mechanisms.
> 
> Cc: Mark Pearson <markpearson@lenovo.com>
> Cc: Elia Devito <eliadevito@gmail.com>
> Cc: Bastien Nocera <hadess@hadess.net>
> Cc: Benjamin Berg <bberg@redhat.com>
> Cc: linux-pm@vger.kernel.org
> Cc: linux-acpi@vger.kernel.org
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>   .../testing/sysfs-class-performance_profile   | 104 ++++++++++++++++++
>   1 file changed, 104 insertions(+)
>   create mode 100644 Documentation/ABI/testing/sysfs-class-performance_profile
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-performance_profile b/Documentation/ABI/testing/sysfs-class-performance_profile
> new file mode 100644
> index 000000000000..9c67cae39600
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-performance_profile
> @@ -0,0 +1,104 @@
> +Performance-profile selection (e.g. /sys/class/performance_profile/thinkpad_acpi/)
> +
> +On modern systems CPU/GPU/... performance is often dynamically configurable
> +in the form of e.g. variable clock-speeds and TPD. The performance is often
> +automatically adjusted to the load by some automatic-mechanism (which may
> +very well live outside the kernel).
> +
> +These auto performance-adjustment mechanisms often can be configured with
> +one of several performance-profiles, with either a bias towards low-power
> +consumption (and cool and quiet) or towards performance (and higher power
> +consumption and thermals).
> +
> +The purpose of the performance_profile class is to offer a generic sysfs
> +API for selecting the performance-profile of these automatic-mechanisms.
> +
> +Note that this API is only for selecting the performance-profile, it is
> +NOT a goal of this API to allow monitoring the resulting performance
> +characteristics. Monitoring performance is best done with device/vendor
> +specific tools such as e.g. turbostat.
> +
> +Specifically when selecting a high-performance profile the actual achieved
> +performance may be limited by various factors such as: the heat generated by
> +other components, room temperature, free air flow at the bottom of a laptop,
> +etc. It is explicitly NOT a goal of this API to let userspace know about
> +any sub-optimal conditions which are impeding reaching the requested
> +performance level.
> +
> +Since numbers are a rather meaningless way to describe performance-profiles
> +this API uses strings to describe the various profiles. To make sure that
> +userspace gets a consistent experience when using this API this API document
> +defines a fixed set of profile-names. Drivers *must* map their internal
> +profile representation/names onto this fixed set.
> +
> +If for some reason there is no good match when mapping then a new profile-name
> +may be added. Drivers which wish to introduce new profile-names must:
> +1. Have very good reasons to do so.
> +2. Add the new profile-name to this document, so that future drivers which also
> +   have a similar problem can use the same new. Usually new profile-names will
> +   be added to the "extra profile-names" section of this document. But in some
> +   cases the set of standard profile-names may be extended.
> +
> +What:		/sys/class/performance_profile/<device>/available_profiles
> +Date:		October 2020
> +Contact:	Hans de Goede <hdegoede@redhat.com>
> +Description:
> +		Reading this file gives a space separated list of profiles
> +		supported for this device.
> +
> +		Drivers must use the following standard profile-names whenever
> +		possible:
> +
> +		low-power:		Emphasises low power consumption
> +					(and also cool and quiet)
> +		balanced-low-power:	Balances between low power consumption
> +					and performance with a slight bias
> +					towards low power
> +		balanced:		Balance between low power consumption
> +					and performance
> +		balanced-performance:	Balances between performance and low
> +					power consumption with a slight bias
> +					towards performance
> +		performance:		Emphasises performance (and may lead to
> +					higher temperatures and fan speeds)
> +
> +		Userspace may expect drivers to offer at least several of these
> +		standard profile-names! If none of the above are a good match
> +		for some of the drivers profiles, then drivers may use one of
> +		these extra profile-names:
> +		<reserved for future use>
> +
> +What:		/sys/class/performance_profile/<device>/current_profile
> +Date:		October 2020
> +Contact:	Hans de Goede <hdegoede@redhat.com>
> +Description:
> +		Reading this file gives the current selected profile for this
> +		device. Writing this file with one of the strings from
> +		available_profiles changes the profile to the new value.
> +
> +		Reading this file may also return "custom". This is intended for
> +		drivers which have and export multiple knobs influencing
> +		performance. Such drivers may very well still want to offer a
> +		set of profiles for easy of use and to be able to offer a
> +		consistent standard API (this API) to userspace for configuring
> +		their performance. The "custom" value is intended for when a
> +		user has directly configured the knobs (through e.g. some
> +		advanced control-panel for a GPU) and the knob values do not
> +		match any of the presets represented by the
> +		performance-profiles. In this case writing this file will
> +		override the modifications and restore the selected presets.
> +
> +What:		/sys/class/performance_profile/<device>/type
> +Date:		October 2020
> +Contact:	Hans de Goede <hdegoede@redhat.com>
> +Description:
> +		Performance-profiles may be system-wide, or for a specific
> +		device (e.g. CPU / GPU). System-wide profiles are typically
> +		used on devices where where a single cooling solution is
> +		shared between all components, such as laptops and NUCs.
> +
> +		Reading this file indicates the type of the device for which
> +		the thermal-profile is being configured.
> +
> +		Valid values: "system"
> +		Reserved for future use values: "cpu", "gpu"
> 
Thanks for putting this together. From my point of view it looks good 
and no objections or suggestions to add. It will work nicely for what we 
are trying to do, but I think it has enough flexibility to fit other use 
cases. I like it.

Mark

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-03 13:19 ` [RFC] " Hans de Goede
  2020-10-04  1:33   ` [External] " Mark Pearson
@ 2020-10-04 22:29   ` Elia Devito
  2020-10-09 10:52     ` Hans de Goede
  2020-10-05 12:58   ` Limonciello, Mario
  2020-10-05 13:13   ` Benjamin Berg
  3 siblings, 1 reply; 26+ messages in thread
From: Elia Devito @ 2020-10-04 22:29 UTC (permalink / raw)
  To: Darren Hart, Andy Shevchenko, Mark Gross, Hans de Goede
  Cc: Hans de Goede, Mario Limonciello, Mark Pearson, Bastien Nocera,
	Benjamin Berg, linux-pm, linux-acpi, platform-driver-x86,
	linux-kernel, Mark Pearson

Hi Hans,

On 2020-10-03 9:19 a.m., Hans de Goede wrote:
> On modern systems CPU/GPU/... performance is often dynamically configurable
> in the form of e.g. variable clock-speeds and TPD. The performance is often
> automatically adjusted to the load by some automatic-mechanism (which may
> very well live outside the kernel).
>
> These auto performance-adjustment mechanisms often can be configured with
> one of several performance-profiles, with either a bias towards low-power
> consumption (and cool and quiet) or towards performance (and higher power
> consumption and thermals).
>
> Introduce a new performance_profile class/sysfs API which offers a generic
> API for selecting the performance-profile of these automatic-mechanisms.
>
> Cc: Mark Pearson <markpearson@lenovo.com>
> Cc: Elia Devito <eliadevito@gmail.com>
> Cc: Bastien Nocera <hadess@hadess.net>
> Cc: Benjamin Berg <bberg@redhat.com>
> Cc: linux-pm@vger.kernel.org
> Cc: linux-acpi@vger.kernel.org
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> .../testing/sysfs-class-performance_profile   | 104 ++++++++++++++++++
> 1 file changed, 104 insertions(+)
> create mode 100644
> Documentation/ABI/testing/sysfs-class-performance_profile
>
> diff --git a/Documentation/ABI/testing/sysfs-class-performance_profile
> b/Documentation/ABI/testing/sysfs-class-performance_profile new file mode
> 100644
> index 000000000000..9c67cae39600
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-performance_profile
> @@ -0,0 +1,104 @@
> +Performance-profile selection (e.g.
> /sys/class/performance_profile/thinkpad_acpi/) +
> +On modern systems CPU/GPU/... performance is often dynamically configurable
> +in the form of e.g. variable clock-speeds and TPD. The performance is
> often +automatically adjusted to the load by some automatic-mechanism
> (which may +very well live outside the kernel).
> +
> +These auto performance-adjustment mechanisms often can be configured with
> +one of several performance-profiles, with either a bias towards low-power
> +consumption (and cool and quiet) or towards performance (and higher power
> +consumption and thermals).
> +
> +The purpose of the performance_profile class is to offer a generic sysfs
> +API for selecting the performance-profile of these automatic-mechanisms.
> +
> +Note that this API is only for selecting the performance-profile, it is
> +NOT a goal of this API to allow monitoring the resulting performance
> +characteristics. Monitoring performance is best done with device/vendor
> +specific tools such as e.g. turbostat.
> +
> +Specifically when selecting a high-performance profile the actual achieved
> +performance may be limited by various factors such as: the heat generated
> by +other components, room temperature, free air flow at the bottom of a
> laptop, +etc. It is explicitly NOT a goal of this API to let userspace know
> about +any sub-optimal conditions which are impeding reaching the requested
> +performance level.
> +
> +Since numbers are a rather meaningless way to describe performance-profiles
> +this API uses strings to describe the various profiles. To make sure that
> +userspace gets a consistent experience when using this API this API
> document +defines a fixed set of profile-names. Drivers *must* map their
> internal +profile representation/names onto this fixed set.
> +
> +If for some reason there is no good match when mapping then a new
> profile-name +may be added. Drivers which wish to introduce new
> profile-names must: +1. Have very good reasons to do so.
> +2. Add the new profile-name to this document, so that future drivers which
> also +   have a similar problem can use the same new. Usually new
> profile-names will +   be added to the "extra profile-names" section of
> this document. But in some +   cases the set of standard profile-names may
> be extended.
> +
> +What:          /sys/class/performance_profile/<device>/
available_profiles
> +Date:          October 2020
> +Contact:       Hans de Goede <hdegoede@redhat.com>
> +Description:
> +               Reading this file gives a space separated list of profiles
> +               supported for this device.
> +
> +               Drivers must use the following standard profile-names 
whenever
> +               possible:
> +
> +               low-power:              Emphasises low power consumption
> +                                       (and also cool and 
quiet)
> +               balanced-low-power:     Balances between low power 
consumption
> +                                       and performance with a 
slight bias
> +                                       towards low power
> +               balanced:               Balance between low power 
consumption
> +                                       and performance
> +               balanced-performance:   Balances between performance and 
low
> +                                       power consumption with 
a slight bias
> +                                       towards performance
> +               performance:            Emphasises performance 
(and may lead to
> +                                       higher temperatures and 
fan speeds)
> +
> +               Userspace may expect drivers to offer at least several of 
these
> +               standard profile-names! If none of the above are a good 
match
> +               for some of the drivers profiles, then drivers may use 
one of
> +               these extra profile-names:
> +               <reserved for future use>
> +
> +What:          /sys/class/performance_profile/<device>/current_profile
> +Date:          October 2020
> +Contact:       Hans de Goede <hdegoede@redhat.com>
> +Description:
> +               Reading this file gives the current selected profile for 
this
> +               device. Writing this file with one of the strings from
> +               available_profiles changes the profile to the new value.
> +
> +               Reading this file may also return "custom". This is 
intended for
> +               drivers which have and export multiple knobs influencing
> +               performance. Such drivers may very well still want to 
offer a
> +               set of profiles for easy of use and to be able to offer a
> +               consistent standard API (this API) to userspace for 
configuring
> +               their performance. The "custom" value is intended for 
when a
> +               user has directly configured the knobs (through e.g. some
> +               advanced control-panel for a GPU) and the knob values do 
not
> +               match any of the presets represented by the
> +               performance-profiles. In this case writing this file will
> +               override the modifications and restore the selected 
presets.
> +
> +What:          /sys/class/performance_profile/<device>/type
> +Date:          October 2020
> +Contact:       Hans de Goede <hdegoede@redhat.com>
> +Description:
> +               Performance-profiles may be system-wide, or for a specific
> +               device (e.g. CPU / GPU). System-wide profiles are 
typically
> +               used on devices where where a single cooling solution is
> +               shared between all components, such as laptops and NUCs.
> +
> +               Reading this file indicates the type of the device for 
which
> +               the thermal-profile is being configured.
> +
> +               Valid values: "system"
> +               Reserved for future use values: "cpu", "gpu"
> --
> 2.28.0

This looks good to me, the only consideration I have is that in my opinion the
quiet profile and the cool profile should not necessarily match the low-power
state because the quiet profile could cause thermal throttling without
benefiting consumption, instead the cool profile (with the fans almost
always on) would lead to an unnecessary increase in noise.

another question is the notebooks that offer both quiet and cool profile,
which profile should be associated as low power?

wouldn't it be better not to associate any of the 2 profiles with low-power
status and eventually expose them through another API maybe setting to
"custom" the current_profile value as you proposed for GPU knobs?

otherwise it seems to me an excellent solution

Elia



^ permalink raw reply	[flat|nested] 26+ messages in thread

* RE: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-03 13:19 ` [RFC] " Hans de Goede
  2020-10-04  1:33   ` [External] " Mark Pearson
  2020-10-04 22:29   ` Elia Devito
@ 2020-10-05 12:58   ` Limonciello, Mario
  2020-10-05 14:19     ` Barnabás Pőcze
                       ` (2 more replies)
  2020-10-05 13:13   ` Benjamin Berg
  3 siblings, 3 replies; 26+ messages in thread
From: Limonciello, Mario @ 2020-10-05 12:58 UTC (permalink / raw)
  To: Hans de Goede, Darren Hart, Andy Shevchenko, Mark Gross
  Cc: Mark Pearson, Elia Devito, Bastien Nocera, Benjamin Berg,
	linux-pm, linux-acpi, platform-driver-x86, linux-kernel,
	Mark Pearson

> On modern systems CPU/GPU/... performance is often dynamically configurable
> in the form of e.g. variable clock-speeds and TPD. The performance is often
> automatically adjusted to the load by some automatic-mechanism (which may
> very well live outside the kernel).
> 
> These auto performance-adjustment mechanisms often can be configured with
> one of several performance-profiles, with either a bias towards low-power
> consumption (and cool and quiet) or towards performance (and higher power
> consumption and thermals).
> 
> Introduce a new performance_profile class/sysfs API which offers a generic
> API for selecting the performance-profile of these automatic-mechanisms.
> 

If introducing an API for this - let me ask the question, why even let each
driver offer a class interface and userspace need to change "each" driver's
performance setting?

I would think that you could just offer something kernel-wide like
/sys/power/performance-profile

Userspace can read and write to a single file.  All drivers can get notified
on this sysfs file changing.

The systems that react in firmware (such as the two that prompted
this discussion) can change at that time.  It leaves the possibility for a
more open kernel implementation that can do the same thing though too by
directly modifying device registers instead of ACPI devices.

> Cc: Mark Pearson <markpearson@lenovo.com>
> Cc: Elia Devito <eliadevito@gmail.com>
> Cc: Bastien Nocera <hadess@hadess.net>
> Cc: Benjamin Berg <bberg@redhat.com>
> Cc: linux-pm@vger.kernel.org
> Cc: linux-acpi@vger.kernel.org
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  .../testing/sysfs-class-performance_profile   | 104 ++++++++++++++++++
>  1 file changed, 104 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-performance_profile
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-performance_profile
> b/Documentation/ABI/testing/sysfs-class-performance_profile
> new file mode 100644
> index 000000000000..9c67cae39600
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-performance_profile
> @@ -0,0 +1,104 @@
> +Performance-profile selection (e.g.
> /sys/class/performance_profile/thinkpad_acpi/)
> +
> +On modern systems CPU/GPU/... performance is often dynamically configurable
> +in the form of e.g. variable clock-speeds and TPD. The performance is often
> +automatically adjusted to the load by some automatic-mechanism (which may
> +very well live outside the kernel).

Are you intending to word this specifically to cover both firmware and userspace
implementations?  Or were you really meaning firmware implementations?

> +
> +These auto performance-adjustment mechanisms often can be configured with
> +one of several performance-profiles, with either a bias towards low-power
> +consumption (and cool and quiet) or towards performance (and higher power
> +consumption and thermals).
> +
> +The purpose of the performance_profile class is to offer a generic sysfs
> +API for selecting the performance-profile of these automatic-mechanisms.
> +
> +Note that this API is only for selecting the performance-profile, it is
> +NOT a goal of this API to allow monitoring the resulting performance
> +characteristics. Monitoring performance is best done with device/vendor
> +specific tools such as e.g. turbostat.

Another thought that comes to mind (which is completely separate from my previous
idea):

Why not make this register to firmware-attributes class as being discussed in the
new Dell driver?

It seems like it could easily be read as:
/sys/class/firmware-attributes/thinkpad-foo/attributes/PerformanceProfile/current_value
/sys/class/firmware-attributes/thinkpad-foo/attributes/PerformanceProfile/possible_values


> +
> +Specifically when selecting a high-performance profile the actual achieved
> +performance may be limited by various factors such as: the heat generated by
> +other components, room temperature, free air flow at the bottom of a laptop,
> +etc. It is explicitly NOT a goal of this API to let userspace know about
> +any sub-optimal conditions which are impeding reaching the requested
> +performance level.
> +
> +Since numbers are a rather meaningless way to describe performance-profiles
> +this API uses strings to describe the various profiles. To make sure that
> +userspace gets a consistent experience when using this API this API document
> +defines a fixed set of profile-names. Drivers *must* map their internal
> +profile representation/names onto this fixed set.
> +
> +If for some reason there is no good match when mapping then a new profile-name
> +may be added. Drivers which wish to introduce new profile-names must:
> +1. Have very good reasons to do so.
> +2. Add the new profile-name to this document, so that future drivers which also
> +   have a similar problem can use the same new. Usually new profile-names will
> +   be added to the "extra profile-names" section of this document. But in some
> +   cases the set of standard profile-names may be extended.
> +
> +What:		/sys/class/performance_profile/<device>/available_profiles
> +Date:		October 2020
> +Contact:	Hans de Goede <hdegoede@redhat.com>
> +Description:
> +		Reading this file gives a space separated list of profiles
> +		supported for this device.
> +
> +		Drivers must use the following standard profile-names whenever
> +		possible:
> +
> +		low-power:		Emphasises low power consumption
> +					(and also cool and quiet)
> +		balanced-low-power:	Balances between low power consumption
> +					and performance with a slight bias
> +					towards low power
> +		balanced:		Balance between low power consumption
> +					and performance
> +		balanced-performance:	Balances between performance and low
> +					power consumption with a slight bias
> +					towards performance
> +		performance:		Emphasises performance (and may lead to
> +					higher temperatures and fan speeds)
> +
> +		Userspace may expect drivers to offer at least several of these
> +		standard profile-names! If none of the above are a good match
> +		for some of the drivers profiles, then drivers may use one of
> +		these extra profile-names:
> +		<reserved for future use>
> +
> +What:		/sys/class/performance_profile/<device>/current_profile
> +Date:		October 2020
> +Contact:	Hans de Goede <hdegoede@redhat.com>
> +Description:
> +		Reading this file gives the current selected profile for this
> +		device. Writing this file with one of the strings from
> +		available_profiles changes the profile to the new value.
> +
> +		Reading this file may also return "custom". This is intended for
> +		drivers which have and export multiple knobs influencing
> +		performance. Such drivers may very well still want to offer a
> +		set of profiles for easy of use and to be able to offer a
> +		consistent standard API (this API) to userspace for configuring
> +		their performance. The "custom" value is intended for when a
> +		user has directly configured the knobs (through e.g. some
> +		advanced control-panel for a GPU) and the knob values do not
> +		match any of the presets represented by the
> +		performance-profiles. In this case writing this file will
> +		override the modifications and restore the selected presets.
> +
> +What:		/sys/class/performance_profile/<device>/type
> +Date:		October 2020
> +Contact:	Hans de Goede <hdegoede@redhat.com>
> +Description:
> +		Performance-profiles may be system-wide, or for a specific
> +		device (e.g. CPU / GPU). System-wide profiles are typically
> +		used on devices where where a single cooling solution is
> +		shared between all components, such as laptops and NUCs.
> +
> +		Reading this file indicates the type of the device for which
> +		the thermal-profile is being configured.
> +
> +		Valid values: "system"
> +		Reserved for future use values: "cpu", "gpu"
> --
> 2.28.0


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-03 13:19 ` [RFC] " Hans de Goede
                     ` (2 preceding siblings ...)
  2020-10-05 12:58   ` Limonciello, Mario
@ 2020-10-05 13:13   ` Benjamin Berg
  2020-10-09 11:15     ` Hans de Goede
  3 siblings, 1 reply; 26+ messages in thread
From: Benjamin Berg @ 2020-10-05 13:13 UTC (permalink / raw)
  To: Hans de Goede, Darren Hart, Andy Shevchenko, Mark Gross
  Cc: Mario Limonciello, Mark Pearson, Elia Devito, Bastien Nocera,
	linux-pm, linux-acpi, platform-driver-x86, linux-kernel,
	Mark Pearson

[-- Attachment #1: Type: text/plain, Size: 6998 bytes --]

Hi,

seems reasonable to me. Quite simple, but likely good enough as we are
sticking to only use well known names.

Just found a small typo.

Benjamin

On Sat, 2020-10-03 at 15:19 +0200, Hans de Goede wrote:
> On modern systems CPU/GPU/... performance is often dynamically configurable
> in the form of e.g. variable clock-speeds and TPD. The performance is often
> automatically adjusted to the load by some automatic-mechanism (which may
> very well live outside the kernel).
> 
> These auto performance-adjustment mechanisms often can be configured with
> one of several performance-profiles, with either a bias towards low-power
> consumption (and cool and quiet) or towards performance (and higher power
> consumption and thermals).
> 
> Introduce a new performance_profile class/sysfs API which offers a generic
> API for selecting the performance-profile of these automatic-mechanisms.
> 
> Cc: Mark Pearson <markpearson@lenovo.com>
> Cc: Elia Devito <eliadevito@gmail.com>
> Cc: Bastien Nocera <hadess@hadess.net>
> Cc: Benjamin Berg <bberg@redhat.com>
> Cc: linux-pm@vger.kernel.org
> Cc: linux-acpi@vger.kernel.org
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  .../testing/sysfs-class-performance_profile   | 104 ++++++++++++++++++
>  1 file changed, 104 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-performance_profile
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-performance_profile b/Documentation/ABI/testing/sysfs-class-performance_profile
> new file mode 100644
> index 000000000000..9c67cae39600
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-performance_profile
> @@ -0,0 +1,104 @@
> +Performance-profile selection (e.g. /sys/class/performance_profile/thinkpad_acpi/)
> +
> +On modern systems CPU/GPU/... performance is often dynamically configurable
> +in the form of e.g. variable clock-speeds and TPD. The performance is often
> +automatically adjusted to the load by some automatic-mechanism (which may
> +very well live outside the kernel).
> +
> +These auto performance-adjustment mechanisms often can be configured with
> +one of several performance-profiles, with either a bias towards low-power
> +consumption (and cool and quiet) or towards performance (and higher power
> +consumption and thermals).
> +
> +The purpose of the performance_profile class is to offer a generic sysfs
> +API for selecting the performance-profile of these automatic-mechanisms.
> +
> +Note that this API is only for selecting the performance-profile, it is
> +NOT a goal of this API to allow monitoring the resulting performance
> +characteristics. Monitoring performance is best done with device/vendor
> +specific tools such as e.g. turbostat.
> +
> +Specifically when selecting a high-performance profile the actual achieved
> +performance may be limited by various factors such as: the heat generated by
> +other components, room temperature, free air flow at the bottom of a laptop,
> +etc. It is explicitly NOT a goal of this API to let userspace know about
> +any sub-optimal conditions which are impeding reaching the requested
> +performance level.
> +
> +Since numbers are a rather meaningless way to describe performance-profiles
> +this API uses strings to describe the various profiles. To make sure that
> +userspace gets a consistent experience when using this API this API document
> +defines a fixed set of profile-names. Drivers *must* map their internal
> +profile representation/names onto this fixed set.
> +
> +If for some reason there is no good match when mapping then a new profile-name
> +may be added. Drivers which wish to introduce new profile-names must:
> +1. Have very good reasons to do so.
> +2. Add the new profile-name to this document, so that future drivers which also
> +   have a similar problem can use the same new. Usually new profile-names will

Typo, "new" -> "name" I suppose.

> +   be added to the "extra profile-names" section of this document. But in some
> +   cases the set of standard profile-names may be extended.
> +
> +What:		/sys/class/performance_profile/<device>/available_profiles
> +Date:		October 2020
> +Contact:	Hans de Goede <hdegoede@redhat.com>
> +Description:
> +		Reading this file gives a space separated list of profiles
> +		supported for this device.
> +
> +		Drivers must use the following standard profile-names whenever
> +		possible:
> +
> +		low-power:		Emphasises low power consumption
> +					(and also cool and quiet)
> +		balanced-low-power:	Balances between low power consumption
> +					and performance with a slight bias
> +					towards low power
> +		balanced:		Balance between low power consumption
> +					and performance
> +		balanced-performance:	Balances between performance and low
> +					power consumption with a slight bias
> +					towards performance
> +		performance:		Emphasises performance (and may lead to
> +					higher temperatures and fan speeds)
> +
> +		Userspace may expect drivers to offer at least several of these
> +		standard profile-names! If none of the above are a good match
> +		for some of the drivers profiles, then drivers may use one of
> +		these extra profile-names:
> +		<reserved for future use>
> +
> +What:		/sys/class/performance_profile/<device>/current_profile
> +Date:		October 2020
> +Contact:	Hans de Goede <hdegoede@redhat.com>
> +Description:
> +		Reading this file gives the current selected profile for this
> +		device. Writing this file with one of the strings from
> +		available_profiles changes the profile to the new value.
> +
> +		Reading this file may also return "custom". This is intended for
> +		drivers which have and export multiple knobs influencing
> +		performance. Such drivers may very well still want to offer a
> +		set of profiles for easy of use and to be able to offer a
> +		consistent standard API (this API) to userspace for configuring
> +		their performance. The "custom" value is intended for when a
> +		user has directly configured the knobs (through e.g. some
> +		advanced control-panel for a GPU) and the knob values do not
> +		match any of the presets represented by the
> +		performance-profiles. In this case writing this file will
> +		override the modifications and restore the selected presets.
> +
> +What:		/sys/class/performance_profile/<device>/type
> +Date:		October 2020
> +Contact:	Hans de Goede <hdegoede@redhat.com>
> +Description:
> +		Performance-profiles may be system-wide, or for a specific
> +		device (e.g. CPU / GPU). System-wide profiles are typically
> +		used on devices where where a single cooling solution is
> +		shared between all components, such as laptops and NUCs.
> +
> +		Reading this file indicates the type of the device for which
> +		the thermal-profile is being configured.
> +
> +		Valid values: "system"
> +		Reserved for future use values: "cpu", "gpu"

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* RE: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-05 12:58   ` Limonciello, Mario
@ 2020-10-05 14:19     ` Barnabás Pőcze
  2020-10-05 16:11       ` Limonciello, Mario
  2020-10-07 11:51     ` Bastien Nocera
  2020-10-09 11:33     ` Hans de Goede
  2 siblings, 1 reply; 26+ messages in thread
From: Barnabás Pőcze @ 2020-10-05 14:19 UTC (permalink / raw)
  To: Limonciello, Mario
  Cc: Hans de Goede, Darren Hart, Andy Shevchenko, Mark Gross,
	Mark Pearson, Elia Devito, Bastien Nocera, Benjamin Berg,
	linux-pm, linux-acpi, platform-driver-x86, linux-kernel,
	Mark Pearson

Hi


2020. október 5., hétfő 14:58 keltezéssel, Limonciello, Mario írta:
> > On modern systems CPU/GPU/... performance is often dynamically configurable
> > in the form of e.g. variable clock-speeds and TPD. The performance is often
> > automatically adjusted to the load by some automatic-mechanism (which may
> > very well live outside the kernel).
> > These auto performance-adjustment mechanisms often can be configured with
> > one of several performance-profiles, with either a bias towards low-power
> > consumption (and cool and quiet) or towards performance (and higher power
> > consumption and thermals).
> > Introduce a new performance_profile class/sysfs API which offers a generic
> > API for selecting the performance-profile of these automatic-mechanisms.
>
> If introducing an API for this - let me ask the question, why even let each
> driver offer a class interface and userspace need to change "each" driver's
> performance setting?
>
> I would think that you could just offer something kernel-wide like
> /sys/power/performance-profile
>
> Userspace can read and write to a single file. All drivers can get notified
> on this sysfs file changing.
>

That makes sense, in my opinion, from the regular user's perspective:
one switch to rule them all, no fuss. However, I don't think that scales well.
What if the hypothetical users wants to run a CPU-heavy workload, and thus wants
to put the GPU into "low-power" mode and the CPU into "performance" mode? What if
the users wants to put one GPU into "low-power" mode, but the other one into
"performance"? With the current specification, the user's needs could be easily
satisfied. I don't see how that's possible with a single switch. Nonetheless, I think
that a single global switch *in addition* to the class devices could possibly
simplify the userspace-kernel interaction for most users.


> The systems that react in firmware (such as the two that prompted
> this discussion) can change at that time. It leaves the possibility for a
> more open kernel implementation that can do the same thing though too by
> directly modifying device registers instead of ACPI devices.
>

Excuse my ignorance, but I don't really see why this interface would be tied to
ACPI devices? Why is it not possible to write a driver that implements this interface
and directly modifies device registers? Am I missing something obvious here?


> [...]


Thanks,
Barnabás Pőcze

^ permalink raw reply	[flat|nested] 26+ messages in thread

* RE: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-05 14:19     ` Barnabás Pőcze
@ 2020-10-05 16:11       ` Limonciello, Mario
  2020-10-05 16:47         ` [External] " Mark Pearson
  0 siblings, 1 reply; 26+ messages in thread
From: Limonciello, Mario @ 2020-10-05 16:11 UTC (permalink / raw)
  To: Barnabás Pőcze
  Cc: Hans de Goede, Darren Hart, Andy Shevchenko, Mark Gross,
	Mark Pearson, Elia Devito, Bastien Nocera, Benjamin Berg,
	linux-pm, linux-acpi, platform-driver-x86, linux-kernel,
	Mark Pearson

> 2020. október 5., hétfő 14:58 keltezéssel, Limonciello, Mario írta:
> > > On modern systems CPU/GPU/... performance is often dynamically
> configurable
> > > in the form of e.g. variable clock-speeds and TPD. The performance is
> often
> > > automatically adjusted to the load by some automatic-mechanism (which may
> > > very well live outside the kernel).
> > > These auto performance-adjustment mechanisms often can be configured with
> > > one of several performance-profiles, with either a bias towards low-power
> > > consumption (and cool and quiet) or towards performance (and higher power
> > > consumption and thermals).
> > > Introduce a new performance_profile class/sysfs API which offers a generic
> > > API for selecting the performance-profile of these automatic-mechanisms.
> >
> > If introducing an API for this - let me ask the question, why even let each
> > driver offer a class interface and userspace need to change "each" driver's
> > performance setting?
> >
> > I would think that you could just offer something kernel-wide like
> > /sys/power/performance-profile
> >
> > Userspace can read and write to a single file. All drivers can get notified
> > on this sysfs file changing.
> >
> 
> That makes sense, in my opinion, from the regular user's perspective:
> one switch to rule them all, no fuss. However, I don't think that scales well.
> What if the hypothetical users wants to run a CPU-heavy workload, and thus
> wants
> to put the GPU into "low-power" mode and the CPU into "performance" mode? What
> if
> the users wants to put one GPU into "low-power" mode, but the other one into
> "performance"? With the current specification, the user's needs could be
> easily
> satisfied. I don't see how that's possible with a single switch. Nonetheless,
> I think
> that a single global switch *in addition* to the class devices could possibly
> simplify the userspace-kernel interaction for most users.

I think that the moment you represent a platform/system device as a switch you
lose the ability to set different policies for other types of devices separately.

What if using the platform/system device actually /also/ orchestrates changes to
those other devices you mention?  Then it becomes an order of events problem, or
worse a problem where one switch shows "wrong" value.

> 
> 
> > The systems that react in firmware (such as the two that prompted
> > this discussion) can change at that time. It leaves the possibility for a
> > more open kernel implementation that can do the same thing though too by
> > directly modifying device registers instead of ACPI devices.
> >
> 
> Excuse my ignorance, but I don't really see why this interface would be tied
> to
> ACPI devices? Why is it not possible to write a driver that implements this
> interface
> and directly modifies device registers? Am I missing something obvious here?
> 

When implemented for the two vendors mentioned here, it would be using a
proprietary "firmware API" implemented by those two vendors.  For example write
arguments (0x1, 0x2) to ACPI-WMI method WMFT and it will cause firmware to coordinate
using undisclosed protocol to affect the platform changes desirable.

This is different in my mind from "kernel writes to a specific register" to set
power properties of a specific device.




^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [External] RE: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-05 16:11       ` Limonciello, Mario
@ 2020-10-05 16:47         ` Mark Pearson
  2020-10-05 16:56           ` Limonciello, Mario
  0 siblings, 1 reply; 26+ messages in thread
From: Mark Pearson @ 2020-10-05 16:47 UTC (permalink / raw)
  To: Limonciello, Mario, Barnabás Pőcze
  Cc: Hans de Goede, Darren Hart, Andy Shevchenko, Mark Gross,
	Mark Pearson, Elia Devito, Bastien Nocera, Benjamin Berg,
	linux-pm, linux-acpi, platform-driver-x86, linux-kernel



On 2020-10-05 12:11 p.m., Limonciello, Mario wrote:
>>
>> Excuse my ignorance, but I don't really see why this interface would be tied
>> to
>> ACPI devices? Why is it not possible to write a driver that implements this
>> interface
>> and directly modifies device registers? Am I missing something obvious here?
>>
> 
> When implemented for the two vendors mentioned here, it would be using a
> proprietary "firmware API" implemented by those two vendors.  For example write
> arguments (0x1, 0x2) to ACPI-WMI method WMFT and it will cause firmware to coordinate
> using undisclosed protocol to affect the platform changes desirable.
> 
> This is different in my mind from "kernel writes to a specific register" to set
> power properties of a specific device.
> 

Just curious on this point - isn't that (mostly) what all hardware does? 
You write to it and the device does "stuff" to achieve the required 
effect. Yes this is in proprietary firmware, but from my experience with 
hardware devices that's not uncommon these days anyway.

Let me know if I'm misunderstanding something here. I couldn't see the 
difference between a register written to via ACPI and one written to via 
some other protocol (SMBUS? or whatever)

Mark



^ permalink raw reply	[flat|nested] 26+ messages in thread

* RE: [External] RE: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-05 16:47         ` [External] " Mark Pearson
@ 2020-10-05 16:56           ` Limonciello, Mario
  2020-10-05 17:46             ` Mark Pearson
  0 siblings, 1 reply; 26+ messages in thread
From: Limonciello, Mario @ 2020-10-05 16:56 UTC (permalink / raw)
  To: Mark Pearson, Barnabás Pőcze
  Cc: Hans de Goede, Darren Hart, Andy Shevchenko, Mark Gross,
	Mark Pearson, Elia Devito, Bastien Nocera, Benjamin Berg,
	linux-pm, linux-acpi, platform-driver-x86, linux-kernel

> On 2020-10-05 12:11 p.m., Limonciello, Mario wrote:
> >>
> >> Excuse my ignorance, but I don't really see why this interface would be
> tied
> >> to
> >> ACPI devices? Why is it not possible to write a driver that implements this
> >> interface
> >> and directly modifies device registers? Am I missing something obvious
> here?
> >>
> >
> > When implemented for the two vendors mentioned here, it would be using a
> > proprietary "firmware API" implemented by those two vendors.  For example
> write
> > arguments (0x1, 0x2) to ACPI-WMI method WMFT and it will cause firmware to
> coordinate
> > using undisclosed protocol to affect the platform changes desirable.
> >
> > This is different in my mind from "kernel writes to a specific register" to
> set
> > power properties of a specific device.
> >
> 
> Just curious on this point - isn't that (mostly) what all hardware does?
> You write to it and the device does "stuff" to achieve the required
> effect. Yes this is in proprietary firmware, but from my experience with
> hardware devices that's not uncommon these days anyway.
> 

Yes I agree.  Even "register" writes to a device are actually an API and
something in the hardware monitors those registers and does something as a
result.

> Let me know if I'm misunderstanding something here. I couldn't see the
> difference between a register written to via ACPI and one written to via
> some other protocol (SMBUS? or whatever)
> 
> Mark
> 

The reason I'm calling out a distinction here is that "platform" and "device"
can cover a lot more things.  In this case it's an API provided by the platform's
firmware, not an individual device's firmware.  So you can't actually guarantee
what the platform's firmware did.  It could have sent any number of sideband
commands to devices that it controls.  The "platform" could have potentially
told the GPU to turn up its fans, or lower it's clock as a result of this, but you
can't possibly know.

However if we go the GPU example alone, it's a specific single device you're
controlling.  You put the GPU into the characterization that you expected and it
operates that way.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [External] RE: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-05 16:56           ` Limonciello, Mario
@ 2020-10-05 17:46             ` Mark Pearson
  0 siblings, 0 replies; 26+ messages in thread
From: Mark Pearson @ 2020-10-05 17:46 UTC (permalink / raw)
  To: Limonciello, Mario, Barnabás Pőcze
  Cc: Hans de Goede, Darren Hart, Andy Shevchenko, Mark Gross,
	Mark Pearson, Elia Devito, Bastien Nocera, Benjamin Berg,
	linux-pm, linux-acpi, platform-driver-x86, linux-kernel



On 2020-10-05 12:56 p.m., Limonciello, Mario wrote:
>>>
>>> When implemented for the two vendors mentioned here, it would be using a
>>> proprietary "firmware API" implemented by those two vendors.  For example
>> write
>>> arguments (0x1, 0x2) to ACPI-WMI method WMFT and it will cause firmware to
>> coordinate
>>> using undisclosed protocol to affect the platform changes desirable.
>>>
>>> This is different in my mind from "kernel writes to a specific register" to
>> set
>>> power properties of a specific device.
>>>
>>
>> Just curious on this point - isn't that (mostly) what all hardware does?
>> You write to it and the device does "stuff" to achieve the required
>> effect. Yes this is in proprietary firmware, but from my experience with
>> hardware devices that's not uncommon these days anyway.
>>
> 
> Yes I agree.  Even "register" writes to a device are actually an API and
> something in the hardware monitors those registers and does something as a
> result.
> 
>> Let me know if I'm misunderstanding something here. I couldn't see the
>> difference between a register written to via ACPI and one written to via
>> some other protocol (SMBUS? or whatever)
>>
>> Mark
>>
> 
> The reason I'm calling out a distinction here is that "platform" and "device"
> can cover a lot more things.  In this case it's an API provided by the platform's
> firmware, not an individual device's firmware.  So you can't actually guarantee
> what the platform's firmware did.  It could have sent any number of sideband
> commands to devices that it controls.  The "platform" could have potentially
> told the GPU to turn up its fans, or lower it's clock as a result of this, but you
> can't possibly know.
> 
> However if we go the GPU example alone, it's a specific single device you're
> controlling.  You put the GPU into the characterization that you expected and it
> operates that way.
> 
Got it - fair enough :) Thanks for the explain.
Thanks
Mark

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-05 12:58   ` Limonciello, Mario
  2020-10-05 14:19     ` Barnabás Pőcze
@ 2020-10-07 11:51     ` Bastien Nocera
  2020-10-07 15:58       ` Limonciello, Mario
  2020-10-09 11:33     ` Hans de Goede
  2 siblings, 1 reply; 26+ messages in thread
From: Bastien Nocera @ 2020-10-07 11:51 UTC (permalink / raw)
  To: Limonciello, Mario, Hans de Goede, Darren Hart, Andy Shevchenko,
	Mark Gross
  Cc: Mark Pearson, Elia Devito, Benjamin Berg, linux-pm, linux-acpi,
	platform-driver-x86, linux-kernel, Mark Pearson

On Mon, 2020-10-05 at 12:58 +0000, Limonciello, Mario wrote:
> > On modern systems CPU/GPU/... performance is often dynamically
> > configurable
> > in the form of e.g. variable clock-speeds and TPD. The performance
> > is often
> > automatically adjusted to the load by some automatic-mechanism
> > (which may
> > very well live outside the kernel).
> > 
> > These auto performance-adjustment mechanisms often can be
> > configured with
> > one of several performance-profiles, with either a bias towards
> > low-power
> > consumption (and cool and quiet) or towards performance (and higher
> > power
> > consumption and thermals).
> > 
> > Introduce a new performance_profile class/sysfs API which offers a
> > generic
> > API for selecting the performance-profile of these automatic-
> > mechanisms.
> > 
> 
> If introducing an API for this - let me ask the question, why even let each
> driver offer a class interface and userspace need to change "each" driver's
> performance setting?
> 
> I would think that you could just offer something kernel-wide like
> /sys/power/performance-profile
> 
> Userspace can read and write to a single file.  All drivers can get notified
> on this sysfs file changing.
> 
> The systems that react in firmware (such as the two that prompted
> this discussion) can change at that time.  It leaves the possibility for a
> more open kernel implementation that can do the same thing though too by
> directly modifying device registers instead of ACPI devices.

The problem, as I've mentioned in previous discussions we had about
this, is that, as you've seen in replies to this mail, this would
suddenly be making the kernel apply policy.

There's going to be pushback as soon as policy is enacted in the
kernel, and you take away the different knobs for individual components
(or you can control them centrally as well as individually). As much as
I hate the quantity of knobs[1], I don't think that trying to reduce
the number of knobs in the kernel is a good use of our time, and easier
to enact, coordinated with design targets, in user-space.

Unless you can think of a way to implement this kernel wide setting
without adding one more exponent on the number of possibilities for the
testing matrix, I'll +1 Hans' original API.

Cheers

[1]: https://ometer.com/preferences.html


^ permalink raw reply	[flat|nested] 26+ messages in thread

* RE: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-07 11:51     ` Bastien Nocera
@ 2020-10-07 15:58       ` Limonciello, Mario
  2020-10-07 16:34         ` Bastien Nocera
  0 siblings, 1 reply; 26+ messages in thread
From: Limonciello, Mario @ 2020-10-07 15:58 UTC (permalink / raw)
  To: Bastien Nocera, Hans de Goede, Darren Hart, Andy Shevchenko, Mark Gross
  Cc: Mark Pearson, Elia Devito, Benjamin Berg, linux-pm, linux-acpi,
	platform-driver-x86, linux-kernel, Mark Pearson

 
> On Mon, 2020-10-05 at 12:58 +0000, Limonciello, Mario wrote:
> > > On modern systems CPU/GPU/... performance is often dynamically
> > > configurable
> > > in the form of e.g. variable clock-speeds and TPD. The performance
> > > is often
> > > automatically adjusted to the load by some automatic-mechanism
> > > (which may
> > > very well live outside the kernel).
> > >
> > > These auto performance-adjustment mechanisms often can be
> > > configured with
> > > one of several performance-profiles, with either a bias towards
> > > low-power
> > > consumption (and cool and quiet) or towards performance (and higher
> > > power
> > > consumption and thermals).
> > >
> > > Introduce a new performance_profile class/sysfs API which offers a
> > > generic
> > > API for selecting the performance-profile of these automatic-
> > > mechanisms.
> > >
> >
> > If introducing an API for this - let me ask the question, why even let each
> > driver offer a class interface and userspace need to change "each" driver's
> > performance setting?
> >
> > I would think that you could just offer something kernel-wide like
> > /sys/power/performance-profile
> >
> > Userspace can read and write to a single file.  All drivers can get notified
> > on this sysfs file changing.
> >
> > The systems that react in firmware (such as the two that prompted
> > this discussion) can change at that time.  It leaves the possibility for a
> > more open kernel implementation that can do the same thing though too by
> > directly modifying device registers instead of ACPI devices.
> 
> The problem, as I've mentioned in previous discussions we had about
> this, is that, as you've seen in replies to this mail, this would
> suddenly be making the kernel apply policy.
> 
> There's going to be pushback as soon as policy is enacted in the
> kernel, and you take away the different knobs for individual components
> (or you can control them centrally as well as individually). As much as
> I hate the quantity of knobs[1], I don't think that trying to reduce
> the number of knobs in the kernel is a good use of our time, and easier
> to enact, coordinated with design targets, in user-space.
> 
> Unless you can think of a way to implement this kernel wide setting
> without adding one more exponent on the number of possibilities for the
> testing matrix, I'll +1 Hans' original API.
> 
Actually I offered two proposals in my reply.  So are you NAKing both?
The other one suggested to use the same firmware attributes class being
introduced by the new Dell driver (https://patchwork.kernel.org/patch/11818343/)
since this is actually a knob to a specific firmware setting.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-07 15:58       ` Limonciello, Mario
@ 2020-10-07 16:34         ` Bastien Nocera
  2020-10-07 18:41           ` Limonciello, Mario
  0 siblings, 1 reply; 26+ messages in thread
From: Bastien Nocera @ 2020-10-07 16:34 UTC (permalink / raw)
  To: Limonciello, Mario, Hans de Goede, Darren Hart, Andy Shevchenko,
	Mark Gross
  Cc: Mark Pearson, Elia Devito, Benjamin Berg, linux-pm, linux-acpi,
	platform-driver-x86, linux-kernel, Mark Pearson

On Wed, 2020-10-07 at 15:58 +0000, Limonciello, Mario wrote:
>  
> > On Mon, 2020-10-05 at 12:58 +0000, Limonciello, Mario wrote:
> > > > On modern systems CPU/GPU/... performance is often dynamically
> > > > configurable
> > > > in the form of e.g. variable clock-speeds and TPD. The
> > > > performance
> > > > is often
> > > > automatically adjusted to the load by some automatic-mechanism
> > > > (which may
> > > > very well live outside the kernel).
> > > > 
> > > > These auto performance-adjustment mechanisms often can be
> > > > configured with
> > > > one of several performance-profiles, with either a bias towards
> > > > low-power
> > > > consumption (and cool and quiet) or towards performance (and
> > > > higher
> > > > power
> > > > consumption and thermals).
> > > > 
> > > > Introduce a new performance_profile class/sysfs API which
> > > > offers a
> > > > generic
> > > > API for selecting the performance-profile of these automatic-
> > > > mechanisms.
> > > > 
> > > 
> > > If introducing an API for this - let me ask the question, why
> > > even let each
> > > driver offer a class interface and userspace need to change
> > > "each" driver's
> > > performance setting?
> > > 
> > > I would think that you could just offer something kernel-wide
> > > like
> > > /sys/power/performance-profile
> > > 
> > > Userspace can read and write to a single file.  All drivers can
> > > get notified
> > > on this sysfs file changing.
> > > 
> > > The systems that react in firmware (such as the two that prompted
> > > this discussion) can change at that time.  It leaves the
> > > possibility for a
> > > more open kernel implementation that can do the same thing though
> > > too by
> > > directly modifying device registers instead of ACPI devices.
> > 
> > The problem, as I've mentioned in previous discussions we had about
> > this, is that, as you've seen in replies to this mail, this would
> > suddenly be making the kernel apply policy.
> > 
> > There's going to be pushback as soon as policy is enacted in the
> > kernel, and you take away the different knobs for individual
> > components
> > (or you can control them centrally as well as individually). As
> > much as
> > I hate the quantity of knobs[1], I don't think that trying to
> > reduce
> > the number of knobs in the kernel is a good use of our time, and
> > easier
> > to enact, coordinated with design targets, in user-space.
> > 
> > Unless you can think of a way to implement this kernel wide setting
> > without adding one more exponent on the number of possibilities for
> > the
> > testing matrix, I'll +1 Hans' original API.
> > 
> Actually I offered two proposals in my reply.  So are you NAKing
> both?

No, this is only about the first portion of the email, which I quoted.
And I'm not NAK'ing it, but I don't see how it can work without being
antithetical to what kernel "users" expect, or what the folks consuming
those interfaces (presumably us both) would expect to be able to test
and maintain.

> The other one suggested to use the same firmware attributes class
> being
> introduced by the new Dell driver ( 
> https://patchwork.kernel.org/patch/11818343/)
> since this is actually a knob to a specific firmware setting.

This seemed to me like an implementation detail (eg. the same metadata
is being exported, but in a different way), and I don't feel strongly
about it either way.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* RE: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-07 16:34         ` Bastien Nocera
@ 2020-10-07 18:41           ` Limonciello, Mario
  2020-10-12 16:42             ` Rafael J. Wysocki
  0 siblings, 1 reply; 26+ messages in thread
From: Limonciello, Mario @ 2020-10-07 18:41 UTC (permalink / raw)
  To: Bastien Nocera, Hans de Goede, Darren Hart, Andy Shevchenko, Mark Gross
  Cc: Mark Pearson, Elia Devito, Benjamin Berg, linux-pm, linux-acpi,
	platform-driver-x86, linux-kernel, Mark Pearson

> On Wed, 2020-10-07 at 15:58 +0000, Limonciello, Mario wrote:
> >
> > > On Mon, 2020-10-05 at 12:58 +0000, Limonciello, Mario wrote:
> > > > > On modern systems CPU/GPU/... performance is often dynamically
> > > > > configurable
> > > > > in the form of e.g. variable clock-speeds and TPD. The
> > > > > performance
> > > > > is often
> > > > > automatically adjusted to the load by some automatic-mechanism
> > > > > (which may
> > > > > very well live outside the kernel).
> > > > >
> > > > > These auto performance-adjustment mechanisms often can be
> > > > > configured with
> > > > > one of several performance-profiles, with either a bias towards
> > > > > low-power
> > > > > consumption (and cool and quiet) or towards performance (and
> > > > > higher
> > > > > power
> > > > > consumption and thermals).
> > > > >
> > > > > Introduce a new performance_profile class/sysfs API which
> > > > > offers a
> > > > > generic
> > > > > API for selecting the performance-profile of these automatic-
> > > > > mechanisms.
> > > > >
> > > >
> > > > If introducing an API for this - let me ask the question, why
> > > > even let each
> > > > driver offer a class interface and userspace need to change
> > > > "each" driver's
> > > > performance setting?
> > > >
> > > > I would think that you could just offer something kernel-wide
> > > > like
> > > > /sys/power/performance-profile
> > > >
> > > > Userspace can read and write to a single file.  All drivers can
> > > > get notified
> > > > on this sysfs file changing.
> > > >
> > > > The systems that react in firmware (such as the two that prompted
> > > > this discussion) can change at that time.  It leaves the
> > > > possibility for a
> > > > more open kernel implementation that can do the same thing though
> > > > too by
> > > > directly modifying device registers instead of ACPI devices.
> > >
> > > The problem, as I've mentioned in previous discussions we had about
> > > this, is that, as you've seen in replies to this mail, this would
> > > suddenly be making the kernel apply policy.
> > >
> > > There's going to be pushback as soon as policy is enacted in the
> > > kernel, and you take away the different knobs for individual
> > > components
> > > (or you can control them centrally as well as individually). As
> > > much as
> > > I hate the quantity of knobs[1], I don't think that trying to
> > > reduce
> > > the number of knobs in the kernel is a good use of our time, and
> > > easier
> > > to enact, coordinated with design targets, in user-space.
> > >
> > > Unless you can think of a way to implement this kernel wide setting
> > > without adding one more exponent on the number of possibilities for
> > > the
> > > testing matrix, I'll +1 Hans' original API.
> > >
> > Actually I offered two proposals in my reply.  So are you NAKing
> > both?
> 
> No, this is only about the first portion of the email, which I quoted.
> And I'm not NAK'ing it, but I don't see how it can work without being
> antithetical to what kernel "users" expect, or what the folks consuming
> those interfaces (presumably us both) would expect to be able to test
> and maintain.
> 

(Just so others are aware, Bastien and I had a previous discussion on this topic
that he alluded to here: https://gitlab.freedesktop.org/hadess/power-profiles-daemon/-/issues/1)

In general I agree that we shouldn't be offering 100's of knobs to change
things and protect users from themselves where possible.

Whether the decisions are made in the kernel or in userspace you still have a matrix once
you're letting someone change 2 different kernel devices that offer policy.  I'd argue it's
actually worse if you let userspace change it though.

Let's go back to the my GPU and platform example and lets say both offer the new knob here
for both.  Userspace software such as your PPD picks performance.  Both the platform device
and GPU device get changed, hopefully no conflicts.
Then user decides no, I don't want my GPU in performance mode, I only want my platform.
So they change the knob for the GPU manually, and now you have a new config in your matrix.

However if you left it to a single kernel knob, both GPU and platform get moved together and
you don't have these extra configs in your matrix anymore.

The other point I mentioned, that platform might also do something to GPU via a sideband and
you race, you can solve it with kernel too by modifying the ordering the kernel handles it.

Userspace however, you give two knobs and now you have to worry about them getting it right
and supporting them doing them in the wrong order.

> > The other one suggested to use the same firmware attributes class
> > being
> > introduced by the new Dell driver (
> > https://patchwork.kernel.org/patch/11818343/)
> > since this is actually a knob to a specific firmware setting.
> 
> This seemed to me like an implementation detail (eg. the same metadata
> is being exported, but in a different way), and I don't feel strongly
> about it either way.

OK thanks.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-04 22:29   ` Elia Devito
@ 2020-10-09 10:52     ` Hans de Goede
  0 siblings, 0 replies; 26+ messages in thread
From: Hans de Goede @ 2020-10-09 10:52 UTC (permalink / raw)
  To: Elia Devito, Darren Hart, Andy Shevchenko, Mark Gross
  Cc: Mario Limonciello, Mark Pearson, Bastien Nocera, Benjamin Berg,
	linux-pm, linux-acpi, platform-driver-x86, linux-kernel,
	Mark Pearson

Hi,

On 10/5/20 12:29 AM, Elia Devito wrote:
> Hi Hans,
> 
> On 2020-10-03 9:19 a.m., Hans de Goede wrote:
>> On modern systems CPU/GPU/... performance is often dynamically configurable
>> in the form of e.g. variable clock-speeds and TPD. The performance is often
>> automatically adjusted to the load by some automatic-mechanism (which may
>> very well live outside the kernel).
>>
>> These auto performance-adjustment mechanisms often can be configured with
>> one of several performance-profiles, with either a bias towards low-power
>> consumption (and cool and quiet) or towards performance (and higher power
>> consumption and thermals).
>>
>> Introduce a new performance_profile class/sysfs API which offers a generic
>> API for selecting the performance-profile of these automatic-mechanisms.
>>
>> Cc: Mark Pearson <markpearson@lenovo.com>
>> Cc: Elia Devito <eliadevito@gmail.com>
>> Cc: Bastien Nocera <hadess@hadess.net>
>> Cc: Benjamin Berg <bberg@redhat.com>
>> Cc: linux-pm@vger.kernel.org
>> Cc: linux-acpi@vger.kernel.org
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> .../testing/sysfs-class-performance_profile   | 104 ++++++++++++++++++
>> 1 file changed, 104 insertions(+)
>> create mode 100644
>> Documentation/ABI/testing/sysfs-class-performance_profile
>>
>> diff --git a/Documentation/ABI/testing/sysfs-class-performance_profile
>> b/Documentation/ABI/testing/sysfs-class-performance_profile new file mode
>> 100644
>> index 000000000000..9c67cae39600
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-class-performance_profile
>> @@ -0,0 +1,104 @@
>> +Performance-profile selection (e.g.
>> /sys/class/performance_profile/thinkpad_acpi/) +
>> +On modern systems CPU/GPU/... performance is often dynamically configurable
>> +in the form of e.g. variable clock-speeds and TPD. The performance is
>> often +automatically adjusted to the load by some automatic-mechanism
>> (which may +very well live outside the kernel).
>> +
>> +These auto performance-adjustment mechanisms often can be configured with
>> +one of several performance-profiles, with either a bias towards low-power
>> +consumption (and cool and quiet) or towards performance (and higher power
>> +consumption and thermals).
>> +
>> +The purpose of the performance_profile class is to offer a generic sysfs
>> +API for selecting the performance-profile of these automatic-mechanisms.
>> +
>> +Note that this API is only for selecting the performance-profile, it is
>> +NOT a goal of this API to allow monitoring the resulting performance
>> +characteristics. Monitoring performance is best done with device/vendor
>> +specific tools such as e.g. turbostat.
>> +
>> +Specifically when selecting a high-performance profile the actual achieved
>> +performance may be limited by various factors such as: the heat generated
>> by +other components, room temperature, free air flow at the bottom of a
>> laptop, +etc. It is explicitly NOT a goal of this API to let userspace know
>> about +any sub-optimal conditions which are impeding reaching the requested
>> +performance level.
>> +
>> +Since numbers are a rather meaningless way to describe performance-profiles
>> +this API uses strings to describe the various profiles. To make sure that
>> +userspace gets a consistent experience when using this API this API
>> document +defines a fixed set of profile-names. Drivers *must* map their
>> internal +profile representation/names onto this fixed set.
>> +
>> +If for some reason there is no good match when mapping then a new
>> profile-name +may be added. Drivers which wish to introduce new
>> profile-names must: +1. Have very good reasons to do so.
>> +2. Add the new profile-name to this document, so that future drivers which
>> also +   have a similar problem can use the same new. Usually new
>> profile-names will +   be added to the "extra profile-names" section of
>> this document. But in some +   cases the set of standard profile-names may
>> be extended.
>> +
>> +What:          /sys/class/performance_profile/<device>/
> available_profiles
>> +Date:          October 2020
>> +Contact:       Hans de Goede <hdegoede@redhat.com>
>> +Description:
>> +               Reading this file gives a space separated list of profiles
>> +               supported for this device.
>> +
>> +               Drivers must use the following standard profile-names
> whenever
>> +               possible:
>> +
>> +               low-power:              Emphasises low power consumption
>> +                                       (and also cool and
> quiet)
>> +               balanced-low-power:     Balances between low power
> consumption
>> +                                       and performance with a
> slight bias
>> +                                       towards low power
>> +               balanced:               Balance between low power
> consumption
>> +                                       and performance
>> +               balanced-performance:   Balances between performance and
> low
>> +                                       power consumption with
> a slight bias
>> +                                       towards performance
>> +               performance:            Emphasises performance
> (and may lead to
>> +                                       higher temperatures and
> fan speeds)
>> +
>> +               Userspace may expect drivers to offer at least several of
> these
>> +               standard profile-names! If none of the above are a good
> match
>> +               for some of the drivers profiles, then drivers may use
> one of
>> +               these extra profile-names:
>> +               <reserved for future use>
>> +
>> +What:          /sys/class/performance_profile/<device>/current_profile
>> +Date:          October 2020
>> +Contact:       Hans de Goede <hdegoede@redhat.com>
>> +Description:
>> +               Reading this file gives the current selected profile for
> this
>> +               device. Writing this file with one of the strings from
>> +               available_profiles changes the profile to the new value.
>> +
>> +               Reading this file may also return "custom". This is
> intended for
>> +               drivers which have and export multiple knobs influencing
>> +               performance. Such drivers may very well still want to
> offer a
>> +               set of profiles for easy of use and to be able to offer a
>> +               consistent standard API (this API) to userspace for
> configuring
>> +               their performance. The "custom" value is intended for
> when a
>> +               user has directly configured the knobs (through e.g. some
>> +               advanced control-panel for a GPU) and the knob values do
> not
>> +               match any of the presets represented by the
>> +               performance-profiles. In this case writing this file will
>> +               override the modifications and restore the selected
> presets.
>> +
>> +What:          /sys/class/performance_profile/<device>/type
>> +Date:          October 2020
>> +Contact:       Hans de Goede <hdegoede@redhat.com>
>> +Description:
>> +               Performance-profiles may be system-wide, or for a specific
>> +               device (e.g. CPU / GPU). System-wide profiles are
> typically
>> +               used on devices where where a single cooling solution is
>> +               shared between all components, such as laptops and NUCs.
>> +
>> +               Reading this file indicates the type of the device for
> which
>> +               the thermal-profile is being configured.
>> +
>> +               Valid values: "system"
>> +               Reserved for future use values: "cpu", "gpu"
>> --
>> 2.28.0
> 
> This looks good to me, the only consideration I have is that in my opinion the
> quiet profile and the cool profile should not necessarily match the low-power
> state because the quiet profile could cause thermal throttling without
> benefiting consumption, instead the cool profile (with the fans almost
> always on) would lead to an unnecessary increase in noise.

Ah I see, so you are interpreting cool as "cool to touch / low temps" not
as in cool by not generating much heat. That is not an unreasonable interpretation
and I see that the hp-wmi stuff you are working on has separate cool and
quiet profiles (and does not seem to have one which explicit targets low-power).

So I agree that we should not lump these together leading to the following
set of standard profile names (for now, can be extended in the future) :

                 quiet:                  Emphasises quiet running
		cool:                   Emphasises low temperatures
                 low-power:              Emphasises low power consumption
                 balanced-low-power:     Balances between low power consumption
                                         and performance with a slight bias
                                         towards low power
                 balanced:               Balance between low power consumption
                                         and performance
                 balanced-performance:   Balances between performance and low
                                         power consumption with a slight bias
                                         towards performance
                 performance:            Emphasises performance


> another question is the notebooks that offer both quiet and cool profile,
> which profile should be associated as low power?

That is a good question, IMHO the kernels role here is to provide a
mechanism to control these kinda profiles in various systems / components.

If the UI for this takes the form of a slider going from low-power
to performance, then on hardware which only offers cool and quiet
as options, like the hp-wmi interface seems to do, which one to
pick when the user selects low-power is a policy decision left
up to userspace. Likely the performance-profile-daemon Bastien is
working on, so from the sysfs API pov this is solved by just offering
cool and quiet as profile choices and then userspace needs to figure
out what to do (sorry Bastien).

Note if the driver knows that one of cool vs quiet also leads
to low-power, while for the other one that is not so much the case,
then it would make sense for the driver to also offer low-power as
an alias to one of the two (*) since the chances that the driver knows
this are better then that the performance-profile-daemon will know.

Regards,

Hans


*) or maybe in that case offer only low-power + one of the
other 2 options ?  Either way works I guess...


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-05 13:13   ` Benjamin Berg
@ 2020-10-09 11:15     ` Hans de Goede
  0 siblings, 0 replies; 26+ messages in thread
From: Hans de Goede @ 2020-10-09 11:15 UTC (permalink / raw)
  To: Benjamin Berg, Darren Hart, Andy Shevchenko, Mark Gross
  Cc: Mario Limonciello, Mark Pearson, Elia Devito, Bastien Nocera,
	linux-pm, linux-acpi, platform-driver-x86, linux-kernel,
	Mark Pearson

Hi,

On 10/5/20 3:13 PM, Benjamin Berg wrote:
> Hi,
> 
> seems reasonable to me. Quite simple, but likely good enough as we are
> sticking to only use well known names.
> 
> Just found a small typo.
> 
> Benjamin
> 
> On Sat, 2020-10-03 at 15:19 +0200, Hans de Goede wrote:
>> On modern systems CPU/GPU/... performance is often dynamically configurable
>> in the form of e.g. variable clock-speeds and TPD. The performance is often
>> automatically adjusted to the load by some automatic-mechanism (which may
>> very well live outside the kernel).
>>
>> These auto performance-adjustment mechanisms often can be configured with
>> one of several performance-profiles, with either a bias towards low-power
>> consumption (and cool and quiet) or towards performance (and higher power
>> consumption and thermals).
>>
>> Introduce a new performance_profile class/sysfs API which offers a generic
>> API for selecting the performance-profile of these automatic-mechanisms.
>>
>> Cc: Mark Pearson <markpearson@lenovo.com>
>> Cc: Elia Devito <eliadevito@gmail.com>
>> Cc: Bastien Nocera <hadess@hadess.net>
>> Cc: Benjamin Berg <bberg@redhat.com>
>> Cc: linux-pm@vger.kernel.org
>> Cc: linux-acpi@vger.kernel.org
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   .../testing/sysfs-class-performance_profile   | 104 ++++++++++++++++++
>>   1 file changed, 104 insertions(+)
>>   create mode 100644 Documentation/ABI/testing/sysfs-class-performance_profile
>>
>> diff --git a/Documentation/ABI/testing/sysfs-class-performance_profile b/Documentation/ABI/testing/sysfs-class-performance_profile
>> new file mode 100644
>> index 000000000000..9c67cae39600
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-class-performance_profile
>> @@ -0,0 +1,104 @@
>> +Performance-profile selection (e.g. /sys/class/performance_profile/thinkpad_acpi/)
>> +
>> +On modern systems CPU/GPU/... performance is often dynamically configurable
>> +in the form of e.g. variable clock-speeds and TPD. The performance is often
>> +automatically adjusted to the load by some automatic-mechanism (which may
>> +very well live outside the kernel).
>> +
>> +These auto performance-adjustment mechanisms often can be configured with
>> +one of several performance-profiles, with either a bias towards low-power
>> +consumption (and cool and quiet) or towards performance (and higher power
>> +consumption and thermals).
>> +
>> +The purpose of the performance_profile class is to offer a generic sysfs
>> +API for selecting the performance-profile of these automatic-mechanisms.
>> +
>> +Note that this API is only for selecting the performance-profile, it is
>> +NOT a goal of this API to allow monitoring the resulting performance
>> +characteristics. Monitoring performance is best done with device/vendor
>> +specific tools such as e.g. turbostat.
>> +
>> +Specifically when selecting a high-performance profile the actual achieved
>> +performance may be limited by various factors such as: the heat generated by
>> +other components, room temperature, free air flow at the bottom of a laptop,
>> +etc. It is explicitly NOT a goal of this API to let userspace know about
>> +any sub-optimal conditions which are impeding reaching the requested
>> +performance level.
>> +
>> +Since numbers are a rather meaningless way to describe performance-profiles
>> +this API uses strings to describe the various profiles. To make sure that
>> +userspace gets a consistent experience when using this API this API document
>> +defines a fixed set of profile-names. Drivers *must* map their internal
>> +profile representation/names onto this fixed set.
>> +
>> +If for some reason there is no good match when mapping then a new profile-name
>> +may be added. Drivers which wish to introduce new profile-names must:
>> +1. Have very good reasons to do so.
>> +2. Add the new profile-name to this document, so that future drivers which also
>> +   have a similar problem can use the same new. Usually new profile-names will
> 
> Typo, "new" -> "name" I suppose.

Ack, fixed in my local tree.

Thx.

Regards,

Hans


> 
>> +   be added to the "extra profile-names" section of this document. But in some
>> +   cases the set of standard profile-names may be extended.
>> +
>> +What:		/sys/class/performance_profile/<device>/available_profiles
>> +Date:		October 2020
>> +Contact:	Hans de Goede <hdegoede@redhat.com>
>> +Description:
>> +		Reading this file gives a space separated list of profiles
>> +		supported for this device.
>> +
>> +		Drivers must use the following standard profile-names whenever
>> +		possible:
>> +
>> +		low-power:		Emphasises low power consumption
>> +					(and also cool and quiet)
>> +		balanced-low-power:	Balances between low power consumption
>> +					and performance with a slight bias
>> +					towards low power
>> +		balanced:		Balance between low power consumption
>> +					and performance
>> +		balanced-performance:	Balances between performance and low
>> +					power consumption with a slight bias
>> +					towards performance
>> +		performance:		Emphasises performance (and may lead to
>> +					higher temperatures and fan speeds)
>> +
>> +		Userspace may expect drivers to offer at least several of these
>> +		standard profile-names! If none of the above are a good match
>> +		for some of the drivers profiles, then drivers may use one of
>> +		these extra profile-names:
>> +		<reserved for future use>
>> +
>> +What:		/sys/class/performance_profile/<device>/current_profile
>> +Date:		October 2020
>> +Contact:	Hans de Goede <hdegoede@redhat.com>
>> +Description:
>> +		Reading this file gives the current selected profile for this
>> +		device. Writing this file with one of the strings from
>> +		available_profiles changes the profile to the new value.
>> +
>> +		Reading this file may also return "custom". This is intended for
>> +		drivers which have and export multiple knobs influencing
>> +		performance. Such drivers may very well still want to offer a
>> +		set of profiles for easy of use and to be able to offer a
>> +		consistent standard API (this API) to userspace for configuring
>> +		their performance. The "custom" value is intended for when a
>> +		user has directly configured the knobs (through e.g. some
>> +		advanced control-panel for a GPU) and the knob values do not
>> +		match any of the presets represented by the
>> +		performance-profiles. In this case writing this file will
>> +		override the modifications and restore the selected presets.
>> +
>> +What:		/sys/class/performance_profile/<device>/type
>> +Date:		October 2020
>> +Contact:	Hans de Goede <hdegoede@redhat.com>
>> +Description:
>> +		Performance-profiles may be system-wide, or for a specific
>> +		device (e.g. CPU / GPU). System-wide profiles are typically
>> +		used on devices where where a single cooling solution is
>> +		shared between all components, such as laptops and NUCs.
>> +
>> +		Reading this file indicates the type of the device for which
>> +		the thermal-profile is being configured.
>> +
>> +		Valid values: "system"
>> +		Reserved for future use values: "cpu", "gpu"


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-05 12:58   ` Limonciello, Mario
  2020-10-05 14:19     ` Barnabás Pőcze
  2020-10-07 11:51     ` Bastien Nocera
@ 2020-10-09 11:33     ` Hans de Goede
  2 siblings, 0 replies; 26+ messages in thread
From: Hans de Goede @ 2020-10-09 11:33 UTC (permalink / raw)
  To: Limonciello, Mario, Darren Hart, Andy Shevchenko, Mark Gross
  Cc: Mark Pearson, Elia Devito, Bastien Nocera, Benjamin Berg,
	linux-pm, linux-acpi, platform-driver-x86, linux-kernel,
	Mark Pearson

Hi,

On 10/5/20 2:58 PM, Limonciello, Mario wrote:
>> On modern systems CPU/GPU/... performance is often dynamically configurable
>> in the form of e.g. variable clock-speeds and TPD. The performance is often
>> automatically adjusted to the load by some automatic-mechanism (which may
>> very well live outside the kernel).
>>
>> These auto performance-adjustment mechanisms often can be configured with
>> one of several performance-profiles, with either a bias towards low-power
>> consumption (and cool and quiet) or towards performance (and higher power
>> consumption and thermals).
>>
>> Introduce a new performance_profile class/sysfs API which offers a generic
>> API for selecting the performance-profile of these automatic-mechanisms.
>>
> 
> If introducing an API for this - let me ask the question, why even let each
> driver offer a class interface and userspace need to change "each" driver's
> performance setting?
> 
> I would think that you could just offer something kernel-wide like
> /sys/power/performance-profile
> 
> Userspace can read and write to a single file.  All drivers can get notified
> on this sysfs file changing.

In the case of the currently intended users of this API there will be
only 1 provider (using the system type). So that pretty much does what
you suggest.

But I can see there being multiple components in a system which
each can have their own performance-profile. E.g. in some desktop
cases the CPU and GPU may be in separate compartments of the case
which each have their own independent airflow (and thus cooling budget).

Some components may even have their own air cooling with their own
external radiator.

So given the (potential) case with multiple components with each
their own thermal-profile. Then exporting only a single setting
for all components combined has 2 problems:

That would mean either some complicated policy in the kernel
for this, or a simple one where we set them all to the same level.

The simple policy has a number of issues:

1. Setting all components to the same level assume they have
identical profile options, but one component might offer low-power,
while another component does not offer that (the other component
could instead e.g. only offer quiet which is not 100% the same).

2. It is a given that some power users will be wanting to be able
to control the profiles of different components separately, so
having just a simply/naive policy in the kernel for this is not going
to work for all use-cases.

That leaves doing some complex policy mechanism, but in general
where possible the kernel tries to stay out of enforcing policies,
instead (where possible) it is greatly preferred to only offer a
mechanism to allow the functionality and then let userspace handle
any policy decisions.

So I believe that having 1 class device for each component
which offers selectable performance profiles is best.

Note that something like the performance-profile-daemon Bastien
is working on might very will choice to implement a KISS policy
where all components get configured with a similar performance-profile,
resulting in what you are suggesting. But at least this way
leaves the possibility to do things differently open.

>> Cc: Mark Pearson <markpearson@lenovo.com>
>> Cc: Elia Devito <eliadevito@gmail.com>
>> Cc: Bastien Nocera <hadess@hadess.net>
>> Cc: Benjamin Berg <bberg@redhat.com>
>> Cc: linux-pm@vger.kernel.org
>> Cc: linux-acpi@vger.kernel.org
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   .../testing/sysfs-class-performance_profile   | 104 ++++++++++++++++++
>>   1 file changed, 104 insertions(+)
>>   create mode 100644 Documentation/ABI/testing/sysfs-class-performance_profile
>>
>> diff --git a/Documentation/ABI/testing/sysfs-class-performance_profile
>> b/Documentation/ABI/testing/sysfs-class-performance_profile
>> new file mode 100644
>> index 000000000000..9c67cae39600
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-class-performance_profile
>> @@ -0,0 +1,104 @@
>> +Performance-profile selection (e.g.
>> /sys/class/performance_profile/thinkpad_acpi/)
>> +
>> +On modern systems CPU/GPU/... performance is often dynamically configurable
>> +in the form of e.g. variable clock-speeds and TPD. The performance is often
>> +automatically adjusted to the load by some automatic-mechanism (which may
>> +very well live outside the kernel).
> 
> Are you intending to word this specifically to cover both firmware and userspace
> implementations?  Or were you really meaning firmware implementations?

What I'm trying to cover here is both firmware (run by say some microcontroller/EC)
implementations as well and in-kernel implementations like cpufreq and possible
support for controlling GPU frequencies. Note this is intended to future proof
things in case e.g. a GPU driver, where freq. control is done inside the kernel.
would like to use this API. This scenario might never materialize.

Userspace implementations (which seems to likely become a thing on at least ARM)
I would expect to have their own API (e.g. dbus / configfile) to configure their
behavior rather then making a round trip through kernel space with some virtual
device exporting this sysfs API.


>> +These auto performance-adjustment mechanisms often can be configured with
>> +one of several performance-profiles, with either a bias towards low-power
>> +consumption (and cool and quiet) or towards performance (and higher power
>> +consumption and thermals).
>> +
>> +The purpose of the performance_profile class is to offer a generic sysfs
>> +API for selecting the performance-profile of these automatic-mechanisms.
>> +
>> +Note that this API is only for selecting the performance-profile, it is
>> +NOT a goal of this API to allow monitoring the resulting performance
>> +characteristics. Monitoring performance is best done with device/vendor
>> +specific tools such as e.g. turbostat.
> 
> Another thought that comes to mind (which is completely separate from my previous
> idea):
> 
> Why not make this register to firmware-attributes class as being discussed in the
> new Dell driver?
> 
> It seems like it could easily be read as:
> /sys/class/firmware-attributes/thinkpad-foo/attributes/PerformanceProfile/current_value
> /sys/class/firmware-attributes/thinkpad-foo/attributes/PerformanceProfile/possible_values

That would be mixing boot-time settings with runtime settings under the same
API. At least some Thinkpads already have firmware attributes for selecting
the performance profile but that just sets the profile at boot.

More in general we could put pretty much anything which can be expressed as
a key=value pair inside that API, but doing so feels like abusing the API.
The firmware-attributes API really is intended only for setting values which
are stored in some nvram by the firmware, not for runtime changes.

Regards,

Hans


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-07 18:41           ` Limonciello, Mario
@ 2020-10-12 16:42             ` Rafael J. Wysocki
  2020-10-13 13:09               ` Hans de Goede
  0 siblings, 1 reply; 26+ messages in thread
From: Rafael J. Wysocki @ 2020-10-12 16:42 UTC (permalink / raw)
  To: Limonciello, Mario
  Cc: Bastien Nocera, Hans de Goede, Darren Hart, Andy Shevchenko,
	Mark Gross, Mark Pearson, Elia Devito, Benjamin Berg, linux-pm,
	linux-acpi, platform-driver-x86, linux-kernel, Mark Pearson

On Wed, Oct 7, 2020 at 8:41 PM Limonciello, Mario
<Mario.Limonciello@dell.com> wrote:
>
> > On Wed, 2020-10-07 at 15:58 +0000, Limonciello, Mario wrote:
> > >
> > > > On Mon, 2020-10-05 at 12:58 +0000, Limonciello, Mario wrote:
> > > > > > On modern systems CPU/GPU/... performance is often dynamically
> > > > > > configurable
> > > > > > in the form of e.g. variable clock-speeds and TPD. The
> > > > > > performance
> > > > > > is often
> > > > > > automatically adjusted to the load by some automatic-mechanism
> > > > > > (which may
> > > > > > very well live outside the kernel).
> > > > > >
> > > > > > These auto performance-adjustment mechanisms often can be
> > > > > > configured with
> > > > > > one of several performance-profiles, with either a bias towards
> > > > > > low-power
> > > > > > consumption (and cool and quiet) or towards performance (and
> > > > > > higher
> > > > > > power
> > > > > > consumption and thermals).
> > > > > >
> > > > > > Introduce a new performance_profile class/sysfs API which
> > > > > > offers a
> > > > > > generic
> > > > > > API for selecting the performance-profile of these automatic-
> > > > > > mechanisms.
> > > > > >
> > > > >
> > > > > If introducing an API for this - let me ask the question, why
> > > > > even let each
> > > > > driver offer a class interface and userspace need to change
> > > > > "each" driver's
> > > > > performance setting?
> > > > >
> > > > > I would think that you could just offer something kernel-wide
> > > > > like
> > > > > /sys/power/performance-profile
> > > > >
> > > > > Userspace can read and write to a single file.  All drivers can
> > > > > get notified
> > > > > on this sysfs file changing.
> > > > >
> > > > > The systems that react in firmware (such as the two that prompted
> > > > > this discussion) can change at that time.  It leaves the
> > > > > possibility for a
> > > > > more open kernel implementation that can do the same thing though
> > > > > too by
> > > > > directly modifying device registers instead of ACPI devices.
> > > >
> > > > The problem, as I've mentioned in previous discussions we had about
> > > > this, is that, as you've seen in replies to this mail, this would
> > > > suddenly be making the kernel apply policy.
> > > >
> > > > There's going to be pushback as soon as policy is enacted in the
> > > > kernel, and you take away the different knobs for individual
> > > > components
> > > > (or you can control them centrally as well as individually). As
> > > > much as
> > > > I hate the quantity of knobs[1], I don't think that trying to
> > > > reduce
> > > > the number of knobs in the kernel is a good use of our time, and
> > > > easier
> > > > to enact, coordinated with design targets, in user-space.
> > > >
> > > > Unless you can think of a way to implement this kernel wide setting
> > > > without adding one more exponent on the number of possibilities for
> > > > the
> > > > testing matrix, I'll +1 Hans' original API.
> > > >
> > > Actually I offered two proposals in my reply.  So are you NAKing
> > > both?
> >
> > No, this is only about the first portion of the email, which I quoted.
> > And I'm not NAK'ing it, but I don't see how it can work without being
> > antithetical to what kernel "users" expect, or what the folks consuming
> > those interfaces (presumably us both) would expect to be able to test
> > and maintain.
> >
>
> (Just so others are aware, Bastien and I had a previous discussion on this topic
> that he alluded to here: https://gitlab.freedesktop.org/hadess/power-profiles-daemon/-/issues/1)
>
> In general I agree that we shouldn't be offering 100's of knobs to change
> things and protect users from themselves where possible.
>
> Whether the decisions are made in the kernel or in userspace you still have a matrix once
> you're letting someone change 2 different kernel devices that offer policy.  I'd argue it's
> actually worse if you let userspace change it though.
>
> Let's go back to the my GPU and platform example and lets say both offer the new knob here
> for both.  Userspace software such as your PPD picks performance.  Both the platform device
> and GPU device get changed, hopefully no conflicts.
> Then user decides no, I don't want my GPU in performance mode, I only want my platform.
> So they change the knob for the GPU manually, and now you have a new config in your matrix.
>
> However if you left it to a single kernel knob, both GPU and platform get moved together and
> you don't have these extra configs in your matrix anymore.
>
> The other point I mentioned, that platform might also do something to GPU via a sideband and
> you race, you can solve it with kernel too by modifying the ordering the kernel handles it.
>
> Userspace however, you give two knobs and now you have to worry about them getting it right
> and supporting them doing them in the wrong order.
>
> > > The other one suggested to use the same firmware attributes class
> > > being
> > > introduced by the new Dell driver (
> > > https://patchwork.kernel.org/patch/11818343/)
> > > since this is actually a knob to a specific firmware setting.
> >
> > This seemed to me like an implementation detail (eg. the same metadata
> > is being exported, but in a different way), and I don't feel strongly
> > about it either way.
>
> OK thanks.

IMV there are two choices here:  One is between exposing the low-level
interfaces verbatim to user space and wrapping them up into a certain
"translation" layer allowing user space to use a unified interface (I
think that is what everybody wants) and the other  boils down to how
the unified interface between the kernel and user space will look
like.

Personally, I think that something line /sys/power/profile allowing
drivers (and other kernel entities) to register callbacks might work
(as stated in my last reply to Hans).

Cheers!

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-12 16:42             ` Rafael J. Wysocki
@ 2020-10-13 13:09               ` Hans de Goede
  2020-10-14 13:55                 ` Rafael J. Wysocki
  0 siblings, 1 reply; 26+ messages in thread
From: Hans de Goede @ 2020-10-13 13:09 UTC (permalink / raw)
  To: Rafael J. Wysocki, Limonciello, Mario
  Cc: Bastien Nocera, Darren Hart, Andy Shevchenko, Mark Gross,
	Mark Pearson, Elia Devito, Benjamin Berg, linux-pm, linux-acpi,
	platform-driver-x86, linux-kernel, Mark Pearson

Hi,

On 10/12/20 6:42 PM, Rafael J. Wysocki wrote:
> On Wed, Oct 7, 2020 at 8:41 PM Limonciello, Mario
> <Mario.Limonciello@dell.com> wrote:
>>
>>> On Wed, 2020-10-07 at 15:58 +0000, Limonciello, Mario wrote:
>>>>
>>>>> On Mon, 2020-10-05 at 12:58 +0000, Limonciello, Mario wrote:
>>>>>>> On modern systems CPU/GPU/... performance is often dynamically
>>>>>>> configurable
>>>>>>> in the form of e.g. variable clock-speeds and TPD. The
>>>>>>> performance
>>>>>>> is often
>>>>>>> automatically adjusted to the load by some automatic-mechanism
>>>>>>> (which may
>>>>>>> very well live outside the kernel).
>>>>>>>
>>>>>>> These auto performance-adjustment mechanisms often can be
>>>>>>> configured with
>>>>>>> one of several performance-profiles, with either a bias towards
>>>>>>> low-power
>>>>>>> consumption (and cool and quiet) or towards performance (and
>>>>>>> higher
>>>>>>> power
>>>>>>> consumption and thermals).
>>>>>>>
>>>>>>> Introduce a new performance_profile class/sysfs API which
>>>>>>> offers a
>>>>>>> generic
>>>>>>> API for selecting the performance-profile of these automatic-
>>>>>>> mechanisms.
>>>>>>>
>>>>>>
>>>>>> If introducing an API for this - let me ask the question, why
>>>>>> even let each
>>>>>> driver offer a class interface and userspace need to change
>>>>>> "each" driver's
>>>>>> performance setting?
>>>>>>
>>>>>> I would think that you could just offer something kernel-wide
>>>>>> like
>>>>>> /sys/power/performance-profile
>>>>>>
>>>>>> Userspace can read and write to a single file.  All drivers can
>>>>>> get notified
>>>>>> on this sysfs file changing.
>>>>>>
>>>>>> The systems that react in firmware (such as the two that prompted
>>>>>> this discussion) can change at that time.  It leaves the
>>>>>> possibility for a
>>>>>> more open kernel implementation that can do the same thing though
>>>>>> too by
>>>>>> directly modifying device registers instead of ACPI devices.
>>>>>
>>>>> The problem, as I've mentioned in previous discussions we had about
>>>>> this, is that, as you've seen in replies to this mail, this would
>>>>> suddenly be making the kernel apply policy.
>>>>>
>>>>> There's going to be pushback as soon as policy is enacted in the
>>>>> kernel, and you take away the different knobs for individual
>>>>> components
>>>>> (or you can control them centrally as well as individually). As
>>>>> much as
>>>>> I hate the quantity of knobs[1], I don't think that trying to
>>>>> reduce
>>>>> the number of knobs in the kernel is a good use of our time, and
>>>>> easier
>>>>> to enact, coordinated with design targets, in user-space.
>>>>>
>>>>> Unless you can think of a way to implement this kernel wide setting
>>>>> without adding one more exponent on the number of possibilities for
>>>>> the
>>>>> testing matrix, I'll +1 Hans' original API.
>>>>>
>>>> Actually I offered two proposals in my reply.  So are you NAKing
>>>> both?
>>>
>>> No, this is only about the first portion of the email, which I quoted.
>>> And I'm not NAK'ing it, but I don't see how it can work without being
>>> antithetical to what kernel "users" expect, or what the folks consuming
>>> those interfaces (presumably us both) would expect to be able to test
>>> and maintain.
>>>
>>
>> (Just so others are aware, Bastien and I had a previous discussion on this topic
>> that he alluded to here: https://gitlab.freedesktop.org/hadess/power-profiles-daemon/-/issues/1)
>>
>> In general I agree that we shouldn't be offering 100's of knobs to change
>> things and protect users from themselves where possible.
>>
>> Whether the decisions are made in the kernel or in userspace you still have a matrix once
>> you're letting someone change 2 different kernel devices that offer policy.  I'd argue it's
>> actually worse if you let userspace change it though.
>>
>> Let's go back to the my GPU and platform example and lets say both offer the new knob here
>> for both.  Userspace software such as your PPD picks performance.  Both the platform device
>> and GPU device get changed, hopefully no conflicts.
>> Then user decides no, I don't want my GPU in performance mode, I only want my platform.
>> So they change the knob for the GPU manually, and now you have a new config in your matrix.
>>
>> However if you left it to a single kernel knob, both GPU and platform get moved together and
>> you don't have these extra configs in your matrix anymore.
>>
>> The other point I mentioned, that platform might also do something to GPU via a sideband and
>> you race, you can solve it with kernel too by modifying the ordering the kernel handles it.
>>
>> Userspace however, you give two knobs and now you have to worry about them getting it right
>> and supporting them doing them in the wrong order.
>>
>>>> The other one suggested to use the same firmware attributes class
>>>> being
>>>> introduced by the new Dell driver (
>>>> https://patchwork.kernel.org/patch/11818343/)
>>>> since this is actually a knob to a specific firmware setting.
>>>
>>> This seemed to me like an implementation detail (eg. the same metadata
>>> is being exported, but in a different way), and I don't feel strongly
>>> about it either way.
>>
>> OK thanks.
> 
> IMV there are two choices here:  One is between exposing the low-level
> interfaces verbatim to user space and wrapping them up into a certain
> "translation" layer allowing user space to use a unified interface (I
> think that is what everybody wants) and the other  boils down to how
> the unified interface between the kernel and user space will look
> like.
> 
> Personally, I think that something line /sys/power/profile allowing
> drivers (and other kernel entities) to register callbacks might work
> (as stated in my last reply to Hans).

Note to others reading along I pointed to this thread in this thread:
https://lore.kernel.org/linux-pm/20201006122024.14539-1-daniel.lezcano@linaro.org/T/#t
and Rafael's "last reply" above refers to his reply in that thread.

For the sake of people reading along I'm reproducing my reply
there below.

Rafael, it seems more appropriate to continue this discussion
in this thread, so lets discuss this further here ?

My reply to Rafael from the other thread:

First of all thank you for your input, with your expertise in this
area your input is very much appreciated, after all we only get
one chance to get the userspace API for this right.

Your proposal to have a single sysfs file for userspace to talk
to and then use an in kernel subscription mechanism for drivers
to get notified of writes to this file is interesting.

But I see 2 issues with it:

1. How will userspace know which profiles are actually available ?

An obvious solution is to pick a set of standard names and let
subscribers map those as close to their own settings as possible,
the most often mentioned set of profile names in this case seems to be:

low_power
balanced_power
balanced
balanced_performance
performance

Which works fine for the thinkpad_acpi case, but not so much for
the hp-wmi case. In the HP case what happens is that a WMI call
is made which sets a bunch of ACPI variables which influence
the DPTF code (this assumes we have some sort of DPTF support
such as mjg59's reverse engineered support) but the profile-names
under Windows are: "Performance", "HP recommended", "Cool" and
"Quiet".  If you read the discussion from the
"[RFC] Documentation: Add documentation for new performance_profile sysfs class"
thread you will see this was brought up as an issue there.

The problem here is that both "cool" and "quiet" could be
interpreted as low-power. But it seems that they actually mean
what they say, cool focuses on keeping temps low, which can
also be done by making the fan-profile more aggressive. And quiet
is mostly about keeping fan speeds down, at the cost of possible
higher temperatures.

<edit in this version of the reply:>
I wonder if the HP profiles are actually just fan speed profiles ?
Elia do you know ?
</edit>

IOW we don't really have a 1 dimensional axis.
My class proposal fixes this by having a notion of both
standardized names (because anything else would suck) combined
with a way for drivers to advertise which standardized names
the support. So in my proposal I simply add quiet and cool
to the list of standard profile names, and then the HP-wmi
driver can list those as supported, while not listing
low_power as a supported profile.  This way we export the
hardware interface to userspace as is (as much as possible)
while still offering a standardized interface for userspace
to consume.  Granted if userspace now actually want to set
a low_power profile, we have just punted the problem to userspace
but I really do not see a better solution.


2. This only works assuming that all performance-profiles
are system wide. But given a big desktop case there might
be very well be separate cooling zones for e.g. the CPU
and the GPU and I can imagine both having separate
performance-profile settings and some users will doubtlessly
want to be able to control these separately ...

Regards,

Hans


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-13 13:09               ` Hans de Goede
@ 2020-10-14 13:55                 ` Rafael J. Wysocki
  2020-10-14 14:16                   ` Hans de Goede
  0 siblings, 1 reply; 26+ messages in thread
From: Rafael J. Wysocki @ 2020-10-14 13:55 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Rafael J. Wysocki, Limonciello, Mario, Bastien Nocera,
	Darren Hart, Andy Shevchenko, Mark Gross, Mark Pearson,
	Elia Devito, Benjamin Berg, linux-pm, linux-acpi,
	platform-driver-x86, linux-kernel, Mark Pearson,
	Srinivas Pandruvada

On Tue, Oct 13, 2020 at 3:09 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 10/12/20 6:42 PM, Rafael J. Wysocki wrote:
> > On Wed, Oct 7, 2020 at 8:41 PM Limonciello, Mario
> > <Mario.Limonciello@dell.com> wrote:
> >>
> >>> On Wed, 2020-10-07 at 15:58 +0000, Limonciello, Mario wrote:
> >>>>
> >>>>> On Mon, 2020-10-05 at 12:58 +0000, Limonciello, Mario wrote:
> >>>>>>> On modern systems CPU/GPU/... performance is often dynamically
> >>>>>>> configurable
> >>>>>>> in the form of e.g. variable clock-speeds and TPD. The
> >>>>>>> performance
> >>>>>>> is often
> >>>>>>> automatically adjusted to the load by some automatic-mechanism
> >>>>>>> (which may
> >>>>>>> very well live outside the kernel).
> >>>>>>>
> >>>>>>> These auto performance-adjustment mechanisms often can be
> >>>>>>> configured with
> >>>>>>> one of several performance-profiles, with either a bias towards
> >>>>>>> low-power
> >>>>>>> consumption (and cool and quiet) or towards performance (and
> >>>>>>> higher
> >>>>>>> power
> >>>>>>> consumption and thermals).
> >>>>>>>
> >>>>>>> Introduce a new performance_profile class/sysfs API which
> >>>>>>> offers a
> >>>>>>> generic
> >>>>>>> API for selecting the performance-profile of these automatic-
> >>>>>>> mechanisms.
> >>>>>>>
> >>>>>>
> >>>>>> If introducing an API for this - let me ask the question, why
> >>>>>> even let each
> >>>>>> driver offer a class interface and userspace need to change
> >>>>>> "each" driver's
> >>>>>> performance setting?
> >>>>>>
> >>>>>> I would think that you could just offer something kernel-wide
> >>>>>> like
> >>>>>> /sys/power/performance-profile
> >>>>>>
> >>>>>> Userspace can read and write to a single file.  All drivers can
> >>>>>> get notified
> >>>>>> on this sysfs file changing.
> >>>>>>
> >>>>>> The systems that react in firmware (such as the two that prompted
> >>>>>> this discussion) can change at that time.  It leaves the
> >>>>>> possibility for a
> >>>>>> more open kernel implementation that can do the same thing though
> >>>>>> too by
> >>>>>> directly modifying device registers instead of ACPI devices.
> >>>>>
> >>>>> The problem, as I've mentioned in previous discussions we had about
> >>>>> this, is that, as you've seen in replies to this mail, this would
> >>>>> suddenly be making the kernel apply policy.
> >>>>>
> >>>>> There's going to be pushback as soon as policy is enacted in the
> >>>>> kernel, and you take away the different knobs for individual
> >>>>> components
> >>>>> (or you can control them centrally as well as individually). As
> >>>>> much as
> >>>>> I hate the quantity of knobs[1], I don't think that trying to
> >>>>> reduce
> >>>>> the number of knobs in the kernel is a good use of our time, and
> >>>>> easier
> >>>>> to enact, coordinated with design targets, in user-space.
> >>>>>
> >>>>> Unless you can think of a way to implement this kernel wide setting
> >>>>> without adding one more exponent on the number of possibilities for
> >>>>> the
> >>>>> testing matrix, I'll +1 Hans' original API.
> >>>>>
> >>>> Actually I offered two proposals in my reply.  So are you NAKing
> >>>> both?
> >>>
> >>> No, this is only about the first portion of the email, which I quoted.
> >>> And I'm not NAK'ing it, but I don't see how it can work without being
> >>> antithetical to what kernel "users" expect, or what the folks consuming
> >>> those interfaces (presumably us both) would expect to be able to test
> >>> and maintain.
> >>>
> >>
> >> (Just so others are aware, Bastien and I had a previous discussion on this topic
> >> that he alluded to here: https://gitlab.freedesktop.org/hadess/power-profiles-daemon/-/issues/1)
> >>
> >> In general I agree that we shouldn't be offering 100's of knobs to change
> >> things and protect users from themselves where possible.
> >>
> >> Whether the decisions are made in the kernel or in userspace you still have a matrix once
> >> you're letting someone change 2 different kernel devices that offer policy.  I'd argue it's
> >> actually worse if you let userspace change it though.
> >>
> >> Let's go back to the my GPU and platform example and lets say both offer the new knob here
> >> for both.  Userspace software such as your PPD picks performance.  Both the platform device
> >> and GPU device get changed, hopefully no conflicts.
> >> Then user decides no, I don't want my GPU in performance mode, I only want my platform.
> >> So they change the knob for the GPU manually, and now you have a new config in your matrix.
> >>
> >> However if you left it to a single kernel knob, both GPU and platform get moved together and
> >> you don't have these extra configs in your matrix anymore.
> >>
> >> The other point I mentioned, that platform might also do something to GPU via a sideband and
> >> you race, you can solve it with kernel too by modifying the ordering the kernel handles it.
> >>
> >> Userspace however, you give two knobs and now you have to worry about them getting it right
> >> and supporting them doing them in the wrong order.
> >>
> >>>> The other one suggested to use the same firmware attributes class
> >>>> being
> >>>> introduced by the new Dell driver (
> >>>> https://patchwork.kernel.org/patch/11818343/)
> >>>> since this is actually a knob to a specific firmware setting.
> >>>
> >>> This seemed to me like an implementation detail (eg. the same metadata
> >>> is being exported, but in a different way), and I don't feel strongly
> >>> about it either way.
> >>
> >> OK thanks.
> >
> > IMV there are two choices here:  One is between exposing the low-level
> > interfaces verbatim to user space and wrapping them up into a certain
> > "translation" layer allowing user space to use a unified interface (I
> > think that is what everybody wants) and the other  boils down to how
> > the unified interface between the kernel and user space will look
> > like.
> >
> > Personally, I think that something line /sys/power/profile allowing
> > drivers (and other kernel entities) to register callbacks might work
> > (as stated in my last reply to Hans).
>
> Note to others reading along I pointed to this thread in this thread:
> https://lore.kernel.org/linux-pm/20201006122024.14539-1-daniel.lezcano@linaro.org/T/#t
> and Rafael's "last reply" above refers to his reply in that thread.
>
> For the sake of people reading along I'm reproducing my reply
> there below.

For completeness, my response in the other thread is here:

https://lore.kernel.org/linux-pm/CAJZ5v0jpYpu3Tk7qq_MCVs0wUr-Dw0rY5EZELrVbQta0NZaoVA@mail.gmail.com/T/#t

> Rafael, it seems more appropriate to continue this discussion
> in this thread, so lets discuss this further here ?

And because I sent it before reading this message, let me reproduce it
below (with some additions).

> My reply to Rafael from the other thread:
>
> First of all thank you for your input, with your expertise in this
> area your input is very much appreciated, after all we only get
> one chance to get the userspace API for this right.
>
> Your proposal to have a single sysfs file for userspace to talk
> to and then use an in kernel subscription mechanism for drivers
> to get notified of writes to this file is interesting.
>
> But I see 2 issues with it:
>
> 1. How will userspace know which profiles are actually available ?
>
> An obvious solution is to pick a set of standard names and let
> subscribers map those as close to their own settings as possible,
> the most often mentioned set of profile names in this case seems to be:
>
> low_power
> balanced_power
> balanced
> balanced_performance
> performance
>
> Which works fine for the thinkpad_acpi case, but not so much for
> the hp-wmi case. In the HP case what happens is that a WMI call
> is made which sets a bunch of ACPI variables which influence
> the DPTF code (this assumes we have some sort of DPTF support
> such as mjg59's reverse engineered support) but the profile-names
> under Windows are: "Performance", "HP recommended", "Cool" and
> "Quiet".  If you read the discussion from the
> "[RFC] Documentation: Add documentation for new performance_profile sysfs class"
> thread you will see this was brought up as an issue there.

Two different things seem to be conflated here.  One is how to pass a
possible performance-vs-power preference coming from user space down
to device drivers or generally pieces of kernel code that can adjust
the behavior and/or hardware settings depending on what that
preference is and the other is how to expose OEM-provided DPTF system
profile interfaces to user space.

The former assumes that there is a common set of values that can be
understood and acted on in a consistent way by all of the interested
entities within the kernel and the latter is about passing information
from user space down to a side-band power control mechanism working in
its own way behind the kernel's back (and possibly poking at multiple
hardware components in the platform in its own way).

IMO there is no way to provide a common interface covering these two
cases at the same time.

> The problem here is that both "cool" and "quiet" could be
> interpreted as low-power. But it seems that they actually mean
> what they say, cool focuses on keeping temps low, which can
> also be done by making the fan-profile more aggressive. And quiet
> is mostly about keeping fan speeds down, at the cost of possible
> higher temperatures.
>
> <edit in this version of the reply:>
> I wonder if the HP profiles are actually just fan speed profiles ?
> Elia do you know ?
> </edit>

I don't think so.

AFAICS, in both the Thinkpad and HP cases the profile covers the
entire platform, which in particular means that they cannot co-exist.

> IOW we don't really have a 1 dimensional axis.

Well, AFAICS, DPTF system profile interfaces coming from different
OEMs will be different, but they are about side-band power control and
there can be only one thing like that in a platform at the same time.

> My class proposal fixes this by having a notion of both
> standardized names (because anything else would suck) combined
> with a way for drivers to advertise which standardized names
> the support. So in my proposal I simply add quiet and cool
> to the list of standard profile names, and then the HP-wmi
> driver can list those as supported, while not listing
> low_power as a supported profile.  This way we export the
> hardware interface to userspace as is (as much as possible)
> while still offering a standardized interface for userspace
> to consume.  Granted if userspace now actually want to set
> a low_power profile, we have just punted the problem to userspace
> but I really do not see a better solution.

First, a common place to register a DPTF system profile seems to be
needed and, as I said above, I wouldn't expect more than one such
thing to be present in the system at any given time, so it may be
registered along with the list of supported profiles and user space
will have to understand what they mean.

Second, irrespective of the above, it may be useful to have a
consistent way to pass performance-vs-power preference information
from user space to different parts of the kernel so as to allow them
to adjust their operation and this could be done with a system-wide
power profile attribute IMO.

> 2. This only works assuming that all performance-profiles
> are system wide. But given a big desktop case there might
> be very well be separate cooling zones for e.g. the CPU
> and the GPU and I can imagine both having separate
> performance-profile settings and some users will doubtlessly
> want to be able to control these separately ...

Let's say that I'm not convinced. :-)

They cannot be totally separate, because they will affect each other
and making possibly conflicting adjustments needs to be avoided.

Cheers!

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-14 13:55                 ` Rafael J. Wysocki
@ 2020-10-14 14:16                   ` Hans de Goede
  2020-10-14 15:46                     ` Rafael J. Wysocki
  0 siblings, 1 reply; 26+ messages in thread
From: Hans de Goede @ 2020-10-14 14:16 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Limonciello, Mario, Bastien Nocera, Darren Hart, Andy Shevchenko,
	Mark Gross, Mark Pearson, Elia Devito, Benjamin Berg, linux-pm,
	linux-acpi, platform-driver-x86, linux-kernel, Mark Pearson,
	Srinivas Pandruvada

Hi,

On 10/14/20 3:55 PM, Rafael J. Wysocki wrote:
> On Tue, Oct 13, 2020 at 3:09 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> On 10/12/20 6:42 PM, Rafael J. Wysocki wrote:
>>> On Wed, Oct 7, 2020 at 8:41 PM Limonciello, Mario
>>> <Mario.Limonciello@dell.com> wrote:
>>>>
>>>>> On Wed, 2020-10-07 at 15:58 +0000, Limonciello, Mario wrote:
>>>>>>
>>>>>>> On Mon, 2020-10-05 at 12:58 +0000, Limonciello, Mario wrote:
>>>>>>>>> On modern systems CPU/GPU/... performance is often dynamically
>>>>>>>>> configurable
>>>>>>>>> in the form of e.g. variable clock-speeds and TPD. The
>>>>>>>>> performance
>>>>>>>>> is often
>>>>>>>>> automatically adjusted to the load by some automatic-mechanism
>>>>>>>>> (which may
>>>>>>>>> very well live outside the kernel).
>>>>>>>>>
>>>>>>>>> These auto performance-adjustment mechanisms often can be
>>>>>>>>> configured with
>>>>>>>>> one of several performance-profiles, with either a bias towards
>>>>>>>>> low-power
>>>>>>>>> consumption (and cool and quiet) or towards performance (and
>>>>>>>>> higher
>>>>>>>>> power
>>>>>>>>> consumption and thermals).
>>>>>>>>>
>>>>>>>>> Introduce a new performance_profile class/sysfs API which
>>>>>>>>> offers a
>>>>>>>>> generic
>>>>>>>>> API for selecting the performance-profile of these automatic-
>>>>>>>>> mechanisms.
>>>>>>>>>
>>>>>>>>
>>>>>>>> If introducing an API for this - let me ask the question, why
>>>>>>>> even let each
>>>>>>>> driver offer a class interface and userspace need to change
>>>>>>>> "each" driver's
>>>>>>>> performance setting?
>>>>>>>>
>>>>>>>> I would think that you could just offer something kernel-wide
>>>>>>>> like
>>>>>>>> /sys/power/performance-profile
>>>>>>>>
>>>>>>>> Userspace can read and write to a single file.  All drivers can
>>>>>>>> get notified
>>>>>>>> on this sysfs file changing.
>>>>>>>>
>>>>>>>> The systems that react in firmware (such as the two that prompted
>>>>>>>> this discussion) can change at that time.  It leaves the
>>>>>>>> possibility for a
>>>>>>>> more open kernel implementation that can do the same thing though
>>>>>>>> too by
>>>>>>>> directly modifying device registers instead of ACPI devices.
>>>>>>>
>>>>>>> The problem, as I've mentioned in previous discussions we had about
>>>>>>> this, is that, as you've seen in replies to this mail, this would
>>>>>>> suddenly be making the kernel apply policy.
>>>>>>>
>>>>>>> There's going to be pushback as soon as policy is enacted in the
>>>>>>> kernel, and you take away the different knobs for individual
>>>>>>> components
>>>>>>> (or you can control them centrally as well as individually). As
>>>>>>> much as
>>>>>>> I hate the quantity of knobs[1], I don't think that trying to
>>>>>>> reduce
>>>>>>> the number of knobs in the kernel is a good use of our time, and
>>>>>>> easier
>>>>>>> to enact, coordinated with design targets, in user-space.
>>>>>>>
>>>>>>> Unless you can think of a way to implement this kernel wide setting
>>>>>>> without adding one more exponent on the number of possibilities for
>>>>>>> the
>>>>>>> testing matrix, I'll +1 Hans' original API.
>>>>>>>
>>>>>> Actually I offered two proposals in my reply.  So are you NAKing
>>>>>> both?
>>>>>
>>>>> No, this is only about the first portion of the email, which I quoted.
>>>>> And I'm not NAK'ing it, but I don't see how it can work without being
>>>>> antithetical to what kernel "users" expect, or what the folks consuming
>>>>> those interfaces (presumably us both) would expect to be able to test
>>>>> and maintain.
>>>>>
>>>>
>>>> (Just so others are aware, Bastien and I had a previous discussion on this topic
>>>> that he alluded to here: https://gitlab.freedesktop.org/hadess/power-profiles-daemon/-/issues/1)
>>>>
>>>> In general I agree that we shouldn't be offering 100's of knobs to change
>>>> things and protect users from themselves where possible.
>>>>
>>>> Whether the decisions are made in the kernel or in userspace you still have a matrix once
>>>> you're letting someone change 2 different kernel devices that offer policy.  I'd argue it's
>>>> actually worse if you let userspace change it though.
>>>>
>>>> Let's go back to the my GPU and platform example and lets say both offer the new knob here
>>>> for both.  Userspace software such as your PPD picks performance.  Both the platform device
>>>> and GPU device get changed, hopefully no conflicts.
>>>> Then user decides no, I don't want my GPU in performance mode, I only want my platform.
>>>> So they change the knob for the GPU manually, and now you have a new config in your matrix.
>>>>
>>>> However if you left it to a single kernel knob, both GPU and platform get moved together and
>>>> you don't have these extra configs in your matrix anymore.
>>>>
>>>> The other point I mentioned, that platform might also do something to GPU via a sideband and
>>>> you race, you can solve it with kernel too by modifying the ordering the kernel handles it.
>>>>
>>>> Userspace however, you give two knobs and now you have to worry about them getting it right
>>>> and supporting them doing them in the wrong order.
>>>>
>>>>>> The other one suggested to use the same firmware attributes class
>>>>>> being
>>>>>> introduced by the new Dell driver (
>>>>>> https://patchwork.kernel.org/patch/11818343/)
>>>>>> since this is actually a knob to a specific firmware setting.
>>>>>
>>>>> This seemed to me like an implementation detail (eg. the same metadata
>>>>> is being exported, but in a different way), and I don't feel strongly
>>>>> about it either way.
>>>>
>>>> OK thanks.
>>>
>>> IMV there are two choices here:  One is between exposing the low-level
>>> interfaces verbatim to user space and wrapping them up into a certain
>>> "translation" layer allowing user space to use a unified interface (I
>>> think that is what everybody wants) and the other  boils down to how
>>> the unified interface between the kernel and user space will look
>>> like.
>>>
>>> Personally, I think that something line /sys/power/profile allowing
>>> drivers (and other kernel entities) to register callbacks might work
>>> (as stated in my last reply to Hans).
>>
>> Note to others reading along I pointed to this thread in this thread:
>> https://lore.kernel.org/linux-pm/20201006122024.14539-1-daniel.lezcano@linaro.org/T/#t
>> and Rafael's "last reply" above refers to his reply in that thread.
>>
>> For the sake of people reading along I'm reproducing my reply
>> there below.
> 
> For completeness, my response in the other thread is here:
> 
> https://lore.kernel.org/linux-pm/CAJZ5v0jpYpu3Tk7qq_MCVs0wUr-Dw0rY5EZELrVbQta0NZaoVA@mail.gmail.com/T/#t
> 
>> Rafael, it seems more appropriate to continue this discussion
>> in this thread, so lets discuss this further here ?
> 
> And because I sent it before reading this message, let me reproduce it
> below (with some additions).

And I just did the same thing (replied to your reply in the other thread),
I guess I was too quick.

So I too will reproduce my reply here.

And I still believe it is best to then stick to this thread from now on,
because this reproducing thing is not really productive...

>> My reply to Rafael from the other thread:
>>
>> First of all thank you for your input, with your expertise in this
>> area your input is very much appreciated, after all we only get
>> one chance to get the userspace API for this right.
>>
>> Your proposal to have a single sysfs file for userspace to talk
>> to and then use an in kernel subscription mechanism for drivers
>> to get notified of writes to this file is interesting.
>>
>> But I see 2 issues with it:
>>
>> 1. How will userspace know which profiles are actually available ?
>>
>> An obvious solution is to pick a set of standard names and let
>> subscribers map those as close to their own settings as possible,
>> the most often mentioned set of profile names in this case seems to be:
>>
>> low_power
>> balanced_power
>> balanced
>> balanced_performance
>> performance
>>
>> Which works fine for the thinkpad_acpi case, but not so much for
>> the hp-wmi case. In the HP case what happens is that a WMI call
>> is made which sets a bunch of ACPI variables which influence
>> the DPTF code (this assumes we have some sort of DPTF support
>> such as mjg59's reverse engineered support) but the profile-names
>> under Windows are: "Performance", "HP recommended", "Cool" and
>> "Quiet".  If you read the discussion from the
>> "[RFC] Documentation: Add documentation for new performance_profile sysfs class"
>> thread you will see this was brought up as an issue there.
> 
> Two different things seem to be conflated here.  One is how to pass a
> possible performance-vs-power preference coming from user space down
> to device drivers or generally pieces of kernel code that can adjust
> the behavior and/or hardware settings depending on what that
> preference is and the other is how to expose OEM-provided DPTF system
> profile interfaces to user space.

I was hoping / thinking that we could use a single API for both of
these. But I guess that it makes sense to see them as 2 separate
things, esp. since DPTF profiles seem to be somewhat free-form
where as a way to pass a performance-pref to a device could use
a fixes set of values.

So lets say that we indeed want to treat these 2 separately,
then I guess that the issue at hand / my reason to start a
discussion surrounding this is allowing userspace to selecting
the DPTF system profile.

The thinkpad_acpi case at hand is not using DPTF, but that is
because Lenovo decided to implement dynamic DPTF like behavior
inside their embedded controller (for when running Linux) since
DPTF is atm not really supported all that well under Linux and
Lenovo was getting a lot of complaints about sub-optimal
performance because of this.

So the thinkpad_acpi solution is in essence a replacement
for DPTF and it should thus use the same userspace API as
other mechanisms to select DPTF system profiles.

And if we limit this new userspace API solely to setting DPTF
system profiles, then their will indeed be only 1 provider for
this for the entire system.

> The former assumes that there is a common set of values that can be
> understood and acted on in a consistent way by all of the interested
> entities within the kernel and the latter is about passing information
> from user space down to a side-band power control mechanism working in
> its own way behind the kernel's back (and possibly poking at multiple
> hardware components in the platform in its own way).

Ack.

> IMO there is no way to provide a common interface covering these two
> cases at the same time.

I see your point, esp. the free form vs common set of values
argument seems to be exactly what we have been going in circles
about during the discussion about this so far.

>> The problem here is that both "cool" and "quiet" could be
>> interpreted as low-power. But it seems that they actually mean
>> what they say, cool focuses on keeping temps low, which can
>> also be done by making the fan-profile more aggressive. And quiet
>> is mostly about keeping fan speeds down, at the cost of possible
>> higher temperatures.
>>
>> <edit in this version of the reply:>
>> I wonder if the HP profiles are actually just fan speed profiles ?
>> Elia do you know ?
>> </edit>
> 
> I don't think so.
> 
> AFAICS, in both the Thinkpad and HP cases the profile covers the
> entire platform, which in particular means that they cannot co-exist.

Ok.

>> IOW we don't really have a 1 dimensional axis.
> 
> Well, AFAICS, DPTF system profile interfaces coming from different
> OEMs will be different, but they are about side-band power control and
> there can be only one thing like that in a platform at the same time.

Ack.

>> My class proposal fixes this by having a notion of both
>> standardized names (because anything else would suck) combined
>> with a way for drivers to advertise which standardized names
>> the support. So in my proposal I simply add quiet and cool
>> to the list of standard profile names, and then the HP-wmi
>> driver can list those as supported, while not listing
>> low_power as a supported profile.  This way we export the
>> hardware interface to userspace as is (as much as possible)
>> while still offering a standardized interface for userspace
>> to consume.  Granted if userspace now actually want to set
>> a low_power profile, we have just punted the problem to userspace
>> but I really do not see a better solution.
> 
> First, a common place to register a DPTF system profile seems to be
> needed and, as I said above, I wouldn't expect more than one such
> thing to be present in the system at any given time, so it may be
> registered along with the list of supported profiles and user space
> will have to understand what they mean.

Mostly Ack, I would still like to have an enum for DPTF system
profiles in the kernel and have a single piece of code map that
enum to profile names. This enum can then be extended as
necessary, but I want to avoid having one driver use
"Performance" and the other "performance" or one using
"performance-balanced" and the other "balanced-performance", etc.

With the goal being that new drivers use existing values from
the enum as much as possible, but we extend it where necessary.

> Second, irrespective of the above, it may be useful to have a
> consistent way to pass performance-vs-power preference information
> from user space to different parts of the kernel so as to allow them
> to adjust their operation and this could be done with a system-wide
> power profile attribute IMO.

I agree, which is why I tried to tackle both things in one go,
but as you said doing both in 1 API is probably not the best idea.
So I believe we should park this second issue for now and revisit it
when we find a need for it.

Do you have any specific userspace API in mind for the
DPTF system profile selection?

And to get one thing out of the way, in the other thread we had some
discussion about using a single attribute where a cat would result in:

low-power [balanced] performance

Where the [] indicate the active profile, vs having 2 sysfs attributes
one ro with space-separated available (foo_available) values and one
wr with the actual/current value. FWIW userspace folks have indicated
they prefer the solution with 2 separate sysfs-attributes and that is
also what e.g. cpufreq is currently using for governor selection.

I don't really have a strong opinion either way.

Regards,

Hans




^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-14 14:16                   ` Hans de Goede
@ 2020-10-14 15:46                     ` Rafael J. Wysocki
  2020-10-14 17:44                       ` Elia Devito
  0 siblings, 1 reply; 26+ messages in thread
From: Rafael J. Wysocki @ 2020-10-14 15:46 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Rafael J. Wysocki, Limonciello, Mario, Bastien Nocera,
	Darren Hart, Andy Shevchenko, Mark Gross, Mark Pearson,
	Elia Devito, Benjamin Berg, linux-pm, linux-acpi,
	platform-driver-x86, linux-kernel, Mark Pearson,
	Srinivas Pandruvada

On Wed, Oct 14, 2020 at 4:16 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 10/14/20 3:55 PM, Rafael J. Wysocki wrote:
> > On Tue, Oct 13, 2020 at 3:09 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >> Hi,
> >>
> >> On 10/12/20 6:42 PM, Rafael J. Wysocki wrote:
> >>> On Wed, Oct 7, 2020 at 8:41 PM Limonciello, Mario
> >>> <Mario.Limonciello@dell.com> wrote:
> >>>>
> >>>>> On Wed, 2020-10-07 at 15:58 +0000, Limonciello, Mario wrote:
> >>>>>>
> >>>>>>> On Mon, 2020-10-05 at 12:58 +0000, Limonciello, Mario wrote:
> >>>>>>>>> On modern systems CPU/GPU/... performance is often dynamically
> >>>>>>>>> configurable
> >>>>>>>>> in the form of e.g. variable clock-speeds and TPD. The
> >>>>>>>>> performance
> >>>>>>>>> is often
> >>>>>>>>> automatically adjusted to the load by some automatic-mechanism
> >>>>>>>>> (which may
> >>>>>>>>> very well live outside the kernel).
> >>>>>>>>>
> >>>>>>>>> These auto performance-adjustment mechanisms often can be
> >>>>>>>>> configured with
> >>>>>>>>> one of several performance-profiles, with either a bias towards
> >>>>>>>>> low-power
> >>>>>>>>> consumption (and cool and quiet) or towards performance (and
> >>>>>>>>> higher
> >>>>>>>>> power
> >>>>>>>>> consumption and thermals).
> >>>>>>>>>
> >>>>>>>>> Introduce a new performance_profile class/sysfs API which
> >>>>>>>>> offers a
> >>>>>>>>> generic
> >>>>>>>>> API for selecting the performance-profile of these automatic-
> >>>>>>>>> mechanisms.
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> If introducing an API for this - let me ask the question, why
> >>>>>>>> even let each
> >>>>>>>> driver offer a class interface and userspace need to change
> >>>>>>>> "each" driver's
> >>>>>>>> performance setting?
> >>>>>>>>
> >>>>>>>> I would think that you could just offer something kernel-wide
> >>>>>>>> like
> >>>>>>>> /sys/power/performance-profile
> >>>>>>>>
> >>>>>>>> Userspace can read and write to a single file.  All drivers can
> >>>>>>>> get notified
> >>>>>>>> on this sysfs file changing.
> >>>>>>>>
> >>>>>>>> The systems that react in firmware (such as the two that prompted
> >>>>>>>> this discussion) can change at that time.  It leaves the
> >>>>>>>> possibility for a
> >>>>>>>> more open kernel implementation that can do the same thing though
> >>>>>>>> too by
> >>>>>>>> directly modifying device registers instead of ACPI devices.
> >>>>>>>
> >>>>>>> The problem, as I've mentioned in previous discussions we had about
> >>>>>>> this, is that, as you've seen in replies to this mail, this would
> >>>>>>> suddenly be making the kernel apply policy.
> >>>>>>>
> >>>>>>> There's going to be pushback as soon as policy is enacted in the
> >>>>>>> kernel, and you take away the different knobs for individual
> >>>>>>> components
> >>>>>>> (or you can control them centrally as well as individually). As
> >>>>>>> much as
> >>>>>>> I hate the quantity of knobs[1], I don't think that trying to
> >>>>>>> reduce
> >>>>>>> the number of knobs in the kernel is a good use of our time, and
> >>>>>>> easier
> >>>>>>> to enact, coordinated with design targets, in user-space.
> >>>>>>>
> >>>>>>> Unless you can think of a way to implement this kernel wide setting
> >>>>>>> without adding one more exponent on the number of possibilities for
> >>>>>>> the
> >>>>>>> testing matrix, I'll +1 Hans' original API.
> >>>>>>>
> >>>>>> Actually I offered two proposals in my reply.  So are you NAKing
> >>>>>> both?
> >>>>>
> >>>>> No, this is only about the first portion of the email, which I quoted.
> >>>>> And I'm not NAK'ing it, but I don't see how it can work without being
> >>>>> antithetical to what kernel "users" expect, or what the folks consuming
> >>>>> those interfaces (presumably us both) would expect to be able to test
> >>>>> and maintain.
> >>>>>
> >>>>
> >>>> (Just so others are aware, Bastien and I had a previous discussion on this topic
> >>>> that he alluded to here: https://gitlab.freedesktop.org/hadess/power-profiles-daemon/-/issues/1)
> >>>>
> >>>> In general I agree that we shouldn't be offering 100's of knobs to change
> >>>> things and protect users from themselves where possible.
> >>>>
> >>>> Whether the decisions are made in the kernel or in userspace you still have a matrix once
> >>>> you're letting someone change 2 different kernel devices that offer policy.  I'd argue it's
> >>>> actually worse if you let userspace change it though.
> >>>>
> >>>> Let's go back to the my GPU and platform example and lets say both offer the new knob here
> >>>> for both.  Userspace software such as your PPD picks performance.  Both the platform device
> >>>> and GPU device get changed, hopefully no conflicts.
> >>>> Then user decides no, I don't want my GPU in performance mode, I only want my platform.
> >>>> So they change the knob for the GPU manually, and now you have a new config in your matrix.
> >>>>
> >>>> However if you left it to a single kernel knob, both GPU and platform get moved together and
> >>>> you don't have these extra configs in your matrix anymore.
> >>>>
> >>>> The other point I mentioned, that platform might also do something to GPU via a sideband and
> >>>> you race, you can solve it with kernel too by modifying the ordering the kernel handles it.
> >>>>
> >>>> Userspace however, you give two knobs and now you have to worry about them getting it right
> >>>> and supporting them doing them in the wrong order.
> >>>>
> >>>>>> The other one suggested to use the same firmware attributes class
> >>>>>> being
> >>>>>> introduced by the new Dell driver (
> >>>>>> https://patchwork.kernel.org/patch/11818343/)
> >>>>>> since this is actually a knob to a specific firmware setting.
> >>>>>
> >>>>> This seemed to me like an implementation detail (eg. the same metadata
> >>>>> is being exported, but in a different way), and I don't feel strongly
> >>>>> about it either way.
> >>>>
> >>>> OK thanks.
> >>>
> >>> IMV there are two choices here:  One is between exposing the low-level
> >>> interfaces verbatim to user space and wrapping them up into a certain
> >>> "translation" layer allowing user space to use a unified interface (I
> >>> think that is what everybody wants) and the other  boils down to how
> >>> the unified interface between the kernel and user space will look
> >>> like.
> >>>
> >>> Personally, I think that something line /sys/power/profile allowing
> >>> drivers (and other kernel entities) to register callbacks might work
> >>> (as stated in my last reply to Hans).
> >>
> >> Note to others reading along I pointed to this thread in this thread:
> >> https://lore.kernel.org/linux-pm/20201006122024.14539-1-daniel.lezcano@linaro.org/T/#t
> >> and Rafael's "last reply" above refers to his reply in that thread.
> >>
> >> For the sake of people reading along I'm reproducing my reply
> >> there below.
> >
> > For completeness, my response in the other thread is here:
> >
> > https://lore.kernel.org/linux-pm/CAJZ5v0jpYpu3Tk7qq_MCVs0wUr-Dw0rY5EZELrVbQta0NZaoVA@mail.gmail.com/T/#t
> >
> >> Rafael, it seems more appropriate to continue this discussion
> >> in this thread, so lets discuss this further here ?
> >
> > And because I sent it before reading this message, let me reproduce it
> > below (with some additions).
>
> And I just did the same thing (replied to your reply in the other thread),
> I guess I was too quick.

Well, same here.

> So I too will reproduce my reply here.

And so I'm doing below.

> And I still believe it is best to then stick to this thread from now on,
> because this reproducing thing is not really productive...
>
> >> My reply to Rafael from the other thread:
> >>
> >> First of all thank you for your input, with your expertise in this
> >> area your input is very much appreciated, after all we only get
> >> one chance to get the userspace API for this right.
> >>
> >> Your proposal to have a single sysfs file for userspace to talk
> >> to and then use an in kernel subscription mechanism for drivers
> >> to get notified of writes to this file is interesting.
> >>
> >> But I see 2 issues with it:
> >>
> >> 1. How will userspace know which profiles are actually available ?
> >>
> >> An obvious solution is to pick a set of standard names and let
> >> subscribers map those as close to their own settings as possible,
> >> the most often mentioned set of profile names in this case seems to be:
> >>
> >> low_power
> >> balanced_power
> >> balanced
> >> balanced_performance
> >> performance
> >>
> >> Which works fine for the thinkpad_acpi case, but not so much for
> >> the hp-wmi case. In the HP case what happens is that a WMI call
> >> is made which sets a bunch of ACPI variables which influence
> >> the DPTF code (this assumes we have some sort of DPTF support
> >> such as mjg59's reverse engineered support) but the profile-names
> >> under Windows are: "Performance", "HP recommended", "Cool" and
> >> "Quiet".  If you read the discussion from the
> >> "[RFC] Documentation: Add documentation for new performance_profile sysfs class"
> >> thread you will see this was brought up as an issue there.
> >
> > Two different things seem to be conflated here.  One is how to pass a
> > possible performance-vs-power preference coming from user space down
> > to device drivers or generally pieces of kernel code that can adjust
> > the behavior and/or hardware settings depending on what that
> > preference is and the other is how to expose OEM-provided DPTF system
> > profile interfaces to user space.
>
> I was hoping / thinking that we could use a single API for both of
> these. But I guess that it makes sense to see them as 2 separate
> things, esp. since DPTF profiles seem to be somewhat free-form
> where as a way to pass a performance-pref to a device could use
> a fixes set of values.
>
> So lets say that we indeed want to treat these 2 separately,
> then I guess that the issue at hand / my reason to start a
> discussion surrounding this is allowing userspace to selecting
> the DPTF system profile.
>
> The thinkpad_acpi case at hand is not using DPTF, but that is
> because Lenovo decided to implement dynamic DPTF like behavior
> inside their embedded controller (for when running Linux) since
> DPTF is atm not really supported all that well under Linux and
> Lenovo was getting a lot of complaints about sub-optimal
> performance because of this.
>
> So the thinkpad_acpi solution is in essence a replacement
> for DPTF and it should thus use the same userspace API as
> other mechanisms to select DPTF system profiles.
>
> And if we limit this new userspace API solely to setting DPTF
> system profiles, then their will indeed be only 1 provider for
> this for the entire system.
>
> > The former assumes that there is a common set of values that can be
> > understood and acted on in a consistent way by all of the interested
> > entities within the kernel and the latter is about passing information
> > from user space down to a side-band power control mechanism working in
> > its own way behind the kernel's back (and possibly poking at multiple
> > hardware components in the platform in its own way).
>
> Ack.
>
> > IMO there is no way to provide a common interface covering these two
> > cases at the same time.
>
> I see your point, esp. the free form vs common set of values
> argument seems to be exactly what we have been going in circles
> about during the discussion about this so far.
>
> >> The problem here is that both "cool" and "quiet" could be
> >> interpreted as low-power. But it seems that they actually mean
> >> what they say, cool focuses on keeping temps low, which can
> >> also be done by making the fan-profile more aggressive. And quiet
> >> is mostly about keeping fan speeds down, at the cost of possible
> >> higher temperatures.
> >>
> >> <edit in this version of the reply:>
> >> I wonder if the HP profiles are actually just fan speed profiles ?
> >> Elia do you know ?
> >> </edit>
> >
> > I don't think so.
> >
> > AFAICS, in both the Thinkpad and HP cases the profile covers the
> > entire platform, which in particular means that they cannot co-exist.
>
> Ok.
>
> >> IOW we don't really have a 1 dimensional axis.
> >
> > Well, AFAICS, DPTF system profile interfaces coming from different
> > OEMs will be different, but they are about side-band power control and
> > there can be only one thing like that in a platform at the same time.
>
> Ack.
>
> >> My class proposal fixes this by having a notion of both
> >> standardized names (because anything else would suck) combined
> >> with a way for drivers to advertise which standardized names
> >> the support. So in my proposal I simply add quiet and cool
> >> to the list of standard profile names, and then the HP-wmi
> >> driver can list those as supported, while not listing
> >> low_power as a supported profile.  This way we export the
> >> hardware interface to userspace as is (as much as possible)
> >> while still offering a standardized interface for userspace
> >> to consume.  Granted if userspace now actually want to set
> >> a low_power profile, we have just punted the problem to userspace
> >> but I really do not see a better solution.
> >
> > First, a common place to register a DPTF system profile seems to be
> > needed and, as I said above, I wouldn't expect more than one such
> > thing to be present in the system at any given time, so it may be
> > registered along with the list of supported profiles and user space
> > will have to understand what they mean.
>
> Mostly Ack, I would still like to have an enum for DPTF system
> profiles in the kernel and have a single piece of code map that
> enum to profile names. This enum can then be extended as
> necessary, but I want to avoid having one driver use
> "Performance" and the other "performance" or one using
> "performance-balanced" and the other "balanced-performance", etc.
>
> With the goal being that new drivers use existing values from
> the enum as much as possible, but we extend it where necessary.

IOW, just a table of known profile names with specific indices assigned to them.

This sounds reasonable.

> > Second, irrespective of the above, it may be useful to have a
> > consistent way to pass performance-vs-power preference information
> > from user space to different parts of the kernel so as to allow them
> > to adjust their operation and this could be done with a system-wide
> > power profile attribute IMO.
>
> I agree, which is why I tried to tackle both things in one go,
> but as you said doing both in 1 API is probably not the best idea.
> So I believe we should park this second issue for now and revisit it
> when we find a need for it.

Agreed.

> Do you have any specific userspace API in mind for the
> DPTF system profile selection?

Not really.

> And to get one thing out of the way, in the other thread we had some
> discussion about using a single attribute where a cat would result in:
>
> low-power [balanced] performance
>
> Where the [] indicate the active profile, vs having 2 sysfs attributes
> one ro with space-separated available (foo_available) values and one
> wr with the actual/current value. FWIW userspace folks have indicated
> they prefer the solution with 2 separate sysfs-attributes and that is
> also what e.g. cpufreq is currently using for governor selection.

Right.

It also uses that for the EPP "profiles" selection which kind of
belongs to the same broad category of settings.

> I don't really have a strong opinion either way.

Me neither. :-)

I guess from the perspective of a script accessing the interface it is
somewhat more straightforward to read what is available as a
white-space-separated list without the need for extra parsing.

Cheers!

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-14 15:46                     ` Rafael J. Wysocki
@ 2020-10-14 17:44                       ` Elia Devito
  2020-10-14 18:11                         ` Limonciello, Mario
  0 siblings, 1 reply; 26+ messages in thread
From: Elia Devito @ 2020-10-14 17:44 UTC (permalink / raw)
  To: Hans de Goede, Rafael J. Wysocki
  Cc: Rafael J. Wysocki, Limonciello, Mario, Bastien Nocera,
	Darren Hart, Andy Shevchenko, Mark Gross, Mark Pearson,
	Benjamin Berg, linux-pm, linux-acpi, platform-driver-x86,
	linux-kernel, Mark Pearson, Srinivas Pandruvada

Hi,

In data mercoledì 14 ottobre 2020 17:46:43 CEST, Rafael J. Wysocki ha scritto:
> On Wed, Oct 14, 2020 at 4:16 PM Hans de Goede <hdegoede@redhat.com> wrote:
> > Hi,
> > 
> > On 10/14/20 3:55 PM, Rafael J. Wysocki wrote:
> > > On Tue, Oct 13, 2020 at 3:09 PM Hans de Goede <hdegoede@redhat.com> 
wrote:
> > >> Hi,
> > >> 
> > >> On 10/12/20 6:42 PM, Rafael J. Wysocki wrote:
> > >>> On Wed, Oct 7, 2020 at 8:41 PM Limonciello, Mario
> > >>> 
> > >>> <Mario.Limonciello@dell.com> wrote:
> > >>>>> On Wed, 2020-10-07 at 15:58 +0000, Limonciello, Mario wrote:
> > >>>>>>> On Mon, 2020-10-05 at 12:58 +0000, Limonciello, Mario wrote:
> > >>>>>>>>> On modern systems CPU/GPU/... performance is often dynamically
> > >>>>>>>>> configurable
> > >>>>>>>>> in the form of e.g. variable clock-speeds and TPD. The
> > >>>>>>>>> performance
> > >>>>>>>>> is often
> > >>>>>>>>> automatically adjusted to the load by some automatic-mechanism
> > >>>>>>>>> (which may
> > >>>>>>>>> very well live outside the kernel).
> > >>>>>>>>> 
> > >>>>>>>>> These auto performance-adjustment mechanisms often can be
> > >>>>>>>>> configured with
> > >>>>>>>>> one of several performance-profiles, with either a bias towards
> > >>>>>>>>> low-power
> > >>>>>>>>> consumption (and cool and quiet) or towards performance (and
> > >>>>>>>>> higher
> > >>>>>>>>> power
> > >>>>>>>>> consumption and thermals).
> > >>>>>>>>> 
> > >>>>>>>>> Introduce a new performance_profile class/sysfs API which
> > >>>>>>>>> offers a
> > >>>>>>>>> generic
> > >>>>>>>>> API for selecting the performance-profile of these automatic-
> > >>>>>>>>> mechanisms.
> > >>>>>>>> 
> > >>>>>>>> If introducing an API for this - let me ask the question, why
> > >>>>>>>> even let each
> > >>>>>>>> driver offer a class interface and userspace need to change
> > >>>>>>>> "each" driver's
> > >>>>>>>> performance setting?
> > >>>>>>>> 
> > >>>>>>>> I would think that you could just offer something kernel-wide
> > >>>>>>>> like
> > >>>>>>>> /sys/power/performance-profile
> > >>>>>>>> 
> > >>>>>>>> Userspace can read and write to a single file.  All drivers can
> > >>>>>>>> get notified
> > >>>>>>>> on this sysfs file changing.
> > >>>>>>>> 
> > >>>>>>>> The systems that react in firmware (such as the two that prompted
> > >>>>>>>> this discussion) can change at that time.  It leaves the
> > >>>>>>>> possibility for a
> > >>>>>>>> more open kernel implementation that can do the same thing though
> > >>>>>>>> too by
> > >>>>>>>> directly modifying device registers instead of ACPI devices.
> > >>>>>>> 
> > >>>>>>> The problem, as I've mentioned in previous discussions we had
> > >>>>>>> about
> > >>>>>>> this, is that, as you've seen in replies to this mail, this would
> > >>>>>>> suddenly be making the kernel apply policy.
> > >>>>>>> 
> > >>>>>>> There's going to be pushback as soon as policy is enacted in the
> > >>>>>>> kernel, and you take away the different knobs for individual
> > >>>>>>> components
> > >>>>>>> (or you can control them centrally as well as individually). As
> > >>>>>>> much as
> > >>>>>>> I hate the quantity of knobs[1], I don't think that trying to
> > >>>>>>> reduce
> > >>>>>>> the number of knobs in the kernel is a good use of our time, and
> > >>>>>>> easier
> > >>>>>>> to enact, coordinated with design targets, in user-space.
> > >>>>>>> 
> > >>>>>>> Unless you can think of a way to implement this kernel wide
> > >>>>>>> setting
> > >>>>>>> without adding one more exponent on the number of possibilities
> > >>>>>>> for
> > >>>>>>> the
> > >>>>>>> testing matrix, I'll +1 Hans' original API.
> > >>>>>> 
> > >>>>>> Actually I offered two proposals in my reply.  So are you NAKing
> > >>>>>> both?
> > >>>>> 
> > >>>>> No, this is only about the first portion of the email, which I
> > >>>>> quoted.
> > >>>>> And I'm not NAK'ing it, but I don't see how it can work without
> > >>>>> being
> > >>>>> antithetical to what kernel "users" expect, or what the folks
> > >>>>> consuming
> > >>>>> those interfaces (presumably us both) would expect to be able to
> > >>>>> test
> > >>>>> and maintain.
> > >>>> 
> > >>>> (Just so others are aware, Bastien and I had a previous discussion on
> > >>>> this topic that he alluded to here:
> > >>>> https://gitlab.freedesktop.org/hadess/power-profiles-daemon/-/issues
> > >>>> /1)
> > >>>> 
> > >>>> In general I agree that we shouldn't be offering 100's of knobs to
> > >>>> change
> > >>>> things and protect users from themselves where possible.
> > >>>> 
> > >>>> Whether the decisions are made in the kernel or in userspace you
> > >>>> still have a matrix once you're letting someone change 2 different
> > >>>> kernel devices that offer policy.  I'd argue it's actually worse if
> > >>>> you let userspace change it though.
> > >>>> 
> > >>>> Let's go back to the my GPU and platform example and lets say both
> > >>>> offer the new knob here for both.  Userspace software such as your
> > >>>> PPD picks performance.  Both the platform device and GPU device get
> > >>>> changed, hopefully no conflicts.
> > >>>> Then user decides no, I don't want my GPU in performance mode, I only
> > >>>> want my platform. So they change the knob for the GPU manually, and
> > >>>> now you have a new config in your matrix.
> > >>>> 
> > >>>> However if you left it to a single kernel knob, both GPU and platform
> > >>>> get moved together and you don't have these extra configs in your
> > >>>> matrix anymore.
> > >>>> 
> > >>>> The other point I mentioned, that platform might also do something to
> > >>>> GPU via a sideband and you race, you can solve it with kernel too by
> > >>>> modifying the ordering the kernel handles it.
> > >>>> 
> > >>>> Userspace however, you give two knobs and now you have to worry about
> > >>>> them getting it right and supporting them doing them in the wrong
> > >>>> order.
> > >>>> 
> > >>>>>> The other one suggested to use the same firmware attributes class
> > >>>>>> being
> > >>>>>> introduced by the new Dell driver (
> > >>>>>> https://patchwork.kernel.org/patch/11818343/)
> > >>>>>> since this is actually a knob to a specific firmware setting.
> > >>>>> 
> > >>>>> This seemed to me like an implementation detail (eg. the same
> > >>>>> metadata
> > >>>>> is being exported, but in a different way), and I don't feel
> > >>>>> strongly
> > >>>>> about it either way.
> > >>>> 
> > >>>> OK thanks.
> > >>> 
> > >>> IMV there are two choices here:  One is between exposing the low-level
> > >>> interfaces verbatim to user space and wrapping them up into a certain
> > >>> "translation" layer allowing user space to use a unified interface (I
> > >>> think that is what everybody wants) and the other  boils down to how
> > >>> the unified interface between the kernel and user space will look
> > >>> like.
> > >>> 
> > >>> Personally, I think that something line /sys/power/profile allowing
> > >>> drivers (and other kernel entities) to register callbacks might work
> > >>> (as stated in my last reply to Hans).
> > >> 
> > >> Note to others reading along I pointed to this thread in this thread:
> > >> https://lore.kernel.org/linux-pm/20201006122024.14539-1-daniel.lezcano@
> > >> linaro.org/T/#t and Rafael's "last reply" above refers to his reply in
> > >> that thread.
> > >> 
> > >> For the sake of people reading along I'm reproducing my reply
> > >> there below.
> > > 
> > > For completeness, my response in the other thread is here:
> > > 
> > > https://lore.kernel.org/linux-pm/CAJZ5v0jpYpu3Tk7qq_MCVs0wUr-Dw0rY5EZELr
> > > VbQta0NZaoVA@mail.gmail.com/T/#t> > 
> > >> Rafael, it seems more appropriate to continue this discussion
> > >> in this thread, so lets discuss this further here ?
> > > 
> > > And because I sent it before reading this message, let me reproduce it
> > > below (with some additions).
> > 
> > And I just did the same thing (replied to your reply in the other thread),
> > I guess I was too quick.
> 
> Well, same here.
> 
> > So I too will reproduce my reply here.
> 
> And so I'm doing below.
> 
> > And I still believe it is best to then stick to this thread from now on,
> > because this reproducing thing is not really productive...
> > 
> > >> My reply to Rafael from the other thread:
> > >> 
> > >> First of all thank you for your input, with your expertise in this
> > >> area your input is very much appreciated, after all we only get
> > >> one chance to get the userspace API for this right.
> > >> 
> > >> Your proposal to have a single sysfs file for userspace to talk
> > >> to and then use an in kernel subscription mechanism for drivers
> > >> to get notified of writes to this file is interesting.
> > >> 
> > >> But I see 2 issues with it:
> > >> 
> > >> 1. How will userspace know which profiles are actually available ?
> > >> 
> > >> An obvious solution is to pick a set of standard names and let
> > >> subscribers map those as close to their own settings as possible,
> > >> the most often mentioned set of profile names in this case seems to be:
> > >> 
> > >> low_power
> > >> balanced_power
> > >> balanced
> > >> balanced_performance
> > >> performance
> > >> 
> > >> Which works fine for the thinkpad_acpi case, but not so much for
> > >> the hp-wmi case. In the HP case what happens is that a WMI call
> > >> is made which sets a bunch of ACPI variables which influence
> > >> the DPTF code (this assumes we have some sort of DPTF support
> > >> such as mjg59's reverse engineered support) but the profile-names
> > >> under Windows are: "Performance", "HP recommended", "Cool" and
> > >> "Quiet".  If you read the discussion from the
> > >> "[RFC] Documentation: Add documentation for new performance_profile
> > >> sysfs class" thread you will see this was brought up as an issue
> > >> there.
> > > 
> > > Two different things seem to be conflated here.  One is how to pass a
> > > possible performance-vs-power preference coming from user space down
> > > to device drivers or generally pieces of kernel code that can adjust
> > > the behavior and/or hardware settings depending on what that
> > > preference is and the other is how to expose OEM-provided DPTF system
> > > profile interfaces to user space.
> > 
> > I was hoping / thinking that we could use a single API for both of
> > these. But I guess that it makes sense to see them as 2 separate
> > things, esp. since DPTF profiles seem to be somewhat free-form
> > where as a way to pass a performance-pref to a device could use
> > a fixes set of values.
> > 
> > So lets say that we indeed want to treat these 2 separately,
> > then I guess that the issue at hand / my reason to start a
> > discussion surrounding this is allowing userspace to selecting
> > the DPTF system profile.
> > 
> > The thinkpad_acpi case at hand is not using DPTF, but that is
> > because Lenovo decided to implement dynamic DPTF like behavior
> > inside their embedded controller (for when running Linux) since
> > DPTF is atm not really supported all that well under Linux and
> > Lenovo was getting a lot of complaints about sub-optimal
> > performance because of this.
> > 
> > So the thinkpad_acpi solution is in essence a replacement
> > for DPTF and it should thus use the same userspace API as
> > other mechanisms to select DPTF system profiles.
> > 
> > And if we limit this new userspace API solely to setting DPTF
> > system profiles, then their will indeed be only 1 provider for
> > this for the entire system.
> > 
> > > The former assumes that there is a common set of values that can be
> > > understood and acted on in a consistent way by all of the interested
> > > entities within the kernel and the latter is about passing information
> > > from user space down to a side-band power control mechanism working in
> > > its own way behind the kernel's back (and possibly poking at multiple
> > > hardware components in the platform in its own way).
> > 
> > Ack.
> > 
> > > IMO there is no way to provide a common interface covering these two
> > > cases at the same time.
> > 
> > I see your point, esp. the free form vs common set of values
> > argument seems to be exactly what we have been going in circles
> > about during the discussion about this so far.
> > 
> > >> The problem here is that both "cool" and "quiet" could be
> > >> interpreted as low-power. But it seems that they actually mean
> > >> what they say, cool focuses on keeping temps low, which can
> > >> also be done by making the fan-profile more aggressive. And quiet
> > >> is mostly about keeping fan speeds down, at the cost of possible
> > >> higher temperatures.
> > >> 
> > >> <edit in this version of the reply:>
> > >> I wonder if the HP profiles are actually just fan speed profiles ?
> > >> Elia do you know ?
> > >> </edit>
> > > 
> > > I don't think so.
> > > 
> > > AFAICS, in both the Thinkpad and HP cases the profile covers the
> > > entire platform, which in particular means that they cannot co-exist.
> > 
> > Ok.
> > 

As far as I know, the profiles affect  the thermal behavior like "how long to 
wait before starting the fan and at what temperature" or "how fast to run the 
fan with the current cpu load and temperature".

The only way that firmware uses to "control" performance should be the odvp0 
DPTF variable.

On Windows HP expose  both "Cool" and "Quiet" profile respectively as "Ideal 
for when the computer feels warm to the touch" and "Ideal for quiet 
environments".

more detail: https://support.hp.com/in-en/document/c06063108

To be precise "Quiet" profile should be available only on platform without 
dedicate GPU (I'm investigating if there is other case), instead the other 3 
profiles ("HP Recommended", "Performance" and "Cool") are available on all 
platform that support thermal profile.

reading here seems that Dell offer identical profiles:
https://www.dell.com/support/manuals/it/it/itbsdt1/dell-command-power-manager-v2.2/userguide_dell/thermal-management?guid=guid-c05d2582-fc07-4e3e-918a-965836d20752&lang=en-us 

Regards,

Elia



^ permalink raw reply	[flat|nested] 26+ messages in thread

* RE: [RFC] Documentation: Add documentation for new performance_profile sysfs class
  2020-10-14 17:44                       ` Elia Devito
@ 2020-10-14 18:11                         ` Limonciello, Mario
  0 siblings, 0 replies; 26+ messages in thread
From: Limonciello, Mario @ 2020-10-14 18:11 UTC (permalink / raw)
  To: Elia Devito, Hans de Goede, Rafael J. Wysocki
  Cc: Rafael J. Wysocki, Bastien Nocera, Darren Hart, Andy Shevchenko,
	Mark Gross, Mark Pearson, Benjamin Berg, linux-pm, linux-acpi,
	platform-driver-x86, linux-kernel, Mark Pearson,
	Srinivas Pandruvada

> As far as I know, the profiles affect  the thermal behavior like "how long to
> wait before starting the fan and at what temperature" or "how fast to run the
> fan with the current cpu load and temperature".
> 
> The only way that firmware uses to "control" performance should be the odvp0
> DPTF variable.
> 
> On Windows HP expose  both "Cool" and "Quiet" profile respectively as "Ideal
> for when the computer feels warm to the touch" and "Ideal for quiet
> environments".
> 
> more detail: https://support.hp.com/in-en/document/c06063108
> 
> To be precise "Quiet" profile should be available only on platform without
> dedicate GPU (I'm investigating if there is other case), instead the other 3
> profiles ("HP Recommended", "Performance" and "Cool") are available on all
> platform that support thermal profile.
> 
> reading here seems that Dell offer identical profiles:
> https://www.dell.com/support/manuals/it/it/itbsdt1/dell-command-power-manager-
> v2.2/userguide_dell/thermal-management?guid=guid-c05d2582-fc07-4e3e-918a-
> 965836d20752&lang=en-us
> 

The Dell profiles here I don't think should be conflated with DTT/DPTF though.
These won't affect anything in the DTT vault.

Those profiles are already accessible from Linux userland, there is already a
command line tool in libsmbios that can do it.

$ sudo smbios-thermal-ctl -i
Libsmbios version : 2.4.3
smbios-thermal-ctl version : 2.4.3

 Print all the Available Thermal Information of your system:
-------------------------------------------------------------------

Supported Thermal Modes:
         Balanced
         Cool Bottom
         Quiet
         Performance

Supported Active Acoustic Controller (AAC) modes:

Supported AAC Configuration type:
        Global (AAC enable/disable applies to all supported USTT modes)\

$ sudo smbios-thermal-ctl -g
Helper function to Get current Thermal Mode settings

 Print Current Status of Thermal Information:
-------------------------------------------------------------------

Current Thermal Modes:
         Balanced

Current Active Acoustic Controller (AAC) Mode:
         AAC mode Disabled

Current Active Acoustic Controller (AAC) Mode:
        Global (AAC enable/disable applies to all supported USTT modes)

---
*If* we choose to export them for Dell from the kernel, we should probably also
filter from kernel side and modify the existing userspace tool to use the kernel
API when possible.

I'm not convinced that Dell would want to tie into this proposed knob with those
settings because I expect that existing software like thermald will not work as
efficiently.


^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2020-10-14 18:12 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-03 13:19 [RFC 0/1] Documentation: Add documentation for new performance_profile sysfs class Hans de Goede
2020-10-03 13:19 ` [RFC] " Hans de Goede
2020-10-04  1:33   ` [External] " Mark Pearson
2020-10-04 22:29   ` Elia Devito
2020-10-09 10:52     ` Hans de Goede
2020-10-05 12:58   ` Limonciello, Mario
2020-10-05 14:19     ` Barnabás Pőcze
2020-10-05 16:11       ` Limonciello, Mario
2020-10-05 16:47         ` [External] " Mark Pearson
2020-10-05 16:56           ` Limonciello, Mario
2020-10-05 17:46             ` Mark Pearson
2020-10-07 11:51     ` Bastien Nocera
2020-10-07 15:58       ` Limonciello, Mario
2020-10-07 16:34         ` Bastien Nocera
2020-10-07 18:41           ` Limonciello, Mario
2020-10-12 16:42             ` Rafael J. Wysocki
2020-10-13 13:09               ` Hans de Goede
2020-10-14 13:55                 ` Rafael J. Wysocki
2020-10-14 14:16                   ` Hans de Goede
2020-10-14 15:46                     ` Rafael J. Wysocki
2020-10-14 17:44                       ` Elia Devito
2020-10-14 18:11                         ` Limonciello, Mario
2020-10-09 11:33     ` Hans de Goede
2020-10-05 13:13   ` Benjamin Berg
2020-10-09 11:15     ` Hans de Goede
2020-10-03 13:39 ` [RFC 0/1] " Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.