Skip to contentSkip to conversation

powernow-k8-acpi driver

PATCH Pavel Machek 6 participants 15 messages Mar 3, 2004 – Mar 7, 2004

lore
Share & tools
lore

Pavel Machek

author
#0 lore raw

Hi!

Lots of machines have broken PST tables, so current in-kernel driver refuses to works on them. Vendors do get ACPI tables right because apparently Windows use them ;-). So this driver tends to work.

Comments? Could we get this into mainline?

Pavel

arch/i386/kernel/cpu/cpufreq/Kconfig renamed from linux/arch/i386/kernel/cpu/cpufreq/Kconfig +13 -0
@@ -93,6 +93,19 @@
 	depends on CPU_FREQ && EXPERIMENTAL
 	help
 	  This adds the CPUFreq driver for mobile AMD Opteron/Athlon64 processors.
+	  It relies on old "PST" tables. Unfortunately, many BIOSes get this table
+	  wrong. 
+
+	  For details, take a look at linux/Documentation/cpu-freq. 
+
+	  If in doubt, say N.
+
+config X86_POWERNOW_K8_ACPI
+	tristate "AMD Opteron/Athlon64 PowerNow! using ACPI"
+	depends on CPU_FREQ && EXPERIMENTAL
+	help
+	  This adds the CPUFreq driver for mobile AMD Opteron/Athlon64 processors.
+	  It relies on ACPI.
 
 	  For details, take a look at linux/Documentation/cpu-freq.
arch/i386/kernel/cpu/cpufreq/Makefile renamed from linux/arch/i386/kernel/cpu/cpufreq/Makefile +1 -0
@@ -1,6 +1,7 @@
 obj-$(CONFIG_X86_POWERNOW_K6)	+= powernow-k6.o
 obj-$(CONFIG_X86_POWERNOW_K7)	+= powernow-k7.o
 obj-$(CONFIG_X86_POWERNOW_K8)	+= powernow-k8.o
+obj-$(CONFIG_X86_POWERNOW_K8_ACPI)	+= powernow-k8-acpi.o
 obj-$(CONFIG_X86_LONGHAUL)	+= longhaul.o
 obj-$(CONFIG_X86_P4_CLOCKMOD)	+= p4-clockmod.o
 obj-$(CONFIG_ELAN_CPUFREQ)	+= elanfreq.o
arch/i386/kernel/cpu/cpufreq/powernow-k8-acpi.c renamed from linux/arch/i386/kernel/cpu/cpufreq/powernow-k8-acpi.c +1184 -0

1,184 lines not on this page · raw message

arch/i386/kernel/cpu/cpufreq/powernow-k8-acpi.h renamed from linux/arch/i386/kernel/cpu/cpufreq/powernow-k8-acpi.h +143 -0

143 lines not on this page · raw message

Signature · 2 lines
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]
#1 lore raw
Quoted 7 lines · Pavel Machek

Hi!

Lots of machines have broken PST tables, so current in-kernel driver refuses to works on them. Vendors do get ACPI tables right because apparently Windows use them ;-). So this driver tends to work.

Comments? Could we get this into mainline?

I really dislike the idea of having >1 driver for this. Why can't we have a "use_acpi" module_param to switch to this ?

Dave

#2 lore raw
Quoted 11 lines · Dave Jones
Quoted 7 lines · Pavel Machek

Hi!

Lots of machines have broken PST tables, so current in-kernel driver refuses to works on them. Vendors do get ACPI tables right because apparently Windows use them ;-). So this driver tends to work.

Comments? Could we get this into mainline?

I really dislike the idea of having >1 driver for this. Why can't we have a "use_acpi" module_param to switch to this ?

Well, that would probably not even link on kernel without ACPI...

We could make that functionality depend on CONFIG_ACPI, and allow runtime selection only if its defined... But those two drivers are pretty different just now and acpi-dependend chunk is pretty big. (It does funny stuff like polling for AC plug removal if we are in high-power state and battery would not handle that. Old driver simply refused to use high-power states on such machines.)

Pavel

Signature · 2 lines
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]
#3 lore raw
Quoted 6 lines · Pavel Machek

We could make that functionality depend on CONFIG_ACPI, and allow runtime selection only if its defined... But those two drivers are pretty different just now and acpi-dependend chunk is pretty big. (It does funny stuff like polling for AC plug removal if we are in high-power state and battery would not handle that. Old driver simply refused to use high-power states on such machines.)

you're aware of Dominik/Bruno's work on the 'acpilib'[1] stuff in this area right ? We'll need that anyway for Powernow-k7 and maybe longhaul too and its senseless duplicating this code.

One thing is bugging me though. Whats wrong with the ACPI P-state cpufreq driver ? Does that not work these days ? It's been a long time since I even looked at it.

Dave

#4 lore raw

Hi!

Quoted 10 lines
Quoted 6 lines

We could make that functionality depend on CONFIG_ACPI, and allow runtime selection only if its defined... But those two drivers are pretty different just now and acpi-dependend chunk is pretty big. (It does funny stuff like polling for AC plug removal if we are in high-power state and battery would not handle that. Old driver simply refused to use high-power states on such machines.)

you're aware of Dominik/Bruno's work on the 'acpilib'[1] stuff in this area right ? We'll need that anyway for Powernow-k7 and maybe longhaul too and its senseless duplicating this code.

That [1] looks like promise of url, but I don't see that url.

Quoted 3 lines

One thing is bugging me though. Whats wrong with the ACPI P-state cpufreq driver ? Does that not work these days ? It's been a long time since I even looked at it.

No idea.
Pavel

Signature · 2 lines
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]
#5 lore raw
Quoted 9 lines · Pavel Machek

Hi!

Quoted 7 lines
Quoted 6 lines

We could make that functionality depend on CONFIG_ACPI, and allow runtime selection only if its defined... But those two drivers are pretty different just now and acpi-dependend chunk is pretty big. (It does funny stuff like polling for AC plug removal if we are in high-power state and battery would not handle that. Old driver simply refused to use high-power states on such machines.)

Quoted 5 lines
Quoted 3 lines

you're aware of Dominik/Bruno's work on the 'acpilib'[1] stuff in this area right ? We'll need that anyway for Powernow-k7 and maybe longhaul too and its senseless duplicating this code.

That [1] looks like promise of url, but I don't see that url.

Hmm, cpufreq mailing list archives are your best bet. What I meant to add was..

[1] acpilib is a made up name I just came up with, I've no idea what the guys who wrote it are referring to it as.

Dave

#6 lore raw

Hi!

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines

We could make that functionality depend on CONFIG_ACPI, and allow runtime selection only if its defined... But those two drivers are pretty different just now and acpi-dependend chunk is pretty big. (It does funny stuff like polling for AC plug removal if we are in high-power state and battery would not handle that. Old driver simply refused to use high-power states on such machines.)

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

you're aware of Dominik/Bruno's work on the 'acpilib'[1] stuff in this area right ? We'll need that anyway for Powernow-k7 and maybe longhaul too and its senseless duplicating this code.

That [1] looks like promise of url, but I don't see that url.

Hmm, cpufreq mailing list archives are your best bet. What I meant to add was..

Ahha. Unfortunately, cpufreq mailing lists are only available to list subscribers. Ouch.

Pavel

Signature · 2 lines
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]
#7 lore raw
Quoted 2 lines · Pavel Machek

Ahha. Unfortunately, cpufreq mailing lists are only available to list subscribers. Ouch.

So... subscribe to the list - if you don't actually want to receive mail, you can turn on an option which says that.

Signature · 4 lines
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core
#8 lore raw
In reply to Pavel Machek
Quoted 10 lines · Pavel Machek

Hi!

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 6 lines

We could make that functionality depend on CONFIG_ACPI, and allow runtime selection only if its defined... But those two drivers are pretty different just now and acpi-dependend chunk is pretty big. (It does funny stuff like polling for AC plug removal if we are in high-power state and battery would not handle that. Old driver simply refused to use high-power states on such machines.)

you're aware of Dominik/Bruno's work on the 'acpilib'[1] stuff in this

Ahem, that's more Dominik work, not mine.

Quoted 11 lines
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

area right ? We'll need that anyway for Powernow-k7 and maybe longhaul too and its senseless duplicating this code.

That [1] looks like promise of url, but I don't see that url.

Hmm, cpufreq mailing list archives are your best bet. What I meant to add was..

Ahha. Unfortunately, cpufreq mailing lists are only available to list subscribers. Ouch.

Yes. But that a pretty low traffic mailing list anyway, and if really you have to touch to cpufreq, even only for a single driver, you should subscribe so that you know already of changes of cpufreq core which may impact your driver.

For example, you may have a trouble with the new 'sticky bit' feature with powernow-k8, especially if you want ACPI perflib included in your driver.

Signature · 4 lines
Bruno Ducrot

--  Which is worse:  ignorance or apathy?
--  Don't know.  Don't care.
#9 lore raw
Quoted 8 lines · Bruno Ducrot
Quoted 2 lines · Pavel Machek

Ahha. Unfortunately, cpufreq mailing lists are only available to list subscribers. Ouch.

Yes. But that a pretty low traffic mailing list anyway, and if really you have to touch to cpufreq, even only for a single driver, you should subscribe so that you know already of changes of cpufreq core which may impact your driver.

FWIW, the archives are also available via http://gmane.org, in the gmane.linux.kernel.cpufreq group.

Cheers,
Colin

#10 lore raw
In reply to Dave Jones

Hi!

Quoted 7 lines
Quoted 6 lines

We could make that functionality depend on CONFIG_ACPI, and allow runtime selection only if its defined... But those two drivers are pretty different just now and acpi-dependend chunk is pretty big. (It does funny stuff like polling for AC plug removal if we are in high-power state and battery would not handle that. Old driver simply refused to use high-power states on such machines.)

Quoted 7 lines

you're aware of Dominik/Bruno's work on the 'acpilib'[1] stuff in this area right ? We'll need that anyway for Powernow-k7 and maybe longhaul too and its senseless duplicating this code.

One thing is bugging me though. Whats wrong with the ACPI P-state cpufreq driver ? Does that not work these days ? It's been a long time since I even looked at it.

Paul, could you apply this? It fixes some typo (Dave's address was wrong), and makes header file closer to current version. No code changes.

Pavel

arch/i386/kernel/cpu/cpufreq/powernow-k8-acpi.c renamed from linux/arch/i386/kernel/cpu/cpufreq/powernow-k8-acpi.c +1 -1
@@ -11,7 +11,7 @@
  *  Support : [email protected]
  *
  *  Based on the powernow-k7.c module written by Dave Jones.
- *  (c) 2003 Dave Jones <[email protected]> on behalf of SuSE Labs
+ *  (c) 2003 Dave Jones <[email protected]> on behalf of SuSE Labs
  *  Licensed under the terms of the GNU GPL License version 2.
  *  Based upon datasheets & sample CPUs kindly provided by AMD.
  *
arch/i386/kernel/cpu/cpufreq/powernow-k8-acpi.h renamed from linux/arch/i386/kernel/cpu/cpufreq/powernow-k8-acpi.h +7 -6
@@ -28,7 +28,7 @@
 #define MSR_C_HI_STP_GNT_BENIGN   1
 
 /* status MSR - low part */
-#define MSR_S_LO_CHANGE_PENDING   0x80000000 /* cleared when completed */
+#define MSR_S_LO_CHANGE_PENDING   0x80000000   /* cleared when completed */
 #define MSR_S_LO_MAX_RAMP_VID     0x1f000000
 #define MSR_S_LO_MAX_FID          0x003f0000
 #define MSR_S_LO_START_FID        0x00003f00

@@ -54,12 +54,13 @@
 #define MIN_FREQ 800
 #define MAX_FREQ 5000
 
-#define INVALID_FID_MASK 0xc1
-#define INVALID_VID_MASK 0xe0
+#define INVALID_FID_MASK 0xffffffc1  /* not a valid fid if these bits are set */
+#define INVALID_VID_MASK 0xffffffe0  /* not a valid vid if these bits are set */
+
 
 #define STOP_GRANT_5NS    1 /* min memory access latency for voltage change   */
 #define MAXIMUM_VID_STEPS 1 /* Current cpus only allow a single step of 25mV  */
-#define VST_UNITS_20US   20 /* Voltage Stabalization Time is in units of 20us */
+#define VST_UNITS_20US   20 /* Voltage Stabilization Time is in units of 20us */
 
 #define PLL_LOCK_CONVERSION (1000/5) /* ms to ns, then divide by clock period */
 

@@ -117,7 +118,7 @@
 #define VID_MASK     0x1f
 #define FID_MASK     0x3f
 
-#define POW_AC  0  /* The power suppy states we care about - mains, battery,  */
+#define POW_AC  0  /* The power supply states we care about - mains, battery, */
 #define POW_BAT 1  /* or unknown, which presumably means that there is no     */
 #define POW_UNK 2  /* acpi support for the psr object, so there is no battery.*/
 

@@ -126,7 +127,7 @@
 #define POLLER_UNLOAD      2  /* are on mains power, at a high frequency, and */
 #define POLLER_DEAD        3  /* if there are battery restrictions.           */
 
-#define PFX "powernow-k8: "
+#define PFX "powernow-k8-acpi: "
 #define DFX KERN_DEBUG PFX
 #define IFX KERN_INFO  PFX
 #define EFX KERN_ERR   PFX
Signature · 2 lines
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]
#11 lore raw
In reply to Dave Jones

Hi!

Quoted 7 lines
Quoted 6 lines

We could make that functionality depend on CONFIG_ACPI, and allow runtime selection only if its defined... But those two drivers are pretty different just now and acpi-dependend chunk is pretty big. (It does funny stuff like polling for AC plug removal if we are in high-power state and battery would not handle that. Old driver simply refused to use high-power states on such machines.)

Quoted 7 lines

you're aware of Dominik/Bruno's work on the 'acpilib'[1] stuff in this area right ? We'll need that anyway for Powernow-k7 and maybe longhaul too and its senseless duplicating this code.

One thing is bugging me though. Whats wrong with the ACPI P-state cpufreq driver ? Does that not work these days ? It's been a long time since I even looked at it.

Dave, could you apply these? That are cleanups from Paul's new version of driver (killed few unused defines, right names for MSR's (hopefully!), more linux-like comments). No code changes.
Pavel

arch/i386/kernel/cpu/cpufreq/powernow-k8.c renamed from linux/arch/i386/kernel/cpu/cpufreq/powernow-k8.c +6 -6
@@ -7,7 +7,7 @@
  *  Support : [email protected]
  *
  *  Based on the powernow-k7.c module written by Dave Jones.
- *  (C) 2003 Dave Jones <[email protected]> on behalf of SuSE Labs
+ *  (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.

@@ -33,7 +33,7 @@
 
 #define PFX "powernow-k8: "
 #define BFX PFX "BIOS error: "
-#define VERSION "version 1.00.08a"
+#define VERSION "version 1.00.08b"
 #include "powernow-k8.h"
 
 static u32 vstable;	/* voltage stabalization time, from PSB, units 20 us */

@@ -97,7 +97,7 @@
 {
 	u32 lo, hi;
 
-	rdmsr(MSR_FIDVID_STATUS, lo, hi);
+	rdmsr(MSR_FIDVID_STAT, lo, hi);
 	return lo & MSR_S_LO_CHANGE_PENDING ? 1 : 0;
 }
 

@@ -117,7 +117,7 @@
 			printk(KERN_ERR PFX "detected change pending stuck\n");
 			return 1;
 		}
-		rdmsr(MSR_FIDVID_STATUS, lo, hi);
+		rdmsr(MSR_FIDVID_STAT, lo, hi);
 	}
 
 	currvid = hi & MSR_S_HI_CURRENT_VID;

@@ -154,7 +154,7 @@
 		return 1;
 	}
 
-	lo = fid | (currvid << MSR_C_LO_VID_SHIFT) | MSR_C_LO_INIT_FID_VID;
+	lo = fid | (currvid << MSR_C_LO_VID_SHIFT) | MSR_C_LO_INIT;
 
 	dprintk(KERN_DEBUG PFX "writing fid %x, lo %x, hi %x\n",
 		fid, lo, plllock * PLL_LOCK_CONVERSION);

@@ -195,7 +195,7 @@
 		return 1;
 	}
 
-	lo = currfid | (vid << MSR_C_LO_VID_SHIFT) | MSR_C_LO_INIT_FID_VID;
+	lo = currfid | (vid << MSR_C_LO_VID_SHIFT) | MSR_C_LO_INIT;
 
 	dprintk(KERN_DEBUG PFX "writing vid %x, lo %x, hi %x\n",
 		vid, lo, STOP_GRANT_5NS);
arch/i386/kernel/cpu/cpufreq/powernow-k8.h renamed from linux/arch/i386/kernel/cpu/cpufreq/powernow-k8.h +24 -35

111 lines not on this page · raw message

Signature · 2 lines
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]
#12 lore raw
Quoted 3 lines · Pavel Machek

Dave, could you apply these? That are cleanups from Paul's new version of driver (killed few unused defines, right names for MSR's (hopefully!), more linux-like comments). No code changes.

Looks trivial enough. I just bounced it forward to Linus directly. (I'm travelling this week, and bitkeeper on a laptop is unfunny)

Quoted 7 lines

- * (c) 2003 Advanced Micro Devices, Inc.
+ * (c) 2003, 2004 Advanced Micro Devices, Inc.

  *  Your use of this code is subject to the terms and conditions of the
- *  GNU general public license version 2. See "../../../COPYING" or
+ *  GNU general public license version 2. See "../../../../../COPYING" or
  *  http://www.gnu.org/licenses/gpl.html

*/

This bit seems really silly though, but thats just my opinion 8-) I'd just kill the ../'s completely.

Dave

#13 lore raw

Hi!

Quoted 6 lines
Quoted 3 lines

Dave, could you apply these? That are cleanups from Paul's new version of driver (killed few unused defines, right names for MSR's (hopefully!), more linux-like comments). No code changes.

Looks trivial enough. I just bounced it forward to Linus directly. (I'm travelling this week, and bitkeeper on a laptop is unfunny)

:-)

Quoted 10 lines
Quoted 7 lines

- * (c) 2003 Advanced Micro Devices, Inc.
+ * (c) 2003, 2004 Advanced Micro Devices, Inc.

  *  Your use of this code is subject to the terms and conditions of the
- *  GNU general public license version 2. See "../../../COPYING" or
+ *  GNU general public license version 2. See "../../../../../COPYING" or
  *  http://www.gnu.org/licenses/gpl.html

*/

This bit seems really silly though, but thats just my opinion 8-) I'd just kill the ../'s completely.

Agreed, I assume AMD lawyers wanted that... If not, perhaps we can make it disappear.
Pavel

Signature · 2 lines
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]
#14 lore raw
In reply to Dave Jones

Hi!

Quoted 7 lines
Quoted 6 lines

We could make that functionality depend on CONFIG_ACPI, and allow runtime selection only if its defined... But those two drivers are pretty different just now and acpi-dependend chunk is pretty big. (It does funny stuff like polling for AC plug removal if we are in high-power state and battery would not handle that. Old driver simply refused to use high-power states on such machines.)

Quoted 7 lines

you're aware of Dominik/Bruno's work on the 'acpilib'[1] stuff in this area right ? We'll need that anyway for Powernow-k7 and maybe longhaul too and its senseless duplicating this code.

One thing is bugging me though. Whats wrong with the ACPI P-state cpufreq driver ? Does that not work these days ? It's been a long time since I even looked at it.

One more thing: is there any reason for "use-array-as-struct"?

static int query_current_values_with_pending_wait(u8 *perproc) {
...

        perproc[PP_OFF_CVID] = hi & MSR_S_HI_CURRENT_VID;
        perproc[PP_OFF_CFID] = lo & MSR_S_LO_CURRENT_FID;
}

having

struct cpu_power {
	int numps, share, cvid, cfid;
	char pstates[0];
}

should do the trick...
Pavel

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