Message ID | 1395968038-11577-1-git-send-email-kamal@canonical.com |
---|---|
State | New |
Headers | show |
Dear Kamal, please wait for now as a slight regression was reported for one system and the patch will be changed. Best, Dennis Am Freitag, 28. März 2014 schrieb Kamal Mostafa : > This is a note to let you know that I have just added a patch titled > > ACPI / EC: Clear stale EC events on Samsung systems > > to the linux-3.8.y-queue branch of the 3.8.y.z extended stable tree > which can be found at: > > > http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.8.y-queue > > This patch is scheduled to be released in version 3.8.13.21. > > If you, or anyone else, feels it should not be added to this tree, please > reply to this email. > > For more information about the 3.8.y.z tree, see > https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable > > Thanks. > -Kamal > > ------ > > From 1393f3f58ecb25d9ce797d30baf41eeccf975516 Mon Sep 17 00:00:00 2001 > From: Kieran Clancy <clancy.kieran@gmail.com <javascript:;>> > Date: Sat, 1 Mar 2014 00:42:28 +1030 > Subject: ACPI / EC: Clear stale EC events on Samsung systems > > commit ad332c8a45330d170bb38b95209de449b31cd1b4 upstream. > > A number of Samsung notebooks (530Uxx/535Uxx/540Uxx/550Pxx/900Xxx/etc) > continue to log events during sleep (lid open/close, AC plug/unplug, > battery level change), which accumulate in the EC until a buffer fills. > After the buffer is full (tests suggest it holds 8 events), GPEs stop > being triggered for new events. This state persists on wake or even on > power cycle, and prevents new events from being registered until the EC > is manually polled. > > This is the root cause of a number of bugs, including AC not being > detected properly, lid close not triggering suspend, and low ambient > light not triggering the keyboard backlight. The bug also seemed to be > responsible for performance issues on at least one user's machine. > > Juan Manuel Cabo found the cause of bug and the workaround of polling > the EC manually on wake. > > The loop which clears the stale events is based on an earlier patch by > Lan Tianyu (see referenced attachment). > > This patch: > - Adds a function acpi_ec_clear() which polls the EC for stale _Q > events at most ACPI_EC_CLEAR_MAX (currently 100) times. A warning is > logged if this limit is reached. > - Adds a flag EC_FLAGS_CLEAR_ON_RESUME which is set to 1 if the DMI > system vendor is Samsung. This check could be replaced by several > more specific DMI vendor/product pairs, but it's likely that the bug > affects more Samsung products than just the five series mentioned > above. Further, it should not be harmful to run acpi_ec_clear() on > systems without the bug; it will return immediately after finding no > data waiting. > - Runs acpi_ec_clear() on initialisation (boot), from acpi_ec_add() > - Runs acpi_ec_clear() on wake, from acpi_ec_unblock_transactions() > > References: https://bugzilla.kernel.org/show_bug.cgi?id=44161 > References: https://bugzilla.kernel.org/show_bug.cgi?id=45461 > References: https://bugzilla.kernel.org/show_bug.cgi?id=57271 > References: https://bugzilla.kernel.org/attachment.cgi?id=126801 > Suggested-by: Juan Manuel Cabo <juanmanuel.cabo@gmail.com <javascript:;>> > Signed-off-by: Kieran Clancy <clancy.kieran@gmail.com <javascript:;>> > Reviewed-by: Lan Tianyu <tianyu.lan@intel.com <javascript:;>> > Reviewed-by: Dennis Jansen <dennis.jansen@web.de <javascript:;>> > Tested-by: Kieran Clancy <clancy.kieran@gmail.com <javascript:;>> > Tested-by: Juan Manuel Cabo <juanmanuel.cabo@gmail.com <javascript:;>> > Tested-by: Dennis Jansen <dennis.jansen@web.de <javascript:;>> > Tested-by: Maurizio D'Addona <mauritiusdadd@gmail.com <javascript:;>> > Tested-by: San Zamoyski <san@plusnet.pl <javascript:;>> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com<javascript:;> > > > [ kamal: backport to 3.8 (context) ] > Signed-off-by: Kamal Mostafa <kamal@canonical.com <javascript:;>> > --- > drivers/acpi/ec.c | 64 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 64 insertions(+) > > diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c > index 97fcfb0..092f862 100644 > --- a/drivers/acpi/ec.c > +++ b/drivers/acpi/ec.c > @@ -70,6 +70,8 @@ enum ec_command { > #define ACPI_EC_DELAY 500 /* Wait 500ms max. during EC ops */ > #define ACPI_EC_UDELAY_GLK 1000 /* Wait 1ms max. to get global > lock */ > #define ACPI_EC_MSI_UDELAY 550 /* Wait 550us for MSI EC */ > +#define ACPI_EC_CLEAR_MAX 100 /* Maximum number of events to > query > + * when trying to clear the EC */ > > enum { > EC_FLAGS_QUERY_PENDING, /* Query is pending */ > @@ -123,6 +125,7 @@ EXPORT_SYMBOL(first_ec); > static int EC_FLAGS_MSI; /* Out-of-spec MSI controller */ > static int EC_FLAGS_VALIDATE_ECDT; /* ASUStec ECDTs need to be validated > */ > static int EC_FLAGS_SKIP_DSDT_SCAN; /* Not all BIOS survive early DSDT > scan */ > +static int EC_FLAGS_CLEAR_ON_RESUME; /* Needs acpi_ec_clear() on > boot/resume */ > > /* > -------------------------------------------------------------------------- > Transaction Management > @@ -468,6 +471,29 @@ acpi_handle ec_get_handle(void) > > EXPORT_SYMBOL(ec_get_handle); > > +static int acpi_ec_query_unlocked(struct acpi_ec *ec, u8 *data); > + > +/* > + * Clears stale _Q events that might have accumulated in the EC. > + * Run with locked ec mutex. > + */ > +static void acpi_ec_clear(struct acpi_ec *ec) > +{ > + int i, status; > + u8 value = 0; > + > + for (i = 0; i < ACPI_EC_CLEAR_MAX; i++) { > + status = acpi_ec_query_unlocked(ec, &value); > + if (status || !value) > + break; > + } > + > + if (unlikely(i == ACPI_EC_CLEAR_MAX)) > + pr_warn("Warning: Maximum of %d stale EC events > cleared\n", i); > + else > + pr_info("%d stale EC events cleared\n", i); > +} > + > void acpi_ec_block_transactions(void) > { > struct acpi_ec *ec = first_ec; > @@ -491,6 +517,10 @@ void acpi_ec_unblock_transactions(void) > mutex_lock(&ec->mutex); > /* Allow transactions to be carried out again */ > clear_bit(EC_FLAGS_BLOCKED, &ec->flags); > + > + if (EC_FLAGS_CLEAR_ON_RESUME) > + acpi_ec_clear(ec); > + > mutex_unlock(&ec->mutex); > } > > @@ -848,6 +878,13 @@ static int acpi_ec_add(struct acpi_device *device) > > /* EC is fully operational, allow queries */ > clear_bit(EC_FLAGS_QUERY_PENDING, &ec->flags); > + > + /* Clear stale _Q events if hardware might require that */ > + if (EC_FLAGS_CLEAR_ON_RESUME) { > + mutex_lock(&ec->mutex); > + acpi_ec_clear(ec); > + mutex_unlock(&ec->mutex); > + } > return ret; > } > > @@ -949,6 +986,30 @@ static int ec_enlarge_storm_threshold(const struct > dmi_system_id *id) > return 0; > } > > +/* > + * On some hardware it is necessary to clear events accumulated by the EC > during > + * sleep. These ECs stop reporting GPEs until they are manually polled, > if too > + * many events are accumulated. (e.g. Samsung Series 5/9 notebooks) > + * > + * https://bugzilla.kernel.org/show_bug.cgi?id=44161 > + * > + * Ideally, the EC should also be instructed NOT to accumulate events > during > + * sleep (which Windows seems to do somehow), but the interface to > control this > + * behaviour is not known at this time. > + * > + * Models known to be affected are Samsung > 530Uxx/535Uxx/540Uxx/550Pxx/900Xxx, > + * however it is very likely that other Samsung models are affected. > + * > + * On systems which don't accumulate _Q events during sleep, this extra > check > + * should be harmless. > + */ > +static int ec_clear_on_resume(const struct dmi_system_id *id) > +{ > + pr_debug("Detected system needing EC poll on resume.\n"); > + EC_FLAGS_CLEAR_ON_RESUME = 1; > + return 0; > +} > + > static struct dmi_system_id __initdata ec_dmi_table[] = { > { > ec_skip_dsdt_scan, "Compal JFL92", { > @@ -992,6 +1053,9 @@ static struct dmi_system_id __initdata ec_dmi_table[] > = { > ec_validate_ecdt, "ASUS hardware", { > DMI_MATCH(DMI_SYS_VENDOR, "ASUSTek Computer Inc."), > DMI_MATCH(DMI_PRODUCT_NAME, "L4R"),}, NULL}, > + { > + ec_clear_on_resume, "Samsung hardware", { > + DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD.")}, NULL}, > {}, > }; > > -- > 1.8.3.2 > >
On Fri, 2014-03-28 at 15:38 +0100, D. G. Jansen wrote: > Dear Kamal, > > > please wait for now as a slight regression was reported for one system > and the patch will be changed. > Ok, I have dropped this one from the 3.8-stable queue. Thanks very much, Dennis. -Kamal > Best, > > > Dennis > > Am Freitag, 28. März 2014 schrieb Kamal Mostafa : > This is a note to let you know that I have just added a patch > titled > > ACPI / EC: Clear stale EC events on Samsung systems > > to the linux-3.8.y-queue branch of the 3.8.y.z extended stable > tree > which can be found at: > > http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.8.y-queue > > This patch is scheduled to be released in version 3.8.13.21. > > If you, or anyone else, feels it should not be added to this > tree, please > reply to this email. > > For more information about the 3.8.y.z tree, see > https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable > > Thanks. > -Kamal > > ------ > > From 1393f3f58ecb25d9ce797d30baf41eeccf975516 Mon Sep 17 > 00:00:00 2001 > From: Kieran Clancy <clancy.kieran@gmail.com> > Date: Sat, 1 Mar 2014 00:42:28 +1030 > Subject: ACPI / EC: Clear stale EC events on Samsung systems > > commit ad332c8a45330d170bb38b95209de449b31cd1b4 upstream. > > A number of Samsung notebooks > (530Uxx/535Uxx/540Uxx/550Pxx/900Xxx/etc) > continue to log events during sleep (lid open/close, AC > plug/unplug, > battery level change), which accumulate in the EC until a > buffer fills. > After the buffer is full (tests suggest it holds 8 events), > GPEs stop > being triggered for new events. This state persists on wake or > even on > power cycle, and prevents new events from being registered > until the EC > is manually polled. > > This is the root cause of a number of bugs, including AC not > being > detected properly, lid close not triggering suspend, and low > ambient > light not triggering the keyboard backlight. The bug also > seemed to be > responsible for performance issues on at least one user's > machine. > > Juan Manuel Cabo found the cause of bug and the workaround of > polling > the EC manually on wake. > > The loop which clears the stale events is based on an earlier > patch by > Lan Tianyu (see referenced attachment). > > This patch: > - Adds a function acpi_ec_clear() which polls the EC for > stale _Q > events at most ACPI_EC_CLEAR_MAX (currently 100) times. A > warning is > logged if this limit is reached. > - Adds a flag EC_FLAGS_CLEAR_ON_RESUME which is set to 1 if > the DMI > system vendor is Samsung. This check could be replaced by > several > more specific DMI vendor/product pairs, but it's likely > that the bug > affects more Samsung products than just the five series > mentioned > above. Further, it should not be harmful to run > acpi_ec_clear() on > systems without the bug; it will return immediately after > finding no > data waiting. > - Runs acpi_ec_clear() on initialisation (boot), from > acpi_ec_add() > - Runs acpi_ec_clear() on wake, from > acpi_ec_unblock_transactions() > > References: https://bugzilla.kernel.org/show_bug.cgi?id=44161 > References: https://bugzilla.kernel.org/show_bug.cgi?id=45461 > References: https://bugzilla.kernel.org/show_bug.cgi?id=57271 > References: > https://bugzilla.kernel.org/attachment.cgi?id=126801 > Suggested-by: Juan Manuel Cabo <juanmanuel.cabo@gmail.com> > Signed-off-by: Kieran Clancy <clancy.kieran@gmail.com> > Reviewed-by: Lan Tianyu <tianyu.lan@intel.com> > Reviewed-by: Dennis Jansen <dennis.jansen@web.de> > Tested-by: Kieran Clancy <clancy.kieran@gmail.com> > Tested-by: Juan Manuel Cabo <juanmanuel.cabo@gmail.com> > Tested-by: Dennis Jansen <dennis.jansen@web.de> > Tested-by: Maurizio D'Addona <mauritiusdadd@gmail.com> > Tested-by: San Zamoyski <san@plusnet.pl> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > [ kamal: backport to 3.8 (context) ] > Signed-off-by: Kamal Mostafa <kamal@canonical.com> > --- > drivers/acpi/ec.c | 64 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 64 insertions(+) > > diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c > index 97fcfb0..092f862 100644 > --- a/drivers/acpi/ec.c > +++ b/drivers/acpi/ec.c > @@ -70,6 +70,8 @@ enum ec_command { > #define ACPI_EC_DELAY 500 /* Wait 500ms max. > during EC ops */ > #define ACPI_EC_UDELAY_GLK 1000 /* Wait 1ms max. to > get global lock */ > #define ACPI_EC_MSI_UDELAY 550 /* Wait 550us for MSI > EC */ > +#define ACPI_EC_CLEAR_MAX 100 /* Maximum number of > events to query > + * when trying to > clear the EC */ > > enum { > EC_FLAGS_QUERY_PENDING, /* Query is pending */ > @@ -123,6 +125,7 @@ EXPORT_SYMBOL(first_ec); > static int EC_FLAGS_MSI; /* Out-of-spec MSI controller */ > static int EC_FLAGS_VALIDATE_ECDT; /* ASUStec ECDTs need to > be validated */ > static int EC_FLAGS_SKIP_DSDT_SCAN; /* Not all BIOS survive > early DSDT scan */ > +static int EC_FLAGS_CLEAR_ON_RESUME; /* Needs acpi_ec_clear() > on boot/resume */ > > /* > -------------------------------------------------------------------------- > Transaction Management > @@ -468,6 +471,29 @@ acpi_handle ec_get_handle(void) > > EXPORT_SYMBOL(ec_get_handle); > > +static int acpi_ec_query_unlocked(struct acpi_ec *ec, u8 > *data); > + > +/* > + * Clears stale _Q events that might have accumulated in the > EC. > + * Run with locked ec mutex. > + */ > +static void acpi_ec_clear(struct acpi_ec *ec) > +{ > + int i, status; > + u8 value = 0; > + > + for (i = 0; i < ACPI_EC_CLEAR_MAX; i++) { > + status = acpi_ec_query_unlocked(ec, &value); > + if (status || !value) > + break; > + } > + > + if (unlikely(i == ACPI_EC_CLEAR_MAX)) > + pr_warn("Warning: Maximum of %d stale EC > events cleared\n", i); > + else > + pr_info("%d stale EC events cleared\n", i); > +} > + > void acpi_ec_block_transactions(void) > { > struct acpi_ec *ec = first_ec; > @@ -491,6 +517,10 @@ void acpi_ec_unblock_transactions(void) > mutex_lock(&ec->mutex); > /* Allow transactions to be carried out again */ > clear_bit(EC_FLAGS_BLOCKED, &ec->flags); > + > + if (EC_FLAGS_CLEAR_ON_RESUME) > + acpi_ec_clear(ec); > + > mutex_unlock(&ec->mutex); > } > > @@ -848,6 +878,13 @@ static int acpi_ec_add(struct acpi_device > *device) > > /* EC is fully operational, allow queries */ > clear_bit(EC_FLAGS_QUERY_PENDING, &ec->flags); > + > + /* Clear stale _Q events if hardware might require > that */ > + if (EC_FLAGS_CLEAR_ON_RESUME) { > + mutex_lock(&ec->mutex); > + acpi_ec_clear(ec); > + mutex_unlock(&ec->mutex); > + } > return ret; > } > > @@ -949,6 +986,30 @@ static int > ec_enlarge_storm_threshold(const struct dmi_system_id *id) > return 0; > } > > +/* > + * On some hardware it is necessary to clear events > accumulated by the EC during > + * sleep. These ECs stop reporting GPEs until they are > manually polled, if too > + * many events are accumulated. (e.g. Samsung Series 5/9 > notebooks) > + * > + * https://bugzilla.kernel.org/show_bug.cgi?id=44161 > + * > + * Ideally, the EC should also be instructed NOT to > accumulate events during > + * sleep (which Windows seems to do somehow), but the > interface to control this > + * behaviour is not known at this time. > + * > + * Models known to be affected are Samsung > 530Uxx/535Uxx/540Uxx/550Pxx/900Xxx, > + * however it is very likely that other Samsung models are > affected. > + * > + * On systems which don't accumulate _Q events during sleep, > this extra check > + * should be harmless. > + */ > +static int ec_clear_on_resume(const struct dmi_system_id *id) > +{ > + pr_debug("Detected system needing EC poll on > resume.\n"); > + EC_FLAGS_CLEAR_ON_RESUME = 1; > + return 0; > +} > + > static struct dmi_system_id __initdata ec_dmi_table[] = { > { > ec_skip_dsdt_scan, "Compal JFL92", { > @@ -992,6 +1053,9 @@ static struct dmi_system_id __initdata > ec_dmi_table[] = { > ec_validate_ecdt, "ASUS hardware", { > DMI_MATCH(DMI_SYS_VENDOR, "ASUSTek Computer Inc."), > DMI_MATCH(DMI_PRODUCT_NAME, "L4R"),}, NULL}, > + { > + ec_clear_on_resume, "Samsung hardware", { > + DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., > LTD.")}, NULL}, > {}, > }; > > -- > 1.8.3.2 > > > > -- > Dipl.-Jur. Dennis G. Jansen, LL.M. (Berkeley) > Tel. 0163 834 8558 | LinkedIn > Master of Laws, University of California, Berkeley, School of Law > ("Boalt Hall") > Certificate in Law and Technology, University of California, Berkeley, > School of Law ("Boalt Hall") > Certificate in Transnational Legal Studies, Georgetown University Law > Center > Certificate in Internationalization of Law, Free University Berlin > Diploma of Law, Free University Berlin >
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 97fcfb0..092f862 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -70,6 +70,8 @@ enum ec_command { #define ACPI_EC_DELAY 500 /* Wait 500ms max. during EC ops */ #define ACPI_EC_UDELAY_GLK 1000 /* Wait 1ms max. to get global lock */ #define ACPI_EC_MSI_UDELAY 550 /* Wait 550us for MSI EC */ +#define ACPI_EC_CLEAR_MAX 100 /* Maximum number of events to query + * when trying to clear the EC */ enum { EC_FLAGS_QUERY_PENDING, /* Query is pending */ @@ -123,6 +125,7 @@ EXPORT_SYMBOL(first_ec); static int EC_FLAGS_MSI; /* Out-of-spec MSI controller */ static int EC_FLAGS_VALIDATE_ECDT; /* ASUStec ECDTs need to be validated */ static int EC_FLAGS_SKIP_DSDT_SCAN; /* Not all BIOS survive early DSDT scan */ +static int EC_FLAGS_CLEAR_ON_RESUME; /* Needs acpi_ec_clear() on boot/resume */ /* -------------------------------------------------------------------------- Transaction Management @@ -468,6 +471,29 @@ acpi_handle ec_get_handle(void) EXPORT_SYMBOL(ec_get_handle); +static int acpi_ec_query_unlocked(struct acpi_ec *ec, u8 *data); + +/* + * Clears stale _Q events that might have accumulated in the EC. + * Run with locked ec mutex. + */ +static void acpi_ec_clear(struct acpi_ec *ec) +{ + int i, status; + u8 value = 0; + + for (i = 0; i < ACPI_EC_CLEAR_MAX; i++) { + status = acpi_ec_query_unlocked(ec, &value); + if (status || !value) + break; + } + + if (unlikely(i == ACPI_EC_CLEAR_MAX)) + pr_warn("Warning: Maximum of %d stale EC events cleared\n", i); + else + pr_info("%d stale EC events cleared\n", i); +} + void acpi_ec_block_transactions(void) { struct acpi_ec *ec = first_ec; @@ -491,6 +517,10 @@ void acpi_ec_unblock_transactions(void) mutex_lock(&ec->mutex); /* Allow transactions to be carried out again */ clear_bit(EC_FLAGS_BLOCKED, &ec->flags); + + if (EC_FLAGS_CLEAR_ON_RESUME) + acpi_ec_clear(ec); + mutex_unlock(&ec->mutex); } @@ -848,6 +878,13 @@ static int acpi_ec_add(struct acpi_device *device) /* EC is fully operational, allow queries */ clear_bit(EC_FLAGS_QUERY_PENDING, &ec->flags); + + /* Clear stale _Q events if hardware might require that */ + if (EC_FLAGS_CLEAR_ON_RESUME) { + mutex_lock(&ec->mutex); + acpi_ec_clear(ec); + mutex_unlock(&ec->mutex); + } return ret; } @@ -949,6 +986,30 @@ static int ec_enlarge_storm_threshold(const struct dmi_system_id *id) return 0; } +/* + * On some hardware it is necessary to clear events accumulated by the EC during + * sleep. These ECs stop reporting GPEs until they are manually polled, if too + * many events are accumulated. (e.g. Samsung Series 5/9 notebooks) + * + * https://bugzilla.kernel.org/show_bug.cgi?id=44161 + * + * Ideally, the EC should also be instructed NOT to accumulate events during + * sleep (which Windows seems to do somehow), but the interface to control this + * behaviour is not known at this time. + * + * Models known to be affected are Samsung 530Uxx/535Uxx/540Uxx/550Pxx/900Xxx, + * however it is very likely that other Samsung models are affected. + * + * On systems which don't accumulate _Q events during sleep, this extra check + * should be harmless. + */ +static int ec_clear_on_resume(const struct dmi_system_id *id) +{ + pr_debug("Detected system needing EC poll on resume.\n"); + EC_FLAGS_CLEAR_ON_RESUME = 1; + return 0; +} + static struct dmi_system_id __initdata ec_dmi_table[] = { { ec_skip_dsdt_scan, "Compal JFL92", { @@ -992,6 +1053,9 @@ static struct dmi_system_id __initdata ec_dmi_table[] = { ec_validate_ecdt, "ASUS hardware", { DMI_MATCH(DMI_SYS_VENDOR, "ASUSTek Computer Inc."), DMI_MATCH(DMI_PRODUCT_NAME, "L4R"),}, NULL}, + { + ec_clear_on_resume, "Samsung hardware", { + DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD.")}, NULL}, {}, };