Offload TX data section to DMA (releases up to 12.5ms back to RTOS)

CMakeLists.txt

@@ -91,17 +91,24 @@ target_compile_options(avclan PUBLIC
 target_include_directories(avclan PUBLIC
     ${CMAKE_CURRENT_SOURCE_DIR}/src/avclan)
 
-add_executable(mockingboard
-    src/sniffer.cc
-)
-target_link_libraries(mockingboard avclan)
+# Pull in the selected hardware target: its port sources, per-target headers,
+# hardware-specific compile options/definitions, device-pack handling, and the
+# flashing target. Added after the targets above so it can extend them.
+add_subdirectory(src/avclan/target/${AVCLAN_TARGET})
+
+if(NOT TARGET mockingboard)
+  add_executable(mockingboard
+      src/sniffer.cc
+  )
+  target_link_libraries(mockingboard avclan)
+endif()
 
 # Report firmware section sizes on every build, when the toolchain provides a
 # size tool (CMAKE_SIZE). A standalone custom target (rather than a POST_BUILD
 # command on mockingboard) so it prints even when the link is up to date:
 # custom targets with no output are always considered stale and re-run.
 # add_dependencies orders it after the link.
-if(CMAKE_SIZE)
+if(TARGET mockingboard AND CMAKE_SIZE)
     add_custom_target(mockingboard-size ALL
         COMMAND ${CMAKE_SIZE} $<TARGET_FILE:mockingboard>
         COMMENT "Section sizes: mockingboard firmware"
@@ -110,11 +117,6 @@ if(CMAKE_SIZE)
     add_dependencies(mockingboard-size mockingboard)
 endif()
 
-# Pull in the selected hardware target: its port sources, per-target headers,
-# hardware-specific compile options/definitions, device-pack handling, and the
-# flashing target. Added after the targets above so it can extend them.
-add_subdirectory(src/avclan/target/${AVCLAN_TARGET})
-
 # --- Polyfill headers for std facilities the toolchain may lack -------------
 # tl::expected / tl::optional back-fill std::expected / std::optional (with
 # C++23 monadic ops) when the build's stdlib predates them. Runs after the port

src/avclan/target/pico2/CMakeLists.txt

@@ -50,31 +50,30 @@ pico_enable_stdio_usb(avclan 1)
 
 # stdio_write_nonblock() is all-or-nothing and the largest buffer is a ~186 byte
 # frame log line; the SDK default of 64 would drop every one. PUBLIC so the
-# TinyUSB sources compiled into `mockingboard_pico` agree on the FIFO size.
+# TinyUSB sources compiled into `mockingboard` agree on the FIFO size.
 target_compile_definitions(avclan PUBLIC CFG_TUD_CDC_TX_BUFSIZE=256)
 
 # Needs separate/new executable target because `pico_add_extra_outputs` only
 # works on locally (to this file/directory) defined targets
-add_executable(mockingboard_pico
+add_executable(mockingboard
     ${avclan-mockingboard_SOURCE_DIR}/src/sniffer.cc
 )
-target_link_libraries(mockingboard_pico PUBLIC
+target_link_libraries(mockingboard PUBLIC
     avclan
 )
 
 # Silence warnings in TinyUSB, and only there.
 file(GLOB_RECURSE _tinyusb_sources ${PICO_TINYUSB_PATH}/src/*.c)
 set_source_files_properties(${_tinyusb_sources}
-    TARGET_DIRECTORY avclan mockingboard_pico
+    TARGET_DIRECTORY avclan mockingboard
     PROPERTIES COMPILE_OPTIONS "-w")
 unset(_tinyusb_sources)
 
-pico_add_extra_outputs(mockingboard_pico)
+pico_add_extra_outputs(mockingboard)
 
-pico_set_program_name(mockingboard_pico mockingboard)
-pico_set_program_description(mockingboard_pico
+pico_set_program_description(mockingboard
     "Emulate an external CD Changer on an AVCLAN bus to add Bluetooth and aux-input audio to older Toyota's"
 )
-pico_set_program_version(mockingboard_pico 
+pico_set_program_version(mockingboard 
     "${avclan-mockingboard_VERSION}")
-pico_set_program_url(mockingboard_pico "https://github.com/halleysfifthinc/AVCLAN-Mockingboard")
+pico_set_program_url(mockingboard "https://github.com/halleysfifthinc/AVCLAN-Mockingboard")

src/avclan/target/pico2/iebus.pio

@@ -133,17 +133,18 @@ push_slot:
 
 ; TX Encoding (8 deep TX FIFO):
 ; | MSB ............................................. LSB |
-; |   4 bits   | N bits |  1 bit   | 1 bit  | 26 - N bits |
-; | Length (N) | Data+P | has ACK  | NAK ok |   padding   |
+; |   4 bits   | N bits | 5 bits |  1 bit  | 22 - N bits |
+; | Length (N) | Data+P |  Next  | NAK ok  |   padding   |
 ;
 ; Parity is generated by the driver and appended as the last (least significant)
-; bit of the data field
+; bit of the data field. "Next" should be the address of `reset` or `ack_drive`
 
 .in 1 left
 .out 1 left
 
 .define public nak_irq      7
 .define public lost_arb_irq 6
+.define public eof_irq      3   ; An acknowledge slot is over
 
 ;;; Emit or synchronize to a start bit
 ; Reached by a CPU initiated force exec jmp to tx_startbit, with x = JOIN_SAMPLES
@@ -182,8 +183,9 @@ handle_nak:
 ; the one bit in a frame we don't drive to completion ourselves. A slot nobody
 ; answered is already recessive and falls straight through.
 ack_done:
-    wait 1 pin 0    [12]    ; 15 cycles from the release to `set pins, 0` below
+    wait 1 pin 0    [11]    ; 15 cycles from the release to `set pins, 0` below
                             ; (~AVCLAN_BIT0_LOGIC_1)
+    irq set eof_irq
 
 reset:
 .wrap_target
@@ -213,8 +215,7 @@ lost:
 bit_end:
     set pins, 1         [9]     ; Delay must be common to all following control flow paths
     jmp x-- slow_jmp            ; Fall through once x (the bit count) is exhausted
-    out x, 1
-    jmp !x reset                ; Fall through to read ACK if "has ACK" is set
+    out pc, 5           [1]     ; "Next": reset, or ack_drive to read ACK
 
 ;;; read ack
 public ack_drive:
@@ -227,9 +228,8 @@ public ack_release:
     jmp ack_done
 
 slow_jmp:
-;;; The recessive ("prep") period in the send_bit loop is 2 instr's shorter than
-; the non-"has ACK" branch and
-    jmp send_bit        [3]
+    jmp send_bit        [3]     ; 15 cycles from bit_end to `set pins, 0`, the same
+                                ; as the last bit's path through `out pc` and reset
 
 ;;; Send an ack for iebus_rx
 ; Run by its own SM, wrapping from ack_release back to ack_entry: it borrows

src/avclan/target/pico2/phy.cc

@@ -3,12 +3,14 @@
 #include <cstdint>
 #include <cstring>
 #include <hardware/clocks.h>
+#include <hardware/dma.h>
 #include <limits>
 
 #include "FreeRTOS.h" // IWYU pragma: export
 #include "semphr.h"
 
 #include "avclan.h"
+#include "frame.hpp"
 #include "hal/phy.h"
 #include "hardware/gpio.h"
 #include "hardware/pio.h"
@@ -382,8 +384,6 @@ private:
     begin_frame();
   }
 
-  // The initialized instance, for irq_handler: SDK IRQ handlers take no
-  // context.
   static inline IEBusRx *instance_ = nullptr;
 
   PIO pio_;
@@ -445,16 +445,27 @@ public:
   // relies on. The pad keeps its PIO function select: handing it back to SIO
   // would float the line, and a floating bus reads dominant.
   ~IEBusTx() {
-    if (!claimed_)
+    if (instance_ != this)
       return;
+    irq_set_enabled(irq_, false);
+    irq_remove_handler(irq_, irq_handler);
+    irq_set_enabled(DMA_IRQ_0, false);
+    irq_remove_handler(DMA_IRQ_0, dma_irq_handler);
+    pio_set_irq0_source_mask_enabled(pio_, TX_SOURCES, false);
     pio_set_sm_mask_enabled(pio_, sm_mask(), false);
     pio_remove_program_and_unclaim_sm(&iebus_tx_program, pio_, tx_sm_,
                                       tx_offset_);
     pio_sm_unclaim(pio_, ack_sm_);
+    dma_channel_cleanup(tx_dma_);
+    dma_channel_unclaim(tx_dma_);
+    instance_ = nullptr;
   }
 
   // Claims and configures both SMs, but doesn't start them.
   void init(PIO pio, uint pin_rx, uint pin_tx) {
+    hard_assert(instance_ == nullptr);
+    instance_ = this;
+
     pio_ = pio;
     pin_tx_ = pin_tx;
     tx_offset_ = (uint)pio_add_program(pio_, &iebus_tx_program);
@@ -480,6 +491,22 @@ public:
     // Entry is `reset`; offset 0 is the start-bit block.
     pio_sm_init(pio_, tx_sm_, tx_offset_ + iebus_tx_wrap_target, &tx_cfg);
 
+    tx_dma_ = (uint)dma_claim_unused_channel(true);
+    dma_cfg_ = dma_channel_get_default_config(tx_dma_);
+    channel_config_set_dreq(&dma_cfg_, pio_get_dreq(pio_, tx_sm_, true));
+    dma_channel_configure(tx_dma_, &dma_cfg_, &pio_->txf[tx_sm_], nullptr, 0,
+                          false);
+
+    irq_set_exclusive_handler(DMA_IRQ_0, dma_irq_handler);
+    irq_set_priority(DMA_IRQ_0, configMAX_SYSCALL_INTERRUPT_PRIORITY);
+    irq_set_enabled(DMA_IRQ_0, true);
+
+    // Sources stay masked until send_done waits on them.
+    irq_ = (uint)pio_get_irq_num(pio_, 0);
+    irq_set_exclusive_handler(irq_, irq_handler);
+    irq_set_priority(irq_, configMAX_SYSCALL_INTERRUPT_PRIORITY);
+    irq_set_enabled(irq_, true);
+
     pio_sm_config ack_cfg = iebus_tx_program_get_default_config(tx_offset_);
     sm_config_set_wrap(&ack_cfg, tx_offset_ + iebus_tx_offset_ack_entry,
                        tx_offset_ + iebus_tx_offset_ack_release);
@@ -488,7 +515,6 @@ public:
 
     pio_sm_init(pio_, ack_sm_, tx_offset_ + iebus_tx_offset_ack_entry,
                 &ack_cfg);
-    claimed_ = true;
   }
 
   PIO pio() const { return pio_; }
@@ -557,26 +583,24 @@ public:
     if (muted_)
       return MUTED;
     data_start_ = words_;
-    for (uint8_t i = 0; i < length && !check(); i++)
-      put(encode_tx(8, data[i], true, expect_ack));
+
+    if (check())
+      return Send{0};
+    for (uint8_t i = 0; i < length; i++)
+      dma_buf_[i] = encode_tx(8, data[i], true, expect_ack);
+    words_ += length;
+    dma_channel_acknowledge_irq0(tx_dma_); // Clear any stale IRQ
+    dma_channel_transfer_from_buffer_now(tx_dma_, &dma_buf_, length);
     return Send{0};
   }
 
   Send send_done(uint8_t *data_index) {
-    const auto flagged = [this] {
-      return pio_interrupt_get(pio_, iebus_tx_lost_arb_irq) ||
-             pio_interrupt_get(pio_, iebus_tx_nak_irq);
-    };
-
-    // The second PC test confirms that SM has finished sending last value and
-    // not in-progress (i.e. pulled the last FIFO value and still mid-send)
-    const auto idle = [this] {
-      return pio_sm_is_tx_fifo_empty(pio_, tx_sm_) && // Read before the pc
-             pio_sm_get_pc(pio_, tx_sm_) == tx_offset_ + iebus_tx_wrap_target;
-    };
-
-    while (!(flagged() || idle()))
-      tight_loop_contents();
+    // Pending the IRQ has the ISR judge a frame that ended before the unmask.
+    tx_task_ = xTaskGetCurrentTaskHandle();
+    pio_set_irq0_source_mask_enabled(pio_, FAULT_SOURCES, true);
+    dma_channel_set_irq0_enabled(tx_dma_, true); // Its ISR unmasks eof
+    irq_set_pending(irq_);
+    ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
 
     check();
     // Our frame is over, won or not.
@@ -590,22 +614,81 @@ public:
   }
 
 private:
-  // One field's word: [len:4][data+parity:len+1][has_ack:1][nak_ok:1][pad]. The
+  static constexpr uint32_t EOF_SOURCE = 1U
+                                         << (pis_interrupt0 + iebus_tx_eof_irq);
+  static constexpr uint32_t FAULT_SOURCES =
+      (1U << (pis_interrupt0 + iebus_tx_nak_irq)) |
+      (1U << (pis_interrupt0 + iebus_tx_lost_arb_irq));
+  static constexpr uint32_t TX_SOURCES = EOF_SOURCE | FAULT_SOURCES;
+
+  // Has the SM flagged a NAK or lost arbitration error?
+  bool flagged() const {
+    return (pio_->irq &
+            ((1U << iebus_tx_nak_irq) | (1U << iebus_tx_lost_arb_irq))) != 0;
+  }
+
+  // Has the SM returned to the reset instruction after emptying the fifo?
+  bool idle() const {
+    return pio_sm_is_tx_fifo_empty(pio_, tx_sm_) && // Read before the pc
+           pio_sm_get_pc(pio_, tx_sm_) == tx_offset_ + iebus_tx_wrap_target;
+  }
+
+  static void __time_critical_func(irq_handler)() { instance_->isr(); }
+
+  // Wakes tx task (blocked in send_done) once the frame is over, error or not.
+  // A frame ends with one of:
+  //  - Fault (NAK or bus contention)
+  //  - Idle DMA and idle SM
+  // The fault flags remain until check() collects them.
+  void __time_critical_func(isr)() {
+    if (!flagged()) {
+      // Clear/reset EOF trigger since not nak/lost_arb
+      pio_interrupt_clear(pio_, iebus_tx_eof_irq);
+      if (dma_channel_is_busy(tx_dma_) || !idle())
+        return; // Frame end conditions not met
+    }
+    // Past this point, the frame is over. The fault flags remain until check()
+    // collects them.
+
+    // Disable the IRQs since the frame is over
+    pio_set_irq0_source_mask_enabled(pio_, TX_SOURCES, false);
+    dma_channel_set_irq0_enabled(tx_dma_, false);
+
+    BaseType_t xHigherPriorityTaskWoken = pdFALSE;
+    vTaskNotifyGiveFromISR(tx_task_, &xHigherPriorityTaskWoken);
+    portYIELD_FROM_ISR(xHigherPriorityTaskWoken);
+  }
+
+  static void __time_critical_func(dma_irq_handler)() { instance_->dma_isr(); }
+
+  // The data words are all in the FIFO, so only these last few eofs can signal
+  // end of frame. INTS is clear when the PIO ISR already ended it.
+  void __time_critical_func(dma_isr)() {
+    if (!dma_channel_get_irq0_status(tx_dma_))
+      return;
+    dma_channel_acknowledge_irq0(tx_dma_);
+    dma_channel_set_irq0_enabled(tx_dma_, false);
+    pio_set_irq0_source_mask_enabled(pio_, EOF_SOURCE, true);
+  }
+
+  // One field's word: [len:4][data+parity:len+1][next:5][nak_ok:1][pad]. The
   // SM sends count + 1 bits, so the count is `len` and the parity bit the
-  // driver appends rides along as the extra one. `has_ack` emits the
-  // acknowledge slot; a NAK in it raises nak_irq unless `nak_ok`, i.e. unless
-  // we don't `expect_ack`.
-  static constexpr uint32_t encode_tx(size_t len, uint16_t bits, bool has_ack,
-                                      bool expect_ack) {
+  // driver appends rides along as the extra one. `has_ack` sends it on to
+  // ack_drive for the acknowledge slot; a NAK in it raises nak_irq unless
+  // `nak_ok`, i.e. unless we don't `expect_ack`.
+  uint32_t encode_tx(size_t len, uint16_t bits, bool has_ack,
+                     bool expect_ack) const {
     const auto n = (uint8_t)(len + 1);
     // Masked so a bit above the field (the broadcast bit's copy of itself, when
     // len is 0) can't spill into the count.
     const uint16_t with_parity =
         ((bits << 1) | parity(bits)) & ((1U << n) - 1U);
+    const uint32_t next = tx_offset_ + (has_ack ? iebus_tx_offset_ack_drive
+                                                : iebus_tx_wrap_target);
     uint32_t word = (uint32_t)len << 28;
     word |= with_parity << (28 - n);
-    word |= (uint32_t)has_ack << (27 - n);
-    word |= (uint32_t)!expect_ack << (26 - n);
+    word |= next << (23 - n);
+    word |= (uint32_t)!expect_ack << (22 - n);
     return word;
   }
 
@@ -616,6 +699,15 @@ private:
     if (!lost_arb && !pio_interrupt_get(pio_, iebus_tx_nak_irq))
       return fault_ != Send{0};
 
+    // A parked SM stalls the DMA with words still to write; stop it before the
+    // clear it would refill.
+    if (dma_channel_is_busy(tx_dma_)) {
+      words_ -= (uint8_t)(dma_channel_hw_addr(tx_dma_)->transfer_count &
+                          DMA_CH0_TRANS_COUNT_COUNT_BITS);
+      dma_channel_cleanup(tx_dma_);
+      dma_channel_set_config(tx_dma_, &dma_cfg_, false);
+    }
+
     // Read the level before the clear throws it away, and clear before the
     // flag: releasing the SM with words still queued would send the rest of the
     // frame.
@@ -666,13 +758,19 @@ private:
   }
 
   IEBusRx &rx_;
+  static inline IEBusTx *instance_ = nullptr;
 
   PIO pio_;
   uint pin_tx_;
   uint tx_sm_;
   uint tx_offset_;
   uint ack_sm_;
-  bool claimed_ = false; // init() ran, so the destructor has something to undo
+  uint irq_;
+
+  uint tx_dma_;
+  dma_channel_config dma_cfg_;
+  TaskHandle_t tx_task_ = nullptr;
+  std::array<uint32_t, Frame::MAXLENGTH> dma_buf_ = {};
 
   bool muted_ = false;