2.6.2 to 2.6.3 regression: powernow-k8 doesn't work anymore

PATCH Pavel Machek 2 participants 4 messages Feb 28, 2004 – Mar 1, 2004 3 messages not in the archive

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Pavel Machek

author

2004-02-28 00:16 UTC #3 lore raw

Hi!

Quoted 17 lines

Quoted 17 lines
Andrew, we made mistake in 2.6.2 to 2.6.3 update; this one should fix it. Please apply,
Pavel
--- clean/arch/i386/kernel/cpu/cpufreq/powernow-k8.c 2004-02-22
23:30:27.000000000 +0100
+++ linux/arch/i386/kernel/cpu/cpufreq/powernow-k8.c 2004-02-22
23:29:38.000000000 +0100
(quoted diff) +2 -1
@@ -521,7 +517,8 @@
 		}
 		if ((pst[j].fid > MAX_FID)
 		    || (pst[j].fid & 1)
-		    || (pst[j].fid < HI_FID_TABLE_BOTTOM)){
+		    || (j && (pst[j].fid < HI_FID_TABLE_BOTTOM))) {
+			/* Only first fid is allowed to be in "low" range */
 			printk(KERN_ERR PFX "fid %d invalid : 0x%x\n", j, 

Quoted 8 lines

Quoted 5 lines
(quoted diff) +0 -0
 			pst[j].fid);
 			return -EINVAL;
		}
While this patch seems to fix one problem, there's still another one:

Sorry, I do not have production hardware here.

Quoted 18 lines

powernow-k8: Found AMD64 processor supporting PowerNow (version 1.00.08a)
powernow-k8: voltage stable in 100 usec, ramp voltage offset: 2, 
isochronous relief time: 3, maximum voltage step: 0
powernow-k8: pll lock time: 0x2, maxfid 0xc (2000 MHz), maxvid 0x0
powernow-k8:    0 : fid 0x0 (800 MHz), vid 0xa
powernow-k8:    1 : fid 0xa (1800 MHz), vid 0x6
powernow-k8:    2 : fid 0xc (2000 MHz), vid 0x2
powernow-k8: currfid 0xc (2000 MHz), currvid 0x0
powernow-k8: BIOS error: currfid/vid do not match PST, ignoring
powernow-k8: cpu_init done, current fid 0xc, vid 0x0
powernow-k8: ph2 null fid transition 0xc

(this looks similar to what the working version from 2.6.2 prints)

but...:

Quoted 1 line

cat /proc/cpufreq

Quoted 8 lines

          minimum CPU frequency  -  maximum CPU frequency  -  policy
CPU  0         2000 kHz (100 %)  -       2000 kHz (100 %)  -  powersave
Well... looks like a funny idea to run an Athlon64 at 2MHz, but it feels much faster... but what is really a problem: No matter what I "echo" into /proc/cpufreq, nothing changes.
A factor 1000 lost somewhere?

Yes.

Try this one. [Warning: extremely ugly hand-edited patch follows; taking akpm off Cc list, I do not want him to even see it]

Pavel

arch/i386/kernel/cpu/cpufreq/powernow-k8.c +26 ~~-13~~

@@ -8,6 +8,8 @@
  *
  *  Based on the powernow-k7.c module written by Dave Jones.
  *  (C) 2003 Dave Jones <[email protected]> on behalf of SuSE Labs
+ *  (C) 2004 Dominik Brodowski <[email protected]>
+ *  (C) 2004 Pavel Machek <[email protected]>
  *  Licensed under the terms of the GNU GPL License version 2.
  *  Based upon datasheets & sample CPUs kindly provided by AMD.
  *

@@ -34,10 +36,6 @@
 #define VERSION "version 1.00.08a"
 #include "powernow-k8.h"
 
-#ifdef CONFIG_PREEMPT
-#warning this driver has not been tested on a preempt system
-#endif
-
 static u32 vstable;	/* voltage stabalization time, from PSB, units 20 us */
 static u32 plllock;	/* pll lock time, from PSB, units 1 us */
 static u32 numps;	/* number of p-states, from PSB */

@@ -521,7 +519,8 @@
 		}
 		if ((pst[j].fid > MAX_FID)
 		    || (pst[j].fid & 1)
-		    || (pst[j].fid < HI_FID_TABLE_BOTTOM)){
+		    || (j && (pst[j].fid < HI_FID_TABLE_BOTTOM))) {
+			/* Only first fid is allowed to be in "low" range */
 			printk(KERN_ERR PFX "fid %d invalid : 0x%x\n", j, pst[j].fid);
 			return -EINVAL;
 		}

@@ -548,6 +547,7 @@
 	unsigned int i, j;
 	u32 mvs;
 	u8 maxvid;
+	int arima = 0;
 
 	for (i = 0xc0000; i < 0xffff0; i += 0x10) {
 		/* Scan BIOS looking for the signature. */

@@ -589,7 +589,7 @@
 		dprintk(KERN_DEBUG PFX "numpst: 0x%x\n", psb->numpst);
 		if (psb->numpst != 1) {
 			printk(KERN_ERR BFX "numpst must be 1\n");
-			return -ENODEV;
+			arima = 0;
 		}
 
 		dprintk(KERN_DEBUG PFX "cpuid: 0x%x\n", psb->cpuid);

@@ -601,7 +601,7 @@
 		printk("maxfid 0x%x (%d MHz), maxvid 0x%x\n", 
 		       psb->maxfid, find_freq_from_fid(psb->maxfid), maxvid);
 
-		numps = psb->numpstates;
+		numps = arima ? 3 : psb->numpstates;
 		if (numps < 2) {
 			printk(KERN_ERR BFX "no p states to transition\n");
 			return -ENODEV;

@@ -626,8 +626,10 @@
 		}
 
 		pst = (struct pst_s *) (psb + 1);
-		if (check_pst_table(pst, maxvid))
-			return -EINVAL;
+		if (check_pst_table(pst, maxvid)) {
+			if (!arima)
+				return -EINVAL;
+		}
 
 		powernow_table = kmalloc((sizeof(struct cpufreq_frequency_table) * (numps + 1)), GFP_KERNEL);
 		if (!powernow_table) {

@@ -635,13 +637,24 @@
 			return -ENOMEM;
 		}
 
-		for (j = 0; j < numps; j++) {
-			printk(KERN_INFO PFX "   %d : fid 0x%x (%d MHz), vid 0x%x\n", j,
-			       pst[j].fid, find_freq_from_fid(pst[j].fid), pst[j].vid);
+		for (j = 0; j < numps; j++) {
 			powernow_table[j].index = pst[j].fid; /* lower 8 bits */
 			powernow_table[j].index |= (pst[j].vid << 8); /* upper 8 bits */
-			powernow_table[j].frequency = find_freq_from_fid(pst[j].fid);
 		}
+
+		if (arima) {
+			powernow_table[1].index = 0x0608;
+			powernow_table[2].index = 0x020a;
+		}
+
+		for (j = 0; j < numps; j++) {
+			powernow_table[j].frequency = find_freq_from_fid(powernow_table[j].index & 0xff)*1000;
+			printk(KERN_INFO PFX "   %d : fid 0x%x (%d MHz), vid 0x%x\n", j,
+			       powernow_table[j].index & 0xff, 
+			       powernow_table[j].frequency/1000,
+			       powernow_table[j].index >> 8);
+		}
+
 		powernow_table[numps].frequency = CPUFREQ_TABLE_END;
 		powernow_table[numps].index = 0;

Signature · 2 lines

When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Pavel Machek

author

2004-02-28 13:15 UTC #4 lore raw

Hi!

Quoted 9 lines

Quoted 9 lines
(this looks similar to what the working version from 2.6.2 prints)
but...:
Quoted 1 line
cat /proc/cpufreq
          minimum CPU frequency  -  maximum CPU frequency  -  policy
CPU  0         2000 kHz (100 %)  -       2000 kHz (100 %)  -  powersave

Quoted 7 lines

Quoted 5 lines
Well... looks like a funny idea to run an Athlon64 at 2MHz, but it feels much faster... but what is really a problem: No matter what I "echo" into /proc/cpufreq, nothing changes.
A factor 1000 lost somewhere?
Yes.

Better patch follows. I do not have explanation why it is 2MHz - 2MHz. On my system I removed bug which prevented my system from being reported as 0.8MHz - 1.8MHz. (And modulo cosmetic uglyness, it worked).

This fixes cosmetic uglyness, adds proper copyrights, removes warning "untested on PREEMPT" (someone tested it, and makes it easier to override PST by user (eMachine users will need that one). Andrew, please apply.

Pavel

Index: arch/i386/kernel/cpu/cpufreq/powernow-k8.c
===================================================================
RCS file: /home/pavel/sf/bitbucket/bkcvs/linux-2.5/arch/i386/kernel/cpu/cpufreq/powernow-k8.c,v retrieving revision 1.6

i386/kernel/cpu/cpufreq/powernow-k8.c +15 -8

diff -u -u -r1.6 powernow-k8.c
@@ -8,6 +8,8 @@
  *
  *  Based on the powernow-k7.c module written by Dave Jones.
  *  (C) 2003 Dave Jones <[email protected]> on behalf of SuSE Labs
+ *  (C) 2004 Dominik Brodowski <[email protected]>
+ *  (C) 2004 Pavel Machek <[email protected]>
  *  Licensed under the terms of the GNU GPL License version 2.
  *  Based upon datasheets & sample CPUs kindly provided by AMD.
  *

@@ -34,10 +36,6 @@
 #define VERSION "version 1.00.08a"
 #include "powernow-k8.h"
 
-#ifdef CONFIG_PREEMPT
-#warning this driver has not been tested on a preempt system
-#endif
-
 static u32 vstable;	/* voltage stabalization time, from PSB, units 20 us */
 static u32 plllock;	/* pll lock time, from PSB, units 1 us */
 static u32 numps;	/* number of p-states, from PSB */

@@ -636,13 +636,22 @@
 			return -ENOMEM;
 		}
 
-		for (j = 0; j < numps; j++) {
-			printk(KERN_INFO PFX "   %d : fid 0x%x (%d MHz), vid 0x%x\n", j,
-			       pst[j].fid, find_freq_from_fid(pst[j].fid), pst[j].vid);
+		for (j = 0; j < psb->numpstates; j++) {
 			powernow_table[j].index = pst[j].fid; /* lower 8 bits */
 			powernow_table[j].index |= (pst[j].vid << 8); /* upper 8 bits */
-			powernow_table[j].frequency = find_freq_from_fid(pst[j].fid);
 		}
+
+		/* If you want to override your frequency tables, this
+		   is right place. */
+
+		for (j = 0; j < numps; j++) {
+			powernow_table[j].frequency = find_freq_from_fid(powernow_table[j].index & 0xff)*1000;
+			printk(KERN_INFO PFX "   %d : fid 0x%x (%d MHz), vid 0x%x\n", j,
+			       powernow_table[j].index & 0xff, 
+			       powernow_table[j].frequency/1000,
+			       powernow_table[j].index >> 8);
+		}
+
 		powernow_table[numps].frequency = CPUFREQ_TABLE_END;
 		powernow_table[numps].index = 0;

Signature · 2 lines

When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Lutz Vieweg

2004-03-01 10:15 UTC #5 lore raw

Quoted 6 lines · Pavel Machek

Quoted 3 lines
Quoted 1 line
A factor 1000 lost somewhere?
Yes.
Better patch follows.

Ah, I just realized I replied to the wrong mail - of course I just tried this "better patch" :-) and confirmed that it works with the K8V board.

Regards,

Lutz Vieweg

Quoted 53 lines

i386/kernel/cpu/cpufreq/powernow-k8.c +15 -8

@@ -8,6 +8,8 @@
  *
  *  Based on the powernow-k7.c module written by Dave Jones.
  *  (C) 2003 Dave Jones <[email protected]> on behalf of SuSE Labs
+ *  (C) 2004 Dominik Brodowski <[email protected]>
+ *  (C) 2004 Pavel Machek <[email protected]>
  *  Licensed under the terms of the GNU GPL License version 2.
  *  Based upon datasheets & sample CPUs kindly provided by AMD.
  *

@@ -34,10 +36,6 @@
 #define VERSION "version 1.00.08a"
 #include "powernow-k8.h"
 
-#ifdef CONFIG_PREEMPT
-#warning this driver has not been tested on a preempt system
-#endif
-
 static u32 vstable;	/* voltage stabalization time, from PSB, units 20 us */
 static u32 plllock;	/* pll lock time, from PSB, units 1 us */
 static u32 numps;	/* number of p-states, from PSB */

@@ -636,13 +636,22 @@
 			return -ENOMEM;
 		}
 
-		for (j = 0; j < numps; j++) {
-			printk(KERN_INFO PFX "   %d : fid 0x%x (%d MHz), vid 0x%x\n", j,
-			       pst[j].fid, find_freq_from_fid(pst[j].fid), pst[j].vid);
+		for (j = 0; j < psb->numpstates; j++) {
 			powernow_table[j].index = pst[j].fid; /* lower 8 bits */
 			powernow_table[j].index |= (pst[j].vid << 8); /* upper 8 bits */
-			powernow_table[j].frequency = find_freq_from_fid(pst[j].fid);
 		}
+
+		/* If you want to override your frequency tables, this
+		   is right place. */
+
+		for (j = 0; j < numps; j++) {
+			powernow_table[j].frequency = find_freq_from_fid(powernow_table[j].index & 0xff)*1000;
+			printk(KERN_INFO PFX "   %d : fid 0x%x (%d MHz), vid 0x%x\n", j,
+			       powernow_table[j].index & 0xff, 
+			       powernow_table[j].frequency/1000,
+			       powernow_table[j].index >> 8);
+		}
+
 		powernow_table[numps].frequency = CPUFREQ_TABLE_END;
 		powernow_table[numps].index = 0;

Signature · 4 lines

 Dipl. Phys. Lutz Vieweg   | email: [email protected]
 IS.Teledata AG            | Phone/Fax: +49-69-505030 -120/-505
 Sandweg 94                | http://www.isg.de/people/lkv/
 60316 Frankfurt am Main   | ^^^ PGP key available here ^^^

Lutz Vieweg

2004-03-01 10:12 UTC #6 lore raw

In reply to Pavel Machek

Quoted 6 lines · Pavel Machek

Quoted 1 line
A factor 1000 lost somewhere?
Yes.
Try this one. [Warning: extremely ugly hand-edited patch follows; taking akpm off Cc list, I do not want him to even see it]

This second patch (applied addtionally to the first one) fixes the cpufreq behaviour for the K8V board - thanks!

Regards,

Lutz Vieweg

Quoted 103 lines

arch/i386/kernel/cpu/cpufreq/powernow-k8.c +26 ~~-13~~

@@ -8,6 +8,8 @@
  *
  *  Based on the powernow-k7.c module written by Dave Jones.
  *  (C) 2003 Dave Jones <[email protected]> on behalf of SuSE Labs
+ *  (C) 2004 Dominik Brodowski <[email protected]>
+ *  (C) 2004 Pavel Machek <[email protected]>
  *  Licensed under the terms of the GNU GPL License version 2.
  *  Based upon datasheets & sample CPUs kindly provided by AMD.
  *

@@ -34,10 +36,6 @@
 #define VERSION "version 1.00.08a"
 #include "powernow-k8.h"
 
-#ifdef CONFIG_PREEMPT
-#warning this driver has not been tested on a preempt system
-#endif
-
 static u32 vstable;	/* voltage stabalization time, from PSB, units 20 us */
 static u32 plllock;	/* pll lock time, from PSB, units 1 us */
 static u32 numps;	/* number of p-states, from PSB */

@@ -521,7 +519,8 @@
 		}
 		if ((pst[j].fid > MAX_FID)
 		    || (pst[j].fid & 1)
-		    || (pst[j].fid < HI_FID_TABLE_BOTTOM)){
+		    || (j && (pst[j].fid < HI_FID_TABLE_BOTTOM))) {
+			/* Only first fid is allowed to be in "low" range */
 			printk(KERN_ERR PFX "fid %d invalid : 0x%x\n", j, pst[j].fid);
 			return -EINVAL;
 		}

@@ -548,6 +547,7 @@
 	unsigned int i, j;
 	u32 mvs;
 	u8 maxvid;
+	int arima = 0;
 
 	for (i = 0xc0000; i < 0xffff0; i += 0x10) {
 		/* Scan BIOS looking for the signature. */

@@ -589,7 +589,7 @@
 		dprintk(KERN_DEBUG PFX "numpst: 0x%x\n", psb->numpst);
 		if (psb->numpst != 1) {
 			printk(KERN_ERR BFX "numpst must be 1\n");
-			return -ENODEV;
+			arima = 0;
 		}
 
 		dprintk(KERN_DEBUG PFX "cpuid: 0x%x\n", psb->cpuid);

@@ -601,7 +601,7 @@
 		printk("maxfid 0x%x (%d MHz), maxvid 0x%x\n", 
 		       psb->maxfid, find_freq_from_fid(psb->maxfid), maxvid);
 
-		numps = psb->numpstates;
+		numps = arima ? 3 : psb->numpstates;
 		if (numps < 2) {
 			printk(KERN_ERR BFX "no p states to transition\n");
 			return -ENODEV;

@@ -626,8 +626,10 @@
 		}
 
 		pst = (struct pst_s *) (psb + 1);
-		if (check_pst_table(pst, maxvid))
-			return -EINVAL;
+		if (check_pst_table(pst, maxvid)) {
+			if (!arima)
+				return -EINVAL;
+		}
 
 		powernow_table = kmalloc((sizeof(struct cpufreq_frequency_table) * (numps + 1)), GFP_KERNEL);
 		if (!powernow_table) {

@@ -635,13 +637,24 @@
 			return -ENOMEM;
 		}
 
-		for (j = 0; j < numps; j++) {
-			printk(KERN_INFO PFX "   %d : fid 0x%x (%d MHz), vid 0x%x\n", j,
-			       pst[j].fid, find_freq_from_fid(pst[j].fid), pst[j].vid);
+		for (j = 0; j < numps; j++) {
 			powernow_table[j].index = pst[j].fid; /* lower 8 bits */
 			powernow_table[j].index |= (pst[j].vid << 8); /* upper 8 bits */
-			powernow_table[j].frequency = find_freq_from_fid(pst[j].fid);
 		}
+
+		if (arima) {
+			powernow_table[1].index = 0x0608;
+			powernow_table[2].index = 0x020a;
+		}
+
+		for (j = 0; j < numps; j++) {
+			powernow_table[j].frequency = find_freq_from_fid(powernow_table[j].index & 0xff)*1000;
+			printk(KERN_INFO PFX "   %d : fid 0x%x (%d MHz), vid 0x%x\n", j,
+			       powernow_table[j].index & 0xff, 
+			       powernow_table[j].frequency/1000,
+			       powernow_table[j].index >> 8);
+		}
+
 		powernow_table[numps].frequency = CPUFREQ_TABLE_END;
 		powernow_table[numps].index = 0;