[alsa-devel] [PATCH v2 0/6] ASoC: fsl_ssi: Fixing various channel slips and bad samples insertions
This series of patch is an attempt to fix the fsl_ssi driver to use it in TDM mode (DSP A or B) with a large number of channels/slots.
Version 2: - fixing a missing patch - checkpatch.pl - ... well, learning how to send a patchset. Sorry. ;)
Bugs are detected and fixed on a imx6sl platform with linux 4.3, where 2 SSI interfaces are used. SSI3 configured as a master of the bus, SSI2 as a slave.
Various loopback scenario are tested: * scenario 1: SSI3 (master) with TXD to RXD loopback
| |---> TXD --\ SSI3 |<--- RXD --/ (master) |---> SYNC |---> BCLK | * scenario 2: SSI3 connected to SSI2 | |---> TXD --------\ SSI3 |<--- RXD -----\ | (master) |---> SYNC --\ | | |---> BCLK | | | | | | | | | | | | | |<--- BCLK | | | SSI2 |<--- SYNC --/ | | (slave) |---> TXD ------/ | |<--- RXD --------/ |
A test software (called atest) was developed and available. [1] It basically generate/check specials frames with S16_LE sequence samples NNN0, NNN1, NNN2 ... NNNC with NNN = frame number, and C the number of channels-1
Patch 1: Limitation fix. Prerequisite to use the fsl_ssi driver with up to 32 channels / slots
Patch 2: Bug fix. Prerequisite to setup a relative high bitclk for our tests in 8ch / 16bits / 48kHz
Patch 3: Simply save a 'dev' reference inside ssi_private for dev_err() purpose. (ssi_private is only available in lot of places, and ssi_private->pdev is deprecated and NULL indeed)
Patch 4: Fix playback samples being dropped because the TX fifo was not ready at the time the DMA starts filling (only in case where playback is started AFTER the capture)
Detected in loopback scenario 1, with following script (> 80 % of reproducibility) $ atest -D SSI3 -c 8 -r 48000 capture play dbg: set period size: 960 dbg: SSI3: capture_start dbg: SSI3: playback_start err: invalid frame after 1 null frames err: 0000 0000 0013 0014 0015 0016 0017 0020 dbg: SIGINT total number of sequence errors: 21119 here we see that samples 0000 0001 ... 0012 are not present in the output. In addition, this the number of samples dropped is not a multiple of the number of channel, we have a a channel slip.
Patch 5: This is the Caleb Crome's case [2], where the SSI starts to generate samples while the TX FIFO is not filled by the DMA yet. Void samples can be inserted before DMA streaming.
Detected in loopback scenario 2, with following script (reproducibility < 1%) $ atest -D SSI3 -c 8 -r 48000 capture & $ sleep 0.2 $ atest -d 1 -D SSI2 -c 8 -r 48000 play dbg: SSI3: capture_start dbg: set period size: 960 dbg: SSI2: playback_start dbg: start a 1 seconds duration timer err: invalid frame after 11568 null frames err: 0000 0010 0011 0012 0013 0014 0015 0016 err: 0017 0020 0021 0022 0023 0024 0025 0026
Patch 6: Deals with Capture restart whereas Playback is still running (or the opposite, Playback restart whereas Capture is till running). Capture restart whereas Playback case is detected in loopback scenario 1 (reproducibility 100%). A playback session is running in background, and 2 consecutive Captures are performed. The first is fine, the second fails. We see at the 2nd capture startup that we receive 15 samples still pending in the RX FIFO from the previous capture session. => We are receiving invalid samples + slips the channels $ atest -D SSI3 -c 8 -r 48000 play & dbg: SSI3: playback_start $ atest -d 1 -D SSI3 -c 8 -r 48000 capture dbg: SSI3: capture_start dbg: start a 1 seconds duration timer warn: Valid frame after 1 null frames warn: 3a90 3a91 3a92 3a93 3a94 3a95 3a96 3a97 total number of sequence errors: 0 $ atest -I 5 -d 1 -D SSI3 -c 8 -r 48000 capture # restart the capture dbg: SSI3: capture_start dbg: start a 1 seconds duration timer err: invalid frame after 1 null frames err: 8dd6 8dd7 8de0 8de1 8de2 8de3 8de4 8de5 err: 8de6 8de7 8df0 8df1 8df2 8df3 8df4 c0c0 err: c0c1 c0c2 c0c3 c0c4 c0c5 c0c6 c0c7 c0d0 err: c0d1 c0d2 c0d3 c0d4 c0d5 c0d6 c0d7 c0e0 err: c0e1 c0e2 c0e3 c0e4 c0e5 c0e6 c0e7 c0f0
Playback restart whereas Capture case is also detected with loopback scenario 1 (reproducibility 100%), capturing continuously, and playing by periods of time. $ atest -a -D SSI3 -c 8 -r 48000 capture play -r 1000,200 dbg: SSI3: capture_start dbg: SSI3: playback_start dbg: SSI3: will stop every 1000 ms during 200 ms warn: Valid frame after 4 null frames warn: 0010 0011 0012 0013 0014 0015 0016 0017 warn: SSI3: PT_W4_STOP warn: SSI3: PT_W4_RESTART err: invalid frame after 1602 null frames err: eaa1 eaa2 eaa3 eaa4 eaa5 eaa6 eaa7 eab0 err: eab1 eab2 eab3 eab4 5810 5811 5812 5813 dbg: stop on first error total number of sequence errors: 143
Both cases are resolved by using the mostly undocumented SOR.RX_CLR and SOR TX_CLR (we can find the documentation in IMX51 reference manual at section 56.3.3.15). Arnaud
[1] https://github.com/amouiche/atest [2] http://mailman.alsa-project.org/pipermail/alsa-devel/2015-October/099221.htm...
Arnaud Mouiche (6): ASoC: fsl_ssi: Real hardware channels max number is 32 ASoC: fsl_ssi: The IPG/5 limitation concerns the bitclk, not the sysclk. ASoC: fsl_ssi: Save a dev reference for dev_err() purpose. ASoC: fsl_ssi: Fix samples being dropped as Playback startup ASoC: fsl_ssi: Fix channel slipping in Playback at startup ASoC: fsl_ssi: Fix channel slipping on capture (or playback) restart in full duplex.
sound/soc/fsl/fsl_ssi.c | 74 +++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 63 insertions(+), 11 deletions(-)
The max number of slots in TDM mode is 32: - Frame Rate Divider Control is a 5bit value - Time slot mask registers control 32 slots.
Signed-off-by: Arnaud Mouiche arnaud.mouiche@invoxia.com --- sound/soc/fsl/fsl_ssi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 37c5cd4..4e5d7db 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -1084,14 +1084,14 @@ static struct snd_soc_dai_driver fsl_ssi_dai_template = { .playback = { .stream_name = "CPU-Playback", .channels_min = 1, - .channels_max = 2, + .channels_max = 32, .rates = FSLSSI_I2S_RATES, .formats = FSLSSI_I2S_FORMATS, }, .capture = { .stream_name = "CPU-Capture", .channels_min = 1, - .channels_max = 2, + .channels_max = 32, .rates = FSLSSI_I2S_RATES, .formats = FSLSSI_I2S_FORMATS, },
Hi Arnaud,
On Thu, Nov 26, 2015 at 1:05 PM, Arnaud Mouiche arnaud.mouiche@invoxia.com wrote:
The max number of slots in TDM mode is 32:
- Frame Rate Divider Control is a 5bit value
- Time slot mask registers control 32 slots.
Signed-off-by: Arnaud Mouiche arnaud.mouiche@invoxia.com
Thanks for your series. It seems you missed to Cc the SSI driver maintainer: Nicolin Chen.
The patch
ASoC: fsl_ssi: Real hardware channels max number is 32
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 48a260eec301fd7a112d1737ca2755d91558a349 Mon Sep 17 00:00:00 2001
From: Arnaud Mouiche arnaud.mouiche@invoxia.com Date: Tue, 3 May 2016 14:13:55 +0200 Subject: [PATCH] ASoC: fsl_ssi: Real hardware channels max number is 32
The max number of slots in TDM mode is 32: - Frame Rate Divider Control is a 5bit value - Time slot mask registers control 32 slots.
Signed-off-by: Arnaud Mouiche arnaud.mouiche@invoxia.com Reviewed-by: Fabio Estevam fabio.estevam@nxp.com Tested-by: Caleb Crome caleb@crome.org Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/fsl/fsl_ssi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index ed8de1035cda..8d5f3c192de2 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -1158,14 +1158,14 @@ static struct snd_soc_dai_driver fsl_ssi_dai_template = { .playback = { .stream_name = "CPU-Playback", .channels_min = 1, - .channels_max = 2, + .channels_max = 32, .rates = FSLSSI_I2S_RATES, .formats = FSLSSI_I2S_FORMATS, }, .capture = { .stream_name = "CPU-Capture", .channels_min = 1, - .channels_max = 2, + .channels_max = 32, .rates = FSLSSI_I2S_RATES, .formats = FSLSSI_I2S_FORMATS, },
im6sl reference manual 47.7.4: " Bit clock - Used to serially clock the data bits in and out of the SSI port. This clock is either generated internally (from SSI's sys clock) or taken from external clock source (through the Tx/Rx clock ports). [...] Care should be taken to ensure that the bit clock frequency (either internally generated by dividing the SSI's sys clock or sourced from external device through Tx/Rx clock ports) is never greater than 1/5 of the ipg_clk (from CCM) frequency. "
Since, in master mode, the sysclk is a multiple of bitclk, we can easily reach a high sysclk value, whereas keeping a reasonable bitclk.
ex: 8ch x 16bit x 48kHz = 6144000, requires a 24576000 sysclk (PM=1) yet ipg_clk/5 = 66Mhz/5 = 13.2
Signed-off-by: Arnaud Mouiche arnaud.mouiche@invoxia.com --- sound/soc/fsl/fsl_ssi.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 4e5d7db..b34c09f 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -595,6 +595,15 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream *substream, if (IS_ERR(ssi_private->baudclk)) return -EINVAL;
+ /* + * Hardware limitation: The bclk rate must be + * never greater than 1/5 IPG clock rate + */ + if (freq * 5 > clk_get_rate(ssi_private->clk)) { + dev_err(cpu_dai->dev, "bitclk > ipgclk/5\n"); + return -EINVAL; + } + baudclk_is_used = ssi_private->baudclk_streams & ~(BIT(substream->stream));
/* It should be already enough to divide clock by setting pm alone */ @@ -611,13 +620,6 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream *substream, else clkrate = clk_round_rate(ssi_private->baudclk, tmprate);
- /* - * Hardware limitation: The bclk rate must be - * never greater than 1/5 IPG clock rate - */ - if (clkrate * 5 > clk_get_rate(ssi_private->clk)) - continue; - clkrate /= factor; afreq = clkrate / (i + 1);
Most of functions only receive the ssi_private reference and don't have a knowledge of 'dev' pointer, even for debug purpose.
Signed-off-by: Arnaud Mouiche arnaud.mouiche@invoxia.com --- sound/soc/fsl/fsl_ssi.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index b34c09f..243c9af 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -191,6 +191,7 @@ struct fsl_ssi_private { struct fsl_ssi_dbg dbg_stats;
const struct fsl_ssi_soc_data *soc; + struct device *dev; };
/* @@ -1329,6 +1330,7 @@ static int fsl_ssi_probe(struct platform_device *pdev) }
ssi_private->soc = of_id->data; + ssi_private->dev = &pdev->dev;
sprop = of_get_property(np, "fsl,mode", NULL); if (sprop) {
If the capture is already running while playback is started, it is highly probable (>80% in a 8 channels scenario) that samples are lost between the DMA and TX fifo.
The reason is that SIER.TDMAE is set before STCR.TFEN0, leaving a time window where the FIFO doesn't receive the samples written by the DMA.
This particular case happened only if capture is already enabled as SCR.SSIEN is already set at the playback startup instant.
Signed-off-by: Arnaud Mouiche arnaud.mouiche@invoxia.com --- sound/soc/fsl/fsl_ssi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 243c9af..78ea6e1 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -404,9 +404,9 @@ static void fsl_ssi_config(struct fsl_ssi_private *ssi_private, bool enable, * (online configuration) */ if (enable) { - regmap_update_bits(regs, CCSR_SSI_SIER, vals->sier, vals->sier); 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; u32 srcr;
Previously, SCR.SSIEN and SCR.TE were enabled at once if no capture stream was also running. This may not give a chance for the DMA to write the first sample in TX FIFO before the streaming starts on the PCM bus, inserting void samples first. Those void samples are then responsible for slipping the channels.
Signed-off-by: Arnaud Mouiche arnaud.mouiche@invoxia.com --- sound/soc/fsl/fsl_ssi.c | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 78ea6e1..9d5c677 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -436,8 +436,40 @@ static void fsl_ssi_config(struct fsl_ssi_private *ssi_private, bool enable,
config_done: /* Enabling of subunits is done after configuration */ - if (enable) + if (enable) { + if (ssi_private->use_dma && (vals->scr & CCSR_SSI_SCR_TE)) { + /* + * Be sure the Tx FIFO is filled when TE is set. + * Otherwise, there are some chances to start the + * playback with some void samples inserted first, + * generating a channel slip. + * + * First, SSIEN must be set, to let the FIFO be filled. + * + * Notes: + * - Limit this fix to the DMA case until FIQ cases can + * be tested. + * - Limit the length of the busy loop to not lock the + * system too long, even if 1-2 loops are sufficient + * in general. + */ + int i; + int max_loop = 100; + regmap_update_bits(regs, CCSR_SSI_SCR, + CCSR_SSI_SCR_SSIEN, CCSR_SSI_SCR_SSIEN); + for (i = 0; i < max_loop; i++) { + u32 sfcsr; + regmap_read(regs, CCSR_SSI_SFCSR, &sfcsr); + if (CCSR_SSI_SFCSR_TFCNT0(sfcsr)) + break; + } + if (i == max_loop) { + dev_err(ssi_private->dev, + "Timeout waiting TX FIFO filling\n"); + } + } regmap_update_bits(regs, CCSR_SSI_SCR, vals->scr, vals->scr); + } }
The patch
ASoC: fsl_ssi: Fix channel slipping in Playback at startup
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 61fcf10a0ee44763e0347b297a377137f8950772 Mon Sep 17 00:00:00 2001
From: Arnaud Mouiche arnaud.mouiche@invoxia.com Date: Tue, 3 May 2016 14:13:59 +0200 Subject: [PATCH] ASoC: fsl_ssi: Fix channel slipping in Playback at startup
Previously, SCR.SSIEN and SCR.TE were enabled at once if no capture stream was also running. This may not give a chance for the DMA to write the first sample in TX FIFO before the streaming starts on the PCM bus, inserting void samples first. Those void samples are then responsible for slipping the channels.
Signed-off-by: Arnaud Mouiche arnaud.mouiche@invoxia.com Reviewed-by: Fabio Estevam fabio.estevam@nxp.com Tested-by: Caleb Crome caleb@crome.org Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/fsl/fsl_ssi.c | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 47ebb835f3f5..8944af542b4f 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -507,8 +507,40 @@ static void fsl_ssi_config(struct fsl_ssi_private *ssi_private, bool enable,
config_done: /* Enabling of subunits is done after configuration */ - if (enable) + if (enable) { + if (ssi_private->use_dma && (vals->scr & CCSR_SSI_SCR_TE)) { + /* + * Be sure the Tx FIFO is filled when TE is set. + * Otherwise, there are some chances to start the + * playback with some void samples inserted first, + * generating a channel slip. + * + * First, SSIEN must be set, to let the FIFO be filled. + * + * Notes: + * - Limit this fix to the DMA case until FIQ cases can + * be tested. + * - Limit the length of the busy loop to not lock the + * system too long, even if 1-2 loops are sufficient + * in general. + */ + int i; + int max_loop = 100; + regmap_update_bits(regs, CCSR_SSI_SCR, + CCSR_SSI_SCR_SSIEN, CCSR_SSI_SCR_SSIEN); + for (i = 0; i < max_loop; i++) { + u32 sfcsr; + regmap_read(regs, CCSR_SSI_SFCSR, &sfcsr); + if (CCSR_SSI_SFCSR_TFCNT0(sfcsr)) + break; + } + if (i == max_loop) { + dev_err(ssi_private->dev, + "Timeout waiting TX FIFO filling\n"); + } + } regmap_update_bits(regs, CCSR_SSI_SCR, vals->scr, vals->scr); + } }
Happened when the Playback (or Capture) is running continuously and Capture (or Playback) is restarted (xrun, manual stop/start...)
Since the RX (or TX) FIFO are only reset when the whole SSI is disabled, pending samples from previous capture (or playback) session may still be present. They must be erased to not introduce channel slipping.
FIFO Clear register fields are documented in IMX51, IMX35 reference manual. They are not documented in IMX50 or IMX6 RM, despite they are working as expected on IMX6SL.
Signed-off-by: Arnaud Mouiche arnaud.mouiche@invoxia.com --- sound/soc/fsl/fsl_ssi.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 9d5c677..0fd548e 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -404,6 +404,22 @@ static void fsl_ssi_config(struct fsl_ssi_private *ssi_private, bool enable, * (online configuration) */ if (enable) { + /* + * Clear RX or TX FIFO to remove samples from the previous + * stream session which may be still present in the FIFO and + * may introduce bad samples and/or channel slipping. + * + * 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); + } + 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);
Hi all.
Yes, Caleb and I are working on issues when dealing with N channels, N > 2, but everyone can experience those issues with N=2. It is true that a Left/Right switching on audio in not tragic for most users, but I think is will be good if this kind of channel switch can be definitely fixed by the patches I propose.
Here is a very basic test to demonstrate a 2 channels classical scenario that everyone can experience on a imx6 SOC.
#!/bin/sh
amixer set PCM 100%,0% arecord -D hw:0,0 -c 2 -r 48000 -f S16_LE /tmp/foo.wav </dev/null & pid=$! speaker-test -t sine -D hw:0,0 -r 48000 -c 2 -l 1 kill $pid
what is done: - Right channel is muted at the codec level (so if we try to play something on the right channel only, nothing is heard). - We start recording (arecord) - then we start a tone generation (speaker-test), starting by the left channel during few first seconds.
what is expected: - We should hear the tone on left channel right after speaker-test starts.
what is experienced: - 50% of the time, we don't here anything, until speaker-test switch to the right meaning Left and Right channels are switched.
Regards, Arnaud
Le 26/11/2015 16:05, Arnaud Mouiche a écrit :
This series of patch is an attempt to fix the fsl_ssi driver to use it in TDM mode (DSP A or B) with a large number of channels/slots.
Version 2:
- fixing a missing patch
- checkpatch.pl
- ... well, learning how to send a patchset. Sorry. ;)
Bugs are detected and fixed on a imx6sl platform with linux 4.3, where 2 SSI interfaces are used. SSI3 configured as a master of the bus, SSI2 as a slave.
Various loopback scenario are tested:
scenario 1: SSI3 (master) with TXD to RXD loopback
| |---> TXD --\SSI3 |<--- RXD --/ (master) |---> SYNC |---> BCLK |
scenario 2: SSI3 connected to SSI2 | |---> TXD --------\ SSI3 |<--- RXD -----\ | (master) |---> SYNC --\ | | |---> BCLK | | | | | | | | | | | | | |<--- BCLK | | | SSI2 |<--- SYNC --/ | | (slave) |---> TXD ------/ | |<--- RXD --------/ |
A test software (called atest) was developed and available. [1] It basically generate/check specials frames with S16_LE sequence samples NNN0, NNN1, NNN2 ... NNNC with NNN = frame number, and C the number of channels-1
Patch 1: Limitation fix. Prerequisite to use the fsl_ssi driver with up to 32 channels / slots
Patch 2: Bug fix. Prerequisite to setup a relative high bitclk for our tests in 8ch / 16bits / 48kHz
Patch 3: Simply save a 'dev' reference inside ssi_private for dev_err() purpose. (ssi_private is only available in lot of places, and ssi_private->pdev is deprecated and NULL indeed)
Patch 4: Fix playback samples being dropped because the TX fifo was not ready at the time the DMA starts filling (only in case where playback is started AFTER the capture)
Detected in loopback scenario 1, with following script (> 80 % of reproducibility) $ atest -D SSI3 -c 8 -r 48000 capture play dbg: set period size: 960 dbg: SSI3: capture_start dbg: SSI3: playback_start err: invalid frame after 1 null frames err: 0000 0000 0013 0014 0015 0016 0017 0020 dbg: SIGINT total number of sequence errors: 21119
here we see that samples 0000 0001 ... 0012 are not present in the output. In addition, this the number of samples dropped is not a multiple of the number of channel, we have a a channel slip.
Patch 5: This is the Caleb Crome's case [2], where the SSI starts to generate samples while the TX FIFO is not filled by the DMA yet. Void samples can be inserted before DMA streaming.
Detected in loopback scenario 2, with following script (reproducibility < 1%) $ atest -D SSI3 -c 8 -r 48000 capture & $ sleep 0.2 $ atest -d 1 -D SSI2 -c 8 -r 48000 play dbg: SSI3: capture_start dbg: set period size: 960 dbg: SSI2: playback_start dbg: start a 1 seconds duration timer err: invalid frame after 11568 null frames err: 0000 0010 0011 0012 0013 0014 0015 0016 err: 0017 0020 0021 0022 0023 0024 0025 0026
Patch 6: Deals with Capture restart whereas Playback is still running (or the opposite, Playback restart whereas Capture is till running).
Capture restart whereas Playback case is detected in loopback scenario 1 (reproducibility 100%). A playback session is running in background, and 2 consecutive Captures are performed. The first is fine, the second fails.
We see at the 2nd capture startup that we receive 15 samples still pending in the RX FIFO from the previous capture session. => We are receiving invalid samples + slips the channels
$ atest -D SSI3 -c 8 -r 48000 play & dbg: SSI3: playback_start $ atest -d 1 -D SSI3 -c 8 -r 48000 capture dbg: SSI3: capture_start dbg: start a 1 seconds duration timer warn: Valid frame after 1 null frames warn: 3a90 3a91 3a92 3a93 3a94 3a95 3a96 3a97 total number of sequence errors: 0
$ atest -I 5 -d 1 -D SSI3 -c 8 -r 48000 capture # restart the capture dbg: SSI3: capture_start dbg: start a 1 seconds duration timer err: invalid frame after 1 null frames err: 8dd6 8dd7 8de0 8de1 8de2 8de3 8de4 8de5 err: 8de6 8de7 8df0 8df1 8df2 8df3 8df4 c0c0 err: c0c1 c0c2 c0c3 c0c4 c0c5 c0c6 c0c7 c0d0 err: c0d1 c0d2 c0d3 c0d4 c0d5 c0d6 c0d7 c0e0 err: c0e1 c0e2 c0e3 c0e4 c0e5 c0e6 c0e7 c0f0
Playback restart whereas Capture case is also detected with loopback scenario 1 (reproducibility 100%), capturing continuously, and playing by periods of time.
$ atest -a -D SSI3 -c 8 -r 48000 capture play -r 1000,200 dbg: SSI3: capture_start dbg: SSI3: playback_start dbg: SSI3: will stop every 1000 ms during 200 ms warn: Valid frame after 4 null frames warn: 0010 0011 0012 0013 0014 0015 0016 0017 warn: SSI3: PT_W4_STOP warn: SSI3: PT_W4_RESTART err: invalid frame after 1602 null frames err: eaa1 eaa2 eaa3 eaa4 eaa5 eaa6 eaa7 eab0 err: eab1 eab2 eab3 eab4 5810 5811 5812 5813 dbg: stop on first error total number of sequence errors: 143
Both cases are resolved by using the mostly undocumented SOR.RX_CLR and SOR TX_CLR (we can find the documentation in IMX51 reference manual at section 56.3.3.15).
Arnaud
[1] https://github.com/amouiche/atest [2] http://mailman.alsa-project.org/pipermail/alsa-devel/2015-October/099221.htm...
Arnaud Mouiche (6): ASoC: fsl_ssi: Real hardware channels max number is 32 ASoC: fsl_ssi: The IPG/5 limitation concerns the bitclk, not the sysclk. ASoC: fsl_ssi: Save a dev reference for dev_err() purpose. ASoC: fsl_ssi: Fix samples being dropped as Playback startup ASoC: fsl_ssi: Fix channel slipping in Playback at startup ASoC: fsl_ssi: Fix channel slipping on capture (or playback) restart in full duplex.
sound/soc/fsl/fsl_ssi.c | 74 +++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 63 insertions(+), 11 deletions(-)
On Mon, Dec 7, 2015 at 4:41 AM, arnaud.mouiche@invoxia.com arnaud.mouiche@invoxia.com wrote:
Hi all.
Yes, Caleb and I are working on issues when dealing with N channels, N > 2, but everyone can experience those issues with N=2. It is true that a Left/Right switching on audio in not tragic for most users, but I think is will be good if this kind of channel switch can be definitely fixed by the patches I propose.
Here is a very basic test to demonstrate a 2 channels classical scenario that everyone can experience on a imx6 SOC.
#!/bin/sh
amixer set PCM 100%,0% arecord -D hw:0,0 -c 2 -r 48000 -f S16_LE /tmp/foo.wav </dev/null & pid=$! speaker-test -t sine -D hw:0,0 -r 48000 -c 2 -l 1 kill $pid
what is done:
- Right channel is muted at the codec level (so if we try to play something on the right channel only, nothing is heard).
- We start recording (arecord)
- then we start a tone generation (speaker-test), starting by the left channel during few first seconds.
what is expected:
- We should hear the tone on left channel right after speaker-test starts.
what is experienced:
- 50% of the time, we don't here anything, until speaker-test switch to the right meaning Left and Right channels are switched.
Regards, Arnaud
Since I haven't been testing with channels==2, I never noticed that the problem was very common even with 2 channels! Yikes.
I've been delayed in working on your patch set. I hope to get to it soon, but other deadlines are looming, so I might have to wait for the new year to put it together.
Thanks so much for the patches, I'm really looking forward to finally having a robust MX6 driver.
-Caleb
participants (5)
-
Arnaud Mouiche -
arnaud.mouiche＠invoxia.com
-
Caleb Crome -
Fabio Estevam -
Mark Brown