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.da...@gmail.com>
There also should have been:
Tested by: David Naylor <naylor.b.da...@gmail.com>,
kuba guzik <kuba...@gmail.com>,
Nicolas <nicolas.alcou...@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
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
