Skip to content

Commit

Permalink
cxd56xx/cxd56_audio: Fix an issue when driver is stopping
Browse files Browse the repository at this point in the history 
The audio driver release an audio buffer to user application via message
queue. A message queue can be used in irq context, but it has limitation
of depth of the queue.
So the driver stacked when the driver is stopping with many audio buffers
over the limitation, because it tries to release all audio buffers by
mq_send.

To avoid this behavior, change that releasing audio buffers are done in
LPWORKER.
  • Loading branch information
@SPRESENSE
SPRESENSE committed Nov 5, 2024
1 parent 6e34b36 commit 744b436
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 16 deletions.
76 changes: 60 additions & 16 deletions arch/arm/src/cxd56xx/cxd56_audio_dma.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ static struct cxd56_dmachannel_s mic_dma =
dma_mic_chassign, /* chassign */
cxd56_audio_irq_en, /* irqen */
NULL, /* irq_arg */
{{{{0}}}}, /* delay_work */
NULL, /* setvolumme */
NULL, /* maxch */
AUDIO_TYPE_INPUT | AUDIO_TYPE_FEATURE, /* caps */
Expand Down Expand Up @@ -202,6 +203,7 @@ static struct cxd56_i2sdmachannel_s i2s0out_dma =
dma_i2s_chassign, /* chassign */
cxd56_audio_i2sirq_en, /* irqen */
NULL, /* irq_arg */
{{{{0}}}}, /* delay_work */
NULL, /* setvolumme */
NULL, /* maxch */
AUDIO_TYPE_OUTPUT | AUDIO_TYPE_FEATURE, /* caps */
Expand Down Expand Up @@ -247,6 +249,7 @@ static struct cxd56_i2sdmachannel_s i2s1out_dma =
dma_i2s_chassign, /* chassign */
cxd56_audio_i2sirq_en, /* irqen */
NULL, /* irq_arg */
{{{{0}}}}, /* delay_work */
NULL, /* setvolumme */
NULL, /* maxch */
AUDIO_TYPE_OUTPUT | AUDIO_TYPE_FEATURE, /* caps */
Expand Down Expand Up @@ -292,6 +295,7 @@ static struct cxd56_i2sdmachannel_s i2s1in_dma =
dma_i2s_chassign, /* chassign */
cxd56_audio_i2sirq_en, /* irqen */
NULL, /* irq_arg */
{{{{0}}}}, /* delay_work */
NULL, /* setvolumme */
NULL, /* maxch */
AUDIO_TYPE_INPUT | AUDIO_TYPE_FEATURE, /* caps */
Expand Down Expand Up @@ -383,6 +387,49 @@ static void send_message_underrun(FAR struct cxd56_dmachannel_s *dmach)
(FAR struct ap_buffer_s *)&msg, OK);
}

static void release_all_apb_for_stopping(FAR void *arg)
{
FAR struct cxd56_dmachannel_s *dmach =
(FAR struct cxd56_dmachannel_s *)arg;

FAR struct ap_buffer_s *apb;
irqstate_t flags;

flags = spin_lock_irqsave(&dmach->dma_lock);
if (AUDSTATE_IS_STOPPING(dmach) || AUDSTATE_IS_COMPLETING(dmach))
{
while ((apb = dq_get(dmach->dma_pendq)) != NULL)
{
spin_unlock_irqrestore(&dmach->dma_lock, flags);
dequeue_apb_to_app(dmach, apb);
flags = spin_lock_irqsave(&dmach->dma_lock);
}
}

AUDSTATECHG_CONFIGURED(dmach);
spin_unlock_irqrestore(&dmach->dma_lock, flags);
}

static void release_all_apb_for_shutdown(FAR void *arg)
{
FAR struct cxd56_dmachannel_s *dmach =
(FAR struct cxd56_dmachannel_s *)arg;

FAR struct ap_buffer_s *apb;
irqstate_t flags;

flags = spin_lock_irqsave(&dmach->dma_lock);
while ((apb = dq_get(dmach->dma_pendq)) != NULL)
{
spin_unlock_irqrestore(&dmach->dma_lock, flags);
dequeue_apb_to_app(dmach, apb);
flags = spin_lock_irqsave(&dmach->dma_lock);
}

AUDSTATECHG_SHUTDOWN(dmach);
spin_unlock_irqrestore(&dmach->dma_lock, flags);
}

static void handle_actual_dmaaction(FAR struct cxd56_dmachannel_s *dmach,
uint32_t intbit)
{
Expand Down Expand Up @@ -423,42 +470,39 @@ static void handle_actual_dmaaction(FAR struct cxd56_dmachannel_s *dmach,
dmach->hwresource_irq(dmach, false);
leave_critical_section(flags);

flags = spin_lock_irqsave(&dmach->dma_lock);

if (AUDSTATE_IS_STOPPING(dmach))
if (work_available(&dmach->delay_work))
{
/* Release buffers not processed */
/* Set delay worker for releasing all apb in pendq */

while ((apb = dq_get(dmach->dma_pendq)) != NULL)
{
spin_unlock_irqrestore(&dmach->dma_lock, flags);
dequeue_apb_to_app(dmach, apb);
flags = spin_lock_irqsave(&dmach->dma_lock);
}
work_queue(LPWORK, &dmach->delay_work, release_all_apb_for_stopping,
(FAR void *)dmach, 0);
}

AUDSTATECHG_CONFIGURED(dmach);
flags = spin_lock_irqsave(&dmach->dma_lock);
}
else if (AUDSTATE_IS_PAUSING(dmach))
{
AUDSTATECHG_PAUSED(dmach);
}
else
{
spin_unlock_irqrestore(&dmach->dma_lock, flags);

flags = enter_critical_section();
dmach->hwresource_irq(dmach, false);
leave_critical_section(flags);

/* Release buffers not processed */

while ((apb = dq_get(dmach->dma_pendq)) != NULL)
if (work_available(&dmach->delay_work))
{
spin_unlock_irqrestore(&dmach->dma_lock, flags);
dequeue_apb_to_app(dmach, apb);
flags = spin_lock_irqsave(&dmach->dma_lock);
/* Set delay worker for releasing all apb in pendq */

work_queue(LPWORK, &dmach->delay_work, release_all_apb_for_shutdown,
(FAR void *)dmach, 0);
}

AUDSTATECHG_SHUTDOWN(dmach);
flags = spin_lock_irqsave(&dmach->dma_lock);
}
}
else if (intbit & dmach->intrbit_err || intbit & dmach->intrbit_cmb)
Expand Down
2 changes: 2 additions & 0 deletions arch/arm/src/cxd56xx/cxd56_audio_driver.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <stdint.h>
#include <nuttx/audio/audio.h>
#include <nuttx/queue.h>
#include <nuttx/wqueue.h>
#include "arch/chip/cxd56_audio_lower.h"
#include "cxd56_audio_reg.h"

Expand Down Expand Up @@ -147,6 +148,7 @@ struct cxd56_dmachannel_s
CODE void (*chassign)(FAR struct cxd56_dmachannel_s *dma_ch);
CODE int (*irqen)(FAR struct cxd56_dmachannel_s *dmach, bool en);
void *irq_arg;
struct work_s delay_work;

/* Related on NuttX audio framework */

Expand Down

0 comments on commit 744b436

Please sign in to comment.