[RFC PATCH 0/2] ASoC: soc-dai: add flag to mute and unmute stream during trigger.
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(-)
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);
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[] = {
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
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:
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 :(
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 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
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 02:00:13PM +0200, Amadeusz Sławiński wrote:
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.
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
...
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,
...
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?
If the issue is that the device misbehaves as soon as the incoming data stream stops then we need to do something strictly before we do the actual trigger operation which we don't currently have hooks for. Even if we run quickly after the trigger there's going to be a window where there's no incoming audio and no mute.
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
participants (5)
-
Amadeusz Sławiński -
Greg Kroah-Hartman -
Johan Hovold -
Mark Brown -
Srinivas Kandagatla