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

[PATCH] 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.

Signed-off-by: Christian Hewitt <christianshewitt@gmail.com>
---
 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..826055a71421 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,12 +69,16 @@ 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:
 		fifo_spdif_dcu_enable(component, false);
+		snd_soc_component_write(component, AIU_RST_SOFT,
+					AIU_RST_SOFT_958_FAST);
 		break;
 	default:
 		return -EINVAL;
-- 
2.43.0


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

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

[Christian Hewitt]

> On 26 Jun 2026, at 12:04 pm, 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”)

^ I can send a v2 with it done properly if needed?

> Signed-off-by: Christian Hewitt <christianshewitt@gmail.com>
> ---
> 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..826055a71421 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,12 +69,16 @@ 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:
> fifo_spdif_dcu_enable(component, false);
> + snd_soc_component_write(component, AIU_RST_SOFT,
> + AIU_RST_SOFT_958_FAST);
> break;
> default:
> return -EINVAL;
> -- 
> 2.43.0

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

Re: [PATCH] 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 3 potential issue(s) to consider:

New issues:
- [High] The S/PDIF fast domain soft reset is issued after the DMA FIFO is enabled on the START path, causing potential data loss and DMA desynchronization.
- [High] Calling `snd_soc_component_write()` in the ALSA PCM trigger callback causes a sleep-in-atomic lockdep BUG.

Pre-existing issues:
- [High] Preexisting sleeping in atomic context bug in the I2S FIFO trigger callback due to `snd_soc_component_write()` and `snd_soc_component_read()`.
--

commit 60846173cae07d2808fb45b843fa96a97f4f8eb6
Author: Christian Hewitt <christianshewitt@gmail.com>

ASoC: meson: aiu: fifo-spdif: soft reset the S/PDIF datapath on start/stop
    
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.

> diff --git a/sound/soc/meson/aiu-fifo-spdif.c b/sound/soc/meson/aiu-fifo-spdif.c
> index e0e00ec026dcc..826055a71421d 100644
> --- a/sound/soc/meson/aiu-fifo-spdif.c
> +++ b/sound/soc/meson/aiu-fifo-spdif.c

[ ... ]

> @@ -68,12 +69,16 @@ 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 introduce a race condition on start? In fifo_spdif_trigger(),
aiu_fifo_trigger() is called before the switch statement, which starts the
upstream DMA engine before we issue the downstream S/PDIF fast domain soft
reset.

Could this discard audio frames fetched by the DMA into the FIFO before the
reset, causing left/right channel swapping and audio artifacts?

Additionally, the ALSA PCM trigger callback executes in atomic context with
the stream spinlock held. Is it safe to call snd_soc_component_write() here?

sound/soc/soc-component.c:snd_soc_component_write() {
	int ret;

	mutex_lock(&component->io_mutex);
	ret = soc_component_write_no_lock(component, reg, val);
	mutex_unlock(&component->io_mutex);

	return ret;
}

Acquiring a sleeping mutex inside an atomic section can cause scheduling while
atomic bugs and potential system hangs.

This is a pre-existing issue, but the same sleeping in atomic context pattern
is present in the I2S FIFO trigger callback:

sound/soc/meson/aiu-fifo-i2s.c:aiu_fifo_i2s_trigger() {
	...
	snd_soc_component_write(component, AIU_RST_SOFT,
				AIU_RST_SOFT_I2S_FAST);
	snd_soc_component_read(component, AIU_I2S_SYNC);
	...
}

Both snd_soc_component_write() and snd_soc_component_read() take the sleeping
io_mutex inside the atomic I2S trigger callback.

>  		fifo_spdif_dcu_enable(component, true);
>  		break;
>  	case SNDRV_PCM_TRIGGER_SUSPEND:
>  	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
>  	case SNDRV_PCM_TRIGGER_STOP:
>  		fifo_spdif_dcu_enable(component, false);
> +		snd_soc_component_write(component, AIU_RST_SOFT,
> +					AIU_RST_SOFT_958_FAST);
[Severity: High]
Can this call to snd_soc_component_write() also cause a scheduling while
atomic regression? It will acquire the sleeping io_mutex while in the atomic
STOP/SUSPEND trigger context.

>  		break;
>  	default:
>  		return -EINVAL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626080422.4191435-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] ASoC: meson: aiu: fifo-spdif: soft reset the S/PDIF datapath on start/stop

[Martin Blumenstingl]

[ ... ]
>
> > @@ -68,12 +69,16 @@ 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 introduce a race condition on start? In fifo_spdif_trigger(),
> aiu_fifo_trigger() is called before the switch statement, which starts the
> upstream DMA engine before we issue the downstream S/PDIF fast domain soft
> reset.
>
> Could this discard audio frames fetched by the DMA into the FIFO before the
> reset, causing left/right channel swapping and audio artifacts?
I don't think so. The sequence introduced by Christian matches the
vendor driver (see my other email with review notes).

> Additionally, the ALSA PCM trigger callback executes in atomic context with
> the stream spinlock held. Is it safe to call snd_soc_component_write() here?
My understanding is that ALSA uses mutex instead of spinlocks, meaning
this comment is incorrect.

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

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

[Martin Blumenstingl]

On Fri, Jun 26, 2026 at 10:04 AM 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.
>
> Signed-off-by: Christian Hewitt <christianshewitt@gmail.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

This matches the vendor driver, references:
- fast-reset SPDIF is triggered on enable and disable: [0]
- fast-reset SPDIF is triggered after all of the configuration is
written, then DCU_FF_CTRL is enabled: [1]

[...]
>         case SNDRV_PCM_TRIGGER_SUSPEND:
>         case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
>         case SNDRV_PCM_TRIGGER_STOP:
>                 fifo_spdif_dcu_enable(component, false);
> +               snd_soc_component_write(component, AIU_RST_SOFT,
> +                                       AIU_RST_SOFT_958_FAST);
It doesn't seem to make any difference, so just for the record:
The vendor driver first triggers AIU_RST_SOFT_958_FAST then disables DCU: [2]


Best regards,
Martin


[0] https://github.com/endlessm/linux-s905x/blob/c1391ba1116aed801aaabfd9c7bbb76fda9801ec/sound/soc/aml/m8/aml_audio_hw.c#L1016
[1] https://github.com/endlessm/linux-s905x/blob/c1391ba1116aed801aaabfd9c7bbb76fda9801ec/sound/soc/aml/m8/aml_audio_hw.c#L979-L983
[2] https://github.com/endlessm/linux-s905x/blob/c1391ba1116aed801aaabfd9c7bbb76fda9801ec/sound/soc/aml/m8/aml_audio_hw.c#L1016-L1018

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

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

[Jerome Brunet]

On ven. 26 juin 2026 at 22:41, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> On Fri, Jun 26, 2026 at 10:04 AM 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.
>>
>> Signed-off-by: Christian Hewitt <christianshewitt@gmail.com>
> Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>
> This matches the vendor driver, references:
> - fast-reset SPDIF is triggered on enable and disable: [0]
> - fast-reset SPDIF is triggered after all of the configuration is
> written, then DCU_FF_CTRL is enabled: [1]

Take what the vendor driver does with a grain of salt, especially when
it comes to audio

>
> [...]
>>         case SNDRV_PCM_TRIGGER_SUSPEND:
>>         case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
>>         case SNDRV_PCM_TRIGGER_STOP:
>>                 fifo_spdif_dcu_enable(component, false);
>> +               snd_soc_component_write(component, AIU_RST_SOFT,
>> +                                       AIU_RST_SOFT_958_FAST);
> It doesn't seem to make any difference, so just for the record:
> The vendor driver first triggers AIU_RST_SOFT_958_FAST then disables DCU: [2]
>

One could even wonder if there is any point in applying a reset when
everything is stopped, if the enable path will apply this same
reset before anything else is started again ? Does it really changes
anything to the reported issue ?

>
> Best regards,
> Martin
>
>
> [0] https://github.com/endlessm/linux-s905x/blob/c1391ba1116aed801aaabfd9c7bbb76fda9801ec/sound/soc/aml/m8/aml_audio_hw.c#L1016
> [1] https://github.com/endlessm/linux-s905x/blob/c1391ba1116aed801aaabfd9c7bbb76fda9801ec/sound/soc/aml/m8/aml_audio_hw.c#L979-L983
> [2] https://github.com/endlessm/linux-s905x/blob/c1391ba1116aed801aaabfd9c7bbb76fda9801ec/sound/soc/aml/m8/aml_audio_hw.c#L1016-L1018

-- 
Jerome

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

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

[Christian Hewitt]

> On 29 Jun 2026, at 4:04 pm, Jerome Brunet <jbrunet@baylibre.com> wrote:
> 
> On ven. 26 juin 2026 at 22:41, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:
> 
>> On Fri, Jun 26, 2026 at 10:04 AM 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.
>>> 
>>> Signed-off-by: Christian Hewitt <christianshewitt@gmail.com>
>> Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> 
>> This matches the vendor driver, references:
>> - fast-reset SPDIF is triggered on enable and disable: [0]
>> - fast-reset SPDIF is triggered after all of the configuration is
>> written, then DCU_FF_CTRL is enabled: [1]
> 
> Take what the vendor driver does with a grain of salt, especially when
> it comes to audio
> 
>> 
>> [...]
>>>        case SNDRV_PCM_TRIGGER_SUSPEND:
>>>        case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
>>>        case SNDRV_PCM_TRIGGER_STOP:
>>>                fifo_spdif_dcu_enable(component, false);
>>> +               snd_soc_component_write(component, AIU_RST_SOFT,
>>> +                                       AIU_RST_SOFT_958_FAST);
>> It doesn't seem to make any difference, so just for the record:
>> The vendor driver first triggers AIU_RST_SOFT_958_FAST then disables DCU: [2]
>> 
> 
> One could even wonder if there is any point in applying a reset when
> everything is stopped, if the enable path will apply this same
> reset before anything else is started again ? Does it really changes
> anything to the reported issue ?

Using disable/reset or reset/disable didn’t make any difference in my
testing, but without any reset playback stop on the WeTek Play2 board
with hardwired DAC always triggers the buffer underrun.

CH.


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