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

Skip to content

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