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-25 Thread Hans de Goede
Hi,

On 10/19/20 8:49 PM, Mark Pearson wrote:
> Hi
> 
>> On 19/10/2020 14:43, Hans de Goede  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  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  wrote:
>>
>> > 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  
>>> wrote:
 On 10/14/20 3:33 PM, Rafael J. Wysocki wrote:
>>
>> 
>>
> 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-a...@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 

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 Thread Mark Pearson

Hi

> On 19/10/2020 14:43, Hans de Goede  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  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  wrote:




Hi,

On 10/14/20 5:42 PM, Rafael J. Wysocki wrote:

On Wed, Oct 14, 2020 at 4:06 PM Hans de Goede  wrote:

On 10/14/20 3:33 PM, Rafael J. Wysocki wrote:





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

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 Thread Elia Devito
Hi,

In data venerdì 16 ottobre 2020 16:43:09 CEST, Mark Pearson ha scritto:
>  based address>
> 
> On 2020-10-16 10:32 a.m., Mark Pearson wrote:
> > 
> > *From:* Elia Devito 
> > *Sent:* October 16, 2020 10:26
> > *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-a...@vger.kernel.org
> > ; platform-driver-...@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:
> >>  >> 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  
wrote:
> >> >> On 10/14/20 3:33 PM, Rafael J. Wysocki wrote:
> >> 
> >> 
> >> >>> 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




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 Thread Mark Pearson
based address>


On 2020-10-16 10:32 a.m., Mark Pearson wrote:




*From:* Elia Devito 
*Sent:* October 16, 2020 10:26
*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-a...@vger.kernel.org 
; platform-driver-...@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:



Hi,

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


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


Re: [External] RE: [RFC] Documentation: Add documentation for new performance_profile sysfs class

2020-10-05 Thread Mark Pearson




On 2020-10-05 12:56 p.m., Limonciello, Mario wrote:


When implemented for the two vendors mentioned here, it would be using a
proprietary "firmware API" implemented by those two vendors.  For example

write

arguments (0x1, 0x2) to ACPI-WMI method WMFT and it will cause firmware to

coordinate

using undisclosed protocol to affect the platform changes desirable.

This is different in my mind from "kernel writes to a specific register" to

set

power properties of a specific device.



Just curious on this point - isn't that (mostly) what all hardware does?
You write to it and the device does "stuff" to achieve the required
effect. Yes this is in proprietary firmware, but from my experience with
hardware devices that's not uncommon these days anyway.



Yes I agree.  Even "register" writes to a device are actually an API and
something in the hardware monitors those registers and does something as a
result.


Let me know if I'm misunderstanding something here. I couldn't see the
difference between a register written to via ACPI and one written to via
some other protocol (SMBUS? or whatever)

Mark



The reason I'm calling out a distinction here is that "platform" and "device"
can cover a lot more things.  In this case it's an API provided by the 
platform's
firmware, not an individual device's firmware.  So you can't actually guarantee
what the platform's firmware did.  It could have sent any number of sideband
commands to devices that it controls.  The "platform" could have potentially
told the GPU to turn up its fans, or lower it's clock as a result of this, but 
you
can't possibly know.

However if we go the GPU example alone, it's a specific single device you're
controlling.  You put the GPU into the characterization that you expected and it
operates that way.


Got it - fair enough :) Thanks for the explain.
Thanks
Mark


RE: [External] RE: [RFC] Documentation: Add documentation for new performance_profile sysfs class

2020-10-05 Thread Limonciello, Mario
> On 2020-10-05 12:11 p.m., Limonciello, Mario wrote:
> >>
> >> Excuse my ignorance, but I don't really see why this interface would be
> tied
> >> to
> >> ACPI devices? Why is it not possible to write a driver that implements this
> >> interface
> >> and directly modifies device registers? Am I missing something obvious
> here?
> >>
> >
> > When implemented for the two vendors mentioned here, it would be using a
> > proprietary "firmware API" implemented by those two vendors.  For example
> write
> > arguments (0x1, 0x2) to ACPI-WMI method WMFT and it will cause firmware to
> coordinate
> > using undisclosed protocol to affect the platform changes desirable.
> >
> > This is different in my mind from "kernel writes to a specific register" to
> set
> > power properties of a specific device.
> >
> 
> Just curious on this point - isn't that (mostly) what all hardware does?
> You write to it and the device does "stuff" to achieve the required
> effect. Yes this is in proprietary firmware, but from my experience with
> hardware devices that's not uncommon these days anyway.
> 

Yes I agree.  Even "register" writes to a device are actually an API and
something in the hardware monitors those registers and does something as a
result.

> Let me know if I'm misunderstanding something here. I couldn't see the
> difference between a register written to via ACPI and one written to via
> some other protocol (SMBUS? or whatever)
> 
> Mark
> 

The reason I'm calling out a distinction here is that "platform" and "device"
can cover a lot more things.  In this case it's an API provided by the 
platform's
firmware, not an individual device's firmware.  So you can't actually guarantee
what the platform's firmware did.  It could have sent any number of sideband
commands to devices that it controls.  The "platform" could have potentially
told the GPU to turn up its fans, or lower it's clock as a result of this, but 
you
can't possibly know.

However if we go the GPU example alone, it's a specific single device you're
controlling.  You put the GPU into the characterization that you expected and it
operates that way.


Re: [External] RE: [RFC] Documentation: Add documentation for new performance_profile sysfs class

2020-10-05 Thread Mark Pearson




On 2020-10-05 12:11 p.m., Limonciello, Mario wrote:


Excuse my ignorance, but I don't really see why this interface would be tied
to
ACPI devices? Why is it not possible to write a driver that implements this
interface
and directly modifies device registers? Am I missing something obvious here?



When implemented for the two vendors mentioned here, it would be using a
proprietary "firmware API" implemented by those two vendors.  For example write
arguments (0x1, 0x2) to ACPI-WMI method WMFT and it will cause firmware to 
coordinate
using undisclosed protocol to affect the platform changes desirable.

This is different in my mind from "kernel writes to a specific register" to set
power properties of a specific device.



Just curious on this point - isn't that (mostly) what all hardware does? 
You write to it and the device does "stuff" to achieve the required 
effect. Yes this is in proprietary firmware, but from my experience with 
hardware devices that's not uncommon these days anyway.


Let me know if I'm misunderstanding something here. I couldn't see the 
difference between a register written to via ACPI and one written to via 
some other protocol (SMBUS? or whatever)


Mark