| Message ID | 20231027105747.32450-1-srinivas.kandagatla@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | ASoC: soc-dai: add flag to mute and unmute stream during trigger. | expand |
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 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, 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
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