RP2040 PWMAudioOut: Release DMA channels after play has finished

ports/atmel-samd/audio_dma.c

@@ -131,7 +131,7 @@ void audio_dma_load_next_block(audio_dma_t *dma) {
     uint8_t *buffer;
     uint32_t buffer_length;
     audioio_get_buffer_result_t get_buffer_result =
-        audiosample_get_buffer(dma->sample, dma->single_channel, dma->audio_channel,
+        audiosample_get_buffer(dma->sample, dma->single_channel_output, dma->audio_channel,
             &buffer, &buffer_length);
 
     DmacDescriptor *descriptor = dma->second_descriptor;
@@ -155,7 +155,7 @@ void audio_dma_load_next_block(audio_dma_t *dma) {
     descriptor->SRCADDR.reg = ((uint32_t)output_buffer) + output_buffer_length;
     if (get_buffer_result == GET_BUFFER_DONE) {
         if (dma->loop) {
-            audiosample_reset_buffer(dma->sample, dma->single_channel, dma->audio_channel);
+            audiosample_reset_buffer(dma->sample, dma->single_channel_output, dma->audio_channel);
         } else {
             descriptor->DESCADDR.reg = 0;
         }
@@ -183,7 +183,7 @@ static void setup_audio_descriptor(DmacDescriptor *descriptor, uint8_t beat_size
 audio_dma_result audio_dma_setup_playback(audio_dma_t *dma,
     mp_obj_t sample,
     bool loop,
-    bool single_channel,
+    bool single_channel_output,
     uint8_t audio_channel,
     bool output_signed,
     uint32_t output_register_address,
@@ -195,20 +195,20 @@ audio_dma_result audio_dma_setup_playback(audio_dma_t *dma,
 
     dma->sample = sample;
     dma->loop = loop;
-    dma->single_channel = single_channel;
+    dma->single_channel_output = single_channel_output;
     dma->audio_channel = audio_channel;
     dma->dma_channel = dma_channel;
     dma->signed_to_unsigned = false;
     dma->unsigned_to_signed = false;
     dma->second_descriptor = NULL;
     dma->spacing = 1;
     dma->first_descriptor_free = true;
-    audiosample_reset_buffer(sample, single_channel, audio_channel);
+    audiosample_reset_buffer(sample, single_channel_output, audio_channel);
 
     bool single_buffer;
     bool samples_signed;
     uint32_t max_buffer_length;
-    audiosample_get_buffer_structure(sample, single_channel, &single_buffer, &samples_signed,
+    audiosample_get_buffer_structure(sample, single_channel_output, &single_buffer, &samples_signed,
         &max_buffer_length, &dma->spacing);
     uint8_t output_spacing = dma->spacing;
     if (output_signed != samples_signed) {
@@ -254,12 +254,12 @@ audio_dma_result audio_dma_setup_playback(audio_dma_t *dma,
     } else {
         dma->beat_size = 1;
         dma->bytes_per_sample = 1;
-        if (single_channel) {
+        if (single_channel_output) {
             output_register_address += 1;
         }
     }
     // Transfer both channels at once.
-    if (!single_channel && audiosample_channel_count(sample) == 2) {
+    if (!single_channel_output && audiosample_channel_count(sample) == 2) {
         dma->beat_size *= 2;
     }
 

ports/atmel-samd/audio_dma.h

@@ -42,7 +42,7 @@ typedef struct {
     uint8_t beat_size;
     uint8_t spacing;
     bool loop;
-    bool single_channel;
+    bool single_channel_output;
     bool signed_to_unsigned;
     bool unsigned_to_signed;
     bool first_buffer_free;
@@ -72,16 +72,16 @@ void dma_free_channel(uint8_t channel);
 // This sets everything up but doesn't start the timer.
 // Sample is the python object for the sample to play.
 // loop is true if we should loop the sample.
-// single_channel is true if we only output a single channel. When false, all channels will be
+// single_channel_output is true if we only output a single channel. When false, all channels will be
 //   output.
-// audio_channel is the index of the channel to dma. single_channel must be false in this case.
+// audio_channel is the index of the channel to dma. single_channel_output must be false in this case.
 // output_signed is true if the dma'd data should be signed. False and it will be unsigned.
 // output_register_address is the address to copy data to.
 // dma_trigger_source is the DMA trigger source which cause another copy
 audio_dma_result audio_dma_setup_playback(audio_dma_t *dma,
     mp_obj_t sample,
     bool loop,
-    bool single_channel,
+    bool single_channel_output,
     uint8_t audio_channel,
     bool output_signed,
     uint32_t output_register_address,

ports/atmel-samd/boards/meowmeow/mpconfigboard.mk

@@ -6,6 +6,8 @@ USB_MANUFACTURER = "Electronic Cats"
 CHIP_VARIANT = SAMD21G18A
 CHIP_FAMILY = samd21
 
+CIRCUITPY_PWMIO = 0
+
 INTERNAL_FLASH_FILESYSTEM = 1
 LONGINT_IMPL = NONE
 CIRCUITPY_FULL_BUILD = 0

ports/raspberrypi/audio_dma.c

@@ -129,7 +129,7 @@ void audio_dma_load_next_block(audio_dma_t *dma) {
     uint8_t *buffer;
     uint32_t buffer_length;
     get_buffer_result = audiosample_get_buffer(dma->sample,
-        dma->single_channel, dma->audio_channel, &buffer, &buffer_length);
+        dma->single_channel_output, dma->audio_channel, &buffer, &buffer_length);
 
     if (get_buffer_result == GET_BUFFER_ERROR) {
         audio_dma_stop(dma);
@@ -148,7 +148,7 @@ void audio_dma_load_next_block(audio_dma_t *dma) {
     dma_channel_set_read_addr(dma_channel, output_buffer, false /* trigger */);
     if (get_buffer_result == GET_BUFFER_DONE) {
         if (dma->loop) {
-            audiosample_reset_buffer(dma->sample, dma->single_channel, dma->audio_channel);
+            audiosample_reset_buffer(dma->sample, dma->single_channel_output, dma->audio_channel);
         } else {
             // Set channel trigger to ourselves so we don't keep going.
             dma_channel_hw_t *c = &dma_hw->ch[dma_channel];
@@ -161,13 +161,13 @@ void audio_dma_load_next_block(audio_dma_t *dma) {
 audio_dma_result audio_dma_setup_playback(audio_dma_t *dma,
     mp_obj_t sample,
     bool loop,
-    bool single_channel,
+    bool single_channel_output,
     uint8_t audio_channel,
     bool output_signed,
     uint8_t output_resolution,
     uint32_t output_register_address,
     uint8_t dma_trigger_source) {
-    // Use two DMA channels to  because the DMA can't wrap to itself without the
+    // 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);
@@ -180,7 +180,7 @@ audio_dma_result audio_dma_setup_playback(audio_dma_t *dma,
 
     dma->sample = sample;
     dma->loop = loop;
-    dma->single_channel = single_channel;
+    dma->single_channel_output = single_channel_output;
     dma->audio_channel = audio_channel;
     dma->signed_to_unsigned = false;
     dma->unsigned_to_signed = false;
@@ -189,12 +189,12 @@ audio_dma_result audio_dma_setup_playback(audio_dma_t *dma,
     dma->first_channel_free = true;
     dma->output_resolution = output_resolution;
     dma->sample_resolution = audiosample_bits_per_sample(sample);
-    audiosample_reset_buffer(sample, single_channel, audio_channel);
+    audiosample_reset_buffer(sample, single_channel_output, audio_channel);
 
     bool single_buffer;
     bool samples_signed;
     uint32_t max_buffer_length;
-    audiosample_get_buffer_structure(sample, single_channel, &single_buffer, &samples_signed,
+    audiosample_get_buffer_structure(sample, single_channel_output, &single_buffer, &samples_signed,
         &max_buffer_length, &dma->sample_spacing);
 
     // Check to see if we have to scale the resolution up.
@@ -227,10 +227,9 @@ audio_dma_result audio_dma_setup_playback(audio_dma_t *dma,
         dma->output_size = 1;
     }
     // Transfer both channels at once.
-    if (!single_channel && audiosample_channel_count(sample) == 2) {
+    if (!single_channel_output && audiosample_channel_count(sample) == 2) {
         dma->output_size *= 2;
     }
-
     enum dma_channel_transfer_size dma_size = DMA_SIZE_8;
     if (dma->output_size == 2) {
         dma_size = DMA_SIZE_16;
@@ -324,20 +323,19 @@ void audio_dma_stop(audio_dma_t *dma) {
 // to hold the previous value.
 void audio_dma_pause(audio_dma_t *dma) {
     dma_hw->ch[dma->channel[0]].al1_ctrl &= ~DMA_CH0_CTRL_TRIG_EN_BITS;
-    dma_hw->ch[dma->channel[1]].al1_ctrl &= ~DMA_CH0_CTRL_TRIG_EN_BITS;
+    dma_hw->ch[dma->channel[1]].al1_ctrl &= ~DMA_CH1_CTRL_TRIG_EN_BITS;
 }
 
 void audio_dma_resume(audio_dma_t *dma) {
     // Always re-enable the non-busy channel first so it's ready to continue when the busy channel
     // finishes and chains to it. (An interrupt could make the time between enables long.)
-    size_t first = 0;
-    size_t second = 1;
     if (dma_channel_is_busy(dma->channel[0])) {
-        first = 1;
-        second = 0;
+        dma_hw->ch[dma->channel[1]].al1_ctrl |= DMA_CH1_CTRL_TRIG_EN_BITS;
+        dma_hw->ch[dma->channel[0]].al1_ctrl |= DMA_CH0_CTRL_TRIG_EN_BITS;
+    } else {
+        dma_hw->ch[dma->channel[0]].al1_ctrl |= DMA_CH0_CTRL_TRIG_EN_BITS;
+        dma_hw->ch[dma->channel[1]].al1_ctrl |= DMA_CH1_CTRL_TRIG_EN_BITS;
     }
-    dma_hw->ch[dma->channel[first]].al1_ctrl |= DMA_CH0_CTRL_TRIG_EN_BITS;
-    dma_hw->ch[dma->channel[second]].al1_ctrl |= DMA_CH0_CTRL_TRIG_EN_BITS;
 }
 
 bool audio_dma_get_paused(audio_dma_t *dma) {
@@ -378,6 +376,8 @@ bool audio_dma_get_playing(audio_dma_t *dma) {
 
 // WARN(tannewt): DO NOT print from here, or anything it calls. Printing calls
 // background tasks such as this and causes a stack overflow.
+// NOTE(dhalbert): I successfully printed from here while debugging.
+// So it's possible, but be careful.
 STATIC void dma_callback_fun(void *arg) {
     audio_dma_t *dma = arg;
     if (dma == NULL) {

ports/raspberrypi/audio_dma.h

@@ -39,7 +39,7 @@ typedef struct {
     uint8_t output_size;
     uint8_t sample_spacing;
     bool loop;
-    bool single_channel;
+    bool single_channel_output;
     bool signed_to_unsigned;
     bool unsigned_to_signed;
     bool output_signed;
@@ -66,16 +66,16 @@ void audio_dma_reset(void);
 // This sets everything up but doesn't start the timer.
 // Sample is the python object for the sample to play.
 // loop is true if we should loop the sample.
-// single_channel is true if we only output a single channel. When false, all channels will be
+// single_channel_output is true if we only output a single channel. When false, all channels will be
 //   output.
-// audio_channel is the index of the channel to dma. single_channel must be false in this case.
+// audio_channel is the index of the channel to dma. single_channel_output must be false in this case.
 // output_signed is true if the dma'd data should be signed. False and it will be unsigned.
 // output_register_address is the address to copy data to.
 // dma_trigger_source is the DMA trigger source which cause another copy
 audio_dma_result audio_dma_setup_playback(audio_dma_t *dma,
     mp_obj_t sample,
     bool loop,
-    bool single_channel,
+    bool single_channel_output,
     uint8_t audio_channel,
     bool output_signed,
     uint8_t output_resolution,

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

@@ -1,3 +1,4 @@
+
 /*
  * This file is part of the MicroPython project, http://micropython.org/
  *
@@ -36,10 +37,10 @@
 // We don't bit pack because we'll only have two at most. Its better to save code size instead.
 typedef struct {
     mp_obj_base_t base;
-    bool left_justified;
     rp2pio_statemachine_obj_t state_machine;
-    bool playing;
     audio_dma_t dma;
+    bool left_justified;
+    bool playing;
 } audiobusio_i2sout_obj_t;
 
 void i2sout_reset(void);

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

@@ -96,11 +96,6 @@ void common_hal_audiopwmio_pwmaudioout_construct(audiopwmio_pwmaudioout_obj_t *s
         mp_raise_RuntimeError(translate("All timers in use"));
     }
 
-    claim_pin(left_channel);
-    if (right_channel != NULL) {
-        claim_pin(right_channel);
-    }
-
     audio_dma_init(&self->dma);
     self->pacing_timer = NUM_DMA_TIMERS;
 
@@ -220,7 +215,10 @@ bool common_hal_audiopwmio_pwmaudioout_get_playing(audiopwmio_pwmaudioout_obj_t
     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);
     }
+
     return playing;
 }
 

ports/raspberrypi/common-hal/pwmio/PWMOut.c

@@ -163,6 +163,8 @@ pwmout_result_t common_hal_pwmio_pwmout_construct(pwmio_pwmout_obj_t *self,
     self->variable_frequency = variable_frequency;
     self->duty_cycle = duty;
 
+    claim_pin(pin);
+
     if (frequency == 0 || frequency > (common_hal_mcu_processor_get_frequency() / 2)) {
         return PWMOUT_INVALID_FREQUENCY;
     }

shared-module/audiocore/RawSample.c

@@ -67,31 +67,31 @@ uint8_t common_hal_audioio_rawsample_get_channel_count(audioio_rawsample_obj_t *
 }
 
 void audioio_rawsample_reset_buffer(audioio_rawsample_obj_t *self,
-    bool single_channel,
+    bool single_channel_output,
     uint8_t channel) {
 }
 
 audioio_get_buffer_result_t audioio_rawsample_get_buffer(audioio_rawsample_obj_t *self,
-    bool single_channel,
+    bool single_channel_output,
     uint8_t channel,
     uint8_t **buffer,
     uint32_t *buffer_length) {
     *buffer_length = self->len;
-    if (single_channel) {
+    if (single_channel_output) {
         *buffer = self->buffer + (channel % self->channel_count) * (self->bits_per_sample / 8);
     } else {
         *buffer = self->buffer;
     }
     return GET_BUFFER_DONE;
 }
 
-void audioio_rawsample_get_buffer_structure(audioio_rawsample_obj_t *self, bool single_channel,
+void audioio_rawsample_get_buffer_structure(audioio_rawsample_obj_t *self, bool single_channel_output,
     bool *single_buffer, bool *samples_signed,
     uint32_t *max_buffer_length, uint8_t *spacing) {
     *single_buffer = true;
     *samples_signed = self->samples_signed;
     *max_buffer_length = self->len;
-    if (single_channel) {
+    if (single_channel_output) {
         *spacing = self->channel_count;
     } else {
         *spacing = 1;

shared-module/audiocore/RawSample.h

@@ -44,14 +44,14 @@ typedef struct {
 
 // These are not available from Python because it may be called in an interrupt.
 void audioio_rawsample_reset_buffer(audioio_rawsample_obj_t *self,
-    bool single_channel,
+    bool single_channel_output,
     uint8_t channel);
 audioio_get_buffer_result_t audioio_rawsample_get_buffer(audioio_rawsample_obj_t *self,
-    bool single_channel,
+    bool single_channel_output,
     uint8_t channel,
     uint8_t **buffer,
     uint32_t *buffer_length);                                                      // length in bytes
-void audioio_rawsample_get_buffer_structure(audioio_rawsample_obj_t *self, bool single_channel,
+void audioio_rawsample_get_buffer_structure(audioio_rawsample_obj_t *self, bool single_channel_output,
     bool *single_buffer, bool *samples_signed,
     uint32_t *max_buffer_length, uint8_t *spacing);
 

shared-module/audiocore/WaveFile.c

@@ -169,9 +169,9 @@ uint32_t audioio_wavefile_max_buffer_length(audioio_wavefile_obj_t *self) {
 }
 
 void audioio_wavefile_reset_buffer(audioio_wavefile_obj_t *self,
-    bool single_channel,
+    bool single_channel_output,
     uint8_t channel) {
-    if (single_channel && channel == 1) {
+    if (single_channel_output && channel == 1) {
         return;
     }
     // We don't reset the buffer index in case we're looping and we have an odd number of buffer
@@ -184,11 +184,11 @@ void audioio_wavefile_reset_buffer(audioio_wavefile_obj_t *self,
 }
 
 audioio_get_buffer_result_t audioio_wavefile_get_buffer(audioio_wavefile_obj_t *self,
-    bool single_channel,
+    bool single_channel_output,
     uint8_t channel,
     uint8_t **buffer,
     uint32_t *buffer_length) {
-    if (!single_channel) {
+    if (!single_channel_output) {
         channel = 0;
     }
 
@@ -265,13 +265,14 @@ audioio_get_buffer_result_t audioio_wavefile_get_buffer(audioio_wavefile_obj_t *
     return self->bytes_remaining == 0 ? GET_BUFFER_DONE : GET_BUFFER_MORE_DATA;
 }
 
-void audioio_wavefile_get_buffer_structure(audioio_wavefile_obj_t *self, bool single_channel,
+void audioio_wavefile_get_buffer_structure(audioio_wavefile_obj_t *self, bool single_channel_output,
     bool *single_buffer, bool *samples_signed,
     uint32_t *max_buffer_length, uint8_t *spacing) {
     *single_buffer = false;
+    // In WAV files, 8-bit samples are always unsigned, and larger samples are always signed.
     *samples_signed = self->bits_per_sample > 8;
     *max_buffer_length = 512;
-    if (single_channel) {
+    if (single_channel_output) {
         *spacing = self->channel_count;
     } else {
         *spacing = 1;