[RFC/Patch] 4xx idle loop

[RFC/Patch] 4xx idle loop

[akuster]

I was going though some old 4xx kernels I have and found an
implimentation of idle.c that had support for 4xx.  I believe the reason
it never made is out was the hacking we did to make it work.  So I
propose that we modify the kernel/Makefile to use ppc4xx_idel.o instead
of idle.o when 4xx is configured.  That way we have a clean
implimentation for 4xx idle functions and not touch the generic ppc idle
file.  The function names would not change.

Armin


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: [RFC/Patch] 4xx idle loop

[Matthew Locke]

akuster wrote:

>
> I was going though some old 4xx kernels I have and found an
> implimentation of idle.c that had support for 4xx.  I believe the reason
> it never made is out was the hacking we did to make it work.  So I
> propose that we modify the kernel/Makefile to use ppc4xx_idel.o instead
> of idle.o when 4xx is configured.  That way we have a clean
> implimentation for 4xx idle functions and not touch the generic ppc idle
> file.  The function names would not change.


Rather than replace the whole idle loop, I would allow for
sub-arch/processor specific code in the idle loop.  Perhaps move the
power_save routine into the platform code?

>
> Armin
>
>
>


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: [RFC/Patch] 4xx idle loop

[akuster]

Matthew Locke wrote:
>
>
> Rather than replace the whole idle loop, I would allow for
> sub-arch/processor specific code in the idle loop.  Perhaps move the
> power_save routine into the platform code?
>

This sounds like a good idea if we could use
  if( ppc_md.powersave != NULL)
       ppc_md.powersave();

If it is determined that calling power_save() which is resides in an
arch/processor specific file then we are talking about many files being
hit.  and the current power_save seems to common for many other ppc
platforms other than 4xx & 8xx

armin


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: [RFC/Patch] 4xx idle loop

[cort]

There's need for the indirection for this sort of thing.  The only reason
I created ppc_md was to allow for different machines to run off a single
binary but not different chip families.

A big #ifdef for the chip-type would work fine since the chip family is
known at compile time.  The machine type isn't always know which is the
reason for ppc_md.

} This sounds like a good idea if we could use
}   if( ppc_md.powersave != NULL)
}        ppc_md.powersave();
}
} If it is determined that calling power_save() which is resides in an
} arch/processor specific file then we are talking about many files being
} hit.  and the current power_save seems to common for many other ppc
} platforms other than 4xx & 8xx

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: [RFC/Patch] 4xx idle loop

[cort]

That should read "there's no need for the indirection..."

} There's need for the indirection for this sort of thing.  The only reason
} I created ppc_md was to allow for different machines to run off a single
} binary but not different chip families.
}
} A big #ifdef for the chip-type would work fine since the chip family is
} known at compile time.  The machine type isn't always know which is the
} reason for ppc_md.
}
} } This sounds like a good idea if we could use
} }   if( ppc_md.powersave != NULL)
} }        ppc_md.powersave();
} }
} } If it is determined that calling power_save() which is resides in an
} } arch/processor specific file then we are talking about many files being
} } hit.  and the current power_save seems to common for many other ppc
} } platforms other than 4xx & 8xx
}

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: [RFC/Patch] 4xx idle loop

[Matthew Locke]

cort@fsmlabs.com wrote:

>There's need for the indirection for this sort of thing.  The only reason
>I created ppc_md was to allow for different machines to run off a single
>binary but not different chip families.
>
>A big #ifdef for the chip-type would work fine since the chip family is
>known at compile time.  The machine type isn't always know which is the
>reason for ppc_md.
>

I thought one of the linuxppc desgin goals was to keep the ifdefs to a
minimum.  I can see idle.c growing quite large and full of #ifdefs if we
do it that way.  Rather than using ppc_md, make power_save an
abstraction similar to platform_init.

>
>
>} This sounds like a good idea if we could use
>}   if( ppc_md.powersave != NULL)
>}        ppc_md.powersave();
>}
>} If it is determined that calling power_save() which is resides in an
>} arch/processor specific file then we are talking about many files being
>} hit.  and the current power_save seems to common for many other ppc
>} platforms other than 4xx & 8xx
>


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: [RFC/Patch] 4xx idle loop

[Cort Dougan]

I can only think of three ifdef's that would be necessary now but it could
grow.  If the #ifdef snarl is unattractive in idle.c it's easy enough to
move it to chipfamily-specific headers so that idle.c just needs to call
arch_idle() to enter an idle state.

The function pointer isn't desirable.  What the correct strategy for power
saving is known at compile time so there shouldn't be a function pointer
dereference.  How the #ifdef's are done doesn't really matter as long as
the inefficiency of a function pointer is avoided.

} I thought one of the linuxppc desgin goals was to keep the ifdefs to a
} minimum.  I can see idle.c growing quite large and full of #ifdefs if we
} do it that way.  Rather than using ppc_md, make power_save an
} abstraction similar to platform_init.
}
} >
} >
} >} This sounds like a good idea if we could use
} >}   if( ppc_md.powersave != NULL)
} >}        ppc_md.powersave();
} >}
} >} If it is determined that calling power_save() which is resides in an
} >} arch/processor specific file then we are talking about many files being
} >} hit.  and the current power_save seems to common for many other ppc
} >} platforms other than 4xx & 8xx

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: [RFC/Patch] 4xx idle loop

[Benjamin Herrenschmidt]

>
>I can only think of three ifdef's that would be necessary now but it could
>grow.  If the #ifdef snarl is unattractive in idle.c it's easy enough to
>move it to chipfamily-specific headers so that idle.c just needs to call
>arch_idle() to enter an idle state.
>
>The function pointer isn't desirable.  What the correct strategy for power
>saving is known at compile time so there shouldn't be a function pointer
>dereference.  How the #ifdef's are done doesn't really matter as long as
>the inefficiency of a function pointer is avoided.

Well, while I tend to agree with you on this, experience proved that
slightly abusing the ppc_md. indirection somewhat helped make the
code cleaner (read: more self-contained, less cruft, ...)

Also, in this specific case, we might well want to have an machine
specific power saving feature: I've had various tweaks in mind for
powermac laptops that I never ended up implementing...

ben.


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: [RFC/Patch] 4xx idle loop

[Cort Dougan]

That can be implemented within the arch_idle() for the given chip type.  If
6xx chips want to do some pmac specific idle it can have a table of its
own or look int ppc_md.board_specific_power_save.  On 4xx there may not
be a need for that so there's no need for the indirection.

The chip idle is invariant after the compile but the board specific idle
may or may not be so push that into the chip specific function.  That way
no indirection is forced on configurations that don't need it.

} Well, while I tend to agree with you on this, experience proved that
} slightly abusing the ppc_md. indirection somewhat helped make the
} code cleaner (read: more self-contained, less cruft, ...)
}
} Also, in this specific case, we might well want to have an machine
} specific power saving feature: I've had various tweaks in mind for
} powermac laptops that I never ended up implementing...

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: [RFC/Patch] 4xx idle loop

[Todd Poynor]

Benjamin Herrenschmidt wrote:

> Also, in this specific case, we might well want to have an machine
> specific power saving feature: I've had various tweaks in mind for
> powermac laptops that I never ended up implementing...

This will also be true for the IBM 405LP (starting with the "Beech" eval
board), which has a number of new PM features (
http://www.research.ibm.com/arl/projects/papers/405LP.pdf ).  IBM has
internally used a pm_idle function pointer a la i386, hoping that would
be the community-accepted way, in case ya wanna add that to the
suggestion pile, but I think any of the methods proposed would be fine.


--
Todd


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: [RFC/Patch] 4xx idle loop

[Dan Malek]

Benjamin Herrenschmidt wrote:


> Well, while I tend to agree with you on this, experience proved that
> slightly abusing the ppc_md. indirection somewhat helped make the
> code cleaner (read: more self-contained, less cruft, ...)

All of the architectures except PowerPC seem to have a indirect
pointer to a power saving idle function from the idle loop.  If
you don't want to follow this, we could have all of the board
specific files contain a 'power_save()' function, which could be
empty, always compile it and always call it.  Today, the power
saving stuff is all 6xx/7xx/Mac specific, which kinda needs to
change if we want address the needs of embedded processors and
products.


	-- Dan


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: [RFC/Patch] 4xx idle loop

[akuster]

Dan Malek wrote:
> Benjamin Herrenschmidt wrote:
>
>
>> Well, while I tend to agree with you on this, experience proved that
>> slightly abusing the ppc_md. indirection somewhat helped make the
>> code cleaner (read: more self-contained, less cruft, ...)
>
>
> All of the architectures except PowerPC seem to have a indirect
> pointer to a power saving idle function from the idle loop.  If
> you don't want to follow this, we could have all of the board
> specific files contain a 'power_save()' function, which could be
> empty, always compile it and always call it.  Today, the power
> saving stuff is all 6xx/7xx/Mac specific, which kinda needs to
> change if we want address the needs of embedded processors and
> products.
>
>
>     -- Dan
>
>
>

Here is what I think might work.  I am borrowing the idea from i386 &
Arm.  This will allow greater flexibilty for thos who need it.  I have
an example for both a 4xx impilmentation and what would be needed in idle.c.

What do think :)


armin



diff -Nru a/arch/ppc/kernel/idle.c b/arch/ppc/kernel/idle.c
--- a/arch/ppc/kernel/idle.c	Sat Jul 27 09:24:06 2002
+++ b/arch/ppc/kernel/idle.c	Sat Jul 27 09:24:06 2002
@@ -1,5 +1,5 @@
  /*
- * BK Id: %F% %I% %G% %U% %#%
+ * BK Id: SCCS/s.idle.c 1.31 04/16/02 21:42:08 paulus
   */
  /*
   * Idle daemon for PowerPC.  Idle daemon will handle any action
@@ -11,6 +11,11 @@
   * modify it under the terms of the GNU General Public License
   * as published by the Free Software Foundation; either version
   * 2 of the License, or (at your option) any later version.
+ *
+ * 	07/27/02 - Armin
+ * 	added powersave idel loop indirection scheme borrowed from
+ * 	i386 & Arm so other ppc archs can have their own if the
+ * 	default is not sufficiant.
   */
  #include <linux/config.h>
  #include <linux/errno.h>
@@ -50,6 +55,8 @@

  void zero_paged(void);
  void power_save(void);
+void (*pm_idle)(void);
+

  unsigned long zero_paged_on = 0;
  unsigned long powersave_nap = 0;
@@ -96,9 +103,12 @@

	}
  		}
  #endif
-
	if (do_power_save && !current->need_resched)
-
		power_save();
+
	void (*idle)(void) = pm_idle;
+
	if (!idle)
+
		   idle = power_save;

+
	if (do_power_save && !current->need_resched)
+
		idle();
  		if (current->need_resched) {

	run_light_on(1);

	schedule();
diff -Nru a/arch/ppc/kernel/ppc4xx_setup.c b/arch/ppc/kernel/ppc4xx_setup.c
--- a/arch/ppc/kernel/ppc4xx_setup.c	Sat Jul 27 09:24:06 2002
+++ b/arch/ppc/kernel/ppc4xx_setup.c	Sat Jul 27 09:24:06 2002
@@ -27,6 +27,9 @@
   *	History: 04/18/02 - Armin
   *	added ash to setting CETE bit in calibrate()
   *
+ *		: 07/27/02 - Armin
+ *		Added powersave idle loop
+ *
   */

  #include <linux/config.h>
@@ -60,6 +63,7 @@

  /* Function Prototypes */
  static void ppc4xx_gdb_init(void);
+static void arch_power_save(void);

  extern void abort(void);
  extern void ppc4xx_find_bridges(void);
@@ -87,6 +91,7 @@
  extern void board_io_mapping(void);
  extern void board_setup_irq(void);
  extern void board_init(void);
+extern void (*pm_idle) (void);

  /* Global Variables */
  unsigned char __res[sizeof (bd_t)];
@@ -94,7 +99,7 @@
  static void __init
  ppc4xx_setup_arch(void)
  {
-
+
  	/* Setup PCI host bridges */

  #ifdef CONFIG_PCI
@@ -110,6 +115,9 @@
  	board_setup_arch();

  	ppc4xx_gdb_init();
+

+
pm_idle = arch_power_save;
+
  }

  /*
@@ -454,4 +462,38 @@
  	board_init();

  	return;
+}
+
+void arch_power_save(void)
+{
+
extern void (*pm_idle) (void);
+
extern unsigned long powersave_nap;
+
int nap = powersave_nap;
+

+
pm_idle = arch_power_save;
+

+
if (!(nap || (cur_cpu_spec[smp_processor_id()]->cpu_features &
CPU_FTR_CAN_DOZE)))
+
	return;
+
/*
+
  * Disable interrupts to prevent a lost wakeup
+
  * when going to sleep.  This is necessary even with
+
  * RTLinux since we are not guaranteed an interrupt
+
  * didn't come in and is waiting for a __sti() before
+
  * emulating one.  This way, we really do hard disable.
+
  *
+
  * We assume that we're sti-ed when we come in here.  We
+
  * are in the idle loop so if we're cli-ed then it's a bug
+
  * anyway.
+
  *  -- Cort
+
  */
+
_nmask_and_or_msr(MSR_EE, 0);
+
if (!current->need_resched)
+

+
	/* set the POW bit in the MSR, and enable interrupts
+
	 * so we wake up sometime! */
+
	_nmask_and_or_msr(0, MSR_POW | MSR_EE);
+

+
_nmask_and_or_msr(0, MSR_EE);
+

+
pm_idle = arch_power_save;
  }


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/