[-,dmix,v3,1/1] pcm: dmix: Align slave_hw_ptr to slave period boundary
diff mbox series

Message ID 1541508773-32718-1-git-send-email-twischer@de.adit-jv.com
State New
Headers show
Series
  • pcm: dmix: Align slave_hw_ptr to slave period boundary
Related show

Commit Message

Timo Wischer Nov. 6, 2018, 12:52 p.m. UTC
From: Laxmi Devi <Laxmi.Devi@in.bosch.com>

These changes are required due to the kernel commit 07b7acb51d283d8469696c906b91f1882696a4d4
("ASoC: rsnd: update pointer more accurate")

Issue is that snd_pcm_wait() goes back to waiting because the hw_ptr
is not period aligned. Therefore snd_pcm_wait() will block for a longer
time as required.

With these rcar driver changes the exact position of the dma is returned.
During snd_pcm_start they read hw_ptr as reference, and this hw_ptr
is now not period aligned, and is a little ahead over the period while it
is read. Therefore when the avail is calculated during snd_pcm_wait(),
it is missing the avail_min by a few frames.
Consider the below example:

Considering the application is trying to write 0x120 frames and the
period_size = 0x60, avail_min = 0x120 and buffersize = 0x360 :

rsnd_pointer=0x12c -> dma pointer at the end of second write during
snd_pcm_dmix_start().
Since another 0x120 buffer is available, application writes 0x120 and goes
to snd_pcm_wait().
It is woken up after 3 snd_pcm_period_elapsed() to see rsnd_pointer=0x248.
So hw_ptr = new_slave_hw_ptr - reference_slave_hw_ptr = 0x248 - 0x12c = 0x11c.
It needs 4 more frames to be able to write. And so it goes back to waiting.

But since 3 snd_pcm_period_elapsed(), 3 periods should be available and it
should have been able to write.
If rsnd_pointer during the start was 0x120 which is 3 periods
then 0x248 - 0x120 =  128 it could go on with write.

Signed-off-by: Laxmi Devi <Laxmi.Devi@in.bosch.com>
Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
---

With this patch the slave_hw_ptr will always contain a period align value.

 src/pcm/pcm_dmix.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Takashi Iwai Nov. 6, 2018, 1:14 p.m. UTC | #1
On Tue, 06 Nov 2018 13:52:53 +0100,
<twischer@de.adit-jv.com> wrote:
> 
> From: Laxmi Devi <Laxmi.Devi@in.bosch.com>
> 
> These changes are required due to the kernel commit 07b7acb51d283d8469696c906b91f1882696a4d4
> ("ASoC: rsnd: update pointer more accurate")
> 
> Issue is that snd_pcm_wait() goes back to waiting because the hw_ptr
> is not period aligned. Therefore snd_pcm_wait() will block for a longer
> time as required.
> 
> With these rcar driver changes the exact position of the dma is returned.
> During snd_pcm_start they read hw_ptr as reference, and this hw_ptr
> is now not period aligned, and is a little ahead over the period while it
> is read. Therefore when the avail is calculated during snd_pcm_wait(),
> it is missing the avail_min by a few frames.
> Consider the below example:
> 
> Considering the application is trying to write 0x120 frames and the
> period_size = 0x60, avail_min = 0x120 and buffersize = 0x360 :
> 
> rsnd_pointer=0x12c -> dma pointer at the end of second write during
> snd_pcm_dmix_start().
> Since another 0x120 buffer is available, application writes 0x120 and goes
> to snd_pcm_wait().
> It is woken up after 3 snd_pcm_period_elapsed() to see rsnd_pointer=0x248.
> So hw_ptr = new_slave_hw_ptr - reference_slave_hw_ptr = 0x248 - 0x12c = 0x11c.
> It needs 4 more frames to be able to write. And so it goes back to waiting.
> 
> But since 3 snd_pcm_period_elapsed(), 3 periods should be available and it
> should have been able to write.
> If rsnd_pointer during the start was 0x120 which is 3 periods
> then 0x248 - 0x120 =  128 it could go on with write.
> 
> Signed-off-by: Laxmi Devi <Laxmi.Devi@in.bosch.com>
> Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
> ---
> 
> With this patch the slave_hw_ptr will always contain a period align value.

By enforcing that, application won't be able to get the fine-grained
hwptr update any longer, hence it is a clear regression.

That is, we can't apply this as is.  If any, such adjustment has to be
applied only conditionally for certain hardware setup.


thanks,

Takashi

Patch
diff mbox series

diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c
index 881154e..6c9e052 100644
--- a/src/pcm/pcm_dmix.c
+++ b/src/pcm/pcm_dmix.c
@@ -398,6 +398,8 @@  static int snd_pcm_dmix_sync_ptr0(snd_pcm_t *pcm, snd_pcm_uframes_t slave_hw_ptr
 	snd_pcm_sframes_t diff;
 	
 	old_slave_hw_ptr = dmix->slave_hw_ptr;
+	slave_hw_ptr = ((slave_hw_ptr / dmix->slave_period_size)
+				*  dmix->slave_period_size);
 	dmix->slave_hw_ptr = slave_hw_ptr;
 	diff = slave_hw_ptr - old_slave_hw_ptr;
 	if (diff == 0)		/* fast path */
@@ -560,6 +562,8 @@  static int snd_pcm_dmix_hwsync(snd_pcm_t *pcm)
 static void reset_slave_ptr(snd_pcm_t *pcm, snd_pcm_direct_t *dmix)
 {
 	dmix->slave_appl_ptr = dmix->slave_hw_ptr = *dmix->spcm->hw.ptr;
+	dmix->slave_hw_ptr = ((dmix->slave_hw_ptr / dmix->slave_period_size)
+				* dmix->slave_period_size);
 	if (pcm->buffer_size > pcm->period_size * 2)
 		return;
 	/* If we have too litte periods, better to align the start position