Can't load speedstep-centrino on IBM x336?

Can't load speedstep-centrino on IBM x336?

[Darrick J. Wong]

[-- Attachment #1.1.1: Type: text/plain, Size: 2328 bytes --]

Hi all,

I've been trying to get Enhanced SpeedStep working on an IBM x336
(3.6GHz Xeon).  If I load the speedstep-centrino driver with cpufreq
debugging turned on, I get a message about "Invalid control/status
registers (1 - 1)" and the module refuses to load.  It seems that the
stumbling point is this chunk of code in centrino_cpu_init_acpi():

if ((p.control_register.space_id != ACPI_ADR_SPACE_FIXED_HARDWARE) ||
    (p.status_register.space_id != ACPI_ADR_SPACE_FIXED_HARDWARE)) {
        dprintk("Invalid control/status registers (%x - %x)\n",
               p.control_register.space_id, p.status_register.space_id);
        result = -EIO;
        goto err_unreg;
}

I decompiled the DSDT code that the BIOS supplies, and it seems that the
_PCT method returns control and status registers in SystemIO space if
\SMIM is set (it is set to 0x1 earlier):


Method (_PCT, 0, NotSerialized)
{
    If (\SMIM)
    {
        Return (Package (0x02)
        {
            ResourceTemplate ()
            {
                Register (SystemIO, 0x10, 0x00, 0x0000000000000800)
            },
            ResourceTemplate ()
            {
                Register (SystemIO, 0x10, 0x00, 0x0000000000000804)
            }
        })
    }
    Return (Package (0x02)
    {
        ResourceTemplate ()
        {
            Register (FFixedHW, 0x40, 0x00, 0x0000000000000199)
        },
        ResourceTemplate ()
        {
            Register (FFixedHW, 0x10, 0x00, 0x0000000000000198)
        }
    })
}

I checked with the ACPI specs v2.0c and v3.0, and neither of them seem
to restrict the register address space to "fixed hardware".  Linux,
however, expects FIXED_HARDWARE, not SYSTEM_IO, causing the error
space_id checking to trip.  However, EST seems to work just fine on the
x336 without this check.  I built a kernel without this code and was
able to switch frequencies and governors in a loop without any adverse
effects.  /proc/cpuinfo was getting updated, too.

So now I'm wondering three things: Is there a reason why there is this
address space check?  Can we get rid of it?  And, am I supposed to be
using acpi_cpufreq instead?  The speedstep_centrino code seems to
indicate that my CPU revision (0xF41) should be supported by this driver...

(Attached is a patch against 2.6.14-rc4 to get rid of the check.)

--Darrick

[-- Attachment #1.1.2: x336-centrino.patch --]
[-- Type: text/x-patch, Size: 807 bytes --]

diff -Naur b/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c a/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c
--- b/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c	2005-10-24 18:24:06.000000000 -0700
+++ a/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c	2005-10-24 18:28:22.000000000 -0700
@@ -392,14 +392,6 @@
 		goto err_unreg;
 	}
 
-	if ((p.control_register.space_id != ACPI_ADR_SPACE_FIXED_HARDWARE) ||
-	    (p.status_register.space_id != ACPI_ADR_SPACE_FIXED_HARDWARE)) {
-		dprintk("Invalid control/status registers (%x - %x)\n",
-			p.control_register.space_id, p.status_register.space_id);
-		result = -EIO;
-		goto err_unreg;
-	}
-
 	for (i=0; i<p.state_count; i++) {
 		if (p.states[i].control != p.states[i].status) {
 			dprintk("Different control (%llu) and status values (%llu)\n",

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 147 bytes --]

_______________________________________________
Cpufreq mailing list
Cpufreq@lists.linux.org.uk
http://lists.linux.org.uk/mailman/listinfo/cpufreq

RE: Can't load speedstep-centrino on IBM x336?

[Pallipadi, Venkatesh]

It is just that speedstep-centrino driver can only handle
FIXED_HARDWARE. For SYSTEM_IO you should be using acpi-cpufreq driver.
Please try that driver and things should work fine.

Thanks,
Venki 

>-----Original Message-----
>From: cpufreq-bounces@lists.linux.org.uk 
>[mailto:cpufreq-bounces@lists.linux.org.uk] On Behalf Of 
>Darrick J. Wong
>Sent: Monday, October 24, 2005 6:48 PM
>To: Dave Jones
>Cc: cpufreq@lists.linux.org.uk; Chris McDermott
>Subject: Can't load speedstep-centrino on IBM x336?
>
>Hi all,
>
>I've been trying to get Enhanced SpeedStep working on an IBM x336
>(3.6GHz Xeon).  If I load the speedstep-centrino driver with cpufreq
>debugging turned on, I get a message about "Invalid control/status
>registers (1 - 1)" and the module refuses to load.  It seems that the
>stumbling point is this chunk of code in centrino_cpu_init_acpi():
>
>if ((p.control_register.space_id != ACPI_ADR_SPACE_FIXED_HARDWARE) ||
>    (p.status_register.space_id != ACPI_ADR_SPACE_FIXED_HARDWARE)) {
>        dprintk("Invalid control/status registers (%x - %x)\n",
>               p.control_register.space_id, 
>p.status_register.space_id);
>        result = -EIO;
>        goto err_unreg;
>}
>
>I decompiled the DSDT code that the BIOS supplies, and it 
>seems that the
>_PCT method returns control and status registers in SystemIO space if
>\SMIM is set (it is set to 0x1 earlier):
>
>
>Method (_PCT, 0, NotSerialized)
>{
>    If (\SMIM)
>    {
>        Return (Package (0x02)
>        {
>            ResourceTemplate ()
>            {
>                Register (SystemIO, 0x10, 0x00, 0x0000000000000800)
>            },
>            ResourceTemplate ()
>            {
>                Register (SystemIO, 0x10, 0x00, 0x0000000000000804)
>            }
>        })
>    }
>    Return (Package (0x02)
>    {
>        ResourceTemplate ()
>        {
>            Register (FFixedHW, 0x40, 0x00, 0x0000000000000199)
>        },
>        ResourceTemplate ()
>        {
>            Register (FFixedHW, 0x10, 0x00, 0x0000000000000198)
>        }
>    })
>}
>
>I checked with the ACPI specs v2.0c and v3.0, and neither of them seem
>to restrict the register address space to "fixed hardware".  Linux,
>however, expects FIXED_HARDWARE, not SYSTEM_IO, causing the error
>space_id checking to trip.  However, EST seems to work just fine on the
>x336 without this check.  I built a kernel without this code and was
>able to switch frequencies and governors in a loop without any adverse
>effects.  /proc/cpuinfo was getting updated, too.
>
>So now I'm wondering three things: Is there a reason why there is this
>address space check?  Can we get rid of it?  And, am I supposed to be
>using acpi_cpufreq instead?  The speedstep_centrino code seems to
>indicate that my CPU revision (0xF41) should be supported by 
>this driver...
>
>(Attached is a patch against 2.6.14-rc4 to get rid of the check.)
>
>--Darrick
>

Re: Can't load speedstep-centrino on IBM x336?

[Darrick J. Wong]

[-- Attachment #1.1: Type: text/plain, Size: 3413 bytes --]

Ok, I tried acpi-cpufreq and tried to change the frequency from 3.6GHz
to 2.8.  scaling_cur_freq says I'm at 2800MHz, but /proc/cpuinfo still
says 3600.  In dmesg, I see "Writing 0x00000e1e to port 0x0800" at the
very end of the log, but I don't see "Looking for 0x00000e1e from port
0x0804" like I think I should.  I don't see "Invalid port width..."
either--it's as if we fell out of the function.

--D

Pallipadi, Venkatesh wrote:
> It is just that speedstep-centrino driver can only handle
> FIXED_HARDWARE. For SYSTEM_IO you should be using acpi-cpufreq driver.
> Please try that driver and things should work fine.
> 
> Thanks,
> Venki 
> 
> 
>>-----Original Message-----
>>From: cpufreq-bounces@lists.linux.org.uk 
>>[mailto:cpufreq-bounces@lists.linux.org.uk] On Behalf Of 
>>Darrick J. Wong
>>Sent: Monday, October 24, 2005 6:48 PM
>>To: Dave Jones
>>Cc: cpufreq@lists.linux.org.uk; Chris McDermott
>>Subject: Can't load speedstep-centrino on IBM x336?
>>
>>Hi all,
>>
>>I've been trying to get Enhanced SpeedStep working on an IBM x336
>>(3.6GHz Xeon).  If I load the speedstep-centrino driver with cpufreq
>>debugging turned on, I get a message about "Invalid control/status
>>registers (1 - 1)" and the module refuses to load.  It seems that the
>>stumbling point is this chunk of code in centrino_cpu_init_acpi():
>>
>>if ((p.control_register.space_id != ACPI_ADR_SPACE_FIXED_HARDWARE) ||
>>   (p.status_register.space_id != ACPI_ADR_SPACE_FIXED_HARDWARE)) {
>>       dprintk("Invalid control/status registers (%x - %x)\n",
>>              p.control_register.space_id, 
>>p.status_register.space_id);
>>       result = -EIO;
>>       goto err_unreg;
>>}
>>
>>I decompiled the DSDT code that the BIOS supplies, and it 
>>seems that the
>>_PCT method returns control and status registers in SystemIO space if
>>\SMIM is set (it is set to 0x1 earlier):
>>
>>
>>Method (_PCT, 0, NotSerialized)
>>{
>>   If (\SMIM)
>>   {
>>       Return (Package (0x02)
>>       {
>>           ResourceTemplate ()
>>           {
>>               Register (SystemIO, 0x10, 0x00, 0x0000000000000800)
>>           },
>>           ResourceTemplate ()
>>           {
>>               Register (SystemIO, 0x10, 0x00, 0x0000000000000804)
>>           }
>>       })
>>   }
>>   Return (Package (0x02)
>>   {
>>       ResourceTemplate ()
>>       {
>>           Register (FFixedHW, 0x40, 0x00, 0x0000000000000199)
>>       },
>>       ResourceTemplate ()
>>       {
>>           Register (FFixedHW, 0x10, 0x00, 0x0000000000000198)
>>       }
>>   })
>>}
>>
>>I checked with the ACPI specs v2.0c and v3.0, and neither of them seem
>>to restrict the register address space to "fixed hardware".  Linux,
>>however, expects FIXED_HARDWARE, not SYSTEM_IO, causing the error
>>space_id checking to trip.  However, EST seems to work just fine on the
>>x336 without this check.  I built a kernel without this code and was
>>able to switch frequencies and governors in a loop without any adverse
>>effects.  /proc/cpuinfo was getting updated, too.
>>
>>So now I'm wondering three things: Is there a reason why there is this
>>address space check?  Can we get rid of it?  And, am I supposed to be
>>using acpi_cpufreq instead?  The speedstep_centrino code seems to
>>indicate that my CPU revision (0xF41) should be supported by 
>>this driver...
>>
>>(Attached is a patch against 2.6.14-rc4 to get rid of the check.)
>>
>>--Darrick
>>
> 
> 

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]

[-- Attachment #2: Type: text/plain, Size: 147 bytes --]

_______________________________________________
Cpufreq mailing list
Cpufreq@lists.linux.org.uk
http://lists.linux.org.uk/mailman/listinfo/cpufreq

Re: Can't load speedstep-centrino on IBM x336?

[Venkatesh Pallipadi]

On Mon, Oct 24, 2005 at 11:18:19PM -0700, Darrick J. Wong wrote:
> Ok, I tried acpi-cpufreq and tried to change the frequency from 3.6GHz
> to 2.8.  scaling_cur_freq says I'm at 2800MHz, but /proc/cpuinfo still
> says 3600.  In dmesg, I see "Writing 0x00000e1e to port 0x0800" at the
> very end of the log, but I don't see "Looking for 0x00000e1e from port
> 0x0804" like I think I should.  I don't see "Invalid port width..."
> either--it's as if we fell out of the function.
> 

If scaling_cur_freq shows you the right freq, then things should be working.
We recently removed "Looking for * from port" in the common path.

What /proc/cpuinfo shows kind of depends on other things. I had this patch 
in my queue that fixes things with /proc/cpuinfo. This should help here..

Thanks,
Venki


What is the value shown in "cpu MHz" of /proc/cpuinfo when CPUs are capable of 
changing frequency?

Today the answer is: It depends.
On i386:
SMP kernel - It is always the boot frequency
UP kernel - Scales with the frequency change and shows that was last set.

On x86_64:
There is one single variable cpu_khz that gets written by all the CPUs. So,
the frequency set by last CPU will be seen on /proc/cpuinfo of all the
CPUs in the system.

On ia64:
It is always boot time frequency of a particular CPU that gets displayed.


The patch below changes this to:
Show the last known frequency of the particular CPU, when cpufreq is present. If
cpu doesnot support changing of frequency through cpufreq, then boot frequency 
will be shown. The patch affects i386, x86_64 and ia64 architectures.

Signed-off-by: Venkatesh Pallipadi<venkatesh.pallipadi@intel.com>

Index: linux-2.6.12/arch/i386/kernel/cpu/proc.c
===================================================================
--- linux-2.6.12.orig/arch/i386/kernel/cpu/proc.c	2005-08-30 11:10:46.000000000 -0700
+++ linux-2.6.12/arch/i386/kernel/cpu/proc.c	2005-10-07 15:39:48.000000000 -0700
@@ -3,6 +3,7 @@
 #include <linux/string.h>
 #include <asm/semaphore.h>
 #include <linux/seq_file.h>
+#include <linux/cpufreq.h>
 
 /*
  *	Get CPU information for use by the procfs.
@@ -86,8 +87,11 @@
 		seq_printf(m, "stepping\t: unknown\n");
 
 	if ( cpu_has(c, X86_FEATURE_TSC) ) {
+		unsigned int freq = cpufreq_quick_get(n);
+		if (!freq)
+			freq = cpu_khz;
 		seq_printf(m, "cpu MHz\t\t: %u.%03u\n",
-			cpu_khz / 1000, (cpu_khz % 1000));
+			freq / 1000, (freq % 1000));
 	}
 
 	/* Cache size */
Index: linux-2.6.12/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-2.6.12.orig/drivers/cpufreq/cpufreq.c	2005-09-26 14:59:23.000000000 -0700
+++ linux-2.6.12/drivers/cpufreq/cpufreq.c	2005-10-07 15:46:08.000000000 -0700
@@ -830,6 +830,30 @@
 
 
 /** 
+ * cpufreq_quick_get - get the CPU frequency (in kHz) frpm policy->cur
+ * @cpu: CPU number
+ *
+ * This is the last known freq, without actually getting it from the driver.
+ * Return value will be same as what is shown in scaling_cur_freq in sysfs.
+ */
+unsigned int cpufreq_quick_get(unsigned int cpu)
+{
+	struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
+	unsigned int ret = 0;
+
+	if (policy) {
+		down(&policy->lock);
+		ret = policy->cur;
+		up(&policy->lock);
+		cpufreq_cpu_put(policy);
+	}
+
+	return (ret);
+}
+EXPORT_SYMBOL(cpufreq_quick_get);
+
+
+/** 
  * cpufreq_get - get the current CPU frequency (in kHz)
  * @cpu: CPU number
  *
Index: linux-2.6.12/arch/x86_64/kernel/setup.c
===================================================================
--- linux-2.6.12.orig/arch/x86_64/kernel/setup.c	2005-08-31 14:46:39.000000000 -0700
+++ linux-2.6.12/arch/x86_64/kernel/setup.c	2005-10-07 15:40:24.000000000 -0700
@@ -42,6 +42,7 @@
 #include <linux/edd.h>
 #include <linux/mmzone.h>
 #include <linux/kexec.h>
+#include <linux/cpufreq.h>
 
 #include <asm/mtrr.h>
 #include <asm/uaccess.h>
@@ -1187,8 +1188,11 @@
 		seq_printf(m, "stepping\t: unknown\n");
 	
 	if (cpu_has(c,X86_FEATURE_TSC)) {
+		unsigned int freq = cpufreq_quick_get(cpu);
+		if (!freq)
+			freq = cpu_khz;
 		seq_printf(m, "cpu MHz\t\t: %u.%03u\n",
-			     cpu_khz / 1000, (cpu_khz % 1000));
+			     freq / 1000, (freq % 1000));
 	}
 
 	/* Cache size */
Index: linux-2.6.12/arch/ia64/kernel/setup.c
===================================================================
--- linux-2.6.12.orig/arch/ia64/kernel/setup.c	2005-08-31 14:46:39.000000000 -0700
+++ linux-2.6.12/arch/ia64/kernel/setup.c	2005-10-07 15:41:38.000000000 -0700
@@ -43,6 +43,7 @@
 #include <linux/initrd.h>
 #include <linux/platform.h>
 #include <linux/pm.h>
+#include <linux/cpufreq.h>
 
 #include <asm/ia32.h>
 #include <asm/machvec.h>
@@ -474,6 +475,7 @@
 	char family[32], features[128], *cp, sep;
 	struct cpuinfo_ia64 *c = v;
 	unsigned long mask;
+	unsigned int proc_freq;
 	int i;
 
 	mask = c->features;
@@ -506,6 +508,10 @@
 		sprintf(cp, " 0x%lx", mask);
 	}
 
+	proc_freq = cpufreq_quick_get(cpunum);
+	if (!proc_freq)
+		proc_freq = c->proc_freq / 1000;
+
 	seq_printf(m,
 		   "processor  : %d\n"
 		   "vendor     : %s\n"
@@ -522,7 +528,7 @@
 		   "BogoMIPS   : %lu.%02lu\n",
 		   cpunum, c->vendor, family, c->model, c->revision, c->archrev,
 		   features, c->ppn, c->number,
-		   c->proc_freq / 1000000, c->proc_freq % 1000000,
+		   proc_freq / 1000, proc_freq % 1000,
 		   c->itc_freq / 1000000, c->itc_freq % 1000000,
 		   lpj*HZ/500000, (lpj*HZ/5000) % 100);
 #ifdef CONFIG_SMP
Index: linux-2.6.12/include/linux/cpufreq.h
===================================================================
--- linux-2.6.12.orig/include/linux/cpufreq.h	2005-09-26 14:59:25.000000000 -0700
+++ linux-2.6.12/include/linux/cpufreq.h	2005-10-07 14:19:05.000000000 -0700
@@ -259,6 +259,16 @@
 /* query the current CPU frequency (in kHz). If zero, cpufreq couldn't detect it */
 unsigned int cpufreq_get(unsigned int cpu);
 
+/* query the last known CPU freq (in kHz). If zero, cpufreq couldn't detect it */
+#ifdef CONFIG_CPU_FREQ
+unsigned int cpufreq_quick_get(unsigned int cpu);
+#else
+unsigned int cpufreq_quick_get(unsigned int cpu)
+{
+	return 0;
+}
+#endif
+
 
 /*********************************************************************
  *                       CPUFREQ DEFAULT GOVERNOR                    *

Re: Can't load speedstep-centrino on IBM x336?

[Darrick J. Wong]

[-- Attachment #1.1: Type: text/plain, Size: 1217 bytes --]

Hmm... ok then, mainline is working.  Thanks for the data!

Unfortunately, RHEL4 U2's "acpi" (it's based on 2.6.9) module doesn't
work--it prints the "Looking..." message and then "Transition failed."
when it doesn't find the value it's looking for (0x0E1E) and gets 0xFFFF
instead.  Do you have any suggestions?  (If you're not familiar with
RHEL4, that's fine too.)

--D

Venkatesh Pallipadi wrote:
> On Mon, Oct 24, 2005 at 11:18:19PM -0700, Darrick J. Wong wrote:
> 
>>Ok, I tried acpi-cpufreq and tried to change the frequency from 3.6GHz
>>to 2.8.  scaling_cur_freq says I'm at 2800MHz, but /proc/cpuinfo still
>>says 3600.  In dmesg, I see "Writing 0x00000e1e to port 0x0800" at the
>>very end of the log, but I don't see "Looking for 0x00000e1e from port
>>0x0804" like I think I should.  I don't see "Invalid port width..."
>>either--it's as if we fell out of the function.
>>
> 
> 
> If scaling_cur_freq shows you the right freq, then things should be working.
> We recently removed "Looking for * from port" in the common path.
> 
> What /proc/cpuinfo shows kind of depends on other things. I had this patch 
> in my queue that fixes things with /proc/cpuinfo. This should help here..
> 
> Thanks,
> Venki

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 147 bytes --]

_______________________________________________
Cpufreq mailing list
Cpufreq@lists.linux.org.uk
http://lists.linux.org.uk/mailman/listinfo/cpufreq

RE: Can't load speedstep-centrino on IBM x336?

[Pallipadi, Venkatesh]

 

>-----Original Message-----
>From: Darrick J. Wong [mailto:djwong@us.ibm.com] 
>
>Hmm... ok then, mainline is working.  Thanks for the data!
>
>Unfortunately, RHEL4 U2's "acpi" (it's based on 2.6.9) module doesn't
>work--it prints the "Looking..." message and then "Transition failed."
>when it doesn't find the value it's looking for (0x0E1E) and 
>gets 0xFFFF

When using SYSTEM_IO, The acpi driver does the IO call with proper
parameters to change the frequency. BIOS then comes in and does the
actual P-state transition in SMM mode (after the cordination across
logical CPUs). After the transition, BIOS is supposed to export the
result in another IO port, which OS tries to read and verify. This
verification was removed in the latest base kernel, but is present in
2.6.9 (and RHEL4).

So, the problem here may be:
1) BIOS is not doing the transition. Hence when OS tries to verify by
readon the status IO port, it sees the transition has failed.
2) BIOS is doing the transition, but not reflecting the status in status
IO port or reporting some error there. Hence again OS fails to see the
change.
3) Some other bug in the OS driver.

Having seen various bugs in different BIOSes related to this, and I am
kind of confident that this also ia a bug in BIOS and not a OS issue.
Can you please check with the BIOS folks on how exactly they are doing
these transitions in SMM. You can also let me know if you need any more
details about the driver itself.

Thanks,
Venki 

>instead.  Do you have any suggestions?  (If you're not familiar with
>RHEL4, that's fine too.)
>
>--D
>
>Venkatesh Pallipadi wrote:
>> On Mon, Oct 24, 2005 at 11:18:19PM -0700, Darrick J. Wong wrote:
>> 
>>>Ok, I tried acpi-cpufreq and tried to change the frequency 
>from 3.6GHz
>>>to 2.8.  scaling_cur_freq says I'm at 2800MHz, but 
>/proc/cpuinfo still
>>>says 3600.  In dmesg, I see "Writing 0x00000e1e to port 
>0x0800" at the
>>>very end of the log, but I don't see "Looking for 0x00000e1e 
>from port
>>>0x0804" like I think I should.  I don't see "Invalid port width..."
>>>either--it's as if we fell out of the function.
>>>
>> 
>> 
>> If scaling_cur_freq shows you the right freq, then things 
>should be working.
>> We recently removed "Looking for * from port" in the common path.
>> 
>> What /proc/cpuinfo shows kind of depends on other things. I 
>had this patch 
>> in my queue that fixes things with /proc/cpuinfo. This 
>should help here..
>> 
>> Thanks,
>> Venki
>

Re: Can't load speedstep-centrino on IBM x336?

[Darrick J. Wong]

[-- Attachment #1.1: Type: text/plain, Size: 3670 bytes --]

Venkatesh,

It turns out that RHEL4 wasn't calling the SMM to tell it that the OS
would handle the p-state transitions.  Thus, the status register trap
never got set up, which is why I kept reading 0xFFFF.  RHEL4 also
obliterates the value of PSTATE_CNT if the FADT revision is 1.0, so I
sent RH a backport of mainline's cpufreq, which only obliterates
PSTATE_CNT if acpi=strict and also makes that call to SMM.  With that
patch, DBS works great.

However, I am curious about the removal of the status register check in
acpi_cpufreq--how slow is reading that status register?  It seems to me
that it's a rather good idea to check that the operation actually
succeeded, instead of assuming that it does and changing the reported
clock speed as if it did, because any failures that happen will be
silent; we could only tell if the switch succeeded for sure by running
CPU benchmarks.  But, perhaps there's a justification for removing it
that I simply don't know about?

Anyway, thanks for your help with debugging this issue!

--Darrick

Pallipadi, Venkatesh wrote:
>  
> 
> 
>>-----Original Message-----
>>From: Darrick J. Wong [mailto:djwong@us.ibm.com] 
>>
>>Hmm... ok then, mainline is working.  Thanks for the data!
>>
>>Unfortunately, RHEL4 U2's "acpi" (it's based on 2.6.9) module doesn't
>>work--it prints the "Looking..." message and then "Transition failed."
>>when it doesn't find the value it's looking for (0x0E1E) and 
>>gets 0xFFFF
> 
> 
> When using SYSTEM_IO, The acpi driver does the IO call with proper
> parameters to change the frequency. BIOS then comes in and does the
> actual P-state transition in SMM mode (after the cordination across
> logical CPUs). After the transition, BIOS is supposed to export the
> result in another IO port, which OS tries to read and verify. This
> verification was removed in the latest base kernel, but is present in
> 2.6.9 (and RHEL4).
> 
> So, the problem here may be:
> 1) BIOS is not doing the transition. Hence when OS tries to verify by
> readon the status IO port, it sees the transition has failed.
> 2) BIOS is doing the transition, but not reflecting the status in status
> IO port or reporting some error there. Hence again OS fails to see the
> change.
> 3) Some other bug in the OS driver.
> 
> Having seen various bugs in different BIOSes related to this, and I am
> kind of confident that this also ia a bug in BIOS and not a OS issue.
> Can you please check with the BIOS folks on how exactly they are doing
> these transitions in SMM. You can also let me know if you need any more
> details about the driver itself.
> 
> Thanks,
> Venki 
> 
> 
>>instead.  Do you have any suggestions?  (If you're not familiar with
>>RHEL4, that's fine too.)
>>
>>--D
>>
>>Venkatesh Pallipadi wrote:
>>
>>>On Mon, Oct 24, 2005 at 11:18:19PM -0700, Darrick J. Wong wrote:
>>>
>>>
>>>>Ok, I tried acpi-cpufreq and tried to change the frequency 
> 
>>from 3.6GHz
> 
>>>>to 2.8.  scaling_cur_freq says I'm at 2800MHz, but 
>>
>>/proc/cpuinfo still
>>
>>>>says 3600.  In dmesg, I see "Writing 0x00000e1e to port 
>>
>>0x0800" at the
>>
>>>>very end of the log, but I don't see "Looking for 0x00000e1e 
> 
>>from port
> 
>>>>0x0804" like I think I should.  I don't see "Invalid port width..."
>>>>either--it's as if we fell out of the function.
>>>>
>>>
>>>
>>>If scaling_cur_freq shows you the right freq, then things 
>>
>>should be working.
>>
>>>We recently removed "Looking for * from port" in the common path.
>>>
>>>What /proc/cpuinfo shows kind of depends on other things. I 
>>
>>had this patch 
>>
>>>in my queue that fixes things with /proc/cpuinfo. This 
>>
>>should help here..
>>
>>>Thanks,
>>>Venki
>>
> 

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 147 bytes --]

_______________________________________________
Cpufreq mailing list
Cpufreq@lists.linux.org.uk
http://lists.linux.org.uk/mailman/listinfo/cpufreq

RE: Can't load speedstep-centrino on IBM x336?

[Pallipadi, Venkatesh]

 

>-----Original Message-----
>From: Darrick J. Wong [mailto:djwong@us.ibm.com] 
>Sent: Thursday, October 27, 2005 5:54 PM
>To: Pallipadi, Venkatesh
>Cc: Dave Jones; cpufreq@lists.linux.org.uk; Chris McDermott
>Subject: Re: Can't load speedstep-centrino on IBM x336?
>
>Venkatesh,
>
>It turns out that RHEL4 wasn't calling the SMM to tell it that the OS
>would handle the p-state transitions.  Thus, the status register trap
>never got set up, which is why I kept reading 0xFFFF.  RHEL4 also
>obliterates the value of PSTATE_CNT if the FADT revision is 1.0, so I
>sent RH a backport of mainline's cpufreq, which only obliterates
>PSTATE_CNT if acpi=strict and also makes that call to SMM.  With that
>patch, DBS works great.
>

Ok. That's good news. Can you share that patch here? Thanks.

>However, I am curious about the removal of the status register check in
>acpi_cpufreq--how slow is reading that status register?  

It is really slow due to SMM. 100+ mS. One one system I measured it as
500 mS.

> It seems to me
>that it's a rather good idea to check that the operation actually
>succeeded, instead of assuming that it does and changing the reported
>clock speed as if it did, because any failures that happen will be
>silent; we could only tell if the switch succeeded for sure by running
>CPU benchmarks.  But, perhaps there's a justification for removing it
>that I simply don't know about?

It is mostly needed for enabling new platforms. But, once thinsg are
working, it really is not required to be tested all the time. It doubles
the time for a P-state transition.

Thanks,
Venki

Re: Can't load speedstep-centrino on IBM x336?

[Darrick J. Wong]

[-- Attachment #1.1.1: Type: text/plain, Size: 1825 bytes --]

Attached are patches against RHEL4 U2 and SLES9 SP2.

100-500ms is pretty bad; I guess it's worth the check removal.

--D

Pallipadi, Venkatesh wrote:
>  
> 
> 
>>-----Original Message-----
>>From: Darrick J. Wong [mailto:djwong@us.ibm.com] 
>>Sent: Thursday, October 27, 2005 5:54 PM
>>To: Pallipadi, Venkatesh
>>Cc: Dave Jones; cpufreq@lists.linux.org.uk; Chris McDermott
>>Subject: Re: Can't load speedstep-centrino on IBM x336?
>>
>>Venkatesh,
>>
>>It turns out that RHEL4 wasn't calling the SMM to tell it that the OS
>>would handle the p-state transitions.  Thus, the status register trap
>>never got set up, which is why I kept reading 0xFFFF.  RHEL4 also
>>obliterates the value of PSTATE_CNT if the FADT revision is 1.0, so I
>>sent RH a backport of mainline's cpufreq, which only obliterates
>>PSTATE_CNT if acpi=strict and also makes that call to SMM.  With that
>>patch, DBS works great.
>>
> 
> 
> Ok. That's good news. Can you share that patch here? Thanks.
> 
> 
>>However, I am curious about the removal of the status register check in
>>acpi_cpufreq--how slow is reading that status register?  
> 
> 
> It is really slow due to SMM. 100+ mS. One one system I measured it as
> 500 mS.
> 
> 
>>It seems to me
>>that it's a rather good idea to check that the operation actually
>>succeeded, instead of assuming that it does and changing the reported
>>clock speed as if it did, because any failures that happen will be
>>silent; we could only tell if the switch succeeded for sure by running
>>CPU benchmarks.  But, perhaps there's a justification for removing it
>>that I simply don't know about?
> 
> 
> It is mostly needed for enabling new platforms. But, once thinsg are
> working, it really is not required to be tested all the time. It doubles
> the time for a P-state transition.
> 
> Thanks,
> Venki
> 

[-- Attachment #1.1.2: dbs-rhel4-x336_2.patch --]
[-- Type: text/x-patch, Size: 10652 bytes --]

diff -Naur orig/arch/i386/kernel/cpu/cpufreq/acpi.c linux-2.6.9/arch/i386/kernel/cpu/cpufreq/acpi.c
--- orig/arch/i386/kernel/cpu/cpufreq/acpi.c	2005-10-27 13:26:30.000000000 -0700
+++ linux-2.6.9/arch/i386/kernel/cpu/cpufreq/acpi.c	2005-10-28 15:02:07.000000000 -0700
@@ -62,6 +62,8 @@
 
 static struct cpufreq_driver acpi_cpufreq_driver;
 
+static unsigned int acpi_pstate_strict;
+
 static int
 acpi_processor_write_port(
 	u16	port,
@@ -166,30 +168,40 @@
 	}
 
 	/*
-	 * Then we read the 'status_register' and compare the value with the
-	 * target state's 'status' to make sure the transition was successful.
-	 * Note that we'll poll for up to 1ms (100 cycles of 10us) before
-	 * giving up.
+	 * Assume the write went through when acpi_pstate_strict is not used.
+	 * As read status_register is an expensive operation and there
+	 * are no specific error cases where an IO port write will fail.
 	 */
+	if (acpi_pstate_strict) {
+		/*
+		 * Then we read the 'status_register' and compare the value with the
+		 * target state's 'status' to make sure the transition was successful.
+		 * Note that we'll poll for up to 1ms (100 cycles of 10us) before
+		 * giving up.
+		 */
 
-	port = data->acpi_data.status_register.address;
-	bit_width = data->acpi_data.status_register.bit_width;
+		port = data->acpi_data.status_register.address;
+		bit_width = data->acpi_data.status_register.bit_width;
 
-	ACPI_DEBUG_PRINT((ACPI_DB_INFO, 
-		"Looking for 0x%08x from port 0x%04x\n",
-		(u32) data->acpi_data.states[state].status, port));
+		ACPI_DEBUG_PRINT((ACPI_DB_INFO, 
+			"Looking for 0x%08x from port 0x%04x\n",
+			(u32) data->acpi_data.states[state].status, port));
 
-	for (i=0; i<100; i++) {
-		ret = acpi_processor_read_port(port, bit_width, &value);
-		if (ret) {	
-			ACPI_DEBUG_PRINT((ACPI_DB_WARN,
-				"Invalid port width 0x%04x\n", bit_width));
-			retval = ret;
-			goto migrate_end;
+		for (i=0; i<100; i++) {
+			ret = acpi_processor_read_port(port, bit_width, &value);
+			if (ret) {	
+				ACPI_DEBUG_PRINT((ACPI_DB_WARN,
+					"Invalid port width 0x%04x\n", bit_width));
+				retval = ret;
+				goto migrate_end;
+			}
+			if (value == (u32) data->acpi_data.states[state].status)
+				break;
+			udelay(10);
 		}
-		if (value == (u32) data->acpi_data.states[state].status)
-			break;
-		udelay(10);
+	} else {
+		i = 0;
+		value = (u32) data->acpi_data.states[state].status;
 	}
 
 	/* notify cpufreq */
@@ -436,6 +448,8 @@
 		goto err_freqfree;
 	}
 		
+	/* notify BIOS that we exist */
+	acpi_processor_notify_smm(THIS_MODULE);
 
 	printk(KERN_INFO "cpufreq: CPU%u - ACPI performance management activated.\n",
 	       cpu);
@@ -520,6 +534,8 @@
 	return_VOID;
 }
 
+module_param(acpi_pstate_strict, uint, 0644);
+MODULE_PARM_DESC(acpi_pstate_strict, "value 0 or non-zero. non-zero -> strict ACPI checks are performed during frequency changes.");
 
 late_initcall(acpi_cpufreq_init);
 module_exit(acpi_cpufreq_exit);
diff -Naur orig/arch/i386/kernel/cpu/cpufreq/powernow-k7.c linux-2.6.9/arch/i386/kernel/cpu/cpufreq/powernow-k7.c
--- orig/arch/i386/kernel/cpu/cpufreq/powernow-k7.c	2004-10-18 14:53:06.000000000 -0700
+++ linux-2.6.9/arch/i386/kernel/cpu/cpufreq/powernow-k7.c	2005-10-27 13:32:24.000000000 -0700
@@ -400,6 +400,9 @@
 	powernow_table[i].frequency = CPUFREQ_TABLE_END;
 	powernow_table[i].index = 0;
 
+	/* notify BIOS that we exist */
+	acpi_processor_notify_smm(THIS_MODULE);
+
 	return 0;
 
 err2:
diff -Naur orig/arch/i386/kernel/cpu/cpufreq/powernow-k8.c linux-2.6.9/arch/i386/kernel/cpu/cpufreq/powernow-k8.c
--- orig/arch/i386/kernel/cpu/cpufreq/powernow-k8.c	2005-10-27 13:26:30.000000000 -0700
+++ linux-2.6.9/arch/i386/kernel/cpu/cpufreq/powernow-k8.c	2005-10-27 13:32:55.000000000 -0700
@@ -779,6 +779,10 @@
 	data->numps = data->acpi_data.state_count;
 	print_basics(data);
 	powernow_k8_acpi_pst_values(data, 0);
+
+	/* notify BIOS that we exist */
+	acpi_processor_notify_smm(THIS_MODULE);
+
 	return 0;
 err_out:
 	acpi_processor_unregister_performance(&data->acpi_data, data->cpu);
diff -Naur orig/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c linux-2.6.9/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c
--- orig/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c	2005-10-27 13:26:30.000000000 -0700
+++ linux-2.6.9/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c	2005-10-27 13:33:15.000000000 -0700
@@ -448,6 +448,9 @@
 			centrino_model[policy->cpu]->op_points[i].frequency = CPUFREQ_ENTRY_INVALID;
 	}
 
+	/* notify BIOS that we exist */
+	acpi_processor_notify_smm(THIS_MODULE);
+
 	return 0;
 
  err_kfree_all:
diff -Naur orig/drivers/acpi/processor.c linux-2.6.9/drivers/acpi/processor.c
--- orig/drivers/acpi/processor.c	2005-10-27 13:26:34.000000000 -0700
+++ linux-2.6.9/drivers/acpi/processor.c	2005-10-27 15:08:09.000000000 -0700
@@ -770,7 +770,10 @@
  * policy is adjusted accordingly.
  */
 
-static int acpi_processor_ppc_is_init = 0;
+#define PPC_REGISTERED   1
+#define PPC_IN_USE       2
+
+static int acpi_processor_ppc_status = 0;
 
 static int acpi_processor_ppc_notifier(struct notifier_block *nb, 
 	unsigned long event,
@@ -828,6 +831,10 @@
 	 * (e.g. 0 = states 0..n; 1 = states 1..n; etc.
 	 */
 	status = acpi_evaluate_integer(pr->handle, "_PPC", NULL, &ppc);
+
+	if (status != AE_NOT_FOUND)
+		acpi_processor_ppc_status |= PPC_IN_USE;
+
 	if(ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
 		ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "Error evaluating _PPC\n"));
 		return_VALUE(-ENODEV);
@@ -852,17 +859,17 @@
 
 static void acpi_processor_ppc_init(void) {
 	if (!cpufreq_register_notifier(&acpi_ppc_notifier_block, CPUFREQ_POLICY_NOTIFIER))
-		acpi_processor_ppc_is_init = 1;
+		acpi_processor_ppc_status |= PPC_REGISTERED;
 	else
 		printk(KERN_DEBUG "Warning: Processor Platform Limit not supported.\n");
 }
 
 
 static void acpi_processor_ppc_exit(void) {
-	if (acpi_processor_ppc_is_init)
+	if (acpi_processor_ppc_status & PPC_REGISTERED)
 		cpufreq_unregister_notifier(&acpi_ppc_notifier_block, CPUFREQ_POLICY_NOTIFIER);
 
-	acpi_processor_ppc_is_init = 0;
+	acpi_processor_ppc_status &= ~PPC_REGISTERED;
 }
 
 /*
@@ -1095,6 +1102,72 @@
 	return_VALUE(0);
 }
 
+int acpi_processor_notify_smm(struct module *calling_module) {
+	acpi_status		status;
+	static int		is_done = 0;
+
+	ACPI_FUNCTION_TRACE("acpi_processor_notify_smm");
+
+	if (!(acpi_processor_ppc_status & PPC_REGISTERED))
+		return_VALUE(-EBUSY);
+
+	if (!try_module_get(calling_module))
+		return_VALUE(-EINVAL);
+
+	/* is_done is set to negative if an error occured,
+	 * and to postitive if _no_ error occured, but SMM
+	 * was already notified. This avoids double notification
+	 * which might lead to unexpected results...
+	 */
+	if (is_done > 0) {
+		module_put(calling_module);
+		return_VALUE(0);
+	}
+	else if (is_done < 0) {
+		module_put(calling_module);
+		return_VALUE(is_done);
+	}
+
+	is_done = -EIO;
+
+	/* Can't write pstate_cnt to smi_cmd if either value is zero */
+	if ((!acpi_fadt.smi_cmd) ||
+	    (!acpi_fadt.pstate_cnt)) {
+		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
+			"No SMI port or pstate_cnt\n"));
+		module_put(calling_module);
+		return_VALUE(0);
+	}
+
+	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Writing pstate_cnt [0x%x] to smi_cmd [0x%x]\n", acpi_fadt.pstate_cnt, acpi_fadt.smi_cmd));
+
+	/* FADT v1 doesn't support pstate_cnt, many BIOS vendors use
+	 * it anyway, so we need to support it... */
+	if (acpi_fadt_is_v1) {
+		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Using v1.0 FADT reserved value for pstate_cnt\n"));
+	}
+
+	status = acpi_os_write_port (acpi_fadt.smi_cmd,
+				     (u32) acpi_fadt.pstate_cnt, 8);
+	if (ACPI_FAILURE (status)) {
+		ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
+				  "Failed to write pstate_cnt [0x%x] to "
+				  "smi_cmd [0x%x]\n", acpi_fadt.pstate_cnt, acpi_fadt.smi_cmd));
+		module_put(calling_module);
+		return_VALUE(status);
+	}
+
+	/* Success. If there's no _PPC, we need to fear nothing, so
+	 * we can allow the cpufreq driver to be rmmod'ed. */
+	is_done = 1;
+
+	if (!(acpi_processor_ppc_status & PPC_IN_USE))
+		module_put(calling_module);
+
+	return_VALUE(0);
+}
+EXPORT_SYMBOL(acpi_processor_notify_smm);
+
 
 #ifdef CONFIG_X86_ACPI_CPUFREQ_PROC_INTF
 /* /proc/acpi/processor/../performance interface (DEPRECATED) */
@@ -1252,7 +1325,7 @@
 
 	ACPI_FUNCTION_TRACE("acpi_processor_register_performance");
 
-	if (!acpi_processor_ppc_is_init)
+	if (!(acpi_processor_ppc_status & PPC_REGISTERED))
 		return_VALUE(-EINVAL);
 
 	down(&performance_sem);
@@ -1293,9 +1366,6 @@
 
 	ACPI_FUNCTION_TRACE("acpi_processor_unregister_performance");
 
-	if (!acpi_processor_ppc_is_init)
-		return_VOID;
-
 	down(&performance_sem);
 
 	pr = processors[cpu];
diff -Naur orig/drivers/acpi/tables/tbconvrt.c linux-2.6.9/drivers/acpi/tables/tbconvrt.c
--- orig/drivers/acpi/tables/tbconvrt.c	2005-10-27 13:26:33.000000000 -0700
+++ linux-2.6.9/drivers/acpi/tables/tbconvrt.c	2005-10-27 14:47:28.000000000 -0700
@@ -49,6 +49,8 @@
 #define _COMPONENT          ACPI_TABLES
 	 ACPI_MODULE_NAME    ("tbconvrt")
 
+u8 acpi_fadt_is_v1;
+EXPORT_SYMBOL(acpi_fadt_is_v1);
 
 /*******************************************************************************
  *
@@ -212,6 +214,7 @@
 
 	/* ACPI 1.0 FACS */
 	/* The BIOS stored FADT should agree with Revision 1.0 */
+	acpi_fadt_is_v1 = 1;
 
 	/*
 	 * Copy the table header and the common part of the tables.
@@ -242,7 +245,8 @@
 	 * the SMI_CMD register to assume processor performance state control
 	 * responsibility. There isn't any equivalence in 1.0, leave it zeroed.
 	 */
-	local_fadt->pstate_cnt = 0;
+	if (acpi_strict)
+		local_fadt->pstate_cnt = 0;
 
 	/*
 	 * Support for the _CST object and C States change notification.
diff -Naur orig/include/acpi/actbl.h linux-2.6.9/include/acpi/actbl.h
--- orig/include/acpi/actbl.h	2004-10-18 14:54:08.000000000 -0700
+++ linux-2.6.9/include/acpi/actbl.h	2005-10-27 14:06:07.000000000 -0700
@@ -330,6 +330,8 @@
 #include "actbl1.h"   /* Acpi 1.0 table definitions */
 #include "actbl2.h"   /* Acpi 2.0 table definitions */
 
+extern u8 acpi_fadt_is_v1; /* is set to 1 if FADT is revision 1,
+			    * needed for certain workarounds */
 
 #pragma pack(1)
 /*
diff -Naur orig/include/acpi/processor.h linux-2.6.9/include/acpi/processor.h
--- orig/include/acpi/processor.h	2004-10-18 14:53:51.000000000 -0700
+++ linux-2.6.9/include/acpi/processor.h	2005-10-27 13:36:27.000000000 -0700
@@ -140,4 +140,8 @@
 	struct acpi_processor_performance * performance,
 	unsigned int cpu);
 
+/* note: this locks both the calling module and the processor module
+         if a _PPC object exists, rmmod is disallowed then */
+int acpi_processor_notify_smm(struct module *calling_module);
+
 #endif

[-- Attachment #1.1.3: dbs-sles9-x336_2.patch --]
[-- Type: text/x-patch, Size: 11241 bytes --]

diff -Naur orig/arch/i386/kernel/cpu/cpufreq/acpi.c linux-2.6.5/arch/i386/kernel/cpu/cpufreq/acpi.c
--- orig/arch/i386/kernel/cpu/cpufreq/acpi.c	2005-10-27 17:24:58.000000000 -0700
+++ linux-2.6.5/arch/i386/kernel/cpu/cpufreq/acpi.c	2005-10-28 15:28:46.667369840 -0700
@@ -59,6 +59,7 @@
 
 static struct cpufreq_acpi_io	*acpi_io_data[NR_CPUS];
 
+static unsigned int acpi_pstate_strict;
 
 static int
 acpi_processor_write_port(
@@ -170,30 +171,40 @@
 	}
 
 	/*
-	 * Then we read the 'status_register' and compare the value with the
-	 * target state's 'status' to make sure the transition was successful.
-	 * Note that we'll poll for up to 1ms (100 cycles of 10us) before
-	 * giving up.
+	 * Assume the write went through when acpi_pstate_strict is not used.
+	 * As read status_register is an expensive operation and there
+	 * are no specific error cases where an IO port write will fail.
 	 */
-
-	port = data->acpi_data.status_register.address;
-	bit_width = data->acpi_data.status_register.bit_width;
-
-	ACPI_DEBUG_PRINT((ACPI_DB_INFO, 
-		"Looking for 0x%08x from port 0x%04x\n",
-		(u32) data->acpi_data.states[state].status, port));
-
-	for (i=0; i<100; i++) {
-		ret = acpi_processor_read_port(port, bit_width, &value);
-		if (ret) {	
-			ACPI_DEBUG_PRINT((ACPI_DB_WARN,
-				"Invalid port width 0x%04x\n", bit_width));
-			retval = ret;
-			goto migrate_end;
+	if (acpi_pstate_strict) {
+		/*
+		 * Then we read the 'status_register' and compare the value with the
+		 * target state's 'status' to make sure the transition was successful.
+		 * Note that we'll poll for up to 1ms (100 cycles of 10us) before
+		 * giving up.
+		 */
+
+		port = data->acpi_data.status_register.address;
+		bit_width = data->acpi_data.status_register.bit_width;
+
+		ACPI_DEBUG_PRINT((ACPI_DB_INFO, 
+			"Looking for 0x%08x from port 0x%04x\n",
+			(u32) data->acpi_data.states[state].status, port));
+
+		for (i=0; i<100; i++) {
+			ret = acpi_processor_read_port(port, bit_width, &value);
+			if (ret) {	
+				ACPI_DEBUG_PRINT((ACPI_DB_WARN,
+					"Invalid port width 0x%04x\n", bit_width));
+				retval = ret;
+				goto migrate_end;
+			}
+			if (value == (u32) data->acpi_data.states[state].status)
+				break;
+			udelay(10);
 		}
-		if (value == (u32) data->acpi_data.states[state].status)
-			break;
-		udelay(10);
+	} else {
+		i = 0;
+		value = (u32) data->acpi_data.states[state].status;
 	}
 
 	/* notify cpufreq */
@@ -410,6 +421,9 @@
 	if (result) {
 		goto err_freqfree;
 	}
+
+	/* notify BIOS that we exist */
+	acpi_processor_notify_smm(THIS_MODULE);
 		
 
 	printk(KERN_INFO "cpufreq: CPU%u - ACPI performance management activated.\n",
@@ -491,6 +505,8 @@
 	return_VOID;
 }
 
+module_param(acpi_pstate_strict, uint, 0644);
+MODULE_PARM_DESC(acpi_pstate_strict, "value 0 or non-zero. non-zero -> strict ACPI checks are performed during frequency changes.");
 
 late_initcall(acpi_cpufreq_init);
 module_exit(acpi_cpufreq_exit);
diff -Naur orig/arch/i386/kernel/cpu/cpufreq/powernow-k7.c linux-2.6.5/arch/i386/kernel/cpu/cpufreq/powernow-k7.c
--- orig/arch/i386/kernel/cpu/cpufreq/powernow-k7.c	2005-10-27 17:24:42.000000000 -0700
+++ linux-2.6.5/arch/i386/kernel/cpu/cpufreq/powernow-k7.c	2005-10-27 17:33:39.000000000 -0700
@@ -179,6 +179,9 @@
 	powernow_table[number_scales].frequency = CPUFREQ_TABLE_END;
 	powernow_table[number_scales].index = 0;
 
+	/* notify BIOS that we exist */
+	acpi_processor_notify_smm(THIS_MODULE);
+
 	return 0;
 }
 
diff -Naur orig/arch/i386/kernel/cpu/cpufreq/powernow-k8.c linux-2.6.5/arch/i386/kernel/cpu/cpufreq/powernow-k8.c
--- orig/arch/i386/kernel/cpu/cpufreq/powernow-k8.c	2005-10-27 17:25:03.000000000 -0700
+++ linux-2.6.5/arch/i386/kernel/cpu/cpufreq/powernow-k8.c	2005-10-27 17:34:02.000000000 -0700
@@ -790,6 +790,10 @@
 	data->numps = data->acpi_data.state_count;
 	print_basics(data);
 	powernow_k8_acpi_pst_values(data, 0);
+
+	/* notify BIOS that we exist */
+	acpi_processor_notify_smm(THIS_MODULE);
+
 	return 0;
 err_out:
 	acpi_processor_unregister_performance(&data->acpi_data, data->cpu);
diff -Naur orig/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c linux-2.6.5/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c
--- orig/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c	2005-10-27 17:24:55.000000000 -0700
+++ linux-2.6.5/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c	2005-10-27 17:34:18.000000000 -0700
@@ -456,6 +456,9 @@
 			centrino_model[policy->cpu]->op_points[i].frequency = CPUFREQ_ENTRY_INVALID;
 	}
 
+	/* notify BIOS that we exist */
+	acpi_processor_notify_smm(THIS_MODULE);
+
 	return 0;
 
  err_kfree_all:
diff -Naur orig/drivers/acpi/osl.c linux-2.6.5/drivers/acpi/osl.c
--- orig/drivers/acpi/osl.c	2005-10-27 17:24:58.000000000 -0700
+++ linux-2.6.5/drivers/acpi/osl.c	2005-10-27 18:01:08.000000000 -0700
@@ -360,6 +360,7 @@
 	return AE_OK;
 }
 
+EXPORT_SYMBOL(acpi_os_write_port);
 acpi_status
 acpi_os_write_port(
 	acpi_io_address	port,
diff -Naur orig/drivers/acpi/processor.c linux-2.6.5/drivers/acpi/processor.c
--- orig/drivers/acpi/processor.c	2005-10-27 17:24:58.000000000 -0700
+++ linux-2.6.5/drivers/acpi/processor.c	2005-10-27 17:58:31.000000000 -0700
@@ -52,7 +52,7 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 #include <acpi/processor.h>
-
+#include <acpi/acpiosxf.h>
 
 #define ACPI_PROCESSOR_COMPONENT	0x01000000
 #define ACPI_PROCESSOR_CLASS		"processor"
@@ -771,7 +771,10 @@
  * policy is adjusted accordingly.
  */
 
-static int acpi_processor_ppc_is_init = 0;
+#define PPC_REGISTERED   1
+#define PPC_IN_USE       2
+
+static int acpi_processor_ppc_status = 0;
 
 static int acpi_processor_ppc_notifier(struct notifier_block *nb, 
 	unsigned long event,
@@ -829,6 +832,10 @@
 	 * (e.g. 0 = states 0..n; 1 = states 1..n; etc.
 	 */
 	status = acpi_evaluate_integer(pr->handle, "_PPC", NULL, &ppc);
+
+	if (status != AE_NOT_FOUND)
+		acpi_processor_ppc_status |= PPC_IN_USE;
+
 	if(ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
 		ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "Error evaluating _PPC\n"));
 		return_VALUE(-ENODEV);
@@ -853,17 +860,17 @@
 
 static void acpi_processor_ppc_init(void) {
 	if (!cpufreq_register_notifier(&acpi_ppc_notifier_block, CPUFREQ_POLICY_NOTIFIER))
-		acpi_processor_ppc_is_init = 1;
+		acpi_processor_ppc_status |= PPC_REGISTERED;
 	else
 		printk(KERN_DEBUG "Warning: Processor Platform Limit not supported.\n");
 }
 
 
 static void acpi_processor_ppc_exit(void) {
-	if (acpi_processor_ppc_is_init)
+	if (acpi_processor_ppc_status & PPC_REGISTERED)
 		cpufreq_unregister_notifier(&acpi_ppc_notifier_block, CPUFREQ_POLICY_NOTIFIER);
 
-	acpi_processor_ppc_is_init = 0;
+	acpi_processor_ppc_status &= ~PPC_REGISTERED;
 }
 
 /*
@@ -1088,6 +1095,72 @@
 	return_VALUE(0);
 }
 
+int acpi_processor_notify_smm(struct module *calling_module) {
+	acpi_status		status;
+	static int		is_done = 0;
+
+	ACPI_FUNCTION_TRACE("acpi_processor_notify_smm");
+
+	if (!(acpi_processor_ppc_status & PPC_REGISTERED))
+		return_VALUE(-EBUSY);
+
+	if (!try_module_get(calling_module))
+		return_VALUE(-EINVAL);
+
+	/* is_done is set to negative if an error occured,
+	 * and to postitive if _no_ error occured, but SMM
+	 * was already notified. This avoids double notification
+	 * which might lead to unexpected results...
+	 */
+	if (is_done > 0) {
+		module_put(calling_module);
+		return_VALUE(0);
+	}
+	else if (is_done < 0) {
+		module_put(calling_module);
+		return_VALUE(is_done);
+	}
+
+	is_done = -EIO;
+
+	/* Can't write pstate_cnt to smi_cmd if either value is zero */
+	if ((!acpi_fadt.smi_cmd) ||
+	    (!acpi_fadt.pstate_cnt)) {
+		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
+			"No SMI port or pstate_cnt\n"));
+		module_put(calling_module);
+		return_VALUE(0);
+	}
+
+	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Writing pstate_cnt [0x%x] to smi_cmd [0x%x]\n", acpi_fadt.pstate_cnt, acpi_fadt.smi_cmd));
+
+	/* FADT v1 doesn't support pstate_cnt, many BIOS vendors use
+	 * it anyway, so we need to support it... */
+	if (acpi_fadt_is_v1) {
+		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Using v1.0 FADT reserved value for pstate_cnt\n"));
+	}
+
+	status = acpi_os_write_port (acpi_fadt.smi_cmd,
+				     (u32) acpi_fadt.pstate_cnt, 8);
+	if (ACPI_FAILURE (status)) {
+		ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
+				  "Failed to write pstate_cnt [0x%x] to "
+				  "smi_cmd [0x%x]\n", acpi_fadt.pstate_cnt, acpi_fadt.smi_cmd));
+		module_put(calling_module);
+		return_VALUE(status);
+	}
+
+	/* Success. If there's no _PPC, we need to fear nothing, so
+	 * we can allow the cpufreq driver to be rmmod'ed. */
+	is_done = 1;
+
+	if (!(acpi_processor_ppc_status & PPC_IN_USE))
+		module_put(calling_module);
+
+	return_VALUE(0);
+}
+EXPORT_SYMBOL(acpi_processor_notify_smm);
+
 
 #ifdef CONFIG_X86_ACPI_CPUFREQ_PROC_INTF
 /* /proc/acpi/processor/../performance interface (DEPRECATED) */
@@ -1245,7 +1318,7 @@
 
 	ACPI_FUNCTION_TRACE("acpi_processor_register_performance");
 
-	if (!acpi_processor_ppc_is_init)
+	if (!(acpi_processor_ppc_status & PPC_REGISTERED))
 		return_VALUE(-EINVAL);
 
 	down(&performance_sem);
@@ -1286,9 +1359,6 @@
 
 	ACPI_FUNCTION_TRACE("acpi_processor_unregister_performance");
 
-	if (!acpi_processor_ppc_is_init)
-		return_VOID;
-
 	down(&performance_sem);
 
 	pr = processors[cpu];
diff -Naur orig/drivers/acpi/tables/tbconvrt.c linux-2.6.5/drivers/acpi/tables/tbconvrt.c
--- orig/drivers/acpi/tables/tbconvrt.c	2005-10-27 17:25:03.000000000 -0700
+++ linux-2.6.5/drivers/acpi/tables/tbconvrt.c	2005-10-27 17:38:30.000000000 -0700
@@ -49,6 +49,8 @@
 #define _COMPONENT          ACPI_TABLES
 	 ACPI_MODULE_NAME    ("tbconvrt")
 
+u8 acpi_fadt_is_v1;
+EXPORT_SYMBOL(acpi_fadt_is_v1);
 
 /*******************************************************************************
  *
@@ -212,6 +214,7 @@
 
 	/* ACPI 1.0 FACS */
 	/* The BIOS stored FADT should agree with Revision 1.0 */
+	acpi_fadt_is_v1 = 1;
 
 	/*
 	 * Copy the table header and the common part of the tables.
@@ -242,7 +245,8 @@
 	 * the SMI_CMD register to assume processor performance state control
 	 * responsibility. There isn't any equivalence in 1.0, leave it zeroed.
 	 */
-	local_fadt->pstate_cnt = 0;
+	if (acpi_strict)
+		local_fadt->pstate_cnt = 0;
 
 	/*
 	 * Support for the _CST object and C States change notification.
diff -Naur orig/include/acpi/actbl.h linux-2.6.5/include/acpi/actbl.h
--- orig/include/acpi/actbl.h	2004-04-03 19:37:07.000000000 -0800
+++ linux-2.6.5/include/acpi/actbl.h	2005-10-27 17:38:51.000000000 -0700
@@ -343,5 +343,7 @@
 #include "actbl1.h"   /* Acpi 1.0 table definitions */
 #include "actbl2.h"   /* Acpi 2.0 table definitions */
 
+extern u8 acpi_fadt_is_v1; /* is set to 1 if FADT is revision 1,
+			    * needed for certain workarounds */
 
 #endif /* __ACTBL_H__ */
diff -Naur orig/include/acpi/processor.h linux-2.6.5/include/acpi/processor.h
--- orig/include/acpi/processor.h	2004-04-03 19:36:57.000000000 -0800
+++ linux-2.6.5/include/acpi/processor.h	2005-10-27 17:39:05.000000000 -0700
@@ -140,4 +140,8 @@
 	struct acpi_processor_performance * performance,
 	unsigned int cpu);
 
+/* note: this locks both the calling module and the processor module
+         if a _PPC object exists, rmmod is disallowed then */
+int acpi_processor_notify_smm(struct module *calling_module);
+
 #endif

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]

[-- Attachment #2: Type: text/plain, Size: 147 bytes --]

_______________________________________________
Cpufreq mailing list
Cpufreq@lists.linux.org.uk
http://lists.linux.org.uk/mailman/listinfo/cpufreq