[PATCH v2 0/2] ASoC: fsl_sai: Fill Tx FIFO to avoid initial underruns

[PATCH v2 0/2] ASoC: fsl_sai: Fill Tx FIFO to avoid initial underruns

[Sascha Hauer]

This series fixes initial underruns that can occur in the TX queue of
the fsl_sai interface when starting playback. These patches are around
here for quite some time and have proven useful. Time to upstream them.

Sascha

Changes since v1:
- Add missing Signed-off-by:

Ahmad Fatoum (2):
  ASoC: fsl_sai: refactor TDM slots calculation into helper function
  ASoC: fsl_sai: Fill Tx FIFO to avoid initial underruns

 sound/soc/fsl/fsl_sai.c | 38 +++++++++++++++++++++++++++++++++-----
 sound/soc/fsl/fsl_sai.h |  1 +
 2 files changed, 34 insertions(+), 5 deletions(-)

-- 
2.39.2

[PATCH v2 1/2] ASoC: fsl_sai: refactor TDM slots calculation into helper function

[Sascha Hauer]

From: Ahmad Fatoum <a.fatoum@pengutronix.de>

Splitting the calculation between the initializer and later on makes it
harder to follow. A follow-up commit will also need to do this calculation,
so move it into a helper function. No functional change.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---

Notes:
    Changes since v1:
    - Add missing Signed-off-by

 sound/soc/fsl/fsl_sai.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index e3105d48fb651..36f6115469843 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -516,6 +516,19 @@ static int fsl_sai_set_bclk(struct snd_soc_dai *dai, bool tx, u32 freq)
 	return 0;
 }
 
+static unsigned int fsl_sai_get_tdm_slots(struct fsl_sai *sai,
+					  unsigned int channels,
+					  unsigned int slot_width)
+{
+	if (sai->slots)
+		return sai->slots;
+
+	if (sai->bclk_ratio)
+		return sai->bclk_ratio / slot_width;
+
+	return channels == 1 ? 2 : channels;
+}
+
 static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
 		struct snd_pcm_hw_params *params,
 		struct snd_soc_dai *cpu_dai)
@@ -531,7 +544,7 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
 	int dl_cfg_cnt = sai->dl_cfg_cnt;
 	u32 dl_type = FSL_SAI_DL_I2S;
 	u32 val_cr4 = 0, val_cr5 = 0;
-	u32 slots = (channels == 1) ? 2 : channels;
+	u32 slots;
 	u32 slot_width = word_width;
 	int adir = tx ? RX : TX;
 	u32 pins, bclk;
@@ -541,10 +554,7 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
 	if (sai->slot_width)
 		slot_width = sai->slot_width;
 
-	if (sai->slots)
-		slots = sai->slots;
-	else if (sai->bclk_ratio)
-		slots = sai->bclk_ratio / slot_width;
+	slots = fsl_sai_get_tdm_slots(sai, channels, slot_width);
 
 	pins = DIV_ROUND_UP(channels, slots);
 
-- 
2.39.2

[PATCH v2 2/2] ASoC: fsl_sai: Fill Tx FIFO to avoid initial underruns

[Sascha Hauer]

From: Ahmad Fatoum <a.fatoum@pengutronix.de>

JACK handles XRuns by stopping and start the ALSA device. On occasion,
this leads to early underruns on start leading to reorderd output
channels.

By filling the FIFO initially, we can avoid these early underruns.
This is also suggested by the i.MX8MM reference manual:

  "If the Transmit FIFO is empty, then to avoid a FIFO underrun, the
  Transmit Data Register must be written at least 3 bit clocks before
  the start of the next unmasked word. Before enabling the transmitter,
  the Transmit FIFO should be initialized with data (since after the
  transmitter is enabled, the transmitter will start a new frame, and
  if no data is in the FIFO, then the transmitter will immediately give
  an error)"

[1]: Rev. 0, 02/2019, 13.9.3.5.2 FIFO pointers
Fixes: 435508214942 ("ASoC: Add SAI SoC Digital Audio Interface driver.")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---

Notes:
    Changes since v1:
    - Add missing Signed-off-by

 sound/soc/fsl/fsl_sai.c | 18 ++++++++++++++++++
 sound/soc/fsl/fsl_sai.h |  1 +
 2 files changed, 19 insertions(+)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 36f6115469843..6a4f990110d91 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -755,6 +755,21 @@ static void fsl_sai_config_disable(struct fsl_sai *sai, int dir)
 	}
 }
 
+static void fsl_sai_tx_fill_fifo(struct fsl_sai *sai,
+				 struct snd_pcm_runtime *runtime)
+{
+	u32 slots, slot_width, pins;
+	int i;
+
+	slot_width = sai->slot_width ?: snd_pcm_format_physical_width(runtime->format);
+
+	slots = fsl_sai_get_tdm_slots(sai, runtime->channels, slot_width);
+	pins = DIV_ROUND_UP(runtime->channels, slots);
+
+	for (i = 0; i < runtime->channels; i++)
+		regmap_write(sai->regmap, FSL_SAI_TDR(i % pins), 0x0);
+}
+
 static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
 		struct snd_soc_dai *cpu_dai)
 {
@@ -784,6 +799,9 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		/* Fill FIFO to avoid initial underruns */
+		if (tx)
+			fsl_sai_tx_fill_fifo(sai, substream->runtime);
 		regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
 				   FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
 
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index a53c4f0e25faf..66a136d97a441 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -34,6 +34,7 @@
 #define FSL_SAI_TDR5	0x34 /* SAI Transmit Data 5 */
 #define FSL_SAI_TDR6	0x38 /* SAI Transmit Data 6 */
 #define FSL_SAI_TDR7	0x3C /* SAI Transmit Data 7 */
+#define FSL_SAI_TDR(ofs)	(FSL_SAI_TDR0 + (ofs) * 4)
 #define FSL_SAI_TFR0	0x40 /* SAI Transmit FIFO 0 */
 #define FSL_SAI_TFR1	0x44 /* SAI Transmit FIFO 1 */
 #define FSL_SAI_TFR2	0x48 /* SAI Transmit FIFO 2 */
-- 
2.39.2

Re: [PATCH v2 2/2] ASoC: fsl_sai: Fill Tx FIFO to avoid initial underruns

[Shengjiu Wang]

On Thu, Jun 29, 2023 at 10:10 PM Sascha Hauer <s.hauer@pengutronix.de>
wrote:

> From: Ahmad Fatoum <a.fatoum@pengutronix.de>
>
> JACK handles XRuns by stopping and start the ALSA device. On occasion,
> this leads to early underruns on start leading to reorderd output
> channels.
>
> By filling the FIFO initially, we can avoid these early underruns.
> This is also suggested by the i.MX8MM reference manual:
>
>   "If the Transmit FIFO is empty, then to avoid a FIFO underrun, the
>   Transmit Data Register must be written at least 3 bit clocks before
>   the start of the next unmasked word. Before enabling the transmitter,
>   the Transmit FIFO should be initialized with data (since after the
>   transmitter is enabled, the transmitter will start a new frame, and
>   if no data is in the FIFO, then the transmitter will immediately give
>   an error)"
>
> [1]: Rev. 0, 02/2019, 13.9.3.5.2 FIFO pointers
> Fixes: 435508214942 ("ASoC: Add SAI SoC Digital Audio Interface driver.")
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>
> Notes:
>     Changes since v1:
>     - Add missing Signed-off-by
>
>  sound/soc/fsl/fsl_sai.c | 18 ++++++++++++++++++
>  sound/soc/fsl/fsl_sai.h |  1 +
>  2 files changed, 19 insertions(+)
>
> diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> index 36f6115469843..6a4f990110d91 100644
> --- a/sound/soc/fsl/fsl_sai.c
> +++ b/sound/soc/fsl/fsl_sai.c
> @@ -755,6 +755,21 @@ static void fsl_sai_config_disable(struct fsl_sai
> *sai, int dir)
>         }
>  }
>
> +static void fsl_sai_tx_fill_fifo(struct fsl_sai *sai,
> +                                struct snd_pcm_runtime *runtime)
> +{
> +       u32 slots, slot_width, pins;
> +       int i;
> +
> +       slot_width = sai->slot_width ?:
> snd_pcm_format_physical_width(runtime->format);
> +
> +       slots = fsl_sai_get_tdm_slots(sai, runtime->channels, slot_width);
> +       pins = DIV_ROUND_UP(runtime->channels, slots);
> +
> +       for (i = 0; i < runtime->channels; i++)
> +               regmap_write(sai->regmap, FSL_SAI_TDR(i % pins), 0x0);
>

There may be an issue here for the case that selected pins are not
continuous. for example: select pin 0, 2, 4, 6

best regards
wang shengjiu

> +}
> +
>  static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
>                 struct snd_soc_dai *cpu_dai)
>  {
> @@ -784,6 +799,9 @@ static int fsl_sai_trigger(struct snd_pcm_substream
> *substream, int cmd,
>         case SNDRV_PCM_TRIGGER_START:
>         case SNDRV_PCM_TRIGGER_RESUME:
>         case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> +               /* Fill FIFO to avoid initial underruns */
> +               if (tx)
> +                       fsl_sai_tx_fill_fifo(sai, substream->runtime);
>                 regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
>                                    FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
>
> diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
> index a53c4f0e25faf..66a136d97a441 100644
> --- a/sound/soc/fsl/fsl_sai.h
> +++ b/sound/soc/fsl/fsl_sai.h
> @@ -34,6 +34,7 @@
>  #define FSL_SAI_TDR5   0x34 /* SAI Transmit Data 5 */
>  #define FSL_SAI_TDR6   0x38 /* SAI Transmit Data 6 */
>  #define FSL_SAI_TDR7   0x3C /* SAI Transmit Data 7 */
> +#define FSL_SAI_TDR(ofs)       (FSL_SAI_TDR0 + (ofs) * 4)
>  #define FSL_SAI_TFR0   0x40 /* SAI Transmit FIFO 0 */
>  #define FSL_SAI_TFR1   0x44 /* SAI Transmit FIFO 1 */
>  #define FSL_SAI_TFR2   0x48 /* SAI Transmit FIFO 2 */
> --
> 2.39.2
>
>