Skip to content

Rp2040 audio fixes; disallow ctrl-C interrupts of SPI and PIO. #4974

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.

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 3 commits into from
Jul 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 11 additions & 6 deletions ports/raspberrypi/audio_dma.c
Original file line number Diff line number Diff line change
Expand Up @@ -169,15 +169,20 @@ audio_dma_result audio_dma_setup_playback(audio_dma_t *dma,
uint8_t dma_trigger_source) {
// Use two DMA channels to play because the DMA can't wrap to itself without the
// buffer being power of two aligned.
dma->channel[0] = dma_claim_unused_channel(false);
dma->channel[1] = dma_claim_unused_channel(false);
if (dma->channel[0] == NUM_DMA_CHANNELS || dma->channel[1] == NUM_DMA_CHANNELS) {
if (dma->channel[0] < NUM_DMA_CHANNELS) {
dma_channel_unclaim(dma->channel[0]);
}
int dma_channel_0_maybe = dma_claim_unused_channel(false);
if (dma_channel_0_maybe < 0) {
return AUDIO_DMA_DMA_BUSY;
}

int dma_channel_1_maybe = dma_claim_unused_channel(false);
if (dma_channel_1_maybe < 0) {
dma_channel_unclaim((uint)dma_channel_0_maybe);
return AUDIO_DMA_DMA_BUSY;
}

dma->channel[0] = (uint8_t)dma_channel_0_maybe;
dma->channel[1] = (uint8_t)dma_channel_1_maybe;

dma->sample = sample;
dma->loop = loop;
dma->single_channel_output = single_channel_output;
Expand Down
7 changes: 7 additions & 0 deletions ports/raspberrypi/common-hal/audiobusio/I2SOut.c
Original file line number Diff line number Diff line change
Expand Up @@ -145,14 +145,21 @@ void common_hal_audiobusio_i2sout_deinit(audiobusio_i2sout_obj_t *self) {
return;
}

if (common_hal_audiobusio_i2sout_get_playing(self)) {
common_hal_audiobusio_i2sout_stop(self);
}

common_hal_rp2pio_statemachine_deinit(&self->state_machine);

audio_dma_deinit(&self->dma);
}

void common_hal_audiobusio_i2sout_play(audiobusio_i2sout_obj_t *self,
mp_obj_t sample, bool loop) {
if (common_hal_audiobusio_i2sout_get_playing(self)) {
common_hal_audiobusio_i2sout_stop(self);
}

uint8_t bits_per_sample = audiosample_bits_per_sample(sample);
// Make sure we transmit a minimum of 16 bits.
// TODO: Maybe we need an intermediate object to upsample instead. This is
Expand Down
74 changes: 47 additions & 27 deletions ports/raspberrypi/common-hal/audiopwmio/PWMAudioOut.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@

#define NUM_DMA_TIMERS 4

// The PWM clock frequency is base_clock_rate / PWM_TOP, typically 125_000_000 / PWM_TOP.
// We pick BITS_PER_SAMPLE so we get a clock frequency that is above what would cause aliasing.
#define BITS_PER_SAMPLE 10
#define SAMPLE_BITS_TO_DISCARD (16 - BITS_PER_SAMPLE)
#define PWM_TOP ((1 << BITS_PER_SAMPLE) - 1)

void audiopwmout_reset() {
for (size_t i = 0; i < NUM_DMA_TIMERS; i++) {
dma_hw->timer[i] = 0;
Expand All @@ -55,7 +61,10 @@ void audiopwmout_reset() {
// Caller validates that pins are free.
void common_hal_audiopwmio_pwmaudioout_construct(audiopwmio_pwmaudioout_obj_t *self,
const mcu_pin_obj_t *left_channel, const mcu_pin_obj_t *right_channel, uint16_t quiescent_value) {
if (left_channel != NULL && right_channel != NULL) {

self->stereo = right_channel != NULL;

if (self->stereo) {
if (pwm_gpio_to_slice_num(left_channel->number) != pwm_gpio_to_slice_num(right_channel->number)) {
mp_raise_ValueError(translate("Pins must share PWM slice"));
}
Expand All @@ -72,22 +81,20 @@ void common_hal_audiopwmio_pwmaudioout_construct(audiopwmio_pwmaudioout_obj_t *s
// we want. ;-) We mark ourselves variable only if we're a mono output to
// prevent other PWM use on the other channel. If stereo, we say fixed
// frequency so we can allocate with ourselves.
bool mono = right_channel == NULL;

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

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

pwmout_result_t result = common_hal_pwmio_pwmout_construct(&self->left_pwm,
left_channel, 0, frequency, mono);
pwmout_result_t result =
common_hal_pwmio_pwmout_construct(&self->left_pwm, left_channel, 0, frequency, !self->stereo);
if (result == PWMOUT_OK && right_channel != NULL) {
result = common_hal_pwmio_pwmout_construct(&self->right_pwm,
right_channel, 0, frequency, false);
result =
common_hal_pwmio_pwmout_construct(&self->right_pwm, right_channel, 0, frequency, false);
if (result != PWMOUT_OK) {
common_hal_pwmio_pwmout_deinit(&self->left_pwm);
}
Expand All @@ -96,10 +103,16 @@ void common_hal_audiopwmio_pwmaudioout_construct(audiopwmio_pwmaudioout_obj_t *s
mp_raise_RuntimeError(translate("All timers in use"));
}

self->quiescent_value = quiescent_value >> SAMPLE_BITS_TO_DISCARD;
common_hal_pwmio_pwmout_set_duty_cycle(&self->left_pwm, self->quiescent_value);
pwmio_pwmout_set_top(&self->left_pwm, PWM_TOP);
if (self->stereo) {
common_hal_pwmio_pwmout_set_duty_cycle(&self->right_pwm, self->quiescent_value);
pwmio_pwmout_set_top(&self->right_pwm, PWM_TOP);
}

audio_dma_init(&self->dma);
self->pacing_timer = NUM_DMA_TIMERS;

self->quiescent_value = quiescent_value;
}

bool common_hal_audiopwmio_pwmaudioout_deinited(audiopwmio_pwmaudioout_obj_t *self) {
Expand All @@ -110,6 +123,7 @@ void common_hal_audiopwmio_pwmaudioout_deinit(audiopwmio_pwmaudioout_obj_t *self
if (common_hal_audiopwmio_pwmaudioout_deinited(self)) {
return;
}

if (common_hal_audiopwmio_pwmaudioout_get_playing(self)) {
common_hal_audiopwmio_pwmaudioout_stop(self);
}
Expand Down Expand Up @@ -139,29 +153,28 @@ void common_hal_audiopwmio_pwmaudioout_play(audiopwmio_pwmaudioout_obj_t *self,
mp_raise_RuntimeError(translate("No DMA pacing timer found"));
}
uint32_t tx_register = (uint32_t)&pwm_hw->slice[self->left_pwm.slice].cc;
if (common_hal_pwmio_pwmout_deinited(&self->right_pwm)) {
// Shift the destination if we are outputting to the second PWM channel.
if (self->stereo) {
// Shift the destination if we are outputting to both PWM channels.
tx_register += self->left_pwm.channel * sizeof(uint16_t);
}

pwmio_pwmout_set_top(&self->left_pwm, 1023);

audio_dma_result result = audio_dma_setup_playback(
&self->dma,
sample,
loop,
false, // single channel
0, // audio channel
false, // output signed
10,
(uint32_t)tx_register, // output register
BITS_PER_SAMPLE,
(uint32_t)tx_register, // output register: PWM cc register
0x3b + pacing_timer); // data request line

if (result == AUDIO_DMA_DMA_BUSY) {
// common_hal_audiobusio_i2sout_stop(self);
common_hal_audiopwmio_pwmaudioout_stop(self);
mp_raise_RuntimeError(translate("No DMA channel found"));
} else if (result == AUDIO_DMA_MEMORY_ERROR) {
// common_hal_audiobusio_i2sout_stop(self);
}
if (result == AUDIO_DMA_MEMORY_ERROR) {
common_hal_audiopwmio_pwmaudioout_stop(self);
mp_raise_RuntimeError(translate("Unable to allocate buffers for signed conversion"));
}

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

uint32_t sample_rate = audiosample_sample_rate(sample);

uint32_t system_clock = common_hal_mcu_processor_get_frequency();
uint32_t best_numerator = 0;
uint32_t best_denominator = 0;
Expand Down Expand Up @@ -204,19 +218,25 @@ void common_hal_audiopwmio_pwmaudioout_play(audiopwmio_pwmaudioout_obj_t *self,
}

void common_hal_audiopwmio_pwmaudioout_stop(audiopwmio_pwmaudioout_obj_t *self) {
dma_hw->timer[self->pacing_timer] = 0;
self->pacing_timer = NUM_DMA_TIMERS;
if (self->pacing_timer < NUM_DMA_TIMERS) {
dma_hw->timer[self->pacing_timer] = 0;
self->pacing_timer = NUM_DMA_TIMERS;
}

audio_dma_stop(&self->dma);

// Set to quiescent level.
pwm_hw->slice[self->left_pwm.slice].cc = self->quiescent_value;
if (self->stereo) {
pwm_hw->slice[self->right_pwm.slice].cc = self->quiescent_value;
}
}

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

audio_dma_stop(&self->dma);
if (!playing && self->pacing_timer < NUM_DMA_TIMERS) {
common_hal_audiopwmio_pwmaudioout_stop(self);
}

return playing;
Expand Down
1 change: 1 addition & 0 deletions ports/raspberrypi/common-hal/audiopwmio/PWMAudioOut.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ typedef struct {
audio_dma_t dma;
uint16_t quiescent_value;
uint8_t pacing_timer;
bool stereo; // if false, only using left_pwm.
} audiopwmio_pwmaudioout_obj_t;

void audiopwmout_reset(void);
Expand Down
13 changes: 2 additions & 11 deletions ports/raspberrypi/common-hal/busio/SPI.c
Original file line number Diff line number Diff line change
Expand Up @@ -212,15 +212,6 @@ static bool _transfer(busio_spi_obj_t *self,
while (dma_channel_is_busy(chan_rx) || dma_channel_is_busy(chan_tx)) {
// TODO: We should idle here until we get a DMA interrupt or something else.
RUN_BACKGROUND_TASKS;
if (mp_hal_is_interrupted()) {
if (dma_channel_is_busy(chan_rx)) {
dma_channel_abort(chan_rx);
}
if (dma_channel_is_busy(chan_tx)) {
dma_channel_abort(chan_tx);
}
break;
}
}
}

Expand All @@ -233,15 +224,15 @@ static bool _transfer(busio_spi_obj_t *self,
dma_channel_unclaim(chan_tx);
}

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

while (!mp_hal_is_interrupted() && (rx_remaining || tx_remaining)) {
while (rx_remaining || tx_remaining) {
if (tx_remaining && spi_is_writable(self->peripheral) && rx_remaining - tx_remaining < fifo_depth) {
spi_get_hw(self->peripheral)->dr = (uint32_t)*data_out;
// Increment only if the buffer is the transfer length. It's 1 otherwise.
Expand Down
17 changes: 1 addition & 16 deletions ports/raspberrypi/common-hal/rp2pio/StateMachine.c
Original file line number Diff line number Diff line change
Expand Up @@ -654,15 +654,6 @@ static bool _transfer(rp2pio_statemachine_obj_t *self,
(tx && dma_channel_is_busy(chan_tx))) {
// TODO: We should idle here until we get a DMA interrupt or something else.
RUN_BACKGROUND_TASKS;
if (mp_hal_is_interrupted()) {
if (rx && dma_channel_is_busy(chan_rx)) {
dma_channel_abort(chan_rx);
}
if (tx && dma_channel_is_busy(chan_tx)) {
dma_channel_abort(chan_tx);
}
break;
}
}
// Clear the stall bit so we can detect when the state machine is done transmitting.
self->pio->fdebug = stall_mask;
Expand All @@ -677,7 +668,7 @@ static bool _transfer(rp2pio_statemachine_obj_t *self,
dma_channel_unclaim(chan_tx);
}

if (!use_dma && !mp_hal_is_interrupted()) {
if (!use_dma) {
// Use software for small transfers, or if couldn't claim two DMA channels
size_t rx_remaining = in_len / in_stride_in_bytes;
size_t tx_remaining = out_len / out_stride_in_bytes;
Expand Down Expand Up @@ -706,9 +697,6 @@ static bool _transfer(rp2pio_statemachine_obj_t *self,
--rx_remaining;
}
RUN_BACKGROUND_TASKS;
if (mp_hal_is_interrupted()) {
break;
}
}
// Clear the stall bit so we can detect when the state machine is done transmitting.
self->pio->fdebug = stall_mask;
Expand All @@ -719,9 +707,6 @@ static bool _transfer(rp2pio_statemachine_obj_t *self,
while (!pio_sm_is_tx_fifo_empty(self->pio, self->state_machine) ||
(self->wait_for_txstall && (self->pio->fdebug & stall_mask) == 0)) {
RUN_BACKGROUND_TASKS;
if (mp_hal_is_interrupted()) {
break;
}
}
}
return true;
Expand Down
4 changes: 2 additions & 2 deletions ports/raspberrypi/mpconfigport.mk
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ CIRCUITPY_ALARM ?= 1

CIRCUITPY_RP2PIO ?= 1
CIRCUITPY_NEOPIXEL_WRITE ?= $(CIRCUITPY_RP2PIO)
CIRCUITPY_FRAMEBUFFERIO ?= 1
CIRCUITPY_FRAMEBUFFERIO ?= $(CIRCUITPY_DISPLAYIO)
CIRCUITPY_FULL_BUILD ?= 1
CIRCUITPY_AUDIOMP3 ?= 1
CIRCUITPY_BITOPS ?= 1
CIRCUITPY_IMAGECAPTURE ?= 1
CIRCUITPY_PWMIO ?= 1
CIRCUITPY_RGBMATRIX ?= 1
CIRCUITPY_RGBMATRIX ?= $(CIRCUITPY_DISPLAYIO)
CIRCUITPY_ROTARYIO ?= 1
CIRCUITPY_ROTARYIO_SOFTENCODER = 1

Expand Down