On Sat, Jan 9, 2016 at 3:02 AM, arnaud.mouiche@invoxia.com arnaud.mouiche@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.
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 caleb@crome.org 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 caleb@crome.org --- sound/soc/fsl/fsl_ssi.c | 44 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 7 deletions(-)
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. */ @@ -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,