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. > >> consumption or towards performance (and higher power consumption and >> thermals). > > s/thermal/temperature/ > > "A thermal" is something else (it's seasonal underwear for me ;) > >> 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. > >> +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". > >> +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. > >> 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" > >> + >> +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..." > >> + >> + 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". > >> + Userspace may expect drivers to offer at least >> several of these >> + standard profile-names. > > Replce "at least several" with "more than one". > >> + >> +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
