[PATCH] Documentation: Add documentation for new platform_profile sysfs attribute

[PATCH] Documentation: Add documentation for new platform_profile sysfs attribute

[Mark Pearson]

From: Hans de Goede <hdegoede@redhat.com>

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
consumption or towards performance (and higher power consumption and
thermals).

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@lenovo.com>
Signed-off-by: Mark Pearson <markpearson@lenovo.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
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
+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 name.
+
+What:		/sys/firmware/acpi/platform_profile_choices
+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:
+
+		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)
+
+		Userspace may expect drivers to offer at least several of these
+		standard profile-names.
+
+What:		/sys/firmware/acpi/platform_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.
-- 
2.28.0

Re: [PATCH] Documentation: Add documentation for new platform_profile sysfs attribute

[Elia Devito]

Hi to all,

In data martedì 27 ottobre 2020 17:42:19 CET, Mark Pearson ha scritto:
> From: Hans de Goede <hdegoede@redhat.com>
> 
> 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
> consumption or towards performance (and higher power consumption and
> thermals).
> 
> 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@lenovo.com>
> Signed-off-by: Mark Pearson <markpearson@lenovo.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> 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
> +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 name.
> +
> +What:		/sys/firmware/acpi/platform_profile_choices
> +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:
> +
> +		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)
> +
> +		Userspace may expect drivers to offer at least several of 
these
> +		standard profile-names.
> +
> +What:		/sys/firmware/acpi/platform_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.
> --
> 2.28.0

From my perspective now is perfect, thanks to all for the work.

Regards
Elia

Re: [PATCH] Documentation: Add documentation for new platform_profile sysfs attribute

[Hans de Goede]

Hi,

A few minor nitpicks below, mostly stuff which I missed before, sorry.

I suggest you make v2 part of the series where you actually add the
drivers/acpi/... and the thinkpad_acpi.c bits to implement this.

On 10/27/20 5:42 PM, Mark Pearson wrote:
> From: Hans de Goede <hdegoede@redhat.com>
> 
> 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

s/somei/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).

I think it is better to also drop the " (and higher power consumption and
thermals)" bit here (and below) like you did for the cool and quiet parts.

Regards,

Hans

> 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@lenovo.com>
> Signed-off-by: Mark Pearson <markpearson@lenovo.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> 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
> +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 name.
> +
> +What:		/sys/firmware/acpi/platform_profile_choices
> +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:
> +
> +		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)
> +
> +		Userspace may expect drivers to offer at least several of these
> +		standard profile-names.
> +
> +What:		/sys/firmware/acpi/platform_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.
> 

Re: [PATCH] Documentation: Add documentation for new platform_profile sysfs attribute

[Bastien Nocera]

Hey Hans, Mark,

On Tue, 2020-10-27 at 12:42 -0400, Mark Pearson wrote:
> From: Hans de Goede <hdegoede@redhat.com>
> 
> 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@lenovo.com>
> Signed-off-by: Mark Pearson <markpearson@lenovo.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> 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?

> +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@redhat.com>
> +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@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.

Is there another file which explains whether those sysfs value will
contain a trailing linefeed?

Cheers

Re: [PATCH] Documentation: Add documentation for new platform_profile sysfs attribute

[Hans de Goede]

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@redhat.com>
>>
>> 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@lenovo.com>
>> Signed-off-by: Mark Pearson <markpearson@lenovo.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> 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@redhat.com>
>> +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@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.
> 
> 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

Re: [External] Re: [PATCH] Documentation: Add documentation for new platform_profile sysfs attribute

[Mark Pearson]

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@redhat.com>
>>>
>>> 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@lenovo.com>
>>> Signed-off-by: Mark Pearson <markpearson@lenovo.com>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>> 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@redhat.com>
>>> +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@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.
>>
>> 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

Re: [External] Re: [PATCH] Documentation: Add documentation for new platform_profile sysfs attribute

[Hans de Goede]

Hi,

On 10/29/20 1:55 AM, Mark Pearson wrote:
> Thanks Hans and Bastien,
> 
> On 28/10/2020 13:23, Hans de Goede wrote:

<big snip>

>>> 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?

IMHO it does not belong in the sysfs API docs for the platform_profile
stuff. But I guess it would be good to document it somewhere in some
generic syfs API rules/expectations document (with a note that their
might be exceptions).

Ideally we would already have such a file somewhere, but I don't know
if we do (I did not look). So if you feel like it (and such a file does
not exist yet) then I guess a patch adding such a doc file would be good.

Regards,

Hans

Re: [PATCH] Documentation: Add documentation for new platform_profile sysfs attribute

[Rafael J. Wysocki]

On Thu, Oct 29, 2020 at 2:53 AM Bastien Nocera <hadess@hadess.net> wrote:
>
> Hey Hans, Mark,
>
> On Tue, 2020-10-27 at 12:42 -0400, Mark Pearson wrote:
> > From: Hans de Goede <hdegoede@redhat.com>
> >
> > 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

Strictly speaking, power is a rate, so it cannot be consumed.

So I would say "low-power operation" here.

> 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

And analogously here.

> > thermals).
>
> s/thermal/temperature/

Right.

> "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@lenovo.com>
> > Signed-off-by: Mark Pearson <markpearson@lenovo.com>
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > ---
> > 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 just is not useful. :-)

> > +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.

Right.

> >  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@redhat.com>
> > +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

Let's be precise, so "low-power operation", please (see above for the reason).

> > +               cool:                   Emphasises cooler operation
> > +               quiet:                  Emphasises quieter operation
> > +               balanced:               Balance between low power
> > consumption

And same here and analogously everywhere.

> > +                                       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@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.
>
> Is there another file which explains whether those sysfs value will
> contain a trailing linefeed?
>
> Cheers
>

Re: [PATCH] Documentation: Add documentation for new platform_profile sysfs attribute

[Bastien Nocera]

On Wed, 2020-10-28 at 18:23 +0100, Hans de Goede wrote:
> 
> > 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.

What do you think of:

> +Since numbers are a rather meaningless way to describe platform-
profiles

"Since numbers on their own cannot represent the multiple variables
that a profile will adjust (power consumption, heat generation, etc.)
..."

> +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.

Re: [External] Re: [PATCH] Documentation: Add documentation for new platform_profile sysfs attribute

[Bastien Nocera]

On Thu, 2020-10-29 at 10:46 +0100, Hans de Goede wrote:
> <
> <snip>

> IMHO it does not belong in the sysfs API docs for the
> platform_profile
> stuff. But I guess it would be good to document it somewhere in some
> generic syfs API rules/expectations document (with a note that their
> might be exceptions).
> 
> Ideally we would already have such a file somewhere, but I don't know
> if we do (I did not look). So if you feel like it (and such a file
> does
> not exist yet) then I guess a patch adding such a doc file would be
> good.

I don't know enough about the helpers and the code around it to know
whether documenting this would be needed, but I'm fine with knowing
that we're not breaking new ground here.