Re: [stable] RE: [patch 10/26] ACPI: Prefer _CST over FADT for C-state capabilities

Re: [stable] RE: [patch 10/26] ACPI: Prefer _CST over FADT for C-state capabilities

[Greg KH]

On Tue, Dec 13, 2005 at 01:56:26PM -0500, Brown, Len wrote:
> yes, the 3rd patch should go with the first two.
> I asked Linus to pull the 3rd patch upstream
> http://lkml.org/lkml/2005/12/6/32
> but this was just as he cut -rc5 and headed out for a week.
> 
> Linus,
> Can you pull that patch upstream before cutting 2.6.15?
> 
> Thanks Daniel for the follow-up,

Can someone send the patch to stable@ when it goes in so I can include
it in the next round?  I've moved the other two acpi patches for the
next release, pending this patch.

thanks,

greg k-h

Re: [patch 10/26] ACPI: Prefer _CST over FADT for C-state capabilities

[Daniel Drake]

Linus,

Brown, Len wrote:
> yes, the 3rd patch should go with the first two.
> I asked Linus to pull the 3rd patch upstream
> http://lkml.org/lkml/2005/12/6/32
> but this was just as he cut -rc5 and headed out for a week.
> 
> Linus,
> Can you pull that patch upstream before cutting 2.6.15?

This patch still isn't in your tree. Can you please pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux-acpi-2.6.git release

Thanks,
Daniel

[patch 10/26] ACPI: Prefer _CST over FADT for C-state capabilities

[Greg KH]

[-- Attachment #1: acpi-prefer-_cst-over-fadt-for-c-state-capabilities.patch --]
[-- Type: text/plain, Size: 1602 bytes --]

-stable review patch.  If anyone has any objections, please let us know.

------------------
From: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>

Note: This ACPI standard compliance may cause regression
on some system, if they have _CST present, but _CST value
is bogus. "nocst" module parameter should workaround
that regression.

http://bugzilla.kernel.org/show_bug.cgi?id=5165

(cherry picked from 883baf7f7e81cca26f4683ae0d25ba48f094cc08 commit)

Signed-off-by: Venkatesh Pallipadi<venkatesh.pallipadi@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
---
 drivers/acpi/processor_idle.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

--- linux-2.6.14.3.orig/drivers/acpi/processor_idle.c
+++ linux-2.6.14.3/drivers/acpi/processor_idle.c
@@ -687,7 +687,7 @@ static int acpi_processor_get_power_info
 
 	/* Validate number of power states discovered */
 	if (pr->power.count < 2)
-		status = -ENODEV;
+		status = -EFAULT;
 
       end:
 	acpi_os_free(buffer.pointer);
@@ -838,11 +838,11 @@ static int acpi_processor_get_power_info
 	 * this function */
 
 	result = acpi_processor_get_power_info_cst(pr);
-	if ((result) || (acpi_processor_power_verify(pr) < 2)) {
+	if (result == -ENODEV)
 		result = acpi_processor_get_power_info_fadt(pr);
-		if ((result) || (acpi_processor_power_verify(pr) < 2))
-			result = acpi_processor_get_power_info_default_c1(pr);
-	}
+
+	if ((result) || (acpi_processor_power_verify(pr) < 2))
+		result = acpi_processor_get_power_info_default_c1(pr);
 
 	/*
 	 * Set Default Policy

--

Re: [patch 10/26] ACPI: Prefer _CST over FADT for C-state capabilities

[Daniel Drake]

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

Greg KH wrote:
> -stable review patch.  If anyone has any objections, please let us know.
> 
> ------------------
> From: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> 
> Note: This ACPI standard compliance may cause regression
> on some system, if they have _CST present, but _CST value
> is bogus. "nocst" module parameter should workaround
> that regression.
> 
> http://bugzilla.kernel.org/show_bug.cgi?id=5165
> 
> (cherry picked from 883baf7f7e81cca26f4683ae0d25ba48f094cc08 commit)
> 
> Signed-off-by: Venkatesh Pallipadi<venkatesh.pallipadi@intel.com>
> Signed-off-by: Len Brown <len.brown@intel.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> ---
>  drivers/acpi/processor_idle.c |   10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

Venkatesh followed up in a private email that a 3rd patch is needed to  solve 
the hyperthreading slowdown issue. This patch is not yet in Linus' tree (it is 
in acpi-test).

Maybe we should drop these patches (10 and 12) until the 3rd patch has been 
merged. I haven't been shipping the 3rd patch in Gentoo (yet) so I'm not able 
to gauge its effect...

Attaching the 3rd patch anyway.

Daniel

[-- Attachment #2: p_LVL2_UP-flag-increment.patch --]
[-- Type: text/x-patch, Size: 1826 bytes --]

From: Len Brown <len.brown@intel.com>

Bug fix for bugzilla #5165 http://bugzilla.kernel.org/show_bug.cgi?id=5165

Incremental changes to earlier patch.
* Changing the polarity of plvl2_up
* Skip promotion/demotion code when not needed.

Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Signed-off-by: Shaohua Li <shaohua.li@intel.com>

Index: linux-acpi-2.6/drivers/acpi/processor_idle.c
===================================================================
--- linux-acpi-2.6.orig/drivers/acpi/processor_idle.c
+++ linux-acpi-2.6/drivers/acpi/processor_idle.c
@@ -278,8 +278,6 @@ static void acpi_processor_idle(void)
 		}
 	}
 
-	cx->usage++;
-
 #ifdef CONFIG_HOTPLUG_CPU
 	/*
 	 * Check for P_LVL2_UP flag before entering C2 and above on
@@ -287,9 +285,12 @@ static void acpi_processor_idle(void)
 	 * detection phase, to work cleanly with logical CPU hotplug.
 	 */
 	if ((cx->type != ACPI_STATE_C1) && (num_online_cpus() > 1) && 
-	    !pr->flags.has_cst && acpi_fadt.plvl2_up)
-		cx->type = ACPI_STATE_C1;
+	    !pr->flags.has_cst && !acpi_fadt.plvl2_up)
+		cx = &pr->power.states[ACPI_STATE_C1];
 #endif
+
+	cx->usage++;
+
 	/*
 	 * Sleep:
 	 * ------
@@ -378,6 +379,15 @@ static void acpi_processor_idle(void)
 
 	next_state = pr->power.state;
 
+#ifdef CONFIG_HOTPLUG_CPU
+	/* Don't do promotion/demotion */
+	if ((cx->type == ACPI_STATE_C1) && (num_online_cpus() > 1) &&
+	    !pr->flags.has_cst && !acpi_fadt.plvl2_up) {
+		next_state = cx;
+		goto end;
+	}
+#endif
+
 	/*
 	 * Promotion?
 	 * ----------
@@ -549,7 +559,7 @@ static int acpi_processor_get_power_info
 	 * Check for P_LVL2_UP flag before entering C2 and above on
 	 * an SMP system. 
 	 */
-	if ((num_online_cpus() > 1) && acpi_fadt.plvl2_up)
+	if ((num_online_cpus() > 1) && !acpi_fadt.plvl2_up)
 		return_VALUE(-ENODEV);
 #endif
 

Re: [stable] Re: [patch 10/26] ACPI: Prefer _CST over FADT for C-state capabilities

[Greg KH]

On Tue, Dec 13, 2005 at 04:00:47PM +0000, Daniel Drake wrote:
> Greg KH wrote:
> >-stable review patch.  If anyone has any objections, please let us know.
> >
> >------------------
> >From: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> >
> >Note: This ACPI standard compliance may cause regression
> >on some system, if they have _CST present, but _CST value
> >is bogus. "nocst" module parameter should workaround
> >that regression.
> >
> >http://bugzilla.kernel.org/show_bug.cgi?id=5165
> >
> >(cherry picked from 883baf7f7e81cca26f4683ae0d25ba48f094cc08 commit)
> >
> >Signed-off-by: Venkatesh Pallipadi<venkatesh.pallipadi@intel.com>
> >Signed-off-by: Len Brown <len.brown@intel.com>
> >Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> >---
> > drivers/acpi/processor_idle.c |   10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> 
> Venkatesh followed up in a private email that a 3rd patch is needed to  
> solve the hyperthreading slowdown issue. This patch is not yet in Linus' 
> tree (it is in acpi-test).
> 
> Maybe we should drop these patches (10 and 12) until the 3rd patch has been 
> merged. I haven't been shipping the 3rd patch in Gentoo (yet) so I'm not 
> able to gauge its effect...

Ok, as these two patches don't solve anything, I'll just drop them from
the series.  If the third one makes it into mainline, let us know and we
can add all 3 to the -stable tree.

thanks,

greg k-h