Re: macppc: support for Dynamic Frequency Switching

2011-05-16 Thread Martin Pieuchot 
On 15/05/11(Sun) 21:24, Stuart Henderson wrote:
 On 2011/05/15 15:56, Martin Pieuchot wrote:
  Updated diff, now looking for oks for the driver and the manpage.
 
 This diff doesn't include the cpu.c/cpu.h part

Right. Here's the complete diff. I've also fixed the typo you told me.

Index: conf/GENERIC
===
RCS file: /cvs/src/sys/arch/macppc/conf/GENERIC,v
retrieving revision 1.207
diff -u -p -r1.207 GENERIC
--- conf/GENERIC19 Apr 2011 23:07:54 -  1.207
+++ conf/GENERIC16 May 2011 12:45:27 -
@@ -155,6 +155,7 @@ macgpio*at macobio? # GPIO, PMU interru
 macgpio*   at macgpio? # GPIO, PMU interrupt router.
 sysbutton* at macgpio? # Xserve system id button.
 pgs*   at macgpio? # Programmer Switch.
+dfs*   at macgpio? # Dynamic Frequency Switching.
 akbd*  at adb? # ADB keyboard
 wskbd* at akbd? mux 1
 ams*   at adb? # ADB mouse
Index: conf/files.macppc
===
RCS file: /cvs/src/sys/arch/macppc/conf/files.macppc,v
retrieving revision 1.63
diff -u -p -r1.63 files.macppc
--- conf/files.macppc   6 Dec 2010 20:10:18 -   1.63
+++ conf/files.macppc   16 May 2011 12:45:27 -
@@ -237,6 +237,10 @@ device pgs {}
 attach pgs at macgpio
 file   arch/macppc/dev/pgs.c
 
+device dfs {}
+attach dfs at macgpio
+file   arch/macppc/dev/dfs.c
+
 attach wdc at mediabay, macobio, kauaiata with wdc_obio
 file   arch/macppc/dev/wdc_obio.c  wdc_obio
 
Index: dev/dfs.c
===
RCS file: dev/dfs.c
diff -N dev/dfs.c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ dev/dfs.c   16 May 2011 12:45:27 -
@@ -0,0 +1,167 @@
+/* $OpenBSD$   */
+/*
+ * Copyright (c) 2011 Martin Pieuchot m...@nolizard.org
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include sys/param.h
+#include sys/proc.h
+#include sys/sysctl.h
+
+#include dev/ofw/openfirm.h
+
+#include machine/cpu.h
+#include machine/autoconf.h
+#include macppc/pci/macobio.h
+
+#define DFS2   (1  22)   /* Divide-by-Two */
+#define DFS4   (1  23)   /* Divide-by-Four (MPC7448 Specific) */
+
+extern int perflevel;
+
+struct dfs_softc {
+   struct device   sc_dev;
+   int sc_voltage;
+};
+
+intdfs_match(struct device *, void *, void *);
+void   dfs_attach(struct device *, struct device *, void *);
+void   dfs_setperf(int);
+void   dfs_scale_frequency(u_int);
+
+struct cfattach dfs_ca = {
+   sizeof(struct dfs_softc), dfs_match, dfs_attach
+};
+
+struct cfdriver dfs_cd = {
+   NULL, dfs, DV_DULL
+};
+
+int
+dfs_match(struct device *parent, void *arg, void *aux)
+{
+   struct confargs *ca = aux;
+   uint16_t cpu;
+
+   if (strcmp(ca-ca_name, cpu-vcore-select) != 0)
+   return (0);
+
+   cpu = ppc_mfpvr()  16;
+   if (cpu == PPC_CPU_MPC7447A || cpu == PPC_CPU_MPC7448)
+   return (1);
+
+   return (0);
+}
+
+void
+dfs_attach(struct device *parent, struct device *self, void *aux)
+{
+   struct dfs_softc *sc = (struct dfs_softc *)self;
+   struct confargs *ca = aux;
+   uint32_t hid1, reg;
+   uint16_t cpu;
+
+   /*
+* On some models the vcore-select offset is relative to
+* its parent offset and not to the bus base address.
+*/
+   OF_getprop(OF_parent(ca-ca_node), reg, reg, sizeof(reg));
+   if (reg  ca-ca_reg[0])
+   sc-sc_voltage = reg + ca-ca_reg[0];
+   else
+   sc-sc_voltage = ca-ca_reg[0];
+
+   hid1 = ppc_mfhid1();
+
+   if (hid1  DFS4) {
+   ppc_curfreq = ppc_maxfreq / 4;
+   perflevel = 25;
+   } else if (hid1  DFS2) {
+   ppc_curfreq = ppc_maxfreq / 2;
+   perflevel = 50;
+   }
+
+   cpu_setperf = dfs_setperf;
+
+   printf(: speeds: %d, %d, ppc_maxfreq, ppc_maxfreq / 2);
+
+   cpu = ppc_mfpvr()  16;
+   if (cpu == PPC_CPU_MPC7448)
+   printf(, %d, ppc_maxfreq / 4);
+   printf( MHz\n);
+}
+
+void
+dfs_setperf(int perflevel)
+{
+   struct dfs_softc *sc = dfs_cd.cd_devs[0];
+
+   if (perflevel

Re: macppc: support for Dynamic Frequency Switching

2011-05-15 Thread Martin Pieuchot 
On 04/05/11(Wed) 20:29, Miod Vallat wrote:
   Speaking of DELAY()... it is implemented using the processor internal
   counter register. Is this register impacted by frequency changes? If so,
   shouldn't you update the computed ns_per_tick delay() constant?
  
  Reading the doc again, it's said that the time base register is clocked
  at one-fourth of the bus clock. But the DFS feature divides the processor 
  to system bus ratio. So, if I understand well there is no impact on the
  time base counter frequency.
 
 Good. This is easy to check, does ntpd start complaining after running a
 few minutes at `setperf=0' speed?
 
  Index: sys/arch/macppc/dev/dfs.c
 
  +#include sys/param.h
  +#include sys/filedesc.h
 
 Could you use sys/proc.h instead of sys/filedesc.h here? This is the
 preferred (yet objectionable) form of satisfying sys/sysctl.h
 dependencies.
 
  +struct cfattach dfs_ca = {
  +   sizeof(struct device), dfs_match, dfs_attach
 
 This needs to be sizeof(struct dfs_softc) now. That is, unless you want
 to get funny panics after dfs0 attaches.

After some tests and discussions with the powermac owners it appears
that there is no obvious way to enable dfs on these machines. So no dfs
support for the mac mini.

Updated diff, now looking for oks for the driver and the manpage.

Martin

Index: conf/GENERIC
===
RCS file: /cvs/src/sys/arch/macppc/conf/GENERIC,v
retrieving revision 1.207
diff -u -p -r1.207 GENERIC
--- conf/GENERIC19 Apr 2011 23:07:54 -  1.207
+++ conf/GENERIC15 May 2011 09:25:14 -
@@ -155,6 +155,7 @@ macgpio*at macobio? # GPIO, PMU interru
 macgpio*   at macgpio? # GPIO, PMU interrupt router.
 sysbutton* at macgpio? # Xserve system id button.
 pgs*   at macgpio? # Programmer Switch.
+dfs*   at macgpio? # Dynamic Frequence Switching.
 akbd*  at adb? # ADB keyboard
 wskbd* at akbd? mux 1
 ams*   at adb? # ADB mouse
Index: conf/files.macppc
===
RCS file: /cvs/src/sys/arch/macppc/conf/files.macppc,v
retrieving revision 1.63
diff -u -p -r1.63 files.macppc
--- conf/files.macppc   6 Dec 2010 20:10:18 -   1.63
+++ conf/files.macppc   15 May 2011 09:25:14 -
@@ -237,6 +237,10 @@ device pgs {}
 attach pgs at macgpio
 file   arch/macppc/dev/pgs.c
 
+device dfs {}
+attach dfs at macgpio
+file   arch/macppc/dev/dfs.c
+
 attach wdc at mediabay, macobio, kauaiata with wdc_obio
 file   arch/macppc/dev/wdc_obio.c  wdc_obio
 
Index: dev/dfs.c
===
RCS file: dev/dfs.c
diff -N dev/dfs.c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ dev/dfs.c   15 May 2011 09:25:14 -
@@ -0,0 +1,167 @@
+/* $OpenBSD$   */
+/*
+ * Copyright (c) 2011 Martin Pieuchot m...@nolizard.org
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include sys/param.h
+#include sys/proc.h
+#include sys/sysctl.h
+
+#include dev/ofw/openfirm.h
+
+#include machine/cpu.h
+#include machine/autoconf.h
+#include macppc/pci/macobio.h
+
+#define DFS2   (1  22)   /* Divide-by-Two */
+#define DFS4   (1  23)   /* Divide-by-Four (MPC7448 Specific) */
+
+extern int perflevel;
+
+struct dfs_softc {
+   struct device   sc_dev;
+   int sc_voltage;
+};
+
+intdfs_match(struct device *, void *, void *);
+void   dfs_attach(struct device *, struct device *, void *);
+void   dfs_setperf(int);
+void   dfs_scale_frequency(u_int);
+
+struct cfattach dfs_ca = {
+   sizeof(struct dfs_softc), dfs_match, dfs_attach
+};
+
+struct cfdriver dfs_cd = {
+   NULL, dfs, DV_DULL
+};
+
+int
+dfs_match(struct device *parent, void *arg, void *aux)
+{
+   struct confargs *ca = aux;
+   uint16_t cpu;
+
+   if (strcmp(ca-ca_name, cpu-vcore-select) != 0)
+   return (0);
+
+   cpu = ppc_mfpvr()  16;
+   if (cpu == PPC_CPU_MPC7447A || cpu == PPC_CPU_MPC7448)
+   return (1);
+
+   return (0);
+}
+
+void
+dfs_attach(struct device *parent, struct device *self, void *aux)
+{
+   struct dfs_softc *sc = (struct dfs_softc *)self;
+   struct confargs *ca = aux;
+

Re: macppc: support for Dynamic Frequency Switching

2011-05-15 Thread Kenneth R Westerback 
On Sun, May 15, 2011 at 03:56:16PM +0530, Martin Pieuchot wrote:
 On 04/05/11(Wed) 20:29, Miod Vallat wrote:
Speaking of DELAY()... it is implemented using the processor internal
counter register. Is this register impacted by frequency changes? If so,
shouldn't you update the computed ns_per_tick delay() constant?
   
   Reading the doc again, it's said that the time base register is clocked
   at one-fourth of the bus clock. But the DFS feature divides the processor 
   to system bus ratio. So, if I understand well there is no impact on the
   time base counter frequency.
  
  Good. This is easy to check, does ntpd start complaining after running a
  few minutes at `setperf=0' speed?
  
   Index: sys/arch/macppc/dev/dfs.c
  
   +#include sys/param.h
   +#include sys/filedesc.h
  
  Could you use sys/proc.h instead of sys/filedesc.h here? This is the
  preferred (yet objectionable) form of satisfying sys/sysctl.h
  dependencies.
  
   +struct cfattach dfs_ca = {
   + sizeof(struct device), dfs_match, dfs_attach
  
  This needs to be sizeof(struct dfs_softc) now. That is, unless you want
  to get funny panics after dfs0 attaches.
 
 After some tests and discussions with the powermac owners it appears
 that there is no obvious way to enable dfs on these machines. So no dfs
 support for the mac mini.
 
 Updated diff, now looking for oks for the driver and the manpage.
 
 Martin
 
 Index: conf/GENERIC
 ===
 RCS file: /cvs/src/sys/arch/macppc/conf/GENERIC,v
 retrieving revision 1.207
 diff -u -p -r1.207 GENERIC
 --- conf/GENERIC  19 Apr 2011 23:07:54 -  1.207
 +++ conf/GENERIC  15 May 2011 09:25:14 -
 @@ -155,6 +155,7 @@ macgpio*  at macobio? # GPIO, PMU interru
  macgpio* at macgpio? # GPIO, PMU interrupt router.
  sysbutton*   at macgpio? # Xserve system id button.
  pgs* at macgpio? # Programmer Switch.
 +dfs* at macgpio? # Dynamic Frequence Switching.
  akbd*at adb? # ADB keyboard
  wskbd*   at akbd? mux 1
  ams* at adb? # ADB mouse
 Index: conf/files.macppc
 ===
 RCS file: /cvs/src/sys/arch/macppc/conf/files.macppc,v
 retrieving revision 1.63
 diff -u -p -r1.63 files.macppc
 --- conf/files.macppc 6 Dec 2010 20:10:18 -   1.63
 +++ conf/files.macppc 15 May 2011 09:25:14 -
 @@ -237,6 +237,10 @@ device   pgs {}
  attach   pgs at macgpio
  file arch/macppc/dev/pgs.c
  
 +device   dfs {}
 +attach   dfs at macgpio
 +file arch/macppc/dev/dfs.c
 +
  attach   wdc at mediabay, macobio, kauaiata with wdc_obio
  file arch/macppc/dev/wdc_obio.c  wdc_obio
  
 Index: dev/dfs.c
 ===
 RCS file: dev/dfs.c
 diff -N dev/dfs.c
 --- /dev/null 1 Jan 1970 00:00:00 -
 +++ dev/dfs.c 15 May 2011 09:25:14 -
 @@ -0,0 +1,167 @@
 +/*   $OpenBSD$   */
 +/*
 + * Copyright (c) 2011 Martin Pieuchot m...@nolizard.org
 + *
 + * Permission to use, copy, modify, and distribute this software for any
 + * purpose with or without fee is hereby granted, provided that the above
 + * copyright notice and this permission notice appear in all copies.
 + *
 + * THE SOFTWARE IS PROVIDED AS IS AND THE AUTHOR DISCLAIMS ALL WARRANTIES
 + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
 + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
 + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
 + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
 + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 + */
 +
 +#include sys/param.h
 +#include sys/proc.h
 +#include sys/sysctl.h
 +
 +#include dev/ofw/openfirm.h
 +
 +#include machine/cpu.h
 +#include machine/autoconf.h
 +#include macppc/pci/macobio.h
 +
 +#define DFS2 (1  22)   /* Divide-by-Two */
 +#define DFS4 (1  23)   /* Divide-by-Four (MPC7448 Specific) */
 +
 +extern int perflevel;
 +
 +struct dfs_softc {
 + struct device   sc_dev;
 + int sc_voltage;
 +};
 +
 +int  dfs_match(struct device *, void *, void *);
 +void dfs_attach(struct device *, struct device *, void *);
 +void dfs_setperf(int);
 +void dfs_scale_frequency(u_int);
 +
 +struct cfattach dfs_ca = {
 + sizeof(struct dfs_softc), dfs_match, dfs_attach
 +};
 +
 +struct cfdriver dfs_cd = {
 + NULL, dfs, DV_DULL
 +};
 +
 +int
 +dfs_match(struct device *parent, void *arg, void *aux)
 +{
 + struct confargs *ca = aux;
 + uint16_t cpu;
 +
 + if (strcmp(ca-ca_name, cpu-vcore-select) != 0)
 + return (0);
 +
 + cpu = ppc_mfpvr()  16;
 + if (cpu == PPC_CPU_MPC7447A || cpu == PPC_CPU_MPC7448)
 + return (1);
 +
 + return (0);
 +}
 +

Re: macppc: support for Dynamic Frequency Switching

2011-05-15 Thread Stuart Henderson 
On 2011/05/15 15:56, Martin Pieuchot wrote:
 Updated diff, now looking for oks for the driver and the manpage.

This diff doesn't include the cpu.c/cpu.h part

Re: macppc: support for Dynamic Frequency Switching

2011-05-05 Thread Martin Pieuchot 
On 04/05/11(Wed) 20:29, Miod Vallat wrote:
   Speaking of DELAY()... it is implemented using the processor internal
   counter register. Is this register impacted by frequency changes? If so,
   shouldn't you update the computed ns_per_tick delay() constant?
  
  Reading the doc again, it's said that the time base register is clocked
  at one-fourth of the bus clock. But the DFS feature divides the processor 
  to system bus ratio. So, if I understand well there is no impact on the
  time base counter frequency.
 
 Good. This is easy to check, does ntpd start complaining after running a
 few minutes at `setperf=0' speed?

It doesn't complain and adjusts the clock the same way than with
setperf=100.

 
  Index: sys/arch/macppc/dev/dfs.c
 
  +#include sys/param.h
  +#include sys/filedesc.h
 
 Could you use sys/proc.h instead of sys/filedesc.h here? This is the
 preferred (yet objectionable) form of satisfying sys/sysctl.h
 dependencies.

Understood.

  +struct cfattach dfs_ca = {
  +   sizeof(struct device), dfs_match, dfs_attach
 
 This needs to be sizeof(struct dfs_softc) now. That is, unless you want
 to get funny panics after dfs0 attaches.

Of course. I'm waiting some more test from the powermac owners to know
if their machine support DFS or not, before sending a new diff.

Re: macppc: support for Dynamic Frequency Switching

2011-05-04 Thread Martin Pieuchot 
On 03/05/11(Tue) 22:24, Mark Kettenis wrote:
  Date: Tue, 26 Apr 2011 18:29:16 +0530
  From: Martin Pieuchot mpieuc...@nolizard.org
  
  The following diff adds support for dfs. It requires my precedent patch
  about GPIOs. I don't have a machine with a MPC7448 so it's only tested
  with a MPC7447A.
  
  I'm also interested in various device-tree dumps for further development.
  If you can send me yours, contact me off list.
  
  Comments ?
 
 Looks good!  Unfortunately the G4 mini (which has the MPC7447A)
 doesn't support this.

Can you send me your device-tree dump? If it's really a MPC7447A (not a
MPC7447) then the processor has the HID1 register and supports DFS2. So
the question is how to change the core voltage.

 Just two comments:
 
  Index: dev/dfs.c
  [...]
  +
  +#include sys/param.h
  +#include sys/filedesc.h
 
 Why do you need to include sys/filedesc.h for?

Its needed by sys/sysctl.h, otherwise I get a struct filedesc not declared
warning. Should I include an other file instead?

 
  +#include sys/sysctl.h
  +
  +#include machine/cpu.h
  +#include machine/autoconf.h
  +#include macppc/pci/macobio.h
  +
  +#defineDFS2(1  22)   /* Divide-by-Two */
  +#defineDFS4(1  23)   /* Divide-by-Four (MPC7448 Specific) */
  +
  +extern int perflevel;
  +static int voltage;
 
 You should store the voltage inside a softc (pointed out by miod@).

ok.

  [...]
  +
  +   printf(: Dynamic Frequency Switching, speeds: );
  +   printf(%d, %d, ppc_maxfreq, ppc_maxfreq / 2);
 
 We typically use pure lower case for stuff that shows up in dmesg.
 And I'm not sure the Dynamic Frequency Switching part really adds
 useful information.  Perhaps just change this in:
 
  +   printf(: speeds: %d, %d, ppc_maxfreq, ppc_maxfreq / 2);

New patch coming.

Re: macppc: support for Dynamic Frequency Switching

2011-05-04 Thread Stuart Henderson 
On 2011/05/04 16:11, Martin Pieuchot wrote:
  
  Looks good!  Unfortunately the G4 mini (which has the MPC7447A)
  doesn't support this.
 
 Can you send me your device-tree dump? If it's really a MPC7447A (not a
 MPC7447) then the processor has the HID1 register and supports DFS2. So
 the question is how to change the core voltage.

http://junkpile.org/macmini-g4.txt

(I would normally inline it but I figured 1300 lines is probably a
bit big to send to the list for something only a few people will want
to see).

Re: macppc: support for Dynamic Frequency Switching

2011-05-04 Thread Martin Pieuchot 
On 03/05/11(Tue) 20:37, Miod Vallat wrote:
  Index: dev/dfs.c
 
  +static int voltage;
 
 There is no reason for this variable to be global. You should put it in
 your device' softc instead.

Ok, new diff below.

  +int
  +dfs_match(struct device *parent, void *arg, void *aux)
  +{
  +   struct confargs *ca = aux;
  +   uint16_t cpu;
  +
  +   if (strcmp(ca-ca_name, cpu-vcore-select) != 0)
  +   return (0);
  +
  +   cpu = ppc_mfpvr()  16;
 
 Is there any possibility for this device to attach to multiprocessor
 machines? If so, what if there are multiple cpu-vcore-select nodes, one
 per processor? Oh well, I suppose there won't be multiprocessor systems
 with different cpu models... at least not on macppc.

I'm not aware of such machine but for what I understand the
cpu-vcore-select node shouldn't be per processor.

  +   cpu = ppc_mfpvr()  16;
  +   if (cpu == PPC_CPU_MPC7448)
  +   printf(, %d MHz\n, ppc_maxfreq / 4);
  +   else
  +   printf( MHz\n);
 
 Nitpicking, but I'd rather see the optional /4 freq be printed as , %d
 and an unconditional  MHz\n in all cases.

See below.

  +void
  +dfs_setperf(int perflevel)
  +{
  +   if (perflevel  50) {
  +   if (ppc_curfreq != ppc_maxfreq) {
  +   macobio_write(voltage, GPIO_DDR_OUTPUT | 1);
  +   DELAY(1000);
 
 Speaking of DELAY()... it is implemented using the processor internal
 counter register. Is this register impacted by frequency changes? If so,
 shouldn't you update the computed ns_per_tick delay() constant?

Reading the doc again, it's said that the time base register is clocked
at one-fourth of the bus clock. But the DFS feature divides the processor 
to system bus ratio. So, if I understand well there is no impact on the
time base counter frequency.

 Also, this register is used to feed the timecounter. Is the clock still
 accurate after running for a while at a lower frequency? Is there any
 other, fixed, clock source which can be used as a safe monotonic clock
 source?

I don't know for an other clock source but for the accuracy of the time
base register, like I just said if I understand well, there should be
no impact.

New diff, manpage included with some tweaks by jmc@ .

Martin


Index: sys/arch/macppc/dev/dfs.c
===
RCS file: sys/arch/macppc/dev/dfs.c
diff -N sys/arch/macppc/dev/dfs.c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ sys/arch/macppc/dev/dfs.c   4 May 2011 12:40:54 -
@@ -0,0 +1,167 @@
+/* $OpenBSD$   */
+/*
+ * Copyright (c) 2011 Martin Pieuchot m...@nolizard.org
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include sys/param.h
+#include sys/filedesc.h
+#include sys/sysctl.h
+
+#include dev/ofw/openfirm.h
+
+#include machine/cpu.h
+#include machine/autoconf.h
+#include macppc/pci/macobio.h
+
+#define DFS2   (1  22)   /* Divide-by-Two */
+#define DFS4   (1  23)   /* Divide-by-Four (MPC7448 Specific) */
+
+extern int perflevel;
+
+struct dfs_softc {
+   struct device   sc_dev;
+   int sc_voltage;
+};
+
+intdfs_match(struct device *, void *, void *);
+void   dfs_attach(struct device *, struct device *, void *);
+void   dfs_setperf(int);
+void   dfs_scale_frequency(u_int);
+
+struct cfattach dfs_ca = {
+   sizeof(struct device), dfs_match, dfs_attach
+};
+
+struct cfdriver dfs_cd = {
+   NULL, dfs, DV_DULL
+};
+
+int
+dfs_match(struct device *parent, void *arg, void *aux)
+{
+   struct confargs *ca = aux;
+   uint16_t cpu;
+
+   if (strcmp(ca-ca_name, cpu-vcore-select) != 0)
+   return (0);
+
+   cpu = ppc_mfpvr()  16;
+   if (cpu == PPC_CPU_MPC7447A || cpu == PPC_CPU_MPC7448)
+   return (1);
+
+   return (0);
+}
+
+void
+dfs_attach(struct device *parent, struct device *self, void *aux)
+{
+   struct dfs_softc *sc = (struct dfs_softc *)self;
+   struct confargs *ca = aux;
+   uint32_t hid1, reg;
+   uint16_t cpu;
+
+   /*
+* On some models the vcore-select offset is relative to
+* its parent offset and not to the bus base address.
+*/
+   OF_getprop(OF_parent(ca-ca_node), reg, reg, sizeof(reg));
+   if (reg  ca-ca_reg[0])
+   sc-sc_voltage = reg +

Re: macppc: support for Dynamic Frequency Switching

2011-05-04 Thread Kenneth R Westerback 
On Wed, May 04, 2011 at 11:58:16AM +0100, Stuart Henderson wrote:
 On 2011/05/04 16:11, Martin Pieuchot wrote:
   
   Looks good!  Unfortunately the G4 mini (which has the MPC7447A)
   doesn't support this.
  
  Can you send me your device-tree dump? If it's really a MPC7447A (not a
  MPC7447) then the processor has the HID1 register and supports DFS2. So
  the question is how to change the core voltage.
 
 http://junkpile.org/macmini-g4.txt
 
 (I would normally inline it but I figured 1300 lines is probably a
 bit big to send to the list for something only a few people will want
 to see).
 

I apparently have a 7447A mac mini too:

cpu0 at mainbus0: 7447A (Revision 0x105): 1499 MHz: 512KB L2 cache

So if you need more info let me know. I'll be trying to get your
diffs into may tree asap.

 Ken

Re: macppc: support for Dynamic Frequency Switching

2011-05-04 Thread Miod Vallat 
  Speaking of DELAY()... it is implemented using the processor internal
  counter register. Is this register impacted by frequency changes? If so,
  shouldn't you update the computed ns_per_tick delay() constant?
 
 Reading the doc again, it's said that the time base register is clocked
 at one-fourth of the bus clock. But the DFS feature divides the processor 
 to system bus ratio. So, if I understand well there is no impact on the
 time base counter frequency.

Good. This is easy to check, does ntpd start complaining after running a
few minutes at `setperf=0' speed?

 Index: sys/arch/macppc/dev/dfs.c

 +#include sys/param.h
 +#include sys/filedesc.h

Could you use sys/proc.h instead of sys/filedesc.h here? This is the
preferred (yet objectionable) form of satisfying sys/sysctl.h
dependencies.

 +struct cfattach dfs_ca = {
 + sizeof(struct device), dfs_match, dfs_attach

This needs to be sizeof(struct dfs_softc) now. That is, unless you want
to get funny panics after dfs0 attaches.

Re: macppc: support for Dynamic Frequency Switching

2011-05-03 Thread Miod Vallat 
 You can check if your machine supports it by looking for the
 cpu-vcore-select node in your openfirmware dump:
 
 $ eeprom -p |grep vcore

FWIW, the only known (to us) systems with that node are:
PowerBook4,3
PowerBook6,5
PowerBook6,7
PowerBook6,8

as reported on the `mainbus0' line in dmesg.

Miod

Re: macppc: support for Dynamic Frequency Switching

2011-05-03 Thread Mark Kettenis 
 Date: Tue, 26 Apr 2011 18:29:16 +0530
 From: Martin Pieuchot mpieuc...@nolizard.org
 
 The following diff adds support for dfs. It requires my precedent patch
 about GPIOs. I don't have a machine with a MPC7448 so it's only tested
 with a MPC7447A.
 
 I'm also interested in various device-tree dumps for further development.
 If you can send me yours, contact me off list.
 
 Comments ?

Looks good!  Unfortunately the G4 mini (which has the MPC7447A)
doesn't support this.

Just two comments:

 Index: dev/dfs.c
 ===
 RCS file: dev/dfs.c
 diff -N dev/dfs.c
 --- /dev/null 1 Jan 1970 00:00:00 -
 +++ dev/dfs.c 26 Apr 2011 12:13:22 -
 @@ -0,0 +1,150 @@
 +/*   $OpenBSD$   */
 +/*
 + * Copyright (c) 2011 Martin Pieuchot m...@nolizard.org
 + *
 + * Permission to use, copy, modify, and distribute this software for any
 + * purpose with or without fee is hereby granted, provided that the above
 + * copyright notice and this permission notice appear in all copies.
 + *
 + * THE SOFTWARE IS PROVIDED AS IS AND THE AUTHOR DISCLAIMS ALL WARRANTIES
 + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
 + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
 + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
 + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
 + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 + */
 +
 +#include sys/param.h
 +#include sys/filedesc.h

Why do you need to include sys/filedesc.h for?

 +#include sys/sysctl.h
 +
 +#include machine/cpu.h
 +#include machine/autoconf.h
 +#include macppc/pci/macobio.h
 +
 +#define  DFS2(1  22)   /* Divide-by-Two */
 +#define  DFS4(1  23)   /* Divide-by-Four (MPC7448 Specific) */
 +
 +extern int perflevel;
 +static int voltage;

You should store the voltage inside a softc (pointed out by miod@).

 +int  dfs_match(struct device *, void *, void *);
 +void dfs_attach(struct device *, struct device *, void *);
 +void dfs_setperf(int);
 +void dfs_scale_frequency(u_int);
 +
 +struct cfattach dfs_ca = {
 + sizeof(struct device), dfs_match, dfs_attach
 +};
 +
 +struct cfdriver dfs_cd = {
 + NULL, dfs, DV_DULL
 +};
 +
 +int
 +dfs_match(struct device *parent, void *arg, void *aux)
 +{
 + struct confargs *ca = aux;
 + uint16_t cpu;
 +
 + if (strcmp(ca-ca_name, cpu-vcore-select) != 0)
 + return (0);
 +
 + cpu = ppc_mfpvr()  16;
 + if (cpu == PPC_CPU_MPC7447A || cpu == PPC_CPU_MPC7448)
 + return (1);
 +
 + return (0);
 +}
 +
 +void
 +dfs_attach(struct device *parent, struct device *self, void *aux)
 +{
 + struct confargs *ca = aux;
 + uint32_t hid1;
 + uint16_t cpu;
 +
 + voltage = ca-ca_reg[0];
 +
 + hid1 = ppc_mfhid1();
 +
 + if (hid1  DFS4) {
 + ppc_curfreq = ppc_maxfreq / 4;
 + perflevel = 25;
 + } else if (hid1  DFS2) {
 + ppc_curfreq = ppc_maxfreq / 2;
 + perflevel = 50;
 + }
 +
 + cpu_setperf = dfs_setperf;
 +
 + printf(: Dynamic Frequency Switching, speeds: );
 + printf(%d, %d, ppc_maxfreq, ppc_maxfreq / 2);

We typically use pure lower case for stuff that shows up in dmesg.
And I'm not sure the Dynamic Frequency Switching part really adds
useful information.  Perhaps just change this in:

 + printf(: speeds: %d, %d, ppc_maxfreq, ppc_maxfreq / 2);

Re: macppc: support for Dynamic Frequency Switching

2011-05-03 Thread Miod Vallat 
 Index: dev/dfs.c

 +static int voltage;

There is no reason for this variable to be global. You should put it in
your device' softc instead.

 +int
 +dfs_match(struct device *parent, void *arg, void *aux)
 +{
 + struct confargs *ca = aux;
 + uint16_t cpu;
 +
 + if (strcmp(ca-ca_name, cpu-vcore-select) != 0)
 + return (0);
 +
 + cpu = ppc_mfpvr()  16;

Is there any possibility for this device to attach to multiprocessor
machines? If so, what if there are multiple cpu-vcore-select nodes, one
per processor? Oh well, I suppose there won't be multiprocessor systems
with different cpu models... at least not on macppc.

 + cpu = ppc_mfpvr()  16;
 + if (cpu == PPC_CPU_MPC7448)
 + printf(, %d MHz\n, ppc_maxfreq / 4);
 + else
 + printf( MHz\n);

Nitpicking, but I'd rather see the optional /4 freq be printed as , %d
and an unconditional  MHz\n in all cases.

 +void
 +dfs_setperf(int perflevel)
 +{
 + if (perflevel  50) {
 + if (ppc_curfreq != ppc_maxfreq) {
 + macobio_write(voltage, GPIO_DDR_OUTPUT | 1);
 + DELAY(1000);

Speaking of DELAY()... it is implemented using the processor internal
counter register. Is this register impacted by frequency changes? If so,
shouldn't you update the computed ns_per_tick delay() constant?

Also, this register is used to feed the timecounter. Is the clock still
accurate after running for a while at a lower frequency? Is there any
other, fixed, clock source which can be used as a safe monotonic clock
source?

Miod

Re: macppc: support for Dynamic Frequency Switching

2011-05-03 Thread Miod Vallat 
  +   printf(: Dynamic Frequency Switching, speeds: );
  +   printf(%d, %d, ppc_maxfreq, ppc_maxfreq / 2);
 
 We typically use pure lower case for stuff that shows up in dmesg.
 And I'm not sure the Dynamic Frequency Switching part really adds
 useful information.  Perhaps just change this in:

Well, x86 writes `Enhanced SpeedStep', and sets a bad example.

Miod

Re: macppc: support for Dynamic Frequency Switching

2011-04-27 Thread Martin Pieuchot 
On 27/04/11(Wed) 12:41, Martin Pieuchot wrote:
 On 26/04/11(Tue) 18:29, Martin Pieuchot wrote:
  The following diff adds support for dfs. It requires my precedent patch
  about GPIOs. I don't have a machine with a MPC7448 so it's only tested
  with a MPC7447A.
  
  I'm also interested in various device-tree dumps for further development.
  If you can send me yours, contact me off list.
 
 Updated diff, correct the voltage offset in case the value of the reg
 property is relative to the gpio controller offset.

As pointed by landry@ I didn't explain what's dfs. Dynamic Frequency
Switching, aka dfs, is a feature available (at least) in the MPC7447A
and MPC7448 processors that adds the ability to divide the processor bus
ratio. Concretely it allows you to reduce the speed and power consumption
on some Apple G4 machine. 

For example, the difference here on an ibook G4:

-hw.sensors.adt0.volt1=1.54 VDC (Vccp)
+hw.sensors.adt0.volt1=1.29 VDC (Vccp)
 hw.sensors.adt0.volt2=3.28 VDC (Vcc)
 hw.sensors.adt0.volt3=0.00 VDC (+5V)
 hw.sensors.adt0.volt4=0.00 VDC (+12V)
-hw.cpuspeed=1333
-hw.setperf=100
+hw.cpuspeed=666
+hw.setperf=0

You can check if your machine supports it by looking for the
cpu-vcore-select node in your openfirmware dump:

$ eeprom -p |grep vcore

Regards,
Martin