[RFC][PATCH] notify BIOS of cpufreq support (pstate_cnt)

[RFC][PATCH] notify BIOS of cpufreq support (pstate_cnt)

[Dominik Brodowski]

[Follow-up to http://marc.theaimsgroup.com/?l=acpi4linux&m=107684287525298&w=2 
which was the last try of getting this right last February]

Notify SMM of cpufreq

Often, the BIOS changes the CPU frequency if a notebook is (dis)connected
from its AC adapter. This interferes with a) an user setting the CPU frequency
and b) kernel timing code which needs to know loops_per_jiffy, cpu_khz etc.

So, ACPI 2.0 offers a method to disable such BIOS changes -- writing
a value contained in the FADT (pstate_cnt) to the smi_cmd port. This patch
utilizes this method by doing this write in processor.c.

However, most current notebooks only offer 1.0 ACPI tables, where the
pstate_cnt entry is marked as "reserved". Unless ACPI is in strict mode,
the value in this "reserved" field is used as well, but a warning is
printed.

So, when is the BIOS notified? When should it be notified? The ACPI 
specification doesn't say much about it. This patch does it if either
of the following cpufreq drivers is pretty much done with a successful
registration:

- acpi
- powernow-k7 [if either it is blacklisted or acpi_force is used]
- powernow-k8 [if the preferred DSDT method is used -- normally, it is...]
- speedstep-centrino [if the preferred DSDT method is used -- normally, it is...]
- speedstep-ich [if the ACPI processor module is built into the kernel. Dave, Arjan: is it in FC?]
- speedstep-smi [if the processor module is built into the kernel. Dave, Arjan: is it in FC?]

There's a small problem with the whole pstate_cnt approach: if there is a 
pyhsical[*] platform limit, it might be necessary to adjust the CPU frequency
to avoid serious problems. Unless the BIOS is notified, the BIOS will likely
do such adjustments on its own. As it is notified now that the OS is handling
P-States, it might be that the BIOS won't handle such serious limits -- but
if the cpufreq driver is "rmmod'ed" in between, it won't be handled. This
is a serious error, but it will _also_ happen on _any_ OS if:
a) the pstate_cnt is written to the BIOS
b) the system locks up hard afterwards so that even IRQs are not handled any more
c) the physical platform limit event occurs

However, I'm unsure as to whether we should "lock" the cpufreq driver once it called
acpi_processor_notify_smm() to minimize the risk mentioned above. What's the opinion of the ACPI
team at Intel?

[*] not the "we-want-to-have-X-hours-of-battery-time-in-that-test" type of platform limits...

Signed-off-by: Dominik Brodowski <linux@brodo.de>

 arch/i386/kernel/cpu/cpufreq/acpi.c               |    4 +
 arch/i386/kernel/cpu/cpufreq/powernow-k7.c        |    3 +
 arch/i386/kernel/cpu/cpufreq/powernow-k8.c        |    3 +
 arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c |    3 +
 arch/i386/kernel/cpu/cpufreq/speedstep-ich.c      |   10 ++++
 arch/i386/kernel/cpu/cpufreq/speedstep-smi.c      |   10 ++++
 drivers/acpi/processor.c                          |   51 ++++++++++++++++++++++
 drivers/acpi/tables/tbconvrt.c                    |   10 +++-
 include/acpi/actbl.h                              |    2
 include/acpi/processor.h                          |    2
 10 files changed, 96 insertions(+), 2 deletions(-)

diff -ruN linux-original/arch/i386/kernel/cpu/cpufreq/acpi.c linux/arch/i386/kernel/cpu/cpufreq/acpi.c
--- linux-original/arch/i386/kernel/cpu/cpufreq/acpi.c	2004-06-14 21:30:37.137669896 +0200
+++ linux/arch/i386/kernel/cpu/cpufreq/acpi.c	2004-06-14 23:10:07.929971496 +0200
@@ -324,6 +324,9 @@
 	}
 	policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
 
+	/* notify BIOS that we exist */
+	acpi_processor_notify_smm();
+
 	/* The current speed is unknown and not detectable by ACPI...  */
 	policy->cur = acpi_cpufreq_guess_freq(data, policy->cpu);
 
@@ -353,6 +356,7 @@
 			(u32) data->acpi_data.states[i].transition_latency);
 
 	cpufreq_frequency_table_get_attr(data->freq_table, policy->cpu);
+
 	return_VALUE(result);
 
  err_freqfree:
diff -ruN linux-original/arch/i386/kernel/cpu/cpufreq/powernow-k7.c linux/arch/i386/kernel/cpu/cpufreq/powernow-k7.c
--- linux-original/arch/i386/kernel/cpu/cpufreq/powernow-k7.c	2004-06-14 18:20:27.000000000 +0200
+++ linux/arch/i386/kernel/cpu/cpufreq/powernow-k7.c	2004-06-14 23:13:31.260060624 +0200
@@ -393,6 +393,9 @@
 	powernow_table[i].frequency = CPUFREQ_TABLE_END;
 	powernow_table[i].index = 0;
 
+	/* As all looks fine by now, notify BIOS that we exist */
+	acpi_processor_notify_smm();
+
 	return 0;
 
 err2:
diff -ruN linux-original/arch/i386/kernel/cpu/cpufreq/powernow-k8.c linux/arch/i386/kernel/cpu/cpufreq/powernow-k8.c
--- linux-original/arch/i386/kernel/cpu/cpufreq/powernow-k8.c	2004-06-14 21:29:18.996549152 +0200
+++ linux/arch/i386/kernel/cpu/cpufreq/powernow-k8.c	2004-06-14 23:13:14.003683992 +0200
@@ -764,6 +764,9 @@
 	powernow_table[data->acpi_data.state_count].index = 0;
 	data->powernow_table = powernow_table;
 
+	/* As all looks fine by now, notify BIOS that we exist */
+	acpi_processor_notify_smm();
+
 	/* fill in data */
 	data->numps = data->acpi_data.state_count;
 	print_basics(data);
diff -ruN linux-original/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c linux/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c
--- linux-original/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c	2004-06-14 23:26:56.074710200 +0200
+++ linux/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c	2004-06-14 23:11:52.340098744 +0200
@@ -371,6 +371,9 @@
                 goto err_kfree;
         }
 
+	/* notify BIOS that we exist */
+	acpi_processor_notify_smm();
+
 	cur_freq = get_cur_freq(0);
 
         for (i=0; i<p.state_count; i++) {
diff -ruN linux-original/arch/i386/kernel/cpu/cpufreq/speedstep-ich.c linux/arch/i386/kernel/cpu/cpufreq/speedstep-ich.c
--- linux-original/arch/i386/kernel/cpu/cpufreq/speedstep-ich.c	2004-06-14 21:29:19.047541400 +0200
+++ linux/arch/i386/kernel/cpu/cpufreq/speedstep-ich.c	2004-06-14 23:10:53.856989528 +0200
@@ -25,6 +25,11 @@
 #include <linux/pci.h>
 #include <linux/slab.h>
 
+#ifdef CONFIG_ACPI_PROCESSOR
+#include <linux/acpi.h>
+#include <acpi/processor.h>
+#endif
+
 #include "speedstep-lib.h"
 
 
@@ -329,6 +334,11 @@
 		return result;
 	}
 
+#ifdef CONFIG_ACPI_PROCESSOR
+	/* notify BIOS that we exist */
+	acpi_processor_notify_smm();
+#endif
+
 	/* get current speed setting */
 	speed = speedstep_get_processor_frequency(speedstep_processor);
 	set_cpus_allowed(current, cpus_allowed);
diff -ruN linux-original/arch/i386/kernel/cpu/cpufreq/speedstep-smi.c linux/arch/i386/kernel/cpu/cpufreq/speedstep-smi.c
--- linux-original/arch/i386/kernel/cpu/cpufreq/speedstep-smi.c	2004-06-14 21:29:19.102533040 +0200
+++ linux/arch/i386/kernel/cpu/cpufreq/speedstep-smi.c	2004-06-14 23:10:14.806926040 +0200
@@ -22,6 +22,11 @@
 #include <linux/delay.h>
 #include <asm/ist.h>
 
+#ifdef CONFIG_ACPI_PROCESSOR
+#include <linux/acpi.h>
+#include <acpi/processor.h>
+#endif
+
 #include "speedstep-lib.h"
 
 #define PFX "speedstep-smi: "
@@ -273,6 +278,11 @@
 			dprintk(KERN_INFO PFX "workaround worked.\n");
 	}
 
+#ifdef CONFIG_ACPI_PROCESSOR
+	/* notify BIOS that we exist */
+	acpi_processor_notify_smm();
+#endif
+
 	/* get current speed setting */
 	state = speedstep_get_state();
 	speed = speedstep_freqs[state].frequency;
diff -ruN linux-original/drivers/acpi/processor.c linux/drivers/acpi/processor.c
--- linux-original/drivers/acpi/processor.c	2004-06-14 18:20:28.000000000 +0200
+++ linux/drivers/acpi/processor.c	2004-06-14 23:29:40.773672152 +0200
@@ -1072,6 +1072,57 @@
 }
 
 
+int acpi_processor_notify_smm(void) {
+	acpi_status		status;
+	static int		is_done = 0;
+
+	ACPI_FUNCTION_TRACE("acpi_processor_notify_smm");
+
+	if (!acpi_processor_ppc_is_init)
+		return_VALUE(-EBUSY);
+
+	/* 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)
+		return_VALUE(0);
+	else if (is_done < 0)
+		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"));
+		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));
+	} else
+		is_done = 1;
+
+	return_VALUE(0);
+}
+EXPORT_SYMBOL(acpi_processor_notify_smm);
+
+
 #ifdef CONFIG_X86_ACPI_CPUFREQ_PROC_INTF
 /* /proc/acpi/processor/../performance interface (DEPRECATED) */
 
diff -ruN linux-original/drivers/acpi/tables/tbconvrt.c linux/drivers/acpi/tables/tbconvrt.c
--- linux-original/drivers/acpi/tables/tbconvrt.c	2004-06-14 18:20:28.000000000 +0200
+++ linux/drivers/acpi/tables/tbconvrt.c	2004-06-14 22:37:57.198487296 +0200
@@ -50,6 +50,8 @@
 	 ACPI_MODULE_NAME    ("tbconvrt")
 
 
+u8 acpi_fadt_is_v1;
+
 /*******************************************************************************
  *
  * FUNCTION:    acpi_tb_get_table_count
@@ -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.
@@ -240,9 +243,12 @@
 	/*
 	 * Processor Performance State Control. This is the value OSPM writes to
 	 * the SMI_CMD register to assume processor performance state control
-	 * responsibility. There isn't any equivalence in 1.0, leave it zeroed.
+	 * responsibility. There isn't any equivalence in 1.0, but as many 1.x
+	 * ACPI tables contain _PCT and _PSS we also keep this value, unless
+	 * acpi_strict is set.
 	 */
-	local_fadt->pstate_cnt = 0;
+	if (acpi_strict)
+		local_fadt->pstate_cnt = 0;
 
 	/*
 	 * Support for the _CST object and C States change notification.
diff -ruN linux-original/include/acpi/actbl.h linux/include/acpi/actbl.h
--- linux-original/include/acpi/actbl.h	2004-06-14 18:20:30.000000000 +0200
+++ linux/include/acpi/actbl.h	2004-06-14 22:37:27.009076784 +0200
@@ -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 -ruN linux-original/include/acpi/processor.h linux/include/acpi/processor.h
--- linux-original/include/acpi/processor.h	2004-06-14 18:20:30.000000000 +0200
+++ linux/include/acpi/processor.h	2004-06-14 23:04:16.551389160 +0200
@@ -140,4 +140,6 @@
 	struct acpi_processor_performance * performance,
 	unsigned int cpu);
 
+extern int acpi_processor_notify_smm (void);
+
 #endif

Presario R3120US not updating present rate

[Ken Hughes]

[-- Attachment #1: Type: text/plain, Size: 439 bytes --]

I noticed that I alway get "present rate: 0 mA" when I
cat /proc/acpi/battery/BAT1/state .  Running acpi -ab
gives the following:

  35 budgie: ./acpi -ab
      Battery 1: charging, 88%, charging at zero rate - will never fully 
charge.
    AC Adapter 1: on-line
  36 budgie: ./acpi -ab
      Battery 1: discharging, 88%, charging at zero rate - will never 
fully charge.
    AC Adapter 1: off-line

The DSDT is attached - any hints?

Ken

[-- Attachment #2: DSDT --]
[-- Type: application/octet-stream, Size: 17761 bytes --]

Re: Presario R3120US not updating present rate

[liste-9nAOAgdJVo4b1SvskN2V4Q]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Salut Ken,
On Mon, 14 Jun 2004, Ken Hughes wrote:
> I noticed that I alway get "present rate: 0 mA" when I
> cat /proc/acpi/battery/BAT1/state .  Running acpi -ab
> gives the following:
>
> 35 budgie: ./acpi -ab
>     Battery 1: charging, 88%, charging at zero rate - will never fully 
> charge.
>   AC Adapter 1: on-line
> 36 budgie: ./acpi -ab
>     Battery 1: discharging, 88%, charging at zero rate - will never fully 
> charge.
>   AC Adapter 1: off-line
I did not look into your DSDT, but as you are surely not using a 
primary-battery (as opposed to an akkumulator), the offline-case needs to 
report a sensible value, according to the ACPI-1.0 spec.
For the online-case if would suffice to report the unknown-value (its the 
MAX-UINT...). So your BIOS is NOT ACPI-compilant!
The BIOS of my Acer TM-63x reports in both case the unknown-value (so is 
less buggy, but still not ACPI-compliant). The problem might be, that XP 
does not care about the present-rate.

cheers
   hartwig felger

Hartwig Felger informatics

- -- 
1024D/339FD693 Hartwig Felger <hgfelger-9nAOAgdJVo4b1SvskN2V4Q@public.gmane.org>
Key fingerprint = FB2F 3EE9 345A D55B 6FF2  0EC1 F5B0 684F 339F D693
For the pulic keys, please visit my page.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)

iD8DBQFAzq+49bBoTzOf1pMRAmJJAKDQGDDIWYr0UQDM20K4Msqd9zM0HQCg7dYu
Nhl5EPwT1FW+psYYNP94AtI=
=O+qE
-----END PGP SIGNATURE-----


-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND

Re: Presario R3120US not updating present rate

[Bruno Ducrot]

On Mon, Jun 14, 2004 at 07:31:05PM -0700, Ken Hughes wrote:
> I noticed that I alway get "present rate: 0 mA" when I
> cat /proc/acpi/battery/BAT1/state .  Running acpi -ab
> gives the following:
> 
>  35 budgie: ./acpi -ab
>      Battery 1: charging, 88%, charging at zero rate - will never fully 
> charge.
>    AC Adapter 1: on-line
>  36 budgie: ./acpi -ab
>      Battery 1: discharging, 88%, charging at zero rate - will never 
> fully charge.
>    AC Adapter 1: off-line
> 
> The DSDT is attached - any hints?
> 

You have that in the ASL:
	Device (BAT1)
	{
	    Name (_HID, EisaId ("PNP0C0A"))
	    Name (_UID, 0x01)

            ...
            ...

	    Name (PBST, Package (0x04)
	    {
		0x01, 
		0xFFFFFFFF, 
		0xFFFFFFFF, 
		0x39D0
	    })

            ...
            ...

	    Method (_BST, 0, NotSerialized)
	    {
		If (ECOK ())
		{
		    Sleep (0x14)
		    Store (\_SB.PCI0.LPC0.EC0.BST0, Local0)
		    And (Local0, 0x07, Local0)
		    Store (Local0, Index (PBST, 0x00))
		    Sleep (0x14)
		    Store (\_SB.PCI0.LPC0.EC0.GAU0, Local2)
		    Sleep (0x14)
		    Store (\_SB.PCI0.LPC0.EC0.BPV0, Index (PBST, 0x03))
		    Sleep (0x14)
		    Store (\_SB.PCI0.LPC0.EC0.BFC0, Local1)
		    Sleep (0x14)
		    If (Local2)
		    {
			Multiply (Local2, Local1, Local2)
			Divide (Local2, 0x64, Local6, Local2)
			If (Local6)
			{
			    Increment (Local2)
			}
		    }

		    Store (0x00, Index (PBST, 0x01))
		    Store (Local2, Index (PBST, 0x02))
		}

		Return (PBST)
	    }

The 'Store(0, Index(PBST, 0x01))' will put 0 in the second entry to the
PBST.  This entry match the present-rate.  As you can see, 0 will always
be put, so it's a DSDT trouble, not a linux one unfortunately.

Cheers, 


-- 
Bruno Ducrot

--  Which is worse:  ignorance or apathy?
--  Don't know.  Don't care.


-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND

Re: Presario R3120US not updating present rate

[Ken Hughes]

Ken Hughes wrote:
> I noticed that I alway get "present rate: 0 mA" when I
> cat /proc/acpi/battery/BAT1/state .  Running acpi -ab
> gives the following:
> 
>  35 budgie: ./acpi -ab
>      Battery 1: charging, 88%, charging at zero rate - will never fully 
> charge.
>    AC Adapter 1: on-line
>  36 budgie: ./acpi -ab
>      Battery 1: discharging, 88%, charging at zero rate - will never 
> fully charge.
>    AC Adapter 1: off-line

Discovered that one field read from the embedded controller actually
returns the precentage of charge remaining in the battery.

In any case, is there a way thru software to determine the type of
embedded controller?  I'd rather not take the screwdriver to my
laptop to find out.

Thanks -- Ken


-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND