On Tue, 30 Oct 2018 13:04:33 +0100, Timo Wischer wrote:

Best regards Timo Wischer Engineering Software Base (ADITG/ESB)

Tel. +49 5121 49 6938 On 10/30/18 11:49, Takashi Iwai wrote:

On Tue, 30 Oct 2018 11:39:38 +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>
    ---

        On 10/29/18 16:54, Takashi Iwai wrote:
        The problem is that aligning the start essentially imposes an
        artificial long latency, and changes the behavior out of sudden.

    Now, we are only align the salve_hw_ptr which also solves our delay issue.
    But this change should not increase the latency because the write position
    is given by slave_appl_ptr.
    Do you see any other drawbacks?

How does this work?  I thought hwptr is synced to the actual slave
value soon later in anyway?

The slave_hw_ptr is initialized in reset_slave_ptr() and updated in snd_pcm_dmix_sync_ptr() with

dmix->slave_hw_ptr = *dmix->spcm->hw.ptr;

The hw_ptr will be incremented in snd_pcm_dmix_sync_ptr() by adding the diff between the last slave_hw_ptr and the current slave_hw_ptr. Due to the alignment of the slave_hw_ptr on startup (in reset_slave_ptr()) the offset  between the slave_hw_ptr and hw_ptr is a multiple of slave_period.

Yes, and is this correct? Suppose you start a stream at the position one sample before the next period (psize * N + (psize-1)). Then slave_hw_ptr is psize * N. At the next moment, the dpcm->hw.ptr reaches to psize * (N+1), and snd_pcm_dmix_sync_ptr() gets called. Then slave_hw_ptr will be updated to psize * (N+1) although only one sample has been processed?

The following slave_hw_ptr will be always x * period_size + y where x is always incremented for each snd_pcm_period_elapsed() call. y can be sometimes 0 but also contain some frames due to the delay between the DMA interrupt till reading the current DMA position.

The diff between the salve_hw_ptr will be continuously added to hw_ptr. Sometimes the diff can be < period_size but anyway the hw_ptr follows the same interval as the salve_hw_ptr:

• 1 * period_size + y1
• 2 * period_size + y2
• 3 * period_size + y3
• ...

y can differ between the snd_pcm_dmix_sync_ptr() calls. But it is never <0. Therefore the application can always write one period if it writes all chunks in periods.

The update timing is right, but what about the first samples?

thanks,

Takashi

Best regards

Timo

thanks,

Takashi

    Best regards

    Timo

    diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c
    index a6a8f3a..eaa0b0f 100644
    --- a/src/pcm/pcm_dmix.c
    +++ b/src/pcm/pcm_dmix.c
    @@ -560,6 +560,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
    -- 
    2.7.4