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