Date: Tue, 12 Oct 2010 20:56:55 +0300 From: Andriy Gapon <avg@freebsd.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r213737 - head/sys/dev/acpica Message-ID: <4CB4A167.5010105@freebsd.org> In-Reply-To: <201010121753.o9CHr1X3055607@svn.freebsd.org> References: <201010121753.o9CHr1X3055607@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
on 12/10/2010 20:53 Andriy Gapon said the following: > Author: avg > Date: Tue Oct 12 17:53:01 2010 > New Revision: 213737 > URL: http://svn.freebsd.org/changeset/base/213737 > > Log: > acpi_ec: changes in communication with hardware > > Short description of the changes: > - attempt to retry some commands for which it is possible (read, query) > - always make a short sleep before checking EC status in polled mode > - periodically poll EC status in interrupt mode > - change logic for detecting broken interrupt delivery and falling back > to polled mode > - check that EC is ready for input before starting a new command, wait > if necessary > > This commit is based on the original patch by David Naylor. > > PR: kern/150517 > Submitted by: David Naylor <naylor.b.david@gmail.com> There also should have been: Tested by: David Naylor <naylor.b.david@gmail.com>, kuba guzik <kuba.g4@gmail.com>, Nicolas <nicolas.alcouffe@orange.fr> > Reviewed by: jkim > MFC after: 3 weeks > > Modified: > head/sys/dev/acpica/acpi_ec.c > > Modified: head/sys/dev/acpica/acpi_ec.c > ============================================================================== > --- head/sys/dev/acpica/acpi_ec.c Tue Oct 12 17:40:45 2010 (r213736) > +++ head/sys/dev/acpica/acpi_ec.c Tue Oct 12 17:53:01 2010 (r213737) > @@ -153,7 +153,7 @@ struct acpi_ec_softc { > int ec_glkhandle; > int ec_burstactive; > int ec_sci_pend; > - u_int ec_gencount; > + volatile u_int ec_gencount; > int ec_suspending; > }; > > @@ -165,7 +165,7 @@ struct acpi_ec_softc { > #define EC_LOCK_TIMEOUT 1000 > > /* Default delay in microseconds between each run of the status polling loop. */ > -#define EC_POLL_DELAY 5 > +#define EC_POLL_DELAY 50 > > /* Total time in ms spent waiting for a response from EC. */ > #define EC_TIMEOUT 750 > @@ -599,12 +599,32 @@ acpi_ec_write_method(device_t dev, u_int > return (0); > } > > +static ACPI_STATUS > +EcCheckStatus(struct acpi_ec_softc *sc, const char *msg, EC_EVENT event) > +{ > + ACPI_STATUS status; > + EC_STATUS ec_status; > + > + status = AE_NO_HARDWARE_RESPONSE; > + ec_status = EC_GET_CSR(sc); > + if (sc->ec_burstactive && !(ec_status & EC_FLAG_BURST_MODE)) { > + CTR1(KTR_ACPI, "ec burst disabled in waitevent (%s)", msg); > + sc->ec_burstactive = FALSE; > + } > + if (EVENT_READY(event, ec_status)) { > + CTR2(KTR_ACPI, "ec %s wait ready, status %#x", msg, ec_status); > + status = AE_OK; > + } > + return (status); > +} > + > static void > EcGpeQueryHandler(void *Context) > { > struct acpi_ec_softc *sc = (struct acpi_ec_softc *)Context; > UINT8 Data; > ACPI_STATUS Status; > + int retry; > char qxx[5]; > > ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__); > @@ -625,7 +645,16 @@ EcGpeQueryHandler(void *Context) > * that may arise from running the query from causing another query > * to be queued, we clear the pending flag only after running it. > */ > - Status = EcCommand(sc, EC_COMMAND_QUERY); > + for (retry = 0; retry < 2; retry++) { > + Status = EcCommand(sc, EC_COMMAND_QUERY); > + if (ACPI_SUCCESS(Status)) > + break; > + if (EcCheckStatus(sc, "retr_check", > + EC_EVENT_INPUT_BUFFER_EMPTY) == AE_OK) > + continue; > + else > + break; > + } > sc->ec_sci_pend = FALSE; > if (ACPI_FAILURE(Status)) { > EcUnlock(sc); > @@ -678,7 +707,7 @@ EcGpeHandler(void *Context) > * address and then data values.) > */ > atomic_add_int(&sc->ec_gencount, 1); > - wakeup(&sc->ec_gencount); > + wakeup(&sc); > > /* > * If the EC_SCI bit of the status register is set, queue a query handler. > @@ -789,68 +818,27 @@ EcSpaceHandler(UINT32 Function, ACPI_PHY > } > > static ACPI_STATUS > -EcCheckStatus(struct acpi_ec_softc *sc, const char *msg, EC_EVENT event) > -{ > - ACPI_STATUS status; > - EC_STATUS ec_status; > - > - status = AE_NO_HARDWARE_RESPONSE; > - ec_status = EC_GET_CSR(sc); > - if (sc->ec_burstactive && !(ec_status & EC_FLAG_BURST_MODE)) { > - CTR1(KTR_ACPI, "ec burst disabled in waitevent (%s)", msg); > - sc->ec_burstactive = FALSE; > - } > - if (EVENT_READY(event, ec_status)) { > - CTR2(KTR_ACPI, "ec %s wait ready, status %#x", msg, ec_status); > - status = AE_OK; > - } > - return (status); > -} > - > -static ACPI_STATUS > EcWaitEvent(struct acpi_ec_softc *sc, EC_EVENT Event, u_int gen_count) > { > + static int no_intr = 0; > ACPI_STATUS Status; > - int count, i, slp_ival; > + int count, i, need_poll, slp_ival; > > ACPI_SERIAL_ASSERT(ec); > Status = AE_NO_HARDWARE_RESPONSE; > - int need_poll = cold || rebooting || ec_polled_mode || sc->ec_suspending; > - /* > - * The main CPU should be much faster than the EC. So the status should > - * be "not ready" when we start waiting. But if the main CPU is really > - * slow, it's possible we see the current "ready" response. Since that > - * can't be distinguished from the previous response in polled mode, > - * this is a potential issue. We really should have interrupts enabled > - * during boot so there is no ambiguity in polled mode. > - * > - * If this occurs, we add an additional delay before actually entering > - * the status checking loop, hopefully to allow the EC to go to work > - * and produce a non-stale status. > - */ > - if (need_poll) { > - static int once; > - > - if (EcCheckStatus(sc, "pre-check", Event) == AE_OK) { > - if (!once) { > - device_printf(sc->ec_dev, > - "warning: EC done before starting event wait\n"); > - once = 1; > - } > - AcpiOsStall(10); > - } > - } > + need_poll = cold || rebooting || ec_polled_mode || sc->ec_suspending; > > /* Wait for event by polling or GPE (interrupt). */ > if (need_poll) { > count = (ec_timeout * 1000) / EC_POLL_DELAY; > if (count == 0) > count = 1; > + DELAY(10); > for (i = 0; i < count; i++) { > Status = EcCheckStatus(sc, "poll", Event); > if (Status == AE_OK) > break; > - AcpiOsStall(EC_POLL_DELAY); > + DELAY(EC_POLL_DELAY); > } > } else { > slp_ival = hz / 1000; > @@ -869,34 +857,37 @@ EcWaitEvent(struct acpi_ec_softc *sc, EC > * EC query). > */ > for (i = 0; i < count; i++) { > - if (gen_count != sc->ec_gencount) { > - /* > - * Record new generation count. It's possible the GPE was > - * just to notify us that a query is needed and we need to > - * wait for a second GPE to signal the completion of the > - * event we are actually waiting for. > - */ > - gen_count = sc->ec_gencount; > - Status = EcCheckStatus(sc, "sleep", Event); > - if (Status == AE_OK) > - break; > + if (gen_count == sc->ec_gencount) > + tsleep(&sc, 0, "ecgpe", slp_ival); > + /* > + * Record new generation count. It's possible the GPE was > + * just to notify us that a query is needed and we need to > + * wait for a second GPE to signal the completion of the > + * event we are actually waiting for. > + */ > + Status = EcCheckStatus(sc, "sleep", Event); > + if (Status == AE_OK) { > + if (gen_count == sc->ec_gencount) > + no_intr++; > + else > + no_intr = 0; > + break; > } > - tsleep(&sc->ec_gencount, PZERO, "ecgpe", slp_ival); > + gen_count = sc->ec_gencount; > } > > /* > * We finished waiting for the GPE and it never arrived. Try to > * read the register once and trust whatever value we got. This is > - * the best we can do at this point. Then, force polled mode on > - * since this system doesn't appear to generate GPEs. > + * the best we can do at this point. > */ > - if (Status != AE_OK) { > + if (Status != AE_OK) > Status = EcCheckStatus(sc, "sleep_end", Event); > - device_printf(sc->ec_dev, > - "wait timed out (%sresponse), forcing polled mode\n", > - Status == AE_OK ? "" : "no "); > - ec_polled_mode = TRUE; > - } > + } > + if (!need_poll && no_intr > 10) { > + device_printf(sc->ec_dev, > + "not getting interrupts, switched to polled mode\n"); > + ec_polled_mode = 1; > } > if (Status != AE_OK) > CTR0(KTR_ACPI, "error: ec wait timed out"); > @@ -933,6 +924,14 @@ EcCommand(struct acpi_ec_softc *sc, EC_C > return (AE_BAD_PARAMETER); > } > > + /* > + * Ensure empty input buffer before issuing command. > + * Use generation count of zero to force a quick check. > + */ > + status = EcWaitEvent(sc, EC_EVENT_INPUT_BUFFER_EMPTY, 0); > + if (ACPI_FAILURE(status)) > + return (status); > + > /* Run the command and wait for the chosen event. */ > CTR1(KTR_ACPI, "ec running command %#x", cmd); > gen_count = sc->ec_gencount; > @@ -955,24 +954,31 @@ EcRead(struct acpi_ec_softc *sc, UINT8 A > { > ACPI_STATUS status; > u_int gen_count; > + int retry; > > ACPI_SERIAL_ASSERT(ec); > CTR1(KTR_ACPI, "ec read from %#x", Address); > > - status = EcCommand(sc, EC_COMMAND_READ); > - if (ACPI_FAILURE(status)) > - return (status); > + for (retry = 0; retry < 2; retry++) { > + status = EcCommand(sc, EC_COMMAND_READ); > + if (ACPI_FAILURE(status)) > + return (status); > > - gen_count = sc->ec_gencount; > - EC_SET_DATA(sc, Address); > - status = EcWaitEvent(sc, EC_EVENT_OUTPUT_BUFFER_FULL, gen_count); > - if (ACPI_FAILURE(status)) { > - device_printf(sc->ec_dev, "EcRead: failed waiting to get data\n"); > - return (status); > + gen_count = sc->ec_gencount; > + EC_SET_DATA(sc, Address); > + status = EcWaitEvent(sc, EC_EVENT_OUTPUT_BUFFER_FULL, gen_count); > + if (ACPI_FAILURE(status)) { > + if (EcCheckStatus(sc, "retr_check", > + EC_EVENT_INPUT_BUFFER_EMPTY) == AE_OK) > + continue; > + else > + break; > + } > + *Data = EC_GET_DATA(sc); > + return (AE_OK); > } > - *Data = EC_GET_DATA(sc); > - > - return (AE_OK); > + device_printf(sc->ec_dev, "EcRead: failed waiting to get data\n"); > + return (status); > } > > static ACPI_STATUS > @@ -992,7 +998,7 @@ EcWrite(struct acpi_ec_softc *sc, UINT8 > EC_SET_DATA(sc, Address); > status = EcWaitEvent(sc, EC_EVENT_INPUT_BUFFER_EMPTY, gen_count); > if (ACPI_FAILURE(status)) { > - device_printf(sc->ec_dev, "EcRead: failed waiting for sent address\n"); > + device_printf(sc->ec_dev, "EcWrite: failed waiting for sent address\n"); > return (status); > } > -- Andriy Gapon
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4CB4A167.5010105>