| Message ID | 1549001621-23355-1-git-send-email-rui.zhang@intel.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Rafael Wysocki |
| Headers | show |
| Series | Revert "ACPI / EC: Remove old CLEAR_ON_RESUME quirk" | expand |
On Friday, February 1, 2019 7:13:41 AM CET Zhang Rui wrote: > On some Samsung 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. > Thus the CLEAR_ON_RESUME quirk is introduced to send EC query commands > unconditionally after resume to clear all the EC query events on those > platforms. > > Later, commit 4c237371f290 ("ACPI / EC: Remove old CLEAR_ON_RESUME quirk") > removes the CLEAR_ON_RESUME quirk because we thought the new EC IRQ > polling logic should handle this case. > > Now it has been proved that the EC IRQ Polling logic does not fix the > issue actually because we got regression report on these Samsung > platforms after removing the quirk. > > Thus revert commit 4c237371f290 ("ACPI / EC: Remove old CLEAR_ON_RESUME > quirk") to introduce back the Samsung quirk in this patch. > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=44161 > Tested-by: Ortwin Glück <odi@odi.ch> > Tested-by: Francisco Cribari <cribari@gmail.com> > Tested-by: Balazs Varga <balazs4web@gmail.com> > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > --- > drivers/acpi/ec.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 53 insertions(+) > > diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c > index 9d66a47..49e16f0 100644 > --- a/drivers/acpi/ec.c > +++ b/drivers/acpi/ec.c > @@ -194,6 +194,7 @@ static struct workqueue_struct *ec_query_wq; > static int EC_FLAGS_QUERY_HANDSHAKE; /* Needs QR_EC issued when SCI_EVT set */ > static int EC_FLAGS_CORRECT_ECDT; /* Needs ECDT port address correction */ > static int EC_FLAGS_IGNORE_DSDT_GPE; /* Needs ECDT GPE as correction setting */ > +static int EC_FLAGS_CLEAR_ON_RESUME; /* Needs acpi_ec_clear() on boot/resume */ > > /* -------------------------------------------------------------------------- > * Logging/Debugging > @@ -499,6 +500,26 @@ static inline void __acpi_ec_disable_event(struct acpi_ec *ec) > ec_log_drv("event blocked"); > } > > +/* > + * Process _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(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); > +} > + > static void acpi_ec_enable_event(struct acpi_ec *ec) > { > unsigned long flags; > @@ -507,6 +528,10 @@ static void acpi_ec_enable_event(struct acpi_ec *ec) > if (acpi_ec_started(ec)) > __acpi_ec_enable_event(ec); > spin_unlock_irqrestore(&ec->lock, flags); > + > + /* Drain additional events if hardware requires that */ > + if (EC_FLAGS_CLEAR_ON_RESUME) > + acpi_ec_clear(ec); > } > > #ifdef CONFIG_PM_SLEEP > @@ -1821,6 +1846,31 @@ static int ec_flag_query_handshake(const struct dmi_system_id *id) > #endif > > /* > + * 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; > + ec_event_clearing = ACPI_EC_EVT_TIMING_STATUS; > + return 0; > +} > + > +/* > * Some ECDTs contain wrong register addresses. > * MSI MS-171F > * https://bugzilla.kernel.org/show_bug.cgi?id=12461 > @@ -1869,6 +1919,9 @@ static const struct dmi_system_id ec_dmi_table[] __initconst = { > ec_honor_ecdt_gpe, "ASUS X580VD", { > DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), > DMI_MATCH(DMI_PRODUCT_NAME, "X580VD"),}, NULL}, > + { > + ec_clear_on_resume, "Samsung hardware", { > + DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD.")}, NULL}, > {}, > }; > > Applied, thanks!
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 9d66a47..49e16f0 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -194,6 +194,7 @@ static struct workqueue_struct *ec_query_wq; static int EC_FLAGS_QUERY_HANDSHAKE; /* Needs QR_EC issued when SCI_EVT set */ static int EC_FLAGS_CORRECT_ECDT; /* Needs ECDT port address correction */ static int EC_FLAGS_IGNORE_DSDT_GPE; /* Needs ECDT GPE as correction setting */ +static int EC_FLAGS_CLEAR_ON_RESUME; /* Needs acpi_ec_clear() on boot/resume */ /* -------------------------------------------------------------------------- * Logging/Debugging @@ -499,6 +500,26 @@ static inline void __acpi_ec_disable_event(struct acpi_ec *ec) ec_log_drv("event blocked"); } +/* + * Process _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(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); +} + static void acpi_ec_enable_event(struct acpi_ec *ec) { unsigned long flags; @@ -507,6 +528,10 @@ static void acpi_ec_enable_event(struct acpi_ec *ec) if (acpi_ec_started(ec)) __acpi_ec_enable_event(ec); spin_unlock_irqrestore(&ec->lock, flags); + + /* Drain additional events if hardware requires that */ + if (EC_FLAGS_CLEAR_ON_RESUME) + acpi_ec_clear(ec); } #ifdef CONFIG_PM_SLEEP @@ -1821,6 +1846,31 @@ static int ec_flag_query_handshake(const struct dmi_system_id *id) #endif /* + * 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; + ec_event_clearing = ACPI_EC_EVT_TIMING_STATUS; + return 0; +} + +/* * Some ECDTs contain wrong register addresses. * MSI MS-171F * https://bugzilla.kernel.org/show_bug.cgi?id=12461 @@ -1869,6 +1919,9 @@ static const struct dmi_system_id ec_dmi_table[] __initconst = { ec_honor_ecdt_gpe, "ASUS X580VD", { DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), DMI_MATCH(DMI_PRODUCT_NAME, "X580VD"),}, NULL}, + { + ec_clear_on_resume, "Samsung hardware", { + DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD.")}, NULL}, {}, };
On some Samsung 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. Thus the CLEAR_ON_RESUME quirk is introduced to send EC query commands unconditionally after resume to clear all the EC query events on those platforms. Later, commit 4c237371f290 ("ACPI / EC: Remove old CLEAR_ON_RESUME quirk") removes the CLEAR_ON_RESUME quirk because we thought the new EC IRQ polling logic should handle this case. Now it has been proved that the EC IRQ Polling logic does not fix the issue actually because we got regression report on these Samsung platforms after removing the quirk. Thus revert commit 4c237371f290 ("ACPI / EC: Remove old CLEAR_ON_RESUME quirk") to introduce back the Samsung quirk in this patch. Link: https://bugzilla.kernel.org/show_bug.cgi?id=44161 Tested-by: Ortwin Glück <odi@odi.ch> Tested-by: Francisco Cribari <cribari@gmail.com> Tested-by: Balazs Varga <balazs4web@gmail.com> Signed-off-by: Zhang Rui <rui.zhang@intel.com> --- drivers/acpi/ec.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)