Skip to contentSkip to conversation

ASoC: qcom: volume fixes and codec cleanups

PATCH Superseded Johan Hovold 1 participant 4 messages Jan 17, 2024

Subsystems (4)

v2 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

v2 cover 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(-)
Signature · 1 line
2.41.0
#1 lore raw

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.

---
 sound/soc/qcom/sc8280xp.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
sound/soc/qcom/sc8280xp.c +5 -3
index ed4bb551bfbb..aa43903421f5 100644
@@ -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;
Signature · 1 line
2.41.0
#2 lore raw

PATCH v2 2/3 ASoC: codecs: lpass-wsa-macro: fix compander volume hack

In reply to Johan Hovold

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 Srinivas Kandagatla <[email protected]>
---
 sound/soc/codecs/lpass-wsa-macro.c | 7 -------
 1 file changed, 7 deletions(-)
sound/soc/codecs/lpass-wsa-macro.c +0 -7
index 7e21cec3c2fb..6ce309980cd1 100644
@@ -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);
Signature · 1 line
2.41.0
#3 lore raw

PATCH v2 3/3 ASoC: codecs: wcd9335: drop unused gain hack remnant

In reply to Johan Hovold

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.

---
 sound/soc/codecs/wcd9335.c | 4 ----
 1 file changed, 4 deletions(-)
sound/soc/codecs/wcd9335.c +0 -4
index 43c648efd0d9..deb15b95992d 100644
@@ -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:
Signature · 1 line
2.41.0