On 10/19/2012 11:50 PM, Julius Werner wrote:
When cpuidle drivers do not supply explicit power_usage values, cpuidle/driver.c inserts dummy values instead. When a running processor dynamically gains new C-states (e.g. after ACPI events), the power_usage values of those states will stay uninitialized, and cpuidle governors will never choose to enter them.
This patch ensures that the ACPI cpuidle driver sets those dummy power values itself whenever it (re-)initializes its idle states. Tested and verified on an Acer AC700 Chromebook, which supports the C3 state only when it switches from AC to battery power.
Signed-off-by: Julius Werner jwerner@chromium.org
I am not against this patch but I am wondering if it is not time to do some cleanup around this.
The flag 'power_specified' is never used in any driver.
And the field 'power_usage' is used only in the tegra3 driver where logically as power_specified is not set, it will be overwritten at the init (could someone could check the /sys/devices/system/cpu/cpu0/cpuidle/state1/power is different from 600 on tegra3 ?)
The drivers define their states in a power consumption descendant order making de facto the same ordering than the 'set_power_state' function in driver.c
The governor looks at the power_usage (which is always filled by 'set_power_state').
static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev) [ ... ]
if (s->power_usage < power_usage) { power_usage = s->power_usage; data->last_state_idx = i; data->exit_us = s->exit_latency; }
[ ... ]
Could we just say this is always true because state[i+1] consumes less than state[i] ?
And then just remove the 'set_power_state' function, and the field 'driver->power_specified' ?
That will cleanup the code and fix this problem, no ?
Thanks -- Daniel
drivers/acpi/processor_idle.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index e8086c7..078e22f 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -1090,6 +1090,9 @@ static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr) state->exit_latency = cx->latency; state->target_residency = cx->latency * latency_factor;
/* reinitialize this in case new states are added after boot */state->power_usage = -1 - count;- state->flags = 0; switch (cx->type) { case ACPI_STATE_C1: