aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRafael J. Wysocki <rafael.j.wysocki@intel.com>2021-11-23 19:44:46 +0100
committerRafael J. Wysocki <rafael.j.wysocki@intel.com>2021-12-01 20:17:32 +0100
commitc33676aa48249b007d55198dc8348cd117e3d8cc (patch)
tree44d64b8bd5081c87832b94440b323cf5ea184a22
parentACPI: EC: Avoid queuing unnecessary work in acpi_ec_submit_event() (diff)
downloadlinux-dev-c33676aa48249b007d55198dc8348cd117e3d8cc.tar.xz
linux-dev-c33676aa48249b007d55198dc8348cd117e3d8cc.zip
ACPI: EC: Make the event work state machine visible
The EC driver uses a relatively simple state machine for the event work handling, but it is not really straightforward to figure out. The states are as follows: "Ready": The event handling work can be submitted. In this state, the EC_FLAGS_QUERY_PENDING flag is clear. "In progress": The event handling work is pending or is being processed. It cannot be submitted again. In ths state, the EC_FLAGS_QUERY_PENDING flag is set and both the events_to_process count is nonzero and the EC_FLAGS_QUERY_GUARDING flag is clear. "Complete": The event handling work has been completed, but it still cannot be submitted again. In ths state, the EC_FLAGS_QUERY_PENDING flag is set and the events_to_process count is zero or the EC_FLAGS_QUERY_GUARDING flag is set. The state changes from "Ready" to "In progress" when new event is detected by advance_transaction() and acpi_ec_submit_event() is called by it. Next, the state can change from "In progress" directly to "Ready" in the following situations: * ec_event_clearing is ACPI_EC_EVT_TIMING_STATUS and the state of an ACPI_EC_COMMAND_QUERY transaction becomes ACPI_EC_COMMAND_POLL. * ec_event_clearing is ACPI_EC_EVT_TIMING_QUERY and the state of an ACPI_EC_COMMAND_QUERY transaction becomes ACPI_EC_COMMAND_COMPLETE. * ec_event_clearing is either ACPI_EC_EVT_TIMING_STATUS or ACPI_EC_EVT_TIMING_QUERY and there are no more events to process (ie. ec->events_to_process becomes 0). If ec_event_clearing is ACPI_EC_EVT_TIMING_EVENT, however, the state must change from "In progress" to "Complete" before it can change to "Ready". The changes from "In progress" to "Complete" in that case occur in the following situations: * The state of an ACPI_EC_COMMAND_QUERY transaction becomes ACPI_EC_COMMAND_COMPLETE. * There are no more events to process (ie. ec->events_to_process becomes 0). Finally, the state changes from "Complete" to "Ready" when advance_transaction() is invoked when the state is "Complete" and the state of the current transaction is not ACPI_EC_COMMAND_POLL. To make this state machine visible in the code, add a new event_state field to struct acpi_ec and modify the code to use it istead the EC_FLAGS_QUERY_PENDING and EC_FLAGS_QUERY_GUARDING flags. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
-rw-r--r--drivers/acpi/ec.c75
-rw-r--r--drivers/acpi/internal.h8
2 files changed, 52 insertions, 31 deletions
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index d578f41410ef..7d7bff4d2100 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -92,8 +92,6 @@ enum ec_command {
enum {
EC_FLAGS_QUERY_ENABLED, /* Query is enabled */
- EC_FLAGS_QUERY_PENDING, /* Query is pending */
- EC_FLAGS_QUERY_GUARDING, /* Guard for SCI_EVT check */
EC_FLAGS_EVENT_HANDLER_INSTALLED, /* Event handler installed */
EC_FLAGS_EC_HANDLER_INSTALLED, /* OpReg handler installed */
EC_FLAGS_QUERY_METHODS_INSTALLED, /* _Qxx handlers installed */
@@ -450,9 +448,11 @@ static bool acpi_ec_submit_event(struct acpi_ec *ec)
if (!acpi_ec_event_enabled(ec))
return false;
- if (!test_and_set_bit(EC_FLAGS_QUERY_PENDING, &ec->flags)) {
+ if (ec->event_state == EC_EVENT_READY) {
ec_dbg_evt("Command(%s) submitted/blocked",
acpi_ec_cmd_string(ACPI_EC_COMMAND_QUERY));
+
+ ec->event_state = EC_EVENT_IN_PROGRESS;
/*
* If events_to_process is greqter than 0 at this point, the
* while () loop in acpi_ec_event_handler() is still running
@@ -474,11 +474,19 @@ static bool acpi_ec_submit_event(struct acpi_ec *ec)
return true;
}
+static void acpi_ec_complete_event(struct acpi_ec *ec)
+{
+ if (ec->event_state == EC_EVENT_IN_PROGRESS)
+ ec->event_state = EC_EVENT_COMPLETE;
+}
+
static void acpi_ec_close_event(struct acpi_ec *ec)
{
- if (test_and_clear_bit(EC_FLAGS_QUERY_PENDING, &ec->flags))
+ if (ec->event_state != EC_EVENT_READY)
ec_dbg_evt("Command(%s) unblocked",
acpi_ec_cmd_string(ACPI_EC_COMMAND_QUERY));
+
+ ec->event_state = EC_EVENT_READY;
acpi_ec_unmask_events(ec);
}
@@ -565,8 +573,8 @@ void acpi_ec_flush_work(void)
static bool acpi_ec_guard_event(struct acpi_ec *ec)
{
- bool guarded = true;
unsigned long flags;
+ bool guarded;
spin_lock_irqsave(&ec->lock, flags);
/*
@@ -575,19 +583,15 @@ static bool acpi_ec_guard_event(struct acpi_ec *ec)
* evaluating _Qxx, so we need to re-check SCI_EVT after waiting an
* acceptable period.
*
- * The guarding period begins when EC_FLAGS_QUERY_PENDING is
- * flagged, which means SCI_EVT check has just been performed.
- * But if the current transaction is ACPI_EC_COMMAND_QUERY, the
- * guarding should have already been performed (via
- * EC_FLAGS_QUERY_GUARDING) and should not be applied so that the
- * ACPI_EC_COMMAND_QUERY transaction can be transitioned into
- * ACPI_EC_COMMAND_POLL state immediately.
+ * The guarding period is applicable if the event state is not
+ * EC_EVENT_READY, but otherwise if the current transaction is of the
+ * ACPI_EC_COMMAND_QUERY type, the guarding should have elapsed already
+ * and it should not be applied to let the transaction transition into
+ * the ACPI_EC_COMMAND_POLL state immediately.
*/
- if (ec_event_clearing == ACPI_EC_EVT_TIMING_STATUS ||
- ec_event_clearing == ACPI_EC_EVT_TIMING_QUERY ||
- !test_bit(EC_FLAGS_QUERY_PENDING, &ec->flags) ||
- (ec->curr && ec->curr->command == ACPI_EC_COMMAND_QUERY))
- guarded = false;
+ guarded = ec_event_clearing == ACPI_EC_EVT_TIMING_EVENT &&
+ ec->event_state != EC_EVENT_READY &&
+ (!ec->curr || ec->curr->command != ACPI_EC_COMMAND_QUERY);
spin_unlock_irqrestore(&ec->lock, flags);
return guarded;
}
@@ -619,16 +623,26 @@ static int ec_transaction_completed(struct acpi_ec *ec)
static inline void ec_transaction_transition(struct acpi_ec *ec, unsigned long flag)
{
ec->curr->flags |= flag;
- if (ec->curr->command == ACPI_EC_COMMAND_QUERY) {
- if (ec_event_clearing == ACPI_EC_EVT_TIMING_STATUS &&
- flag == ACPI_EC_COMMAND_POLL)
+
+ if (ec->curr->command != ACPI_EC_COMMAND_QUERY)
+ return;
+
+ switch (ec_event_clearing) {
+ case ACPI_EC_EVT_TIMING_STATUS:
+ if (flag == ACPI_EC_COMMAND_POLL)
acpi_ec_close_event(ec);
- if (ec_event_clearing == ACPI_EC_EVT_TIMING_QUERY &&
- flag == ACPI_EC_COMMAND_COMPLETE)
+
+ return;
+
+ case ACPI_EC_EVT_TIMING_QUERY:
+ if (flag == ACPI_EC_COMMAND_COMPLETE)
acpi_ec_close_event(ec);
- if (ec_event_clearing == ACPI_EC_EVT_TIMING_EVENT &&
- flag == ACPI_EC_COMMAND_COMPLETE)
- set_bit(EC_FLAGS_QUERY_GUARDING, &ec->flags);
+
+ return;
+
+ case ACPI_EC_EVT_TIMING_EVENT:
+ if (flag == ACPI_EC_COMMAND_COMPLETE)
+ acpi_ec_complete_event(ec);
}
}
@@ -674,11 +688,9 @@ static bool advance_transaction(struct acpi_ec *ec, bool interrupt)
*/
if (!t || !(t->flags & ACPI_EC_COMMAND_POLL)) {
if (ec_event_clearing == ACPI_EC_EVT_TIMING_EVENT &&
- (!ec->events_to_process ||
- test_bit(EC_FLAGS_QUERY_GUARDING, &ec->flags))) {
- clear_bit(EC_FLAGS_QUERY_GUARDING, &ec->flags);
+ ec->event_state == EC_EVENT_COMPLETE)
acpi_ec_close_event(ec);
- }
+
if (!t)
goto out;
}
@@ -1246,8 +1258,9 @@ static void acpi_ec_event_handler(struct work_struct *work)
* event handling work again regardless of whether or not the query
* queued up above is processed successfully.
*/
- if (ec_event_clearing == ACPI_EC_EVT_TIMING_STATUS ||
- ec_event_clearing == ACPI_EC_EVT_TIMING_QUERY)
+ if (ec_event_clearing == ACPI_EC_EVT_TIMING_EVENT)
+ acpi_ec_complete_event(ec);
+ else
acpi_ec_close_event(ec);
spin_unlock_irq(&ec->lock);
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index de546be3bc6a..1db3a2f81763 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -166,6 +166,13 @@ static inline void acpi_early_processor_osc(void) {}
/* --------------------------------------------------------------------------
Embedded Controller
-------------------------------------------------------------------------- */
+
+enum acpi_ec_event_state {
+ EC_EVENT_READY = 0, /* Event work can be submitted */
+ EC_EVENT_IN_PROGRESS, /* Event work is pending or being processed */
+ EC_EVENT_COMPLETE, /* Event work processing has completed */
+};
+
struct acpi_ec {
acpi_handle handle;
int gpe;
@@ -182,6 +189,7 @@ struct acpi_ec {
spinlock_t lock;
struct work_struct work;
unsigned long timestamp;
+ enum acpi_ec_event_state event_state;
unsigned int events_to_process;
unsigned int events_in_progress;
unsigned int queries_in_progress;