Re: [PATCH 0/4] powercap/dtpm: Add the DTPM framework

[Hans de Goede]

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