mkbp: Separate FIFO from the keyboard driver

Move protocol-related functionality out from the keyboard driver. This
change is required to allow passing button events via MKBP on devices with
non-MKBP keyboards. It reorganizes the code without changing the logic.

BUG=b:170966461
BRANCH=main,firmware-dedede-13606.B,firmware-volteer-13672.B-main
TEST=None

Signed-off-by: Boris Mittelberg <bmbm@google.com>
Change-Id: Ifb5b9d8e605f491313ee1dfe2c9950eb52152aa8
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2669355

1 files changed, 15 insertions, 222 deletions

diff --git a/common/keyboard_mkbp.c b/common/keyboard_mkbp.c
index 4cd92f7041..58a55477f0 100644
--- a/common/keyboard_mkbp.c
+++ b/common/keyboard_mkbp.c
@@ -23,6 +23,7 @@
 #include "keyboard_test.h"
 #include "lid_switch.h"
 #include "mkbp_event.h"
+#include "mkbp_fifo.h"
 #include "power_button.h"
 #include "system.h"
 #include "tablet_mode.h"
@@ -34,17 +35,6 @@
 #define CPUTS(outstr) cputs(CC_KEYBOARD, outstr)
 #define CPRINTS(format, args...) cprints(CC_KEYBOARD, format, ## args)
 
-/*
- * Common FIFO depth.  This needs to be big enough not to overflow if a
- * series of keys is pressed in rapid succession and the kernel is too busy
- * to read them out right away.
- *
- * RAM usage is (depth * #cols); A 16-entry FIFO will consume 16x13=208 bytes,
- * which is non-trivial but not horrible.
- */
-
-#define FIFO_DEPTH 16
-
 /* Changes to col,row here need to also be reflected in kernel.
  * drivers/input/mkbp.c ... see KEY_BATTERY.
  */
@@ -52,21 +42,6 @@
 #define BATTERY_KEY_ROW 7
 #define BATTERY_KEY_ROW_MASK BIT(BATTERY_KEY_ROW)
 
-static uint32_t fifo_start;	/* first entry */
-static uint32_t fifo_end;	/* last entry */
-static uint32_t fifo_entries;	/* number of existing entries */
-static struct ec_response_get_next_event fifo[FIFO_DEPTH];
-/*
- * Mutex for critical sections of mkbp_fifo_add(), which is called
- * from various tasks.
- */
-K_MUTEX_DEFINE(fifo_add_mutex);
-/*
- * Mutex for critical sections of fifo_remove(), which is called from the
- * hostcmd task and from keyboard_clear_buffer().
- */
-K_MUTEX_DEFINE(fifo_remove_mutex);
-
 /* Button and switch state. */
 static uint32_t mkbp_button_state;
 static uint32_t mkbp_switch_state;
@@ -97,172 +72,24 @@ static struct ec_mkbp_protocol_config config = {
 	.fifo_max_depth = FIFO_DEPTH,
 };
 
-static int get_data_size(enum ec_mkbp_event e)
-{
-	switch (e) {
-	case EC_MKBP_EVENT_KEY_MATRIX:
-		return keyboard_cols;
-
-#ifdef CONFIG_HOST_EVENT64
-	case EC_MKBP_EVENT_HOST_EVENT64:
-		return sizeof(uint64_t);
-#endif
-
-	case EC_MKBP_EVENT_HOST_EVENT:
-	case EC_MKBP_EVENT_BUTTON:
-	case EC_MKBP_EVENT_SWITCH:
-	case EC_MKBP_EVENT_SYSRQ:
-		return sizeof(uint32_t);
-	default:
-		/* For unknown types, say it's 0. */
-		return 0;
-	}
-}
-
-/**
- * Pop MKBP event data from FIFO
- *
- * @return EC_SUCCESS if entry popped, EC_ERROR_UNKNOWN if FIFO is empty
- */
-static int fifo_remove(uint8_t *buffp)
-{
-	int size;
-
-	mutex_lock(&fifo_remove_mutex);
-	if (!fifo_entries) {
-		/* no entry remaining in FIFO : return last known state */
-		int last = (fifo_start + FIFO_DEPTH - 1) % FIFO_DEPTH;
-
-		size = get_data_size(fifo[last].event_type);
-
-		memcpy(buffp, &fifo[last].data, size);
-		mutex_unlock(&fifo_remove_mutex);
-
-		/*
-		 * Bail out without changing any FIFO indices and let the
-		 * caller know something strange happened. The buffer will
-		 * will contain the last known state of the keyboard.
-		 */
-		return EC_ERROR_UNKNOWN;
-	}
-
-	/* Return just the event data. */
-	if (buffp) {
-		size = get_data_size(fifo[fifo_start].event_type);
-		/* skip over event_type. */
-		memcpy(buffp, &fifo[fifo_start].data, size);
-	}
-
-	fifo_start = (fifo_start + 1) % FIFO_DEPTH;
-	atomic_sub(&fifo_entries, 1);
-	mutex_unlock(&fifo_remove_mutex);
-
-	return EC_SUCCESS;
-}
-
 /*****************************************************************************/
 /* Interface */
 
 void keyboard_clear_buffer(void)
 {
-	int i, new_fifo_entries = 0;
-
-	CPRINTS("clear keyboard MKBP fifo");
-
-	/*
-	 * Order of these locks is important to prevent deadlock since
-	 * mkbp_fifo_add() may call fifo_remove().
-	 */
-	mutex_lock(&fifo_add_mutex);
-	mutex_lock(&fifo_remove_mutex);
-
-	/* Reset the end position */
-	fifo_end = fifo_start;
-
-	for (i = 0; i < fifo_entries; i++) {
-		int cur = (fifo_start + i) % FIFO_DEPTH;
-
-		/* Drop keyboard events */
-		if (fifo[cur].event_type == EC_MKBP_EVENT_KEY_MATRIX)
-			continue;
-
-		/* And move other events to the front */
-		memmove(&fifo[fifo_end], &fifo[cur], sizeof(fifo[cur]));
-		fifo_end = (fifo_end + 1) % FIFO_DEPTH;
-		++new_fifo_entries;
-	}
-	fifo_entries = new_fifo_entries;
-
-	mutex_unlock(&fifo_remove_mutex);
-	mutex_unlock(&fifo_add_mutex);
-}
-
-void mkbp_clear_fifo(void)
-{
-	int i;
-
-	CPRINTS("clear MKBP fifo");
-
-	/*
-	 * Order of these locks is important to prevent deadlock since
-	 * mkbp_fifo_add() may call fifo_remove().
-	 */
-	mutex_lock(&fifo_add_mutex);
-	mutex_lock(&fifo_remove_mutex);
-
-	fifo_start = 0;
-	fifo_end = 0;
-	/* This assignment is safe since both mutexes are held. */
-	fifo_entries = 0;
-	for (i = 0; i < FIFO_DEPTH; i++)
-		memset(&fifo[i], 0, sizeof(struct ec_response_get_next_event));
-
-	mutex_unlock(&fifo_remove_mutex);
-	mutex_unlock(&fifo_add_mutex);
-}
-
-test_mockable int keyboard_fifo_add(const uint8_t *buffp)
-{
-	return mkbp_fifo_add((uint8_t)EC_MKBP_EVENT_KEY_MATRIX, buffp);
+	mkbp_fifo_clear_keyboard();
 }
 
-test_mockable int mkbp_fifo_add(uint8_t event_type, const uint8_t *buffp)
+test_mockable int mkbp_keyboard_add(const uint8_t *buffp)
 {
-	uint8_t size;
-
 	/*
-	 * If the data is a keyboard matrix and the keyboard protocol is not
-	 * enabled, don't save the state to the FIFO or trigger an interrupt.
+	 * If the keyboard protocol is not enabled, don't save the state to
+	 * the FIFO or trigger an interrupt.
 	 */
-	if (!(config.flags & EC_MKBP_FLAGS_ENABLE) &&
-	    (event_type == EC_MKBP_EVENT_KEY_MATRIX))
+	if (!(config.flags & EC_MKBP_FLAGS_ENABLE))
 		return EC_SUCCESS;
 
-	mutex_lock(&fifo_add_mutex);
-	if (fifo_entries >= config.fifo_max_depth) {
-		mutex_unlock(&fifo_add_mutex);
-		CPRINTS("MKBP common FIFO depth %d reached",
-			config.fifo_max_depth);
-
-		return EC_ERROR_OVERFLOW;
-	}
-
-	size = get_data_size(event_type);
-	fifo[fifo_end].event_type = event_type;
-	memcpy(&fifo[fifo_end].data, buffp, size);
-	fifo_end = (fifo_end + 1) % FIFO_DEPTH;
-	atomic_add(&fifo_entries, 1);
-
-	/*
-	 * If our event didn't generate an interrupt then the host is still
-	 * asleep. In this case, we don't want to queue our event, except if
-	 * another event just woke the host (and wake is already in progress).
-	 */
-	if (!mkbp_send_event(event_type) && fifo_entries == 1)
-		fifo_remove(NULL);
-
-	mutex_unlock(&fifo_add_mutex);
-	return EC_SUCCESS;
+	return mkbp_fifo_add((uint8_t)EC_MKBP_EVENT_KEY_MATRIX, buffp);
 }
 
 void mkbp_update_switches(uint32_t sw, int state)
@@ -378,64 +205,28 @@ DECLARE_HOOK(HOOK_POWER_BUTTON_CHANGE, keyboard_power_button,
 	     HOOK_PRIO_DEFAULT);
 #endif /* defined(CONFIG_POWER_BUTTON) */
 
-static int get_next_event(uint8_t *out, enum ec_mkbp_event evt)
-{
-	uint8_t t = fifo[fifo_start].event_type;
-	uint8_t size;
-
-	if (!fifo_entries)
-		return -1;
-
-	/*
-	 * We need to peek at the next event to check that we were called with
-	 * the correct event.
-	 */
-	if (t != (uint8_t)evt) {
-		/*
-		 * We were called with the wrong event.  The next element in the
-		 * FIFO's event type doesn't match with what we were called
-		 * with.  Return an error that we're busy.  The caller will need
-		 * to call us with the correct event first.
-		 */
-		return -EC_ERROR_BUSY;
-	}
-
-	fifo_remove(out);
-
-	/* Keep sending events if FIFO is not empty */
-	if (fifo_entries)
-		mkbp_send_event(fifo[fifo_start].event_type);
-
-	/* Return the correct size of the data. */
-	size = get_data_size(t);
-	if (size)
-		return size;
-	else
-		return -EC_ERROR_UNKNOWN;
-}
-
 static int keyboard_get_next_event(uint8_t *out)
 {
-	return get_next_event(out, EC_MKBP_EVENT_KEY_MATRIX);
+	return mkbp_fifo_get_next_event(out, EC_MKBP_EVENT_KEY_MATRIX);
 }
 DECLARE_EVENT_SOURCE(EC_MKBP_EVENT_KEY_MATRIX, keyboard_get_next_event);
 
 static int button_get_next_event(uint8_t *out)
 {
-	return get_next_event(out, EC_MKBP_EVENT_BUTTON);
+	return mkbp_fifo_get_next_event(out, EC_MKBP_EVENT_BUTTON);
 }
 DECLARE_EVENT_SOURCE(EC_MKBP_EVENT_BUTTON, button_get_next_event);
 
 static int switch_get_next_event(uint8_t *out)
 {
-	return get_next_event(out, EC_MKBP_EVENT_SWITCH);
+	return mkbp_fifo_get_next_event(out, EC_MKBP_EVENT_SWITCH);
 }
 DECLARE_EVENT_SOURCE(EC_MKBP_EVENT_SWITCH, switch_get_next_event);
 
 #ifdef CONFIG_EMULATED_SYSRQ
 static int sysrq_get_next_event(uint8_t *out)
 {
-	return get_next_event(out, EC_MKBP_EVENT_SYSRQ);
+	return mkbp_fifo_get_next_event(out, EC_MKBP_EVENT_SYSRQ);
 }
 DECLARE_EVENT_SOURCE(EC_MKBP_EVENT_SYSRQ, sysrq_get_next_event);
 #endif
@@ -450,7 +241,7 @@ void keyboard_send_battery_key(void)
 
 	/* Add to FIFO only if AP is on or else it will wake from suspend */
 	if (chipset_in_state(CHIPSET_STATE_ON))
-		keyboard_fifo_add(state);
+		mkbp_keyboard_add(state);
 }
 
 void clear_typematic_key(void)
@@ -591,7 +382,7 @@ static void simulate_key(int row, int col, int pressed)
 	if (pressed)
 		simulated_key[col] |= BIT(row);
 
-	keyboard_fifo_add(simulated_key);
+	mkbp_keyboard_add(simulated_key);
 }
 
 static int command_mkbp_keyboard_press(int argc, char **argv)
@@ -736,6 +527,8 @@ host_command_mkbp_set_config(struct host_cmd_handler_args *args)
 			    config.valid_mask & req->config.valid_mask,
 			    config.valid_flags & req->config.valid_flags);
 
+	mkbp_fifo_depth_update(config.fifo_max_depth);
+
 	return EC_RES_SUCCESS;
 }
 DECLARE_HOST_COMMAND(EC_CMD_MKBP_SET_CONFIG,