[PATCH] ACPI: DMI init_set_sci_en_on_resume for multiple Lenovo ThinkPads

[PATCH] ACPI: DMI init_set_sci_en_on_resume for multiple Lenovo ThinkPads

[Alex Chiang]

Multiple Lenovo ThinkPad models with Intel Core i5/i7 CPUs can
successfully suspend/resume once, and then hang on the second s/r
cycle.

We got confirmation that this was due to a BIOS defect. The BIOS
did not properly set SCI_EN coming out of S3. The BIOS guys
hinted that The Other Leading OS ignores the fact that hardware
owns the bit and sets it manually.

In any case, an existing DMI table exists for machines where this
defect is a known problem. Lenovo promise to fix their BIOS, but
for folks who either won't or can't upgrade their BIOS, allow
Linux to workaround the issue.

https://bugzilla.kernel.org/show_bug.cgi?id=15407
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/532374

Confirmed by numerous testers in the launchpad bug that using
acpi_sleep=sci_force_enable fixes the issue. We add the machines
to acpisleep_dmi_table[] to automatically enable this workaround.

Cc: stable@kernel.org
Cc: Colin King <colin.king@canonical.com>
Signed-off-by: Alex Chiang <achiang@canonical.com>
---
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index f74834a..ce54cd0 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -450,6 +450,46 @@ static struct dmi_system_id __initdata acpisleep_dmi_table[] = {
 		},
 	},
 	{
+	.callback = init_set_sci_en_on_resume,
+	.ident = "Lenovo ThinkPad T410",
+	.matches = {
+		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+		DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T410"),
+		},
+	},
+	{
+	.callback = init_set_sci_en_on_resume,
+	.ident = "Lenovo ThinkPad T510",
+	.matches = {
+		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+		DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T510"),
+		},
+	},
+	{
+	.callback = init_set_sci_en_on_resume,
+	.ident = "Lenovo ThinkPad W510",
+	.matches = {
+		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+		DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad W510"),
+		},
+	},
+	{
+	.callback = init_set_sci_en_on_resume,
+	.ident = "Lenovo ThinkPad X201",
+	.matches = {
+		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+		DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X201"),
+		},
+	},
+	{
+	.callback = init_set_sci_en_on_resume,
+	.ident = "Lenovo ThinkPad X201",
+	.matches = {
+		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+		DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X201s"),
+		},
+	},
+	{
 	.callback = init_old_suspend_ordering,
 	.ident = "Panasonic CF51-2L",
 	.matches = {

Re: [PATCH] ACPI: DMI init_set_sci_en_on_resume for multiple Lenovo ThinkPads

[Matthew Garrett]

On Tue, Apr 20, 2010 at 08:03:14AM -0600, Alex Chiang wrote:

> In any case, an existing DMI table exists for machines where this
> defect is a known problem. Lenovo promise to fix their BIOS, but
> for folks who either won't or can't upgrade their BIOS, allow
> Linux to workaround the issue.

While this is certainly the low impact fix for now, I'd prefer the patch 
I sent as a long-term solution in order to avoid having to add more 
machines to the table in future.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] ACPI: DMI init_set_sci_en_on_resume for multiple Lenovo ThinkPads

[Alex Chiang]

* Matthew Garrett <mjg59@srcf.ucam.org>:
> On Tue, Apr 20, 2010 at 08:03:14AM -0600, Alex Chiang wrote:
> 
> > In any case, an existing DMI table exists for machines where
> > this defect is a known problem. Lenovo promise to fix their
> > BIOS, but for folks who either won't or can't upgrade their
> > BIOS, allow Linux to workaround the issue.
> 
> While this is certainly the low impact fix for now, I'd prefer
> the patch I sent as a long-term solution in order to avoid
> having to add more machines to the table in future.

Ah, yes, I should have been more clear.

I want to get this patch upstream so we can get it into stable,
so that we can get it into Ubuntu 10.04.

Certainly what I'm proposing is suitable for stable: it's
minimal, specific to a few machines, etc.

For the long term, I support the patch that Matthew sent out
yesterday, and we can perhaps think about removing the quirks
entirely.

Thanks,
/ac

Re: [PATCH] ACPI: DMI init_set_sci_en_on_resume for multiple Lenovo ThinkPads

[Jerone Young]

Don't think this patch should go in as Lenovo is issuing a BIOS to
address this issue. On these Thinkpads listed and more. Seems to add
uneeded quirks in my opinion.

Though some over all patch may be good for all machines. As Windows
seems to be setting this bit on it's own.

				Jerone

On Tue, 2010-04-20 at 08:03 -0600, Alex Chiang wrote:
> Multiple Lenovo ThinkPad models with Intel Core i5/i7 CPUs can
> successfully suspend/resume once, and then hang on the second s/r
> cycle.
> 
> We got confirmation that this was due to a BIOS defect. The BIOS
> did not properly set SCI_EN coming out of S3. The BIOS guys
> hinted that The Other Leading OS ignores the fact that hardware
> owns the bit and sets it manually.
> 
> In any case, an existing DMI table exists for machines where this
> defect is a known problem. Lenovo promise to fix their BIOS, but
> for folks who either won't or can't upgrade their BIOS, allow
> Linux to workaround the issue.
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=15407
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/532374
> 
> Confirmed by numerous testers in the launchpad bug that using
> acpi_sleep=sci_force_enable fixes the issue. We add the machines
> to acpisleep_dmi_table[] to automatically enable this workaround.
> 
> Cc: stable@kernel.org
> Cc: Colin King <colin.king@canonical.com>
> Signed-off-by: Alex Chiang <achiang@canonical.com>
> ---
> diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> index f74834a..ce54cd0 100644
> --- a/drivers/acpi/sleep.c
> +++ b/drivers/acpi/sleep.c
> @@ -450,6 +450,46 @@ static struct dmi_system_id __initdata acpisleep_dmi_table[] = {
>  		},
>  	},
>  	{
> +	.callback = init_set_sci_en_on_resume,
> +	.ident = "Lenovo ThinkPad T410",
> +	.matches = {
> +		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> +		DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T410"),
> +		},
> +	},
> +	{
> +	.callback = init_set_sci_en_on_resume,
> +	.ident = "Lenovo ThinkPad T510",
> +	.matches = {
> +		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> +		DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T510"),
> +		},
> +	},
> +	{
> +	.callback = init_set_sci_en_on_resume,
> +	.ident = "Lenovo ThinkPad W510",
> +	.matches = {
> +		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> +		DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad W510"),
> +		},
> +	},
> +	{
> +	.callback = init_set_sci_en_on_resume,
> +	.ident = "Lenovo ThinkPad X201",
> +	.matches = {
> +		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> +		DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X201"),
> +		},
> +	},
> +	{
> +	.callback = init_set_sci_en_on_resume,
> +	.ident = "Lenovo ThinkPad X201",
> +	.matches = {
> +		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> +		DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X201s"),
> +		},
> +	},
> +	{
>  	.callback = init_old_suspend_ordering,
>  	.ident = "Panasonic CF51-2L",
>  	.matches = {
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI: DMI init_set_sci_en_on_resume for multiple Lenovo ThinkPads

[Matthew Garrett]

On Tue, Apr 20, 2010 at 11:56:41AM -0500, Jerone Young wrote:
> Don't think this patch should go in as Lenovo is issuing a BIOS to
> address this issue. On these Thinkpads listed and more. Seems to add
> uneeded quirks in my opinion.
> 
> Though some over all patch may be good for all machines. As Windows
> seems to be setting this bit on it's own.

It's a perfectly safe workaround that's limited to a well-bounded set of 
machines and fixes a pretty serious problem with them. While I'd prefer 
my approach and then a removal of the existing quirk entries, Alex's 
patch makes more sense for stable. The BIOS update is as yet unreleased, 
requires either Windows or an optical drive (which some of these 
machines don't have) and is a far from discoverable solution to the 
issue.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] ACPI: DMI init_set_sci_en_on_resume for multiple Lenovo ThinkPads

[Jerone Young]

On Tue, 2010-04-20 at 18:14 +0100, Matthew Garrett wrote:
> On Tue, Apr 20, 2010 at 11:56:41AM -0500, Jerone Young wrote:
> > Don't think this patch should go in as Lenovo is issuing a BIOS to
> > address this issue. On these Thinkpads listed and more. Seems to add
> > uneeded quirks in my opinion.
> > 
> > Though some over all patch may be good for all machines. As Windows
> > seems to be setting this bit on it's own.
> 
> It's a perfectly safe workaround that's limited to a well-bounded set of 
> machines and fixes a pretty serious problem with them. While I'd prefer 
> my approach and then a removal of the existing quirk entries, Alex's 
> patch makes more sense for stable. The BIOS update is as yet unreleased, 
> requires either Windows or an optical drive (which some of these 
> machines don't have) and is a far from discoverable solution to the 
> issue.

I could agree, but the only issue is if you go to the thinkpad-acpi or
ibm-acpi mailing list they would probably disagree as there have been
updates to the bios in the past required for proper functionality of
Thinkpads ;-) .. the bios update methods have not changed since then for
Thinkpads.

			Thanks,
				Jerone

Re: [PATCH] ACPI: DMI init_set_sci_en_on_resume for multiple Lenovo ThinkPads

[Matthew Garrett]

On Tue, Apr 20, 2010 at 12:18:56PM -0500, Jerone Young wrote:

> I could agree, but the only issue is if you go to the thinkpad-acpi or
> ibm-acpi mailing list they would probably disagree as there have been
> updates to the bios in the past required for proper functionality of
> Thinkpads ;-) .. the bios update methods have not changed since then for
> Thinkpads.

If another OS works, and if Linux can be made to work without impairing 
the functionality of working machines, then Linux should be made to 
work. Always. Updating the BIOS should be limited to cases where it's 
impossible to fix things up in the OS.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] ACPI: DMI init_set_sci_en_on_resume for multiple Lenovo ThinkPads

[Rafael J. Wysocki]

On Tuesday 20 April 2010, Matthew Garrett wrote:
> On Tue, Apr 20, 2010 at 12:18:56PM -0500, Jerone Young wrote:
> 
> > I could agree, but the only issue is if you go to the thinkpad-acpi or
> > ibm-acpi mailing list they would probably disagree as there have been
> > updates to the bios in the past required for proper functionality of
> > Thinkpads ;-) .. the bios update methods have not changed since then for
> > Thinkpads.
> 
> If another OS works, and if Linux can be made to work without impairing 
> the functionality of working machines, then Linux should be made to 
> work. Always. Updating the BIOS should be limited to cases where it's 
> impossible to fix things up in the OS.

Agreed.

Rafael

Re: [PATCH] ACPI: DMI init_set_sci_en_on_resume for multiple Lenovo ThinkPads

[Alex Chiang]

* Jerone Young <jerone.young@canonical.com>:
> Don't think this patch should go in as Lenovo is issuing a BIOS to
> address this issue. On these Thinkpads listed and more. Seems to add
> uneeded quirks in my opinion.

Sure, we'll let Len decide if he wants to take it or not. I'm ok
with that.

> Though some over all patch may be good for all machines. As Windows
> seems to be setting this bit on it's own.

Yes, that's what Matthew's patch does.
	http://thread.gmane.org/gmane.linux.acpi.devel/45635

/ac

Re: [PATCH] ACPI: DMI init_set_sci_en_on_resume for multiple Lenovo ThinkPads

[Alex Chiang]

* Alex Chiang <achiang@canonical.com>:
> +	.callback = init_set_sci_en_on_resume,
> +	.ident = "Lenovo ThinkPad X201",
> +	.matches = {
> +		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> +		DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X201"),
> +		},
> +	},
> +	{
> +	.callback = init_set_sci_en_on_resume,
> +	.ident = "Lenovo ThinkPad X201",
> +	.matches = {
> +		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> +		DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X201s"),
> +		},
> +	},
> +	{

Whoops, not only is the .ident line wrong for the second entry
above (should be X201s), it's also extraneous, as the first entry
will catch it.

I'll resubmit.

/ac

Re: [PATCH] ACPI: DMI init_set_sci_en_on_resume for multiple Lenovo ThinkPads

[Len Brown]

applied for 2.6.34 (and acked for .stable)

hopefully we'll figure out how to safely nuke this DMI list shortly.

thanks,
Len Brown, Intel Open Source Technology Center

Re: [PATCH] ACPI: DMI init_set_sci_en_on_resume for multiple Lenovo ThinkPads

[Alex Chiang]

* Len Brown <lenb@kernel.org>:
> applied for 2.6.34 (and acked for .stable)

Whoops, you picked up the v1, not the v2 version.

The v2 version is a little shorter; it elides the X201S entry
because the X201 entry will catch it.

Doesn't affect functionality / correctness. It's just a little
superfluous to have the 2nd entry.

fyi,
/ac