From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [RFC PATCH (alsa-lib)] pcm: Modify check condition in snd_pcm_sw_params_set_avail_min Date: Thu, 03 Sep 2015 09:08:14 +0200 Message-ID: References: <1441250454-38271-1-git-send-email-koro.chen@mediatek.com> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1441250454-38271-1-git-send-email-koro.chen@mediatek.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Koro Chen Cc: alsa-devel@alsa-project.org, broonie@kernel.org, linux-mediatek@lists.infradead.org, lgirdwood@gmail.com, srv_heupstream@mediatek.com List-Id: linux-mediatek@lists.infradead.org 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? > 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. thanks, Takashi > > Signed-off-by: Koro Chen > --- > 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 >