trigger is atomic (non-schedulable), and soundwire register writes are not
safe to run in an atomic context. (bus is locked with a mutex, and qcom
driver's callback can also sleep if the FIFO is full).

The important part of fixing the click/pop issue was removing the PA_EN
writes from the dapm events, AFAICT this flag doesn't help anyway.

Fixes: 28b0b18d5346 ("ASoC: codec: wsa884x: make use of new mute_unmute_on_trigger flag")
Signed-off-by: Jonathan Marek <jonathan@marek.ca>
---
 sound/soc/codecs/wsa884x.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/soc/codecs/wsa884x.c b/sound/soc/codecs/wsa884x.c
index 2484d4b8e2d94..0218dfc13bc77 100644
--- a/sound/soc/codecs/wsa884x.c
+++ b/sound/soc/codecs/wsa884x.c
@@ -1840,7 +1840,6 @@ static const struct snd_soc_dai_ops wsa884x_dai_ops = {
 	.hw_free = wsa884x_hw_free,
 	.mute_stream = wsa884x_mute_stream,
 	.set_stream = wsa884x_set_stream,
-	.mute_unmute_on_trigger = true,
 };
 
 static struct snd_soc_dai_driver wsa884x_dais[] = {
-- 
2.51.0

On 11/24/25 6:45 AM, Jonathan Marek wrote:
> trigger is atomic (non-schedulable), and soundwire register writes are not
> safe to run in an atomic context. (bus is locked with a mutex, and qcom
> driver's callback can also sleep if the FIFO is full).
> 
Thanks Jonathan for the patch,

We have nonatomic=1 flag set for all the Qualcomm sound cards, Did you
hit any schedule while atomic bug?



In-fact this change has helped suppress most of the click and pop noises
on laptops, specially with wsa codecs as they accumulate static if the
ports are kept open without sending any data.

--srini


> The important part of fixing the click/pop issue was removing the PA_EN
> writes from the dapm events, AFAICT this flag doesn't help anyway.
> 
> Fixes: 28b0b18d5346 ("ASoC: codec: wsa884x: make use of new mute_unmute_on_trigger flag")
> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
> ---
>  sound/soc/codecs/wsa884x.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/sound/soc/codecs/wsa884x.c b/sound/soc/codecs/wsa884x.c
> index 2484d4b8e2d94..0218dfc13bc77 100644
> --- a/sound/soc/codecs/wsa884x.c
> +++ b/sound/soc/codecs/wsa884x.c
> @@ -1840,7 +1840,6 @@ static const struct snd_soc_dai_ops wsa884x_dai_ops = {
>  	.hw_free = wsa884x_hw_free,
>  	.mute_stream = wsa884x_mute_stream,
>  	.set_stream = wsa884x_set_stream,
> -	.mute_unmute_on_trigger = true,
>  };
>  
>  static struct snd_soc_dai_driver wsa884x_dais[] = {

On 11/24/25 9:08 AM, Srinivas Kandagatla wrote:
> 
> 
> On 11/24/25 6:45 AM, Jonathan Marek wrote:
>> trigger is atomic (non-schedulable), and soundwire register writes are not
>> safe to run in an atomic context. (bus is locked with a mutex, and qcom
>> driver's callback can also sleep if the FIFO is full).
>>
> Thanks Jonathan for the patch,
> 
> We have nonatomic=1 flag set for all the Qualcomm sound cards, Did you
> hit any schedule while atomic bug?
> 

Right, I missed that. I'm using a different driver which does not set 
nonatomic. But this driver to not need nonatomic - 
mute_unmute_on_trigger is a hack, if there is a timing requirement - 
then it needs to be explicit, the different timing with this flag is not 
reliable).

> 
> 
> In-fact this change has helped suppress most of the click and pop noises
> on laptops, specially with wsa codecs as they accumulate static if the
> ports are kept open without sending any data.
> 

28b0b18d5346 is important to fix the click and pop noises. But the 
useful part is the rest of the commit, not the mute_unmute_on_trigger 
flag. As long as the mute_stream() happens while the soundwire stream is 
enabled (between sdw_enable_stream and sdw_disable_stream), there should 
be no pop click.

AFAIK the pop/click is because of PDM: zeros (soundwire stream off) 
represent the minimum (negative maximum) amplitude, and the soundwire 
stream needs to be enabled to output a zero amplitude (alternating 
ones/zeros). Turning on the amp while the soundwire stream is not 
enabled will cause jumps between the minimum and zero amplitude.

> --srini
> 
> 
>> The important part of fixing the click/pop issue was removing the PA_EN
>> writes from the dapm events, AFAICT this flag doesn't help anyway.
>>
>> Fixes: 28b0b18d5346 ("ASoC: codec: wsa884x: make use of new mute_unmute_on_trigger flag")
>> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
>> ---
>>   sound/soc/codecs/wsa884x.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/sound/soc/codecs/wsa884x.c b/sound/soc/codecs/wsa884x.c
>> index 2484d4b8e2d94..0218dfc13bc77 100644
>> --- a/sound/soc/codecs/wsa884x.c
>> +++ b/sound/soc/codecs/wsa884x.c
>> @@ -1840,7 +1840,6 @@ static const struct snd_soc_dai_ops wsa884x_dai_ops = {
>>   	.hw_free = wsa884x_hw_free,
>>   	.mute_stream = wsa884x_mute_stream,
>>   	.set_stream = wsa884x_set_stream,
>> -	.mute_unmute_on_trigger = true,
>>   };
>>   
>>   static struct snd_soc_dai_driver wsa884x_dais[] = {
>

On 11/24/25 2:55 PM, Jonathan Marek wrote:
> On 11/24/25 9:08 AM, Srinivas Kandagatla wrote:
>>
Sorry for the delay, I have been trying to test these patches,

>>
>> On 11/24/25 6:45 AM, Jonathan Marek wrote:
>>> trigger is atomic (non-schedulable), and soundwire register writes
>>> are not
>>> safe to run in an atomic context. (bus is locked with a mutex, and qcom
>>> driver's callback can also sleep if the FIFO is full).
>>>
>> Thanks Jonathan for the patch,
>>
>> We have nonatomic=1 flag set for all the Qualcomm sound cards, Did you
>> hit any schedule while atomic bug?
>>
> 
> Right, I missed that. I'm using a different driver which does not set
> nonatomic. But this driver to not need nonatomic -


Interesting, I guess this is not an upstream driver. The change and log
itself does not make any sense on upstream. pl correct me otherwise.

> mute_unmute_on_trigger is a hack, if there is a timing requirement -
> then it needs to be explicit, the different timing with this flag is not
> reliable).
> 
>>
>>
>> In-fact this change has helped suppress most of the click and pop noises
>> on laptops, specially with wsa codecs as they accumulate static if the
>> ports are kept open without sending any data.
>>
> 
> 28b0b18d5346 is important to fix the click and pop noises. But the
> useful part is the rest of the commit, not the mute_unmute_on_trigger
> flag. As long as the mute_stream() happens while the soundwire stream is
> enabled (between sdw_enable_stream and sdw_disable_stream), there should
> be no pop click.
> 
> AFAIK the pop/click is because of PDM: zeros (soundwire stream off)
> represent the minimum (negative maximum) amplitude, and the soundwire
> stream needs to be enabled to output a zero amplitude (alternating ones/
> zeros). Turning on the amp while the soundwire stream is not enabled
> will cause jumps between the minimum and zero amplitude.
That is true, The issue that enable stream happens at machine driver
prepare and disable happens in hw_params_free.
We have window of opportunity for click and pop between the mute and
enable and in reverse direction.

Am not sure this patch is improving or fixing anything on this side.

Also I noticed during testing on T14s one of the speakers (left) went
into mute, not sure if this is something which is an existing issue but
reverting the patch made it work again.. Need more testing and debugging
to understand what could be going wrong.

I do acknowledge there is still some cases of pop-n-clicks which needs
fixing.


--srini

> 
>> --srini
>>
>>
>>> The important part of fixing the click/pop issue was removing the PA_EN
>>> writes from the dapm events, AFAICT this flag doesn't help anyway.
>>>
>>> Fixes: 28b0b18d5346 ("ASoC: codec: wsa884x: make use of new
>>> mute_unmute_on_trigger flag")
>>> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
>>> ---
>>>   sound/soc/codecs/wsa884x.c | 1 -
>>>   1 file changed, 1 deletion(-)
>>>
>>> diff --git a/sound/soc/codecs/wsa884x.c b/sound/soc/codecs/wsa884x.c
>>> index 2484d4b8e2d94..0218dfc13bc77 100644
>>> --- a/sound/soc/codecs/wsa884x.c
>>> +++ b/sound/soc/codecs/wsa884x.c
>>> @@ -1840,7 +1840,6 @@ static const struct snd_soc_dai_ops
>>> wsa884x_dai_ops = {
>>>       .hw_free = wsa884x_hw_free,
>>>       .mute_stream = wsa884x_mute_stream,
>>>       .set_stream = wsa884x_set_stream,
>>> -    .mute_unmute_on_trigger = true,
>>>   };
>>>     static struct snd_soc_dai_driver wsa884x_dais[] = {
>>

On 11/27/25 5:14 AM, Srinivas Kandagatla wrote:

> 
> 
> Interesting, I guess this is not an upstream driver. The change and log
> itself does not make any sense on upstream. pl correct me otherwise.
> 

> That is true, The issue that enable stream happens at machine driver
> prepare and disable happens in hw_params_free.
> We have window of opportunity for click and pop between the mute and
> enable and in reverse direction.
> 
> Am not sure this patch is improving or fixing anything on this side.
> 

This patch isn't improving or fixing anything (at least upstream),
its removing a hidden (and unnecessary) nonatomic dependency. It 
shouldn't change anything with the pop/click situation. Without 
mute_unmute_on_trigger the mute/unmute still happens inside that "window 
of opportunity".

My driver is not upstream but it could be, and I need atomic trigger. 
You can imagine that I am using the lpass-cpu driver (which is similar 
and upstream), which also does not want nonatomic trigger.

note:
- the common qcom sndcard parsing only sets nonatomic if a "platform" is 
set, which the lpass-cpu dts don't have
- none of the upstream dts that use lpass-cpu use wsa884x/wsa883x, but 
its possible to use these parts with sc7280
- upstream sc7280 uses wcd9385, so using mute_unmute_on_trigger in that 
driver would break upstream sc7280 audio

> Also I noticed during testing on T14s one of the speakers (left) went
> into mute, not sure if this is something which is an existing issue but
> reverting the patch made it work again.. Need more testing and debugging
> to understand what could be going wrong.
> 

This workaround might be relevant:

https://github.com/flto/linux/commit/83b6c10d7c6248b9633a1516b816f211607d4eca

On Mon Nov 24, 2025 at 2:55 PM GMT, Jonathan Marek wrote:
> On 11/24/25 9:08 AM, Srinivas Kandagatla wrote:
>> On 11/24/25 6:45 AM, Jonathan Marek wrote:
>>> trigger is atomic (non-schedulable), and soundwire register writes are not
>>> safe to run in an atomic context. (bus is locked with a mutex, and qcom
>>> driver's callback can also sleep if the FIFO is full).
>>>
>> Thanks Jonathan for the patch,
>> 
>> We have nonatomic=1 flag set for all the Qualcomm sound cards, Did you
>> hit any schedule while atomic bug?
>
> Right, I missed that. I'm using a different driver which does not set 
> nonatomic. But this driver to not need nonatomic - 
> mute_unmute_on_trigger is a hack, if there is a timing requirement - 
> then it needs to be explicit, the different timing with this flag is not 
> reliable).
>
>> In-fact this change has helped suppress most of the click and pop noises
>> on laptops, specially with wsa codecs as they accumulate static if the
>> ports are kept open without sending any data.
>> 
>
> 28b0b18d5346 is important to fix the click and pop noises. But the 
> useful part is the rest of the commit, not the mute_unmute_on_trigger 
> flag. As long as the mute_stream() happens while the soundwire stream is 
> enabled (between sdw_enable_stream and sdw_disable_stream), there should 
> be no pop click.
>
> AFAIK the pop/click is because of PDM: zeros (soundwire stream off) 
> represent the minimum (negative maximum) amplitude, and the soundwire 
> stream needs to be enabled to output a zero amplitude (alternating 
> ones/zeros). Turning on the amp while the soundwire stream is not 
> enabled will cause jumps between the minimum and zero amplitude.

>>> -	.mute_unmute_on_trigger = true,

FWIW for wsa881x in analog mode on RB1/RB2 boards I noticed that pop/clicks
sound behaviour is much better when mute_unmute_on_trigger is false.

Although, for wsa881x in soundwire mode the mute_unmute_on_trigger = true
gives decent result of supressing pop/clicks noise on the devices I have.

Maybe it varies from platform to platform and from board to board.

BR,
Alexey