On Mon, 29 Oct 2018 16:24:55 +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.

Well, no, snd_pcm_period_elapsed() calls don't guarantee the expectation above in the case of dmix. It's clearer to assume that the stream were started just one frame before the next period, for example.

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

---

We are not completely sure if this is the best approach but we had also no better idea to solve this problem (with similar CPU usage).

Therefore if anyone has a better solution please feel free to describe this one.

The problem is that aligning the start essentially imposes an artificial long latency, and changes the behavior out of sudden.

Hence I'm inclined to make it optional; e.g. create a dmix config align_hw_ptr, and let user decide. It'll be a string, and as default (e.g. "auto"), we should keep the current behavior. For other values, it can be translated as a boolean (call snd_config_get_bool_ascii()) to choose whether to align or not.

thanks,

Takashi

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

diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c index a6a8f3a..d3e9319 100644 --- a/src/pcm/pcm_dmix.c +++ b/src/pcm/pcm_dmix.c @@ -560,11 +560,10 @@ 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;

• if (pcm->buffer_size > pcm->period_size * 2)

• return;
  

• /* If we have too litte periods, better to align the start position

• * to the period boundary so that the interrupt can be handled properly
  

• * at the right time.
  

• dmix->slave_hw_ptr = ((dmix->slave_hw_ptr / dmix->slave_period_size)

• 		* dmix->slave_period_size);
  

• /* Better to align the start position to the period boundary so that

• * the interrupt can be handled properly at the right time.
  

  */ dmix->slave_appl_ptr = ((dmix->slave_appl_ptr + dmix->slave_period_size - 1) / dmix->slave_period_size) * dmix->slave_period_size;

-- 2.7.4