Hi Hans,
On Tue, Oct 13, 2020 at 3:04 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
Hi Rafael,
On 10/12/20 6:37 PM, Rafael J. Wysocki wrote:
On Mon, Oct 12, 2020 at 1:46 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
<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 ?
It doesn't need to work this way IMV.
It may also work by allowing drivers (or whatever kernel entities are
interested in that) to subscribe to it, so that they get notified
whenever a new value is written to it by user space (eg. each driver
may be able to register a callback to be invoked when that happens).
The information coming from user space will just be passed to the
subscribers of that interface and they will do about it what they want
(eg. it may be translated into a value to be written to a
performance-vs-power interface provided by the platform or similar).
This really is similar to having a class interface with one file per
"subscribed" device except that the aggregation is done in the kernel
and not in user space and the subscribers need not be related to
specific devices. It still allows to avoid exposing the low-level
interfaces to user space verbatim and it just passes the "policy"
choice from user space down to the entities that can take it into
account.
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.
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).
IMO there is no way to provide a common interface covering these two
cases at the same time.
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. IOW we don't really have a 1 dimensional
axis.
Well, AFAICS, DPTF system profile interfaces coming from different
OEMs will be different, but they are about side-band power control and
there can be only one thing like that in a platform at the same time.
My class proposal fixes this by having a notion of both
standardized names (because anything else would suck) combined
with a way for drivers to advertise which standardized names
the support. So in my proposal I simply add quiet and cool
to the list of standard profile names, and then the HP-wmi
driver can list those as supported, while not listing
low_power as a supported profile. This way we export the
hardware interface to userspace as is (as much as possible)
while still offering a standardized interface for userspace
to consume. Granted if userspace now actually want to set
a low_power profile, we have just punted the problem to userspace
but I really do not see a better solution.
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.
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.