Repository navigation
Conversation
NAV-SIG (M9, M10) and NAV-SAT (M8) were requested at every navigation epoch. They feed only the satellite list in the CLI, yet they are most of what the receiver sends: NAV-SIG carries 16 bytes for each tracked signal, several hundred bytes per epoch on a multi-constellation receiver, against about 100 for the NAV-PVT that navigation runs on. They are now requested every (gps_ublox_nav_hz + 1) / 2 epochs, which is about twice a second at any navigation rate.
A receive buffer has to hold what arrives between two runs of the task that drains it. For the GPS task, 20 ms apart, that is up to 230 bytes at 115200 and 460 at 230400, and a reply to a poll has to fit whole: MON-VER alone can be 258 bytes. With the 256 bytes every port has, a NEO-F10N at 230400 was never identified, because its MON-VER reply was always cut. The GPS port now gets a 512 byte buffer of its own, in FASTRAM, where there is room: CCM on F405, RAM1 on AT32F43x. serialSetRxBuffer() swaps it in right after the port is opened. Every other port keeps its 256 bytes. The note that occupied sizes are returned as uint8_t no longer held: the driver functions and the ring indices are all 32 bit.
The satellite details also feed the OSD's GPS extra stats, not only gpssats, so the comment now says both. The comments are one line each, the buffer states the rate it covers (230400; at 460800 and above a NAV-PVT plus a large NAV-SIG can still overflow it), its size is asserted to be a power of two because softserial masks with size - 1, and it only exists where the u-blox provider does, the only one that opens a port. The note in serial_uart.h about sending a UBLOX SVINFO was stale.
A UART receiving through a DMA stream costs no interrupt per byte, and keeps the bytes that arrive while interrupts are held off. A target turns it on per port with UARTx_RX_DMA; a port without one, and a target that names none, build and behave exactly as before. The stream fills the port's own ring, whose head is read back from the transfer counter, so nothing above the driver changes. Ports whose owner takes each byte through rxCallback (the serial receivers, which frame by timing) stay on the interrupt even when a stream is named. F4 and F7 wire each receiver to fixed streams, so a tag naming another is a build error. H7 and AT32 route through the DMAMUX. On H7 the ring moves to D2 SRAM, out of the data cache. A stream any timer output is mapped to is left to the timers, since MSP ports open before the motors do. On F7, H7 and AT32 the interrupt handler tested the RX flag without the enable, so a port receiving through DMA could have lost a byte to the interrupt raised for what it sends; it now leaves the data register alone. Enabled on TBS_LUCID_H7_WING for UART2, on DMA2 stream 5.
serialSetRxBuffer() only swapped the ring's pointer and size, but a DMA stream keeps writing to the memory it was started on, so a port receiving through DMA would have gone on filling its old ring while the reader looked at the new one. The serial vtable gets an optional setRxBuffer, which the UARTs implement by swapping the ring and, if a stream is running, starting it again on the new one. The GPS ring must then be memory a stream can reach on targets that receive through DMA: D2 SRAM on H7 rather than DTCM, and on F4 and AT32F43x plain RAM rather than CCM or RAM1. Everywhere else it stays in FASTRAM, so a target without UARTx_RX_DMA builds exactly as before. On the TBS Lucid H7 Wing, GPS on UART2 through DMA2 stream 5, NEO-F10N: identified with no errors or timeouts at 115200 and after autobaud to 230400, NAV-PVT at the configured 10 Hz. Over ten reboots, 1 error and 2 timeouts at startup, against 1 and 1 over nine with the byte interrupt.
A UART sends one byte per interrupt. A target can now name a stream for a port's transmitter (UARTx_TX_DMA), as it can for the receiver: the stream empties the transmit ring and the port takes one interrupt per transfer. writeBuf() queues a message whole, so MSP and MAVLink send each in one transfer; what is queued while one runs goes in the next. On F4, F7, H7 and AT32. The transmit ring moves to D2 SRAM on H7, as the receive ring does. The stream check the receiver used now serves both, and F4/F7 targets naming a stream the transmitter is not wired to fail to build. The TBS Lucid H7 Wing sends on UART2 through DMA2 stream 7.
iNavFlight#12119 turns the F7 D-cache on and makes DMA_RAM an uncached SRAM2 region. A ring left in cached RAM could then hand the CPU a line it cached before the stream filled it. The rings go in DMA_RAM, as on H7; on F7 that is plain RAM until iNavFlight#12119, so on its own this changes nothing. Not seen to fail at 115200: FLYWOOF722PRO with iNavFlight#12119 and the GPS sending on the interrupt had 0-1 errors per boot with the rings in cached RAM and in DMA_RAM alike.
On F7 and H7, stopping the stream clears USART_CR3_DMAR and only uartReconfigure() set it again. After serialSetRxBuffer() the stream ran but the UART never fed it, and with the byte interrupt off for a DMA port the port stayed deaf until the next baud, mode or option change. The GPS got away with it because the u-blox driver changes the baud rate right after swapping its ring. F4 and AT32 already set the request when the stream starts. With a test patch that swaps a larger GPS ring in 10 s after boot, FLYWOOF722PRO and KAKUTEH7MINI had a timeout on every boot without this and none with it.
# Conflicts: # src/main/drivers/serial_uart_stm32f7xx.c
MSP DisplayPort and the serial gimbal swap their own TX buffer into the port after opening it. On H7 that buffer is in AXI SRAM, which the CPU caches, so the stream sends what is in RAM rather than what was just written. Kakute H7 Mini with the GPS port given such a buffer (a test patch): the u-blox never got its configuration (3 timeouts, no navigation messages); with this, a navigation message every 120 ms and 0 errors over 3 reboots. On F7 every ring is cached once iNavFlight#12119 turns its D-cache on: FLYWOOF722PRO with iNavFlight#12119 had 11-14 GPS errors and no fix on every boot, and 0 with this. The lines the transfer covers are written back first. Without a D-cache, or on the uncached D2 rings, that does nothing.
The stream check refused every stream any timer output is mapped to, so a target naming a UART stream on F4 or F7, where each UART has one or two fixed streams, would find most of them taken. Now it refuses a stream while DSHOT motors that may take it have not started, and the LED strip's stream when that feature is on. Started motors already own their streams, so a port opened after them gets the streams of outputs that ended up as servos or unused, and every stream but the LED strip's when the motors are not on DSHOT. The choice is made at each boot.
Contributor
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
PR Summary by QodoAdd UART DMA with use-aware timer stream reservations
AI Description
Diagram
High-Level Assessment
Files changed (21)
|
Code Review by Qodo
1. Motorless craft use slower serial ports
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+295
to
+297
| // In DMA_RAM, which stays uncached once the D-cache is on: from the cache the CPU would not see what the stream wrote | ||
| #ifdef UART1_RX_DMA | ||
| static DMA_RAM uint8_t uart1RxDmaBuffer[UART_RX_BUFFER_SIZE]; |
There was a problem hiding this comment.
2. F7 receive ring comment promises an uncached buffer it lacks 🐞 Bug ⚙ Maintainability
The F7 driver gives each named UART a separate DMA_RAM receive ring and says this keeps it uncached once the D-cache is on, but build_config.h defines DMA_RAM as empty on STM32F7, so the ring is ordinary .bss and the port's own uart->rxBuffer sits unused (256 B each). RX DMA on F7 is correct today only because SCB_EnableDCache() is commented out in system_stm32f7xx.c; if someone turns the cache on, trusting this comment (and the matching assumption for gpsRxBuffer in gps.c), the CPU would read stale bytes and nothing would warn.
Agent Prompt
## Issue description
On STM32F7, `DMA_RAM` expands to nothing (build_config.h), so the per-UART `uartNRxDmaBuffer` arrays in serial_uart_stm32f7xx.c are plain .bss duplicates of `uart->rxBuffer`. The comment says they stay uncached once the D-cache is on, which is wrong. RX DMA works only because the F7 D-cache is disabled.
## Fix Focus Areas
- src/main/drivers/serial_uart_stm32f7xx.c[295-346]
- src/main/drivers/serial_uart_stm32f7xx.c[644-648]
- src/main/io/gps.c[98-106]
## Recommended Fix
Remove the F7 `uartRxDmaBuffer` arrays and keep `uart->rxBuffer`, saving 256 B for each named port. Change the comment (and the one in gps.c) to say that F7 runs with the D-cache disabled, so any SRAM buffer works. Optionally add a `#error` or STATIC_ASSERT guard to catch the D-cache being enabled on F7 while USE_UART_RX_DMA is defined.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Contributor
Author
There was a problem hiding this comment.
On its own it is plain RAM: the rings are there for #12119, which turns the F7 D-cache on and makes DMA_RAM an uncached SRAM2 section. Until then each ring only duplicated the port's own, 256 B for every UART with a stream once #12129 is in. They are now compiled only where DMA_RAM is uncached (DMA_RAM_UNCACHED, set by #12119 together with the D-cache): d2cafb6 on #12033, 4d2c700 on #12119, so either can go in first. On #12129 that gives back 1280 B of RAM on MATEKF722SE and 1536 B on KAKUTEF7; with #12119 merged the rings are in SRAM2 again.
|
RAM / Flash usage vs. base commit
See RAM/flash optimization guide for techniques to reduce usage. |
|
Test firmware build ready — commit Download firmware for PR #12128 251 targets built. Find your board's
|
On F7 DMA_RAM is plain RAM until iNavFlight#12119 turns the D-cache on and makes it an uncached SRAM2 section, so until then each ring only duplicated the port's own, 256 B per UART with a receive stream. They are compiled only where build_config.h says DMA_RAM is uncached (DMA_RAM_UNCACHED, set together with the D-cache), so either PR can go in first.
This was referenced Oct 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on #12062 (UART TX DMA), which builds on
#12033 (UART RX DMA); until those merge this diff carries their commits too, and only the last commit is this PR.
Stacked on this one: #12129 and #12132.
With #12033/#12062 a UART refuses any DMA stream that any timer output in
target.cis mapped to, because MSP portsopen before the motors do. On F4 and F7 each UART can only use one or two fixed streams, and the timer outputs take
most of them: counting the 182 F4/F7 target folders on maintenance-10.x (931 UARTs), 42 % of the UARTs find no
stream to receive on and 50 % none to send on.
A stream is now refused only when a timer output will really use it:
took, which they own by then;
Outputs that end up as servos or unused, and every output when the motors are not on DSHOT, leave their streams to
the serial ports, except the LED strip's pad while that feature is on. With the same count, the UARTs with no stream to receive on drop to 26 % on a quad (four DSHOT
motors) and 6 % on a plane (one).
On its own this changes nothing for the targets in the tree today: only TBS_LUCID_H7_WING names a UART stream, and none of its outputs is mapped to it. It matters once ports get streams without naming them, which #12129 (DMA on every UART, stacked on this one) does; without this rule half of those ports on F4 and F7 would find none.
This is decided at each boot: after changing the motor protocol, the mixer or the LED strip feature, a port gets or
gives back its stream at the next reboot. The target guide (
timer-dma-conflicts.md) says so.Tested
A test-only patch (not in this PR) names, on a locally modified target, a UART stream that an unused output is also
mapped to, and prints the owner of the streams in
status.FLYWOOF722PRO, GPS (u-blox M10) on UART5, receive stream DMA1 S0 = output S7:
GPS for 3 minutes on DMA and 3 on the interrupt: 0 errors, 0 timeouts, about 10.4 packets/s both ways. With the GPS on
DMA and the ESC powered (no props), disarmed motor steps drew the same current as on the base image, with 0 GPS errors.
KAKUTEH7MINI, GPS (SAM-M10Q) on UART2, receive stream DMA1 S7 = output S8: free on #12062; with this PR taken by
UART2 with 4 DSHOT motors, taken by the timer with 8, taken by UART2 again with 8 on ONESHOT125. GPS 0 errors.
The unit test
uart_dma_stream_unittestcovers the same rules on plain data, with a source check against the livefunction.
Not tested on hardware: F4 and AT32 (no boards here). The rule is the same code on every family.
Size
Identical on every target whose UARTs have no stream named; TBS_LUCID_H7_WING, the only one that names one today,
+64 B of flash.