Re: [PATCH v2 4/6] pcm: direct: Round up of slave_app_ptr pointer if buffer size is less than 2 period size.

From: Takashi Iwai <tiwai@suse.de>
To: "Channaiah Vanitha (RBEI/ECF3)" <Vanitha.Channaiah@in.bosch.com>
Cc: "Wischer Timo (ADITG/ESS)" <twischer@de.adit-jv.com>,
	"alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>
Subject: Re: [PATCH v2 4/6] pcm: direct: Round up of slave_app_ptr pointer if buffer size is less than 2 period size.
Date: Tue, 16 Jul 2019 07:04:51 +0200	[thread overview]
Message-ID: <s5h8ssyefik.wl-tiwai@suse.de> (raw)
In-Reply-To: <b5b496a391ca4b4981894a572d14ab92@in.bosch.com>

On Tue, 16 Jul 2019 05:57:51 +0200,
Channaiah Vanitha (RBEI/ECF3) wrote:
> 
> Hello Takashi-san,
> 
> Can you please reply your feedback for below mail chain.
> 
> Best regards,
> Vanitha Channaiah 
> RBEI/ECF3
> 
> _____________________________________________
> From: Channaiah Vanitha (RBEI/ECF3) 
> Sent: Tuesday, June 18, 2019 4:44 AM
> To: 'Takashi Iwai' <tiwai@suse.de>
> Cc: alsa-devel@alsa-project.org; Wischer Timo (ADITG/ESS) <twischer@de.adit-jv.com>
> Subject: RE: [PATCH v2 4/6] pcm: direct: Round up of slave_app_ptr pointer if buffer size is less than 2 period size.
> 
> 
> Hello Takashi-san,
> 
> Firstly, very sorry for the late reply.
> 
> > The current condition was chosen because otherwise it'll cause 
> > underrun errors.  If the round down is needed for avoiding errors, it 
> > should be changed, yes.  Otherwise, it needs a careful evaluation.
> 
> > If buffer=2*period, the chance to slip the update is quite high unless you align the start.  And the instability with 2xperiod is the very reason we've added this hack at the beginning.
> 
> If this is the case, current condition would also solve under run errors for the condition buffer=2*period+1frame.
> For, buffer=2*period+1frame, snd_pcm_wait() is waiting for more than a period and this could lead to xruns.
> The chance to slip the condition is more in case of buffer=2*period-1frame. Because, after first write (one period write), avail is less than one period by 1 frame. It has to wait until next DMA interrupt. There is more chance of xruns.
> In case of buffer=2*period, after first write (one period write) avail is still one period size and there is less chance of xruns.	
> 	
> > In anyway, the description in the patch doesn't match with the change.  
> > Please update it to fit with the actual change if we still need to 
> > take this change inevitably.
> 	
> For buffer>=2*period, round down of slave pointers and for buffer=2*period-1frame, round up of slave pointers will avoid xruns. 
> Which otherwise causes snd_pcm_wait() to block for more than a period time and leads to xruns.

Hmm, it's still not clear at all.  Please repost the patch with a more
elaborated and correct description that matches with the code change.

thanks,

Takashi