serial: leave a UART only the DMA streams a timer output will use

docs/development/targets/timer-dma-conflicts.md

@@ -150,6 +150,108 @@ STM32H7 uses DMAMUX which eliminates most DMA conflicts:
 - Fewer headaches for target developers
 - Conflicts still possible but rare, and some channels (e.g. `TIM15_CH2`, see above) have no DMA request line at all regardless of DMAMUX
 
+## UART Receive Through DMA
+
+A UART can receive through a DMA stream instead of taking an interrupt for every byte.
+A target turns this on per port, by naming a stream for it in `target.h`:
+
+```c
+#define UART2_RX_DMA            DMA_TAG(2, 5, 0)    // DMA2 stream 5
+```
+
+A port without `UARTx_RX_DMA` behaves exactly as before, and a target that names none
+builds exactly as before.
+
+**What it changes.** The stream writes into the port's receive ring by itself, so a busy
+port no longer costs an interrupt per byte, and bytes that arrive while interrupts are
+held off (an internal flash write, for instance) are not lost. The ring and its size do
+not change. On an H7 at 480 MHz the saving measured around 240 cycles per byte, a fraction
+of a percent of the CPU; relatively it is larger on a slower MCU with a busy port.
+
+**Which ports use it.** Those whose owner reads through the ring: GPS, MSP, telemetry and
+the like. Serial receivers (CRSF, SBUS, IBUS, GHST, SRXL2, ...) take each byte through a
+callback as it lands, because they find their frames by timing, so their port stays on the
+interrupt even when the target names a stream for it.
+
+### Choosing the stream
+
+**F4 and F7.** Each UART's receiver is wired to fixed streams on a fixed channel, and the
+build fails if the tag names any other:
+
+| UART | Stream and channel |
+|------|--------------------|
+| UART1 | `DMA_TAG(2, 2, 4)` or `DMA_TAG(2, 5, 4)` |
+| UART2 | `DMA_TAG(1, 5, 4)` |
+| UART3 | `DMA_TAG(1, 1, 4)` |
+| UART4 | `DMA_TAG(1, 2, 4)` |
+| UART5 | `DMA_TAG(1, 0, 4)` |
+| UART6 | `DMA_TAG(2, 1, 5)` or `DMA_TAG(2, 2, 5)` |
+| UART7 | `DMA_TAG(1, 3, 5)` |
+| UART8 | `DMA_TAG(1, 6, 5)` |
+
+**H7 and AT32F43x.** The DMAMUX connects any UART to any stream (channel, on the AT32),
+so only the controller and the stream matter; leave the last field 0.
+
+**The stream has to be free on that board.** The streams are shared, so this is a
+property of the target rather than of the MCU:
+
+- *Timer outputs.* A stream a timer output will use is refused, and the port stays on the
+  interrupt, so a motor never loses DSHOT to a serial port:
+  - with the motors on DSHOT, every output's stream until the motors start, and afterwards the
+    streams the motors took. Ports opened before the motors (MSP, DJI HD OSD, SmartPort
+    master, LOG) therefore get no output's stream on a DSHOT board;
+  - the LED strip's pad, when that feature is on.
+
+  Outputs that end up as servos or unused, and every output except the LED strip's when the
+  motors are not on DSHOT, leave their streams to the serial ports. This is decided at each
+  boot: after changing the motor protocol, the mixer, the output modes or the LED strip
+  feature, a port gets or gives back its stream at the next reboot. On an H7 the `dmavar` of
+  `DEF_TIM()` is the stream: 0 to 7 are DMA1 streams 0 to 7, 8 to 15 are DMA2 streams 0 to 7.
+- *The ADC* is not checked, so avoid its stream: on F4 and AT32 `ADC1_DMA_STREAM` where the
+  target sets it, otherwise DMA2 stream 0 and DMA2 channel 1; DMA2 stream 0 on F7; DMA2
+  stream 0, 1 or 2 on H7, for `ADC_INSTANCE` ADC1, ADC2 or ADC3.
+- *The SD card* on F4 and F7 SDIO uses DMA2 stream 3 or 6 (`SDCARD_SDIO_DMA`) and is not
+  checked against the serial ports either: if a UART takes that stream first, the card is
+  left unused.
+- *Two UARTs* cannot share one; the second keeps its interrupt.
+
+On an H7 the ring of a port receiving through DMA moves to D2 SRAM, where the data cache
+does not reach: 256 bytes per port named.
+
+## UART Transmit Through DMA
+
+The same for sending: `UARTx_TX_DMA` names the stream that empties the port's transmit
+ring.
+
+```c
+#define UART2_TX_DMA            DMA_TAG(2, 7, 0)    // DMA2 stream 7
+```
+
+**What it changes.** The port takes one interrupt per transfer instead of one per byte. A
+message written whole with `serialWriteBuf()`, as MSP and MAVLink do, goes out in one
+transfer; bytes written one at a time, as the serial blackbox does, go out a few transfers
+at a time, since whatever is queued while one runs goes in the next. On an H7 at 480 MHz,
+MAVLink at 921600 baud sending 12.6 kB/s took 12 959 interrupts a second, 0.66% of the CPU,
+and 144 through DMA, 0.006%. It is worth a stream on a port that sends a lot: serial
+blackbox, MAVLink, MSP DisplayPort.
+
+**F4 and F7.** Each UART's transmitter is wired to fixed streams too:
+
+| UART | Stream and channel |
+|------|--------------------|
+| UART1 | `DMA_TAG(2, 7, 4)` |
+| UART2 | `DMA_TAG(1, 6, 4)` |
+| UART3 | `DMA_TAG(1, 3, 4)` or `DMA_TAG(1, 4, 7)` |
+| UART4 | `DMA_TAG(1, 4, 4)` |
+| UART5 | `DMA_TAG(1, 7, 4)` |
+| UART6 | `DMA_TAG(2, 6, 5)` or `DMA_TAG(2, 7, 5)` |
+| UART7 | `DMA_TAG(1, 1, 5)` |
+| UART8 | `DMA_TAG(1, 0, 5)` |
+
+On H7 and AT32F43x any free stream (channel) will do, and the rules for a free one are the
+ones above. A port's transmit stream has to be a different one from its receive stream,
+and on an H7 its transmit ring moves to D2 SRAM as well.
+
 ## Related Documentation
 
 - **overview.md** - Target system basics

src/main/drivers/pwm_mapping.c

@@ -526,12 +526,14 @@ static void pwmInitServos(timMotorServoHardware_t * timOutputs)
 
 
 static timMotorServoHardware_t timOutputsStatic;
+static bool motorsInitialised = false;
 
 bool pwmMotorAndServoInit(void)
 {
     pwmBuildTimerOutputList(&timOutputsStatic);
     pwmInitMotors(&timOutputsStatic);
     pwmInitServos(&timOutputsStatic);
+    motorsInitialised = true;
     return (pwmInitError == PWM_INIT_ERROR_NONE);
 }
 
@@ -540,6 +542,27 @@ const timMotorServoHardware_t *pwmGetOutputAssignment(void)
     return &timOutputsStatic;
 }
 
+bool pwmIsDmaStreamReserved(DMA_t dma)
+{
+    // Started DSHOT motors own their streams; before they start nothing can have changed the protocol
+    const bool motorsWillClaim = !motorsInitialised && getMotorProtocolProperties(motorConfig()->motorPwmProtocol)->isDSHOT;
+
+    for (int i = 0; i < timerHardwareCount; i++) {
+        const timerHardware_t *timHw = &timerHardware[i];
+        if (dmaGetByTag(timHw->dmaTag) != dma) {
+            continue;
+        }
+        if (motorsWillClaim) {
+            return true;
+        }
+        // The LED strip claims its stream only once it is enabled, late in init
+        if (feature(FEATURE_LED_STRIP) && (TIM_IS_LED(timHw->usageFlags) || timerOverrides(timer2id(timHw->tim))->outputMode == OUTPUT_MODE_LED)) {
+            return true;
+        }
+    }
+    return false;
+}
+
 // Upper bound for timerHardware[] size across all supported targets.
 // timerHardwareCount is a runtime value; this constant prevents a VLA on the MSP
 // task stack. Targets with 22+ entries (e.g. OMNIBUSF4) need more than MAX_PWM_OUTPUTS.

src/main/drivers/pwm_mapping.h

@@ -103,5 +103,7 @@ pwmInitError_e getPwmInitError(void);
 const char * getPwmInitErrorMessage(void);
 #ifndef SITL_BUILD
 const timMotorServoHardware_t *pwmGetOutputAssignment(void);
+// A stream a timer output will claim later in init; one it already owns is not counted
+bool pwmIsDmaStreamReserved(DMA_t dma);
 void pwmCalculateAssignment(timMotorServoHardware_t *out, const uint8_t *proposedModes);
 #endif // SITL_BUILD

src/main/drivers/serial.c

@@ -20,6 +20,10 @@
 
 #include "platform.h"
 
+#include "build/atomic.h"
+
+#include "drivers/nvic.h"
+
 #include "serial.h"
 
 void serialPrint(serialPort_t *instance, const char *str)
@@ -133,3 +137,24 @@ bool serialIsIdle(serialPort_t *instance)
     else
         return false;
 }
+
+// Swaps in a larger receive buffer; what the old one held is dropped
+void serialSetRxBuffer(serialPort_t *instance, volatile uint8_t *buffer, uint32_t size)
+{
+    if (size <= instance->rxBufferSize) {
+        return;
+    }
+
+    if (instance->vTable->setRxBuffer) {
+        instance->vTable->setRxBuffer(instance, buffer, size);
+        return;
+    }
+
+    // The receive interrupt must never see the new buffer with the old size or indices
+    ATOMIC_BLOCK(NVIC_PRIO_MAX) {
+        instance->rxBuffer = buffer;
+        instance->rxBufferSize = size;
+        instance->rxBufferHead = 0;
+        instance->rxBufferTail = 0;
+    }
+}

src/main/drivers/serial.h

@@ -106,6 +106,9 @@ struct serialPortVTable {
     // Optional functions used to buffer large writes.
     void (*beginWrite)(serialPort_t *instance);
     void (*endWrite)(serialPort_t *instance);
+
+    // Optional: for a port whose reception is more than the ring, such as a DMA stream
+    void (*setRxBuffer)(serialPort_t *instance, volatile uint8_t *buffer, uint32_t size);
 };
 
 void serialWrite(serialPort_t *instance, uint8_t ch);
@@ -122,6 +125,7 @@ void serialPrint(serialPort_t *instance, const char *str);
 uint32_t serialGetBaudRate(serialPort_t *instance);
 bool serialIsConnected(const serialPort_t *instance);
 bool serialIsIdle(serialPort_t *instance);
+void serialSetRxBuffer(serialPort_t *instance, volatile uint8_t *buffer, uint32_t size);
 
 // A shim that adapts the bufWriter API to the serialWriteBuf() API.
 void serialWriteBufShim(void *instance, const uint8_t *data, int count);

src/main/drivers/serial_uart.c

@@ -135,6 +135,9 @@ serialPort_t *uartOpen(USART_TypeDef *USARTx, serialReceiveCallbackPtr rxCallbac
         return (serialPort_t *)s;
     }
 
+    // A transfer still on its way would go on reading the ring about to be reset
+    uartTxDmaStop(s);
+
     // common serial initialisation code should move to serialPort::init()
     s->port.rxBufferHead = s->port.rxBufferTail = 0;
     s->port.txBufferHead = s->port.txBufferTail = 0;
@@ -147,12 +150,12 @@ serialPort_t *uartOpen(USART_TypeDef *USARTx, serialReceiveCallbackPtr rxCallbac
 
     uartReconfigure(s);
 
-    if (mode & MODE_RX) {
+    if ((mode & MODE_RX) && !uartRxDmaStart(s)) {
         USART_ClearITPendingBit(s->USARTx, USART_IT_RXNE);
         USART_ITConfig(s->USARTx, USART_IT_RXNE, ENABLE);
     }
 
-    if (mode & MODE_TX) {
+    if ((mode & MODE_TX) && !uartTxDmaStart(s)) {
         USART_ITConfig(s->USARTx, USART_IT_TXE, ENABLE);
     }
 
@@ -186,10 +189,12 @@ uint32_t uartTotalRxBytesWaiting(const serialPort_t *instance)
 {
     const uartPort_t *s = (const uartPort_t*)instance;
 
-    if (s->port.rxBufferHead >= s->port.rxBufferTail) {
-        return s->port.rxBufferHead - s->port.rxBufferTail;
+    const uint32_t head = uartRxBufferHead(s);
+
+    if (head >= s->port.rxBufferTail) {
+        return head - s->port.rxBufferTail;
     } else {
-        return s->port.rxBufferSize + s->port.rxBufferHead - s->port.rxBufferTail;
+        return s->port.rxBufferSize + head - s->port.rxBufferTail;
     }
 }
 
@@ -229,17 +234,52 @@ uint8_t uartRead(serialPort_t *instance)
     return ch;
 }
 
-void uartWrite(serialPort_t *instance, uint8_t ch)
+static void uartQueue(uartPort_t *s, uint8_t ch)
 {
-    uartPort_t *s = (uartPort_t *)instance;
     s->port.txBuffer[s->port.txBufferHead] = ch;
     if (s->port.txBufferHead + 1 >= s->port.txBufferSize) {
         s->port.txBufferHead = 0;
     } else {
         s->port.txBufferHead++;
     }
+}
 
-    USART_ITConfig(s->USARTx, USART_IT_TXE, ENABLE);
+void uartWrite(serialPort_t *instance, uint8_t ch)
+{
+    uartPort_t *s = (uartPort_t *)instance;
+    uartQueue(s, ch);
+
+    if (uartTxDmaRunning(s)) {
+        uartStartTxDMA(s);
+    } else {
+        USART_ITConfig(s->USARTx, USART_IT_TXE, ENABLE);
+    }
+}
+
+// Queued whole before the stream starts, so a message goes out in one transfer
+static void uartWriteBuf(serialPort_t *instance, const void *data, int count)
+{
+    uartPort_t *s = (uartPort_t *)instance;
+    const uint8_t *p = data;
+
+    for (; count > 0; count--, p++) {
+        while (!uartTotalTxBytesFree(instance)) {
+            // The ring is full: sending is what makes room
+            if (uartTxDmaRunning(s)) {
+                uartStartTxDMA(s);
+            }
+        }
+
+        if (uartTxDmaRunning(s)) {
+            uartQueue(s, *p);
+        } else {
+            uartWrite(instance, *p);
+        }
+    }
+
+    if (uartTxDmaRunning(s)) {
+        uartStartTxDMA(s);
+    }
 }
 
 bool isUartIdle(serialPort_t *instance)
@@ -264,9 +304,10 @@ const struct serialPortVTable uartVTable[] = {
         .setMode = uartSetMode,
         .setOptions = uartSetOptions,
         .isConnected = NULL,
-        .writeBuf = NULL,
+        .writeBuf = uartWriteBuf,
         .beginWrite = NULL,
         .endWrite = NULL,
         .isIdle = isUartIdle,
+        .setRxBuffer = uartSetRxBuffer,
     }
 };

src/main/drivers/serial_uart.h

@@ -20,11 +20,10 @@
 #define UART_AF(uart, af) CONCAT3(GPIO_AF, af, _ ## uart)
 
 // Since serial ports can be used for any function these buffer sizes should be equal
-// The two largest things that need to be sent are: 1, MSP responses, 2, UBLOX SVINFO packet.
+// The largest thing that needs to be sent is an MSP response.
 
-// Size must be a power of two due to various optimizations which use 'and' instead of 'mod'
-// Various serial routines return the buffer occupied size as uint8_t which would need to be extended in order to
-// increase size further.
+// Size must be a power of two due to various optimizations which use 'and' instead of 'mod'.
+// The GPS port gets a larger receive buffer of its own, see gps.c.
 #define UART1_RX_BUFFER_SIZE    256
 #define UART1_TX_BUFFER_SIZE    256
 #define UART2_RX_BUFFER_SIZE    256
@@ -66,6 +65,13 @@ typedef struct {
         USART_TypeDef *USARTx;
     #endif
 
+#ifdef USE_UART_RX_DMA
+    struct dmaChannelDescriptor_s *rxDma;   // the stream filling rxBuffer, NULL where the byte interrupt does
+#endif
+#ifdef USE_UART_TX_DMA
+    struct dmaChannelDescriptor_s *txDma;   // the stream emptying txBuffer, NULL where the byte interrupt does
+    volatile uint32_t txDmaCount;           // bytes of the transfer on its way, 0 when the stream is idle
+#endif
 } uartPort_t;
 
 void uartGetPortPins(UARTDevice_e device, serialPortPins_t * pins);