[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fafe5de2-f682-4769-bb1f-166858808746@oss.qualcomm.com>
Date: Thu, 27 Nov 2025 10:14:47 +0000
From: Srinivas Kandagatla <srinivas.kandagatla@....qualcomm.com>
To: Jonathan Marek <jonathan@...ek.ca>,
Srinivas Kandagatla <srinivas.kandagatla@....qualcomm.com>,
linux-arm-msm@...r.kernel.org
Cc: Srinivas Kandagatla <srini@...nel.org>,
Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
Krzysztof Kozlowski <krzk@...nel.org>,
Neil Armstrong <neil.armstrong@...aro.org>,
"open list:QCOM AUDIO (ASoC) DRIVERS" <linux-sound@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/6] ASoC: codecs: wsa884x: remove mute_unmute_on_trigger
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@...ek.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[] = {
>>
Powered by blists - more mailing lists