valid channels in audio_dma_stop; cleaner supervisor_ticks mgmt in keypad

Author: dhalbert

ports/atmel-samd/common-hal/audiobusio/I2SOut.c

@@ -44,7 +44,7 @@
 
 #include "atmel_start_pins.h"
 #include "hal/include/hal_gpio.h"
-#include "hpl//hpl_gclk_base.h"
+#include "hpl/gclk/hpl_gclk_base.h"
 #include "peripheral_clk_config.h"
 
 #ifdef SAMD21

ports/raspberrypi/audio_dma.c

@@ -37,10 +37,8 @@
 
 #if CIRCUITPY_AUDIOPWMIO || CIRCUITPY_AUDIOBUSIO
 
-#define AUDIO_DMA_CHANNEL_COUNT NUM_DMA_CHANNELS
-
 void audio_dma_reset(void) {
-    for (size_t channel = 0; channel < AUDIO_DMA_CHANNEL_COUNT; channel++) {
+    for (size_t channel = 0; channel < NUM_DMA_CHANNELS; channel++) {
         if (MP_STATE_PORT(playing_audio)[channel] == NULL) {
             continue;
         }
@@ -171,6 +169,7 @@ void audio_dma_load_next_block(audio_dma_t *dma) {
                 !dma_channel_is_busy(dma->channel[1])) {
                 // No data has been read, and both DMA channels have now finished, so it's safe to stop.
                 audio_dma_stop(dma);
+                dma->playing_in_progress = false;
             } else {
                 // Set channel trigger to ourselves so we don't keep going.
                 dma_channel_hw_t *c = &dma_hw->ch[dma_channel];
@@ -318,14 +317,22 @@ audio_dma_result audio_dma_setup_playback(audio_dma_t *dma,
         irq_set_mask_enabled(1 << DMA_IRQ_0, true);
     }
 
+    dma->playing_in_progress = true;
     dma_channel_start(dma->channel[0]);
 
     return AUDIO_DMA_OK;
 }
 
 void audio_dma_stop(audio_dma_t *dma) {
     // Disable our interrupts.
-    dma_hw->inte0 &= ~((1 << dma->channel[0]) | (1 << dma->channel[1]));
+    uint32_t channel_mask = 0;
+    if (dma->channel[0] < NUM_DMA_CHANNELS) {
+        channel_mask |= 1 << dma->channel[0];
+    }
+    if (dma->channel[1] < NUM_DMA_CHANNELS) {
+        channel_mask |= 1 << dma->channel[1];
+    }
+    dma_hw->inte0 &= ~channel_mask;
     irq_set_mask_enabled(1 << DMA_IRQ_0, false);
 
     // Run any remaining audio tasks because we remove ourselves from
@@ -334,6 +341,10 @@ void audio_dma_stop(audio_dma_t *dma) {
 
     for (size_t i = 0; i < 2; i++) {
         size_t channel = dma->channel[i];
+        if (channel == NUM_DMA_CHANNELS) {
+            // Channel not in use.
+            continue;
+        }
 
         dma_channel_config c = dma_channel_get_default_config(dma->channel[i]);
         channel_config_set_enable(&c, false);
@@ -357,6 +368,7 @@ void audio_dma_stop(audio_dma_t *dma) {
         MP_STATE_PORT(playing_audio)[channel] = NULL;
         dma->channel[i] = NUM_DMA_CHANNELS;
     }
+    dma->playing_in_progress = false;
 
     // Hold onto our buffers.
 }
@@ -381,7 +393,7 @@ void audio_dma_resume(audio_dma_t *dma) {
 }
 
 bool audio_dma_get_paused(audio_dma_t *dma) {
-    if (dma->channel[0] >= AUDIO_DMA_CHANNEL_COUNT) {
+    if (dma->channel[0] >= NUM_DMA_CHANNELS) {
         return false;
     }
     uint32_t control = dma_hw->ch[dma->channel[0]].ctrl_trig;
@@ -408,12 +420,7 @@ bool audio_dma_get_playing(audio_dma_t *dma) {
     if (dma->channel[0] == NUM_DMA_CHANNELS) {
         return false;
     }
-    if (!dma_channel_is_busy(dma->channel[0]) &&
-        !dma_channel_is_busy(dma->channel[1])) {
-        return false;
-    }
-
-    return true;
+    return dma->playing_in_progress;
 }
 
 // WARN(tannewt): DO NOT print from here, or anything it calls. Printing calls

ports/raspberrypi/audio_dma.h

@@ -45,6 +45,7 @@ typedef struct {
     bool output_signed;
     bool first_channel_free;
     bool first_buffer_free;
+    bool playing_in_progress;
     uint8_t output_resolution; // in bits
     uint8_t sample_resolution; // in bits
     uint8_t *first_buffer;

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

@@ -38,7 +38,6 @@
 #include "shared-bindings/microcontroller/__init__.h"
 #include "shared-bindings/microcontroller/Pin.h"
 #include "shared-bindings/microcontroller/Processor.h"
-#include "supervisor/shared/tick.h"
 #include "supervisor/shared/translate.h"
 
 #include "src/rp2040/hardware_structs/include/hardware/structs/dma.h"

shared-module/keypad/KeyMatrix.c

@@ -77,8 +77,6 @@ void common_hal_keypad_keymatrix_construct(keypad_keymatrix_obj_t *self, mp_uint
 
     // Add self to the list of active keypad scanners.
     keypad_register_scanner((keypad_scanner_obj_t *)self);
-
-    supervisor_enable_tick();
 }
 
 void common_hal_keypad_keymatrix_deinit(keypad_keymatrix_obj_t *self) {

shared-module/keypad/Keys.c

@@ -63,8 +63,6 @@ void common_hal_keypad_keys_construct(keypad_keys_obj_t *self, mp_uint_t num_pin
 
     // Add self to the list of active keypad scanners.
     keypad_register_scanner((keypad_scanner_obj_t *)self);
-
-    supervisor_enable_tick();
 }
 
 void common_hal_keypad_keys_deinit(keypad_keys_obj_t *self) {

shared-module/keypad/ShiftRegisterKeys.c

@@ -71,8 +71,6 @@ void common_hal_keypad_shiftregisterkeys_construct(keypad_shiftregisterkeys_obj_
 
     // Add self to the list of active keypad scanners.
     keypad_register_scanner((keypad_scanner_obj_t *)self);
-
-    supervisor_enable_tick();
 }
 
 void common_hal_keypad_shiftregisterkeys_deinit(keypad_shiftregisterkeys_obj_t *self) {

shared-module/keypad/__init__.c

@@ -57,12 +57,9 @@ void keypad_tick(void) {
 }
 
 void keypad_reset(void) {
-    if (MP_STATE_VM(keypad_scanners_linked_list)) {
-        supervisor_disable_tick();
+    while (MP_STATE_VM(keypad_scanners_linked_list)) {
+        keypad_deregister_scanner(MP_STATE_VM(keypad_scanners_linked_list));
     }
-
-    MP_STATE_VM(keypad_scanners_linked_list) = NULL;
-    keypad_scanners_linked_list_lock = false;
 }
 
 // Register a Keys, KeyMatrix, etc. that will be scanned in the background
@@ -71,10 +68,16 @@ void keypad_register_scanner(keypad_scanner_obj_t *scanner) {
     scanner->next = MP_STATE_VM(keypad_scanners_linked_list);
     MP_STATE_VM(keypad_scanners_linked_list) = scanner;
     supervisor_release_lock(&keypad_scanners_linked_list_lock);
+
+    // One more request for ticks.
+    supervisor_enable_tick();
 }
 
 // Remove scanner from the list of active scanners.
 void keypad_deregister_scanner(keypad_scanner_obj_t *scanner) {
+    // One less request for ticks.
+    supervisor_disable_tick();
+
     supervisor_acquire_lock(&keypad_scanners_linked_list_lock);
     if (MP_STATE_VM(keypad_scanners_linked_list) == scanner) {
         // Scanner is at the front; splice it out.