Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Jerome Brunet <jbrunet@baylibre.com>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: Christian Hewitt <christianshewitt@gmail.com>,
	 Liam Girdwood <lgirdwood@gmail.com>,
	 Mark Brown <broonie@kernel.org>,
	 Jaroslav Kysela <perex@perex.cz>,  Takashi Iwai <tiwai@suse.com>,
	 Neil Armstrong <neil.armstrong@linaro.org>,
	 Kevin Hilman <khilman@baylibre.com>,
	linux-sound@vger.kernel.org,
	 linux-arm-kernel@lists.infradead.org,
	linux-amlogic@lists.infradead.org,  linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ASoC: meson: aiu: fifo-spdif: soft reset the S/PDIF datapath on start/stop
Date: Mon, 29 Jun 2026 14:04:44 +0200	[thread overview]
Message-ID: <1jwlvha6xv.fsf@starbuckisacylon.baylibre.com> (raw)
In-Reply-To: <CAFBinCD5bn5VixbomCz0nmHuopBmgh0CJc-V4jY2vdXhoP_VCw@mail.gmail.com> (Martin Blumenstingl's message of "Fri, 26 Jun 2026 22:41:24 +0200")

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


  reply	other threads:[~2026-06-29 12:04 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-26  8:04 [PATCH] ASoC: meson: aiu: fifo-spdif: soft reset the S/PDIF datapath on start/stop Christian Hewitt
2026-06-26  8:09 ` Christian Hewitt
2026-06-26 20:41 ` Martin Blumenstingl
2026-06-29 12:04   ` Jerome Brunet [this message]
2026-06-29 14:47     ` Christian Hewitt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1jwlvha6xv.fsf@starbuckisacylon.baylibre.com \
    --to=jbrunet@baylibre.com \
    --cc=broonie@kernel.org \
    --cc=christianshewitt@gmail.com \
    --cc=khilman@baylibre.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=martin.blumenstingl@googlemail.com \
    --cc=neil.armstrong@linaro.org \
    --cc=perex@perex.cz \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox