ACPI: DMI init_set_sci_en_on_resume for multiple Lenovo ThinkPads
PATCH 1/1 ACPI: DMI init_set_sci_en_on_resume for multiple Lenovo ThinkPads
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.
---
drivers/acpi/sleep.c +40 -0
@@ -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 = {
Quoted 4 lines · Alex Chiang
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.
Signature · 1 line
Matthew Garrett | [email protected]
* Matthew Garrett <[email protected]>:
Quoted 10 lines · Alex Chiang
Quoted 5 lines
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
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
Quoted 80 lines · 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_ENcoming 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/532374Confirmed 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.
---
drivers/acpi/sleep.c +40
-1@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 6 lines · 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.
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.
Signature · 1 line
Matthew Garrett | [email protected]
Quoted 8 lines · Matthew Garrett
Quoted 6 lines · 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.
Quoted 7 lines
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
* Jerone Young <[email protected]>:
Quoted 3 lines
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.
Quoted 2 lines
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
Quoted 5 lines · Jerone Young
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.
Signature · 1 line
Matthew Garrett | [email protected]
Quoted 8 lines · Matthew Garrett
Quoted 5 lines · Jerone Young
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.
Quoted 4 lines
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
* Alex Chiang <[email protected]>:
Quoted 6 lines
drivers/acpi/sleep.c +6
-0+ .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"), + },
drivers/acpi/sleep.c static struct dmi_system_id __initdata acpisleep_dmi_table[] = { · line 492
Quoted 10 lines
drivers/acpi/sleep.c +10
-0+ }, + { + .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
Len Brown
reviewerapplied 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
* Len Brown <[email protected]>:
Quoted 1 line
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