g: try batching USB stream data under heavy load

USB TX data rate is much higher than UART RX, this results in USB
driver streaming data received from UART in smaller than max size
chunks, which in turn means that per byte overhead of shipping USB
packets to the host is not as low as it could have been.

This patch detects attempt to ship less than full chunk over USB
stream and instead of processing data immediately posts a deferred
function, which is supposed to triggers another send attempt in a few
milliseconds. If there is a high traffic on the stream, the queue
would have much more data after deferred interval ends.

The problem with the posted deferred function is the fact that it is
not guaranteed to run soon enough in case there are other deferred
functions waiting. To address this issue an additional check is being
introduced to make sure that the USB buffer does not overflow: if the
deferred function is posted, and the buffer is half full or more,
let's cancel the deferred function and process the stream right away.

If the deferred function gets to execute - there is a chance that a
UART and or USB interrupt comes while the deferred function is
running, which is likely to mess up USB controller settings by
tx_stream_handler(). To avoid these issues, interrupts are disabled
before the kicker function calls the handler.

Note that this optimization applies only to AP and EC console streams.

BRANCH=cr50, cr50-mp
BUG=b:38448364
TEST=two full chargen streams on an octopus device run indefinitely
     and don't seem to be interrupting even when some CLI command is
     ran on the Cr50 console or when an update is uploaded over USB or
     TPM

Change-Id: Id151c494967d1eb15d2af42acf8f2282966b5147
Signed-off-by: Vadim Bendebury <vbendeb@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1672209
Reviewed-by: Namyoon Woo <namyoon@chromium.org>
(cherry picked from commit 63f4b8c0fee087297286232e51c8e552e31b84fd)
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1684051
(cherry picked from commit 669e8f82e4d4e2c6dac1c6e272002359634305cb)
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1705740
(cherry picked from commit 23a2ef6e4ac6d14359b719344cebc69fc95d1fe2)

2 files changed, 143 insertions, 55 deletions

diff --git a/chip/g/usb-stream.c b/chip/g/usb-stream.c
index 7ab28c37a6..b929cf7d4e 100644
--- a/chip/g/usb-stream.c
+++ b/chip/g/usb-stream.c
@@ -4,6 +4,8 @@
  */
 
 #include "registers.h"
+#include "task.h"
+#include "timer.h"
 #include "usb-stream.h"
 
 /* Let the USB HW IN-to-host FIFO transmit some bytes */
@@ -128,79 +130,129 @@ static inline int tx_fifo_is_ready(struct usb_stream_config const *config)
 }
 
 /* Try to send some bytes to the host */
-void tx_stream_handler(struct usb_stream_config const *config)
+static void tx_stream_handler(struct usb_stream_config const *config)
 {
+	int len[MAX_IN_DESC];
 	size_t count;
+	size_t head;
 	struct queue const *tx_q = config->consumer.queue;
 
-	/* handle the completion of the previous transfer, if there was any. */
-	count = *(config->tx_handled);
-	if (count > 0) {
-		/*
-		 * Since tx completed, let's advance queue head  by the value of
-		 * 'count'.
-		 */
-		queue_advance_head(tx_q, count);
-		*(config->tx_handled) = 0;
-	}
-
 	/* setup to send bytes to the host */
 	count = MIN(queue_count(tx_q), config->tx_size);
-	if (count > 0) {
-		size_t head = tx_q->state->head & tx_q->buffer_units_mask;
-		int len[MAX_IN_DESC];
+	if (!count) {
+		/* Report USB TX transfer is not active any more. */
+		*config->tx_in_progress = 0;
+		return;
+	}
 
+	head = tx_q->state->head & tx_q->buffer_units_mask;
+
+	if (config->is_uart_console) {
+		if (!*config->kicker_running &&
+		    (count < config->tx_size)) {
 		/*
-		 * If queue units are not physically continuous, then
-		 * setup transfer in two USB endpoint descriptors.
+		 * Shipping less than full chunk (64 bytes) over usb is
+		 * wasteful in case there is a lot of data coming from the
+		 * stream source. Let's try collecting more bytes in case more
+		 * is coming.
 		 *
-		 *      buffer                         buffer + buffer_units
-		 *      |     tail                head |
-		 *      |     |                   |    |
-		 *      V     V                   V    V
-		 * tx_q |xxxxxx___________________xxxxx|
-		 *       <---->                   <--->
-		 *      len[1]                    len[0]
+		 * It takes 5.6 ms to transfer 64 bytes over UART at 115200
+		 * bps with one start and one stop bit. Let's set the deferred
+		 * function delay to 3 ms, it will take longer in reality as
+		 * background tasks will get a chance to run.
 		 */
-		len[0] = MIN(count, tx_q->buffer_units - head);
-		len[1] = count - len[0];
+			hook_call_deferred(config->tx_kicker, 3 * MSEC);
+			*config->kicker_running = 1;
+			return;
+		}
+
+		if (*config->kicker_running) {
+			*config->kicker_running = 0;
+			hook_call_deferred(config->tx_kicker, -1);
+		}
+	}
 
-		/*
-		 * Store the amount to advance head when the transfer is done.
-		 * Note: 'tx byte' field in the endpoint descriptor decreases to
-		 *       zero as data get transferred. Need to store the
-		 *       transfer size, which is 'count', aside into *config->
-		 *       tx_handlered.
-		 */
-		*(config->tx_handled) = count;
+	/*
+	 * If queue units are not physically continuous, then setup transfer
+	 * in two USB endpoint descriptors.
+	 *
+	 *      buffer                         buffer + buffer_units
+	 *      |     tail                head |
+	 *      |     |                   |    |
+	 *      V     V                   V    V
+	 * tx_q |xxxxxx___________________xxxxx|
+	 *       <---->                   <--->
+	 *      len[1]                    len[0]
+	 */
+	len[0] = MIN(count, tx_q->buffer_units - head);
+	len[1] = count - len[0];
 
-		/*
-		 * Setup the first endpoint descriptor with start memory address
-		 * No need to setup for the second endpoint, because it is
-		 * always the start address of the queue, and already setup in
-		 * usb_stream_reset().
-		 */
-		config->in_desc[0].addr = (void *)tx_q->buffer + head;
+	/*
+	 * Store the amount to advance head when the transfer is done.
+	 * Note: 'tx byte' field in the endpoint descriptor decreases to zero
+	 *       as data get transferred. Need to store the transfer size,
+	 *       which is 'count', aside into *config-> tx_handlered.
+	 */
+	*(config->tx_handled) = count;
 
-		/*
-		 * Enable USB transfer. usb_enable_tx() will setup the transfer
-		 * size in the first endpoint descriptor, and the second
-		 * descriptor as well if it is needed.
-		 */
-		usb_enable_tx(config, len);
-	} else {
-		/* USB TX transfer is not active. */
-		*config->tx_in_progress = 0;
-	}
+	/*
+	 * Setup the first endpoint descriptor with start memory address No
+	 * need to setup for the second endpoint, because it is always the
+	 * start address of the queue, and already setup in
+	 * usb_stream_reset().
+	 */
+	config->in_desc[0].addr = (void *)tx_q->buffer + head;
+
+	/*
+	 * Enable USB transfer. usb_enable_tx() will setup the transfer size
+	 * in the first endpoint descriptor, and the second descriptor as well
+	 * if it is needed.
+	 */
+	usb_enable_tx(config, len);
+}
+
+/*
+ * Deferred function which gets to run if a UART console does not supply
+ * enough data to fill a USB chunk (64 bytes).
+ */
+void tx_stream_kicker(struct usb_stream_config const *config)
+{
+	/*
+	 * By design this function must run on a task context, i.e. interrupts
+	 * are enabled.
+	 *
+	 * The not so elegant but simplest way to avoid concurrency issues
+	 * with the kicker function execution interrupted by a USB or UART
+	 * event is to invoke tx_stream_handler() with disabled interrupts.
+	 */
+	interrupt_disable();
+
+	if (*config->kicker_running)
+		tx_stream_handler(config);
+
+	interrupt_enable();
 }
 
 /* Tx/IN interrupt handler */
 void usb_stream_tx(struct usb_stream_config const *config)
 {
+	size_t *tx_handled;
+
 	/* Clear the Tx/IN interrupts */
 	GR_USB_DIEPINT(config->endpoint) = 0xffffffff;
 
-	/* Call the Tx FIFO handler */
+	/* Address of the size of the most recent chunk. */
+	tx_handled = config->tx_handled;
+
+	/*
+	 * Transfer completed, advance queue head by the number of bytes
+	 * transmitted in the most recent chunk.
+	 */
+	queue_advance_head(config->consumer.queue, *tx_handled);
+
+	*tx_handled = 0;
+
+	/* See if there is more to transmit. */
 	tx_stream_handler(config);
 }
 
@@ -261,8 +313,24 @@ static void usb_written(struct consumer const *consumer, size_t count)
 		DOWNCAST(consumer, struct usb_stream_config, consumer);
 
 	/* USB TX transfer is active. No need to activate it. */
-	if (*config->tx_in_progress)
-		return;
+	if (*config->tx_in_progress) {
+		struct queue const *tx_q;
+
+		if (!*config->kicker_running)
+			return;
+
+		/*
+		 * If kicker is running for too long and we already have a
+		 * certain amount of data accumulated in the buffer, let's
+		 * proceed even before the kicker had a chance to kick in.
+		 */
+		tx_q = config->consumer.queue;
+		if (queue_count(tx_q) < tx_q->buffer_units_mask)
+			return;
+
+		hook_call_deferred(config->tx_kicker, -1);
+		*config->kicker_running = 0;
+	}
 
 	/*
 	 * if USB Endpoint has not been initialized nor in ready status,
diff --git a/chip/g/usb-stream.h b/chip/g/usb-stream.h
index a0c580acea..6e01c3341c 100644
--- a/chip/g/usb-stream.h
+++ b/chip/g/usb-stream.h
@@ -27,15 +27,18 @@ struct usb_stream_config {
 	/*
 	 * Endpoint index, and pointers to the USB packet RAM buffers.
 	 */
-	int endpoint;
+	uint16_t endpoint;
+	uint16_t is_uart_console;
 
 	/* USB TX transfer is in progress */
 	uint8_t *tx_in_progress;
+	uint8_t *kicker_running;
 
 	/*
 	 * Deferred function to call to handle USB and Queue request.
 	 */
 	const struct deferred_data *deferred_rx;
+	const struct deferred_data *tx_kicker;
 
 	int tx_size;
 	int rx_size;
@@ -62,6 +65,13 @@ struct usb_stream_config {
 extern struct consumer_ops const usb_stream_consumer_ops;
 extern struct producer_ops const usb_stream_producer_ops;
 
+/* Need to define these so that other than Cr50 boards compile cleanly. */
+#ifndef USB_EP_EC
+#define USB_EP_EC -1
+#endif
+#ifndef USB_EP_AP
+#define USB_EP_AP -1
+#endif
 
 /*
  * Convenience macro for defining USB streams and their associated state and
@@ -115,16 +125,23 @@ extern struct producer_ops const usb_stream_producer_ops;
 	static struct g_usb_desc CONCAT2(NAME, _in_desc_)[MAX_IN_DESC];	\
 	static uint8_t CONCAT2(NAME, _buf_rx_)[RX_SIZE];		\
 	static uint8_t CONCAT2(NAME, _tx_in_progress_);			\
+	static uint8_t CONCAT2(NAME, _kicker_running_);			\
 	static void CONCAT2(NAME, _deferred_rx_)(void);			\
+	static void CONCAT2(NAME, _tx_kicker_)(void);			\
 	DECLARE_DEFERRED(CONCAT2(NAME, _deferred_rx_));			\
+	DECLARE_DEFERRED(CONCAT2(NAME, _tx_kicker_));			\
 	static int CONCAT2(NAME, _rx_handled);				\
 	static size_t CONCAT2(NAME, _tx_handled);			\
 	struct usb_stream_config const NAME = {				\
 		.endpoint     = ENDPOINT,				\
+		.is_uart_console = ((ENDPOINT == USB_EP_EC) ||		\
+				    (ENDPOINT == USB_EP_AP)),		\
 		.tx_in_progress = &CONCAT2(NAME, _tx_in_progress_),	\
+		.kicker_running = &CONCAT2(NAME, _kicker_running_),	\
 		.in_desc      = &CONCAT2(NAME, _in_desc_)[0],		\
 		.out_desc     = &CONCAT2(NAME, _out_desc_),		\
 		.deferred_rx  = &CONCAT2(NAME, _deferred_rx__data),	\
+		.tx_kicker    = &CONCAT2(NAME, _tx_kicker__data),	\
 		.tx_size      = TX_SIZE,				\
 		.rx_size      = RX_SIZE,				\
 		.rx_ram       = CONCAT2(NAME, _buf_rx_),		\
@@ -171,6 +188,8 @@ extern struct producer_ops const usb_stream_producer_ops;
 	};								\
 	static void CONCAT2(NAME, _deferred_rx_)(void)			\
 	{ rx_stream_handler(&NAME); }					\
+	static void CONCAT2(NAME, _tx_kicker_)(void)			\
+	{ tx_stream_kicker(&NAME); }					\
 	static void CONCAT2(NAME, _ep_tx)(void)				\
 	{								\
 		usb_stream_tx(&NAME);					\
@@ -213,6 +232,7 @@ extern struct producer_ops const usb_stream_producer_ops;
  * Handle USB and Queue request in a deferred callback.
  */
 void rx_stream_handler(struct usb_stream_config const *config);
+void tx_stream_kicker(struct usb_stream_config const *config);
 
 /*
  * These functions are used by the trampoline functions defined above to