Hi Rob,
On 03/01/2012 06:12 AM, Robert Lee wrote:
> Make necessary changes to implement time keeping and irq enabling
> in the core cpuidle code. This will allow the removal of these
> functionalities from various platform cpuidle implementations whose
> timekeeping and irq enabling follows the form in this common code.
The generic cpuidle changes look good, but is there a reason as
to why these changes are enabled only for ARM and not other
archs ?
> Signed-off-by: Robert Lee <rob.lee@xxxxxxxxxx>
> ---
> arch/arm/include/asm/cpuidle.h | 22 +++++++++++
> arch/arm/kernel/Makefile | 2 +-
> arch/arm/kernel/cpuidle.c | 21 +++++++++++
> drivers/cpuidle/cpuidle.c | 79 ++++++++++++++++++++++++++++++++--------
> include/linux/cpuidle.h | 13 ++++++-
> 5 files changed, 119 insertions(+), 18 deletions(-)
> create mode 100644 arch/arm/include/asm/cpuidle.h
> create mode 100644 arch/arm/kernel/cpuidle.c
>
> diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
> new file mode 100644
> index 0000000..165676e
> --- /dev/null
> +++ b/arch/arm/include/asm/cpuidle.h
> @@ -0,0 +1,22 @@
> +#ifndef __ASM_ARM_CPUIDLE_H
> +#define __ASM_ARM_CPUIDLE_H
> +
> +#ifdef CONFIG_CPU_IDLE
> +extern int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int index);
> +#else
> +static inline int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int index) { return -ENODEV; }
> +#endif
> +
> +/* Common ARM WFI state */
> +#define ARM_CPUIDLE_WFI_STATE {\
> + .enter = arm_cpuidle_simple_enter,\
> + .exit_latency = 1,\
> + .target_residency = 1,\
> + .flags = CPUIDLE_FLAG_TIME_VALID,\
> + .name = "WFI",\
> + .desc = "ARM WFI",\
> +}
> +
> +#endif
> diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
> index 43b740d..940c27f 100644
> --- a/arch/arm/kernel/Makefile
> +++ b/arch/arm/kernel/Makefile
> @@ -21,7 +21,7 @@ obj-$(CONFIG_DEPRECATED_PARAM_STRUCT) += compat.o
>
> obj-$(CONFIG_LEDS) += leds.o
> obj-$(CONFIG_OC_ETM) += etm.o
> -
> +obj-$(CONFIG_CPU_IDLE) += cpuidle.o
> obj-$(CONFIG_ISA_DMA_API) += dma.o
> obj-$(CONFIG_ARCH_ACORN) += ecard.o
> obj-$(CONFIG_FIQ) += fiq.o fiqasm.o
> diff --git a/arch/arm/kernel/cpuidle.c b/arch/arm/kernel/cpuidle.c
> new file mode 100644
> index 0000000..89545f6
> --- /dev/null
> +++ b/arch/arm/kernel/cpuidle.c
> @@ -0,0 +1,21 @@
> +/*
> + * Copyright 2012 Linaro Ltd.
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +#include <linux/cpuidle.h>
> +#include <asm/proc-fns.h>
> +
> +int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int index)
> +{
> + cpu_do_idle();
> +
> + return index;
> +}
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 59f4261..56de5f7 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -53,6 +53,24 @@ static void cpuidle_kick_cpus(void) {}
>
> static int __cpuidle_register_device(struct cpuidle_device *dev);
>
> +static inline int cpuidle_enter(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int index)
> +{
> + struct cpuidle_state *target_state = &drv->states[index];
> + return target_state->enter(dev, drv, index);
> +}
> +
> +static inline int cpuidle_enter_tk(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int index)
> +{
> + return cpuidle_wrap_enter(dev, drv, index, cpuidle_enter);
> +}
> +
> +typedef int (*cpuidle_enter_t)(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int index);
> +
> +static cpuidle_enter_t cpuidle_enter_ops;
> +
> /**
> * cpuidle_idle_call - the main idle loop
> *
> @@ -63,7 +81,6 @@ int cpuidle_idle_call(void)
> {
> struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
> struct cpuidle_driver *drv = cpuidle_get_driver();
> - struct cpuidle_state *target_state;
> int next_state, entered_state;
>
> if (off)
> @@ -92,12 +109,10 @@ int cpuidle_idle_call(void)
> return 0;
> }
>
> - target_state = &drv->states[next_state];
> -
> trace_power_start(POWER_CSTATE, next_state, dev->cpu);
> trace_cpu_idle(next_state, dev->cpu);
>
> - entered_state = target_state->enter(dev, drv, next_state);
> + entered_state = cpuidle_enter_ops(dev, drv, next_state);
>
> trace_power_end(dev->cpu);
> trace_cpu_idle(PWR_EVENT_EXIT, dev->cpu);
> @@ -110,6 +125,8 @@ int cpuidle_idle_call(void)
> dev->states_usage[entered_state].time +=
> (unsigned long long)dev->last_residency;
> dev->states_usage[entered_state].usage++;
> + } else {
> + dev->last_residency = 0;
> }
>
> /* give the governor an opportunity to reflect on the outcome */
> @@ -164,20 +181,29 @@ void cpuidle_resume_and_unlock(void)
>
> EXPORT_SYMBOL_GPL(cpuidle_resume_and_unlock);
>
> -#ifdef CONFIG_ARCH_HAS_CPU_RELAX
> -static int poll_idle(struct cpuidle_device *dev,
> - struct cpuidle_driver *drv, int index)
> +/**
> + * cpuidle_wrap_enter - performs timekeeping and irqen around enter function
> + * @dev: pointer to a valid cpuidle_device object
> + * @drv: pointer to a valid cpuidle_driver object
> + * @index: index of the target cpuidle state.
> + */
> +int cpuidle_wrap_enter(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int index,
> + int (*enter)(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int index))
> {
> - ktime_t t1, t2;
> + ktime_t time_start, time_end;
> s64 diff;
>
> - t1 = ktime_get();
> + time_start = ktime_get();
> +
> + index = enter(dev, drv, index);
> +
> + time_end = ktime_get();
> +
> local_irq_enable();
> - while (!need_resched())
> - cpu_relax();
>
> - t2 = ktime_get();
> - diff = ktime_to_us(ktime_sub(t2, t1));
> + diff = ktime_to_us(ktime_sub(time_end, time_start));
> if (diff > INT_MAX)
> diff = INT_MAX;
>
> @@ -186,6 +212,23 @@ static int poll_idle(struct cpuidle_device *dev,
> return index;
> }
>
> +#ifdef CONFIG_ARCH_HAS_CPU_RELAX
> +static inline int __poll_idle(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int index)
> +{
> + while (!need_resched())
> + cpu_relax();
> +
> + return index;
> +}
> +
> +static int poll_idle(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int index)
> +{
> + return cpuidle_wrap_enter(dev, drv, index,
> + __poll_idle);
> +}
> +
> static void poll_idle_init(struct cpuidle_driver *drv)
> {
> struct cpuidle_state *state = &drv->states[0];
> @@ -212,10 +255,11 @@ static void poll_idle_init(struct cpuidle_driver *drv) {}
> int cpuidle_enable_device(struct cpuidle_device *dev)
> {
> int ret, i;
> + struct cpuidle_driver *drv = cpuidle_get_driver();
>
> if (dev->enabled)
> return 0;
> - if (!cpuidle_get_driver() || !cpuidle_curr_governor)
> + if (!drv || !cpuidle_curr_governor)
> return -EIO;
> if (!dev->state_count)
> return -EINVAL;
> @@ -226,13 +270,16 @@ int cpuidle_enable_device(struct cpuidle_device *dev)
> return ret;
> }
>
> - poll_idle_init(cpuidle_get_driver());
> + cpuidle_enter_ops = drv->en_core_tk_irqen ?
> + cpuidle_enter_tk : cpuidle_enter;
> +
> + poll_idle_init(drv);
>
> if ((ret = cpuidle_add_state_sysfs(dev)))
> return ret;
>
> if (cpuidle_curr_governor->enable &&
> - (ret = cpuidle_curr_governor->enable(cpuidle_get_driver(), dev)))
> + (ret = cpuidle_curr_governor->enable(drv, dev)))
> goto fail_sysfs;
>
> for (i = 0; i < dev->state_count; i++) {
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 712abcc..927db28 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -15,6 +15,7 @@
> #include <linux/list.h>
> #include <linux/kobject.h>
> #include <linux/completion.h>
> +#include <linux/hrtimer.h>
>
> #define CPUIDLE_STATE_MAX 8
> #define CPUIDLE_NAME_LEN 16
> @@ -122,6 +123,8 @@ struct cpuidle_driver {
> struct module *owner;
>
> unsigned int power_specified:1;
> + /* set to 1 to use the core cpuidle time keeping (for all states). */
> + unsigned int en_core_tk_irqen:1;
> struct cpuidle_state states[CPUIDLE_STATE_MAX];
> int state_count;
> int safe_state_index;
> @@ -140,7 +143,10 @@ extern void cpuidle_pause_and_lock(void);
> extern void cpuidle_resume_and_unlock(void);
> extern int cpuidle_enable_device(struct cpuidle_device *dev);
> extern void cpuidle_disable_device(struct cpuidle_device *dev);
> -
> +extern int cpuidle_wrap_enter(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int index,
> + int (*enter)(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int index));
> #else
> static inline void disable_cpuidle(void) { }
> static inline int cpuidle_idle_call(void) { return -ENODEV; }
> @@ -157,6 +163,11 @@ static inline void cpuidle_resume_and_unlock(void) { }
> static inline int cpuidle_enable_device(struct cpuidle_device *dev)
> {return -ENODEV; }
> static inline void cpuidle_disable_device(struct cpuidle_device *dev) { }
> +static inline int cpuidle_wrap_enter(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int index,
> + int (*enter)(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int index))
> +{ return -ENODEV; }
>
> #endif
>
For the generic cpuidle changes
Reviewed-by: Deepthi Dharwar <deepthi@xxxxxxxxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
