lkml.org 
[lkml]   [2009]   [Aug]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 2/4]: CPUIDLE: Introduce architecture independent cpuidle_pm_idle in drivers/cpuidle/cpuidle.c
From
Date
On Thu, 2009-08-27 at 17:23 +0530, Arun R Bharadwaj wrote:
> * Arun R Bharadwaj <arun@linux.vnet.ibm.com> [2009-08-27 17:19:08]:
>
> Cpuidle infrastructure assumes pm_idle as the default idle routine.
> But, ppc_md.power_save is the default idle callback in case of pSeries.
>
> So, create a more generic, architecture independent cpuidle_pm_idle
> function pointer in driver/cpuidle/cpuidle.c and allow the idle routines
> of architectures to be set to cpuidle_pm_idle.
>
> Signed-off-by: Arun R Bharadwaj <arun@linux.vnet.ibm.com>
> ---
> drivers/cpuidle/cpuidle.c | 12 +++++++-----
> include/linux/cpuidle.h | 7 +++++++
> 2 files changed, 14 insertions(+), 5 deletions(-)
>
> Index: linux.trees.git/drivers/cpuidle/cpuidle.c
> ===================================================================
> --- linux.trees.git.orig/drivers/cpuidle/cpuidle.c
> +++ linux.trees.git/drivers/cpuidle/cpuidle.c
> @@ -25,6 +25,7 @@ DEFINE_PER_CPU(struct cpuidle_device *,
> DEFINE_MUTEX(cpuidle_lock);
> LIST_HEAD(cpuidle_detected_devices);
> static void (*pm_idle_old)(void);
> +void (*cpuidle_pm_idle)(void);
>
> static int enabled_devices;
>
> @@ -98,10 +99,10 @@ static void cpuidle_idle_call(void)
> */
> void cpuidle_install_idle_handler(void)
> {
> - if (enabled_devices && (pm_idle != cpuidle_idle_call)) {
> + if (enabled_devices && (cpuidle_pm_idle != cpuidle_idle_call)) {
> /* Make sure all changes finished before we switch to new idle */
> smp_wmb();
> - pm_idle = cpuidle_idle_call;
> + cpuidle_pm_idle = cpuidle_idle_call;
> }
> }
>
> @@ -110,8 +111,9 @@ void cpuidle_install_idle_handler(void)
> */
> void cpuidle_uninstall_idle_handler(void)
> {
> - if (enabled_devices && pm_idle_old && (pm_idle != pm_idle_old)) {
> - pm_idle = pm_idle_old;
> + if (enabled_devices && pm_idle_old &&
> + (cpuidle_pm_idle != pm_idle_old)) {
> + cpuidle_pm_idle = pm_idle_old;
> cpuidle_kick_cpus();
> }
> }
> @@ -382,7 +384,7 @@ static int __init cpuidle_init(void)
> {
> int ret;
>
> - pm_idle_old = pm_idle;
> + pm_idle_old = cpuidle_pm_idle;
>
> ret = cpuidle_add_class_sysfs(&cpu_sysdev_class);
> if (ret)
> Index: linux.trees.git/include/linux/cpuidle.h
> ===================================================================
> --- linux.trees.git.orig/include/linux/cpuidle.h
> +++ linux.trees.git/include/linux/cpuidle.h
> @@ -188,4 +188,11 @@ static inline void cpuidle_unregister_go
> #define CPUIDLE_DRIVER_STATE_START 0
> #endif
>
> +/*
> + * Idle callback used by cpuidle to call the cpuidle_idle_call().
> + * Platform drivers can use this to register to cpuidle's idle loop.
> + */
> +
> +extern void (*cpuidle_pm_idle)(void);
> +
> #endif /* _LINUX_CPUIDLE_H */


I'm not quite seeing how this makes anything any better. Not we have 3
function pointers, where 1 should suffice.

/me wonders what's wrong with something like:

struct idle_func_desc {
int power;
int latency;
void (*idle)(void);
struct list_head list;
};
static void spin_idle(void)
{
for (;;)
cpu_relax();
}
static idle_func_desc default_idle_func = {
power = 0, /* doesn't safe any power */
latency = INT_MAX, /* has max latency */
idle = spin_idle,
list = INIT_LIST_HEAD(default_idle_func.list),
};
void (*idle_func)(void);
static struct list_head idle_func_list;

static void pick_idle_func(void)
{
struct idle_func_desc *desc, *idle = &default_idle_desc;
list_for_each_entry(desc, &idle_func_list, list) {
if (desc->power < idle->power)
continue;
if (desc->latency > target_latency);
continue;
idle = desc;
}
pm_idle = idle->idle;
}
void register_idle_func(struct idle_func_desc *desc)
{
WARN_ON_ONCE(!list_empty(&desc->list));
list_add_tail(&idle_func_list, &desc->list);
pick_idle_func();
}
void unregister_idle_func(struct idle_func_desc *desc)
{
WARN_ON_ONCE(list_empty(&desc->list));
list_del_init(&desc->list);
if (idle_func == desc->idle)
pick_idle_func();
}


\
 
 \ /
  Last update: 2009-08-27 14:57    [from the cache]
©2003-2011 Jasper Spaans