On Saturday, March 01, 2014 12:42:28 AM Kieran Clancy wrote: > 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 > Signed-off-by: Kieran Clancy <clancy.kieran@xxxxxxxxx> > Suggested-by: Juan Manuel Cabo <juanmanuel.cabo@xxxxxxxxx> > Reviewed-by: Lan Tianyu <tianyu.lan@xxxxxxxxx> > Reviewed-by: Dennis Jansen <dennis.jansen@xxxxxx> > Tested-by: Kieran Clancy <clancy.kieran@xxxxxxxxx> > Tested-by: Juan Manuel Cabo <juanmanuel.cabo@xxxxxxxxx> > Tested-by: Dennis Jansen <dennis.jansen@xxxxxx> > Tested-by: Maurizio D'Addona <mauritiusdadd@xxxxxxxxx> > Tested-by: San Zamoyski <san@xxxxxxxxxx> Queued up for 3.15, thanks! > --- > > Changes in PATCH v2: > - Changed some of the 'Signed-off-by' lines to better reflect contributions, > as suggested by Rafael J. Wysocki <rjw@xxxxxxxxxxxxx>. > - Directly reference prior work by Lan Tianyu. > - Increase ACPI_EC_CLEAR_MAX to 100, after discussion with > Li Guang <lig.fnst@xxxxxxxxxxxxxx> and Juan Manuel Cabo. > - Made source comments more explicit, thanks to suggestions by Li Guang. > - Marked warning for hitting ACPI_EC_CLEAR_MAX as unlikely(), as suggested by > Dennis Jansen. > > drivers/acpi/ec.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 64 insertions(+) > > diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c > index 959d41a..d7d32c2 100644 > --- a/drivers/acpi/ec.c > +++ b/drivers/acpi/ec.c > @@ -67,6 +67,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 */ > @@ -116,6 +118,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 > @@ -440,6 +443,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; > @@ -463,6 +489,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); > } > > @@ -821,6 +851,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; > } > > @@ -922,6 +959,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 ec_dmi_table[] __initdata = { > { > ec_skip_dsdt_scan, "Compal JFL92", { > @@ -965,6 +1026,9 @@ static struct dmi_system_id ec_dmi_table[] __initdata = { > 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}, > {}, > }; > > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
