On 10/17/2012 12:39 AM, 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 moves the dummy value initialization from
> cpuidle_register_driver to cpuidle_enable_device, which drivers such as
> acpi/processor_idle.c will call again when they add or remove C-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 <jwer...@chromium.org>
> ---
This is specific to the acpi and should be handled in the
processor_idle.c file instead of the cpuidle core code.
Could be the function 'acpi_processor_cst_has_changed' the right place
to set a dummy power value for the power in the new C-state ?
> drivers/cpuidle/cpuidle.c | 24 ++++++++++++++++++++++++
> drivers/cpuidle/driver.c | 25 -------------------------
> 2 files changed, 24 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 7f15b85..bef3a31 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -298,6 +298,28 @@ static void poll_idle_init(struct cpuidle_driver *drv)
> static void poll_idle_init(struct cpuidle_driver *drv) {}
> #endif /* CONFIG_ARCH_HAS_CPU_RELAX */
>
> +static void set_power_states(struct cpuidle_driver *drv)
> +{
> + int i;
> +
> + /*
> + * cpuidle driver should set the drv->power_specified bit
> + * before registering if the driver provides
> + * power_usage numbers.
> + *
> + * If power_specified is not set,
> + * we fill in power_usage with decreasing values as the
> + * cpuidle code has an implicit assumption that state Cn
> + * uses less power than C(n-1).
> + *
> + * With CONFIG_ARCH_HAS_CPU_RELAX, C0 is already assigned
> + * an power value of -1. So we use -2, -3, etc, for other
> + * c-states.
> + */
> + for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++)
> + drv->states[i].power_usage = -1 - i;
> +}
> +
> /**
> * cpuidle_enable_device - enables idle PM for a CPU
> * @dev: the CPU
> @@ -330,6 +352,8 @@ int cpuidle_enable_device(struct cpuidle_device *dev)
> cpuidle_enter_tk : cpuidle_enter;
>
> poll_idle_init(drv);
> + if (!drv->power_specified)
> + set_power_states(drv);
>
> if ((ret = cpuidle_add_state_sysfs(dev)))
> return ret;
> diff --git a/drivers/cpuidle/driver.c b/drivers/cpuidle/driver.c
> index 87db387..caaed27 100644
> --- a/drivers/cpuidle/driver.c
> +++ b/drivers/cpuidle/driver.c
> @@ -18,28 +18,6 @@ static struct cpuidle_driver *cpuidle_curr_driver;
> DEFINE_SPINLOCK(cpuidle_driver_lock);
> int cpuidle_driver_refcount;
>
> -static void set_power_states(struct cpuidle_driver *drv)
> -{
> - int i;
> -
> - /*
> - * cpuidle driver should set the drv->power_specified bit
> - * before registering if the driver provides
> - * power_usage numbers.
> - *
> - * If power_specified is not set,
> - * we fill in power_usage with decreasing values as the
> - * cpuidle code has an implicit assumption that state Cn
> - * uses less power than C(n-1).
> - *
> - * With CONFIG_ARCH_HAS_CPU_RELAX, C0 is already assigned
> - * an power value of -1. So we use -2, -3, etc, for other
> - * c-states.
> - */
> - for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++)
> - drv->states[i].power_usage = -1 - i;
> -}
> -
> /**
> * cpuidle_register_driver - registers a driver
> * @drv: the driver
> @@ -58,9 +36,6 @@ int cpuidle_register_driver(struct cpuidle_driver *drv)
> return -EBUSY;
> }
>
> - if (!drv->power_specified)
> - set_power_states(drv);
> -
> cpuidle_curr_driver = drv;
>
> spin_unlock(&cpuidle_driver_lock);
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
