[PATCH 0/4] ASoC: qcom: fixes for Click/Pop Noise

[PATCH 0/4] ASoC: qcom: fixes for Click/Pop Noise

[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 patches help fix those issues by making sure the PA is Muted/Unmuted
inline with the stream start/stop events.

Other than this there is a patch to fix DSP graph saturation issue as we never unload
playback graph before loading same graph.

Srinivas Kandagatla (4):
  ASoC: qcom: q6apm-lpass-dai: close graphs before opening a new one
  ASoC: qcom: sdw: do not restart soundwire ports for every prepare
  ASoC: codecs: wsa883x: mute/unmute PA in correct sequence
  ASoC: codecs: wsa881x: mute/unmute PA in correct sequence

 sound/soc/codecs/wsa881x.c              | 30 ++++++++++++++++++++-
 sound/soc/codecs/wsa883x.c              | 36 ++++++++++++++++++++-----
 sound/soc/qcom/qdsp6/q6apm-lpass-dais.c |  3 +++
 sound/soc/qcom/sdw.c                    |  7 ++---
 4 files changed, 63 insertions(+), 13 deletions(-)

-- 
2.21.0

[PATCH 1/4] ASoC: qcom: q6apm-lpass-dai: close graphs before opening a new one

[Srinivas Kandagatla]

On multiple prepare calls, its possible that the playback graphs are
not unloaded from the DSP, which can have some wierd side-effects,
one of them is that the data not consumed without any errors.

Fixes: c2ac3aec474d("ASoC: qcom: q6apm-lpass-dai: unprepare stream if its already prepared")
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/qcom/qdsp6/q6apm-lpass-dais.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
index 23d23bc6fbaa..420e8aa11f42 100644
--- a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
+++ b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
@@ -130,6 +130,9 @@ static int q6apm_lpass_dai_prepare(struct snd_pcm_substream *substream, struct s
 	if (dai_data->is_port_started[dai->id]) {
 		q6apm_graph_stop(dai_data->graph[dai->id]);
 		dai_data->is_port_started[dai->id] = false;
+
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+			q6apm_graph_close(dai_data->graph[dai->id]);
 	}
 
 	/**
-- 
2.21.0

[PATCH 2/4] ASoC: qcom: sdw: do not restart soundwire ports for every prepare

[Srinivas Kandagatla]

unpreparing/disabling and preparing/reenabling soundwire ports is not required
for every prepare call, this add lots of click and pop noise if we do this in
middle of playback or capture.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/qcom/sdw.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/sound/soc/qcom/sdw.c b/sound/soc/qcom/sdw.c
index 10249519a39e..1a41419c7eb8 100644
--- a/sound/soc/qcom/sdw.c
+++ b/sound/soc/qcom/sdw.c
@@ -32,11 +32,8 @@ int qcom_snd_sdw_prepare(struct snd_pcm_substream *substream,
 		return 0;
 	}
 
-	if (*stream_prepared) {
-		sdw_disable_stream(sruntime);
-		sdw_deprepare_stream(sruntime);
-		*stream_prepared = false;
-	}
+	if (*stream_prepared)
+		return 0;
 
 	ret = sdw_prepare_stream(sruntime);
 	if (ret)
-- 
2.21.0

[PATCH 3/4] ASoC: codecs: wsa883x: mute/unmute PA in correct sequence

[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.

Moving the mute/unmute to trigger stop/start respectively seems to
help 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 | 36 +++++++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/sound/soc/codecs/wsa883x.c b/sound/soc/codecs/wsa883x.c
index c609cb63dae6..b83b5b0d4bab 100644
--- a/sound/soc/codecs/wsa883x.c
+++ b/sound/soc/codecs/wsa883x.c
@@ -1204,9 +1204,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,
@@ -1219,9 +1216,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:
@@ -1341,10 +1335,38 @@ static int wsa883x_digital_mute(struct snd_soc_dai *dai, int mute, int stream)
 	return 0;
 }
 
+static int wsa883x_trigger(struct snd_pcm_substream *s, int cmd,
+			   struct snd_soc_dai *dai)
+{
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		wsa883x_digital_mute(dai, false, 0);
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		wsa883x_digital_mute(dai, true, 0);
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static int wsa883x_startup(struct snd_pcm_substream *stream,
+			   struct snd_soc_dai *dai)
+{
+	return wsa883x_digital_mute(dai, true, 0);
+}
+
 static const struct snd_soc_dai_ops wsa883x_dai_ops = {
+	.startup = wsa883x_startup,
 	.hw_params = wsa883x_hw_params,
 	.hw_free = wsa883x_hw_free,
-	.mute_stream = wsa883x_digital_mute,
+	.trigger = wsa883x_trigger,
 	.set_stream = wsa883x_set_sdw_stream,
 };
 
-- 
2.21.0

[PATCH 4/4] ASoC: codecs: wsa881x: mute/unmute PA in correct sequence

[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.

Moving the mute/unmute to trigger stop/start respectively seems to
help a lot with this Click/Pop issues reported on this Codec.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/codecs/wsa881x.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/wsa881x.c b/sound/soc/codecs/wsa881x.c
index f709231b1277..4ce72a7f01b6 100644
--- a/sound/soc/codecs/wsa881x.c
+++ b/sound/soc/codecs/wsa881x.c
@@ -1033,11 +1033,39 @@ static int wsa881x_digital_mute(struct snd_soc_dai *dai, int mute, int stream)
 	return 0;
 }
 
+static int wsa881x_trigger(struct snd_pcm_substream *s, int cmd,
+			   struct snd_soc_dai *dai)
+{
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		wsa881x_digital_mute(dai, false, 0);
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		wsa881x_digital_mute(dai, true, 0);
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static int wsa881x_startup(struct snd_pcm_substream *stream,
+			   struct snd_soc_dai *dai)
+{
+	return wsa881x_digital_mute(dai, true, 0);
+}
+
 static const struct snd_soc_dai_ops wsa881x_dai_ops = {
+	.startup = wsa881x_startup,
 	.hw_params = wsa881x_hw_params,
 	.hw_free = wsa881x_hw_free,
-	.mute_stream = wsa881x_digital_mute,
 	.set_stream = wsa881x_set_sdw_stream,
+	.trigger = wsa881x_trigger,
 };
 
 static struct snd_soc_dai_driver wsa881x_dais[] = {
-- 
2.21.0

Re: [PATCH 3/4] ASoC: codecs: wsa883x: mute/unmute PA in correct sequence

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1488 bytes --]

On Thu, Mar 23, 2023 at 04:44:02PM +0000, Srinivas Kandagatla wrote:
> 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.

Wow, that hardware sounds *super* fragile.

> Moving the mute/unmute to trigger stop/start respectively seems to
> help a lot with this Click/Pop issues reported on this Codec.

> +static int wsa883x_trigger(struct snd_pcm_substream *s, int cmd,
> +			   struct snd_soc_dai *dai)
> +{
> +	switch (cmd) {
> +	case SNDRV_PCM_TRIGGER_START:
> +	case SNDRV_PCM_TRIGGER_RESUME:
> +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> +		wsa883x_digital_mute(dai, false, 0);
> +		break;

>  static const struct snd_soc_dai_ops wsa883x_dai_ops = {
> +	.startup = wsa883x_startup,
>  	.hw_params = wsa883x_hw_params,
>  	.hw_free = wsa883x_hw_free,
> -	.mute_stream = wsa883x_digital_mute,
> +	.trigger = wsa883x_trigger,

The trigger is run in atomic context, can you really write safely to a
SoundWire device there?

This feels like we should be doing it at the framework level, either
tightening up where the mute happens in general or having some option
that devices can select if they really need it.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 3/4] ASoC: codecs: wsa883x: mute/unmute PA in correct sequence

[Pierre-Louis Bossart]

>> +static int wsa883x_trigger(struct snd_pcm_substream *s, int cmd,
>> +			   struct snd_soc_dai *dai)
>> +{
>> +	switch (cmd) {
>> +	case SNDRV_PCM_TRIGGER_START:
>> +	case SNDRV_PCM_TRIGGER_RESUME:
>> +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
>> +		wsa883x_digital_mute(dai, false, 0);
>> +		break;
> 
>>  static const struct snd_soc_dai_ops wsa883x_dai_ops = {
>> +	.startup = wsa883x_startup,
>>  	.hw_params = wsa883x_hw_params,
>>  	.hw_free = wsa883x_hw_free,
>> -	.mute_stream = wsa883x_digital_mute,
>> +	.trigger = wsa883x_trigger,
> 
> The trigger is run in atomic context, can you really write safely to a
> SoundWire device there?

Mark, I've seen that comment from you several times, and I wonder if I
am missing something: the triggers for SoundWire managers and dailinks
are typically nonatomic - at least for the Cadence-based solution the
trigger is based on a bank switch that may happen with a delay and with
a wait_for_completion(). Sending a command over the SoundWire channel is
also typically not atomic, there's usually a wait_for_completion() as well.

Re: [PATCH 3/4] ASoC: codecs: wsa883x: mute/unmute PA in correct sequence

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 846 bytes --]

On Thu, Mar 23, 2023 at 01:11:11PM -0500, Pierre-Louis Bossart wrote:

> > The trigger is run in atomic context, can you really write safely to a
> > SoundWire device there?

> Mark, I've seen that comment from you several times, and I wonder if I
> am missing something: the triggers for SoundWire managers and dailinks
> are typically nonatomic - at least for the Cadence-based solution the
> trigger is based on a bank switch that may happen with a delay and with
> a wait_for_completion(). Sending a command over the SoundWire channel is
> also typically not atomic, there's usually a wait_for_completion() as well.

Ah, you're setting the nonatomic flag on your links to disable the
locking.  The default for trigger operations is to run them with local
interrupts disabled.  It looks like at least some of the Qualcomm stuff
does that too.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 3/4] ASoC: codecs: wsa883x: mute/unmute PA in correct sequence

[Srinivas Kandagatla]

On 24/03/2023 00:14, Mark Brown wrote:
> On Thu, Mar 23, 2023 at 01:11:11PM -0500, Pierre-Louis Bossart wrote:
> 
>>> The trigger is run in atomic context, can you really write safely to a
>>> SoundWire device there?
> 
>> Mark, I've seen that comment from you several times, and I wonder if I
>> am missing something: the triggers for SoundWire managers and dailinks
>> are typically nonatomic - at least for the Cadence-based solution the
>> trigger is based on a bank switch that may happen with a delay and with
>> a wait_for_completion(). Sending a command over the SoundWire channel is
>> also typically not atomic, there's usually a wait_for_completion() as well.
> 
> Ah, you're setting the nonatomic flag on your links to disable the
> locking.  The default for trigger operations is to run them with local
> interrupts disabled.  It looks like at least some of the Qualcomm stuff
> does that too.
Yes, by default dailinks are marked as nonatomic in Qualcomm case aswell.

--srini

Re: [PATCH 3/4] ASoC: codecs: wsa883x: mute/unmute PA in correct sequence

[Srinivas Kandagatla]

On 23/03/2023 17:07, Mark Brown wrote:
> On Thu, Mar 23, 2023 at 04:44:02PM +0000, Srinivas Kandagatla wrote:
>> 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.
> 
> Wow, that hardware sounds *super* fragile.
> 
>> Moving the mute/unmute to trigger stop/start respectively seems to
>> help a lot with this Click/Pop issues reported on this Codec.
> 
>> +static int wsa883x_trigger(struct snd_pcm_substream *s, int cmd,
>> +			   struct snd_soc_dai *dai)
>> +{
>> +	switch (cmd) {
>> +	case SNDRV_PCM_TRIGGER_START:
>> +	case SNDRV_PCM_TRIGGER_RESUME:
>> +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
>> +		wsa883x_digital_mute(dai, false, 0);
>> +		break;
> 
>>   static const struct snd_soc_dai_ops wsa883x_dai_ops = {
>> +	.startup = wsa883x_startup,
>>   	.hw_params = wsa883x_hw_params,
>>   	.hw_free = wsa883x_hw_free,
>> -	.mute_stream = wsa883x_digital_mute,
>> +	.trigger = wsa883x_trigger,
> 
> The trigger is run in atomic context, can you really write safely to a
> SoundWire device there?
> 
> This feels like we should be doing it at the framework level, either
> tightening up where the mute happens in general or having some option
> that devices can select if they really need it.
That makes more sense, I can give that a try.

--srini

Re: (subset) [PATCH 0/4] ASoC: qcom: fixes for Click/Pop Noise

[Mark Brown]

On Thu, 23 Mar 2023 16:43:59 +0000, 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 patches help fix those issues by making sure the PA is Muted/Unmuted
> inline with the stream start/stop events.
> 
> [...]

Applied to

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

Thanks!

[1/4] ASoC: qcom: q6apm-lpass-dai: close graphs before opening a new one
      commit: c52615e494f17f44b076ac8ae5a53cfc0041a0dd
[2/4] ASoC: qcom: sdw: do not restart soundwire ports for every prepare
      commit: e2e530886359246ae782c779be248c59bc2ed111

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

Re: [PATCH 3/4] ASoC: codecs: wsa883x: mute/unmute PA in correct sequence

[Johan Hovold]

Hi Mark,

On Fri, Mar 24, 2023 at 06:44:40AM +0000, Srinivas Kandagatla wrote:
> On 23/03/2023 17:07, Mark Brown wrote:
> > On Thu, Mar 23, 2023 at 04:44:02PM +0000, Srinivas Kandagatla wrote:
> >> 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.
> > 
> > Wow, that hardware sounds *super* fragile.
> > 
> >> Moving the mute/unmute to trigger stop/start respectively seems to
> >> help a lot with this Click/Pop issues reported on this Codec.
> > 
> >> +static int wsa883x_trigger(struct snd_pcm_substream *s, int cmd,
> >> +			   struct snd_soc_dai *dai)
> >> +{
> >> +	switch (cmd) {
> >> +	case SNDRV_PCM_TRIGGER_START:
> >> +	case SNDRV_PCM_TRIGGER_RESUME:
> >> +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> >> +		wsa883x_digital_mute(dai, false, 0);
> >> +		break;
> > 
> >>   static const struct snd_soc_dai_ops wsa883x_dai_ops = {
> >> +	.startup = wsa883x_startup,
> >>   	.hw_params = wsa883x_hw_params,
> >>   	.hw_free = wsa883x_hw_free,
> >> -	.mute_stream = wsa883x_digital_mute,
> >> +	.trigger = wsa883x_trigger,

> > This feels like we should be doing it at the framework level, either
> > tightening up where the mute happens in general or having some option
> > that devices can select if they really need it.

> That makes more sense, I can give that a try.

I understand Srini has looked at this but has not yet been able to come
up with a generic implementation. Would it be possible to merge the two
codec fixes as an interim workaround for 6.7?

Without the wsa883x patch there's a loud crackling scary noise when
starting a stream on the Lenovo ThinkPad X13s which users will hit now
that they can run mainline on this machine.

	https://lore.kernel.org/lkml/20230323164403.6654-4-srinivas.kandagatla@linaro.org/

I've been using this one myself for the past seven months without any
issues (even if there's still a faint click when stopping a stream):

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

Getting this backported at least to 6.5 where sound support for the X13s
was added would be great too.

Johan

Re: [PATCH 3/4] ASoC: codecs: wsa883x: mute/unmute PA in correct sequence

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 328 bytes --]

On Wed, Oct 25, 2023 at 09:57:12AM +0200, Johan Hovold wrote:

> I understand Srini has looked at this but has not yet been able to come
> up with a generic implementation. Would it be possible to merge the two
> codec fixes as an interim workaround for 6.7?

You're talking about two fixes here but this is a 4 patch series...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 3/4] ASoC: codecs: wsa883x: mute/unmute PA in correct sequence

[Johan Hovold]

[-- Attachment #1: Type: text/plain, Size: 581 bytes --]

On Wed, Oct 25, 2023 at 01:36:14PM +0100, Mark Brown wrote:
> On Wed, Oct 25, 2023 at 09:57:12AM +0200, Johan Hovold wrote:
> 
> > I understand Srini has looked at this but has not yet been able to come
> > up with a generic implementation. Would it be possible to merge the two
> > codec fixes as an interim workaround for 6.7?
> 
> You're talking about two fixes here but this is a 4 patch series...

Yes, sorry, I should have been more clear. I was talking about the codec
fixes so that's patch 3 and 4.

I believe the first two have already been applied.

Johan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 3/4] ASoC: codecs: wsa883x: mute/unmute PA in correct sequence

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 986 bytes --]

On Wed, Oct 25, 2023 at 02:43:28PM +0200, Johan Hovold wrote:
> On Wed, Oct 25, 2023 at 01:36:14PM +0100, Mark Brown wrote:
> > On Wed, Oct 25, 2023 at 09:57:12AM +0200, Johan Hovold wrote:

> > > I understand Srini has looked at this but has not yet been able to come
> > > up with a generic implementation. Would it be possible to merge the two
> > > codec fixes as an interim workaround for 6.7?

> > You're talking about two fixes here but this is a 4 patch series...

> Yes, sorry, I should have been more clear. I was talking about the codec
> fixes so that's patch 3 and 4.

> I believe the first two have already been applied.

So, having gone and fished the series out of lore to look at what the
patches are I'm pretty surprised here, it's been about six months since
the original discussion and I'd not have expected this to be such a
difficult thing to do, or at least that any issues would have been
flagged up by now.  What are the issues that have been encountered here?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 3/4] ASoC: codecs: wsa883x: mute/unmute PA in correct sequence

[Johan Hovold]

[-- Attachment #1: Type: text/plain, Size: 1200 bytes --]

On Thu, Oct 26, 2023 at 02:05:25PM +0100, Mark Brown wrote:
> On Wed, Oct 25, 2023 at 02:43:28PM +0200, Johan Hovold wrote:
> > On Wed, Oct 25, 2023 at 01:36:14PM +0100, Mark Brown wrote:
> > > On Wed, Oct 25, 2023 at 09:57:12AM +0200, Johan Hovold wrote:
> 
> > > > I understand Srini has looked at this but has not yet been able to come
> > > > up with a generic implementation. Would it be possible to merge the two
> > > > codec fixes as an interim workaround for 6.7?

> So, having gone and fished the series out of lore to look at what the
> patches are I'm pretty surprised here, it's been about six months since
> the original discussion and I'd not have expected this to be such a
> difficult thing to do, or at least that any issues would have been
> flagged up by now.  What are the issues that have been encountered here?

I'm also surprised that this has not yet been resolved, especially given
that I've been reminding Srini about the need to get this fixed
repeatedly for the past six months.

I haven't looked at this myself yet and I don't know what the technical
issues he hinted about having run into would be.

Srini, can you provide some details?

Johan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 3/4] ASoC: codecs: wsa883x: mute/unmute PA in correct sequence

[Srinivas Kandagatla]

On 26/10/2023 14:05, Mark Brown wrote:
> On Wed, Oct 25, 2023 at 02:43:28PM +0200, Johan Hovold wrote:
>> On Wed, Oct 25, 2023 at 01:36:14PM +0100, Mark Brown wrote:
>>> On Wed, Oct 25, 2023 at 09:57:12AM +0200, Johan Hovold wrote:
> 
>>>> I understand Srini has looked at this but has not yet been able to come
>>>> up with a generic implementation. Would it be possible to merge the two
>>>> codec fixes as an interim workaround for 6.7?
> 
>>> You're talking about two fixes here but this is a 4 patch series...
> 
>> Yes, sorry, I should have been more clear. I was talking about the codec
>> fixes so that's patch 3 and 4.
> 
>> I believe the first two have already been applied.
> 
> So, having gone and fished the series out of lore to look at what the
> patches are I'm pretty surprised here, it's been about six months since
> the original discussion and I'd not have expected this to be such a
> difficult thing to do, or at least that any issues would have been
> flagged up by now.  What are the issues that have been encountered here?

The approach I tried was to reuse the existing 
snd_soc_dai_digital_mute() from generic snd_soc_pcm_dai_trigger()

The issue is that prepare also unmutes the stream which is why moving 
this to generic code is not helping in this setup.

To overcome this I have added a flag in snd_soc_dai_ops which seems to 
have helped

Let me send that patch as RFC for more discussion.

thanks,
Srini