Hi!
The PowerOP infrastructure you suggest surely is one path to better runtime
power management in the Linux kernel. However, I don't like it at all in its
current implementation. Here are a few suggestions for improvements,
rewrites, and so on:
First, the table interface you suggest is ugly. If there's indeed the need for
such an abstraction, I'd favour something like
struct powerop {
struct list_head powerop_values; /* linked list of powerop_values */
...
}
struct powerop_value {
unsigned long value_cur;
unsigned long value_min;
unsigned long value_max;
struct list_head next;
u16 type;
struct powerop_value *cross_dependency;
struct powerop_driver *driver;
}
#define POWEROP_TYPE_CPU_FREQUENCY 0x00000001
#define POWEROP_TYPE_CPU_VOLTAGE 0x00000002
#define POWEROP_TYPE_FRONT_SIDE_BUS_SPEED 0x00000004
...
#define POWEROP_TYPE_GPU_FREQUENCY 0x00010000
...
and if CPU_VOLTAGE and CPU_FREQEUNCY can only be modified at the same time, (as
most cpufreq drivers require), type is 0x00000003.
Secondly, you do not adress the cross-relationships between operation points
correctly. If you change the CPU frequency, you may have to switch other
(memory, video) settings; you might even have to validate the frequency
settings for these or even additional reasons (thermal and battery reasons -
ACPI _PPC).
Thirdly, who is to decide on the power management settings? The first and
intuitive answer is the kernel. Therefore, kernel-space cpufreq governors
exist. Only under rare circumstances, you want full userspace control --
that's what the userspace cpufreq governor is for.
Foruthly, the code duplication which your implementation leads to is obvious
for the speedstep-centrino case. And in contrast to Pavel, I do not consider
it a "tiny cleanup".
I'd suggest that you try upgrading the cpufreq infrastructure to provide
full support for multiple types of POWEROPs:
a) Setting of "policies"
- New "min" or "max" values for all powerop_values are set, verified
by powerop lowlevel drivers, powerop governors and external
notifiers. E.g. if a new frequency min/max pair is required, the
voltage level gets a new min and max value as well --> you need to
handle recursion.
- If necessary a new "powerop governor" is started.
- Each powerop governor specifies which POWEROPs it can handle
- current cpufreq governors can handle CPU_FREQUENCY,
CPU_VOLTAGE and FRONT_SIDE_BUS_SPEED
- an userspace fallback-governor always "handles" the
parameters no other governor handles
b) Setting of "values"
- Each governor can initiate transitions between the "min" and "max"
values for operationg points it aquired ownership for.
- The new setting is notified to all other governors and to external
notifiers. If some entitiy decides it cannot live well with this
new setting, it breaks out. Note that this should not happen quite
often, as the "normal" verification takes place in a) above.
Nonetheless, if you want to break out CPU_VOLTAGE and CPU_FREQUENCY, you
need it. And as it makes life for the kernel so much more
difficult, I'm against doing so.
- The low-level driver handling the powerop_value is called
Thanks,
Dominik
Dominik Brodowski wrote:
First, the table interface you suggest is ugly. If there's indeed the need for
such an abstraction, I'd favour something like
I'm planning to adopt the previous suggestions of an opaque data
structure and stop trying to have any generic structure to it. I'll try
to leave dependency checking etc. to the upper layers as much as
possible, since platforms vary greatly in this and so do the needs of
different PM s/w stacks.
Secondly, you do not adress the cross-relationships between operation points
correctly. If you change the CPU frequency, you may have to switch other
(memory, video) settings; you might even have to validate the frequency
settings for these or even additional reasons (thermal and battery reasons -
ACPI _PPC).
This lowest layer basically assumes that upper-layer software has
created an appropriate operating point (for example, in DPM we pretty
much require a system designer to create operating points that match the
h/w specs and don't go to great lengths to encode rules about this),
and/or will call driver notifiers etc. as needed to adapt to the
changes. Although there may be some sanity checking appropriate at the
PowerOP level, cpufreq, DPM, etc. can for the most part continue to
handle the larger issues of how valid operating points are constructed,
driver callbacks, etc. If you do want to handle various dependencies at
the PowerOP layer then there's nothing that prevents that, but PM
frameworks tend to embody assumptions about how frequently operating
points will change and in what contexts (interrupt, idle...), and this
can influence the code for such things.
Thirdly, who is to decide on the power management settings? The first and
intuitive answer is the kernel. Therefore, kernel-space cpufreq governors
exist. Only under rare circumstances, you want full userspace control --
that's what the userspace cpufreq governor is for.
Also something left to the existing upper layers; PowerOP isn't intended
to handle any of that. In the embedded space we usually let the system
designer choose operating points supported by their h/w vendor and that
match their particular system states (hardware enabled at any point in
time, type and power/performance needs of software currently running).
We do recommend that a userspace power policy manager be the component
in charge of PM settings, based on messages from drivers and other apps
on the state of the system. And so that userspace component activates
the operating point (or set of operating points in the case of DPM)
appropriate for current state.
Foruthly, the code duplication which your implementation leads to is obvious
for the speedstep-centrino case.
We could move the tables of valid cpu speeds and corresponding voltages
down to the PowerOP level, and there would probably be little
duplication at that point (in fact, with the current patch there's not a
lot of duplication since the actual MSR access was moved to PowerOP and
PowerOP contains little else, but both levels know how to understand the
MSR format, and a more aggressive port to PowerOP could do away with that).
Your suggestions of changes to cpufreq governors and policies to handle
governance of non-cpu-speed parameters sound interesting, and I'd be
happy to help figure out what to do about those vs. the lower machine
access layer I've discussed up until now. I'll think more about this
real soon now. Thanks,
--
Todd
Dominik Brodowski wrote:
A small add-on:
We need to make sure that we're capable of handling smart CPUs like Transmeta
Crusoe processors in a sane way. This means
quoted
b) Setting of "values"
is optional if the hardware itself can be set to a min/max value (step a
above in previous mail).
Although I haven't looked into the Crusoe processor support, it may be
that there is a different set of power parameters, not cpu speed
directly, that are appropriate to manage on these platforms (after a
brief look, seems to be a range of frequencies and some sort of flags)?
If so, these sorts of machine-specific power parameters are what
PowerOP is trying to address, allowing management of the underlying
machine-specific stuff to upper layers that may be presenting an
abstracted view of power/performance, such as CPU speed or speed ranges,
to the user. Thanks,
--
Todd