Selecting the ppc_md.power_save method at boot time

Selecting the ppc_md.power_save method at boot time

[Arnd Bergmann]

Anthony was asking me to add support for disabling the cbe_idle
function for cell at boot time, in order to make it easier to
use some debugging tools.

I already intend to move the cbe_idle implementation over to
a simpler power_save method, so these two changes could be
combined by making the power_save function selectable at
boot time.

I could imagine two ways to implement this:

1. Add a boot option to simply disable calling ppc_md->power_save
on all platforms. E.g. powersave=off.

2. Make it possible to have multiple implementations, e.g.
powersave=cbe, powersave=nap, powersave=cede, powersave=off,
powersave=power4, with a per-platform default.

Is anyone interested in the second solution? Otherwise I'd post
a simple patch to just do powersave=off.

	Arnd <><

Re: Selecting the ppc_md.power_save method at boot time

[Benjamin Herrenschmidt]

> Is anyone interested in the second solution? Otherwise I'd post
> a simple patch to just do powersave=off.

Simple solution sounds good to me.

Ben.

Re: Selecting the ppc_md.power_save method at boot time

[Olof Johansson]

On Fri, 20 Oct 2006 18:40:13 +0200 Arnd Bergmann <arnd.bergmann@de.ibm.com> wrote:

> 2. Make it possible to have multiple implementations, e.g.
> powersave=cbe, powersave=nap, powersave=cede, powersave=off,
> powersave=power4, with a per-platform default.

Doesn't make a whole lot of sense, since there's no way to use the
power4 powersave on a cell system -- specifying it on the command line
won't do much good.

> Is anyone interested in the second solution? Otherwise I'd post
> a simple patch to just do powersave=off.

Gobal powersave=off sounds good, specific platforms could implement
others as needed.


-Olof

[PATCH] cell: use ppc_md->power_save instead of cbe_idle_loop

[Arnd Bergmann]

This moves the cell idle function to use the default cpu_idle
with a special power_save callback, like all other platforms
except iSeries already do.

It also makes it possible to disable this power_save function
with a new powerpc-specific boot option "powersave=off".

Signed-off-by: Arnd Bergmann <arnd.bergmann@de.ibm.com>

---

Anthony, can you verify that the new options works for you?

I've added this patch to my cell series, and want to submit
it for 2.6.20 if everyone's happy with it.

Index: linux-2.6/arch/powerpc/platforms/cell/pervasive.c
===================================================================
--- linux-2.6.orig/arch/powerpc/platforms/cell/pervasive.c
+++ linux-2.6/arch/powerpc/platforms/cell/pervasive.c
@@ -82,51 +82,24 @@ out:
 	spin_unlock_irq(&cbe_pervasive_lock);
 }
 
-static void cbe_idle(void)
+static void cbe_power_save(void)
 {
 	unsigned long ctrl;
 
-	/* Why do we do that on every idle ? Couldn't that be done once for
-	 * all or do we lose the state some way ? Also, the pm_control
-	 * register setting, that can't be set once at boot ? We really want
-	 * to move that away in order to implement a simple powersave
+	/*
+	 * go into low thread priority, medium priority will be
+	 * restored for us after wake-up.
 	 */
-	cbe_enable_pause_zero();
+	HMT_low();
 
-	while (1) {
-		if (!need_resched()) {
-			local_irq_disable();
-			while (!need_resched()) {
-				/* go into low thread priority */
-				HMT_low();
-
-				/*
-				 * atomically disable thread execution
-				 * and runlatch.
-				 * External and Decrementer exceptions
-				 * are still handled when the thread
-				 * is disabled but now enter in
-				 * cbe_system_reset_exception()
-				 */
-				ctrl = mfspr(SPRN_CTRLF);
-				ctrl &= ~(CTRL_RUNLATCH | CTRL_TE);
-				mtspr(SPRN_CTRLT, ctrl);
-			}
-			/* restore thread prio */
-			HMT_medium();
-			local_irq_enable();
-		}
-
-		/*
-		 * turn runlatch on again before scheduling the
-		 * process we just woke up
-		 */
-		ppc64_runlatch_on();
-
-		preempt_enable_no_resched();
-		schedule();
-		preempt_disable();
-	}
+	/*
+	 * atomically disable thread execution and runlatch.
+	 * External and Decrementer exceptions are still handled when the
+	 * thread is disabled but now enter in cbe_system_reset_exception()
+	 */
+	ctrl = mfspr(SPRN_CTRLF);
+	ctrl &= ~(CTRL_RUNLATCH | CTRL_TE);
+	mtspr(SPRN_CTRLT, ctrl);
 }
 
 static int cbe_system_reset_exception(struct pt_regs *regs)
@@ -161,6 +134,13 @@ void __init cbe_pervasive_init(void)
 	if (!cpu_has_feature(CPU_FTR_PAUSE_ZERO))
 		return;
 
-	ppc_md.idle_loop = cbe_idle;
+	/*
+	 * We need to call cbe_enable_pause_zero on each CPU
+	 * before first entering power_save(). The most convenient
+	 * place for this is ppc_md->idle, because that is called
+	 * right before entering the idle loop.
+	 */
+	ppc_md.idle_loop = cbe_enable_pause_zero;
+	ppc_md.power_save = cbe_power_save;
 	ppc_md.system_reset_exception = cbe_system_reset_exception;
 }
Index: linux-2.6/arch/powerpc/kernel/idle.c
===================================================================
--- linux-2.6.orig/arch/powerpc/kernel/idle.c
+++ linux-2.6/arch/powerpc/kernel/idle.c
@@ -39,13 +39,20 @@
 #define cpu_should_die()	0
 #endif
 
+static int __init powersave_off(char *arg)
+{
+	ppc_md.power_save = NULL;
+	return 0;
+}
+__setup("powersave=off", powersave_off);
+
 /*
  * The body of the idle task.
  */
 void cpu_idle(void)
 {
 	if (ppc_md.idle_loop)
-		ppc_md.idle_loop();	/* doesn't return */
+		ppc_md.idle_loop();	/* usually does not return */
 
 	set_thread_flag(TIF_POLLING_NRFLAG);
 	while (1) {

Re: [Cbe-oss-dev] [PATCH] cell: use ppc_md->power_save instead of cbe_idle_loop

[Benjamin Herrenschmidt]

> +	/*
> +	 * We need to call cbe_enable_pause_zero on each CPU
> +	 * before first entering power_save(). The most convenient
> +	 * place for this is ppc_md->idle, because that is called
> +	 * right before entering the idle loop.
> +	 */
> +	ppc_md.idle_loop = cbe_enable_pause_zero;

I disagree here... this should be done in setup_cpu() for non-boot CPUs,
and sometime in setup_arch for the boot CPU. 

Another option is to have the pm_control bit done once for all CPUs at
boot in setup_arch() or such, and the TSC_CELL SPR bit done
unconditionally in powersave...

Ben.

[PATCH] cell: use ppc_md->power_save instead of cbe_idle_loop

[Arnd Bergmann]

This moves the cell idle function to use the default cpu_idle
with a special power_save callback, like all other platforms
except iSeries already do.

It also makes it possible to disable this power_save function
with a new powerpc-specific boot option "powersave=off".

Signed-off-by: Arnd Bergmann <arnd.bergmann@de.ibm.com>

---

On Monday 23 October 2006 12:16, Benjamin Herrenschmidt wrote:
> I disagree here... this should be done in setup_cpu() for non-boot CPUs,
> and sometime in setup_arch for the boot CPU. 

ok, good point.

> Another option is to have the pm_control bit done once for all CPUs at
> boot in setup_arch() or such, and the TSC_CELL SPR bit done
> unconditionally in powersave...

How about this then?

diff --git a/arch/powerpc/kernel/idle.c b/arch/powerpc/kernel/idle.c
index 4180c39..8994af3 100644
--- a/arch/powerpc/kernel/idle.c
+++ b/arch/powerpc/kernel/idle.c
@@ -39,6 +39,13 @@
 #define cpu_should_die()	0
 #endif
 
+static int __init powersave_off(char *arg)
+{
+	ppc_md.power_save = NULL;
+	return 0;
+}
+__setup("powersave=off", powersave_off);
+
 /*
  * The body of the idle task.
  */
diff --git a/arch/powerpc/platforms/cell/pervasive.c b/arch/powerpc/platforms/cell/pervasive.c
index 9f2e4ed..10b150a 100644
--- a/arch/powerpc/platforms/cell/pervasive.c
+++ b/arch/powerpc/platforms/cell/pervasive.c
@@ -38,32 +38,16 @@
 #include "pervasive.h"
 #include "cbe_regs.h"
 
-static DEFINE_SPINLOCK(cbe_pervasive_lock);
-
-static void __init cbe_enable_pause_zero(void)
+static void cbe_power_save(void)
 {
-	unsigned long thread_switch_control;
-	unsigned long temp_register;
-	struct cbe_pmd_regs __iomem *pregs;
-
-	spin_lock_irq(&cbe_pervasive_lock);
-	pregs = cbe_get_cpu_pmd_regs(smp_processor_id());
-	if (pregs == NULL)
-		goto out;
-
-	pr_debug("Power Management: CPU %d\n", smp_processor_id());
-
-	 /* Enable Pause(0) control bit */
-	temp_register = in_be64(&pregs->pm_control);
-
-	out_be64(&pregs->pm_control,
-		 temp_register | CBE_PMD_PAUSE_ZERO_CONTROL);
+	unsigned long ctrl, thread_switch_control;
+	ctrl = mfspr(SPRN_CTRLF);
 
 	/* Enable DEC and EE interrupt request */
 	thread_switch_control  = mfspr(SPRN_TSC_CELL);
 	thread_switch_control |= TSC_CELL_EE_ENABLE | TSC_CELL_EE_BOOST;
 
-	switch ((mfspr(SPRN_CTRLF) & CTRL_CT)) {
+	switch (ctrl & CTRL_CT) {
 	case CTRL_CT0:
 		thread_switch_control |= TSC_CELL_DEC_ENABLE_0;
 		break;
@@ -75,58 +59,21 @@ static void __init cbe_enable_pause_zero
 			__FUNCTION__);
 		break;
 	}
-
 	mtspr(SPRN_TSC_CELL, thread_switch_control);
 
-out:
-	spin_unlock_irq(&cbe_pervasive_lock);
-}
-
-static void cbe_idle(void)
-{
-	unsigned long ctrl;
-
-	/* Why do we do that on every idle ? Couldn't that be done once for
-	 * all or do we lose the state some way ? Also, the pm_control
-	 * register setting, that can't be set once at boot ? We really want
-	 * to move that away in order to implement a simple powersave
+	/*
+	 * go into low thread priority, medium priority will be
+	 * restored for us after wake-up.
 	 */
-	cbe_enable_pause_zero();
+	HMT_low();
 
-	while (1) {
-		if (!need_resched()) {
-			local_irq_disable();
-			while (!need_resched()) {
-				/* go into low thread priority */
-				HMT_low();
-
-				/*
-				 * atomically disable thread execution
-				 * and runlatch.
-				 * External and Decrementer exceptions
-				 * are still handled when the thread
-				 * is disabled but now enter in
-				 * cbe_system_reset_exception()
-				 */
-				ctrl = mfspr(SPRN_CTRLF);
-				ctrl &= ~(CTRL_RUNLATCH | CTRL_TE);
-				mtspr(SPRN_CTRLT, ctrl);
-			}
-			/* restore thread prio */
-			HMT_medium();
-			local_irq_enable();
-		}
-
-		/*
-		 * turn runlatch on again before scheduling the
-		 * process we just woke up
-		 */
-		ppc64_runlatch_on();
-
-		preempt_enable_no_resched();
-		schedule();
-		preempt_disable();
-	}
+	/*
+	 * atomically disable thread execution and runlatch.
+	 * External and Decrementer exceptions are still handled when the
+	 * thread is disabled but now enter in cbe_system_reset_exception()
+	 */
+	ctrl &= ~(CTRL_RUNLATCH | CTRL_TE);
+	mtspr(SPRN_CTRLT, ctrl);
 }
 
 static int cbe_system_reset_exception(struct pt_regs *regs)
@@ -158,9 +105,20 @@ static int cbe_system_reset_exception(st
 
 void __init cbe_pervasive_init(void)
 {
+	int cpu;
 	if (!cpu_has_feature(CPU_FTR_PAUSE_ZERO))
 		return;
 
-	ppc_md.idle_loop = cbe_idle;
+	for_each_possible_cpu(cpu) {
+		struct cbe_pmd_regs __iomem *regs = cbe_get_cpu_pmd_regs(cpu);
+		if (!regs)
+			continue;
+
+		 /* Enable Pause(0) control bit */
+		out_be64(&regs->pm_control, in_be64(&regs->pm_control) |
+					    CBE_PMD_PAUSE_ZERO_CONTROL);
+	}
+
+	ppc_md.power_save = cbe_power_save;
 	ppc_md.system_reset_exception = cbe_system_reset_exception;
 }