2024-01-17 09:04:18

by Johan Hovold

[permalink] [raw]
Subject: [PATCH v2 0/3] ASoC: qcom: volume fixes and codec cleanups

To reduce the risk of speaker damage the PA gain needs to be limited on
machines like the Lenovo Thinkpad X13s until we have active speaker
protection in place.

Limit the gain to the current default setting provided by the UCM
configuration which most user have so far been using (due to a bug in
the configuration files which prevented hardware volume control [1]).

Included is also a related fix for the LPASS WSA macro driver, which
was changing the digital gain setting behind the back of user space and
which can result in excessive (or too low) digital gain.

There are further Qualcomm codec drivers that similarly appear to
manipulate various gain settings, but on closer inspection it turns out
that they only write back the current settings. Tests reveal that these
writes are indeed needed for any prior updates to take effect (at least
for the WSA and RX macros).

Johan

[1] https://github.com/alsa-project/alsa-ucm-conf/pull/382


Changes in v2
- keep the volume register write on power-on in lpass-wsa-macro
- drop the other patches removing volume register writes on DAPM events
- only drop the constant-zero gain offsets in wcd9335


Johan Hovold (3):
ASoC: qcom: sc8280xp: limit speaker volumes
ASoC: codecs: lpass-wsa-macro: fix compander volume hack
ASoC: codecs: wcd9335: drop unused gain hack remnant

sound/soc/codecs/lpass-wsa-macro.c | 7 -------
sound/soc/codecs/wcd9335.c | 4 ----
sound/soc/qcom/sc8280xp.c | 8 +++++---
3 files changed, 5 insertions(+), 14 deletions(-)

--
2.41.0



2024-01-17 09:04:18

by Johan Hovold

[permalink] [raw]
Subject: [PATCH v2 3/3] ASoC: codecs: wcd9335: drop unused gain hack remnant

The vendor driver appears to be modifying the gain settings behind the
back of user space but these hacks never made it upstream except for
some essentially dead code that adds a constant zero to the current
settings on DAPM events.

Note that the volume registers still need to be written after enabling
clocks in order for any prior updates to take effect.

Signed-off-by: Johan Hovold <[email protected]>
---
sound/soc/codecs/wcd9335.c | 4 ----
1 file changed, 4 deletions(-)

diff --git a/sound/soc/codecs/wcd9335.c b/sound/soc/codecs/wcd9335.c
index 43c648efd0d9..deb15b95992d 100644
--- a/sound/soc/codecs/wcd9335.c
+++ b/sound/soc/codecs/wcd9335.c
@@ -3033,7 +3033,6 @@ static int wcd9335_codec_enable_mix_path(struct snd_soc_dapm_widget *w,
{
struct snd_soc_component *comp = snd_soc_dapm_to_component(w->dapm);
u16 gain_reg;
- int offset_val = 0;
int val = 0;

switch (w->reg) {
@@ -3073,7 +3072,6 @@ static int wcd9335_codec_enable_mix_path(struct snd_soc_dapm_widget *w,
switch (event) {
case SND_SOC_DAPM_POST_PMU:
val = snd_soc_component_read(comp, gain_reg);
- val += offset_val;
snd_soc_component_write(comp, gain_reg, val);
break;
case SND_SOC_DAPM_POST_PMD:
@@ -3294,7 +3292,6 @@ static int wcd9335_codec_enable_interpolator(struct snd_soc_dapm_widget *w,
u16 gain_reg;
u16 reg;
int val;
- int offset_val = 0;

if (!(snd_soc_dapm_widget_name_cmp(w, "RX INT0 INTERP"))) {
reg = WCD9335_CDC_RX0_RX_PATH_CTL;
@@ -3337,7 +3334,6 @@ static int wcd9335_codec_enable_interpolator(struct snd_soc_dapm_widget *w,
case SND_SOC_DAPM_POST_PMU:
wcd9335_config_compander(comp, w->shift, event);
val = snd_soc_component_read(comp, gain_reg);
- val += offset_val;
snd_soc_component_write(comp, gain_reg, val);
break;
case SND_SOC_DAPM_POST_PMD:
--
2.41.0


2024-01-17 09:04:19

by Johan Hovold

[permalink] [raw]
Subject: [PATCH v2 1/3] ASoC: qcom: sc8280xp: limit speaker volumes

The current UCM configuration sets the speaker PA volume to 15 dB when
enabling the speakers but this does not prevent the user from increasing
the volume further.

Limit the PA volume to 15 dB in the machine driver to reduce the risk of
speaker damage until we have active speaker protection in place.

Note that this will probably need to be generalised using
machine-specific limits, but a common limit should do for now.

Cc: [email protected] # 6.5
Reviewed-by: Srinivas Kandagatla <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
sound/soc/qcom/sc8280xp.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/sound/soc/qcom/sc8280xp.c b/sound/soc/qcom/sc8280xp.c
index ed4bb551bfbb..aa43903421f5 100644
--- a/sound/soc/qcom/sc8280xp.c
+++ b/sound/soc/qcom/sc8280xp.c
@@ -32,12 +32,14 @@ static int sc8280xp_snd_init(struct snd_soc_pcm_runtime *rtd)
case WSA_CODEC_DMA_RX_0:
case WSA_CODEC_DMA_RX_1:
/*
- * set limit of 0dB on Digital Volume for Speakers,
- * this can prevent damage of speakers to some extent without
- * active speaker protection
+ * Set limit of 0 dB on Digital Volume and 15 dB on PA Volume
+ * to reduce the risk of speaker damage until we have active
+ * speaker protection in place.
*/
snd_soc_limit_volume(card, "WSA_RX0 Digital Volume", 84);
snd_soc_limit_volume(card, "WSA_RX1 Digital Volume", 84);
+ snd_soc_limit_volume(card, "SpkrLeft PA Volume", 12);
+ snd_soc_limit_volume(card, "SpkrRight PA Volume", 12);
break;
default:
break;
--
2.41.0


2024-01-17 09:04:25

by Johan Hovold

[permalink] [raw]
Subject: [PATCH v2 2/3] ASoC: codecs: lpass-wsa-macro: fix compander volume hack

The LPASS WSA macro codec driver is updating the digital gain settings
behind the back of user space on DAPM events if companding has been
enabled.

As compander control is exported to user space, this can result in the
digital gain setting being incremented (or decremented) every time the
sound server is started and the codec suspended depending on what the
UCM configuration looks like.

Soon enough playback will become distorted (or too quiet).

This is specifically a problem on the Lenovo ThinkPad X13s as this
bypasses the limit for the digital gain setting that has been set by the
machine driver.

Fix this by simply dropping the compander gain offset hack. If someone
cares about modelling the impact of the compander setting this can
possibly be done by exporting it as a volume control later.

Note that the volume registers still need to be written after enabling
clocks in order for any prior updates to take effect.

Fixes: 2c4066e5d428 ("ASoC: codecs: lpass-wsa-macro: add dapm widgets and route")
Cc: [email protected] # 5.11
Cc: Srinivas Kandagatla <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
sound/soc/codecs/lpass-wsa-macro.c | 7 -------
1 file changed, 7 deletions(-)

diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
index 7e21cec3c2fb..6ce309980cd1 100644
--- a/sound/soc/codecs/lpass-wsa-macro.c
+++ b/sound/soc/codecs/lpass-wsa-macro.c
@@ -1584,7 +1584,6 @@ static int wsa_macro_enable_interpolator(struct snd_soc_dapm_widget *w,
u16 gain_reg;
u16 reg;
int val;
- int offset_val = 0;
struct wsa_macro *wsa = snd_soc_component_get_drvdata(component);

if (w->shift == WSA_MACRO_COMP1) {
@@ -1623,10 +1622,8 @@ static int wsa_macro_enable_interpolator(struct snd_soc_dapm_widget *w,
CDC_WSA_RX1_RX_PATH_MIX_SEC0,
CDC_WSA_RX_PGA_HALF_DB_MASK,
CDC_WSA_RX_PGA_HALF_DB_ENABLE);
- offset_val = -2;
}
val = snd_soc_component_read(component, gain_reg);
- val += offset_val;
snd_soc_component_write(component, gain_reg, val);
wsa_macro_config_ear_spkr_gain(component, wsa,
event, gain_reg);
@@ -1654,10 +1651,6 @@ static int wsa_macro_enable_interpolator(struct snd_soc_dapm_widget *w,
CDC_WSA_RX1_RX_PATH_MIX_SEC0,
CDC_WSA_RX_PGA_HALF_DB_MASK,
CDC_WSA_RX_PGA_HALF_DB_DISABLE);
- offset_val = 2;
- val = snd_soc_component_read(component, gain_reg);
- val += offset_val;
- snd_soc_component_write(component, gain_reg, val);
}
wsa_macro_config_ear_spkr_gain(component, wsa,
event, gain_reg);
--
2.41.0