mbox series

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

Message ID 20230629141034.2659669-1-s.hauer@pengutronix.de
Headers show
Series ASoC: fsl_sai: Fill Tx FIFO to avoid initial underruns | expand

Message

Sascha Hauer June 29, 2023, 2:10 p.m. UTC 
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(-)

Comments

Shengjiu Wang June 30, 2023, 3:14 a.m. UTC | #1 
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
>
>