Re: [PATCH] [v5] Add idle wait support for 44x platforms

2008-04-10 Thread Josh Boyer
On Tue, 08 Apr 2008 11:49:14 -0500
Jerone Young [EMAIL PROTECTED] wrote:

 2 files changed, 77 insertions(+), 1 deletion(-)
 arch/powerpc/platforms/44x/Makefile |2 
 arch/powerpc/platforms/44x/idle.c   |   76 +++
 
 
 Updates: Now setting MSR_WE is now default
  Tested on hardware platforms bamboo  sequioa and appears
  things are working fine on actually hardware!
 
 This patch adds the ability for the CPU to go into wait state while in 
 cpu_idle loop. This helps virtulization solutions know when the guest Linux 
 kernel is in an idle state. There are two ways to do it.
 
 Command line
   idle=spin -- CPU will spin
   idle=wait -- set CPU into wait state when idle (default)
 
 
 Signed-off-by: Jerone Young [EMAIL PROTECTED]

Looks great.  I'll add it to my tree shortly.

josh
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] [v5] Add idle wait support for 44x platforms

2008-04-10 Thread Arnd Bergmann
On Tuesday 08 April 2008, Jerone Young wrote:
 +static struct sleep_mode modes[] = {
 +   { .name = wait, .entry = ppc44x_idle },
 +   { .name = spin, .entry = NULL },
 +};
 +
 +int __init ppc44x_idle_init(void)
 +{
 +   void *func = modes[current_mode].entry;
 +   ppc_md.power_save = func;
 +   return 0;
 +}
 +
 +arch_initcall(ppc44x_idle_init);
 +
 +static int __init idle_param(char *p)
 +{ 
 +   int i;
 +
 +   for (i = 0; i  ARRAY_SIZE(modes); i++) {
 +   if (!strcmp(modes[i].name, p)) {
 +   current_mode = i;
 +   break;
 +   }
 +   }
 +
 +   return 0;
 +}
 +
 +early_param(idle, idle_param);

ok, sorry to steal the show again, now that everyone seems to be happy
with the current code, but isn't this equivalent to the simpler

static int __init idle_param(char *p)
{
if (!strcmp(modes[i].name, spin))
ppc_md.power_save = NULL;
}
early_param(idle, idle_param);

if you statically initialize the ppc_md.power_save function to ppc44x_idle
in the platform setup files?

Arnd 
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] [v5] Add idle wait support for 44x platforms

2008-04-10 Thread Jerone Young
On Thu, 2008-04-10 at 15:44 +0200, Arnd Bergmann wrote:
 On Tuesday 08 April 2008, Jerone Young wrote:
  +static struct sleep_mode modes[] = {
  +   { .name = wait, .entry = ppc44x_idle },
  +   { .name = spin, .entry = NULL },
  +};
  +
  +int __init ppc44x_idle_init(void)
  +{
  +   void *func = modes[current_mode].entry;
  +   ppc_md.power_save = func;
  +   return 0;
  +}
  +
  +arch_initcall(ppc44x_idle_init);
  +
  +static int __init idle_param(char *p)
  +{ 
  +   int i;
  +
  +   for (i = 0; i  ARRAY_SIZE(modes); i++) {
  +   if (!strcmp(modes[i].name, p)) {
  +   current_mode = i;
  +   break;
  +   }
  +   }
  +
  +   return 0;
  +}
  +
  +early_param(idle, idle_param);
 
 ok, sorry to steal the show again, now that everyone seems to be happy
 with the current code, but isn't this equivalent to the simple

Well it could be this simple. But the current code leaves a lot more
room to add different type waits or spins if need be (if they are ever
needed ... though none off the top of my head at the moment)...but it
does allow you to create another wait state for whatever reason a lot
easier.

So I really don't think this needs to change. Unless everyone really
feels that it just has to be.

 
 static int __init idle_param(char *p)
 {
   if (!strcmp(modes[i].name, spin))
   ppc_md.power_save = NULL;
 }
 early_param(idle, idle_param);
 
 if you statically initialize the ppc_md.power_save function to ppc44x_idle
 in the platform setup files?

The idea is to not statically initialize ppc_md.power_save to
ppc44x_idle in each platform setup file.

 
   Arnd 

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] [v5] Add idle wait support for 44x platforms

2008-04-10 Thread Arnd Bergmann
On Thursday 10 April 2008, Jerone Young wrote:
 Well it could be this simple. But the current code leaves a lot more
 room to add different type waits or spins if need be (if they are ever
 needed ... though none off the top of my head at the moment)...but it
 does allow you to create another wait state for whatever reason a lot
 easier.
 
 So I really don't think this needs to change. Unless everyone really
 feels that it just has to be.

No, it doesn't need to change, the current patch is entirely correct,
just a little bit more complicated than it needs to be, and I try
not to have code in anticipation of something getting more complicated
in the future, you can always add the complexity at the point where you
need it.

  
  static int __init idle_param(char *p)
  {
    if (!strcmp(modes[i].name, spin))
    ppc_md.power_save = NULL;
  }
  early_param(idle, idle_param);
  
  if you statically initialize the ppc_md.power_save function to ppc44x_idle
  in the platform setup files?
 
 The idea is to not statically initialize ppc_md.power_save to
 ppc44x_idle in each platform setup file.
 

Why not? Unlike the platform_initcall, it wouldn't cost anything and your
current code has the same effect in the end, but in a less obvious way.

Arnd 
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] [v5] Add idle wait support for 44x platforms

2008-04-10 Thread Josh Boyer
On Fri, 2008-04-11 at 02:18 +0200, Arnd Bergmann wrote:
   static int __init idle_param(char *p)
   {
 if (!strcmp(modes[i].name, spin))
 ppc_md.power_save = NULL;
   }
   early_param(idle, idle_param);
   
   if you statically initialize the ppc_md.power_save function to ppc44x_idle
   in the platform setup files?
  
  The idea is to not statically initialize ppc_md.power_save to
  ppc44x_idle in each platform setup file.
  
 
 Why not? Unlike the platform_initcall, it wouldn't cost anything and your
 current code has the same effect in the end, but in a less obvious way.

This is likely something that has evolved from the original patch which
added a machine_late_initcall(machine, ppc44x_idle_init) to every
platform.c file.  It made the original patch overly complicated as
opposed to adding a single arch_initcall in a common file.  That was
also before we decided to make wait the default, which made things
much easier to deal with.

I'll take the blame for the way the minorly suboptimal state the patch
is, as Jerone had to jump through several hoops already at my request.
I agree it's cleaner to do as you suggest in the long run, and may well
make some changes along those lines myself.

josh

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


[PATCH] [v5] Add idle wait support for 44x platforms

2008-04-08 Thread Jerone Young
2 files changed, 77 insertions(+), 1 deletion(-)
arch/powerpc/platforms/44x/Makefile |2 
arch/powerpc/platforms/44x/idle.c   |   76 +++


Updates: Now setting MSR_WE is now default
 Tested on hardware platforms bamboo  sequioa and appears
 things are working fine on actually hardware!

This patch adds the ability for the CPU to go into wait state while in cpu_idle 
loop. This helps virtulization solutions know when the guest Linux kernel is in 
an idle state. There are two ways to do it.

Command line
idle=spin -- CPU will spin
idle=wait -- set CPU into wait state when idle (default)


Signed-off-by: Jerone Young [EMAIL PROTECTED]

diff --git a/arch/powerpc/platforms/44x/Makefile 
b/arch/powerpc/platforms/44x/Makefile
--- a/arch/powerpc/platforms/44x/Makefile
+++ b/arch/powerpc/platforms/44x/Makefile
@@ -1,4 +1,4 @@ obj-$(CONFIG_44x)   := misc_44x.o
-obj-$(CONFIG_44x)  := misc_44x.o
+obj-$(CONFIG_44x)  := misc_44x.o idle.o
 obj-$(CONFIG_EBONY)+= ebony.o
 obj-$(CONFIG_TAISHAN)  += taishan.o
 obj-$(CONFIG_BAMBOO)   += bamboo.o
diff --git a/arch/powerpc/platforms/44x/idle.c 
b/arch/powerpc/platforms/44x/idle.c
new file mode 100644
--- /dev/null
+++ b/arch/powerpc/platforms/44x/idle.c
@@ -0,0 +1,76 @@
+/*
+ * Copyright 2008 IBM Corp. 
+ *
+ * Based on arch/powerpc/platforms/pasemi/idle.c: 
+ * Copyright (C) 2006-2007 PA Semi, Inc
+ *
+ * Added by: Jerone Young [EMAIL PROTECTED]
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
+ *
+ */
+
+#include linux/of.h
+#include linux/kernel.h
+#include asm/machdep.h
+
+static int current_mode;
+
+struct sleep_mode {
+   char *name;
+   void (*entry)(void);
+};
+
+static void ppc44x_idle(void)
+{
+   unsigned long msr_save;
+
+   msr_save = mfmsr();
+   /* set wait state MSR */
+   mtmsr(msr_save|MSR_WE|MSR_EE|MSR_CE|MSR_DE);
+   isync();
+   /* return to initial state */
+   mtmsr(msr_save);
+   isync();
+}
+
+static struct sleep_mode modes[] = {
+   { .name = wait, .entry = ppc44x_idle },
+   { .name = spin, .entry = NULL },
+};
+
+int __init ppc44x_idle_init(void)
+{
+   void *func = modes[current_mode].entry;
+   ppc_md.power_save = func;
+   return 0;
+}
+
+arch_initcall(ppc44x_idle_init);
+
+static int __init idle_param(char *p)
+{ 
+   int i;
+
+   for (i = 0; i  ARRAY_SIZE(modes); i++) {
+   if (!strcmp(modes[i].name, p)) {
+   current_mode = i;
+   break;
+   }
+   }
+
+   return 0;
+}
+
+early_param(idle, idle_param);
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [kvm-ppc-devel] [PATCH] [v5] Add idle wait support for 44x platforms

2008-04-08 Thread Hollis Blanchard
On Tuesday 08 April 2008 11:49:14 Jerone Young wrote:
 2 files changed, 77 insertions(+), 1 deletion(-)
 arch/powerpc/platforms/44x/Makefile |2
 arch/powerpc/platforms/44x/idle.c   |   76
 +++


 Updates: Now setting MSR_WE is now default
  Tested on hardware platforms bamboo  sequioa and appears
  things are working fine on actually hardware!

 This patch adds the ability for the CPU to go into wait state while in
 cpu_idle loop. This helps virtulization solutions know when the guest Linux
 kernel is in an idle state. There are two ways to do it.

 Command line
   idle=spin -- CPU will spin
   idle=wait -- set CPU into wait state when idle (default)


 Signed-off-by: Jerone Young [EMAIL PROTECTED]

Acked-by: Hollis Blanchard [EMAIL PROTECTED]

-- 
Hollis Blanchard
IBM Linux Technology Center
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev