diff --git a/include/pbl/drivers/speaker/qemu/audio.h b/include/pbl/drivers/speaker/qemu/audio.h index a64181374c..3db0a969d4 100644 --- a/include/pbl/drivers/speaker/qemu/audio.h +++ b/include/pbl/drivers/speaker/qemu/audio.h @@ -3,12 +3,14 @@ #pragma once +#include #include #include typedef struct AudioState { AudioTransCB trans_cb; + bool callback_pending; } AudioDeviceState; struct AudioDevice { diff --git a/include/pbl/services/system_task.h b/include/pbl/services/system_task.h index 6494cac511..7c9c12a785 100644 --- a/include/pbl/services/system_task.h +++ b/include/pbl/services/system_task.h @@ -39,6 +39,11 @@ bool system_task_add_callback_droppable(SystemTaskEventCallback cb, void *data); bool system_task_add_callback_from_isr_droppable(SystemTaskEventCallback cb, void *data, bool *should_context_switch); +//! Droppable ISR callback that raises KernelBG priority while pending or executing. +//! The queue releases the priority reference after the callback returns. +bool system_task_add_callback_from_isr_droppable_raised(SystemTaskEventCallback cb, void *data, + bool *should_context_switch); + //! @param cb Callback function that will later be called from the system task //! @param data Context pointer passed to the callback bool system_task_add_callback(SystemTaskEventCallback cb, void *data); @@ -52,10 +57,8 @@ uint32_t system_task_get_available_space(void); //! Debug! Return the callback we're currently executing. void *system_task_get_current_callback(void); -//! @param is_raised When true, priority of the KernelBG task is raised to a higher priority. When -//! false, the priority is set to the normal priority. -//! @note WARNING: if you want to use this, implement ref counting internally. Currently only -//! comm/session.c uses this hence we can get away without ref counting. +//! Acquires or releases a reference that keeps KernelBG at a higher priority. +//! @param is_raised True to acquire a reference, false to release one. Calls must be balanced. void system_task_enable_raised_priority(bool is_raised); //! @return True if the KernelBG task is ready to run (i.e. not blocked by mutex / queue) diff --git a/src/fw/drivers/speaker/nrf5/da7212.c b/src/fw/drivers/speaker/nrf5/da7212.c index a343a7f426..fe9ee12fcf 100644 --- a/src/fw/drivers/speaker/nrf5/da7212.c +++ b/src/fw/drivers/speaker/nrf5/da7212.c @@ -304,8 +304,8 @@ static void prv_maybe_request_refill_from_isr(AudioDeviceState *state) { // underrun beats resetting the system over a full queue. state->callback_pending = true; bool should_context_switch = false; - if (!system_task_add_callback_from_isr_droppable(prv_audio_trans_bg, state, - &should_context_switch)) { + if (!system_task_add_callback_from_isr_droppable_raised(prv_audio_trans_bg, state, + &should_context_switch)) { state->callback_pending = false; } } diff --git a/src/fw/drivers/speaker/qemu/audio.c b/src/fw/drivers/speaker/qemu/audio.c index 2550b46759..da3c49d949 100644 --- a/src/fw/drivers/speaker/qemu/audio.c +++ b/src/fw/drivers/speaker/qemu/audio.c @@ -4,7 +4,7 @@ #include #include -#include "services/system_task.h" +#include "pbl/services/system_task.h" #include @@ -30,15 +30,13 @@ void audio_init(AudioDevice *dev) { } static void prv_audio_system_task_cb(void *data) { - AudioDeviceState *state = (AudioDeviceState *)data; - if (state->trans_cb) { - // Read how many samples the QEMU ring buffer can accept. - // We don't have the base_addr here directly, but the callback - // will trigger audio_write() which checks BUFAVAIL itself. - // Pass a generous free size — the actual limit is enforced by - // audio_write() returning when the QEMU buffer is full. - uint32_t free_bytes = 4096 * sizeof(int16_t); - state->trans_cb(&free_bytes); + AudioDevice *dev = data; + dev->state->callback_pending = false; + if (dev->state->trans_cb) { + uint32_t free_bytes = REG32(dev->base_addr + AUDIO_BUFAVAIL) * sizeof(int16_t); + if (free_bytes) { + dev->state->trans_cb(&free_bytes); + } } } @@ -60,12 +58,16 @@ uint32_t audio_write(AudioDevice *dev, void *buf, uint32_t size) { int16_t *samples = (int16_t *)buf; uint32_t num_samples = size / sizeof(int16_t); + uint32_t avail = REG32(dev->base_addr + AUDIO_BUFAVAIL); + if (num_samples > avail) { + num_samples = avail; + } for (uint32_t i = 0; i < num_samples; i++) { REG32(dev->base_addr + AUDIO_DATA) = (uint32_t)(uint16_t)samples[i]; } // Return how many bytes of free space remain - uint32_t avail = REG32(dev->base_addr + AUDIO_BUFAVAIL); + avail = REG32(dev->base_addr + AUDIO_BUFAVAIL); return avail * sizeof(int16_t); } @@ -87,9 +89,12 @@ void qemu_audio_irq_handler(AudioDevice *dev) { REG32(dev->base_addr + AUDIO_INTSTAT) = INT_BUFAVAIL; // Schedule callback on system task; a drop is retried on the next interrupt - if (dev->state->trans_cb) { + if (dev->state->trans_cb && !dev->state->callback_pending) { bool should_context_switch = false; - system_task_add_callback_from_isr_droppable(prv_audio_system_task_cb, (void *)dev->state, - &should_context_switch); + dev->state->callback_pending = true; + if (!system_task_add_callback_from_isr_droppable_raised(prv_audio_system_task_cb, (void *)dev, + &should_context_switch)) { + dev->state->callback_pending = false; + } } } diff --git a/src/fw/drivers/speaker/sf32lb52/audec.c b/src/fw/drivers/speaker/sf32lb52/audec.c index aede175b4d..e7efa61f5d 100644 --- a/src/fw/drivers/speaker/sf32lb52/audec.c +++ b/src/fw/drivers/speaker/sf32lb52/audec.c @@ -458,8 +458,8 @@ static void prv_dma_request_processing(AudioDeviceState *state) { if (state->trans_cb && !state->callback_pending && free_size >= CFG_AUDIO_PLAYBACK_PIPE_SIZE) { bool system_task_switch_context = false; state->callback_pending = true; - if (!system_task_add_callback_from_isr_droppable(prv_audio_trans_bg, (void *)state, - &system_task_switch_context)) { + if (!system_task_add_callback_from_isr_droppable_raised(prv_audio_trans_bg, (void *)state, + &system_task_switch_context)) { state->callback_pending = false; } } diff --git a/src/fw/services/speaker/speaker_service.c b/src/fw/services/speaker/speaker_service.c index 606efbdd91..59dedf5ef8 100644 --- a/src/fw/services/speaker/speaker_service.c +++ b/src/fw/services/speaker/speaker_service.c @@ -97,7 +97,7 @@ typedef struct { static SpeakerServiceState s_state; -// Serializes public APIs against prv_refill_bg (system task). +// Serializes public APIs against the audio refill callback (system task). static PBL_MUTEX_DEFINE(s_lock); //! Why playback is currently silent, cached so a muted watch logs once per change @@ -115,7 +115,7 @@ static uint32_t s_total_speaker_on_time_ms; // Total speaker on-time tracked static void prv_stop_internal(SpeakerFinishReason reason); static void prv_audio_trans_cb(uint32_t *free_size); -static void prv_refill_bg(void *data); +static void prv_refill_locked(void); static bool prv_is_speaker_muted(void) { if (alerts_preferences_get_speaker_muted()) { @@ -301,8 +301,25 @@ static bool prv_can_preempt(SpeakerPriority new_pri) { //! This is the DMA refill callback path: //! DMA ISR -> system_task_add_callback_from_isr -> audio driver trans_cb -> here static void prv_audio_trans_cb(uint32_t *free_size) { - // Schedule actual refill work on system task to keep ISR-context callback short - system_task_add_callback(prv_refill_bg, NULL); + // The drivers already dispatch on KernelBG; refill here and catch up missed blocks. + uint32_t refill_count = free_size ? *free_size / (SPEAKER_REFILL_SAMPLES * sizeof(int16_t)) : 1; + if (refill_count == 0) { + return; + } + + pbl_mutex_lock(&s_lock, PBL_FOREVER); + if (s_state.source_type != SpeakerSourceStream) { + refill_count = 1; + } + while (refill_count-- && s_state.state != SpeakerStateIdle) { + prv_refill_locked(); + // Catch up queued PCM without inserting silence between short packets. + if (s_state.source_type == SpeakerSourceStream && + pcm_stream_available(&s_state.pcm_stream) == 0) { + break; + } + } + pbl_mutex_unlock(&s_lock); } //! Convert a raw sample from the input buffer to 16-bit signed. @@ -489,12 +506,6 @@ static void prv_refill_locked(void) { } } -static void prv_refill_bg(void *data) { - pbl_mutex_lock(&s_lock, PBL_FOREVER); - prv_refill_locked(); - pbl_mutex_unlock(&s_lock); -} - bool speaker_service_play_note_seq(const SpeakerNote *notes, uint32_t num_notes, SpeakerPriority pri, uint8_t vol) { pbl_mutex_lock(&s_lock, PBL_FOREVER); diff --git a/src/fw/services/system_task/service.c b/src/fw/services/system_task/service.c index 189225b63e..d3156eab2e 100644 --- a/src/fw/services/system_task/service.c +++ b/src/fw/services/system_task/service.c @@ -15,6 +15,8 @@ #include "pbl/kernel/msgq.h" #include "pbl/kernel/poll.h" #include "pbl/kernel/thread.h" +#include "pbl/kernel/irq.h" +#include "system/passert.h" PBL_LOG_MODULE_DEFINE(service_system_task, CONFIG_SERVICE_SYSTEM_TASK_LOG_LEVEL); @@ -23,6 +25,7 @@ PBL_LOG_MODULE_DEFINE(service_system_task, CONFIG_SERVICE_SYSTEM_TASK_LOG_LEVEL) typedef struct { SystemTaskEventCallback cb; void *data; + bool raised_priority; } SystemTaskEvent; #define SYSTEM_TASK_QUEUE_LENGTH 30 @@ -38,6 +41,7 @@ static SystemTaskEventCallback s_current_cb; static bool s_system_task_idle = true; static bool s_should_block_callbacks = false; +static uint32_t s_raised_priority_refcount; static bool prv_is_accepting_callbacks() { return s_initialized && !s_should_block_callbacks; @@ -71,6 +75,9 @@ static void system_task_main(void *paramater) { event.cb(event.data); mcu_fpu_cleanup(); s_current_cb = NULL; + if (event.raised_priority) { + system_task_enable_raised_priority(false); + } } // Refresh the watchdog immediately, just in case that cb() took awhile to run. @@ -79,6 +86,7 @@ static void system_task_main(void *paramater) { } void system_task_init(void) { + s_raised_priority_refcount = 0; pbl_poll_group_add(&s_system_task_queue_set, &s_system_task_queue); pbl_poll_group_add(&s_system_task_queue_set, &s_from_app_system_task_queue); s_initialized = true; @@ -171,6 +179,31 @@ bool system_task_add_callback_from_isr_droppable(SystemTaskEventCallback cb, voi return system_task_add_callback_droppable(cb, data); } +bool system_task_add_callback_from_isr_droppable_raised(SystemTaskEventCallback cb, void *data, + bool *should_context_switch) { + *should_context_switch = false; + if (!prv_is_accepting_callbacks()) { + return false; + } + + SystemTaskEvent event = { + .cb = cb, + .data = data, + .raised_priority = true, + }; + + // The queued event owns the boost until its callback returns, even if the + // device stops in the meantime. Rejected work must not retain a reference. + pbl_irq_lock(); + system_task_enable_raised_priority(true); + bool success = (pbl_msgq_put(&s_system_task_queue, &event, PBL_NO_WAIT) == 0); + if (!success) { + system_task_enable_raised_priority(false); + } + pbl_irq_unlock(); + return success; +} + bool system_task_add_callback(SystemTaskEventCallback cb, void *data) { uintptr_t caller_lr = (uintptr_t)__builtin_return_address(0); if (!prv_is_accepting_callbacks()) { @@ -216,8 +249,30 @@ void *system_task_get_current_callback(void) { void system_task_enable_raised_priority(bool is_raised) { const pbl_prio_t raised_priority_level = PBL_PRIO_IDLE + 3; // Same as KernelMain / BT tasks - pbl_thread_prio_set(pebble_task_get_thread(PebbleTask_KernelBackground), - is_raised ? raised_priority_level : SYSTEM_TASK_PRIORITY); + + pbl_irq_lock(); + if (is_raised) { + PBL_ASSERTN(s_raised_priority_refcount < UINT32_MAX); + if (s_raised_priority_refcount == UINT32_MAX) { + pbl_irq_unlock(); + return; + } + if (s_raised_priority_refcount++ == 0) { + pbl_thread_prio_set(pebble_task_get_thread(PebbleTask_KernelBackground), + raised_priority_level); + } + } else { + PBL_ASSERTN(s_raised_priority_refcount > 0); + if (s_raised_priority_refcount == 0) { + pbl_irq_unlock(); + return; + } + if (--s_raised_priority_refcount == 0) { + pbl_thread_prio_set(pebble_task_get_thread(PebbleTask_KernelBackground), + SYSTEM_TASK_PRIORITY); + } + } + pbl_irq_unlock(); } bool system_task_is_ready_to_run(void) { diff --git a/tests/fw/services/test_speaker_service.c b/tests/fw/services/test_speaker_service.c index 14d68e9256..d9b1ed369a 100644 --- a/tests/fw/services/test_speaker_service.c +++ b/tests/fw/services/test_speaker_service.c @@ -252,3 +252,21 @@ void test_speaker_service__stream_write_keeps_16bit_samples_aligned(void) { cl_assert(speaker_service_stream_open(SpeakerPriorityApp, 50, SpeakerPcmFormat_8kHz_8bit)); cl_assert_equal_i(speaker_service_stream_write(bytes, 3), 3); } + +void test_speaker_service__refill_catches_up_to_driver_capacity(void) { + cl_assert(speaker_service_stream_open(SpeakerPriorityApp, 50, SpeakerPcmFormat_16kHz_16bit)); + int16_t input[1536] = {0}; + cl_assert_equal_i(speaker_service_stream_write(input, sizeof(input)), sizeof(input)); + + uint32_t space = 1024 * sizeof(int16_t); + s_trans_cb(&space); + cl_assert_equal_i(s_samples_written, 1024); + + space = 0; + s_trans_cb(&space); + cl_assert_equal_i(s_samples_written, 1024); + + space = 1024 * sizeof(int16_t); + s_trans_cb(&space); + cl_assert_equal_i(s_samples_written, 1536); +}