[PATCH v2] ASoC: meson: aiu: fifo-spdif: soft reset the S/PDIF datapath on start/stop

[PATCH v2] ASoC: meson: aiu: fifo-spdif: soft reset the S/PDIF datapath on start/stop

[Christian Hewitt]

The I2S FIFO soft-resets its fast domain on start (AIU_RST_SOFT bit 0 +
AIU_I2S_SYNC read in aiu_fifo_i2s_trigger), mirroring the downstream
vendor driver's audio_out_i2s_enable(). The S/PDIF FIFO has no equivalent:
it only toggles the IEC958 DCU, so a stale datapath FIFO can be replayed,
producing the "machine gun noise" buffer underrun - on start when switching
outputs, and on stop when playback ends. The latter is audible on devices
with an always-on S/PDIF-fed DAC (e.g. the ES7144 on the WeTek Play2).

The vendor driver resets the IEC958 fast domain (AIU_RST_SOFT bit 2) on
both enable and disable (audio_hw_958_enable), and when reconfiguring
(audio_hw_958_reset clears AIU_958_DCU_FF_CTRL then resets). Do the same:
reset before enabling the DCU on start, and after disabling it on stop.

Fixes: 6ae9ca9ce986bf ("ASoC: meson: aiu: add i2s and spdif support")
Signed-off-by: Christian Hewitt <christianshewitt@gmail.com>
---
Changes from v1:
- Add missing fixes tag
- Trigger AIU_RST_SOFT_958_FAST then disable DCU as in the vendor driver

 sound/soc/meson/aiu-fifo-spdif.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/soc/meson/aiu-fifo-spdif.c b/sound/soc/meson/aiu-fifo-spdif.c
index e0e00ec026dc..a9861c5d6637 100644
--- a/sound/soc/meson/aiu-fifo-spdif.c
+++ b/sound/soc/meson/aiu-fifo-spdif.c
@@ -24,6 +24,7 @@
 #define AIU_MEM_IEC958_CONTROL_MODE_16BIT	BIT(7)
 #define AIU_MEM_IEC958_CONTROL_MODE_LINEAR	BIT(8)
 #define AIU_MEM_IEC958_BUF_CNTL_INIT		BIT(0)
+#define AIU_RST_SOFT_958_FAST			BIT(2)
 
 #define AIU_FIFO_SPDIF_BLOCK			8
 
@@ -68,11 +69,15 @@ static int fifo_spdif_trigger(struct snd_pcm_substream *substream, int cmd,
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		snd_soc_component_write(component, AIU_RST_SOFT,
+					AIU_RST_SOFT_958_FAST);
 		fifo_spdif_dcu_enable(component, true);
 		break;
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
 	case SNDRV_PCM_TRIGGER_STOP:
+		snd_soc_component_write(component, AIU_RST_SOFT,
+					AIU_RST_SOFT_958_FAST);
 		fifo_spdif_dcu_enable(component, false);
 		break;
 	default:
-- 
2.43.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v2] ASoC: meson: aiu: fifo-spdif: soft reset the S/PDIF datapath on start/stop

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Calling snd_soc_component_write() inside the ALSA PCM trigger callback causes a 'scheduling while atomic' BUG.
- [High] The fast domain soft reset logic uses an incorrect execution sequence that contradicts the commit message, risking audio glitches.
--

commit 75a2cc70a354fd1f43acb5fb951912b82d12eca6
Author: Christian Hewitt <christianshewitt@gmail.com>
Date:   Sat Jun 27 12:50:30 2026 +0000

ASoC: meson: aiu: fifo-spdif: soft reset the S/PDIF datapath on start/stop

This commit introduces a soft reset of the S/PDIF fast domain on start and
stop to prevent a buffer underrun "machine gun noise" issue. It adds calls
to reset the domain before enabling the DCU on start, and after disabling
it on stop.

> The vendor driver resets the IEC958 fast domain (AIU_RST_SOFT bit 2) on
> both enable and disable (audio_hw_958_enable), and when reconfiguring
> (audio_hw_958_reset clears AIU_958_DCU_FF_CTRL then resets). Do the same:
> reset before enabling the DCU on start, and after disabling it on stop.

[Severity: High]
Is the sequence of operations implemented in the stop path inverted compared
to this commit message?

The commit message states the reset should happen after disabling the DCU
on stop, but the code executes the reset before disabling the DCU data flow.

> diff --git a/sound/soc/meson/aiu-fifo-spdif.c b/sound/soc/meson/aiu-fifo-spdif.c
> index e0e00ec026dcc..a9861c5d66374 100644
> --- a/sound/soc/meson/aiu-fifo-spdif.c
> +++ b/sound/soc/meson/aiu-fifo-spdif.c
[ ... ]
> @@ -68,11 +69,15 @@ static int fifo_spdif_trigger(struct snd_pcm_substream *substream, int cmd,
>  	case SNDRV_PCM_TRIGGER_START:
>  	case SNDRV_PCM_TRIGGER_RESUME:
>  	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> +		snd_soc_component_write(component, AIU_RST_SOFT,
> +					AIU_RST_SOFT_958_FAST);

[Severity: High]
Does this cause a 'scheduling while atomic' bug?

The ALSA PCM trigger callback runs in an atomic context with spinlocks held
and interrupts disabled, but snd_soc_component_write() unconditionally acquires
a sleeping lock:

snd_soc_component_write() {
    ...
    mutex_lock(&component->io_mutex);
    ret = soc_component_write_no_lock(component, reg, val);
    ...
}

Also, does the soft reset on the start path happen too late?

Earlier in fifo_spdif_trigger(), aiu_fifo_trigger() is called to start the
main upstream FIFO before this soft reset is executed. Will starting the FIFO
before the reset result in dropped initial audio samples?

>  		fifo_spdif_dcu_enable(component, true);
>  		break;
>  	case SNDRV_PCM_TRIGGER_SUSPEND:
>  	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
>  	case SNDRV_PCM_TRIGGER_STOP:
> +		snd_soc_component_write(component, AIU_RST_SOFT,
> +					AIU_RST_SOFT_958_FAST);
>  		fifo_spdif_dcu_enable(component, false);

[Severity: High]
Will resetting the fast domain while the DCU is still actively fetching data
force the DCU to read from an empty FIFO and trigger an underrun?

As noted in the commit message review above, the soft reset is executed here
before fifo_spdif_dcu_enable(component, false) disables the DCU data flow.

>  		break;
>  	default:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260627125030.808058-1-christianshewitt@gmail.com?part=1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v2] ASoC: meson: aiu: fifo-spdif: soft reset the S/PDIF datapath on start/stop

[Ferass El Hafidi]

Hi Christian,

On Sat, 27 Jun 2026 12:50, Christian Hewitt <christianshewitt@gmail.com> wrote:
>The I2S FIFO soft-resets its fast domain on start (AIU_RST_SOFT bit 0 +
>AIU_I2S_SYNC read in aiu_fifo_i2s_trigger), mirroring the downstream
>vendor driver's audio_out_i2s_enable(). The S/PDIF FIFO has no equivalent:
>it only toggles the IEC958 DCU, so a stale datapath FIFO can be replayed,
>producing the "machine gun noise" buffer underrun - on start when switching
>outputs, and on stop when playback ends. The latter is audible on devices
>with an always-on S/PDIF-fed DAC (e.g. the ES7144 on the WeTek Play2).
>
>The vendor driver resets the IEC958 fast domain (AIU_RST_SOFT bit 2) on
>both enable and disable (audio_hw_958_enable), and when reconfiguring
>(audio_hw_958_reset clears AIU_958_DCU_FF_CTRL then resets). Do the same:
>reset before enabling the DCU on start, and after disabling it on stop.
>
>Fixes: 6ae9ca9ce986bf ("ASoC: meson: aiu: add i2s and spdif support")
>Signed-off-by: Christian Hewitt <christianshewitt@gmail.com>

You may have forgotten to add Martin's R-b here.

Best regards,
Ferass

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v2] ASoC: meson: aiu: fifo-spdif: soft reset the S/PDIF datapath on start/stop

[Christian Hewitt]

> On 27 Jun 2026, at 5:08 pm, Ferass El Hafidi <funderscore@postmarketos.org> wrote:
> 
> Hi Christian,
> 
> On Sat, 27 Jun 2026 12:50, Christian Hewitt <christianshewitt@gmail.com> wrote:
>> The I2S FIFO soft-resets its fast domain on start (AIU_RST_SOFT bit 0 +
>> AIU_I2S_SYNC read in aiu_fifo_i2s_trigger), mirroring the downstream
>> vendor driver's audio_out_i2s_enable(). The S/PDIF FIFO has no equivalent:
>> it only toggles the IEC958 DCU, so a stale datapath FIFO can be replayed,
>> producing the "machine gun noise" buffer underrun - on start when switching
>> outputs, and on stop when playback ends. The latter is audible on devices
>> with an always-on S/PDIF-fed DAC (e.g. the ES7144 on the WeTek Play2).
>> 
>> The vendor driver resets the IEC958 fast domain (AIU_RST_SOFT bit 2) on
>> both enable and disable (audio_hw_958_enable), and when reconfiguring
>> (audio_hw_958_reset clears AIU_958_DCU_FF_CTRL then resets). Do the same:
>> reset before enabling the DCU on start, and after disabling it on stop.
>> 
>> Fixes: 6ae9ca9ce986bf ("ASoC: meson: aiu: add i2s and spdif support")
>> Signed-off-by: Christian Hewitt <christianshewitt@gmail.com>
> 
> You may have forgotten to add Martin's R-b here.

100%, but I also missed amending the commit description for the v2 change so
there’s a v3 where it’s been added. Sorry for the noise everyone, seems I am
a little out of practice at submitting things.

CH.
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic