linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class (Also Re: [PATCH 0/4] powercap/dtpm: Add the DTPM framework)
       [not found]               ` <CAJZ5v0iWmmu5WV7cX7uNb61NMYQ7s0dnhg1K+T0x90b3sBfU9w@mail.gmail.com>
@ 2020-10-16 11:10                 ` Hans de Goede
  2020-10-16 14:26                   ` Elia Devito
  2020-10-16 14:51                   ` Rafael J. Wysocki
  0 siblings, 2 replies; 11+ messages in thread
From: Hans de Goede @ 2020-10-16 11:10 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Daniel Lezcano, Srinivas Pandruvada, Lukasz Luba,
	Linux Kernel Mailing List, Linux PM, Zhang, Rui, Bastien Nocera,
	Mark Pearson, Limonciello, Mario, Darren Hart, Andy Shevchenko,
	Mark Gross, Elia Devito, Benjamin Berg, linux-acpi,
	platform-driver-x86

<note folding the 2 threads we are having on this into one, adding every one from both threads to the Cc>

Hi,

On 10/14/20 5:42 PM, Rafael J. Wysocki wrote:
> On Wed, Oct 14, 2020 at 4:06 PM Hans de Goede <hdegoede@redhat.com> wrote:
>> On 10/14/20 3:33 PM, Rafael J. Wysocki wrote:

<snip>

>>> First, a common place to register a DPTF system profile seems to be
>>> needed and, as I said above, I wouldn't expect more than one such
>>> thing to be present in the system at any given time, so it may be
>>> registered along with the list of supported profiles and user space
>>> will have to understand what they mean.
>>
>> Mostly Ack, I would still like to have an enum for DPTF system
>> profiles in the kernel and have a single piece of code map that
>> enum to profile names. This enum can then be extended as
>> necessary, but I want to avoid having one driver use
>> "Performance" and the other "performance" or one using
>> "performance-balanced" and the other "balanced-performance", etc.
>>
>> With the goal being that new drivers use existing values from
>> the enum as much as possible, but we extend it where necessary.
> 
> IOW, just a table of known profile names with specific indices assigned to them.

Yes.

> This sounds reasonable.
> 
>>> Second, irrespective of the above, it may be useful to have a
>>> consistent way to pass performance-vs-power preference information
>>> from user space to different parts of the kernel so as to allow them
>>> to adjust their operation and this could be done with a system-wide
>>> power profile attribute IMO.
>>
>> I agree, which is why I tried to tackle both things in one go,
>> but as you said doing both in 1 API is probably not the best idea.
>> So I believe we should park this second issue for now and revisit it
>> when we find a need for it.
> 
> Agreed.
> 
>> Do you have any specific userspace API in mind for the
>> DPTF system profile selection?
> 
> Not really.

So before /sys/power/profile was mentioned, but that seems more like
a thing which should have a set of fixed possible values, iow that is
out of scope for this discussion.

Since we all seem to agree that this is something which we need
specifically for DPTF profiles maybe just add:

/sys/power/dptf_current_profile    (rw)
/sys/power/dptf_available_profiles (ro)

(which will only be visible if a dptf-profile handler
 has been registered) ?

Or more generic and thus better (in case other platforms
later need something similar) I think, mirror the:

/sys/bus/cpu/devices/cpu#/cpufreq/energy_performance_* bits
for a system-wide energy-performance setting, so we get:

/sys/power/energy_performance_preference
/sys/power/energy_performance_available_preferences

(again only visible when applicable) ?

I personally like the second option best.

Regards,

Hans


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class (Also Re: [PATCH 0/4] powercap/dtpm: Add the DTPM framework)
  2020-10-16 11:10                 ` [RFC] Documentation: Add documentation for new performance_profile sysfs class (Also Re: [PATCH 0/4] powercap/dtpm: Add the DTPM framework) Hans de Goede
@ 2020-10-16 14:26                   ` Elia Devito
       [not found]                     ` <HK2PR0302MB2449214B28438ADC1790D468BD030@HK2PR0302MB2449.apcprd03.prod.outlook.com>
  2020-10-16 14:51                   ` Rafael J. Wysocki
  1 sibling, 1 reply; 11+ messages in thread
From: Elia Devito @ 2020-10-16 14:26 UTC (permalink / raw)
  To: Rafael J. Wysocki, Hans de Goede
  Cc: Daniel Lezcano, Srinivas Pandruvada, Lukasz Luba,
	Linux Kernel Mailing List, Linux PM, Zhang, Rui, Bastien Nocera,
	Mark Pearson, Limonciello, Mario, Darren Hart, Andy Shevchenko,
	Mark Gross, Benjamin Berg, linux-acpi, platform-driver-x86

Hi,

In data venerdì 16 ottobre 2020 13:10:54 CEST, Hans de Goede ha scritto:
> <note folding the 2 threads we are having on this into one, adding every one
> from both threads to the Cc>
> 
> Hi,
> 
> On 10/14/20 5:42 PM, Rafael J. Wysocki wrote:
> > On Wed, Oct 14, 2020 at 4:06 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >> On 10/14/20 3:33 PM, Rafael J. Wysocki wrote:
> <snip>
> 
> >>> First, a common place to register a DPTF system profile seems to be
> >>> needed and, as I said above, I wouldn't expect more than one such
> >>> thing to be present in the system at any given time, so it may be
> >>> registered along with the list of supported profiles and user space
> >>> will have to understand what they mean.
> >> 
> >> Mostly Ack, I would still like to have an enum for DPTF system
> >> profiles in the kernel and have a single piece of code map that
> >> enum to profile names. This enum can then be extended as
> >> necessary, but I want to avoid having one driver use
> >> "Performance" and the other "performance" or one using
> >> "performance-balanced" and the other "balanced-performance", etc.
> >> 
> >> With the goal being that new drivers use existing values from
> >> the enum as much as possible, but we extend it where necessary.
> > 
> > IOW, just a table of known profile names with specific indices assigned to
> > them.
> Yes.
> 
> > This sounds reasonable.
> > 
> >>> Second, irrespective of the above, it may be useful to have a
> >>> consistent way to pass performance-vs-power preference information
> >>> from user space to different parts of the kernel so as to allow them
> >>> to adjust their operation and this could be done with a system-wide
> >>> power profile attribute IMO.
> >> 
> >> I agree, which is why I tried to tackle both things in one go,
> >> but as you said doing both in 1 API is probably not the best idea.
> >> So I believe we should park this second issue for now and revisit it
> >> when we find a need for it.
> > 
> > Agreed.
> > 
> >> Do you have any specific userspace API in mind for the
> >> DPTF system profile selection?
> > 
> > Not really.
> 
> So before /sys/power/profile was mentioned, but that seems more like
> a thing which should have a set of fixed possible values, iow that is
> out of scope for this discussion.
> 
> Since we all seem to agree that this is something which we need
> specifically for DPTF profiles maybe just add:
> 
> /sys/power/dptf_current_profile    (rw)
> /sys/power/dptf_available_profiles (ro)
> 
> (which will only be visible if a dptf-profile handler
>  has been registered) ?
> 
> Or more generic and thus better (in case other platforms
> later need something similar) I think, mirror the:
> 
> /sys/bus/cpu/devices/cpu#/cpufreq/energy_performance_* bits
> for a system-wide energy-performance setting, so we get:
> 
> /sys/power/energy_performance_preference
> /sys/power/energy_performance_available_preferences
> 
> (again only visible when applicable) ?
> 
> I personally like the second option best.
> 
> Regards,
> 
> Hans

between the two, the second seems to me more appropriate.
Considering that the various profiles interact with thermal behaviors what do 
you think of something like:

/sys/power/thermal_profile_available_profiles
/sys/power/thermal_profile_profile

Regards,
Elia




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Fw: [External] Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class (Also Re: [PATCH 0/4] powercap/dtpm: Add the DTPM framework)
       [not found]                     ` <HK2PR0302MB2449214B28438ADC1790D468BD030@HK2PR0302MB2449.apcprd03.prod.outlook.com>
@ 2020-10-16 14:43                       ` Mark Pearson
  2020-10-16 15:16                         ` Elia Devito
  0 siblings, 1 reply; 11+ messages in thread
From: Mark Pearson @ 2020-10-16 14:43 UTC (permalink / raw)
  To: Rafael J. Wysocki, Hans de Goede, Elia Devito
  Cc: aniel Lezcano, Srinivas Pandruvada, Lukasz Luba,
	Linux Kernel Mailing List, Linux PM, Zhang, Rui, Bastien Nocera,
	Limonciello, Mario, Darren Hart, Andy Shevchenko, Mark Gross,
	Benjamin Berg, linux-acpi, platform-driver-x86

<Note - switched my email address to my more open source non-outlook 
based address>

On 2020-10-16 10:32 a.m., Mark Pearson wrote:
> 
> 
> ------------------------------------------------------------------------
> *From:* Elia Devito <eliadevito@gmail.com>
> *Sent:* October 16, 2020 10:26
> *To:* Rafael J. Wysocki <rafael@kernel.org>; Hans de Goede 
> <hdegoede@redhat.com>
> *Cc:* Daniel Lezcano <daniel.lezcano@linaro.org>; Srinivas Pandruvada 
> <srinivas.pandruvada@linux.intel.com>; Lukasz Luba 
> <lukasz.luba@arm.com>; Linux Kernel Mailing List 
> <linux-kernel@vger.kernel.org>; Linux PM <linux-pm@vger.kernel.org>; 
> Zhang, Rui <rui.zhang@intel.com>; Bastien Nocera <hadess@hadess.net>; 
> Mark Pearson <mpearson@lenovo.com>; Limonciello, Mario 
> <Mario.Limonciello@dell.com>; Darren Hart <dvhart@infradead.org>; Andy 
> Shevchenko <andy@infradead.org>; Mark Gross <mgross@linux.intel.com>; 
> Benjamin Berg <bberg@redhat.com>; linux-acpi@vger.kernel.org 
> <linux-acpi@vger.kernel.org>; platform-driver-x86@vger.kernel.org 
> <platform-driver-x86@vger.kernel.org>
> *Subject:* [External] Re: [RFC] Documentation: Add documentation for new 
> performance_profile sysfs class (Also Re: [PATCH 0/4] powercap/dtpm: Add 
> the DTPM framework)
> Hi,
> 
> In data venerdì 16 ottobre 2020 13:10:54 CEST, Hans de Goede ha scritto:
>> <note folding the 2 threads we are having on this into one, adding every one
>> from both threads to the Cc>
>> 
>> Hi,
>> 
>> On 10/14/20 5:42 PM, Rafael J. Wysocki wrote:
>> > On Wed, Oct 14, 2020 at 4:06 PM Hans de Goede <hdegoede@redhat.com> wrote:
>> >> On 10/14/20 3:33 PM, Rafael J. Wysocki wrote:
>> <snip>
>> 
>> >>> First, a common place to register a DPTF system profile seems to be
>> >>> needed and, as I said above, I wouldn't expect more than one such
>> >>> thing to be present in the system at any given time, so it may be
>> >>> registered along with the list of supported profiles and user space
>> >>> will have to understand what they mean.
>> >> 
>> >> Mostly Ack, I would still like to have an enum for DPTF system
>> >> profiles in the kernel and have a single piece of code map that
>> >> enum to profile names. This enum can then be extended as
>> >> necessary, but I want to avoid having one driver use
>> >> "Performance" and the other "performance" or one using
>> >> "performance-balanced" and the other "balanced-performance", etc.
>> >> 
>> >> With the goal being that new drivers use existing values from
>> >> the enum as much as possible, but we extend it where necessary.
>> > 
>> > IOW, just a table of known profile names with specific indices assigned to
>> > them.
>> Yes.
>> 
>> > This sounds reasonable.
>> > 
>> >>> Second, irrespective of the above, it may be useful to have a
>> >>> consistent way to pass performance-vs-power preference information
>> >>> from user space to different parts of the kernel so as to allow them
>> >>> to adjust their operation and this could be done with a system-wide
>> >>> power profile attribute IMO.
>> >> 
>> >> I agree, which is why I tried to tackle both things in one go,
>> >> but as you said doing both in 1 API is probably not the best idea.
>> >> So I believe we should park this second issue for now and revisit it
>> >> when we find a need for it.
>> > 
>> > Agreed.
>> > 
>> >> Do you have any specific userspace API in mind for the
>> >> DPTF system profile selection?
>> > 
>> > Not really.
>> 
>> So before /sys/power/profile was mentioned, but that seems more like
>> a thing which should have a set of fixed possible values, iow that is
>> out of scope for this discussion.
>> 
>> Since we all seem to agree that this is something which we need
>> specifically for DPTF profiles maybe just add:
>> 
>> /sys/power/dptf_current_profile    (rw)
>> /sys/power/dptf_available_profiles (ro)
>> 
>> (which will only be visible if a dptf-profile handler
>>  has been registered) ?
>> 
>> Or more generic and thus better (in case other platforms
>> later need something similar) I think, mirror the:
>> 
>> /sys/bus/cpu/devices/cpu#/cpufreq/energy_performance_* bits
>> for a system-wide energy-performance setting, so we get:
>> 
>> /sys/power/energy_performance_preference
>> /sys/power/energy_performance_available_preferences
>> 
>> (again only visible when applicable) ?
>> 
>> I personally like the second option best.
>> 
>> Regards,
>> 
>> Hans
> 
> between the two, the second seems to me more appropriate.
> Considering that the various profiles interact with thermal behaviors 
> what do
> you think of something like:
> 
> /sys/power/thermal_profile_available_profiles
> /sys/power/thermal_profile_profile
> 
> Regards,
> Elia
> 
I'm good with either but I do find 'profile_profile' slightly awkward to 
say out loud (even though it's logically correct :))

How about just:
/sys/power/platform_profile
/sys/power/platform_profile_available

As it covers the platform as a whole - fans, temperature, power, and 
anything else that ends up getting thrown in?

Mark

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class (Also Re: [PATCH 0/4] powercap/dtpm: Add the DTPM framework)
  2020-10-16 11:10                 ` [RFC] Documentation: Add documentation for new performance_profile sysfs class (Also Re: [PATCH 0/4] powercap/dtpm: Add the DTPM framework) Hans de Goede
  2020-10-16 14:26                   ` Elia Devito
@ 2020-10-16 14:51                   ` Rafael J. Wysocki
  2020-10-18  9:41                     ` Hans de Goede
  1 sibling, 1 reply; 11+ messages in thread
From: Rafael J. Wysocki @ 2020-10-16 14:51 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Rafael J. Wysocki, Daniel Lezcano, Srinivas Pandruvada,
	Lukasz Luba, Linux Kernel Mailing List, Linux PM, Zhang, Rui,
	Bastien Nocera, Mark Pearson, Limonciello, Mario, Darren Hart,
	Andy Shevchenko, Mark Gross, Elia Devito, Benjamin Berg,
	linux-acpi, platform-driver-x86

On Fri, Oct 16, 2020 at 1:11 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> <note folding the 2 threads we are having on this into one, adding every one from both threads to the Cc>
>
> Hi,
>
> On 10/14/20 5:42 PM, Rafael J. Wysocki wrote:
> > On Wed, Oct 14, 2020 at 4:06 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >> On 10/14/20 3:33 PM, Rafael J. Wysocki wrote:
>
> <snip>
>
> >>> First, a common place to register a DPTF system profile seems to be
> >>> needed and, as I said above, I wouldn't expect more than one such
> >>> thing to be present in the system at any given time, so it may be
> >>> registered along with the list of supported profiles and user space
> >>> will have to understand what they mean.
> >>
> >> Mostly Ack, I would still like to have an enum for DPTF system
> >> profiles in the kernel and have a single piece of code map that
> >> enum to profile names. This enum can then be extended as
> >> necessary, but I want to avoid having one driver use
> >> "Performance" and the other "performance" or one using
> >> "performance-balanced" and the other "balanced-performance", etc.
> >>
> >> With the goal being that new drivers use existing values from
> >> the enum as much as possible, but we extend it where necessary.
> >
> > IOW, just a table of known profile names with specific indices assigned to them.
>
> Yes.
>
> > This sounds reasonable.
> >
> >>> Second, irrespective of the above, it may be useful to have a
> >>> consistent way to pass performance-vs-power preference information
> >>> from user space to different parts of the kernel so as to allow them
> >>> to adjust their operation and this could be done with a system-wide
> >>> power profile attribute IMO.
> >>
> >> I agree, which is why I tried to tackle both things in one go,
> >> but as you said doing both in 1 API is probably not the best idea.
> >> So I believe we should park this second issue for now and revisit it
> >> when we find a need for it.
> >
> > Agreed.
> >
> >> Do you have any specific userspace API in mind for the
> >> DPTF system profile selection?
> >
> > Not really.
>
> So before /sys/power/profile was mentioned, but that seems more like
> a thing which should have a set of fixed possible values, iow that is
> out of scope for this discussion.

Yes.

> Since we all seem to agree that this is something which we need
> specifically for DPTF profiles maybe just add:
>
> /sys/power/dptf_current_profile    (rw)
> /sys/power/dptf_available_profiles (ro)
>
> (which will only be visible if a dptf-profile handler
>  has been registered) ?
>
> Or more generic and thus better (in case other platforms
> later need something similar) I think, mirror the:
>
> /sys/bus/cpu/devices/cpu#/cpufreq/energy_performance_* bits
> for a system-wide energy-performance setting, so we get:
>
> /sys/power/energy_performance_preference
> /sys/power/energy_performance_available_preferences

But this is not about energy vs performance only in general, is it?

> (again only visible when applicable) ?
>
> I personally like the second option best.

But I would put it under /sys/firmware/ instead of /sys/power/ and I
would call it something like platform_profile (and
platform_profile_choices or similar).

Cheers!

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Fw: [External] Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class (Also Re: [PATCH 0/4] powercap/dtpm: Add the DTPM framework)
  2020-10-16 14:43                       ` Fw: [External] " Mark Pearson
@ 2020-10-16 15:16                         ` Elia Devito
  0 siblings, 0 replies; 11+ messages in thread
From: Elia Devito @ 2020-10-16 15:16 UTC (permalink / raw)
  To: Rafael J. Wysocki, Hans de Goede, Mark Pearson
  Cc: aniel Lezcano, Srinivas Pandruvada, Lukasz Luba,
	Linux Kernel Mailing List, Linux PM, Zhang, Rui, Bastien Nocera,
	Limonciello, Mario, Darren Hart, Andy Shevchenko, Mark Gross,
	Benjamin Berg, linux-acpi, platform-driver-x86

Hi,

In data venerdì 16 ottobre 2020 16:43:09 CEST, Mark Pearson ha scritto:
> <Note - switched my email address to my more open source non-outlook
> based address>
> 
> On 2020-10-16 10:32 a.m., Mark Pearson wrote:
> > ------------------------------------------------------------------------
> > *From:* Elia Devito <eliadevito@gmail.com>
> > *Sent:* October 16, 2020 10:26
> > *To:* Rafael J. Wysocki <rafael@kernel.org>; Hans de Goede
> > <hdegoede@redhat.com>
> > *Cc:* Daniel Lezcano <daniel.lezcano@linaro.org>; Srinivas Pandruvada
> > <srinivas.pandruvada@linux.intel.com>; Lukasz Luba
> > <lukasz.luba@arm.com>; Linux Kernel Mailing List
> > <linux-kernel@vger.kernel.org>; Linux PM <linux-pm@vger.kernel.org>;
> > Zhang, Rui <rui.zhang@intel.com>; Bastien Nocera <hadess@hadess.net>;
> > Mark Pearson <mpearson@lenovo.com>; Limonciello, Mario
> > <Mario.Limonciello@dell.com>; Darren Hart <dvhart@infradead.org>; Andy
> > Shevchenko <andy@infradead.org>; Mark Gross <mgross@linux.intel.com>;
> > Benjamin Berg <bberg@redhat.com>; linux-acpi@vger.kernel.org
> > <linux-acpi@vger.kernel.org>; platform-driver-x86@vger.kernel.org
> > <platform-driver-x86@vger.kernel.org>
> > *Subject:* [External] Re: [RFC] Documentation: Add documentation for new
> > performance_profile sysfs class (Also Re: [PATCH 0/4] powercap/dtpm: Add
> > the DTPM framework)
> > Hi,
> > 
> > In data venerdì 16 ottobre 2020 13:10:54 CEST, Hans de Goede ha scritto:
> >> <note folding the 2 threads we are having on this into one, adding every
> >> one from both threads to the Cc>
> >> 
> >> Hi,
> >> 
> >> On 10/14/20 5:42 PM, Rafael J. Wysocki wrote:
> >> > On Wed, Oct 14, 2020 at 4:06 PM Hans de Goede <hdegoede@redhat.com> 
wrote:
> >> >> On 10/14/20 3:33 PM, Rafael J. Wysocki wrote:
> >> <snip>
> >> 
> >> >>> First, a common place to register a DPTF system profile seems to be
> >> >>> needed and, as I said above, I wouldn't expect more than one such
> >> >>> thing to be present in the system at any given time, so it may be
> >> >>> registered along with the list of supported profiles and user space
> >> >>> will have to understand what they mean.
> >> >> 
> >> >> Mostly Ack, I would still like to have an enum for DPTF system
> >> >> profiles in the kernel and have a single piece of code map that
> >> >> enum to profile names. This enum can then be extended as
> >> >> necessary, but I want to avoid having one driver use
> >> >> "Performance" and the other "performance" or one using
> >> >> "performance-balanced" and the other "balanced-performance", etc.
> >> >> 
> >> >> With the goal being that new drivers use existing values from
> >> >> the enum as much as possible, but we extend it where necessary.
> >> > 
> >> > IOW, just a table of known profile names with specific indices assigned
> >> > to
> >> > them.
> >> 
> >> Yes.
> >> 
> >> > This sounds reasonable.
> >> > 
> >> >>> Second, irrespective of the above, it may be useful to have a
> >> >>> consistent way to pass performance-vs-power preference information
> >> >>> from user space to different parts of the kernel so as to allow them
> >> >>> to adjust their operation and this could be done with a system-wide
> >> >>> power profile attribute IMO.
> >> >> 
> >> >> I agree, which is why I tried to tackle both things in one go,
> >> >> but as you said doing both in 1 API is probably not the best idea.
> >> >> So I believe we should park this second issue for now and revisit it
> >> >> when we find a need for it.
> >> > 
> >> > Agreed.
> >> > 
> >> >> Do you have any specific userspace API in mind for the
> >> >> DPTF system profile selection?
> >> > 
> >> > Not really.
> >> 
> >> So before /sys/power/profile was mentioned, but that seems more like
> >> a thing which should have a set of fixed possible values, iow that is
> >> out of scope for this discussion.
> >> 
> >> Since we all seem to agree that this is something which we need
> >> specifically for DPTF profiles maybe just add:
> >> 
> >> /sys/power/dptf_current_profile    (rw)
> >> /sys/power/dptf_available_profiles (ro)
> >> 
> >> (which will only be visible if a dptf-profile handler
> >>
> >>  has been registered) ?
> >>
> >> Or more generic and thus better (in case other platforms
> >> later need something similar) I think, mirror the:
> >> 
> >> /sys/bus/cpu/devices/cpu#/cpufreq/energy_performance_* bits
> >> for a system-wide energy-performance setting, so we get:
> >> 
> >> /sys/power/energy_performance_preference
> >> /sys/power/energy_performance_available_preferences
> >> 
> >> (again only visible when applicable) ?
> >> 
> >> I personally like the second option best.
> >> 
> >> Regards,
> >> 
> >> Hans
> > 
> > between the two, the second seems to me more appropriate.
> > Considering that the various profiles interact with thermal behaviors
> > what do
> > you think of something like:
> > 
> > /sys/power/thermal_profile_available_profiles
> > /sys/power/thermal_profile_profile
> > 
> > Regards,
> > Elia
> 
> I'm good with either but I do find 'profile_profile' slightly awkward to
> say out loud (even though it's logically correct :))
> 
> How about just:
> /sys/power/platform_profile
> /sys/power/platform_profile_available
> 
> As it covers the platform as a whole - fans, temperature, power, and
> anything else that ends up getting thrown in?
> 
> Mark

Completely agree,  I made a typo xD

Elia



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class (Also Re: [PATCH 0/4] powercap/dtpm: Add the DTPM framework)
  2020-10-16 14:51                   ` Rafael J. Wysocki
@ 2020-10-18  9:41                     ` Hans de Goede
  2020-10-18 12:31                       ` Rafael J. Wysocki
  0 siblings, 1 reply; 11+ messages in thread
From: Hans de Goede @ 2020-10-18  9:41 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Daniel Lezcano, Srinivas Pandruvada, Lukasz Luba,
	Linux Kernel Mailing List, Linux PM, Zhang, Rui, Bastien Nocera,
	Mark Pearson, Limonciello, Mario, Darren Hart, Andy Shevchenko,
	Mark Gross, Elia Devito, Benjamin Berg, linux-acpi,
	platform-driver-x86

Hi,

On 10/16/20 4:51 PM, Rafael J. Wysocki wrote:
> On Fri, Oct 16, 2020 at 1:11 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> <note folding the 2 threads we are having on this into one, adding every one from both threads to the Cc>
>>
>> Hi,
>>
>> On 10/14/20 5:42 PM, Rafael J. Wysocki wrote:
>>> On Wed, Oct 14, 2020 at 4:06 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>>> On 10/14/20 3:33 PM, Rafael J. Wysocki wrote:
>>
>> <snip>
>>
>>>>> First, a common place to register a DPTF system profile seems to be
>>>>> needed and, as I said above, I wouldn't expect more than one such
>>>>> thing to be present in the system at any given time, so it may be
>>>>> registered along with the list of supported profiles and user space
>>>>> will have to understand what they mean.
>>>>
>>>> Mostly Ack, I would still like to have an enum for DPTF system
>>>> profiles in the kernel and have a single piece of code map that
>>>> enum to profile names. This enum can then be extended as
>>>> necessary, but I want to avoid having one driver use
>>>> "Performance" and the other "performance" or one using
>>>> "performance-balanced" and the other "balanced-performance", etc.
>>>>
>>>> With the goal being that new drivers use existing values from
>>>> the enum as much as possible, but we extend it where necessary.
>>>
>>> IOW, just a table of known profile names with specific indices assigned to them.
>>
>> Yes.
>>
>>> This sounds reasonable.
>>>
>>>>> Second, irrespective of the above, it may be useful to have a
>>>>> consistent way to pass performance-vs-power preference information
>>>>> from user space to different parts of the kernel so as to allow them
>>>>> to adjust their operation and this could be done with a system-wide
>>>>> power profile attribute IMO.
>>>>
>>>> I agree, which is why I tried to tackle both things in one go,
>>>> but as you said doing both in 1 API is probably not the best idea.
>>>> So I believe we should park this second issue for now and revisit it
>>>> when we find a need for it.
>>>
>>> Agreed.
>>>
>>>> Do you have any specific userspace API in mind for the
>>>> DPTF system profile selection?
>>>
>>> Not really.
>>
>> So before /sys/power/profile was mentioned, but that seems more like
>> a thing which should have a set of fixed possible values, iow that is
>> out of scope for this discussion.
> 
> Yes.
> 
>> Since we all seem to agree that this is something which we need
>> specifically for DPTF profiles maybe just add:
>>
>> /sys/power/dptf_current_profile    (rw)
>> /sys/power/dptf_available_profiles (ro)
>>
>> (which will only be visible if a dptf-profile handler
>>  has been registered) ?
>>
>> Or more generic and thus better (in case other platforms
>> later need something similar) I think, mirror the:
>>
>> /sys/bus/cpu/devices/cpu#/cpufreq/energy_performance_* bits
>> for a system-wide energy-performance setting, so we get:
>>
>> /sys/power/energy_performance_preference
>> /sys/power/energy_performance_available_preferences
> 
> But this is not about energy vs performance only in general, is it?
> 
>> (again only visible when applicable) ?
>>
>> I personally like the second option best.
> 
> But I would put it under /sys/firmware/ instead of /sys/power/ and I
> would call it something like platform_profile (and
> platform_profile_choices or similar).

Currently we only have dirs under /sys/firmware:

[hans@x1 ~]$ ls /sys/firmware
acpi  dmi  efi  memmap

But we do have /sys/firmware/apci/pm_profile:

Documentation/ABI/stable/sysfs-acpi-pmprofile

What:           /sys/firmware/acpi/pm_profile
Date:           03-Nov-2011
KernelVersion:  v3.2
Contact:        linux-acpi@vger.kernel.org
Description:    The ACPI pm_profile sysfs interface exports the platform
                power management (and performance) requirement expectations
                as provided by BIOS. The integer value is directly passed as
                retrieved from the FADT ACPI table.
Values:         For possible values see ACPI specification:
                5.2.9 Fixed ACPI Description Table (FADT)
                Field: Preferred_PM_Profile

                Currently these values are defined by spec:
                0 Unspecified
                1 Desktop
                2 Mobile
                3 Workstation
                4 Enterprise Server
		...

Since all platforms which we need this for are ACPI based
(and the involved interfaces are also all ACPI interfaces)
how about:

/sys/firmware/acpi/platform_profile
/sys/firmware/acpi/platform_profile_choices

?

I think this goes nice together with /sys/firmware/acpi/pm_profile
although that is read-only and this is a read/write setting.

Rafel, would:

/sys/firmware/acpi/platform_profile
/sys/firmware/acpi/platform_profile_choices

work for you ?

Regards,

Hans



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class (Also Re: [PATCH 0/4] powercap/dtpm: Add the DTPM framework)
  2020-10-18  9:41                     ` Hans de Goede
@ 2020-10-18 12:31                       ` Rafael J. Wysocki
  2020-10-19 18:43                         ` Hans de Goede
  0 siblings, 1 reply; 11+ messages in thread
From: Rafael J. Wysocki @ 2020-10-18 12:31 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Rafael J. Wysocki, Daniel Lezcano, Srinivas Pandruvada,
	Lukasz Luba, Linux Kernel Mailing List, Linux PM, Zhang, Rui,
	Bastien Nocera, Mark Pearson, Limonciello, Mario, Darren Hart,
	Andy Shevchenko, Mark Gross, Elia Devito, Benjamin Berg,
	linux-acpi, platform-driver-x86

On Sun, Oct 18, 2020 at 11:41 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 10/16/20 4:51 PM, Rafael J. Wysocki wrote:
> > On Fri, Oct 16, 2020 at 1:11 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >> <note folding the 2 threads we are having on this into one, adding every one from both threads to the Cc>
> >>
> >> Hi,
> >>
> >> On 10/14/20 5:42 PM, Rafael J. Wysocki wrote:
> >>> On Wed, Oct 14, 2020 at 4:06 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >>>> On 10/14/20 3:33 PM, Rafael J. Wysocki wrote:
> >>
> >> <snip>
> >>
> >>>>> First, a common place to register a DPTF system profile seems to be
> >>>>> needed and, as I said above, I wouldn't expect more than one such
> >>>>> thing to be present in the system at any given time, so it may be
> >>>>> registered along with the list of supported profiles and user space
> >>>>> will have to understand what they mean.
> >>>>
> >>>> Mostly Ack, I would still like to have an enum for DPTF system
> >>>> profiles in the kernel and have a single piece of code map that
> >>>> enum to profile names. This enum can then be extended as
> >>>> necessary, but I want to avoid having one driver use
> >>>> "Performance" and the other "performance" or one using
> >>>> "performance-balanced" and the other "balanced-performance", etc.
> >>>>
> >>>> With the goal being that new drivers use existing values from
> >>>> the enum as much as possible, but we extend it where necessary.
> >>>
> >>> IOW, just a table of known profile names with specific indices assigned to them.
> >>
> >> Yes.
> >>
> >>> This sounds reasonable.
> >>>
> >>>>> Second, irrespective of the above, it may be useful to have a
> >>>>> consistent way to pass performance-vs-power preference information
> >>>>> from user space to different parts of the kernel so as to allow them
> >>>>> to adjust their operation and this could be done with a system-wide
> >>>>> power profile attribute IMO.
> >>>>
> >>>> I agree, which is why I tried to tackle both things in one go,
> >>>> but as you said doing both in 1 API is probably not the best idea.
> >>>> So I believe we should park this second issue for now and revisit it
> >>>> when we find a need for it.
> >>>
> >>> Agreed.
> >>>
> >>>> Do you have any specific userspace API in mind for the
> >>>> DPTF system profile selection?
> >>>
> >>> Not really.
> >>
> >> So before /sys/power/profile was mentioned, but that seems more like
> >> a thing which should have a set of fixed possible values, iow that is
> >> out of scope for this discussion.
> >
> > Yes.
> >
> >> Since we all seem to agree that this is something which we need
> >> specifically for DPTF profiles maybe just add:
> >>
> >> /sys/power/dptf_current_profile    (rw)
> >> /sys/power/dptf_available_profiles (ro)
> >>
> >> (which will only be visible if a dptf-profile handler
> >>  has been registered) ?
> >>
> >> Or more generic and thus better (in case other platforms
> >> later need something similar) I think, mirror the:
> >>
> >> /sys/bus/cpu/devices/cpu#/cpufreq/energy_performance_* bits
> >> for a system-wide energy-performance setting, so we get:
> >>
> >> /sys/power/energy_performance_preference
> >> /sys/power/energy_performance_available_preferences
> >
> > But this is not about energy vs performance only in general, is it?
> >
> >> (again only visible when applicable) ?
> >>
> >> I personally like the second option best.
> >
> > But I would put it under /sys/firmware/ instead of /sys/power/ and I
> > would call it something like platform_profile (and
> > platform_profile_choices or similar).
>
> Currently we only have dirs under /sys/firmware:
>
> [hans@x1 ~]$ ls /sys/firmware
> acpi  dmi  efi  memmap
>
> But we do have /sys/firmware/apci/pm_profile:
>
> Documentation/ABI/stable/sysfs-acpi-pmprofile
>
> What:           /sys/firmware/acpi/pm_profile
> Date:           03-Nov-2011
> KernelVersion:  v3.2
> Contact:        linux-acpi@vger.kernel.org
> Description:    The ACPI pm_profile sysfs interface exports the platform
>                 power management (and performance) requirement expectations
>                 as provided by BIOS. The integer value is directly passed as
>                 retrieved from the FADT ACPI table.
> Values:         For possible values see ACPI specification:
>                 5.2.9 Fixed ACPI Description Table (FADT)
>                 Field: Preferred_PM_Profile
>
>                 Currently these values are defined by spec:
>                 0 Unspecified
>                 1 Desktop
>                 2 Mobile
>                 3 Workstation
>                 4 Enterprise Server
>                 ...
>
> Since all platforms which we need this for are ACPI based
> (and the involved interfaces are also all ACPI interfaces)
> how about:
>
> /sys/firmware/acpi/platform_profile
> /sys/firmware/acpi/platform_profile_choices
>
> ?
>
> I think this goes nice together with /sys/firmware/acpi/pm_profile
> although that is read-only and this is a read/write setting.
>
> Rafel, would:
>
> /sys/firmware/acpi/platform_profile
> /sys/firmware/acpi/platform_profile_choices
>
> work for you ?

Yes, it would.

Cheers!

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class (Also Re: [PATCH 0/4] powercap/dtpm: Add the DTPM framework)
  2020-10-18 12:31                       ` Rafael J. Wysocki
@ 2020-10-19 18:43                         ` Hans de Goede
       [not found]                           ` <HK2PR0302MB24494037019FBC7720976735BD1E0@HK2PR0302MB2449.apcprd03.prod.outlook.com>
  2020-10-20 12:34                           ` Rafael J. Wysocki
  0 siblings, 2 replies; 11+ messages in thread
From: Hans de Goede @ 2020-10-19 18:43 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Daniel Lezcano, Srinivas Pandruvada, Lukasz Luba,
	Linux Kernel Mailing List, Linux PM, Zhang, Rui, Bastien Nocera,
	Mark Pearson, Limonciello, Mario, Darren Hart, Andy Shevchenko,
	Mark Gross, Elia Devito, Benjamin Berg, linux-acpi,
	platform-driver-x86

Hi,

On 10/18/20 2:31 PM, Rafael J. Wysocki wrote:
> On Sun, Oct 18, 2020 at 11:41 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> On 10/16/20 4:51 PM, Rafael J. Wysocki wrote:
>>> On Fri, Oct 16, 2020 at 1:11 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>> <note folding the 2 threads we are having on this into one, adding every one from both threads to the Cc>
>>>>
>>>> Hi,
>>>>
>>>> On 10/14/20 5:42 PM, Rafael J. Wysocki wrote:
>>>>> On Wed, Oct 14, 2020 at 4:06 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>> On 10/14/20 3:33 PM, Rafael J. Wysocki wrote:
>>>>
>>>> <snip>
>>>>
>>>>>>> First, a common place to register a DPTF system profile seems to be
>>>>>>> needed and, as I said above, I wouldn't expect more than one such
>>>>>>> thing to be present in the system at any given time, so it may be
>>>>>>> registered along with the list of supported profiles and user space
>>>>>>> will have to understand what they mean.
>>>>>>
>>>>>> Mostly Ack, I would still like to have an enum for DPTF system
>>>>>> profiles in the kernel and have a single piece of code map that
>>>>>> enum to profile names. This enum can then be extended as
>>>>>> necessary, but I want to avoid having one driver use
>>>>>> "Performance" and the other "performance" or one using
>>>>>> "performance-balanced" and the other "balanced-performance", etc.
>>>>>>
>>>>>> With the goal being that new drivers use existing values from
>>>>>> the enum as much as possible, but we extend it where necessary.
>>>>>
>>>>> IOW, just a table of known profile names with specific indices assigned to them.
>>>>
>>>> Yes.
>>>>
>>>>> This sounds reasonable.
>>>>>
>>>>>>> Second, irrespective of the above, it may be useful to have a
>>>>>>> consistent way to pass performance-vs-power preference information
>>>>>>> from user space to different parts of the kernel so as to allow them
>>>>>>> to adjust their operation and this could be done with a system-wide
>>>>>>> power profile attribute IMO.
>>>>>>
>>>>>> I agree, which is why I tried to tackle both things in one go,
>>>>>> but as you said doing both in 1 API is probably not the best idea.
>>>>>> So I believe we should park this second issue for now and revisit it
>>>>>> when we find a need for it.
>>>>>
>>>>> Agreed.
>>>>>
>>>>>> Do you have any specific userspace API in mind for the
>>>>>> DPTF system profile selection?
>>>>>
>>>>> Not really.
>>>>
>>>> So before /sys/power/profile was mentioned, but that seems more like
>>>> a thing which should have a set of fixed possible values, iow that is
>>>> out of scope for this discussion.
>>>
>>> Yes.
>>>
>>>> Since we all seem to agree that this is something which we need
>>>> specifically for DPTF profiles maybe just add:
>>>>
>>>> /sys/power/dptf_current_profile    (rw)
>>>> /sys/power/dptf_available_profiles (ro)
>>>>
>>>> (which will only be visible if a dptf-profile handler
>>>>  has been registered) ?
>>>>
>>>> Or more generic and thus better (in case other platforms
>>>> later need something similar) I think, mirror the:
>>>>
>>>> /sys/bus/cpu/devices/cpu#/cpufreq/energy_performance_* bits
>>>> for a system-wide energy-performance setting, so we get:
>>>>
>>>> /sys/power/energy_performance_preference
>>>> /sys/power/energy_performance_available_preferences
>>>
>>> But this is not about energy vs performance only in general, is it?
>>>
>>>> (again only visible when applicable) ?
>>>>
>>>> I personally like the second option best.
>>>
>>> But I would put it under /sys/firmware/ instead of /sys/power/ and I
>>> would call it something like platform_profile (and
>>> platform_profile_choices or similar).
>>
>> Currently we only have dirs under /sys/firmware:
>>
>> [hans@x1 ~]$ ls /sys/firmware
>> acpi  dmi  efi  memmap
>>
>> But we do have /sys/firmware/apci/pm_profile:
>>
>> Documentation/ABI/stable/sysfs-acpi-pmprofile
>>
>> What:           /sys/firmware/acpi/pm_profile
>> Date:           03-Nov-2011
>> KernelVersion:  v3.2
>> Contact:        linux-acpi@vger.kernel.org
>> Description:    The ACPI pm_profile sysfs interface exports the platform
>>                 power management (and performance) requirement expectations
>>                 as provided by BIOS. The integer value is directly passed as
>>                 retrieved from the FADT ACPI table.
>> Values:         For possible values see ACPI specification:
>>                 5.2.9 Fixed ACPI Description Table (FADT)
>>                 Field: Preferred_PM_Profile
>>
>>                 Currently these values are defined by spec:
>>                 0 Unspecified
>>                 1 Desktop
>>                 2 Mobile
>>                 3 Workstation
>>                 4 Enterprise Server
>>                 ...
>>
>> Since all platforms which we need this for are ACPI based
>> (and the involved interfaces are also all ACPI interfaces)
>> how about:
>>
>> /sys/firmware/acpi/platform_profile
>> /sys/firmware/acpi/platform_profile_choices
>>
>> ?
>>
>> I think this goes nice together with /sys/firmware/acpi/pm_profile
>> although that is read-only and this is a read/write setting.
>>
>> Rafel, would:
>>
>> /sys/firmware/acpi/platform_profile
>> /sys/firmware/acpi/platform_profile_choices
>>
>> work for you ?
> 
> Yes, it would.

Great. So I think hat means that we have the most important part
for moving forward with this.

So I guess the plan for this now looks something like this.

1. Rewrite my API docs RFC to update it for the new /sys/firmware/acpi/platform_profile[_choices]
   plan (should be easy and a bunch of stuff like the "type" bit can just be dropped)

2. Add code somewhere under drivers/acpi which allows code from else where
   to register itself as platform_profile handler/provider.

Rafael, any suggestions / preference for where this should be added under
drivers/acpi ?  In a new .c file perhaps ?

3.1 Use the code from 2 to add support for platform-profile selection in
    thinkpad_acpi (something for me or Mark Pearson) to do
3.2 Use the code from 2 to add support for platform-profile selection
    to hp-wmi
3.3 (and to other drivers in the future).


An open question is who will take care of 1. and 2. Mark (Pearson)
do you feel up to this? or do you want me to take care of this?

Regards,

Hans








^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Fw: [External] Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class (Also Re: [PATCH 0/4] powercap/dtpm: Add the DTPM framework)
       [not found]                           ` <HK2PR0302MB24494037019FBC7720976735BD1E0@HK2PR0302MB2449.apcprd03.prod.outlook.com>
@ 2020-10-19 18:49                             ` Mark Pearson
  2020-10-25 10:13                               ` Hans de Goede
  0 siblings, 1 reply; 11+ messages in thread
From: Mark Pearson @ 2020-10-19 18:49 UTC (permalink / raw)
  To: Hans de Goede, Rafael J. Wysocki
  Cc: Daniel Lezcano, Srinivas Pandruvada ,>, Lukasz Luba ,>,
	Linux Kernel Mailing List ,>,
	Linux PM, > Zhang, Rui, Bastien Nocera, > Mark Pearson,
	Limonciello, Mario ,>,
	Darren Hart, Andy ,> Shevchenko, Mark Gross, > Elia Devito,
	Benjamin Berg, > linux-acpi@vger.kernel.org,
	> platform-driver-x86@vger.kernel.org

Hi

 > On 19/10/2020 14:43, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
> 
> On 10/18/20 2:31 PM, Rafael J. Wysocki wrote:
>> On Sun, Oct 18, 2020 at 11:41 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> Hi,
>>>
>>> On 10/16/20 4:51 PM, Rafael J. Wysocki wrote:
>>>> On Fri, Oct 16, 2020 at 1:11 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>
>>>>> <note folding the 2 threads we are having on this into one, adding every one from both threads to the Cc>
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 10/14/20 5:42 PM, Rafael J. Wysocki wrote:
>>>>>> On Wed, Oct 14, 2020 at 4:06 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>> On 10/14/20 3:33 PM, Rafael J. Wysocki wrote:
>>>>>
>>>>> <snip>
>>>>>
>>>>>>>> First, a common place to register a DPTF system profile seems to be
>>>>>>>> needed and, as I said above, I wouldn't expect more than one such
>>>>>>>> thing to be present in the system at any given time, so it may be
>>>>>>>> registered along with the list of supported profiles and user space
>>>>>>>> will have to understand what they mean.
>>>>>>>
>>>>>>> Mostly Ack, I would still like to have an enum for DPTF system
>>>>>>> profiles in the kernel and have a single piece of code map that
>>>>>>> enum to profile names. This enum can then be extended as
>>>>>>> necessary, but I want to avoid having one driver use
>>>>>>> "Performance" and the other "performance" or one using
>>>>>>> "performance-balanced" and the other "balanced-performance", etc.
>>>>>>>
>>>>>>> With the goal being that new drivers use existing values from
>>>>>>> the enum as much as possible, but we extend it where necessary.
>>>>>>
>>>>>> IOW, just a table of known profile names with specific indices assigned to them.
>>>>>
>>>>> Yes.
>>>>>
>>>>>> This sounds reasonable.
>>>>>>
>>>>>>>> Second, irrespective of the above, it may be useful to have a
>>>>>>>> consistent way to pass performance-vs-power preference information
>>>>>>>> from user space to different parts of the kernel so as to allow them
>>>>>>>> to adjust their operation and this could be done with a system-wide
>>>>>>>> power profile attribute IMO.
>>>>>>>
>>>>>>> I agree, which is why I tried to tackle both things in one go,
>>>>>>> but as you said doing both in 1 API is probably not the best idea.
>>>>>>> So I believe we should park this second issue for now and revisit it
>>>>>>> when we find a need for it.
>>>>>>
>>>>>> Agreed.
>>>>>>
>>>>>>> Do you have any specific userspace API in mind for the
>>>>>>> DPTF system profile selection?
>>>>>>
>>>>>> Not really.
>>>>>
>>>>> So before /sys/power/profile was mentioned, but that seems more like
>>>>> a thing which should have a set of fixed possible values, iow that is
>>>>> out of scope for this discussion.
>>>>
>>>> Yes.
>>>>
>>>>> Since we all seem to agree that this is something which we need
>>>>> specifically for DPTF profiles maybe just add:
>>>>>
>>>>> /sys/power/dptf_current_profile    (rw)
>>>>> /sys/power/dptf_available_profiles (ro)
>>>>>
>>>>> (which will only be visible if a dptf-profile handler
>>>>>  has been registered) ?
>>>>>
>>>>> Or more generic and thus better (in case other platforms
>>>>> later need something similar) I think, mirror the:
>>>>>
>>>>> /sys/bus/cpu/devices/cpu#/cpufreq/energy_performance_* bits
>>>>> for a system-wide energy-performance setting, so we get:
>>>>>
>>>>> /sys/power/energy_performance_preference
>>>>> /sys/power/energy_performance_available_preferences
>>>>
>>>> But this is not about energy vs performance only in general, is it?
>>>>
>>>>> (again only visible when applicable) ?
>>>>>
>>>>> I personally like the second option best.
>>>>
>>>> But I would put it under /sys/firmware/ instead of /sys/power/ and I
>>>> would call it something like platform_profile (and
>>>> platform_profile_choices or similar).
>>>
>>> Currently we only have dirs under /sys/firmware:
>>>
>>> [hans@x1 ~]$ ls /sys/firmware
>>> acpi  dmi  efi  memmap
>>>
>>> But we do have /sys/firmware/apci/pm_profile:
>>>
>>> Documentation/ABI/stable/sysfs-acpi-pmprofile
>>>
>>> What:           /sys/firmware/acpi/pm_profile
>>> Date:           03-Nov-2011
>>> KernelVersion:  v3.2
>>> Contact:        linux-acpi@vger.kernel.org
>>> Description:    The ACPI pm_profile sysfs interface exports the platform
>>>                 power management (and performance) requirement expectations
>>>                 as provided by BIOS. The integer value is directly passed as
>>>                 retrieved from the FADT ACPI table.
>>> Values:         For possible values see ACPI specification:
>>>                 5.2.9 Fixed ACPI Description Table (FADT)
>>>                 Field: Preferred_PM_Profile
>>>
>>>                 Currently these values are defined by spec:
>>>                 0 Unspecified
>>>                 1 Desktop
>>>                 2 Mobile
>>>                 3 Workstation
>>>                 4 Enterprise Server
>>>                 ...
>>>
>>> Since all platforms which we need this for are ACPI based
>>> (and the involved interfaces are also all ACPI interfaces)
>>> how about:
>>>
>>> /sys/firmware/acpi/platform_profile
>>> /sys/firmware/acpi/platform_profile_choices
>>>
>>> ?
>>>
>>> I think this goes nice together with /sys/firmware/acpi/pm_profile
>>> although that is read-only and this is a read/write setting.
>>>
>>> Rafel, would:
>>>
>>> /sys/firmware/acpi/platform_profile
>>> /sys/firmware/acpi/platform_profile_choices
>>>
>>> work for you ?
>> 
>> Yes, it would.
> 
> Great. So I think hat means that we have the most important part
> for moving forward with this.
> 
> So I guess the plan for this now looks something like this.
> 
> 1. Rewrite my API docs RFC to update it for the new 
> /sys/firmware/acpi/platform_profile[_choices]
>     plan (should be easy and a bunch of stuff like the "type" bit can 
> just be dropped)
> 
> 2. Add code somewhere under drivers/acpi which allows code from else where
>     to register itself as platform_profile handler/provider.
> 
> Rafael, any suggestions / preference for where this should be added under
> drivers/acpi ?  In a new .c file perhaps ?
> 
> 3.1 Use the code from 2 to add support for platform-profile selection in
>      thinkpad_acpi (something for me or Mark Pearson) to do
> 3.2 Use the code from 2 to add support for platform-profile selection
>      to hp-wmi
> 3.3 (and to other drivers in the future).
> 
> 
> An open question is who will take care of 1. and 2. Mark (Pearson)
> do you feel up to this? or do you want me to take care of this?
> 
> Regards,
> 
> Hans
> 

Definitely up for (2) and will happily have a go at number (1).

If there's an example of something similar I can look at for reference 
that would be helpful :)

Mark

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class (Also Re: [PATCH 0/4] powercap/dtpm: Add the DTPM framework)
  2020-10-19 18:43                         ` Hans de Goede
       [not found]                           ` <HK2PR0302MB24494037019FBC7720976735BD1E0@HK2PR0302MB2449.apcprd03.prod.outlook.com>
@ 2020-10-20 12:34                           ` Rafael J. Wysocki
  1 sibling, 0 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2020-10-20 12:34 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Rafael J. Wysocki, Daniel Lezcano, Srinivas Pandruvada,
	Lukasz Luba, Linux Kernel Mailing List, Linux PM, Zhang, Rui,
	Bastien Nocera, Mark Pearson, Limonciello, Mario, Darren Hart,
	Andy Shevchenko, Mark Gross, Elia Devito, Benjamin Berg,
	linux-acpi, platform-driver-x86

On Mon, Oct 19, 2020 at 8:43 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 10/18/20 2:31 PM, Rafael J. Wysocki wrote:
> > On Sun, Oct 18, 2020 at 11:41 AM Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >> Hi,
> >>
> >> On 10/16/20 4:51 PM, Rafael J. Wysocki wrote:
> >>> On Fri, Oct 16, 2020 at 1:11 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >>>>
> >>>> <note folding the 2 threads we are having on this into one, adding every one from both threads to the Cc>
> >>>>
> >>>> Hi,
> >>>>
> >>>> On 10/14/20 5:42 PM, Rafael J. Wysocki wrote:
> >>>>> On Wed, Oct 14, 2020 at 4:06 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >>>>>> On 10/14/20 3:33 PM, Rafael J. Wysocki wrote:
> >>>>
> >>>> <snip>
> >>>>
> >>>>>>> First, a common place to register a DPTF system profile seems to be
> >>>>>>> needed and, as I said above, I wouldn't expect more than one such
> >>>>>>> thing to be present in the system at any given time, so it may be
> >>>>>>> registered along with the list of supported profiles and user space
> >>>>>>> will have to understand what they mean.
> >>>>>>
> >>>>>> Mostly Ack, I would still like to have an enum for DPTF system
> >>>>>> profiles in the kernel and have a single piece of code map that
> >>>>>> enum to profile names. This enum can then be extended as
> >>>>>> necessary, but I want to avoid having one driver use
> >>>>>> "Performance" and the other "performance" or one using
> >>>>>> "performance-balanced" and the other "balanced-performance", etc.
> >>>>>>
> >>>>>> With the goal being that new drivers use existing values from
> >>>>>> the enum as much as possible, but we extend it where necessary.
> >>>>>
> >>>>> IOW, just a table of known profile names with specific indices assigned to them.
> >>>>
> >>>> Yes.
> >>>>
> >>>>> This sounds reasonable.
> >>>>>
> >>>>>>> Second, irrespective of the above, it may be useful to have a
> >>>>>>> consistent way to pass performance-vs-power preference information
> >>>>>>> from user space to different parts of the kernel so as to allow them
> >>>>>>> to adjust their operation and this could be done with a system-wide
> >>>>>>> power profile attribute IMO.
> >>>>>>
> >>>>>> I agree, which is why I tried to tackle both things in one go,
> >>>>>> but as you said doing both in 1 API is probably not the best idea.
> >>>>>> So I believe we should park this second issue for now and revisit it
> >>>>>> when we find a need for it.
> >>>>>
> >>>>> Agreed.
> >>>>>
> >>>>>> Do you have any specific userspace API in mind for the
> >>>>>> DPTF system profile selection?
> >>>>>
> >>>>> Not really.
> >>>>
> >>>> So before /sys/power/profile was mentioned, but that seems more like
> >>>> a thing which should have a set of fixed possible values, iow that is
> >>>> out of scope for this discussion.
> >>>
> >>> Yes.
> >>>
> >>>> Since we all seem to agree that this is something which we need
> >>>> specifically for DPTF profiles maybe just add:
> >>>>
> >>>> /sys/power/dptf_current_profile    (rw)
> >>>> /sys/power/dptf_available_profiles (ro)
> >>>>
> >>>> (which will only be visible if a dptf-profile handler
> >>>>  has been registered) ?
> >>>>
> >>>> Or more generic and thus better (in case other platforms
> >>>> later need something similar) I think, mirror the:
> >>>>
> >>>> /sys/bus/cpu/devices/cpu#/cpufreq/energy_performance_* bits
> >>>> for a system-wide energy-performance setting, so we get:
> >>>>
> >>>> /sys/power/energy_performance_preference
> >>>> /sys/power/energy_performance_available_preferences
> >>>
> >>> But this is not about energy vs performance only in general, is it?
> >>>
> >>>> (again only visible when applicable) ?
> >>>>
> >>>> I personally like the second option best.
> >>>
> >>> But I would put it under /sys/firmware/ instead of /sys/power/ and I
> >>> would call it something like platform_profile (and
> >>> platform_profile_choices or similar).
> >>
> >> Currently we only have dirs under /sys/firmware:
> >>
> >> [hans@x1 ~]$ ls /sys/firmware
> >> acpi  dmi  efi  memmap
> >>
> >> But we do have /sys/firmware/apci/pm_profile:
> >>
> >> Documentation/ABI/stable/sysfs-acpi-pmprofile
> >>
> >> What:           /sys/firmware/acpi/pm_profile
> >> Date:           03-Nov-2011
> >> KernelVersion:  v3.2
> >> Contact:        linux-acpi@vger.kernel.org
> >> Description:    The ACPI pm_profile sysfs interface exports the platform
> >>                 power management (and performance) requirement expectations
> >>                 as provided by BIOS. The integer value is directly passed as
> >>                 retrieved from the FADT ACPI table.
> >> Values:         For possible values see ACPI specification:
> >>                 5.2.9 Fixed ACPI Description Table (FADT)
> >>                 Field: Preferred_PM_Profile
> >>
> >>                 Currently these values are defined by spec:
> >>                 0 Unspecified
> >>                 1 Desktop
> >>                 2 Mobile
> >>                 3 Workstation
> >>                 4 Enterprise Server
> >>                 ...
> >>
> >> Since all platforms which we need this for are ACPI based
> >> (and the involved interfaces are also all ACPI interfaces)
> >> how about:
> >>
> >> /sys/firmware/acpi/platform_profile
> >> /sys/firmware/acpi/platform_profile_choices
> >>
> >> ?
> >>
> >> I think this goes nice together with /sys/firmware/acpi/pm_profile
> >> although that is read-only and this is a read/write setting.
> >>
> >> Rafel, would:
> >>
> >> /sys/firmware/acpi/platform_profile
> >> /sys/firmware/acpi/platform_profile_choices
> >>
> >> work for you ?
> >
> > Yes, it would.
>
> Great. So I think hat means that we have the most important part
> for moving forward with this.
>
> So I guess the plan for this now looks something like this.
>
> 1. Rewrite my API docs RFC to update it for the new /sys/firmware/acpi/platform_profile[_choices]
>    plan (should be easy and a bunch of stuff like the "type" bit can just be dropped)
>
> 2. Add code somewhere under drivers/acpi which allows code from else where
>    to register itself as platform_profile handler/provider.

Sounds good to me.

> Rafael, any suggestions / preference for where this should be added under
> drivers/acpi ?  In a new .c file perhaps ?

Yes, that would be most suitable IMV.

> 3.1 Use the code from 2 to add support for platform-profile selection in
>     thinkpad_acpi (something for me or Mark Pearson) to do
> 3.2 Use the code from 2 to add support for platform-profile selection
>     to hp-wmi
> 3.3 (and to other drivers in the future).

Right.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Fw: [External] Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class (Also Re: [PATCH 0/4] powercap/dtpm: Add the DTPM framework)
  2020-10-19 18:49                             ` Fw: [External] " Mark Pearson
@ 2020-10-25 10:13                               ` Hans de Goede
  0 siblings, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2020-10-25 10:13 UTC (permalink / raw)
  To: Mark Pearson, Rafael J. Wysocki
  Cc: Daniel Lezcano, Srinivas Pandruvada ,>, Lukasz Luba ,>,
	Linux Kernel Mailing List ,>,
	Linux PM, > Zhang, Rui, Bastien Nocera, > Mark Pearson,
	Limonciello, Mario ,>,
	Darren Hart, Andy ,> Shevchenko, Mark Gross, > Elia Devito,
	Benjamin Berg, > linux-acpi@vger.kernel.org,
	> platform-driver-x86@vger.kernel.org

Hi,

On 10/19/20 8:49 PM, Mark Pearson wrote:
> Hi
> 
>> On 19/10/2020 14:43, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 10/18/20 2:31 PM, Rafael J. Wysocki wrote:
>>> On Sun, Oct 18, 2020 at 11:41 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 10/16/20 4:51 PM, Rafael J. Wysocki wrote:
>>>>> On Fri, Oct 16, 2020 at 1:11 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>
>>>>>> <note folding the 2 threads we are having on this into one, adding every one from both threads to the Cc>
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On 10/14/20 5:42 PM, Rafael J. Wysocki wrote:
>>>>>>> On Wed, Oct 14, 2020 at 4:06 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>>> On 10/14/20 3:33 PM, Rafael J. Wysocki wrote:
>>>>>>
>>>>>> <snip>
>>>>>>
>>>>>>>>> First, a common place to register a DPTF system profile seems to be
>>>>>>>>> needed and, as I said above, I wouldn't expect more than one such
>>>>>>>>> thing to be present in the system at any given time, so it may be
>>>>>>>>> registered along with the list of supported profiles and user space
>>>>>>>>> will have to understand what they mean.
>>>>>>>>
>>>>>>>> Mostly Ack, I would still like to have an enum for DPTF system
>>>>>>>> profiles in the kernel and have a single piece of code map that
>>>>>>>> enum to profile names. This enum can then be extended as
>>>>>>>> necessary, but I want to avoid having one driver use
>>>>>>>> "Performance" and the other "performance" or one using
>>>>>>>> "performance-balanced" and the other "balanced-performance", etc.
>>>>>>>>
>>>>>>>> With the goal being that new drivers use existing values from
>>>>>>>> the enum as much as possible, but we extend it where necessary.
>>>>>>>
>>>>>>> IOW, just a table of known profile names with specific indices assigned to them.
>>>>>>
>>>>>> Yes.
>>>>>>
>>>>>>> This sounds reasonable.
>>>>>>>
>>>>>>>>> Second, irrespective of the above, it may be useful to have a
>>>>>>>>> consistent way to pass performance-vs-power preference information
>>>>>>>>> from user space to different parts of the kernel so as to allow them
>>>>>>>>> to adjust their operation and this could be done with a system-wide
>>>>>>>>> power profile attribute IMO.
>>>>>>>>
>>>>>>>> I agree, which is why I tried to tackle both things in one go,
>>>>>>>> but as you said doing both in 1 API is probably not the best idea.
>>>>>>>> So I believe we should park this second issue for now and revisit it
>>>>>>>> when we find a need for it.
>>>>>>>
>>>>>>> Agreed.
>>>>>>>
>>>>>>>> Do you have any specific userspace API in mind for the
>>>>>>>> DPTF system profile selection?
>>>>>>>
>>>>>>> Not really.
>>>>>>
>>>>>> So before /sys/power/profile was mentioned, but that seems more like
>>>>>> a thing which should have a set of fixed possible values, iow that is
>>>>>> out of scope for this discussion.
>>>>>
>>>>> Yes.
>>>>>
>>>>>> Since we all seem to agree that this is something which we need
>>>>>> specifically for DPTF profiles maybe just add:
>>>>>>
>>>>>> /sys/power/dptf_current_profile    (rw)
>>>>>> /sys/power/dptf_available_profiles (ro)
>>>>>>
>>>>>> (which will only be visible if a dptf-profile handler
>>>>>>   has been registered) ?
>>>>>>
>>>>>> Or more generic and thus better (in case other platforms
>>>>>> later need something similar) I think, mirror the:
>>>>>>
>>>>>> /sys/bus/cpu/devices/cpu#/cpufreq/energy_performance_* bits
>>>>>> for a system-wide energy-performance setting, so we get:
>>>>>>
>>>>>> /sys/power/energy_performance_preference
>>>>>> /sys/power/energy_performance_available_preferences
>>>>>
>>>>> But this is not about energy vs performance only in general, is it?
>>>>>
>>>>>> (again only visible when applicable) ?
>>>>>>
>>>>>> I personally like the second option best.
>>>>>
>>>>> But I would put it under /sys/firmware/ instead of /sys/power/ and I
>>>>> would call it something like platform_profile (and
>>>>> platform_profile_choices or similar).
>>>>
>>>> Currently we only have dirs under /sys/firmware:
>>>>
>>>> [hans@x1 ~]$ ls /sys/firmware
>>>> acpi  dmi  efi  memmap
>>>>
>>>> But we do have /sys/firmware/apci/pm_profile:
>>>>
>>>> Documentation/ABI/stable/sysfs-acpi-pmprofile
>>>>
>>>> What:           /sys/firmware/acpi/pm_profile
>>>> Date:           03-Nov-2011
>>>> KernelVersion:  v3.2
>>>> Contact:        linux-acpi@vger.kernel.org
>>>> Description:    The ACPI pm_profile sysfs interface exports the platform
>>>>                  power management (and performance) requirement expectations
>>>>                  as provided by BIOS. The integer value is directly passed as
>>>>                  retrieved from the FADT ACPI table.
>>>> Values:         For possible values see ACPI specification:
>>>>                  5.2.9 Fixed ACPI Description Table (FADT)
>>>>                  Field: Preferred_PM_Profile
>>>>
>>>>                  Currently these values are defined by spec:
>>>>                  0 Unspecified
>>>>                  1 Desktop
>>>>                  2 Mobile
>>>>                  3 Workstation
>>>>                  4 Enterprise Server
>>>>                  ...
>>>>
>>>> Since all platforms which we need this for are ACPI based
>>>> (and the involved interfaces are also all ACPI interfaces)
>>>> how about:
>>>>
>>>> /sys/firmware/acpi/platform_profile
>>>> /sys/firmware/acpi/platform_profile_choices
>>>>
>>>> ?
>>>>
>>>> I think this goes nice together with /sys/firmware/acpi/pm_profile
>>>> although that is read-only and this is a read/write setting.
>>>>
>>>> Rafel, would:
>>>>
>>>> /sys/firmware/acpi/platform_profile
>>>> /sys/firmware/acpi/platform_profile_choices
>>>>
>>>> work for you ?
>>>
>>> Yes, it would.
>>
>> Great. So I think hat means that we have the most important part
>> for moving forward with this.
>>
>> So I guess the plan for this now looks something like this.
>>
>> 1. Rewrite my API docs RFC to update it for the new /sys/firmware/acpi/platform_profile[_choices]
>>     plan (should be easy and a bunch of stuff like the "type" bit can just be dropped)
>>
>> 2. Add code somewhere under drivers/acpi which allows code from else where
>>     to register itself as platform_profile handler/provider.
>>
>> Rafael, any suggestions / preference for where this should be added under
>> drivers/acpi ?  In a new .c file perhaps ?
>>
>> 3.1 Use the code from 2 to add support for platform-profile selection in
>>      thinkpad_acpi (something for me or Mark Pearson) to do
>> 3.2 Use the code from 2 to add support for platform-profile selection
>>      to hp-wmi
>> 3.3 (and to other drivers in the future).
>>
>>
>> An open question is who will take care of 1. and 2. Mark (Pearson)
>> do you feel up to this? or do you want me to take care of this?
>>
>> Regards,
>>
>> Hans
>>
> 
> Definitely up for (2) and will happily have a go at number (1).
> 
> If there's an example of something similar I can look at for reference that would be helpful :)

So what I would do is something like this:

1.  Create a new include/acpi/platform_profile_provider.h file and
    in that file define:
1.1 An enum with possible profile values (as discussed we want the driver
    API to use an enum (which may be extended) and then use an array with
    strings inside the shared code to avoid differences like "Performance"
    vs "performance", etc. The enum should end with something like:
    ACPI_PLATFORM_PROFILE_COUNT
1.2 An acpi_platform_profile_provider struct in which contains a number of
    function-pointers for set/get callbacks (these callbacks should get/set
    the enum type, not strings) a "void *user_data" (to be passed back to the callbacks)
    and a: "unsigned long profile_choices[BITS_TO_LONGS(ACPI_PLATFORM_PROFILE_COUNT)]
    member.
1.3 A function to register / unregister a platform_profile_provider

2. Add a new .c file for this under drivers/acpi which should
   contain the actual implementation of the API (left to you) and at least
   2 global variables a "struct acpi_platform_profile_provider *profile_provider;"
   and a mutex protecting this. The register/unregister function should lock
   the mutex to protect the pointer and the register function should check that
   this is the first provider being registered if there already is a provider
   registered then -EBUSY should be returned.
   The register/unregister functions should also add / remove the 2
   /sys/firmware/acpi/platform_profile[_choices] files.

   Hint for the read function of the platform_profile_choices function you
   should use for_each_set_bit(bit, profile_choices, ACPI_PLATFORM_PROFILE_COUNT) {}

Note this is more or less what I would do (minus any changes I would come up
with when implementing this), feel free to use your own judgement here.

Rafael, do you have any comments on this approach ?

Regards,

Hans


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-10-25 10:13 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20201006122024.14539-1-daniel.lezcano@linaro.org>
     [not found] ` <eb26a00d-eee0-a4d1-ed25-61a661ad5683@redhat.com>
     [not found]   ` <8be66efd-7833-2c8a-427d-b0055c2f6ec1@linaro.org>
     [not found]     ` <97e5368b-228d-eca1-85a5-b918dfcfd336@redhat.com>
     [not found]       ` <CAJZ5v0gwc_d1vnwDVWXY+i4f0T2r0tAz8xuWV7oS_afsy7OocQ@mail.gmail.com>
     [not found]         ` <63dfa6a1-0424-7985-7803-756c0c5cc4a5@redhat.com>
     [not found]           ` <CAJZ5v0jpYpu3Tk7qq_MCVs0wUr-Dw0rY5EZELrVbQta0NZaoVA@mail.gmail.com>
     [not found]             ` <87d9a808-39d6-4949-c4f9-6a80d14a3768@redhat.com>
     [not found]               ` <CAJZ5v0iWmmu5WV7cX7uNb61NMYQ7s0dnhg1K+T0x90b3sBfU9w@mail.gmail.com>
2020-10-16 11:10                 ` [RFC] Documentation: Add documentation for new performance_profile sysfs class (Also Re: [PATCH 0/4] powercap/dtpm: Add the DTPM framework) Hans de Goede
2020-10-16 14:26                   ` Elia Devito
     [not found]                     ` <HK2PR0302MB2449214B28438ADC1790D468BD030@HK2PR0302MB2449.apcprd03.prod.outlook.com>
2020-10-16 14:43                       ` Fw: [External] " Mark Pearson
2020-10-16 15:16                         ` Elia Devito
2020-10-16 14:51                   ` Rafael J. Wysocki
2020-10-18  9:41                     ` Hans de Goede
2020-10-18 12:31                       ` Rafael J. Wysocki
2020-10-19 18:43                         ` Hans de Goede
     [not found]                           ` <HK2PR0302MB24494037019FBC7720976735BD1E0@HK2PR0302MB2449.apcprd03.prod.outlook.com>
2020-10-19 18:49                             ` Fw: [External] " Mark Pearson
2020-10-25 10:13                               ` Hans de Goede
2020-10-20 12:34                           ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).