Re: [External] Re: [PATCH] Documentation: Add documentation for new platform_profile sysfs attribute
From: Mark Pearson
Date: Wed Oct 28 2020 - 20:55:30 EST
Thanks Hans and Bastien,
On 28/10/2020 13:23, Hans de Goede wrote:
Hi,
On 10/28/20 2:45 PM, Bastien Nocera wrote:
Hey Hans, Mark,
On Tue, 2020-10-27 at 12:42 -0400, Mark Pearson wrote:
From: Hans de Goede <hdegoede@xxxxxxxxxx>
On modern systems the platform performance, temperature, fan and
other
hardware related characteristics are often dynamically configurable.
The
profile is often automatically adjusted to the load by somei
automatic-mechanism (which may very well live outside the kernel).
These auto platform-adjustment mechanisms often can be configured
with
one of several 'platform-profiles', with either a bias towards low-
power
Can you please make sure to quote 'platform-profile' and 'profile-name'
this way all through the document? They're not existing words, and
quoting them shows that they're attribute names, rather than English.
I'm leaning towards changing these to become "platform profile" and
"profile name" (no quotes in the actual text). Any objections?
consumption or towards performance (and higher power consumption and
thermals).
s/thermal/temperature/
"A thermal" is something else (it's seasonal underwear for me ;)
I'm removing that sentence from an earlier review so it's moot, but
enjoy your underwear! (which reminds me that I need a new set of
thermals for the winter...)
Introduce a new platform_profile sysfs API which offers a generic API
for
selecting the performance-profile of these automatic-mechanisms.
Co-developed-by: Mark Pearson <markpearson@xxxxxxxxxx>
Signed-off-by: Mark Pearson <markpearson@xxxxxxxxxx>
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
Changes in V1:
- Moved from RFC to proposed patch
- Added cool profile as requested
- removed extra-profiles as no longer relevant
.../ABI/testing/sysfs-platform_profile | 66
+++++++++++++++++++
1 file changed, 66 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-platform_profile
diff --git a/Documentation/ABI/testing/sysfs-platform_profile
b/Documentation/ABI/testing/sysfs-platform_profile
new file mode 100644
index 000000000000..240bd3d7532b
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform_profile
@@ -0,0 +1,66 @@
+Platform-profile selection (e.g.
/sys/firmware/acpi/platform_profile)
+
+On modern systems the platform performance, temperature, fan and
other
+hardware related characteristics are often dynamically configurable.
The
+profile is often automatically adjusted to the load by some
+automatic-mechanism (which may very well live outside the kernel).
+
+These auto platform-adjustment mechanisms often can be configured
with
+one of several 'platform-profiles', with either a bias towards low-
power
+consumption or towards performance (and higher power consumption and
+thermals).
+
+The purpose of the platform_profile attribute is to offer a generic
sysfs
+API for selecting the platform-profile of these automatic-
mechanisms.
+
+Note that this API is only for selecting the platform-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 platform-
profiles
It's not meaningless, but rather ambiguous. For a range of 1 to 5, is 1
high performance, and 5 low power, or vice-versa?
It is meaningless because the space we are trying to describe with the
profile-names is not 1 dimensional. E.g. as discussed before cool and
low-power are not necessarily the same thing. If you have a better way
to word this I'm definitely in favor of improving the text here.
I'm good with 'ambiguous' here as numbers are (interestingly) ambiguous.
I've not thought of anything better
Any objections?
+this API uses strings to describe the various profiles. To make sure
that
+userspace gets a consistent experience when using this API this API
you can remove "when using this API".
Ack
+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.
"for some reason" can be removed.
Ack
Drivers which wish to introduce new profile-names must:
+1. Have very good reasons to do so.
"1. Explain why the existing 'profile-names' cannot be used"
+2. Add the new profile-name to this document, so that future drivers
which also
+ have a similar problem can use the same name.
"2. Add the new 'profile-name' to the documentation so that other
drivers can use it, as well as user-space knowing clearly what
behaviour the 'profile-name' corresponds to"
How about just :
"2. Add the new profile name, along with a clear description of the
behaviour, to the documentation."
It should be clear for all 'consumers' - regardless of origin
+
+What: /sys/firmware/acpi/platform_profile_choices
+Date: October 2020
+Contact: Hans de Goede <hdegoede@xxxxxxxxxx>
+Description:
+ Reading this file gives a space separated list of
profiles
+ supported for this device.
"This file contains a space-separated list of profiles..."
Ack
+
+ Drivers must use the following standard profile-
names:
+
+ low-power: Emphasises low power
consumption
+ cool: Emphasises cooler operation
+ quiet: Emphasises quieter operation
+ balanced: Balance between low power
consumption
+ and performance
+ performance: Emphasises performance (and
may lead to
+ higher temperatures and fan
speeds)
I'd replace "Emphasises" with either "Focus on" or the US English
spelling of "Emphasizes".
Darn - Google confirms that Emphasizes is more correct now. For some
reason that's slightly disappointing :)
Ack.
+ Userspace may expect drivers to offer at least
several of these
+ standard profile-names.
Replce "at least several" with "more than one".
Ack
+
+What: /sys/firmware/acpi/platform_profile
+Date: October 2020
+Contact: Hans de Goede <hdegoede@xxxxxxxxxx>
+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.
Is there another file which explains whether those sysfs value will
contain a trailing linefeed?
sysfs APIs are typically created so that they can be used from the shell,
so on read a newline will be added. On write a newline at the end
typically is allowed, but ignored. There are even special helper functions
to deal with properly ignoring the newline on write.
Regards,
Hans
OK - does that need to actually be specified here? Or is that just
something I keep in mind for the implementation?
Mark