[alsa-devel] [PATCH 0/5] ASoC: fsl_ssi: Fixing various channel slips and bad samples insertions

[arnaud.mouiche at invoxia.com]

Le 12/01/2016 00:44, Caleb Crome a écrit :
> On Sat, Jan 9, 2016 at 3:02 AM, arnaud.mouiche at invoxia.com
> <arnaud.mouiche at invoxia.com> wrote:
>> Hello Caleb
>>
>> Le 09/01/2016 01:47, Caleb Crome a écrit :
>>> [...]
>>>
>>> Hello Arnaud,
>>>     I have finally gotten to test your patches, and I'm still having
>>> trouble with channel slips.
>>>
>>> I applied your v2 patch set, along with your changes for using a dummy
>>> codec.
>>>
>>> The full changes are here:
>>> https://github.com/ccrome/linux-caleb-dev/tree/v4.4-rc8-armv7-x3
>>>
>>> This ignores most of my previous patches, and uses your code to bring
>>> up the SSI (without a codec) on a wandboard.
>>>
>>> I am using SSI3, and doing a hardware loopback between TX and RX.
>>>
>>> Here's what I run:
>>> ./atest -r 16000 -c 8 -p 2048   -D default play
>>>
>>> which plays continuously.
>>>
>>> and in another shell:
>>> ./atest -r 16000 -c 8 -p 2048  -D default -d 10 capture
>>>
>>> which captures for 10 seconds.
>>>
>>>
>>> The first time I run the capture command, it succeeds, no problem.
>>>> dbg: dev: 'default'
>>>> dbg: default: capture_start
>>>> dbg: start a 10 seconds duration timer
>>>> warn: First valid frame
>>>> warn:   3400 3401 3402 3403 3404 3405 3406 3407
>>>> dbg: end of tests
>>>> total number of sequence errors: 0
>>>> global tests exit status: OK
>>> But the second and all subsequent captures, it fails with channel slips:
>>>
>>>> dbg: dev: 'default'
>>>> dbg: default: capture_start
>>>> dbg: start a 10 seconds duration timer
>>>> err: invalid frame after 0 null frames
>>>> err:   d2a1 d2a2 d2a3 d2a4 d2a5 d2a6 d2a7 d2c0
>>>> err:   d2c1 d2c2 d2c3 d2c4 d2c5 d2c6 d2c7 78e0
>>>> dbg: end of tests
>>>> total number of sequence errors: 430080
>>>> global tests exit status: OK
>>
>> Can you use -I option to get a little more log of the error
>> $ ./atest -r 16000 -c 8 -p 2048  -D default -d 10 -I 10 capture
>>
>> Just to know if the wrong frames comes from the previous record session,
>> meaning the RX fifo was not cleared correctly,
>> as if the CCSR_SSI_SOR_RX_CLR bit is not working as we might expect.
>> (ie. d2a1 .. d2c7 may come from the previous recording, and 78e0 .. may be
>> the start of the new recording session)
> That does appear to be what's happening.  I read the SFCSR before and
> after you update the SOR register (SOR_RX_CLR), and in both cases the
> rx buffer is full on the second enablement.
>
> I can't seem to empty that buffer, either by using the SOR_RX_CLR or
> by reading the SRX0 register. This is another one of those cases where
> there you cannot modify the register (i.e. fifo or RX0) when RE is
> disabled.
>
> So, instead of clearing on enable, I'm clearing on shutdown and on
> enable.  I attempted using the SOR_RX_CLR, but it doesn't seem to work
> on the MX6.  Rather, I clear the fifo by reading all the words in a
> loop, just before RE is disabled.
so the imx6.sl SSI behaves differently from the imx6.solo SSI ... Or 
some things have changes between 4.3 and 4.4.

May be SOR_RX_CLR and/or RX0 registers can't be read on 'solo' because 
de the SRCR.RFEN0 [or1] are not enabled yet.

In fact, clearing the fifos at the end may still introduce a race since 
the RX path is still enabled when your read from fifo, and
Samples may still be received in the meantime...

Also, I see that the CCSR_SSI_SOR register is not in the regmap 
fsl_ssi_volatile_reg() list (line 140). May be there is an implication 
of some caching mechanism ?

Can you try to move the the fifo clearing part just before the write in 
CCSR_SSI_SIER with the following change in top of my patches ?
Also may be you can additionally add the RX0 / RX1 reading from clearing 
mechanism at the same place.

On my side, I will check the 4.4 tree on imx6sl

Regards,
Arnaud

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 0277097..bd163b2 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -420,6 +420,9 @@ static void fsl_ssi_config(struct fsl_ssi_private 
*ssi_private, int enable,
          * (online configuration)
          */
         if (enable) {
+
+               regmap_update_bits(regs, CCSR_SSI_SRCR, vals->srcr, 
vals->srcr);
+               regmap_update_bits(regs, CCSR_SSI_STCR, vals->stcr, 
vals->stcr);
                 /*
                  * Clear RX or TX FIFO to remove samples from the previous
                  * stream session which may be still present in the 
FIFO and
@@ -435,9 +438,6 @@ static void fsl_ssi_config(struct fsl_ssi_private 
*ssi_private, int enable,
                         regmap_update_bits(regs, CCSR_SSI_SOR,
                                 CCSR_SSI_SOR_TX_CLR, CCSR_SSI_SOR_TX_CLR);

                 }
-
-               regmap_update_bits(regs, CCSR_SSI_SRCR, vals->srcr, 
vals->srcr);
-               regmap_update_bits(regs, CCSR_SSI_STCR, vals->stcr, 
vals->stcr);
                 regmap_update_bits(regs, CCSR_SSI_SIER, vals->sier, 
vals->sier);
         } else {
                 u32 sier;