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

[Elia Devito]

Hi,

In data mercoledì 14 ottobre 2020 17:46:43 CEST, Rafael J. Wysocki ha scritto:
> On Wed, Oct 14, 2020 at 4:16 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> > Hi,
> > 
> > On 10/14/20 3:55 PM, Rafael J. Wysocki wrote:
> > > On Tue, Oct 13, 2020 at 3:09 PM Hans de Goede <hdegoede@xxxxxxxxxx> 
wrote:
> > >> Hi,
> > >> 
> > >> On 10/12/20 6:42 PM, Rafael J. Wysocki wrote:
> > >>> On Wed, Oct 7, 2020 at 8:41 PM Limonciello, Mario
> > >>> 
> > >>> <Mario.Limonciello@xxxxxxxx> wrote:
> > >>>>> On Wed, 2020-10-07 at 15:58 +0000, Limonciello, Mario wrote:
> > >>>>>>> On Mon, 2020-10-05 at 12:58 +0000, Limonciello, Mario wrote:
> > >>>>>>>>> On modern systems CPU/GPU/... performance is often dynamically
> > >>>>>>>>> configurable
> > >>>>>>>>> in the form of e.g. variable clock-speeds and TPD. The
> > >>>>>>>>> performance
> > >>>>>>>>> is often
> > >>>>>>>>> automatically adjusted to the load by some automatic-mechanism
> > >>>>>>>>> (which may
> > >>>>>>>>> very well live outside the kernel).
> > >>>>>>>>> 
> > >>>>>>>>> These auto performance-adjustment mechanisms often can be
> > >>>>>>>>> configured with
> > >>>>>>>>> one of several performance-profiles, with either a bias towards
> > >>>>>>>>> low-power
> > >>>>>>>>> consumption (and cool and quiet) or towards performance (and
> > >>>>>>>>> higher
> > >>>>>>>>> power
> > >>>>>>>>> consumption and thermals).
> > >>>>>>>>> 
> > >>>>>>>>> Introduce a new performance_profile class/sysfs API which
> > >>>>>>>>> offers a
> > >>>>>>>>> generic
> > >>>>>>>>> API for selecting the performance-profile of these automatic-
> > >>>>>>>>> mechanisms.
> > >>>>>>>> 
> > >>>>>>>> If introducing an API for this - let me ask the question, why
> > >>>>>>>> even let each
> > >>>>>>>> driver offer a class interface and userspace need to change
> > >>>>>>>> "each" driver's
> > >>>>>>>> performance setting?
> > >>>>>>>> 
> > >>>>>>>> I would think that you could just offer something kernel-wide
> > >>>>>>>> like
> > >>>>>>>> /sys/power/performance-profile
> > >>>>>>>> 
> > >>>>>>>> Userspace can read and write to a single file.  All drivers can
> > >>>>>>>> get notified
> > >>>>>>>> on this sysfs file changing.
> > >>>>>>>> 
> > >>>>>>>> The systems that react in firmware (such as the two that prompted
> > >>>>>>>> this discussion) can change at that time.  It leaves the
> > >>>>>>>> possibility for a
> > >>>>>>>> more open kernel implementation that can do the same thing though
> > >>>>>>>> too by
> > >>>>>>>> directly modifying device registers instead of ACPI devices.
> > >>>>>>> 
> > >>>>>>> The problem, as I've mentioned in previous discussions we had
> > >>>>>>> about
> > >>>>>>> this, is that, as you've seen in replies to this mail, this would
> > >>>>>>> suddenly be making the kernel apply policy.
> > >>>>>>> 
> > >>>>>>> There's going to be pushback as soon as policy is enacted in the
> > >>>>>>> kernel, and you take away the different knobs for individual
> > >>>>>>> components
> > >>>>>>> (or you can control them centrally as well as individually). As
> > >>>>>>> much as
> > >>>>>>> I hate the quantity of knobs[1], I don't think that trying to
> > >>>>>>> reduce
> > >>>>>>> the number of knobs in the kernel is a good use of our time, and
> > >>>>>>> easier
> > >>>>>>> to enact, coordinated with design targets, in user-space.
> > >>>>>>> 
> > >>>>>>> Unless you can think of a way to implement this kernel wide
> > >>>>>>> setting
> > >>>>>>> without adding one more exponent on the number of possibilities
> > >>>>>>> for
> > >>>>>>> the
> > >>>>>>> testing matrix, I'll +1 Hans' original API.
> > >>>>>> 
> > >>>>>> Actually I offered two proposals in my reply.  So are you NAKing
> > >>>>>> both?
> > >>>>> 
> > >>>>> No, this is only about the first portion of the email, which I
> > >>>>> quoted.
> > >>>>> And I'm not NAK'ing it, but I don't see how it can work without
> > >>>>> being
> > >>>>> antithetical to what kernel "users" expect, or what the folks
> > >>>>> consuming
> > >>>>> those interfaces (presumably us both) would expect to be able to
> > >>>>> test
> > >>>>> and maintain.
> > >>>> 
> > >>>> (Just so others are aware, Bastien and I had a previous discussion on
> > >>>> this topic that he alluded to here:
> > >>>> https://gitlab.freedesktop.org/hadess/power-profiles-daemon/-/issues
> > >>>> /1)
> > >>>> 
> > >>>> In general I agree that we shouldn't be offering 100's of knobs to
> > >>>> change
> > >>>> things and protect users from themselves where possible.
> > >>>> 
> > >>>> Whether the decisions are made in the kernel or in userspace you
> > >>>> still have a matrix once you're letting someone change 2 different
> > >>>> kernel devices that offer policy.  I'd argue it's actually worse if
> > >>>> you let userspace change it though.
> > >>>> 
> > >>>> Let's go back to the my GPU and platform example and lets say both
> > >>>> offer the new knob here for both.  Userspace software such as your
> > >>>> PPD picks performance.  Both the platform device and GPU device get
> > >>>> changed, hopefully no conflicts.
> > >>>> Then user decides no, I don't want my GPU in performance mode, I only
> > >>>> want my platform. So they change the knob for the GPU manually, and
> > >>>> now you have a new config in your matrix.
> > >>>> 
> > >>>> However if you left it to a single kernel knob, both GPU and platform
> > >>>> get moved together and you don't have these extra configs in your
> > >>>> matrix anymore.
> > >>>> 
> > >>>> The other point I mentioned, that platform might also do something to
> > >>>> GPU via a sideband and you race, you can solve it with kernel too by
> > >>>> modifying the ordering the kernel handles it.
> > >>>> 
> > >>>> Userspace however, you give two knobs and now you have to worry about
> > >>>> them getting it right and supporting them doing them in the wrong
> > >>>> order.
> > >>>> 
> > >>>>>> The other one suggested to use the same firmware attributes class
> > >>>>>> being
> > >>>>>> introduced by the new Dell driver (
> > >>>>>> https://patchwork.kernel.org/patch/11818343/)
> > >>>>>> since this is actually a knob to a specific firmware setting.
> > >>>>> 
> > >>>>> This seemed to me like an implementation detail (eg. the same
> > >>>>> metadata
> > >>>>> is being exported, but in a different way), and I don't feel
> > >>>>> strongly
> > >>>>> about it either way.
> > >>>> 
> > >>>> OK thanks.
> > >>> 
> > >>> IMV there are two choices here:  One is between exposing the low-level
> > >>> interfaces verbatim to user space and wrapping them up into a certain
> > >>> "translation" layer allowing user space to use a unified interface (I
> > >>> think that is what everybody wants) and the other  boils down to how
> > >>> the unified interface between the kernel and user space will look
> > >>> like.
> > >>> 
> > >>> Personally, I think that something line /sys/power/profile allowing
> > >>> drivers (and other kernel entities) to register callbacks might work
> > >>> (as stated in my last reply to Hans).
> > >> 
> > >> Note to others reading along I pointed to this thread in this thread:
> > >> https://lore.kernel.org/linux-pm/20201006122024.14539-1-daniel.lezcano@
> > >> linaro.org/T/#t and Rafael's "last reply" above refers to his reply in
> > >> that thread.
> > >> 
> > >> For the sake of people reading along I'm reproducing my reply
> > >> there below.
> > > 
> > > For completeness, my response in the other thread is here:
> > > 
> > > https://lore.kernel.org/linux-pm/CAJZ5v0jpYpu3Tk7qq_MCVs0wUr-Dw0rY5EZELr
> > > VbQta0NZaoVA@xxxxxxxxxxxxxx/T/#t> > 
> > >> Rafael, it seems more appropriate to continue this discussion
> > >> in this thread, so lets discuss this further here ?
> > > 
> > > And because I sent it before reading this message, let me reproduce it
> > > below (with some additions).
> > 
> > And I just did the same thing (replied to your reply in the other thread),
> > I guess I was too quick.
> 
> Well, same here.
> 
> > So I too will reproduce my reply here.
> 
> And so I'm doing below.
> 
> > And I still believe it is best to then stick to this thread from now on,
> > because this reproducing thing is not really productive...
> > 
> > >> My reply to Rafael from the other thread:
> > >> 
> > >> First of all thank you for your input, with your expertise in this
> > >> area your input is very much appreciated, after all we only get
> > >> one chance to get the userspace API for this right.
> > >> 
> > >> Your proposal to have a single sysfs file for userspace to talk
> > >> to and then use an in kernel subscription mechanism for drivers
> > >> to get notified of writes to this file is interesting.
> > >> 
> > >> But I see 2 issues with it:
> > >> 
> > >> 1. How will userspace know which profiles are actually available ?
> > >> 
> > >> An obvious solution is to pick a set of standard names and let
> > >> subscribers map those as close to their own settings as possible,
> > >> the most often mentioned set of profile names in this case seems to be:
> > >> 
> > >> low_power
> > >> balanced_power
> > >> balanced
> > >> balanced_performance
> > >> performance
> > >> 
> > >> Which works fine for the thinkpad_acpi case, but not so much for
> > >> the hp-wmi case. In the HP case what happens is that a WMI call
> > >> is made which sets a bunch of ACPI variables which influence
> > >> the DPTF code (this assumes we have some sort of DPTF support
> > >> such as mjg59's reverse engineered support) but the profile-names
> > >> under Windows are: "Performance", "HP recommended", "Cool" and
> > >> "Quiet".  If you read the discussion from the
> > >> "[RFC] Documentation: Add documentation for new performance_profile
> > >> sysfs class" thread you will see this was brought up as an issue
> > >> there.
> > > 
> > > Two different things seem to be conflated here.  One is how to pass a
> > > possible performance-vs-power preference coming from user space down
> > > to device drivers or generally pieces of kernel code that can adjust
> > > the behavior and/or hardware settings depending on what that
> > > preference is and the other is how to expose OEM-provided DPTF system
> > > profile interfaces to user space.
> > 
> > I was hoping / thinking that we could use a single API for both of
> > these. But I guess that it makes sense to see them as 2 separate
> > things, esp. since DPTF profiles seem to be somewhat free-form
> > where as a way to pass a performance-pref to a device could use
> > a fixes set of values.
> > 
> > So lets say that we indeed want to treat these 2 separately,
> > then I guess that the issue at hand / my reason to start a
> > discussion surrounding this is allowing userspace to selecting
> > the DPTF system profile.
> > 
> > The thinkpad_acpi case at hand is not using DPTF, but that is
> > because Lenovo decided to implement dynamic DPTF like behavior
> > inside their embedded controller (for when running Linux) since
> > DPTF is atm not really supported all that well under Linux and
> > Lenovo was getting a lot of complaints about sub-optimal
> > performance because of this.
> > 
> > So the thinkpad_acpi solution is in essence a replacement
> > for DPTF and it should thus use the same userspace API as
> > other mechanisms to select DPTF system profiles.
> > 
> > And if we limit this new userspace API solely to setting DPTF
> > system profiles, then their will indeed be only 1 provider for
> > this for the entire system.
> > 
> > > The former assumes that there is a common set of values that can be
> > > understood and acted on in a consistent way by all of the interested
> > > entities within the kernel and the latter is about passing information
> > > from user space down to a side-band power control mechanism working in
> > > its own way behind the kernel's back (and possibly poking at multiple
> > > hardware components in the platform in its own way).
> > 
> > Ack.
> > 
> > > IMO there is no way to provide a common interface covering these two
> > > cases at the same time.
> > 
> > I see your point, esp. the free form vs common set of values
> > argument seems to be exactly what we have been going in circles
> > about during the discussion about this so far.
> > 
> > >> The problem here is that both "cool" and "quiet" could be
> > >> interpreted as low-power. But it seems that they actually mean
> > >> what they say, cool focuses on keeping temps low, which can
> > >> also be done by making the fan-profile more aggressive. And quiet
> > >> is mostly about keeping fan speeds down, at the cost of possible
> > >> higher temperatures.
> > >> 
> > >> <edit in this version of the reply:>
> > >> I wonder if the HP profiles are actually just fan speed profiles ?
> > >> Elia do you know ?
> > >> </edit>
> > > 
> > > I don't think so.
> > > 
> > > AFAICS, in both the Thinkpad and HP cases the profile covers the
> > > entire platform, which in particular means that they cannot co-exist.
> > 
> > Ok.
> > 

As far as I know, the profiles affect  the thermal behavior like "how long to 
wait before starting the fan and at what temperature" or "how fast to run the 
fan with the current cpu load and temperature".

The only way that firmware uses to "control" performance should be the odvp0 
DPTF variable.

On Windows HP expose  both "Cool" and "Quiet" profile respectively as "Ideal 
for when the computer feels warm to the touch" and "Ideal for quiet 
environments".

more detail: https://support.hp.com/in-en/document/c06063108

To be precise "Quiet" profile should be available only on platform without 
dedicate GPU (I'm investigating if there is other case), instead the other 3 
profiles ("HP Recommended", "Performance" and "Cool") are available on all 
platform that support thermal profile.

reading here seems that Dell offer identical profiles:
https://www.dell.com/support/manuals/it/it/itbsdt1/dell-command-power-manager-v2.2/userguide_dell/thermal-management?guid=guid-c05d2582-fc07-4e3e-918a-965836d20752&lang=en-us 

Regards,

Elia