[RFC PATCH 0/2] ASoC: soc-dai: add flag to mute and unmute stream during trigger.

[RFC PATCH 0/2] ASoC: soc-dai: add flag to mute and unmute stream during trigger.

[Srinivas Kandagatla]

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

[RFC PATCH 1/2] ASoC: soc-dai: add flag to mute and unmute stream during trigger

[Srinivas Kandagatla]

In some setups like Speaker amps which are very sensitive, ex: keeping them
unmute without actual data stream for very short duration results in a
static charge and results in pop and clicks. To minimize this, provide a way
to mute and unmute such codecs during trigger callbacks.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 include/sound/soc-dai.h |  1 +
 sound/soc/soc-dai.c     |  7 +++++++
 sound/soc/soc-pcm.c     | 12 ++++++++----
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index e3906ecda740..5827b4d882fc 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -355,6 +355,7 @@ struct snd_soc_dai_ops {
 
 	/* bit field */
 	unsigned int no_capture_mute:1;
+	unsigned int mute_unmute_on_trigger:1;
 };
 
 struct snd_soc_cdai_ops {
diff --git a/sound/soc/soc-dai.c b/sound/soc/soc-dai.c
index 02dd64dea179..28d8c6c3d3b2 100644
--- a/sound/soc/soc-dai.c
+++ b/sound/soc/soc-dai.c
@@ -641,6 +641,10 @@ int snd_soc_pcm_dai_trigger(struct snd_pcm_substream *substream,
 			ret = soc_dai_trigger(dai, substream, cmd);
 			if (ret < 0)
 				break;
+
+			if (dai->driver->ops && dai->driver->ops->mute_unmute_on_trigger)
+				snd_soc_dai_digital_mute(dai, 0, substream->stream);
+
 			soc_dai_mark_push(dai, substream, trigger);
 		}
 		break;
@@ -651,6 +655,9 @@ int snd_soc_pcm_dai_trigger(struct snd_pcm_substream *substream,
 			if (rollback && !soc_dai_mark_match(dai, substream, trigger))
 				continue;
 
+			if (dai->driver->ops && dai->driver->ops->mute_unmute_on_trigger)
+				snd_soc_dai_digital_mute(dai, 1, substream->stream);
+
 			r = soc_dai_trigger(dai, substream, cmd);
 			if (r < 0)
 				ret = r; /* use last ret */
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 3aa6b988cb4b..0f9f4da25158 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -703,8 +703,10 @@ static int soc_pcm_clean(struct snd_soc_pcm_runtime *rtd,
 			if (snd_soc_dai_active(dai) == 0)
 				soc_pcm_set_dai_params(dai, NULL);
 
-			if (snd_soc_dai_stream_active(dai, substream->stream) == 0)
-				snd_soc_dai_digital_mute(dai, 1, substream->stream);
+			if (snd_soc_dai_stream_active(dai, substream->stream) ==  0) {
+				if (dai->driver->ops && !dai->driver->ops->mute_unmute_on_trigger)
+					snd_soc_dai_digital_mute(dai, 1, substream->stream);
+			}
 		}
 	}
 
@@ -898,8 +900,10 @@ static int __soc_pcm_prepare(struct snd_soc_pcm_runtime *rtd,
 	snd_soc_dapm_stream_event(rtd, substream->stream,
 			SND_SOC_DAPM_STREAM_START);
 
-	for_each_rtd_dais(rtd, i, dai)
-		snd_soc_dai_digital_mute(dai, 0, substream->stream);
+	for_each_rtd_dais(rtd, i, dai) {
+		if (dai->driver->ops && !dai->driver->ops->mute_unmute_on_trigger)
+			snd_soc_dai_digital_mute(dai, 0, substream->stream);
+	}
 
 out:
 	return soc_pcm_ret(rtd, ret);
-- 
2.21.0

[RFC PATCH 2/2] ASoC: codecs: wsa883x: make use of new mute_unmute_on_trigger flag

[Srinivas Kandagatla]

In the current setup the PA is left unmuted even when the
Soundwire ports are not started streaming. This can lead to click
and pop sounds during start.
There is a same issue in the reverse order where in the PA is
left unmute even after the data stream is stopped, the time
between data stream stopping and port closing is long enough
to accumulate DC on the line resulting in Click/Pop noise
during end of stream.

making use of new mute_unmute_on_trigger flag is helping a
lot with this Click/Pop issues reported on this Codec

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/codecs/wsa883x.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/sound/soc/codecs/wsa883x.c b/sound/soc/codecs/wsa883x.c
index e40d583a1ce6..4ea550b0601b 100644
--- a/sound/soc/codecs/wsa883x.c
+++ b/sound/soc/codecs/wsa883x.c
@@ -1203,9 +1203,6 @@ static int wsa883x_spkr_event(struct snd_soc_dapm_widget *w,
 			break;
 		}
 
-		snd_soc_component_write_field(component, WSA883X_DRE_CTL_1,
-					      WSA883X_DRE_GAIN_EN_MASK,
-					      WSA883X_DRE_GAIN_FROM_CSR);
 		if (wsa883x->port_enable[WSA883X_PORT_COMP])
 			snd_soc_component_write_field(component, WSA883X_DRE_CTL_0,
 						      WSA883X_DRE_OFFSET_MASK,
@@ -1218,9 +1215,6 @@ static int wsa883x_spkr_event(struct snd_soc_dapm_widget *w,
 		snd_soc_component_write_field(component, WSA883X_PDM_WD_CTL,
 					      WSA883X_PDM_EN_MASK,
 					      WSA883X_PDM_ENABLE);
-		snd_soc_component_write_field(component, WSA883X_PA_FSM_CTL,
-					      WSA883X_GLOBAL_PA_EN_MASK,
-					      WSA883X_GLOBAL_PA_ENABLE);
 
 		break;
 	case SND_SOC_DAPM_PRE_PMD:
@@ -1346,6 +1340,7 @@ static const struct snd_soc_dai_ops wsa883x_dai_ops = {
 	.hw_free = wsa883x_hw_free,
 	.mute_stream = wsa883x_digital_mute,
 	.set_stream = wsa883x_set_sdw_stream,
+	.mute_unmute_on_trigger = true,
 };
 
 static struct snd_soc_dai_driver wsa883x_dais[] = {
-- 
2.21.0

Re: [RFC PATCH 0/2] ASoC: soc-dai: add flag to mute and unmute stream during trigger.

[Johan Hovold]

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

Re: [RFC PATCH 0/2] ASoC: soc-dai: add flag to mute and unmute stream during trigger.

[Johan Hovold]

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

Re: [RFC PATCH 0/2] ASoC: soc-dai: add flag to mute and unmute stream during trigger.

[Greg Kroah-Hartman]

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

Re: [RFC PATCH 0/2] ASoC: soc-dai: add flag to mute and unmute stream during trigger.

[Greg Kroah-Hartman]

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 :(

Re: [RFC PATCH 0/2] ASoC: soc-dai: add flag to mute and unmute stream during trigger.

[Johan Hovold]

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

Re: [RFC PATCH 0/2] ASoC: soc-dai: add flag to mute and unmute stream during trigger.

[Johan Hovold]

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

Re: [RFC PATCH 0/2] ASoC: soc-dai: add flag to mute and unmute stream during trigger.

[Johan Hovold]

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

Re: [RFC PATCH 0/2] ASoC: soc-dai: add flag to mute and unmute stream during trigger.

[Amadeusz Sławiński]

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?

Re: [RFC PATCH 0/2] ASoC: soc-dai: add flag to mute and unmute stream during trigger.

[Johan Hovold]

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

Re: [RFC PATCH 0/2] ASoC: soc-dai: add flag to mute and unmute stream during trigger.

[Mark Brown]

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