[alsa-devel] [PATCH bisect 0/2] ASoC: fsl_sai: Overwrite trigger()
This series of patches are bisected from the preivous version so as to apply the PATCH-1 onto asoc-v3.15-4.
* The patches are generated by using '-U2' because the default '-U3' would conflict the baseline without fsl_sai_isr patches.
Nicolin Chen (2): ASoC: fsl_sai: Fix buggy configurations in trigger() ASoC: fsl_sai: Separately enable interrupts for Tx and Rx streams
sound/soc/fsl/fsl_sai.c | 43 +++++++++++++++++++++++-------------------- sound/soc/fsl/fsl_sai.h | 11 +++++++++++ 2 files changed, 34 insertions(+), 20 deletions(-)
The current trigger() has two crucial problems: 1) The DMA request enabling operations (FSL_SAI_CSR_FRDE) for Tx and Rx are now totally exclusive: It would fail to run simultaneous Tx-Rx cases. 2) The TERE disabling operation depends on an incorrect condition -- active reference count that only gets increased in snd_pcm_open() and decreased in snd_pcm_close(): The TERE would never get cleared.
So this patch overwrites the trigger function by following these rules: A) We continue to support tx-async-while-rx-sync-to-tx case alone, which's originally limited by this fsl_sai driver, but we make the code easy to modify for the further support of the opposite case. B) We enable both TE and RE for PLAYBACK stream or CAPTURE stream but only enabling the DMA request bit (FSL_SAI_CSR_FRDE) of the current direction due to the requirement of SAI -- For tx-async-while-rx-sync-to-tx case, the receiver is enabled only when both the transmitter and receiver are enabled.
Tested cases: a) aplay test.wav -d5 b) arecord -r44100 -c2 -fS16_LE test.wav -d5 c) arecord -r44100 -c2 -fS16_LE -d5 | aplay d) (aplay test2.wav &); sleep 1; arecord -r44100 -c2 -fS16_LE test.wav -d1 e) (arecord -r44100 -c2 -fS16_LE test.wav -d5 &); sleep 1; aplay test.wav -d1
Signed-off-by: Nicolin Chen Guangyu.Chen@freescale.com Acked-by: Xiubo Li Li.Xiubo@freescale.com --- sound/soc/fsl/fsl_sai.c | 35 +++++++++++++++++------------------ sound/soc/fsl/fsl_sai.h | 10 ++++++++++ 2 files changed, 27 insertions(+), 18 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c index f088545..bdfd497 100644 --- a/sound/soc/fsl/fsl_sai.c +++ b/sound/soc/fsl/fsl_sai.c @@ -366,4 +366,5 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd, { struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai); + bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK; u32 tcsr, rcsr;
@@ -380,12 +381,4 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd, regmap_read(sai->regmap, FSL_SAI_RCSR, &rcsr);
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { - tcsr |= FSL_SAI_CSR_FRDE; - rcsr &= ~FSL_SAI_CSR_FRDE; - } else { - rcsr |= FSL_SAI_CSR_FRDE; - tcsr &= ~FSL_SAI_CSR_FRDE; - } - /* * It is recommended that the transmitter is the last enabled @@ -396,20 +389,26 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd, case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: - tcsr |= FSL_SAI_CSR_TERE; - rcsr |= FSL_SAI_CSR_TERE; + if (!(tcsr & FSL_SAI_CSR_FRDE || rcsr & FSL_SAI_CSR_FRDE)) { + regmap_update_bits(sai->regmap, FSL_SAI_RCSR, + FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE); + regmap_update_bits(sai->regmap, FSL_SAI_TCSR, + FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE); + }
- regmap_write(sai->regmap, FSL_SAI_RCSR, rcsr); - regmap_write(sai->regmap, FSL_SAI_TCSR, tcsr); + regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx), + FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE); break; case SNDRV_PCM_TRIGGER_STOP: case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_PAUSE_PUSH: - if (!(cpu_dai->playback_active || cpu_dai->capture_active)) { - tcsr &= ~FSL_SAI_CSR_TERE; - rcsr &= ~FSL_SAI_CSR_TERE; + regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx), + FSL_SAI_CSR_FRDE, 0); + + if (!(tcsr & FSL_SAI_CSR_FRDE || rcsr & FSL_SAI_CSR_FRDE)) { + regmap_update_bits(sai->regmap, FSL_SAI_TCSR, + FSL_SAI_CSR_TERE, 0); + regmap_update_bits(sai->regmap, FSL_SAI_RCSR, + FSL_SAI_CSR_TERE, 0); } - - regmap_write(sai->regmap, FSL_SAI_TCSR, tcsr); - regmap_write(sai->regmap, FSL_SAI_RCSR, rcsr); break; default: diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h index a264185..64b6fe7 100644 --- a/sound/soc/fsl/fsl_sai.h +++ b/sound/soc/fsl/fsl_sai.h @@ -36,4 +36,14 @@ #define FSL_SAI_RMR 0xe0 /* SAI Receive Mask */
+#define FSL_SAI_xCSR(tx) (tx ? FSL_SAI_TCSR : FSL_SAI_RCSR) +#define FSL_SAI_xCR1(tx) (tx ? FSL_SAI_TCR1 : FSL_SAI_RCR1) +#define FSL_SAI_xCR2(tx) (tx ? FSL_SAI_TCR2 : FSL_SAI_RCR2) +#define FSL_SAI_xCR3(tx) (tx ? FSL_SAI_TCR3 : FSL_SAI_RCR3) +#define FSL_SAI_xCR4(tx) (tx ? FSL_SAI_TCR4 : FSL_SAI_RCR4) +#define FSL_SAI_xCR5(tx) (tx ? FSL_SAI_TCR5 : FSL_SAI_RCR5) +#define FSL_SAI_xDR(tx) (tx ? FSL_SAI_TDR : FSL_SAI_RDR) +#define FSL_SAI_xFR(tx) (tx ? FSL_SAI_TFR : FSL_SAI_RFR) +#define FSL_SAI_xMR(tx) (tx ? FSL_SAI_TMR : FSL_SAI_RMR) + /* SAI Transmit/Recieve Control Register */ #define FSL_SAI_CSR_TERE BIT(31)
On Tue, Apr 01, 2014 at 11:17:06AM +0800, Nicolin Chen wrote:
The current trigger() has two crucial problems:
- The DMA request enabling operations (FSL_SAI_CSR_FRDE) for Tx and Rx are now totally exclusive: It would fail to run simultaneous Tx-Rx cases.
- The TERE disabling operation depends on an incorrect condition -- active reference count that only gets increased in snd_pcm_open() and decreased in snd_pcm_close(): The TERE would never get cleared.
Applied, thanks.
We only enable one side interrupt for each stream since over/underrun on the opposite stream would be resulted from what we previously did, enabling TERE but remaining FRDE disabled, even though the xrun on the opposite direction will not break the current stream.
Signed-off-by: Nicolin Chen Guangyu.Chen@freescale.com Acked-by: Xiubo Li Li.Xiubo@freescale.com --- sound/soc/fsl/fsl_sai.c | 8 ++++++-- sound/soc/fsl/fsl_sai.h | 1 + 2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c index bdfd497..d64c33f 100644 --- a/sound/soc/fsl/fsl_sai.c +++ b/sound/soc/fsl/fsl_sai.c @@ -397,4 +397,6 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx), + FSL_SAI_CSR_xIE_MASK, FSL_SAI_FLAGS); + regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx), FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE); break; @@ -404,4 +406,6 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd, regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx), FSL_SAI_CSR_FRDE, 0); + regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx), + FSL_SAI_CSR_xIE_MASK, 0);
if (!(tcsr & FSL_SAI_CSR_FRDE || rcsr & FSL_SAI_CSR_FRDE)) { @@ -464,6 +468,6 @@ static int fsl_sai_dai_probe(struct snd_soc_dai *cpu_dai) struct fsl_sai *sai = dev_get_drvdata(cpu_dai->dev);
- regmap_update_bits(sai->regmap, FSL_SAI_TCSR, 0xffffffff, FSL_SAI_FLAGS); - regmap_update_bits(sai->regmap, FSL_SAI_RCSR, 0xffffffff, FSL_SAI_FLAGS); + regmap_update_bits(sai->regmap, FSL_SAI_TCSR, 0xffffffff, 0x0); + regmap_update_bits(sai->regmap, FSL_SAI_RCSR, 0xffffffff, 0x0); regmap_update_bits(sai->regmap, FSL_SAI_TCR1, FSL_SAI_CR1_RFW_MASK, FSL_SAI_MAXBURST_TX * 2); diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h index 64b6fe7..be26d46 100644 --- a/sound/soc/fsl/fsl_sai.h +++ b/sound/soc/fsl/fsl_sai.h @@ -59,4 +59,5 @@ #define FSL_SAI_CSR_FRF BIT(16) #define FSL_SAI_CSR_xIE_SHIFT 8 +#define FSL_SAI_CSR_xIE_MASK (0x1f << FSL_SAI_CSR_xIE_SHIFT) #define FSL_SAI_CSR_WSIE BIT(12) #define FSL_SAI_CSR_SEIE BIT(11)
On Tue, Apr 01, 2014 at 11:17:07AM +0800, Nicolin Chen wrote:
We only enable one side interrupt for each stream since over/underrun on the opposite stream would be resulted from what we previously did, enabling TERE but remaining FRDE disabled, even though the xrun on the opposite direction will not break the current stream.
This still doesn't apply against fsl-sai (nor for-next).
On Tue, Apr 01, 2014 at 01:07:15PM +0100, Mark Brown wrote:
On Tue, Apr 01, 2014 at 11:17:07AM +0800, Nicolin Chen wrote:
We only enable one side interrupt for each stream since over/underrun on the opposite stream would be resulted from what we previously did, enabling TERE but remaining FRDE disabled, even though the xrun on the opposite direction will not break the current stream.
This still doesn't apply against fsl-sai (nor for-next).
Sir, I just rebased my for-next branch again and found that it's missing two applied patches: "ASoC: fsl_sai: Add isr to deal with error flag" and "ASoC: fsl_sai: Improve fsl_sai_isr()", so that's why this PATCH-2 could not be applied against it as it needs the macro that's included in the patch "ASoC: fsl_sai: Add isr to deal with error flag".
What should I do now?
Thank you, Nicolin
On Tue, Apr 01, 2014 at 09:21:57PM +0800, Nicolin Chen wrote:
Sir, I just rebased my for-next branch again and found that it's missing two applied patches: "ASoC: fsl_sai: Add isr to deal with error flag" and "ASoC: fsl_sai: Improve fsl_sai_isr()", so that's why this PATCH-2 could not be applied against it as it needs the macro that's included in the patch "ASoC: fsl_sai: Add isr to deal with error flag".
Ah, those dropped out of -next due to the merge window. I've applied your new patch now.
On Tue, Apr 01, 2014 at 05:42:54PM +0100, Mark Brown wrote:
On Tue, Apr 01, 2014 at 09:21:57PM +0800, Nicolin Chen wrote:
Sir, I just rebased my for-next branch again and found that it's missing two applied patches: "ASoC: fsl_sai: Add isr to deal with error flag" and "ASoC: fsl_sai: Improve fsl_sai_isr()", so that's why this PATCH-2 could not be applied against it as it needs the macro that's included in the patch "ASoC: fsl_sai: Add isr to deal with error flag".
Ah, those dropped out of -next due to the merge window. I've applied your new patch now.
I can find them on the fsl-sai topic now. Thank you.
On Tue, Apr 01, 2014 at 11:17:05AM +0800, Nicolin Chen wrote:
- The patches are generated by using '-U2' because the default '-U3' would conflict the baseline without fsl_sai_isr patches.
What are these "fsi_sai_isr patches"?
On Tue, Apr 01, 2014 at 12:04:12PM +0100, Mark Brown wrote:
On Tue, Apr 01, 2014 at 11:17:05AM +0800, Nicolin Chen wrote:
- The patches are generated by using '-U2' because the default '-U3' would conflict the baseline without fsl_sai_isr patches.
What are these "fsi_sai_isr patches"?
Ah..I should have mentioned the full patch name: e2681a1 ASoC: fsl_sai: Add isr to deal with error flag
The patch conflicts against that asoc-3.15-4 tag at the fsl_sai.h because this isr patch added some new macros to it.
Thank you, Nicolin Chen
participants (2)
-
Mark Brown -
Nicolin Chen