From 6172ccc455dbdbbefa732269a7e1f4b830781d7d Mon Sep 17 00:00:00 2001 From: Fredrik Holmqvist Date: Sat, 1 Sep 2018 19:41:25 +0200 Subject: [PATCH] Embedded Controller, sync with FreeBSD * Tries to handle all pending events at once * Wait is now polling every mS while waiting --- .../bus_managers/acpi/EmbeddedController.cpp | 146 +++++++++--------- .../bus_managers/acpi/EmbeddedController.h | 8 +- 2 files changed, 74 insertions(+), 80 deletions(-) diff --git a/src/add-ons/kernel/bus_managers/acpi/EmbeddedController.cpp b/src/add-ons/kernel/bus_managers/acpi/EmbeddedController.cpp index 92dc164dac..ca13bf19c0 100644 --- a/src/add-ons/kernel/bus_managers/acpi/EmbeddedController.cpp +++ b/src/add-ons/kernel/bus_managers/acpi/EmbeddedController.cpp @@ -508,12 +508,8 @@ EcCheckStatus(struct acpi_ec_cookie* sc, const char* msg, EC_EVENT event) static void -EcGpeQueryHandler(void* context) +EcGpeQueryHandlerSub(struct acpi_ec_cookie *sc) { - struct acpi_ec_cookie* sc = (struct acpi_ec_cookie*)context; - - ASSERT(context != NULL); - // Serialize user access with EcSpaceHandler(). status_t status = EcLock(sc); if (status != B_OK) { @@ -526,7 +522,6 @@ EcGpeQueryHandler(void* context) // interrupt source since we are edge-triggered. To prevent the GPE // that may arise from running the query from causing another query // to be queued, we clear the pending flag only after running it. - int sci_enqueued = sc->ec_sci_pending; acpi_status acpi_status = AE_ERROR; for (uint8 retry = 0; retry < 2; retry++) { acpi_status = EcCommand(sc, EC_COMMAND_QUERY); @@ -537,7 +532,6 @@ EcGpeQueryHandler(void* context) break; } - sc->ec_sci_pending = FALSE; if (acpi_status != AE_OK) { EcUnlock(sc); TRACE("GPE query failed.\n"); @@ -563,14 +557,28 @@ EcGpeQueryHandler(void* context) if (status != B_OK) { TRACE("evaluation of query method %s failed\n", qxx); } +} - // Reenable runtime GPE if its execution was deferred. - if (sci_enqueued) { - status = sc->ec_acpi_module->finish_gpe(sc->ec_gpehandle, sc->ec_gpebit); - if (status != B_OK) - ERROR("reenabling runtime GPE failed.\n"); - } +static void +EcGpeQueryHandler(void* context) +{ + struct acpi_ec_cookie* sc = (struct acpi_ec_cookie*)context; + int32 pending; + + ASSERT(context != NULL); + + do { + // Read the current pending count + pending = atomic_get(&sc->ec_sci_pending); + + // Call GPE handler function + EcGpeQueryHandlerSub(sc); + + // Try to reset the pending count to zero. If this fails we + // know another GPE event has occurred while handling the + // current GPE event and need to loop. + } while (atomic_test_and_set(&sc->ec_sci_pending, 0, pending)); } @@ -595,14 +603,14 @@ EcGpeHandler(acpi_handle gpeDevice, uint32 gpeNumber, void* context) // If the EC_SCI bit of the status register is set, queue a query handler. // It will run the query and _Qxx method later, under the lock. EC_STATUS ecStatus = EC_GET_CSR(sc); - if ((ecStatus & EC_EVENT_SCI) && !sc->ec_sci_pending) { + if ((ecStatus & EC_EVENT_SCI) && atomic_add(&sc->ec_sci_pending, 1) == 0) { TRACE("gpe queueing query handler\n"); acpi_status status = AcpiOsExecute(OSL_GPE_HANDLER, EcGpeQueryHandler, context); - if (status == AE_OK) - sc->ec_sci_pending = TRUE; - else + if (status != AE_OK) { dprintf("EcGpeHandler: queuing GPE query handler failed\n"); + atomic_add(&sc->ec_sci_pending, -1); + } } return ACPI_REENABLE_GPE; } @@ -630,7 +638,8 @@ EcSpaceHandler(uint32 function, acpi_physical_address address, uint32 width, TRACE("enter EcSpaceHandler\n"); struct acpi_ec_cookie* sc = (struct acpi_ec_cookie*)context; - if (function != ACPI_READ && function != ACPI_WRITE) return AE_BAD_PARAMETER; + if (function != ACPI_READ && function != ACPI_WRITE) + return AE_BAD_PARAMETER; if (width % 8 != 0 || value == NULL || context == NULL) return AE_BAD_PARAMETER; if (address + width / 8 > 256) @@ -641,7 +650,8 @@ EcSpaceHandler(uint32 function, acpi_physical_address address, uint32 width, // (Not sure if it's needed) if (gKernelStartup || gKernelShutdown || sc->ec_suspending) { - if ((EC_GET_CSR(sc) & EC_EVENT_SCI)) { + if ((EC_GET_CSR(sc) & EC_EVENT_SCI) && + atomic_add(&sc->ec_sci_pending, 1) == 0) { //CTR0(KTR_ACPI, "ec running gpe handler directly"); EcGpeQueryHandler(sc); } @@ -662,7 +672,7 @@ EcSpaceHandler(uint32 function, acpi_physical_address address, uint32 width, } // Perform the transaction(s), based on width. - acpi_physical_address ecAddr = address; + ACPI_PHYSICAL_ADDRESS ecAddr = address; uint8* ecData = (uint8 *) value; if (function == ACPI_READ) *value = 0; @@ -695,39 +705,19 @@ EcSpaceHandler(uint32 function, acpi_physical_address address, uint32 width, static acpi_status EcWaitEvent(struct acpi_ec_cookie* sc, EC_EVENT event, int32 generationCount) { + static int32 noIntr = 0; acpi_status status = AE_NO_HARDWARE_RESPONSE; int32 count, i; - // int need_poll = cold || rebooting || ec_polled_mode || sc->ec_suspending; - int needPoll = ec_polled_mode || sc->ec_suspending || gKernelStartup || gKernelShutdown; - - // Wait for event by polling or GPE (interrupt). - // 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 (needPoll) { - static int once; - - if (EcCheckStatus(sc, "pre-check", event) == B_OK) { - if (!once) { - TRACE("warning: EC done before starting event wait\n"); - once = 1; - } - spin(10); - } - } + int needPoll = ec_polled_mode || sc->ec_suspending + || gKernelStartup || gKernelShutdown; // Wait for event by polling or GPE (interrupt). if (needPoll) { count = (ec_timeout * 1000) / EC_POLL_DELAY; if (count == 0) count = 1; + spin(10); for (i = 0; i < count; i++) { status = EcCheckStatus(sc, "poll", event); if (status == AE_OK) @@ -735,40 +725,45 @@ EcWaitEvent(struct acpi_ec_cookie* sc, EC_EVENT event, int32 generationCount) spin(EC_POLL_DELAY); } } else { - bigtime_t sleepInterval = system_time() + ec_timeout * 1000; - // Wait for the GPE to signal the status changed, checking the // status register each time we get one. It's possible to get a // GPE for an event we're not interested in here (i.e., SCI for // EC query). status_t waitStatus = B_NO_ERROR; - while (waitStatus != B_TIMED_OUT) { - if (generationCount != 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. - generationCount = sc->ec_gencount; - status = EcCheckStatus(sc, "sleep", event); - if (status == AE_OK) - break; + for (i = 0; i < ec_timeout; i++) { + if (generationCount == sc->ec_gencount) { + waitStatus = + sc->ec_condition_var.Wait(B_RELATIVE_TIMEOUT, 1000); } - waitStatus = sc->ec_condition_var.Wait(B_ABSOLUTE_TIMEOUT, - sleepInterval); + /* + * 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 (generationCount == sc->ec_gencount) + noIntr++; + else + noIntr = 0; + break; + } + generationCount = 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. - // since this system doesn't appear to generate GPEs. - if (status != AE_OK) { + /* + * 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. + */ + if (status != AE_OK) status = EcCheckStatus(sc, "sleep_end", event); - TRACE("wait timed out (%sresponse), forcing polled mode\n", - status == AE_OK ? "" : "no "); - ec_polled_mode = TRUE; - } } - + if (!needPoll && noIntr > 10) { + TRACE("not getting interrupts, switched to polled mode\n"); + ec_polled_mode = true; + } if (status != AE_OK) TRACE("error: ec wait timed out\n"); @@ -840,15 +835,14 @@ EcRead(struct acpi_ec_cookie* sc, uint8 address, uint8* readData) int32 generationCount = sc->ec_gencount; EC_SET_DATA(sc, address); status = EcWaitEvent(sc, EC_EVENT_OUTPUT_BUFFER_FULL, generationCount); - if (status != AE_OK) { - if (EcCheckStatus(sc, "retr_check", - EC_EVENT_INPUT_BUFFER_EMPTY) == AE_OK) - continue; - else - break; + if (status == AE_OK) { + *readData = EC_GET_DATA(sc); + return AE_OK; + } + if (EcCheckStatus(sc, "retr_check", EC_EVENT_INPUT_BUFFER_EMPTY) + != AE_OK) { + break; } - *readData = EC_GET_DATA(sc); - return AE_OK; } TRACE("EcRead: failed waiting to get data\n"); diff --git a/src/add-ons/kernel/bus_managers/acpi/EmbeddedController.h b/src/add-ons/kernel/bus_managers/acpi/EmbeddedController.h index 38488c9a86..d0149d9c30 100644 --- a/src/add-ons/kernel/bus_managers/acpi/EmbeddedController.h +++ b/src/add-ons/kernel/bus_managers/acpi/EmbeddedController.h @@ -150,7 +150,7 @@ struct acpi_ec_cookie { uint32 ec_glkhandle; mutex ec_lock; int ec_burstactive; - int ec_sci_pending; + int32 ec_sci_pending; int32 ec_gencount; ConditionVariable ec_condition_var; int ec_suspending; @@ -165,7 +165,7 @@ struct acpi_ec_cookie { #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 @@ -178,8 +178,8 @@ struct acpi_ec_cookie { -static int ec_burst_mode = 1; -static int ec_polled_mode = 0; +static bool ec_burst_mode = true; +static bool ec_polled_mode = false; static int ec_timeout = EC_TIMEOUT;