Skip to content

Commit 83e28db

Browse files
committed
cdc_uart: performance optimisations
For RX, do burst reads without the SDK wrapper function which checks the FIFO flags for each read. For TX, avoid waiting for the FIFO to have space available, which causes significant thread stalls. Use a slightly pessimistic estimate of the number of bytes the TX FIFO should consume and only write up to that number. Also, there's no point tracking whether or not the scheduler parked the thread in a call to xTaskDelayUntil - missing a scheduler tick can't be recovered from.
1 parent 3d58384 commit 83e28db

File tree

1 file changed

+44
-13
lines changed

1 file changed

+44
-13
lines changed

src/cdc_uart.c

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
*/
2525

2626
#include <pico/stdlib.h>
27+
#include "hardware/uart.h"
2728
#include "FreeRTOS.h"
2829
#include "task.h"
2930
#include "tusb.h"
@@ -50,6 +51,9 @@ static volatile uint tx_led_debounce;
5051
static uint rx_led_debounce;
5152
#endif
5253

54+
// Guesstimate FIFO space per task wakeup
55+
static volatile uint32_t chars_per_interval;
56+
5357
void cdc_uart_init(void) {
5458
gpio_set_function(PROBE_UART_TX, GPIO_FUNC_UART);
5559
gpio_set_function(PROBE_UART_RX, GPIO_FUNC_UART);
@@ -98,9 +102,29 @@ bool cdc_task(void)
98102
static uint cdc_tx_oe = 0;
99103
uint rx_len = 0;
100104
bool keep_alive = false;
105+
uart_hw_t *uart_hw = uart_get_hw(PROBE_UART_INTERFACE);
101106

102107
// Consume uart fifo regardless even if not connected
103-
while(uart_is_readable(PROBE_UART_INTERFACE) && (rx_len < sizeof(rx_buf))) {
108+
if (uart_hw->ris & UART_UARTRIS_RXRIS_BITS) {
109+
// Burst 16 chars - don't use SDK wrapper as it tests RXFE every time
110+
rx_buf[rx_len++] = uart_hw->dr;
111+
rx_buf[rx_len++] = uart_hw->dr;
112+
rx_buf[rx_len++] = uart_hw->dr;
113+
rx_buf[rx_len++] = uart_hw->dr;
114+
rx_buf[rx_len++] = uart_hw->dr;
115+
rx_buf[rx_len++] = uart_hw->dr;
116+
rx_buf[rx_len++] = uart_hw->dr;
117+
rx_buf[rx_len++] = uart_hw->dr;
118+
rx_buf[rx_len++] = uart_hw->dr;
119+
rx_buf[rx_len++] = uart_hw->dr;
120+
rx_buf[rx_len++] = uart_hw->dr;
121+
rx_buf[rx_len++] = uart_hw->dr;
122+
rx_buf[rx_len++] = uart_hw->dr;
123+
rx_buf[rx_len++] = uart_hw->dr;
124+
rx_buf[rx_len++] = uart_hw->dr;
125+
rx_buf[rx_len++] = uart_hw->dr;
126+
}
127+
while (uart_is_readable(PROBE_UART_INTERFACE) && (rx_len < sizeof(rx_buf))) {
104128
rx_buf[rx_len++] = uart_getc(PROBE_UART_INTERFACE);
105129
}
106130

@@ -132,16 +156,16 @@ bool cdc_task(void)
132156
}
133157

134158
/* Reading from a firehose and writing to a FIFO. */
159+
/* Data available? */
135160
size_t watermark = MIN(tud_cdc_available(), sizeof(tx_buf));
136161
if (watermark > 0) {
137162
size_t tx_len;
138163
#ifdef PROBE_UART_TX_LED
139164
gpio_put(PROBE_UART_TX_LED, 1);
140165
tx_led_debounce = debounce_ticks;
141166
#endif
142-
/* Batch up to half a FIFO of data - don't clog up on RX */
143-
watermark = MIN(watermark, 16);
144-
tx_len = tud_cdc_read(tx_buf, watermark);
167+
/* Poke a reasonable number of bytes in */
168+
tx_len = tud_cdc_read(tx_buf, MIN(chars_per_interval, watermark));
145169
uart_write_blocking(PROBE_UART_INTERFACE, tx_buf, tx_len);
146170
} else {
147171
#ifdef PROBE_UART_TX_LED
@@ -178,16 +202,13 @@ bool cdc_task(void)
178202

179203
void cdc_thread(void *ptr)
180204
{
181-
BaseType_t delayed;
182205
last_wake = xTaskGetTickCount();
183206
bool keep_alive;
184207
/* Threaded with a polling interval that scales according to linerate */
185208
while (1) {
186209
keep_alive = cdc_task();
187210
if (!keep_alive) {
188-
delayed = xTaskDelayUntil(&last_wake, interval);
189-
if (delayed == pdFALSE)
190-
last_wake = xTaskGetTickCount();
211+
xTaskDelayUntil(&last_wake, interval);
191212
}
192213
}
193214
}
@@ -196,17 +217,27 @@ void tud_cdc_line_coding_cb(uint8_t itf, cdc_line_coding_t const* line_coding)
196217
{
197218
uart_parity_t parity;
198219
uint data_bits, stop_bits;
220+
uint32_t micros, rtos_period;
221+
222+
rtos_period = (1000 * 1000) / configTICK_RATE_HZ;
199223
/* Set the tick thread interval to the amount of time it takes to
200224
* fill up half a FIFO. Millis is too coarse for integer divide.
201225
*/
202-
uint32_t micros = (1000 * 1000 * 16 * 10) / MAX(line_coding->bit_rate, 1);
226+
micros = (1000 * 1000 * 10) / MAX(line_coding->bit_rate, 1);
227+
interval = MAX(1, (micros * 16) / rtos_period);
228+
// For very fast baudrates, guesstimate how much the TX FIFO will empty each time
229+
chars_per_interval = (uint32_t) (((float)interval * (float)rtos_period) / ((float)micros));
230+
if (chars_per_interval == 0)
231+
chars_per_interval++;
232+
233+
debounce_ticks = MAX(1, configTICK_RATE_HZ / (interval * DEBOUNCE_MS));
234+
probe_info("New baud rate %ld micros %ld interval %lu chars per %lu\n",
235+
line_coding->bit_rate, micros, interval, chars_per_interval);
236+
203237
/* Modifying state, so park the thread before changing it. */
204238
if (tud_cdc_connected())
205239
vTaskSuspend(uart_taskhandle);
206-
interval = MAX(1, micros / ((1000 * 1000) / configTICK_RATE_HZ));
207-
debounce_ticks = MAX(1, configTICK_RATE_HZ / (interval * DEBOUNCE_MS));
208-
probe_info("New baud rate %ld micros %ld interval %lu\n",
209-
line_coding->bit_rate, micros, interval);
240+
210241
uart_deinit(PROBE_UART_INTERFACE);
211242
tud_cdc_write_clear();
212243
tud_cdc_read_flush();

0 commit comments

Comments
 (0)