With the AXG audio subsystem, there is a possible random channel shift on TDM capture, when the slot number per lane is more than 2, and there is more than one lane used.

The problem has been there since the introduction of the axg audio support but such scenario is pretty uncommon. This is why there is no loud complains about the problem.

Solving the problem require to make the links non-atomic and use the trigger() callback to start FEs and BEs in the appropriate order.

This was tried in the past and reverted because it caused the block irq to sleep while atomic. However, instead of reverting, the solution is to call snd_pcm_period_elapsed() in a non atomic context.

Use the bottom half of a threaded IRQ to do so.

Fixes: 6dc4fa179fb8 ("ASoC: meson: add axg fifo base driver") Signed-off-by: Jerome Brunet jbrunet@baylibre.com --- sound/soc/meson/axg-fifo.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/sound/soc/meson/axg-fifo.c b/sound/soc/meson/axg-fifo.c index bebee0ca8e38..ecb3eb7a9723 100644 --- a/sound/soc/meson/axg-fifo.c +++ b/sound/soc/meson/axg-fifo.c @@ -204,18 +204,26 @@ static irqreturn_t axg_fifo_pcm_irq_block(int irq, void *dev_id) unsigned int status;

regmap_read(fifo->map, FIFO_STATUS1, &status); - status = FIELD_GET(STATUS1_INT_STS, status); + axg_fifo_ack_irq(fifo, status); + + /* Use the thread to call period elapsed on nonatomic links */ if (status & FIFO_INT_COUNT_REPEAT) - snd_pcm_period_elapsed(ss); - else - dev_dbg(axg_fifo_dev(ss), "unexpected irq - STS 0x%02x\n", - status); + return IRQ_WAKE_THREAD;

- /* Ack irqs */ - axg_fifo_ack_irq(fifo, status); + dev_dbg(axg_fifo_dev(ss), "unexpected irq - STS 0x%02x\n", + status); + + return IRQ_NONE; +} + +static irqreturn_t axg_fifo_pcm_irq_block_thread(int irq, void *dev_id) +{ + struct snd_pcm_substream *ss = dev_id; + + snd_pcm_period_elapsed(ss);

- return IRQ_RETVAL(status); + return IRQ_HANDLED; }

int axg_fifo_pcm_open(struct snd_soc_component *component, @@ -243,8 +251,9 @@ int axg_fifo_pcm_open(struct snd_soc_component *component, if (ret) return ret;

- ret = request_irq(fifo->irq, axg_fifo_pcm_irq_block, 0, - dev_name(dev), ss); + ret = request_threaded_irq(fifo->irq, axg_fifo_pcm_irq_block, + axg_fifo_pcm_irq_block_thread, + IRQF_ONESHOT, dev_name(dev), ss); if (ret) return ret;

So far, the formatters have been reset/enabled using the .prepare() callback. This was done in this callback because walking the formatters use a mutex. A mutex is used because formatter handling require dealing possibly slow clock operation.

With the support of non-atomic, .trigger() callback may be used which also allows to properly enable and disable formatters on start but also pause/resume.

This solve a random shift on TDMIN as well repeated samples on for TDMOUT.

Fixes: d60e4f1e4be5 ("ASoC: meson: add tdm interface driver") Signed-off-by: Jerome Brunet jbrunet@baylibre.com --- sound/soc/meson/axg-tdm-interface.c | 34 ++++++++++++++++------------- 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/sound/soc/meson/axg-tdm-interface.c b/sound/soc/meson/axg-tdm-interface.c index bf708717635b..8bf3735dedaa 100644 --- a/sound/soc/meson/axg-tdm-interface.c +++ b/sound/soc/meson/axg-tdm-interface.c @@ -349,26 +349,31 @@ static int axg_tdm_iface_hw_params(struct snd_pcm_substream *substream, return 0; }

-static int axg_tdm_iface_hw_free(struct snd_pcm_substream *substream, +static int axg_tdm_iface_trigger(struct snd_pcm_substream *substream, + int cmd, struct snd_soc_dai *dai) { - struct axg_tdm_stream *ts = snd_soc_dai_get_dma_data(dai, substream); + struct axg_tdm_stream *ts = + snd_soc_dai_get_dma_data(dai, substream);

- /* Stop all attached formatters */ - axg_tdm_stream_stop(ts); + switch (cmd) { + case SNDRV_PCM_TRIGGER_START: + case SNDRV_PCM_TRIGGER_RESUME: + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: + axg_tdm_stream_start(ts); + break; + case SNDRV_PCM_TRIGGER_SUSPEND: + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: + case SNDRV_PCM_TRIGGER_STOP: + axg_tdm_stream_stop(ts); + break; + default: + return -EINVAL; + }

return 0; }

-static int axg_tdm_iface_prepare(struct snd_pcm_substream *substream, - struct snd_soc_dai *dai) -{ - struct axg_tdm_stream *ts = snd_soc_dai_get_dma_data(dai, substream); - - /* Force all attached formatters to update */ - return axg_tdm_stream_reset(ts); -} - static int axg_tdm_iface_remove_dai(struct snd_soc_dai *dai) { int stream; @@ -412,8 +417,7 @@ static const struct snd_soc_dai_ops axg_tdm_iface_ops = { .set_fmt = axg_tdm_iface_set_fmt, .startup = axg_tdm_iface_startup, .hw_params = axg_tdm_iface_hw_params, - .prepare = axg_tdm_iface_prepare, - .hw_free = axg_tdm_iface_hw_free, + .trigger = axg_tdm_iface_trigger, };

/* TDM Backend DAIs */