diff mbox series

Documentation: Add documentation for new platform_profile sysfs attribute

Message ID 20201027164219.868839-1-markpearson@lenovo.com (mailing list archive)
State Changes Requested, archived
Headers show
Series Documentation: Add documentation for new platform_profile sysfs attribute | expand

Commit Message

Mark Pearson Oct. 27, 2020, 4:42 p.m. UTC
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

Comments

Elia Devito Oct. 27, 2020, 6:11 p.m. UTC | #1
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
Hans de Goede Oct. 28, 2020, 11:54 a.m. UTC | #2
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.
>
Bastien Nocera Oct. 28, 2020, 1:45 p.m. UTC | #3
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
Hans de Goede Oct. 28, 2020, 5:23 p.m. UTC | #4
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
Mark Pearson Oct. 29, 2020, 12:55 a.m. UTC | #5
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
Hans de Goede Oct. 29, 2020, 9:46 a.m. UTC | #6
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
Rafael J. Wysocki Oct. 29, 2020, 11:25 a.m. UTC | #7
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
>
Bastien Nocera Oct. 29, 2020, 12:33 p.m. UTC | #8
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.
Bastien Nocera Oct. 29, 2020, 2:21 p.m. UTC | #9
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.
diff mbox series

Patch

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.