Click/Pop Noise was a long pending issue with WSA Codecs which are prone
to accumlate DC when ports are active but without any data streams.
There are multiple places in the current setup, where this could happen
in both startup as well as shutdown path.

This patchset adds a new flag mute_unmute_on_trigger to dai_ops to let
generic code do the mute/unmute on trigger.

This patches help fix those issues by making sure the PA is Muted/Unmuted
inline with the stream start/stop events.


Srinivas Kandagatla (2):
  ASoC: soc-dai: add flag to mute and unmute stream during trigger
  ASoC: codecs: wsa883x: make use of new mute_unmute_on_trigger flag

 include/sound/soc-dai.h    |  1 +
 sound/soc/codecs/wsa883x.c |  7 +------
 sound/soc/soc-dai.c        |  7 +++++++
 sound/soc/soc-pcm.c        | 12 ++++++++----
 4 files changed, 17 insertions(+), 10 deletions(-)

-- 
2.21.0

On Fri, 27 Oct 2023 11:57:45 +0100, Srinivas Kandagatla wrote:
> Click/Pop Noise was a long pending issue with WSA Codecs which are prone
> to accumlate DC when ports are active but without any data streams.
> There are multiple places in the current setup, where this could happen
> in both startup as well as shutdown path.
> 
> This patchset adds a new flag mute_unmute_on_trigger to dai_ops to let
> generic code do the mute/unmute on trigger.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/2] ASoC: soc-dai: add flag to mute and unmute stream during trigger
      commit: f0220575e65abe09c09cd17826a3cdea76e8d58f
[2/2] ASoC: codecs: wsa883x: make use of new mute_unmute_on_trigger flag
      commit: 805ce81826c896dd3c351a32814b28557f9edf54

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

On 10/27/2023 12:57 PM, Srinivas Kandagatla wrote:
> Click/Pop Noise was a long pending issue with WSA Codecs which are prone
> to accumlate DC when ports are active but without any data streams.
> There are multiple places in the current setup, where this could happen
> in both startup as well as shutdown path.
> 
> This patchset adds a new flag mute_unmute_on_trigger to dai_ops to let
> generic code do the mute/unmute on trigger.
> 
> This patches help fix those issues by making sure the PA is Muted/Unmuted
> inline with the stream start/stop events.
> 
> 
> Srinivas Kandagatla (2):
>    ASoC: soc-dai: add flag to mute and unmute stream during trigger
>    ASoC: codecs: wsa883x: make use of new mute_unmute_on_trigger flag
> 
>   include/sound/soc-dai.h    |  1 +
>   sound/soc/codecs/wsa883x.c |  7 +------
>   sound/soc/soc-dai.c        |  7 +++++++
>   sound/soc/soc-pcm.c        | 12 ++++++++----
>   4 files changed, 17 insertions(+), 10 deletions(-)
> 

Have you tried something like:
diff --git a/sound/soc/codecs/wsa883x.c b/sound/soc/codecs/wsa883x.c
index e40d583a1ce6..f02362cf95dc 100644
--- a/sound/soc/codecs/wsa883x.c
+++ b/sound/soc/codecs/wsa883x.c
@@ -1229,6 +1229,8 @@ static int wsa883x_spkr_event(struct 
snd_soc_dapm_widget *w,
                 snd_soc_component_write_field(component, 
WSA883X_VBAT_ADC_FLT_CTL,
 
WSA883X_VBAT_ADC_COEF_SEL_MASK,
 
WSA883X_VBAT_ADC_COEF_F_1DIV2);
+               snd_soc_component_write_field(component, WSA883X_DRE_CTL_1,
+                                             WSA883X_DRE_GAIN_EN_MASK, 0);
                 snd_soc_component_write_field(component, 
WSA883X_PA_FSM_CTL,
 
WSA883X_GLOBAL_PA_EN_MASK, 0);
                 snd_soc_component_write_field(component, 
WSA883X_PDM_WD_CTL,


As it is one thing from wsa883x_digital_mute() which seems missing in 
SND_SOC_DAPM_PRE_PMD switch case, so it seems to leave GAIN always enabled?

Anyway this seems like something that if possible should be fixed on 
codec driver side instead of introducing global flag?

On Fri, Oct 27, 2023 at 02:00:13PM +0200, Amadeusz Sławiński wrote:

> Have you tried something like:
> diff --git a/sound/soc/codecs/wsa883x.c b/sound/soc/codecs/wsa883x.c
> index e40d583a1ce6..f02362cf95dc 100644
> --- a/sound/soc/codecs/wsa883x.c
> +++ b/sound/soc/codecs/wsa883x.c
> @@ -1229,6 +1229,8 @@ static int wsa883x_spkr_event(struct 
> snd_soc_dapm_widget *w,
>                  snd_soc_component_write_field(component, 
> WSA883X_VBAT_ADC_FLT_CTL,
>  
> WSA883X_VBAT_ADC_COEF_SEL_MASK,
>  
> WSA883X_VBAT_ADC_COEF_F_1DIV2);
> +               snd_soc_component_write_field(component, WSA883X_DRE_CTL_1,
> +                                             WSA883X_DRE_GAIN_EN_MASK, 0);
>                  snd_soc_component_write_field(component, 
> WSA883X_PA_FSM_CTL,
>  
> WSA883X_GLOBAL_PA_EN_MASK, 0);
>                  snd_soc_component_write_field(component, 
> WSA883X_PDM_WD_CTL,
> 
> 
> As it is one thing from wsa883x_digital_mute() which seems missing in 
> SND_SOC_DAPM_PRE_PMD switch case, so it seems to leave GAIN always enabled?

Since I had the test setup ready, I tried the above quickly but it
doesn't seem to make any difference.

Johan

On Fri, Oct 27, 2023 at 11:57:45AM +0100, Srinivas Kandagatla wrote:
> Click/Pop Noise was a long pending issue with WSA Codecs which are prone
> to accumlate DC when ports are active but without any data streams.
> There are multiple places in the current setup, where this could happen
> in both startup as well as shutdown path.
> 
> This patchset adds a new flag mute_unmute_on_trigger to dai_ops to let
> generic code do the mute/unmute on trigger.
> 
> This patches help fix those issues by making sure the PA is Muted/Unmuted
> inline with the stream start/stop events.
>
> Srinivas Kandagatla (2):
>   ASoC: soc-dai: add flag to mute and unmute stream during trigger
>   ASoC: codecs: wsa883x: make use of new mute_unmute_on_trigger flag

I've verified that this fixes the pop sounds when starting and stopping
a stream on the X13s, even if the click sound when killing pulseaudio
(e.g. on reboot) is still there (as with the previous fixes).

Tested-by: Johan Hovold <johan+linaro@kernel.org>

If these are accepted, can we get them backported to 6.5 as well?

Johan

On Fri, Oct 27, 2023 at 01:52:11PM +0200, Johan Hovold wrote:
> On Fri, Oct 27, 2023 at 11:57:45AM +0100, Srinivas Kandagatla wrote:
> > Click/Pop Noise was a long pending issue with WSA Codecs which are prone
> > to accumlate DC when ports are active but without any data streams.
> > There are multiple places in the current setup, where this could happen
> > in both startup as well as shutdown path.
> > 
> > This patchset adds a new flag mute_unmute_on_trigger to dai_ops to let
> > generic code do the mute/unmute on trigger.
> > 
> > This patches help fix those issues by making sure the PA is Muted/Unmuted
> > inline with the stream start/stop events.
> >
> > Srinivas Kandagatla (2):
> >   ASoC: soc-dai: add flag to mute and unmute stream during trigger
> >   ASoC: codecs: wsa883x: make use of new mute_unmute_on_trigger flag
> 
> I've verified that this fixes the pop sounds when starting and stopping
> a stream on the X13s, even if the click sound when killing pulseaudio
> (e.g. on reboot) is still there (as with the previous fixes).

For the record, the remaining click sounds were incidentally fixed by
commit 3efcb471f871 ("ASoC: soc-pcm.c: Make sure DAI parameters cleared
if the DAI becomes inactive") which also went into 6.7-rc1:

	https://lore.kernel.org/lkml/20230920153621.711373-1-chancel.liu@nxp.com/

Johan

Hi Greg and Sasha,

On Fri, Oct 27, 2023 at 01:52:11PM +0200, Johan Hovold wrote:
> On Fri, Oct 27, 2023 at 11:57:45AM +0100, Srinivas Kandagatla wrote:
> > Click/Pop Noise was a long pending issue with WSA Codecs which are prone
> > to accumlate DC when ports are active but without any data streams.
> > There are multiple places in the current setup, where this could happen
> > in both startup as well as shutdown path.
> > 
> > This patchset adds a new flag mute_unmute_on_trigger to dai_ops to let
> > generic code do the mute/unmute on trigger.
> > 
> > This patches help fix those issues by making sure the PA is Muted/Unmuted
> > inline with the stream start/stop events.
> >
> > Srinivas Kandagatla (2):
> >   ASoC: soc-dai: add flag to mute and unmute stream during trigger
> >   ASoC: codecs: wsa883x: make use of new mute_unmute_on_trigger flag
> 
> I've verified that this fixes the pop sounds when starting and stopping
> a stream on the X13s, even if the click sound when killing pulseaudio
> (e.g. on reboot) is still there (as with the previous fixes).
> 
> Tested-by: Johan Hovold <johan+linaro@kernel.org>
> 
> If these are accepted, can we get them backported to 6.5 as well?

These fixes are now in 6.7-rc1 as

	f0220575e65a ("ASoC: soc-dai: add flag to mute and unmute stream during trigger")
	805ce81826c8 ("ASoC: codecs: wsa883x: make use of new mute_unmute_on_trigger flag")

As these fix a loud scary crackling noise on the Lenovo ThinkPad X13s,
is it possible to get these backported at least to stable 6.6 and 6.5?

Johan

On Wed, Nov 22, 2023 at 04:41:37PM +0100, Johan Hovold wrote:
> Hi Greg and Sasha,
> 
> On Fri, Oct 27, 2023 at 01:52:11PM +0200, Johan Hovold wrote:
> > On Fri, Oct 27, 2023 at 11:57:45AM +0100, Srinivas Kandagatla wrote:
> > > Click/Pop Noise was a long pending issue with WSA Codecs which are prone
> > > to accumlate DC when ports are active but without any data streams.
> > > There are multiple places in the current setup, where this could happen
> > > in both startup as well as shutdown path.
> > > 
> > > This patchset adds a new flag mute_unmute_on_trigger to dai_ops to let
> > > generic code do the mute/unmute on trigger.
> > > 
> > > This patches help fix those issues by making sure the PA is Muted/Unmuted
> > > inline with the stream start/stop events.
> > >
> > > Srinivas Kandagatla (2):
> > >   ASoC: soc-dai: add flag to mute and unmute stream during trigger
> > >   ASoC: codecs: wsa883x: make use of new mute_unmute_on_trigger flag
> > 
> > I've verified that this fixes the pop sounds when starting and stopping
> > a stream on the X13s, even if the click sound when killing pulseaudio
> > (e.g. on reboot) is still there (as with the previous fixes).
> > 
> > Tested-by: Johan Hovold <johan+linaro@kernel.org>
> > 
> > If these are accepted, can we get them backported to 6.5 as well?
> 
> These fixes are now in 6.7-rc1 as
> 
> 	f0220575e65a ("ASoC: soc-dai: add flag to mute and unmute stream during trigger")

This doesn't backport cleanly, can you provide a working backport?

> 	805ce81826c8 ("ASoC: codecs: wsa883x: make use of new mute_unmute_on_trigger flag")

Now queued up, thanks.

greg k-h

On Wed, Nov 22, 2023 at 04:35:17PM +0000, Greg Kroah-Hartman wrote:
> On Wed, Nov 22, 2023 at 04:41:37PM +0100, Johan Hovold wrote:

> > These fixes are now in 6.7-rc1 as
> > 
> > 	f0220575e65a ("ASoC: soc-dai: add flag to mute and unmute stream during trigger")
> 
> This doesn't backport cleanly, can you provide a working backport?

Sure, I'll do that tomorrow.
 
> > 	805ce81826c8 ("ASoC: codecs: wsa883x: make use of new mute_unmute_on_trigger flag")
> 
> Now queued up, thanks.

I don't think this one will build without the former so better to drop
it from your queues and I'll send backports of both patches tomorrow.

Johan

On Wed, Nov 22, 2023 at 05:49:35PM +0100, Johan Hovold wrote:
> On Wed, Nov 22, 2023 at 04:35:17PM +0000, Greg Kroah-Hartman wrote:
> > On Wed, Nov 22, 2023 at 04:41:37PM +0100, Johan Hovold wrote:
> 
> > > These fixes are now in 6.7-rc1 as
> > > 
> > > 	f0220575e65a ("ASoC: soc-dai: add flag to mute and unmute stream during trigger")
> > 
> > This doesn't backport cleanly, can you provide a working backport?
> 
> Sure, I'll do that tomorrow.
>  
> > > 	805ce81826c8 ("ASoC: codecs: wsa883x: make use of new mute_unmute_on_trigger flag")
> > 
> > Now queued up, thanks.
> 
> I don't think this one will build without the former so better to drop
> it from your queues and I'll send backports of both patches tomorrow.

I've just posted backports of these commits to 6.6.2 here:

	https://lore.kernel.org/lkml/20231123094749.20462-1-johan+linaro@kernel.org/

They should apply to 6.5.12 as well.

Turns out we had a conflict with commit 3efcb471f871 ("ASoC: soc-pcm.c:
Make sure DAI parameters cleared if the DAI becomes inactive") which was
just backported to 6.6.2 and 6.5.12.

Johan

On Wed, Nov 22, 2023 at 04:35:17PM +0000, Greg Kroah-Hartman wrote:
> On Wed, Nov 22, 2023 at 04:41:37PM +0100, Johan Hovold wrote:
> > Hi Greg and Sasha,
> > 
> > On Fri, Oct 27, 2023 at 01:52:11PM +0200, Johan Hovold wrote:
> > > On Fri, Oct 27, 2023 at 11:57:45AM +0100, Srinivas Kandagatla wrote:
> > > > Click/Pop Noise was a long pending issue with WSA Codecs which are prone
> > > > to accumlate DC when ports are active but without any data streams.
> > > > There are multiple places in the current setup, where this could happen
> > > > in both startup as well as shutdown path.
> > > > 
> > > > This patchset adds a new flag mute_unmute_on_trigger to dai_ops to let
> > > > generic code do the mute/unmute on trigger.
> > > > 
> > > > This patches help fix those issues by making sure the PA is Muted/Unmuted
> > > > inline with the stream start/stop events.
> > > >
> > > > Srinivas Kandagatla (2):
> > > >   ASoC: soc-dai: add flag to mute and unmute stream during trigger
> > > >   ASoC: codecs: wsa883x: make use of new mute_unmute_on_trigger flag
> > > 
> > > I've verified that this fixes the pop sounds when starting and stopping
> > > a stream on the X13s, even if the click sound when killing pulseaudio
> > > (e.g. on reboot) is still there (as with the previous fixes).
> > > 
> > > Tested-by: Johan Hovold <johan+linaro@kernel.org>
> > > 
> > > If these are accepted, can we get them backported to 6.5 as well?
> > 
> > These fixes are now in 6.7-rc1 as
> > 
> > 	f0220575e65a ("ASoC: soc-dai: add flag to mute and unmute stream during trigger")
> 
> This doesn't backport cleanly, can you provide a working backport?
> 
> > 	805ce81826c8 ("ASoC: codecs: wsa883x: make use of new mute_unmute_on_trigger flag")
> 
> Now queued up, thanks.

And that broke the build, I'll go drop that too :(