Skip to content

Commit 863e615

Browse files
tannewttannewt
authored
Merge pull request #47 from adafruit/spi-dma-fixes
Fix SPI dma problems; other cleanups
2 parents d321022 + a1f1504 commit 863e615

File tree

3 files changed

+106
-74
lines changed

3 files changed

+106
-74
lines changed

‎samd/dma.c‎

Lines changed: 84 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@
2727

2828
#include <string.h>
2929

30-
#include "mphalport.h"
3130
#include "py/gc.h"
31+
#include "py/mphal.h"
3232
#include "py/mpstate.h"
3333

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

5252
static bool dma_allocated[DMA_CHANNEL_COUNT];
5353

54-
uint8_t dma_allocate_channel(bool audio_channel) {
55-
uint8_t channel;
56-
uint8_t lim = audio_channel ? AUDIO_DMA_CHANNEL_COUNT : DMA_CHANNEL_COUNT;
57-
for (channel = (audio_channel ? 0 : AUDIO_DMA_CHANNEL_COUNT); channel < lim; channel++) {
54+
uint8_t dma_allocate_audio_channel(void) {
55+
for (uint8_t channel = 0; channel < AUDIO_DMA_CHANNEL_COUNT; channel++) {
5856
if (!dma_allocated[channel]) {
5957
dma_allocated[channel] = true;
6058
return channel;
6159
}
6260
}
63-
return channel; // i.e., return failure
61+
return NO_DMA_CHANNEL;
62+
}
63+
64+
uint8_t dma_allocate_non_audio_channel(void) {
65+
// Start allocating above audio channels.
66+
for (uint8_t channel = AUDIO_DMA_CHANNEL_COUNT; channel < DMA_CHANNEL_COUNT; channel++) {
67+
if (!dma_allocated[channel]) {
68+
dma_allocated[channel] = true;
69+
return channel;
70+
}
71+
}
72+
return NO_DMA_CHANNEL;
6473
}
6574

6675
void dma_free_channel(uint8_t channel) {
76+
if (channel == NO_DMA_CHANNEL) {
77+
return;
78+
}
6779
assert(dma_allocated[channel]);
6880
dma_disable_channel(channel);
6981
dma_allocated[channel] = false;
@@ -87,6 +99,7 @@ void init_shared_dma(void) {
8799

88100
DMAC->CTRL.reg = DMAC_CTRL_DMAENABLE | DMAC_CTRL_LVLEN0;
89101

102+
// Configure audio channels in advance.
90103
// Non-audio channels will be configured on demand.
91104
for (uint8_t i = 0; i < AUDIO_DMA_CHANNEL_COUNT; i++) {
92105
dma_configure(i, 0, true);
@@ -97,21 +110,24 @@ void init_shared_dma(void) {
97110
// If buffer_out is a real buffer, ignore tx.
98111
// DMAs buffer_out -> dest
99112
// DMAs src -> buffer_in
100-
dma_descr_t shared_dma_transfer_start(void* peripheral,
101-
const uint8_t* buffer_out, volatile uint32_t* dest,
102-
volatile uint32_t* src, uint8_t* buffer_in,
103-
uint32_t length, uint8_t tx) {
104-
dma_descr_t res;
105-
res.progress = 0;
106-
107-
uint8_t tx_channel = dma_allocate_channel(false);
108-
uint8_t rx_channel = dma_allocate_channel(false);
113+
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) {
114+
transfer->progress = 0;
115+
116+
// There's always a tx DMA channel, though it may be sending only the tx byte if buffer_out is NULL.
117+
uint8_t tx_channel = dma_allocate_non_audio_channel();
118+
if (tx_channel == NO_DMA_CHANNEL) {
119+
transfer->failure = DMA_FAILURE_NO_CHANNEL_AVAILABLE;
120+
return;
121+
}
109122

110-
if ((tx_channel >= DMA_CHANNEL_COUNT) || (rx_channel >= DMA_CHANNEL_COUNT) ||
111-
!dma_channel_free(tx_channel) ||
112-
(buffer_in != NULL && !dma_channel_free(rx_channel))) {
113-
res.failure = -1;
114-
return res;
123+
// Allocate an rx dma channel only if we're going to read.
124+
uint8_t rx_channel = NO_DMA_CHANNEL;
125+
if (buffer_in != NULL) {
126+
rx_channel = dma_allocate_non_audio_channel();
127+
if (rx_channel == NO_DMA_CHANNEL) {
128+
transfer->failure = DMA_FAILURE_NO_CHANNEL_AVAILABLE;
129+
return;
130+
}
115131
}
116132

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

161+
// There is always a TX channel.
144162
dma_configure(tx_channel, sercom_index(peripheral) * 2 + FIRST_SERCOM_TX_TRIGSRC, false);
145163
tx_active = true;
146-
if (buffer_in != NULL) {
164+
if (rx_channel != NO_DMA_CHANNEL) {
147165
dma_configure(rx_channel, sercom_index(peripheral) * 2 + FIRST_SERCOM_RX_TRIGSRC, false);
148166
rx_active = true;
149167
}
@@ -225,54 +243,58 @@ dma_descr_t shared_dma_transfer_start(void* peripheral,
225243
is_okay = is_okay || (DMAC->ACTIVE.bit.ABUSY || complete);
226244
}
227245
if (!is_okay) {
228-
for (int i = 0; i < AUDIO_DMA_CHANNEL_COUNT; i++) {
246+
for (int i = 0; i < DMA_CHANNEL_COUNT; i++) {
229247
if(DMAC->Channel[i].CHCTRLA.bit.ENABLE) {
230248
DMAC->Channel[i].CHCTRLA.bit.ENABLE = 0;
231249
DMAC->Channel[i].CHCTRLA.bit.ENABLE = 1;
232250
}
233251
}
234252
}
235253
#endif
236-
res.peripheral = peripheral;
237-
res.length = length;
238-
res.rx_channel = rx_channel;
239-
res.tx_channel = tx_channel;
240-
res.rx_active = rx_active;
241-
res.tx_active = tx_active;
242-
res.sercom = sercom;
243-
res.failure = 0;
244-
return res;
254+
transfer->peripheral = peripheral;
255+
transfer->length = length;
256+
transfer->rx_channel = rx_channel;
257+
transfer->tx_channel = tx_channel;
258+
transfer->rx_active = rx_active;
259+
transfer->tx_active = tx_active;
260+
transfer->sercom = sercom;
261+
transfer->failure = 0;
262+
transfer->progress = 0;
245263
}
246264

247-
bool shared_dma_transfer_finished(dma_descr_t descr) {
248-
if (descr.failure != 0) {
265+
bool shared_dma_transfer_finished(dma_transfer_t* transfer) {
266+
if (transfer->failure != 0) {
249267
return true;
250268
}
251269

252-
if (descr.progress < 1 && descr.rx_active) {
253-
if ((dma_transfer_status(descr.rx_channel) & 0x3) == 0) {
270+
if (transfer->progress < 1 && transfer->rx_active) {
271+
if ((dma_transfer_status(transfer->rx_channel) & 0x3) == 0) {
272+
// RX hasn't finished.
254273
return false;
255274
}
256-
descr.progress = 1;
275+
// RX has finished
276+
transfer->progress = 1;
257277
}
258-
if (descr.progress < 2 && descr.tx_active) {
259-
if ((dma_transfer_status(descr.tx_channel) & 0x3) == 0) {
278+
if (transfer->progress < 2 && transfer->tx_active) {
279+
if ((dma_transfer_status(transfer->tx_channel) & 0x3) == 0) {
280+
// TX hasn't finished.
260281
return false;
261282
}
262-
descr.progress = 2;
283+
// TX has finished. RX has also finished, or there is no RX.
284+
transfer->progress = 2;
263285
}
264286

265-
if (descr.progress < 3 && descr.sercom) {
266-
Sercom* s = (Sercom*) descr.peripheral;
287+
if (transfer->progress < 3 && transfer->sercom) {
288+
Sercom* s = (Sercom*) transfer->peripheral;
267289
// Wait for the SPI transfer to complete.
268290
if (s->SPI.INTFLAG.bit.TXC == 0) {
269291
return false;
270292
}
271-
descr.progress = 3;
293+
transfer->progress = 3;
272294

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

287-
int shared_dma_transfer_close(dma_descr_t descr) {
288-
dma_free_channel(descr.tx_channel);
289-
dma_free_channel(descr.rx_channel);
309+
int shared_dma_transfer_close(dma_transfer_t* transfer) {
310+
// It is not an error if either is NO_DMA_CHANNEL.
311+
dma_free_channel(transfer->tx_channel);
312+
dma_free_channel(transfer->rx_channel);
290313

291-
if (descr.failure != 0) {
292-
return descr.failure;
314+
if (transfer->failure != 0) {
315+
return transfer->failure;
293316
}
294317

295-
if ((!descr.rx_active || dma_transfer_status(descr.rx_channel) == DMAC_CHINTFLAG_TCMPL) &&
296-
(!descr.tx_active || dma_transfer_status(descr.tx_channel) == DMAC_CHINTFLAG_TCMPL)) {
297-
return descr.length;
318+
if ((!transfer->rx_active || dma_transfer_status(transfer->rx_channel) == DMAC_CHINTFLAG_TCMPL) &&
319+
(!transfer->tx_active || dma_transfer_status(transfer->tx_channel) == DMAC_CHINTFLAG_TCMPL)) {
320+
return transfer->length;
298321
}
299-
return -2;
322+
return DMA_FAILURE_INCOMPLETE;
300323
}
301324

302325
// Do write and read simultaneously. If buffer_out is NULL, write the tx byte over and over.
@@ -307,12 +330,14 @@ static int32_t shared_dma_transfer(void* peripheral,
307330
const uint8_t* buffer_out, volatile uint32_t* dest,
308331
volatile uint32_t* src, uint8_t* buffer_in,
309332
uint32_t length, uint8_t tx) {
310-
dma_descr_t descr = shared_dma_transfer_start(peripheral, buffer_out, dest, src, buffer_in, length, tx);
311-
if (descr.failure != 0) {
312-
return descr.failure;
333+
dma_transfer_t transfer;
334+
shared_dma_transfer_start(&transfer, peripheral, buffer_out, dest, src, buffer_in, length, tx);
335+
if (transfer.failure != 0) {
336+
return transfer.failure;
313337
}
314-
while (!shared_dma_transfer_finished(descr)) {}
315-
return shared_dma_transfer_close(descr);
338+
while (!shared_dma_transfer_finished(&transfer)) {}
339+
340+
return shared_dma_transfer_close(&transfer);
316341
}
317342

318343
int32_t sercom_dma_transfer(Sercom* sercom, const uint8_t* buffer_out, uint8_t* buffer_in,

‎samd/dma.h‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,25 @@
3636
// vm) because the general_dma resource will be shared between the REPL and SPI
3737
// flash. Both uses must block each other in order to prevent conflict.
3838
#define AUDIO_DMA_CHANNEL_COUNT 4
39+
40+
#ifdef SAMD21
41+
#define DMA_CHANNEL_COUNT 12
42+
#endif
43+
44+
#ifdef SAM_D5X_E5X
3945
#define DMA_CHANNEL_COUNT 32
46+
#endif
47+
48+
// Returned when a channel can't be allocated.
49+
#define NO_DMA_CHANNEL 0xff
4050

41-
uint8_t dma_allocate_channel(bool audio_channel);
51+
// Failure values
52+
#define DMA_FAILURE_NO_CHANNEL_AVAILABLE (-1)
53+
#define DMA_FAILURE_INCOMPLETE (-2)
54+
#define DMA_FAILURE_ALIGNMENT (-3)
55+
56+
uint8_t dma_allocate_audio_channel(void);
57+
uint8_t dma_allocate_non_audio_channel(void);
4258
void dma_free_channel(uint8_t channel);
4359

4460
void init_shared_dma(void);
@@ -64,14 +80,11 @@ typedef struct {
6480
bool tx_active;
6581
bool sercom;
6682
int8_t failure;
67-
} dma_descr_t;
68-
69-
dma_descr_t shared_dma_transfer_start(void* peripheral,
70-
const uint8_t* buffer_out, volatile uint32_t* dest,
71-
volatile uint32_t* src, uint8_t* buffer_in,
72-
uint32_t length, uint8_t tx);
73-
bool shared_dma_transfer_finished(dma_descr_t descr);
74-
int shared_dma_transfer_close(dma_descr_t descr);
83+
} dma_transfer_t;
84+
85+
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);
86+
bool shared_dma_transfer_finished(dma_transfer_t* transfer);
87+
int shared_dma_transfer_close(dma_transfer_t* transfer);
7588

7689
void dma_configure(uint8_t channel_number, uint8_t trigsrc, bool output_event);
7790
void dma_enable_channel(uint8_t channel_number);

‎samd/samd21/dma.c‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ void dma_configure(uint8_t channel_number, uint8_t trigsrc, bool output_event) {
5858

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

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

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

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

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

10398
uint8_t dma_transfer_status(uint8_t channel_number) {
10499
common_hal_mcu_disable_interrupts();
105-
/** Select the DMA channel and clear software trigger */
106100
DMAC->CHID.reg = DMAC_CHID_ID(channel_number);
107101
uint8_t status = DMAC->CHINTFLAG.reg;
108102
common_hal_mcu_enable_interrupts();

0 commit comments

Comments
 (0)