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

Commit Message

Caleb Crome Jan. 11, 2016, 11:44 p.m. UTC

On Sat, Jan 9, 2016 at 3:02 AM, [e-mail]
<[e-mail]> 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.



>
> May be you can read the RX fifo level just avec applying CCSR_SSI_SOR_RX_CLR
> to assert the fifo is really empty ?
>
> In case it is still not empty, we will still have the possibility to empty
> the fifo by ready the samples one by one manually.

so far, with cursory testing, this patch, on top of your V2 patches
seems to work.



From 15a218858a75163e2d72ed8329a26b0d22a0bfc0 Mon Sep 17 00:00:00 2001
From: Caleb Crome <[e-mail]>
Date: Mon, 11 Jan 2016 15:29:13 -0800
Subject: [PATCH] ASoC: fsl_ssi: Empty rx fifo on rx start and finish to
 eliminate channel slips

Arnaud's previous patches mostly, but not completely work on the MX6.
The previous patch used the SOR_RX_CLR register, which doesn't
seem to work on the MX6.  I guess it's undocumented for a reason.

This patch clears the RX fifo by using successive reads of the
SRX0 (and optionally SRX1) register until the SISR_RDR0/1 bit
is clear.  This ensure's an empty fifo to start capturing with.

Signed-off-by: Caleb Crome <[e-mail]>
---
 sound/soc/fsl/fsl_ssi.c | 44 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 37 insertions(+), 7 deletions(-)

  */
@@ -428,6 +462,7 @@ static void fsl_ssi_config(struct fsl_ssi_private
*ssi_private, bool enable,
     u32 scr_val;
     int keep_active;

+    int is_rx = vals->scr & CCSR_SSI_SCR_RE;
     regmap_read(regs, CCSR_SSI_SCR, &scr_val);

     nr_active_streams = !!(scr_val & CCSR_SSI_SCR_TE) +
@@ -478,13 +513,7 @@ static void fsl_ssi_config(struct fsl_ssi_private
*ssi_private, bool enable,
          * Note: The SOR is not documented in recent IMX datasheet, but
          * is described in IMX51 reference manual at section 56.3.3.15.
          */
-        if (vals->scr & CCSR_SSI_SCR_RE) {
-            regmap_update_bits(regs, CCSR_SSI_SOR,
-                CCSR_SSI_SOR_RX_CLR, CCSR_SSI_SOR_RX_CLR);
-        } else {
-            regmap_update_bits(regs, CCSR_SSI_SOR,
-                CCSR_SSI_SOR_TX_CLR, CCSR_SSI_SOR_TX_CLR);
-        }
+        fsl_empty_fifo(ssi_private->dev, regs, is_rx,
ssi_private->use_dual_fifo);

         regmap_update_bits(regs, CCSR_SSI_SRCR, vals->srcr, vals->srcr);
         regmap_update_bits(regs, CCSR_SSI_STCR, vals->stcr, vals->stcr);
@@ -502,6 +531,7 @@ static void fsl_ssi_config(struct fsl_ssi_private
*ssi_private, bool enable,
          * disabled now. Otherwise we could alter flags of the other
          * stream
          */
+        fsl_empty_fifo(ssi_private->dev, regs, is_rx,
ssi_private->use_dual_fifo);

         /* These assignments are simply vals without bits set in avals*/
         sier = fsl_ssi_disable_val(vals->sier, avals->sier,

Patch

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index e44c9c3..5329aaa 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -366,6 +366,40 @@  static irqreturn_t fsl_ssi_isr(int irq, void *dev_id)
     return IRQ_HANDLED;
 }

+// empty fifo:  must be called when the unit is ENABLED!
+// otherwise it will not be able to empty the fifos.
+static u32 fsl_empty_fifo(struct device *dev, struct regmap *regs,
int is_rx, int dual_fifo)
+{
+    u32 val;
+    u32 sisr;
+    u32 err = 0;
+    int limit = 100;
+    if (is_rx) {
+        do {
+            regmap_read(regs, CCSR_SSI_SRX0, &val);
+            regmap_read(regs, CCSR_SSI_SISR, &sisr);
+        } while ((limit-- >=0) && (sisr & CCSR_SSI_SISR_RDR0));
+        if (limit <= 0)  {
+            dev_err(dev, "Couldn't empty out the RX "
+                "FIFO 0.  SISR = 0x%08x\n", sisr);
+            err = 1;
+        }
+        if (dual_fifo) {
+            do {
+                regmap_read(regs, CCSR_SSI_SRX1, &val);
+                regmap_read(regs, CCSR_SSI_SISR, &sisr);
+            } while ((limit-- >=0) &&
+                 (sisr & CCSR_SSI_SISR_RDR1));
+            if (limit <= 0)  {
+                dev_err(dev, "Couldn't empty out the RX "
+                    "FIFO 1.  SISR = 0x%08x\n", sisr);
+                err = 1;
+            }
+        }
+    }
+    return err;
+}
+
 /*
  * Enable/Disable all rx/tx config flags at once.