All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@us.ibm.com>
To: "Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>
Cc: Dave Jones <davej@codemonkey.org.uk>,
	Chris McDermott <lcm@us.ibm.com>,
	cpufreq@lists.linux.org.uk
Subject: Re: Can't load speedstep-centrino on IBM x336?
Date: Sat, 29 Oct 2005 19:34:36 -0700	[thread overview]
Message-ID: <4364313C.4020902@us.ibm.com> (raw)
In-Reply-To: <88056F38E9E48644A0F562A38C64FB600625F151@scsmsx403.amr.corp.intel.com>


[-- 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

  reply	other threads:[~2005-10-30  2:34 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-10-28  1:05 Can't load speedstep-centrino on IBM x336? Pallipadi, Venkatesh
2005-10-30  2:34 ` Darrick J. Wong [this message]
  -- strict thread matches above, loose matches on Subject: below --
2005-10-26 17:01 Pallipadi, Venkatesh
2005-10-28  0:53 ` Darrick J. Wong
2005-10-25  2:00 Pallipadi, Venkatesh
2005-10-25  6:18 ` Darrick J. Wong
2005-10-25 16:21   ` Venkatesh Pallipadi
2005-10-26  2:31     ` Darrick J. Wong
2005-10-25  1:48 Darrick J. Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4364313C.4020902@us.ibm.com \
    --to=djwong@us.ibm.com \
    --cc=cpufreq@lists.linux.org.uk \
    --cc=davej@codemonkey.org.uk \
    --cc=lcm@us.ibm.com \
    --cc=venkatesh.pallipadi@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.