Skip to content

Fix SPI dma problems; other cleanups #47

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 16, 2025
Merged

Fix SPI dma problems; other cleanups #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
143 changes: 84 additions & 59 deletions samd/dma.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@

#include <string.h>

#include "mphalport.h"
#include "py/gc.h"
#include "py/mphal.h"
#include "py/mpstate.h"

#include "hal/utils/include/utils.h"
Expand All @@ -51,19 +51,31 @@ COMPILER_ALIGNED(16) static DmacDescriptor write_back_descriptors[DMA_CHANNEL_CO

static bool dma_allocated[DMA_CHANNEL_COUNT];

uint8_t dma_allocate_channel(bool audio_channel) {
uint8_t channel;
uint8_t lim = audio_channel ? AUDIO_DMA_CHANNEL_COUNT : DMA_CHANNEL_COUNT;
for (channel = (audio_channel ? 0 : AUDIO_DMA_CHANNEL_COUNT); channel < lim; channel++) {
uint8_t dma_allocate_audio_channel(void) {
for (uint8_t channel = 0; channel < AUDIO_DMA_CHANNEL_COUNT; channel++) {
if (!dma_allocated[channel]) {
dma_allocated[channel] = true;
return channel;
}
}
return channel; // i.e., return failure
return NO_DMA_CHANNEL;
}

uint8_t dma_allocate_non_audio_channel(void) {
// Start allocating above audio channels.
for (uint8_t channel = AUDIO_DMA_CHANNEL_COUNT; channel < DMA_CHANNEL_COUNT; channel++) {
if (!dma_allocated[channel]) {
dma_allocated[channel] = true;
return channel;
}
}
return NO_DMA_CHANNEL;
}

void dma_free_channel(uint8_t channel) {
if (channel == NO_DMA_CHANNEL) {
return;
}
assert(dma_allocated[channel]);
dma_disable_channel(channel);
dma_allocated[channel] = false;
Expand All @@ -87,6 +99,7 @@ void init_shared_dma(void) {

DMAC->CTRL.reg = DMAC_CTRL_DMAENABLE | DMAC_CTRL_LVLEN0;

// Configure audio channels in advance.
// Non-audio channels will be configured on demand.
for (uint8_t i = 0; i < AUDIO_DMA_CHANNEL_COUNT; i++) {
dma_configure(i, 0, true);
Expand All @@ -97,21 +110,24 @@ void init_shared_dma(void) {
// If buffer_out is a real buffer, ignore tx.
// DMAs buffer_out -> dest
// DMAs src -> buffer_in
dma_descr_t shared_dma_transfer_start(void* peripheral,
const uint8_t* buffer_out, volatile uint32_t* dest,
volatile uint32_t* src, uint8_t* buffer_in,
uint32_t length, uint8_t tx) {
dma_descr_t res;
res.progress = 0;

uint8_t tx_channel = dma_allocate_channel(false);
uint8_t rx_channel = dma_allocate_channel(false);
void shared_dma_transfer_start(dma_transfer_t *transfer, void* peripheral, const uint8_t* buffer_out, volatile uint32_t* dest, volatile uint32_t* src, uint8_t* buffer_in, uint32_t length, uint8_t tx) {
transfer->progress = 0;

// There's always a tx DMA channel, though it may be sending only the tx byte if buffer_out is NULL.
uint8_t tx_channel = dma_allocate_non_audio_channel();
if (tx_channel == NO_DMA_CHANNEL) {
transfer->failure = DMA_FAILURE_NO_CHANNEL_AVAILABLE;
return;
}

if ((tx_channel >= DMA_CHANNEL_COUNT) || (rx_channel >= DMA_CHANNEL_COUNT) ||
!dma_channel_free(tx_channel) ||
(buffer_in != NULL && !dma_channel_free(rx_channel))) {
res.failure = -1;
return res;
// Allocate an rx dma channel only if we're going to read.
uint8_t rx_channel = NO_DMA_CHANNEL;
if (buffer_in != NULL) {
rx_channel = dma_allocate_non_audio_channel();
if (rx_channel == NO_DMA_CHANNEL) {
transfer->failure = DMA_FAILURE_NO_CHANNEL_AVAILABLE;
return;
}
}

uint32_t beat_size = DMAC_BTCTRL_BEATSIZE_BYTE;
Expand All @@ -122,10 +138,11 @@ dma_descr_t shared_dma_transfer_start(void* peripheral,
#ifdef SAM_D5X_E5X
if (peripheral == QSPI) {
// Check input alignment on word boundaries.
// NULLs will pass this test, so no need to check for them separately.
if ((((uint32_t) buffer_in) & 0x3) != 0 ||
(((uint32_t) buffer_out) & 0x3) != 0) {
res.failure = -3;
return res;
transfer->failure = DMA_FAILURE_ALIGNMENT;
return;
}
beat_size = DMAC_BTCTRL_BEATSIZE_WORD | DMAC_BTCTRL_SRCINC | DMAC_BTCTRL_DSTINC;
beat_length /= 4;
Expand All @@ -141,9 +158,10 @@ dma_descr_t shared_dma_transfer_start(void* peripheral,
} else {
#endif

// There is always a TX channel.
dma_configure(tx_channel, sercom_index(peripheral) * 2 + FIRST_SERCOM_TX_TRIGSRC, false);
tx_active = true;
if (buffer_in != NULL) {
if (rx_channel != NO_DMA_CHANNEL) {
dma_configure(rx_channel, sercom_index(peripheral) * 2 + FIRST_SERCOM_RX_TRIGSRC, false);
rx_active = true;
}
Expand Down Expand Up @@ -225,54 +243,58 @@ dma_descr_t shared_dma_transfer_start(void* peripheral,
is_okay = is_okay || (DMAC->ACTIVE.bit.ABUSY || complete);
}
if (!is_okay) {
for (int i = 0; i < AUDIO_DMA_CHANNEL_COUNT; i++) {
for (int i = 0; i < DMA_CHANNEL_COUNT; i++) {
if(DMAC->Channel[i].CHCTRLA.bit.ENABLE) {
DMAC->Channel[i].CHCTRLA.bit.ENABLE = 0;
DMAC->Channel[i].CHCTRLA.bit.ENABLE = 1;
}
}
}
#endif
res.peripheral = peripheral;
res.length = length;
res.rx_channel = rx_channel;
res.tx_channel = tx_channel;
res.rx_active = rx_active;
res.tx_active = tx_active;
res.sercom = sercom;
res.failure = 0;
return res;
transfer->peripheral = peripheral;
transfer->length = length;
transfer->rx_channel = rx_channel;
transfer->tx_channel = tx_channel;
transfer->rx_active = rx_active;
transfer->tx_active = tx_active;
transfer->sercom = sercom;
transfer->failure = 0;
transfer->progress = 0;
}

bool shared_dma_transfer_finished(dma_descr_t descr) {
if (descr.failure != 0) {
bool shared_dma_transfer_finished(dma_transfer_t* transfer) {
if (transfer->failure != 0) {
return true;
}

if (descr.progress < 1 && descr.rx_active) {
if ((dma_transfer_status(descr.rx_channel) & 0x3) == 0) {
if (transfer->progress < 1 && transfer->rx_active) {
if ((dma_transfer_status(transfer->rx_channel) & 0x3) == 0) {
// RX hasn't finished.
return false;
}
descr.progress = 1;
// RX has finished
transfer->progress = 1;
}
if (descr.progress < 2 && descr.tx_active) {
if ((dma_transfer_status(descr.tx_channel) & 0x3) == 0) {
if (transfer->progress < 2 && transfer->tx_active) {
if ((dma_transfer_status(transfer->tx_channel) & 0x3) == 0) {
// TX hasn't finished.
return false;
}
descr.progress = 2;
// TX has finished. RX has also finished, or there is no RX.
transfer->progress = 2;
}

if (descr.progress < 3 && descr.sercom) {
Sercom* s = (Sercom*) descr.peripheral;
if (transfer->progress < 3 && transfer->sercom) {
Sercom* s = (Sercom*) transfer->peripheral;
// Wait for the SPI transfer to complete.
if (s->SPI.INTFLAG.bit.TXC == 0) {
return false;
}
descr.progress = 3;
transfer->progress = 3;

// This transmit will cause the RX buffer overflow but we're OK with that.
// So, read the garbage and clear the overflow flag.
if (!descr.rx_active) {
if (!transfer->rx_active) {
while (s->SPI.INTFLAG.bit.RXC == 1) {
s->SPI.DATA.reg;
}
Expand All @@ -284,19 +306,20 @@ bool shared_dma_transfer_finished(dma_descr_t descr) {
return true;
}

int shared_dma_transfer_close(dma_descr_t descr) {
dma_free_channel(descr.tx_channel);
dma_free_channel(descr.rx_channel);
int shared_dma_transfer_close(dma_transfer_t* transfer) {
// It is not an error if either is NO_DMA_CHANNEL.
dma_free_channel(transfer->tx_channel);
dma_free_channel(transfer->rx_channel);

if (descr.failure != 0) {
return descr.failure;
if (transfer->failure != 0) {
return transfer->failure;
}

if ((!descr.rx_active || dma_transfer_status(descr.rx_channel) == DMAC_CHINTFLAG_TCMPL) &&
(!descr.tx_active || dma_transfer_status(descr.tx_channel) == DMAC_CHINTFLAG_TCMPL)) {
return descr.length;
if ((!transfer->rx_active || dma_transfer_status(transfer->rx_channel) == DMAC_CHINTFLAG_TCMPL) &&
(!transfer->tx_active || dma_transfer_status(transfer->tx_channel) == DMAC_CHINTFLAG_TCMPL)) {
return transfer->length;
}
return -2;
return DMA_FAILURE_INCOMPLETE;
}

// Do write and read simultaneously. If buffer_out is NULL, write the tx byte over and over.
Expand All @@ -307,12 +330,14 @@ static int32_t shared_dma_transfer(void* peripheral,
const uint8_t* buffer_out, volatile uint32_t* dest,
volatile uint32_t* src, uint8_t* buffer_in,
uint32_t length, uint8_t tx) {
dma_descr_t descr = shared_dma_transfer_start(peripheral, buffer_out, dest, src, buffer_in, length, tx);
if (descr.failure != 0) {
return descr.failure;
dma_transfer_t transfer;
shared_dma_transfer_start(&transfer, peripheral, buffer_out, dest, src, buffer_in, length, tx);
if (transfer.failure != 0) {
return transfer.failure;
}
while (!shared_dma_transfer_finished(descr)) {}
return shared_dma_transfer_close(descr);
while (!shared_dma_transfer_finished(&transfer)) {}

return shared_dma_transfer_close(&transfer);
}

int32_t sercom_dma_transfer(Sercom* sercom, const uint8_t* buffer_out, uint8_t* buffer_in,
Expand Down
31 changes: 22 additions & 9 deletions samd/dma.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,25 @@
// vm) because the general_dma resource will be shared between the REPL and SPI
// flash. Both uses must block each other in order to prevent conflict.
#define AUDIO_DMA_CHANNEL_COUNT 4

#ifdef SAMD21
#define DMA_CHANNEL_COUNT 12
#endif

#ifdef SAM_D5X_E5X
#define DMA_CHANNEL_COUNT 32
#endif

// Returned when a channel can't be allocated.
#define NO_DMA_CHANNEL 0xff

uint8_t dma_allocate_channel(bool audio_channel);
// Failure values
#define DMA_FAILURE_NO_CHANNEL_AVAILABLE (-1)
#define DMA_FAILURE_INCOMPLETE (-2)
#define DMA_FAILURE_ALIGNMENT (-3)

uint8_t dma_allocate_audio_channel(void);
uint8_t dma_allocate_non_audio_channel(void);
void dma_free_channel(uint8_t channel);

void init_shared_dma(void);
Expand All @@ -64,14 +80,11 @@ typedef struct {
bool tx_active;
bool sercom;
int8_t failure;
} dma_descr_t;

dma_descr_t shared_dma_transfer_start(void* peripheral,
const uint8_t* buffer_out, volatile uint32_t* dest,
volatile uint32_t* src, uint8_t* buffer_in,
uint32_t length, uint8_t tx);
bool shared_dma_transfer_finished(dma_descr_t descr);
int shared_dma_transfer_close(dma_descr_t descr);
} dma_transfer_t;

void shared_dma_transfer_start(dma_transfer_t* transfer, void* peripheral, const uint8_t* buffer_out, volatile uint32_t* dest, volatile uint32_t* src, uint8_t* buffer_in, uint32_t length, uint8_t tx);
bool shared_dma_transfer_finished(dma_transfer_t* transfer);
int shared_dma_transfer_close(dma_transfer_t* transfer);

void dma_configure(uint8_t channel_number, uint8_t trigsrc, bool output_event);
void dma_enable_channel(uint8_t channel_number);
Expand Down
6 changes: 0 additions & 6 deletions samd/samd21/dma.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ void dma_configure(uint8_t channel_number, uint8_t trigsrc, bool output_event) {

void dma_enable_channel(uint8_t channel_number) {
common_hal_mcu_disable_interrupts();
/** Select the DMA channel and clear software trigger */
DMAC->CHID.reg = DMAC_CHID_ID(channel_number);
// Clear any previous interrupts.
DMAC->CHINTFLAG.reg = DMAC_CHINTFLAG_MASK;
Expand All @@ -68,23 +67,20 @@ void dma_enable_channel(uint8_t channel_number) {

void dma_disable_channel(uint8_t channel_number) {
common_hal_mcu_disable_interrupts();
/** Select the DMA channel and clear software trigger */
DMAC->CHID.reg = DMAC_CHID_ID(channel_number);
DMAC->CHCTRLA.bit.ENABLE = false;
common_hal_mcu_enable_interrupts();
}

void dma_suspend_channel(uint8_t channel_number) {
common_hal_mcu_disable_interrupts();
/** Select the DMA channel and clear software trigger */
DMAC->CHID.reg = DMAC_CHID_ID(channel_number);
DMAC->CHCTRLB.bit.CMD = DMAC_CHCTRLB_CMD_SUSPEND_Val;
common_hal_mcu_enable_interrupts();
}

void dma_resume_channel(uint8_t channel_number) {
common_hal_mcu_disable_interrupts();
/** Select the DMA channel and clear software trigger */
DMAC->CHID.reg = DMAC_CHID_ID(channel_number);
DMAC->CHCTRLB.bit.CMD = DMAC_CHCTRLB_CMD_RESUME_Val;
DMAC->CHINTFLAG.reg = DMAC_CHINTFLAG_SUSP;
Expand All @@ -93,7 +89,6 @@ void dma_resume_channel(uint8_t channel_number) {

bool dma_channel_enabled(uint8_t channel_number) {
common_hal_mcu_disable_interrupts();
/** Select the DMA channel and clear software trigger */
DMAC->CHID.reg = DMAC_CHID_ID(channel_number);
bool enabled = DMAC->CHCTRLA.bit.ENABLE;
common_hal_mcu_enable_interrupts();
Expand All @@ -102,7 +97,6 @@ bool dma_channel_enabled(uint8_t channel_number) {

uint8_t dma_transfer_status(uint8_t channel_number) {
common_hal_mcu_disable_interrupts();
/** Select the DMA channel and clear software trigger */
DMAC->CHID.reg = DMAC_CHID_ID(channel_number);
uint8_t status = DMAC->CHINTFLAG.reg;
common_hal_mcu_enable_interrupts();
Expand Down