linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Daniel Lezcano <daniel.lezcano@linaro.org>,
	rafael@kernel.org, srinivas.pandruvada@linux.intel.com
Cc: lukasz.luba@arm.com, linux-kernel@vger.kernel.org,
	linux-pm@vger.kernel.org, rui.zhang@intel.com,
	Mark Pearson <mpearson@lenovo.com>
Subject: Re: [PATCH 0/4] powercap/dtpm: Add the DTPM framework
Date: Tue, 13 Oct 2020 14:47:44 +0200	[thread overview]
Message-ID: <15da0ac7-c992-067c-f101-9775bce717e0@redhat.com> (raw)
In-Reply-To: <bc1a0134-5242-65d7-a753-fbec0d3bb327@linaro.org>

Hi,

On 10/12/20 6:02 PM, Daniel Lezcano wrote:
> On 12/10/2020 13:46, Hans de Goede wrote:
>> Hi Daniel,
>>
>> On 10/12/20 12:30 PM, Daniel Lezcano wrote:

<snip>

>>> Here the proposed interface is already exported in userspace via the
>>> powercap framework which supports today the backend driver for the RAPL
>>> register.
>>
>> You say that some sort of power/ balanced power / balanced /
>> balanced performance / performance setting in is already exported
>> through the powercap interface today (if I understand you correctly)?
> 
> Sorry, I was unclear. I meant 'Here the proposed interface' referring to
> the powercap/dtpm. There is no profile interface in the powercap framework.

Ah, I see.

<snip>

>>> A side note, related to your proposal, not this patch. IMO it suits
>>> better to have /sys/power/profile.
>>>
>>> cat /sys/power/profile
>>>
>>> power
>>> balanced_power *
>>> balanced
>>> balanced_performance
>>> performance
>>>
>>> The (*) being the active profile.
>>
>> Interesting the same thing was brought up in the discussion surrounding
>> RFC which I posted.
>>
>> The downside against this approach is that it assumes that there
>> only is a single system-wide settings. AFAIK that is not always
>> the case, e.g. (AFAIK):
>>
>> 1. The intel pstate driver has something like this
>>     (might this be the rapl setting you mean? )
>>
>> 2. The X1C8 has such a setting for the embedded-controller, controlled
>>     through the ACPI interfaces which thinkpad-acpi used
>>
>> 3. The hp-wmi interface allows selecting a profile which in turn
>>     (through AML code) sets a bunch of variables which influence how
>>     the (dynamic, through mjg59's patches) DPTF code controls various
>>     things
>>
>> At least the pstate setting and the vendor specific settings can
>> co-exist. Also the powercap API has a notion of zones, I can see the
>> same thing here, with a desktop e.g. having separate performance-profile
>> selection for the CPU and a discrete GPU.
>>
>> So limiting the API to a single /sys/power/profile setting seems a
>> bit limited and I have the feeling we will regret making this
>> choice in the future.
>>
>> With that said your proposal would work well for the current
>> thinkpad_acpi / hp-wmi cases, so I'm not 100% against it.
>>
>> This would require adding some internal API to the code which
>> owns the /sys/power root-dir to allow registering a profile
>> provider I guess. But that would also immediately bring the
>> question, what if multiple drivers try to register themselves
>> as /sys/power/profile provider ?
> 
> Did you consider putting the profile on a per device basis ?
> 
> eg.
> 
> /sys/devices/system/cpu/cpu[0-9]/power/profile
> 
> May be make 'energy_performance_preference' obsolete in
> /sys/devices/system/cpu/cpufreq ?
> 
> When one device sets the profile, all children will have the same profile.
> 
> eg.
> 
> A change in /sys/devices/system/cpu/power/profile will impact all the
> underlying cpu[0-9]/power/profile
> 
> Or a change in /sys/devices/power/profile will change all profiles below
> /sys/devices.
> 
> Well that is a high level suggestion, I don't know how that can fit with
> the cyclic sysfs hierarchy.

A problem with I see with making this a per-device power setting is that
only a few devices will actually have this; and then the question becomes
how does userspace discover / find these devices ? Typically for these kinda
discovery problems we use a sysfs class as a solution to group devices
offering the same functionailty (through the same standardized sysfs API)
together. Which circles back to my original RFC proposal for this.

Regards,

Hans


  reply	other threads:[~2020-10-13 12:47 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-06 12:20 [PATCH 0/4] powercap/dtpm: Add the DTPM framework Daniel Lezcano
2020-10-06 12:20 ` [PATCH 1/4] units: Add Watt units Daniel Lezcano
2020-11-10 10:02   ` Lukasz Luba
2020-10-06 12:20 ` [PATCH 2/4] Documentation/powercap/dtpm: Add documentation for dtpm Daniel Lezcano
2020-10-13 22:01   ` Ram Chandrasekar
2020-10-06 12:20 ` [PATCH 3/4] powercap/drivers/dtpm: Add API for dynamic thermal power management Daniel Lezcano
2020-10-06 16:42   ` kernel test robot
2020-10-06 18:05   ` kernel test robot
2020-10-23 10:29   ` Lukasz Luba
2020-11-03 18:42     ` Daniel Lezcano
2020-11-10  9:59   ` Lukasz Luba
2020-11-10 11:05     ` Lukasz Luba
2020-11-10 14:59       ` Daniel Lezcano
2020-11-10 15:04         ` Lukasz Luba
2020-11-10 12:55     ` Daniel Lezcano
2020-10-06 12:20 ` [PATCH 4/4] powercap/drivers/dtpm: Add CPU energy model based support Daniel Lezcano
2020-10-23 13:27   ` Lukasz Luba
2020-11-04 10:47     ` Daniel Lezcano
2020-11-04 10:57       ` Lukasz Luba
2020-11-04 11:15         ` Daniel Lezcano
2020-11-10 12:50   ` Lukasz Luba
2020-10-07 10:43 ` [PATCH 0/4] powercap/dtpm: Add the DTPM framework Hans de Goede
2020-10-12 10:30   ` Daniel Lezcano
2020-10-12 11:46     ` Hans de Goede
2020-10-12 16:02       ` Daniel Lezcano
2020-10-13 12:47         ` Hans de Goede [this message]
2020-10-12 16:37       ` Rafael J. Wysocki
2020-10-13 13:04         ` Hans de Goede
2020-10-14 13:33           ` Rafael J. Wysocki
2020-10-14 14:06             ` Hans de Goede
2020-10-14 15:42               ` Rafael J. Wysocki
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=15da0ac7-c992-067c-f101-9775bce717e0@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=lukasz.luba@arm.com \
    --cc=mpearson@lenovo.com \
    --cc=rafael@kernel.org \
    --cc=rui.zhang@intel.com \
    --cc=srinivas.pandruvada@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).