* [PATCH] ASoC: codecs: lpass-rx-macro: Fix playback quality distortion
@ 2025-09-01 7:44 Krzysztof Kozlowski
2025-09-02 2:40 ` Dmitry Baryshkov
0 siblings, 1 reply; 3+ messages in thread
From: Krzysztof Kozlowski @ 2025-09-01 7:44 UTC (permalink / raw)
To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai, Krzysztof Kozlowski, linux-sound, linux-arm-msm,
linux-kernel
Cc: Alexey Klimov
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 "RX_MACRO RX0 MUX"-like kcontrols which use
"rx_macro_mux_text" array. That "rx_macro_mux_text" array used first
three entries of DAI IDs enum, with value '0' being invalid.
The value passed tp "RX_MACRO 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 headset playback on the
Qualcomm SM8750 MTP8750 board. It seems it also fixes recording silence
(instead of actual sound) via headset, even though that's different
macro codec.
Reported-by: Alexey Klimov <alexey.klimov@linaro.org>
Fixes: bb4a0f497bc1 ("ASoC: codecs: lpass: Drop unused AIF_INVALID first DAI identifier")
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Reported via IRC.
Fix for current v6.17-RC cycle.
---
sound/soc/codecs/lpass-rx-macro.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/sound/soc/codecs/lpass-rx-macro.c b/sound/soc/codecs/lpass-rx-macro.c
index 238dbdb46c18..a8fc842cc94e 100644
--- a/sound/soc/codecs/lpass-rx-macro.c
+++ b/sound/soc/codecs/lpass-rx-macro.c
@@ -618,6 +618,7 @@ static struct interp_sample_rate sr_val_tbl[] = {
{176400, 0xB}, {352800, 0xC},
};
+/* Matches also rx_macro_mux_text */
enum {
RX_MACRO_AIF1_PB,
RX_MACRO_AIF2_PB,
@@ -722,6 +723,7 @@ static const char * const rx_int2_2_interp_mux_text[] = {
"ZERO", "RX INT2_2 MUX",
};
+/* Order must match RX_MACRO_MAX_DAIS enum (offset by 1) */
static const char *const rx_macro_mux_text[] = {
"ZERO", "AIF1_PB", "AIF2_PB", "AIF3_PB", "AIF4_PB"
};
@@ -2474,6 +2476,7 @@ static int rx_macro_mux_put(struct snd_kcontrol *kcontrol,
struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
struct snd_soc_dapm_update *update = NULL;
u32 rx_port_value = ucontrol->value.enumerated.item[0];
+ unsigned int dai_id;
u32 aif_rst;
struct rx_macro *rx = snd_soc_component_get_drvdata(component);
@@ -2490,19 +2493,24 @@ static int rx_macro_mux_put(struct snd_kcontrol *kcontrol,
switch (rx_port_value) {
case 0:
- if (rx->active_ch_cnt[aif_rst]) {
- clear_bit(widget->shift,
- &rx->active_ch_mask[aif_rst]);
- rx->active_ch_cnt[aif_rst]--;
+ /*
+ * active_ch_cnt and active_ch_mask use DAI IDs (RX_MACRO_MAX_DAIS).
+ * active_ch_cnt == 0 was tested in if() above.
+ */
+ dai_id = aif_rst - 1;
+ if (rx->active_ch_cnt[dai_id]) {
+ clear_bit(widget->shift, &rx->active_ch_mask[dai_id]);
+ rx->active_ch_cnt[dai_id]--;
}
break;
case 1:
case 2:
case 3:
case 4:
- set_bit(widget->shift,
- &rx->active_ch_mask[rx_port_value]);
- rx->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(widget->shift, &rx->active_ch_mask[dai_id]);
+ rx->active_ch_cnt[dai_id]++;
break;
default:
dev_err(component->dev,
--
2.48.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] ASoC: codecs: lpass-rx-macro: Fix playback quality distortion
2025-09-01 7:44 [PATCH] ASoC: codecs: lpass-rx-macro: Fix playback quality distortion Krzysztof Kozlowski
@ 2025-09-02 2:40 ` Dmitry Baryshkov
2025-09-02 6:08 ` Krzysztof Kozlowski
0 siblings, 1 reply; 3+ messages in thread
From: Dmitry Baryshkov @ 2025-09-02 2:40 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai, linux-sound, linux-arm-msm, linux-kernel,
Alexey Klimov
On Mon, Sep 01, 2025 at 09:44:04AM +0200, 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 "RX_MACRO RX0 MUX"-like kcontrols which use
> "rx_macro_mux_text" array. That "rx_macro_mux_text" array used first
> three entries of DAI IDs enum, with value '0' being invalid.
>
> The value passed tp "RX_MACRO 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 headset playback on the
> Qualcomm SM8750 MTP8750 board. It seems it also fixes recording silence
> (instead of actual sound) via headset, even though that's different
> macro codec.
Wouldn't it be easier to assign 1 to RX_MACRO_AIF1_PB,
TX_MACRO_AIF1_CAP, etc.?
>
> Reported-by: Alexey Klimov <alexey.klimov@linaro.org>
> Fixes: bb4a0f497bc1 ("ASoC: codecs: lpass: Drop unused AIF_INVALID first DAI identifier")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> ---
>
> Reported via IRC.
> Fix for current v6.17-RC cycle.
> ---
> sound/soc/codecs/lpass-rx-macro.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/sound/soc/codecs/lpass-rx-macro.c b/sound/soc/codecs/lpass-rx-macro.c
> index 238dbdb46c18..a8fc842cc94e 100644
> --- a/sound/soc/codecs/lpass-rx-macro.c
> +++ b/sound/soc/codecs/lpass-rx-macro.c
> @@ -618,6 +618,7 @@ static struct interp_sample_rate sr_val_tbl[] = {
> {176400, 0xB}, {352800, 0xC},
> };
>
> +/* Matches also rx_macro_mux_text */
> enum {
> RX_MACRO_AIF1_PB,
> RX_MACRO_AIF2_PB,
> @@ -722,6 +723,7 @@ static const char * const rx_int2_2_interp_mux_text[] = {
> "ZERO", "RX INT2_2 MUX",
> };
>
> +/* Order must match RX_MACRO_MAX_DAIS enum (offset by 1) */
> static const char *const rx_macro_mux_text[] = {
> "ZERO", "AIF1_PB", "AIF2_PB", "AIF3_PB", "AIF4_PB"
> };
> @@ -2474,6 +2476,7 @@ static int rx_macro_mux_put(struct snd_kcontrol *kcontrol,
> struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
> struct snd_soc_dapm_update *update = NULL;
> u32 rx_port_value = ucontrol->value.enumerated.item[0];
> + unsigned int dai_id;
> u32 aif_rst;
> struct rx_macro *rx = snd_soc_component_get_drvdata(component);
>
> @@ -2490,19 +2493,24 @@ static int rx_macro_mux_put(struct snd_kcontrol *kcontrol,
>
> switch (rx_port_value) {
> case 0:
> - if (rx->active_ch_cnt[aif_rst]) {
> - clear_bit(widget->shift,
> - &rx->active_ch_mask[aif_rst]);
> - rx->active_ch_cnt[aif_rst]--;
> + /*
> + * active_ch_cnt and active_ch_mask use DAI IDs (RX_MACRO_MAX_DAIS).
> + * active_ch_cnt == 0 was tested in if() above.
> + */
> + dai_id = aif_rst - 1;
> + if (rx->active_ch_cnt[dai_id]) {
> + clear_bit(widget->shift, &rx->active_ch_mask[dai_id]);
> + rx->active_ch_cnt[dai_id]--;
> }
> break;
> case 1:
> case 2:
> case 3:
> case 4:
> - set_bit(widget->shift,
> - &rx->active_ch_mask[rx_port_value]);
> - rx->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(widget->shift, &rx->active_ch_mask[dai_id]);
> + rx->active_ch_cnt[dai_id]++;
> break;
> default:
> dev_err(component->dev,
> --
> 2.48.1
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ASoC: codecs: lpass-rx-macro: Fix playback quality distortion
2025-09-02 2:40 ` Dmitry Baryshkov
@ 2025-09-02 6:08 ` Krzysztof Kozlowski
0 siblings, 0 replies; 3+ messages in thread
From: Krzysztof Kozlowski @ 2025-09-02 6:08 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai, linux-sound, linux-arm-msm, linux-kernel,
Alexey Klimov
On 02/09/2025 04:40, Dmitry Baryshkov wrote:
> On Mon, Sep 01, 2025 at 09:44:04AM +0200, 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 "RX_MACRO RX0 MUX"-like kcontrols which use
>> "rx_macro_mux_text" array. That "rx_macro_mux_text" array used first
>> three entries of DAI IDs enum, with value '0' being invalid.
>>
>> The value passed tp "RX_MACRO 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 headset playback on the
>> Qualcomm SM8750 MTP8750 board. It seems it also fixes recording silence
>> (instead of actual sound) via headset, even though that's different
>> macro codec.
>
> Wouldn't it be easier to assign 1 to RX_MACRO_AIF1_PB,
> TX_MACRO_AIF1_CAP, etc.?
That would be basically revert of mentioned commit, so same arguments as
I used in that commit.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-09-02 6:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-01 7:44 [PATCH] ASoC: codecs: lpass-rx-macro: Fix playback quality distortion Krzysztof Kozlowski
2025-09-02 2:40 ` Dmitry Baryshkov
2025-09-02 6:08 ` Krzysztof Kozlowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).