8000 Merge pull request #4974 from dhalbert/rp2040-audio-and-spi-fixes · PontusO/circuitpython@e51d5e4 · GitHub
[go: up one dir, main page]

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit e51d5e4

Browse files
authored
Merge pull request adafruit#4974 from dhalbert/rp2040-audio-and-spi-fixes
Rp2040 audio fixes; disallow ctrl-C interrupts of SPI and PIO.
2 parents 45ed2f4 + ab52a92 commit e51d5e4

File tree

7 files changed

+71
-62
lines changed

7 files changed

+71
-62
lines changed

ports/raspberrypi/audio_dma.c

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -169,15 +169,20 @@ audio_dma_result audio_dma_setup_playback(audio_dma_t *dma,
169169
uint8_t dma_trigger_source) {
170170
// Use two DMA channels to play because the DMA can't wrap to itself without the
171171
// buffer being power of two aligned.
172-
dma->channel[0] = dma_claim_unused_channel(false);
173-
dma->channel[1] = dma_claim_unused_channel(false);
174-
if (dma->channel[0] == NUM_DMA_CHANNELS || dma->channel[1] == NUM_DMA_CHANNELS) {
175-
if (dma->channel[0] < NUM_DMA_CHANNELS) {
176-
dma_channel_unclaim(dma->channel[0]);
177-
}
172+
int dma_channel_0_maybe = dma_claim_unused_channel(false);
173+
if (dma_channel_0_maybe < 0) {
178174
return AUDIO_DMA_DMA_BUSY;
179175
}
180176

177+
int dma_channel_1_maybe = dma_claim_unused_channel(false);
178+
if (dma_channel_1_maybe < 0) {
179+
dma_channel_unclaim((uint)dma_channel_0_maybe);
180+
return AUDIO_DMA_DMA_BUSY;
181+
}
182+
183+
dma->channel[0] = (uint8_t)dma_channel_0_maybe;
184+
dma->channel[1] = (uint8_t)dma_channel_1_maybe;
185+
181186
dma->sample = sample;
182187
dma->loop = loop;
183188
dma->single_channel_output = single_channel_output;

ports/raspberrypi/common-hal/audiobusio/I2SOut.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,14 +145,21 @@ void common_hal_audiobusio_i2sout_deinit(audiobusio_i2sout_obj_t *self) {
145145
return;
146146
}
147147

148+
if (common_hal_audiobusio_i2sout_get_playing(self)) {
149+
common_hal_audiobusio_i2sout_stop(self);
150+
}
151+
148152
common_hal_rp2pio_statemachine_deinit(&self->state_machine);
153+
154+
audio_dma_deinit(&self->dma);
149155
}
150156

151157
void common_hal_audiobusio_i2sout_play(audiobusio_i2sout_obj_t *self,
152158
mp_obj_t sample, bool loop) {
153159
if (common_hal_audiobusio_i2sout_get_playing(self)) {
154160
common_hal_audiobusio_i2sout_stop(self);
155161
}
162+
156163
uint8_t bits_per_sample = audiosample_bits_per_sample(sample);
157164
// Make sure we transmit a minimum of 16 bits.
158165
// TODO: Maybe we need an intermediate object to upsample instead. This is

ports/raspberrypi/common-hal/audiopwmio/PWMAudioOut.c

Lines changed: 47 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,12 @@
4646

4747
#define NUM_DMA_TIMERS 4
4848

49+
// The PWM clock frequency is base_clock_rate / PWM_TOP, typically 125_000_000 / PWM_TOP.
50+
// We pick BITS_PER_SAMPLE so we get a clock frequency that is above what would cause aliasing.
51+
#define BITS_PER_SAMPLE 10
52+
#define SAMPLE_BITS_TO_DISCARD (16 - BITS_PER_SAMPLE)
53+
#define PWM_TOP ((1 << BITS_PER_SAMPLE) - 1)
54+
4955
void audiopwmout_reset() {
5056
for (size_t i = 0; i < NUM_DMA_TIMERS; i++) {
5157
dma_hw->timer[i] = 0;
@@ -55,7 +61,10 @@ void audiopwmout_reset() {
5561
// Caller validates that pins are free.
5662
void common_hal_audiopwmio_pwmaudioout_construct(audiopwmio_pwmaudioout_obj_t *self,
5763
const mcu_pin_obj_t *left_channel, const mcu_pin_obj_t *right_channel, uint16_t quiescent_value) {
58-
if (left_channel != NULL && right_channel != NULL) {
64+
65+
self->stereo = right_channel != NULL;
66+
67+
if (self->stereo) {
5968
if (pwm_gpio_to_slice_num(left_channel->number) != pwm_gpio_to_slice_num(right_channel->number)) {
6069
mp_raise_ValueError(translate("Pins must share PWM slice"));
6170
}
@@ -72,22 +81,20 @@ void common_hal_audiopwmio_pwmaudioout_construct(audiopwmio_pwmaudioout_obj_t *s
7281
// we want. ;-) We mark ourselves variable only if we're a mono output to
7382
// prevent other PWM use on the other channel. If stereo, we say fixed
7483
// frequency so we can allocate with ourselves.
75-
bool mono = right_channel == NULL;
7684

77-
// We don't actually know our frequency yet so just pick one that shouldn't
78-
// match anyone else. (We'll only know the frequency once we play something
79-
// back.)
80-
uint32_t frequency = 12500;
85+
// We don't actually know our frequency yet. It is set when
86+
// pwmio_pwmout_set_top() is called. This value is unimportant; it just needs to be valid.
87+
const uint32_t frequency = 12000000;
8188

8289
// Make sure the PWMOut's are "deinited" by default.
8390
self->left_pwm.pin = NULL;
8491
self->right_pwm.pin = NULL;
8592

86-
pwmout_result_t result = common_hal_pwmio_pwmout_construct(&self->left_pwm,
87-
left_channel, 0, frequency, mono);
93+
pwmout_result_t result =
94+
common_hal_pwmio_pwmout_construct(&self->left_pwm, left_channel, 0, frequency, !self->stereo);
8895
if (result == PWMOUT_OK && right_channel != NULL) {
89-
result = common_hal_pwmio_pwmout_construct(&self->right_pwm,
90-
right_channel, 0, frequency, false);
96+
result =
97+
common_hal_pwmio_pwmout_construct(&self->right_pwm, right_channel, 0, frequency, false);
9198
if (result != PWMOUT_OK) {
9299
common_hal_pwmio_pwmout_deinit(&self->left_pwm);
93100
}
@@ -96,10 +103,16 @@ void common_hal_audiopwmio_pwmaudioout_construct(audiopwmio_pwmaudioout_obj_t *s
96103
mp_raise_RuntimeError(translate("All timers in use"));
97104
}
98105

106+
self->quiescent_value = quiescent_value >> SAMPLE_BITS_TO_DISCARD;
107+
common_hal_pwmio_pwmout_set_duty_cycle(&self->left_pwm, self->quiescent_value);
108+
pwmio_pwmout_set_top(&self->left_pwm, PWM_TOP);
109+
if (self->stereo) {
110+
common_hal_pwmio_pwmout_set_duty_cycle(&self->right_pwm, self->quiescent_value);
111+
pwmio_pwmout_set_top(&self->right_pwm, PWM_TOP);
112+
}
113+
99114
audio_dma_init(&self->dma);
100115
self->pacing_timer = NUM_DMA_TIMERS;
101-
102-
self->quiescent_value = quiescent_value;
103116
}
104117

105118
bool common_hal_audiopwmio_pwmaudioout_deinited(audiopwmio_pwmaudioout_obj_t *self) {
@@ -110,6 +123,7 @@ void common_hal_audiopwmio_pwmaudioout_deinit(audiopwmio_pwmaudioout_obj_t *self
110123
if (common_hal_audiopwmio_pwmaudioout_deinited(self)) {
111124
return;
112125
}
126+
113127
if (common_hal_audiopwmio_pwmaudioout_get_playing(self)) {
114128
common_hal_audiopwmio_pwmaudioout_stop(self);
115129
}
@@ -139,29 +153,28 @@ void common_hal_audiopwmio_pwmaudioout_play(audiopwmio_pwmaudioout_obj_t *self,
139153
mp_raise_RuntimeError(translate("No DMA pacing timer found"));
140154
}
141155
uint32_t tx_register = (uint32_t)&pwm_hw->slice[self->left_pwm.slice].cc;
142-
if (common_hal_pwmio_pwmout_deinited(&self->right_pwm)) {
143-
// Shift the destination if we are outputting to the second PWM channel.
156+
if (self->stereo) {
157+
// Shift the destination if we are outputting to both PWM channels.
144158
tx_register += self->left_pwm.channel * sizeof(uint16_t);
145159
}
146160

147-
pwmio_pwmout_set_top(&self->left_pwm, 1023);
148-
149161
audio_dma_result result = audio_dma_setup_playback(
150162
&self->dma,
151163
sample,
152164
loop,
153165
false, // single channel
154166
0, // audio channel
155167
false, // output signed
156-
10,
157-
(uint32_t)tx_register, // output register
168+
BITS_PER_SAMPLE,
169+
(uint32_t)tx_register, // output register: PWM cc register
158170
0x3b + pacing_timer); // data request line
159171

160172
if (result == AUDIO_DMA_DMA_BUSY) {
161-
// common_hal_audiobusio_i2sout_stop(self);
173+
common_hal_audiopwmio_pwmaudioout_stop(self);
162174
mp_raise_RuntimeError(translate("No DMA channel found"));
163-
} else if (result == AUDIO_DMA_MEMORY_ERROR) {
164-
// common_hal_audiobusio_i2sout_stop(self);
175+
}
176+
if (result == AUDIO_DMA_MEMORY_ERROR) {
177+
common_hal_audiopwmio_pwmaudioout_stop(self);
165178
mp_raise_RuntimeError(translate("Unable to allocate buffers for signed conversion"));
166179
}
167180

@@ -177,6 +190,7 @@ void common_hal_audiopwmio_pwmaudioout_play(audiopwmio_pwmaudioout_obj_t *self,
177190
// are 16-bit.
178191

179192
uint32_t sample_rate = audiosample_sample_rate(sample);
193+
180194
uint32_t system_clock = common_hal_mcu_processor_get_frequency();
181195
uint32_t best_numerator = 0;
182196
uint32_t best_denominator = 0;
@@ -204,19 +218,25 @@ void common_hal_audiopwmio_pwmaudioout_play(audiopwmio_pwmaudioout_obj_t *self,
204218
}
205219

206220
void common_hal_audiopwmio_pwmaudioout_stop(audiopwmio_pwmaudioout_obj_t *self) {
207-
dma_hw->timer[self->pacing_timer] = 0;
208-
self->pacing_timer = NUM_DMA_TIMERS;
221+
if (self->pacing_timer < NUM_DMA_TIMERS) {
222+
dma_hw->timer[self->pacing_timer] = 0;
223+
self->pacing_timer = NUM_DMA_TIMERS;
224+
}
209225

210226
audio_dma_stop(&self->dma);
227+
228+
// Set to quiescent level.
229+
pwm_hw->slice[self->left_pwm.slice].cc = self->quiescent_value;
230+
if (self->stereo) {
231+
pwm_hw->slice[self->right_pwm.slice].cc = self->quiescent_value;
232+
}
211233
}
212234

213235
bool common_hal_audiopwmio_pwmaudioout_get_playing(audiopwmio_pwmaudioout_obj_t *self) {
214236
bool playing = audio_dma_get_playing(&self->dma);
215-
if (!playing && self->pacing_timer < NUM_DMA_TIMERS) {
216-
dma_hw->timer[self->pacing_timer] = 0;
217-
self->pacing_timer = NUM_DMA_TIMERS;
218237

219-
audio_dma_stop(&self->dma);
238+
if (!playing && self->pacing_timer < NUM_DMA_TIMERS) {
239+
common_hal_audiopwmio_pwmaudioout_stop(self);
220240
}
221241

222242
return playing;

ports/raspberrypi/common-hal/audiopwmio/PWMAudioOut.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ typedef struct {
3838
audio_dma_t dma;
3939
uint16_t quiescent_value;
4040
uint8_t pacing_timer;
41+
bool stereo; // if false, only using left_pwm.
4142
} audiopwmio_pwmaudioout_obj_t;
4243

4344
void audiopwmout_reset(void);

ports/raspberrypi/common-hal/busio/SPI.c

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -212,15 +212,6 @@ static bool _transfer(busio_spi_obj_t *self,
212212
while (dma_channel_is_busy(chan_rx) || dma_channel_is_busy(chan_tx)) {
213213
// TODO: We should idle here until we get a DMA interrupt or something else.
214214
RUN_BACKGROUND_TASKS;
215-
if (mp_hal_is_interrupted()) {
216-
if (dma_channel_is_busy(chan_rx)) {
217-
dma_channel_abort(chan_rx);
218-
}
219-
if (dma_channel_is_busy(chan_tx)) {
220-
dma_channel_abort(chan_tx);
221-
}
222-
break;
223-
}
224215
}
225216
}
226217

@@ -233,15 +224,15 @@ static bool _transfer(busio_spi_obj_t *self,
233224
dma_channel_unclaim(chan_tx);
234225
}
235226

236-
if (!use_dma && !mp_hal_is_interrupted()) {
227+
if (!use_dma) {
237228
// Use software for small transfers, or if couldn't claim two DMA channels
238229
// Never have more transfers in flight than will fit into the RX FIFO,
239230
// else FIFO will overflow if this code is heavily interrupted.
240231
const size_t fifo_depth = 8;
241232
size_t rx_remaining = len;
242233
size_t tx_remaining = len;
243234

244-
while (!mp_hal_is_interrupted() && (rx_remaining || tx_remaining)) {
235+
while (rx_remaining || tx_remaining) {
245236
if (tx_remaining && spi_is_writable(self->peripheral) && rx_remaining - tx_remaining < fifo_depth) {
246237
spi_get_hw(self->peripheral)->dr = (uint32_t)*data_out;
247238
// Increment only if the buffer is the transfer length. It's 1 otherwise.

ports/raspberrypi/common-hal/rp2pio/StateMachine.c

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -654,15 +654,6 @@ static bool _transfer(rp2pio_statemachine_obj_t *self,
654654
(tx && dma_channel_is_busy(chan_tx))) {
655655
// TODO: We should idle here until we get a DMA interrupt or something else.
656656
RUN_BACKGROUND_TASKS;
657-
if (mp_hal_is_interrupted()) {
658-
if (rx && dma_channel_is_busy(chan_rx)) {
659-
dma_channel_abort(chan_rx);
660-
}
661-
if (tx && dma_channel_is_busy(chan_tx)) {
662-
dma_channel_abort(chan_tx);
663-
}
664-
break;
665-
}
666657
}
667658
// Clear the stall bit so we can detect when the state machine is done transmitting.
668659
self->pio->fdebug = stall_mask;
@@ -677,7 +668,7 @@ static bool _transfer(rp2pio_statemachine_obj_t *self,
677668
dma_channel_unclaim(chan_tx);
678669
}
679670

680-
if (!use_dma && !mp_hal_is_interrupted()) {
671+
if (!use_dma) {
681672
// Use software for small transfers, or if couldn't claim two DMA channels
682673
size_t rx_remaining = in_len / in_stride_in_bytes;
683674
size_t tx_remaining = out_len / out_stride_in_bytes;
@@ -706,9 +697,6 @@ static bool _transfer(rp2pio_statemachine_obj_t *self,
706697
--rx_remaining;
707698
}
708699
RUN_BACKGROUND_TASKS;
709-
if (mp_hal_is_interrupted()) {
710-
break;
711-
}
712700
}
713701
// Clear the stall bit so we can detect when the state machine is done transmitting.
714702
self->pio->fdebug = stall_mask;
@@ -719,9 +707,6 @@ static bool _transfer(rp2pio_statemachine_obj_t *self,
719707
while (!pio_sm_is_tx_fifo_empty(self->pio, self->state_machine) ||
720708
(self->wait_for_txstall && (self->pio->fdebug & stall_mask) == 0)) {
721709
RUN_BACKGROUND_TASKS;
722-
if (mp_hal_is_interrupted()) {
723-
break;
724-
}
725710
}
726711
}
727712
return true;

ports/raspberrypi/mpconfigport.mk

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,13 @@ CIRCUITPY_ALARM ?= 1
2121

2222
CIRCUITPY_RP2PIO ?= 1
2323
CIRCUITPY_NEOPIXEL_WRITE ?= $(CIRCUITPY_RP2PIO)
24-
CIRCUITPY_FRAMEBUFFERIO ?= 1
24+
CIRCUITPY_FRAMEBUFFERIO ?= $(CIRCUITPY_DISPLAYIO)
2525
CIRCUITPY_FULL_BUILD ?= 1
2626
CIRCUITPY_AUDIOMP3 ?= 1
2727
CIRCUITPY_BITOPS ?= 1
2828
CIRCUITPY_IMAGECAPTURE ?= 1
2929
CIRCUITPY_PWMIO ?= 1
30-
CIRCUITPY_RGBMATRIX ?= 1
30+
CIRCUITPY_RGBMATRIX ?= $(CIRCUITPY_DISPLAYIO)
3131
CIRCUITPY_ROTARYIO ?= 1
3232
CIRCUITPY_ROTARYIO_SOFTENCODER = 1
3333

0 commit comments

Comments
 (0)
0