[alsa-devel] [RFC PATCH (alsa-lib)] pcm: Modify check condition in snd_pcm_sw_params_set_avail_min
Koro Chen
koro.chen at mediatek.com
Thu Sep 3 00:45:46 PDT 2015
On Thu, 2015-09-03 at 09:08 +0200, Takashi Iwai wrote:
> On Thu, 03 Sep 2015 05:20:54 +0200,
> Koro Chen wrote:
> >
> > When we use a ping-ping buffer in capture, and if hw_ptr reported
> > at IRQ is a little smaller than period_size:
> >
> > |xxxxxxxxxxxxxxxxxxxxxxxxxxxx--|-----------------------------|
> > hw_ptr < period_size
>
> How this happens? The period size is the size where irq (or wakeup)
> wakes up for read/write. Why the driver wakes up even if there is no
> enough data?
>
Yes it is odd to what we would normally expect. Due to our HW design,
when irq comes, audio HW actually has collected a full period of data,
but there is a buffer between the audio HW and memory, so at that moment
some samples are still in the buffer, not on the memory. Add a small
delay between triggering capture HW and enabling IRQ can also fix this,
although I think changing the avail_min should be better.
> > This available buffer will not be read since its size is smaller than
> > avail_min (which is set to be period_size), and read thread continues
> > to sleep. If the next hw_ptr is just a little larger than buffer_size,
> > overrun occurs.
> >
> > This could be resolved by setting a small avail_min to kernel,
> > for example, 1, so read thread wakes up and reads every data at IRQ.
> > But current alsa-lib only allows avail_min to be at least period_size.
> > Remove the constraint and only check for zero case.
>
> The restriction was introduced for avoiding CPU hogs with rate plugin
> in many years ago. avail_min=1 *might* work now because of the later
> fix for rate plugin, but this must be verified.
>
Sounds a little dangerous...How should I verify this? It this problem
platform dependent?
>
> thanks,
>
> Takashi
>
> >
> > Signed-off-by: Koro Chen <koro.chen at mediatek.com>
> > ---
> > src/pcm/pcm.c | 8 ++------
> > 1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
> > index f5fc728..8492689 100644
> > --- a/src/pcm/pcm.c
> > +++ b/src/pcm/pcm.c
> > @@ -5958,12 +5958,8 @@ int snd_pcm_sw_params_set_avail_min(snd_pcm_t *pcm, snd_pcm_sw_params_t *params,
> > #endif
> > {
> > assert(pcm && params);
> > - /* Fix avail_min if it's below period size. The period_size
> > - * defines the minimal wake-up timing accuracy, so it doesn't
> > - * make sense to set below that.
> > - */
> > - if (val < pcm->period_size)
> > - val = pcm->period_size;
> > + if (!val)
> > + val = 1;
> > params->avail_min = val;
> > return 0;
> > }
> > --
> > 1.7.9.5
> >
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
More information about the Linux-mediatek
mailing list