linux-kernel - Re: [PATCH] ASoC: codecs: lpass-wsa-macro: Fix speaker quality distortion

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0beec691-a21c-4425-b06b-728d55e7e254@oss.qualcomm.com>
Date: Thu, 4 Sep 2025 08:24:11 +0100
From: Srinivas Kandagatla <srinivas.kandagatla@....qualcomm.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        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>,
        linux-sound@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc: Alexey Klimov <alexey.klimov@...aro.org>
Subject: Re: [PATCH] ASoC: codecs: lpass-wsa-macro: Fix speaker quality
 distortion



On 8/31/25 4:14 PM, Krzysztof Kozlowski wrote:
> Commit bb4a0f497bc1 ("ASoC: codecs: lpass: Drop unused
> AIF_INVALID first DAI identifier") removed first entry in enum with DAI
> identifiers, because it looked unused.  Turns out that there is a
> relation between DAI ID and "WSA RX0 Mux"-like kcontrols (which use
> "rx_mux_text" array).  That "rx_mux_text" array used first three entries
> of DAI IDs enum, with value '0' being invalid.
> 
> The value passed tp "WSA RX0 Mux"-like kcontrols was used as DAI ID and
> set to configure active channel count and mask, which are arrays indexed
> by DAI ID.
> 
> After removal of first AIF_INVALID DAI identifier, this kcontrol was
> updating wrong entries in active channel count and mask arrays which was
> visible in reduced quality (distortions) during speaker playback on
> several boards like Lenovo T14s laptop and Qualcomm SM8550-based boards.
> 
> Reported-by: Alexey Klimov <alexey.klimov@...aro.org>
> Fixes: bb4a0f497bc1 ("ASoC: codecs: lpass: Drop unused AIF_INVALID first DAI identifier")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
> 
Thanks for fixing this,

tested this on T14s, WSA speakers, VA DMICs, RX and TX on headphones..
everything seems to work fine.

Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@....qualcomm.com>
Tested-by: Srinivas Kandagatla <srinivas.kandagatla@....qualcomm.com>


--srini



> ---
> 
> Reported via IRC.
> Fix for current v6.17-RC cycle.
> 
> I will be investigating rest of lpass macro codecs a bit later.
> ---
>  sound/soc/codecs/lpass-wsa-macro.c | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
> index da6adb3de21d..d7eec9fdaf9c 100644
> --- a/sound/soc/codecs/lpass-wsa-macro.c
> +++ b/sound/soc/codecs/lpass-wsa-macro.c
> @@ -368,6 +368,7 @@ static struct interp_sample_rate int_mix_sample_rate_val[] = {
>  	{192000, 0x6},	/* 192K */
>  };
>  
> +/* Matches also rx_mux_text */
>  enum {
>  	WSA_MACRO_AIF1_PB,
>  	WSA_MACRO_AIF_MIX1_PB,
> @@ -465,6 +466,7 @@ static const char *const rx_mix_ec_text[] = {
>  	"ZERO", "RX_MIX_TX0", "RX_MIX_TX1"
>  };
>  
> +/* Order must match WSA_MACRO_MAX_DAIS enum (offset by 1) */
>  static const char *const rx_mux_text[] = {
>  	"ZERO", "AIF1_PB", "AIF_MIX1_PB"
>  };
> @@ -2207,6 +2209,7 @@ static int wsa_macro_rx_mux_put(struct snd_kcontrol *kcontrol,
>  	u32 rx_port_value = ucontrol->value.integer.value[0];
>  	u32 bit_input;
>  	u32 aif_rst;
> +	unsigned int dai_id;
>  	struct wsa_macro *wsa = snd_soc_component_get_drvdata(component);
>  
>  	aif_rst = wsa->rx_port_value[widget->shift];
> @@ -2224,17 +2227,22 @@ static int wsa_macro_rx_mux_put(struct snd_kcontrol *kcontrol,
>  
>  	switch (rx_port_value) {
>  	case 0:
> -		if (wsa->active_ch_cnt[aif_rst]) {
> -			clear_bit(bit_input,
> -				  &wsa->active_ch_mask[aif_rst]);
> -			wsa->active_ch_cnt[aif_rst]--;
> +		/*
> +		 * active_ch_cnt and active_ch_mask use DAI IDs (WSA_MACRO_MAX_DAIS).
> +		 * active_ch_cnt == 0 was tested in if() above.
> +		 */
> +		dai_id = aif_rst - 1;
> +		if (wsa->active_ch_cnt[dai_id]) {
> +			clear_bit(bit_input, &wsa->active_ch_mask[dai_id]);
> +			wsa->active_ch_cnt[dai_id]--;
>  		}
>  		break;
>  	case 1:
>  	case 2:
> -		set_bit(bit_input,
> -			&wsa->active_ch_mask[rx_port_value]);
> -		wsa->active_ch_cnt[rx_port_value]++;
> +		/* active_ch_cnt and active_ch_mask use DAI IDs (WSA_MACRO_MAX_DAIS). */
> +		dai_id = rx_port_value - 1;
> +		set_bit(bit_input, &wsa->active_ch_mask[dai_id]);
> +		wsa->active_ch_cnt[dai_id]++;
>  		break;
>  	default:
>  		dev_err(component->dev,