From 9fed1f9ebbd8e00e1e65fe06db3ac9bf77e89db4 Mon Sep 17 00:00:00 2001 From: Steve Evans Date: Mon, 30 Jun 2025 02:21:56 +0100 Subject: [PATCH] Add support for SPI DMA (#14483) --- src/main/msp/msp.c | 2 +- src/platform/PICO/bus_spi_pico.c | 109 +++++++++++++++--- src/platform/PICO/dma_pico.c | 35 +++--- src/platform/PICO/include/platform/platform.h | 1 - src/platform/PICO/target/RP2350B/target.h | 1 + 5 files changed, 112 insertions(+), 36 deletions(-) diff --git a/src/main/msp/msp.c b/src/main/msp/msp.c index b5fe879f1a..15cab1dec6 100644 --- a/src/main/msp/msp.c +++ b/src/main/msp/msp.c @@ -1830,7 +1830,7 @@ case MSP_NAME: } for (int i = 0; i < 8; i++) { - for (unsigned j; j < ARRAYLEN(gyroDeviceConfig(i)->customAlignment.raw); j++) { + for (unsigned j = 0; j < ARRAYLEN(gyroDeviceConfig(i)->customAlignment.raw); j++) { sbufWriteU16(dst, i < GYRO_COUNT ? gyroDeviceConfig(i)->customAlignment.raw[j] : 0); } } diff --git a/src/platform/PICO/bus_spi_pico.c b/src/platform/PICO/bus_spi_pico.c index 64118e33b4..4ecfd7ef5e 100644 --- a/src/platform/PICO/bus_spi_pico.c +++ b/src/platform/PICO/bus_spi_pico.c @@ -33,7 +33,7 @@ #include "platform.h" #ifdef USE_SPI -#define TESTING_NO_DMA 1 +//#define TESTING_NO_DMA 1 #include "common/maths.h" #include "drivers/bus.h" @@ -43,6 +43,7 @@ #include "drivers/io.h" #include "drivers/io_def.h" #include "drivers/io_impl.h" +#include "drivers/nvic.h" #include "hardware/spi.h" #include "hardware/gpio.h" @@ -126,6 +127,8 @@ const spiHardware_t spiHardware[] = { }, }; +extern busDevice_t spiBusDevice[SPIDEV_COUNT]; + void spiPinConfigure(const struct spiPinConfig_s *pConfig) { for (size_t hwindex = 0 ; hwindex < ARRAYLEN(spiHardware) ; hwindex++) { @@ -242,11 +245,84 @@ void spiInitDevice(SPIDevice device) gpio_set_function(IO_PINBYTAG(spi->sck), GPIO_FUNC_SPI); } +void spiInternalStopDMA (const extDevice_t *dev) +{ + dmaChannelDescriptor_t *dmaRx = dev->bus->dmaRx; + dmaChannelDescriptor_t *dmaTx = dev->bus->dmaTx; + + if (dmaRx && dma_channel_is_busy(dmaRx->channel)) { + // Abort active DMA - this should never happen + dma_channel_abort(dmaRx->channel); + } + + if (dmaTx && dma_channel_is_busy(dmaTx->channel)) { + // Abort active DMA - this should never happen as Tx should complete before Rx + dma_channel_abort(dmaTx->channel); + } +} + + +// Interrupt handler for SPI receive DMA completion +FAST_IRQ_HANDLER static void spiRxIrqHandler(dmaChannelDescriptor_t* descriptor) +{ + const extDevice_t *dev = (const extDevice_t *)descriptor->userParam; + + if (!dev) { + return; + } + + busDevice_t *bus = dev->bus; + + if (bus->curSegment->negateCS) { + // Negate Chip Select + IOHi(dev->busType_u.spi.csnPin); + } + + spiInternalStopDMA(dev); + + spiIrqHandler(dev); +} + +extern dmaChannelDescriptor_t dmaDescriptors[]; + void spiInitBusDMA(void) { - //TODO: implement - // if required to set up mappings of peripherals to DMA instances? - // can just start off with dma_claim_unused_channel in spiInternalInitStream? + for (uint32_t device = 0; device < SPIDEV_COUNT; device++) { + busDevice_t *bus = &spiBusDevice[device]; + int32_t channel_tx; + int32_t channel_rx; + + if (bus->busType != BUS_TYPE_SPI) { + // This bus is not in use + continue; + } + + channel_tx = dma_claim_unused_channel(true); + if (channel_tx == -1) { + // no more available channels so give up + return; + } + + channel_rx = dma_claim_unused_channel(true); + if (channel_rx == -1) { + // no more available channels so give up, first releasing the one + // channel we did claim + dma_channel_unclaim(channel_tx); + return; + } + + bus->dmaTx = &dmaDescriptors[DMA_CHANNEL_TO_INDEX(channel_tx)]; + bus->dmaTx->channel = channel_tx; + + bus->dmaRx = &dmaDescriptors[DMA_CHANNEL_TO_INDEX(channel_rx)]; + bus->dmaRx->channel = channel_rx; + + // The transaction concludes when the data has been received which will be after transmission is complete + dmaSetHandler(DMA_CHANNEL_TO_IDENTIFIER(bus->dmaRx->channel), spiRxIrqHandler, NVIC_PRIO_SPI_DMA, 0); + + // We got the required resources, so we can use DMA on this bus + bus->useDMA = true; + } } void spiInternalResetStream(dmaChannelDescriptor_t *descriptor) @@ -271,26 +347,26 @@ void spiInternalInitStream(const extDevice_t *dev, bool preInit) #else UNUSED(preInit); - int dma_tx = dma_claim_unused_channel(true); - int dma_rx = dma_claim_unused_channel(true); + busDevice_t *bus = dev->bus; - dev->bus->dmaTx->channel = dma_tx; - dev->bus->dmaRx->channel = dma_rx; - dev->bus->dmaTx->irqHandlerCallback = NULL; - dev->bus->dmaRx->irqHandlerCallback = spiInternalResetStream; // TODO: implement - correct callback + volatile busSegment_t *segment = bus->curSegment; const spiDevice_t *spi = &spiDevice[spiDeviceByInstance(dev->bus->busType_u.spi.instance)]; - dma_channel_config config = dma_channel_get_default_config(dma_tx); + dma_channel_config config = dma_channel_get_default_config(dev->bus->dmaTx->channel); channel_config_set_transfer_data_size(&config, DMA_SIZE_8); + channel_config_set_read_increment(&config, true); + channel_config_set_write_increment(&config, false); channel_config_set_dreq(&config, spi_get_dreq(SPI_INST(spi->dev), true)); - dma_channel_configure(dma_tx, &config, &spi_get_hw(SPI_INST(spi->dev))->dr, dev->txBuf, 0, false); + dma_channel_configure(dev->bus->dmaTx->channel, &config, &spi_get_hw(SPI_INST(spi->dev))->dr, segment->u.buffers.txData, 0, false); - config = dma_channel_get_default_config(dma_rx); + config = dma_channel_get_default_config(dev->bus->dmaRx->channel); channel_config_set_transfer_data_size(&config, DMA_SIZE_8); + channel_config_set_read_increment(&config, false); + channel_config_set_write_increment(&config, true); channel_config_set_dreq(&config, spi_get_dreq(SPI_INST(spi->dev), false)); - dma_channel_configure(dma_rx, &config, dev->rxBuf, &spi_get_hw(SPI_INST(spi->dev))->dr, 0, false); + dma_channel_configure(dev->bus->dmaRx->channel, &config, segment->u.buffers.rxData, &spi_get_hw(SPI_INST(spi->dev))->dr, 0, false); #endif } @@ -300,12 +376,13 @@ void spiInternalStartDMA(const extDevice_t *dev) #ifndef USE_DMA UNUSED(dev); #else + dev->bus->dmaRx->userParam = (uint32_t)dev; + // TODO check correct, was len + 1 now len dma_channel_set_trans_count(dev->bus->dmaTx->channel, dev->bus->curSegment->len, false); dma_channel_set_trans_count(dev->bus->dmaRx->channel, dev->bus->curSegment->len, false); - dma_channel_start(dev->bus->dmaTx->channel); - dma_channel_start(dev->bus->dmaRx->channel); + dma_start_channel_mask((1 << dev->bus->dmaTx->channel) | (1 << dev->bus->dmaRx->channel)); #endif } diff --git a/src/platform/PICO/dma_pico.c b/src/platform/PICO/dma_pico.c index 8b4634e4b1..0804f60abd 100644 --- a/src/platform/PICO/dma_pico.c +++ b/src/platform/PICO/dma_pico.c @@ -34,9 +34,6 @@ #include "hardware/dma.h" #include "hardware/irq.h" -volatile bool dma_irq0_handler_registered = false; -volatile bool dma_irq1_handler_registered = false; - dmaChannelDescriptor_t dmaDescriptors[DMA_LAST_HANDLER] = { DEFINE_DMA_CHANNEL(DMA_CH0_HANDLER), DEFINE_DMA_CHANNEL(DMA_CH1_HANDLER), @@ -103,7 +100,7 @@ dmaIdentifier_e dmaGetFreeIdentifier(void) void dmaSetHandler(dmaIdentifier_e identifier, dmaCallbackHandlerFuncPtr callback, uint32_t priority, uint32_t userParam) { - if (identifier < DMA_CH0_HANDLER || identifier > DMA_LAST_HANDLER) { + if (identifier < DMA_FIRST_HANDLER || identifier > DMA_LAST_HANDLER) { return; // Invalid identifier } @@ -129,22 +126,24 @@ void dmaSetHandler(dmaIdentifier_e identifier, dmaCallbackHandlerFuncPtr callbac const int index = DMA_IDENTIFIER_TO_INDEX(identifier); const uint32_t channel = dmaDescriptors[index].channel; + static bool dma_irqN_handler_registered[2]; + + if (!dma_irqN_handler_registered[core]) { + // Register the DMA IRQ handler if needed + dmaDescriptors[index].irqN = core ? DMA_IRQ_1_IRQn : DMA_IRQ_0_IRQn; + irq_handler_t irq_handler = core ? dma_irq1_handler : dma_irq0_handler; + + + irq_set_exclusive_handler(dmaDescriptors[index].irqN, irq_handler); + irq_set_enabled(dmaDescriptors[index].irqN, true); + + dma_irqN_handler_registered[core] = true; + } + if (core) { - // Core 1 uses DMA IRQ1 - if (!dma_irq1_handler_registered) { - irq_set_exclusive_handler(DMA_IRQ_1, dma_irq1_handler); - irq_set_enabled(DMA_IRQ_1, true); - dma_channel_set_irq1_enabled(channel, true); - dma_irq1_handler_registered = true; - } + dma_channel_set_irq1_enabled(channel, true); } else { - // Core 0 uses DMA IRQ0 - if (!dma_irq0_handler_registered) { - irq_set_exclusive_handler(DMA_IRQ_0, dma_irq0_handler); - irq_set_enabled(DMA_IRQ_0, true); - dma_channel_set_irq0_enabled(channel, true); - dma_irq0_handler_registered = true; - } + dma_channel_set_irq0_enabled(channel, true); } dmaDescriptors[index].irqHandlerCallback = callback; diff --git a/src/platform/PICO/include/platform/platform.h b/src/platform/PICO/include/platform/platform.h index 216d46222e..3baae8b741 100644 --- a/src/platform/PICO/include/platform/platform.h +++ b/src/platform/PICO/include/platform/platform.h @@ -61,7 +61,6 @@ typedef enum {DISABLE = 0, ENABLE = !DISABLE} FunctionalState; //#define SystemCoreClock //#define EXTI_TypeDef //#define EXTI_InitTypeDef -//#define IRQn_Type void* // We have to use SPI0_Type (or void) because config will pass in SPI0, SPI1, // which are defined in pico-sdk as SPI0_Type*. diff --git a/src/platform/PICO/target/RP2350B/target.h b/src/platform/PICO/target/RP2350B/target.h index 936ccd1b7e..202c60e637 100644 --- a/src/platform/PICO/target/RP2350B/target.h +++ b/src/platform/PICO/target/RP2350B/target.h @@ -53,6 +53,7 @@ #define USE_SPI #define USE_SPI_DEVICE_0 #define USE_SPI_DEVICE_1 +#define USE_SPI_DMA_ENABLE_LATE #define USE_I2C #define USE_I2C_DEVICE_0