[PATCH 0/7] ASoC: qcom: volume fixes and codec cleanups

[PATCH 0/7] ASoC: qcom: volume fixes and codec cleanups

[Johan Hovold]

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 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 driver that appear to manipulate
various gain settings, but on closer inspection this turned out to be
effectively dead code which can be removed.

Johan

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


Johan Hovold (7):
  ASoC: qcom: sc8280xp: limit speaker volumes
  ASoC: codecs: lpass-wsa-macro: fix compander volume hack
  ASoC: codecs: lpass-wsa-macro: drop dead mixer-path gain hack
  ASoC: codecs: lpass-rx-macro: drop dead mixer-path gain hack
  ASoC: codecs: wcd9335: drop dead gain hacks
  ASoC: codecs: wcd934x: drop dead gain hacks
  ASoC: codecs: msm8916-wcd-digital: drop dead gain hacks

 sound/soc/codecs/lpass-rx-macro.c      |  16 +---
 sound/soc/codecs/lpass-wsa-macro.c     |  19 +---
 sound/soc/codecs/msm8916-wcd-digital.c |  26 +-----
 sound/soc/codecs/wcd9335.c             | 115 ++++---------------------
 sound/soc/codecs/wcd934x.c             | 102 +++++-----------------
 sound/soc/qcom/sc8280xp.c              |   8 +-
 6 files changed, 53 insertions(+), 233 deletions(-)

-- 
2.41.0

[PATCH 1/7] ASoC: qcom: sc8280xp: limit speaker volumes

[Johan Hovold]

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: stable@vger.kernel.org	# 6.5
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 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

[PATCH 2/7] ASoC: codecs: lpass-wsa-macro: fix compander volume hack

[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 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.

Fixes: 2c4066e5d428 ("ASoC: codecs: lpass-wsa-macro: add dapm widgets and route")
Cc: stable@vger.kernel.org      # 5.11
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 sound/soc/codecs/lpass-wsa-macro.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
index 7e21cec3c2fb..7de221464d47 100644
--- a/sound/soc/codecs/lpass-wsa-macro.c
+++ b/sound/soc/codecs/lpass-wsa-macro.c
@@ -1583,8 +1583,6 @@ static int wsa_macro_enable_interpolator(struct snd_soc_dapm_widget *w,
 	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
 	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,11 +1621,7 @@ 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);
 		break;
@@ -1654,10 +1648,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

[PATCH 3/7] ASoC: codecs: lpass-wsa-macro: drop dead mixer-path gain hack

[Johan Hovold]

The vendor driver modifies the gain settings behind the back of user
space but some of these hacks never made it upstream except for some
essentially dead code that reads out the (cached) gain setting and
writes it back again on DAPM events.

Drop the incomplete and pointless hack when enabling mixer paths.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 sound/soc/codecs/lpass-wsa-macro.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
index 7de221464d47..36a8f1980c6e 100644
--- a/sound/soc/codecs/lpass-wsa-macro.c
+++ b/sound/soc/codecs/lpass-wsa-macro.c
@@ -1220,27 +1220,20 @@ static int wsa_macro_enable_mix_path(struct snd_soc_dapm_widget *w,
 				     struct snd_kcontrol *kcontrol, int event)
 {
 	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
-	u16 path_reg, gain_reg;
-	int val;
+	u16 path_reg;
 
 	switch (w->shift) {
 	case WSA_MACRO_RX_MIX0:
 		path_reg = CDC_WSA_RX0_RX_PATH_MIX_CTL;
-		gain_reg = CDC_WSA_RX0_RX_VOL_MIX_CTL;
 		break;
 	case WSA_MACRO_RX_MIX1:
 		path_reg = CDC_WSA_RX1_RX_PATH_MIX_CTL;
-		gain_reg = CDC_WSA_RX1_RX_VOL_MIX_CTL;
 		break;
 	default:
 		return 0;
 	}
 
 	switch (event) {
-	case SND_SOC_DAPM_POST_PMU:
-		val = snd_soc_component_read(component, gain_reg);
-		snd_soc_component_write(component, gain_reg, val);
-		break;
 	case SND_SOC_DAPM_POST_PMD:
 		snd_soc_component_update_bits(component, path_reg,
 					      CDC_WSA_RX_PATH_MIX_CLK_EN_MASK,
-- 
2.41.0

[PATCH 4/7] ASoC: codecs: lpass-rx-macro: drop dead mixer-path gain hack

[Johan Hovold]

The vendor driver modifies the gain settings behind the back of user
space but some of these hacks never made it upstream except for some
essentially dead code that reads out the (cached) gain setting and
writes it back again on DAPM events.

Drop the incomplete and pointless hack when enabling mixer paths.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 sound/soc/codecs/lpass-rx-macro.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/sound/soc/codecs/lpass-rx-macro.c b/sound/soc/codecs/lpass-rx-macro.c
index f35187d69cac..b1ec41eed851 100644
--- a/sound/soc/codecs/lpass-rx-macro.c
+++ b/sound/soc/codecs/lpass-rx-macro.c
@@ -1956,10 +1956,9 @@ static int rx_macro_enable_main_path(struct snd_soc_dapm_widget *w,
 					int event)
 {
 	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
-	u16 gain_reg, reg;
+	u16 reg;
 
 	reg = CDC_RX_RXn_RX_PATH_CTL(w->shift);
-	gain_reg = CDC_RX_RXn_RX_VOL_CTL(w->shift);
 
 	switch (event) {
 	case SND_SOC_DAPM_PRE_PMU:
@@ -1969,10 +1968,6 @@ static int rx_macro_enable_main_path(struct snd_soc_dapm_widget *w,
 						      CDC_RX_PATH_CLK_EN_MASK,
 						      CDC_RX_PATH_CLK_ENABLE);
 		break;
-	case SND_SOC_DAPM_POST_PMU:
-		snd_soc_component_write(component, gain_reg,
-			snd_soc_component_read(component, gain_reg));
-		break;
 	case SND_SOC_DAPM_POST_PMD:
 		rx_macro_enable_interp_clk(component, event, w->shift);
 		break;
@@ -3031,16 +3026,13 @@ static const struct snd_soc_dapm_widget rx_macro_dapm_widgets[] = {
 
 	SND_SOC_DAPM_MUX_E("RX INT0_1 INTERP", SND_SOC_NOPM, INTERP_HPHL, 0,
 		&rx_int0_1_interp_mux, rx_macro_enable_main_path,
-		SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU |
-		SND_SOC_DAPM_POST_PMD),
+		SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
 	SND_SOC_DAPM_MUX_E("RX INT1_1 INTERP", SND_SOC_NOPM, INTERP_HPHR, 0,
 		&rx_int1_1_interp_mux, rx_macro_enable_main_path,
-		SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU |
-		SND_SOC_DAPM_POST_PMD),
+		SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
 	SND_SOC_DAPM_MUX_E("RX INT2_1 INTERP", SND_SOC_NOPM, INTERP_AUX, 0,
 		&rx_int2_1_interp_mux, rx_macro_enable_main_path,
-		SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU |
-		SND_SOC_DAPM_POST_PMD),
+		SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
 
 	SND_SOC_DAPM_MUX("RX INT0_2 INTERP", SND_SOC_NOPM, 0, 0,
 			 &rx_int0_2_interp_mux),
-- 
2.41.0

[PATCH 5/7] ASoC: codecs: wcd9335: drop dead gain hacks

[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 reads out the (cached) gain setting and
writes it back again on DAPM events.

Drop these incomplete and pointless hacks.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 sound/soc/codecs/wcd9335.c | 115 ++++++-------------------------------
 1 file changed, 18 insertions(+), 97 deletions(-)

diff --git a/sound/soc/codecs/wcd9335.c b/sound/soc/codecs/wcd9335.c
index 43c648efd0d9..cee17b309160 100644
--- a/sound/soc/codecs/wcd9335.c
+++ b/sound/soc/codecs/wcd9335.c
@@ -3028,61 +3028,6 @@ static int wcd9335_codec_enable_slim(struct snd_soc_dapm_widget *w,
 	return 0;
 }
 
-static int wcd9335_codec_enable_mix_path(struct snd_soc_dapm_widget *w,
-		struct snd_kcontrol *kc, int event)
-{
-	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) {
-	case WCD9335_CDC_RX0_RX_PATH_MIX_CTL:
-		gain_reg = WCD9335_CDC_RX0_RX_VOL_MIX_CTL;
-		break;
-	case WCD9335_CDC_RX1_RX_PATH_MIX_CTL:
-		gain_reg = WCD9335_CDC_RX1_RX_VOL_MIX_CTL;
-		break;
-	case WCD9335_CDC_RX2_RX_PATH_MIX_CTL:
-		gain_reg = WCD9335_CDC_RX2_RX_VOL_MIX_CTL;
-		break;
-	case WCD9335_CDC_RX3_RX_PATH_MIX_CTL:
-		gain_reg = WCD9335_CDC_RX3_RX_VOL_MIX_CTL;
-		break;
-	case WCD9335_CDC_RX4_RX_PATH_MIX_CTL:
-		gain_reg = WCD9335_CDC_RX4_RX_VOL_MIX_CTL;
-		break;
-	case WCD9335_CDC_RX5_RX_PATH_MIX_CTL:
-		gain_reg = WCD9335_CDC_RX5_RX_VOL_MIX_CTL;
-		break;
-	case WCD9335_CDC_RX6_RX_PATH_MIX_CTL:
-		gain_reg = WCD9335_CDC_RX6_RX_VOL_MIX_CTL;
-		break;
-	case WCD9335_CDC_RX7_RX_PATH_MIX_CTL:
-		gain_reg = WCD9335_CDC_RX7_RX_VOL_MIX_CTL;
-		break;
-	case WCD9335_CDC_RX8_RX_PATH_MIX_CTL:
-		gain_reg = WCD9335_CDC_RX8_RX_VOL_MIX_CTL;
-		break;
-	default:
-		dev_err(comp->dev, "%s: No gain register avail for %s\n",
-			__func__, w->name);
-		return 0;
-	}
-
-	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:
-		break;
-	}
-
-	return 0;
-}
-
 static u16 wcd9335_interp_get_primary_reg(u16 reg, u16 *ind)
 {
 	u16 prim_int_reg = WCD9335_CDC_RX0_RX_PATH_CTL;
@@ -3291,38 +3236,26 @@ static int wcd9335_codec_enable_interpolator(struct snd_soc_dapm_widget *w,
 		struct snd_kcontrol *kc, int event)
 {
 	struct snd_soc_component *comp = snd_soc_dapm_to_component(w->dapm);
-	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;
-		gain_reg = WCD9335_CDC_RX0_RX_VOL_CTL;
 	} else if (!(snd_soc_dapm_widget_name_cmp(w, "RX INT1 INTERP"))) {
 		reg = WCD9335_CDC_RX1_RX_PATH_CTL;
-		gain_reg = WCD9335_CDC_RX1_RX_VOL_CTL;
 	} else if (!(snd_soc_dapm_widget_name_cmp(w, "RX INT2 INTERP"))) {
 		reg = WCD9335_CDC_RX2_RX_PATH_CTL;
-		gain_reg = WCD9335_CDC_RX2_RX_VOL_CTL;
 	} else if (!(snd_soc_dapm_widget_name_cmp(w, "RX INT3 INTERP"))) {
 		reg = WCD9335_CDC_RX3_RX_PATH_CTL;
-		gain_reg = WCD9335_CDC_RX3_RX_VOL_CTL;
 	} else if (!(snd_soc_dapm_widget_name_cmp(w, "RX INT4 INTERP"))) {
 		reg = WCD9335_CDC_RX4_RX_PATH_CTL;
-		gain_reg = WCD9335_CDC_RX4_RX_VOL_CTL;
 	} else if (!(snd_soc_dapm_widget_name_cmp(w, "RX INT5 INTERP"))) {
 		reg = WCD9335_CDC_RX5_RX_PATH_CTL;
-		gain_reg = WCD9335_CDC_RX5_RX_VOL_CTL;
 	} else if (!(snd_soc_dapm_widget_name_cmp(w, "RX INT6 INTERP"))) {
 		reg = WCD9335_CDC_RX6_RX_PATH_CTL;
-		gain_reg = WCD9335_CDC_RX6_RX_VOL_CTL;
 	} else if (!(snd_soc_dapm_widget_name_cmp(w, "RX INT7 INTERP"))) {
 		reg = WCD9335_CDC_RX7_RX_PATH_CTL;
-		gain_reg = WCD9335_CDC_RX7_RX_VOL_CTL;
 	} else if (!(snd_soc_dapm_widget_name_cmp(w, "RX INT8 INTERP"))) {
 		reg = WCD9335_CDC_RX8_RX_PATH_CTL;
-		gain_reg = WCD9335_CDC_RX8_RX_VOL_CTL;
 	} else {
 		dev_err(comp->dev, "%s: Interpolator reg not found\n",
 			__func__);
@@ -3336,9 +3269,6 @@ static int wcd9335_codec_enable_interpolator(struct snd_soc_dapm_widget *w,
 		break;
 	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:
 		wcd9335_config_compander(comp, w->shift, event);
@@ -4367,33 +4297,24 @@ static const struct snd_soc_dapm_widget wcd9335_dapm_widgets[] = {
 	SND_SOC_DAPM_MIXER("SLIM RX5", SND_SOC_NOPM, 0, 0, NULL, 0),
 	SND_SOC_DAPM_MIXER("SLIM RX6", SND_SOC_NOPM, 0, 0, NULL, 0),
 	SND_SOC_DAPM_MIXER("SLIM RX7", SND_SOC_NOPM, 0, 0, NULL, 0),
-	SND_SOC_DAPM_MUX_E("RX INT0_2 MUX", WCD9335_CDC_RX0_RX_PATH_MIX_CTL,
-			5, 0, &rx_int0_2_mux, wcd9335_codec_enable_mix_path,
-			SND_SOC_DAPM_POST_PMU),
-	SND_SOC_DAPM_MUX_E("RX INT1_2 MUX", WCD9335_CDC_RX1_RX_PATH_MIX_CTL,
-			5, 0, &rx_int1_2_mux, wcd9335_codec_enable_mix_path,
-			SND_SOC_DAPM_POST_PMU),
-	SND_SOC_DAPM_MUX_E("RX INT2_2 MUX", WCD9335_CDC_RX2_RX_PATH_MIX_CTL,
-			5, 0, &rx_int2_2_mux, wcd9335_codec_enable_mix_path,
-			SND_SOC_DAPM_POST_PMU),
-	SND_SOC_DAPM_MUX_E("RX INT3_2 MUX", WCD9335_CDC_RX3_RX_PATH_MIX_CTL,
-			5, 0, &rx_int3_2_mux, wcd9335_codec_enable_mix_path,
-			SND_SOC_DAPM_POST_PMU),
-	SND_SOC_DAPM_MUX_E("RX INT4_2 MUX", WCD9335_CDC_RX4_RX_PATH_MIX_CTL,
-			5, 0, &rx_int4_2_mux, wcd9335_codec_enable_mix_path,
-			SND_SOC_DAPM_POST_PMU),
-	SND_SOC_DAPM_MUX_E("RX INT5_2 MUX", WCD9335_CDC_RX5_RX_PATH_MIX_CTL,
-			5, 0, &rx_int5_2_mux, wcd9335_codec_enable_mix_path,
-			SND_SOC_DAPM_POST_PMU),
-	SND_SOC_DAPM_MUX_E("RX INT6_2 MUX", WCD9335_CDC_RX6_RX_PATH_MIX_CTL,
-			5, 0, &rx_int6_2_mux, wcd9335_codec_enable_mix_path,
-			SND_SOC_DAPM_POST_PMU),
-	SND_SOC_DAPM_MUX_E("RX INT7_2 MUX", WCD9335_CDC_RX7_RX_PATH_MIX_CTL,
-			5, 0, &rx_int7_2_mux, wcd9335_codec_enable_mix_path,
-			SND_SOC_DAPM_POST_PMU),
-	SND_SOC_DAPM_MUX_E("RX INT8_2 MUX", WCD9335_CDC_RX8_RX_PATH_MIX_CTL,
-			5, 0, &rx_int8_2_mux, wcd9335_codec_enable_mix_path,
-			SND_SOC_DAPM_POST_PMU),
+	SND_SOC_DAPM_MUX("RX INT0_2 MUX", WCD9335_CDC_RX0_RX_PATH_MIX_CTL,
+			5, 0, &rx_int0_2_mux),
+	SND_SOC_DAPM_MUX("RX INT1_2 MUX", WCD9335_CDC_RX1_RX_PATH_MIX_CTL,
+			5, 0, &rx_int1_2_mux),
+	SND_SOC_DAPM_MUX("RX INT2_2 MUX", WCD9335_CDC_RX2_RX_PATH_MIX_CTL,
+			5, 0, &rx_int2_2_mux),
+	SND_SOC_DAPM_MUX("RX INT3_2 MUX", WCD9335_CDC_RX3_RX_PATH_MIX_CTL,
+			5, 0, &rx_int3_2_mux),
+	SND_SOC_DAPM_MUX("RX INT4_2 MUX", WCD9335_CDC_RX4_RX_PATH_MIX_CTL,
+			5, 0, &rx_int4_2_mux),
+	SND_SOC_DAPM_MUX("RX INT5_2 MUX", WCD9335_CDC_RX5_RX_PATH_MIX_CTL,
+			5, 0, &rx_int5_2_mux),
+	SND_SOC_DAPM_MUX("RX INT6_2 MUX", WCD9335_CDC_RX6_RX_PATH_MIX_CTL,
+			5, 0, &rx_int6_2_mux),
+	SND_SOC_DAPM_MUX("RX INT7_2 MUX", WCD9335_CDC_RX7_RX_PATH_MIX_CTL,
+			5, 0, &rx_int7_2_mux),
+	SND_SOC_DAPM_MUX("RX INT8_2 MUX", WCD9335_CDC_RX8_RX_PATH_MIX_CTL,
+			5, 0, &rx_int8_2_mux),
 	SND_SOC_DAPM_MUX("RX INT0_1 MIX1 INP0", SND_SOC_NOPM, 0, 0,
 		&rx_int0_1_mix_inp0_mux),
 	SND_SOC_DAPM_MUX("RX INT0_1 MIX1 INP1", SND_SOC_NOPM, 0, 0,
-- 
2.41.0

[PATCH 6/7] ASoC: codecs: wcd934x: drop dead gain hacks

[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 reads out the gain and writes it back
again on DAPM events.

Drop these incomplete and pointless hacks.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 sound/soc/codecs/wcd934x.c | 102 ++++++++-----------------------------
 1 file changed, 22 insertions(+), 80 deletions(-)

diff --git a/sound/soc/codecs/wcd934x.c b/sound/soc/codecs/wcd934x.c
index 1b6e376f3833..34e97d39a146 100644
--- a/sound/soc/codecs/wcd934x.c
+++ b/sound/soc/codecs/wcd934x.c
@@ -4357,12 +4357,8 @@ static int wcd934x_codec_enable_mix_path(struct snd_soc_dapm_widget *w,
 					 struct snd_kcontrol *kc, int event)
 {
 	struct snd_soc_component *comp = snd_soc_dapm_to_component(w->dapm);
-	int offset_val = 0;
-	u16 gain_reg, mix_reg;
-	int val = 0;
+	u16 mix_reg;
 
-	gain_reg = WCD934X_CDC_RX0_RX_VOL_MIX_CTL +
-					(w->shift * WCD934X_RX_PATH_CTL_OFFSET);
 	mix_reg = WCD934X_CDC_RX0_RX_PATH_MIX_CTL +
 					(w->shift * WCD934X_RX_PATH_CTL_OFFSET);
 
@@ -4373,12 +4369,6 @@ static int wcd934x_codec_enable_mix_path(struct snd_soc_dapm_widget *w,
 					      WCD934X_CDC_RX_MIX_CLK_EN_MASK,
 					      WCD934X_CDC_RX_MIX_CLK_ENABLE);
 		break;
-
-	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;
 	}
 
 	return 0;
@@ -4418,26 +4408,6 @@ static int wcd934x_codec_set_iir_gain(struct snd_soc_dapm_widget *w,
 	return 0;
 }
 
-static int wcd934x_codec_enable_main_path(struct snd_soc_dapm_widget *w,
-					  struct snd_kcontrol *kcontrol,
-					  int event)
-{
-	struct snd_soc_component *comp = snd_soc_dapm_to_component(w->dapm);
-	u16 gain_reg;
-
-	gain_reg = WCD934X_CDC_RX0_RX_VOL_CTL + (w->shift *
-						 WCD934X_RX_PATH_CTL_OFFSET);
-
-	switch (event) {
-	case SND_SOC_DAPM_POST_PMU:
-		snd_soc_component_write(comp, gain_reg,
-				snd_soc_component_read(comp, gain_reg));
-		break;
-	}
-
-	return 0;
-}
-
 static int wcd934x_codec_ear_dac_event(struct snd_soc_dapm_widget *w,
 				       struct snd_kcontrol *kc, int event)
 {
@@ -5238,32 +5208,25 @@ static const struct snd_soc_dapm_widget wcd934x_dapm_widgets[] = {
 
 	SND_SOC_DAPM_MUX_E("RX INT0_2 MUX", SND_SOC_NOPM, INTERP_EAR, 0,
 			   &rx_int0_2_mux, wcd934x_codec_enable_mix_path,
-			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU |
-			   SND_SOC_DAPM_POST_PMD),
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
 	SND_SOC_DAPM_MUX_E("RX INT1_2 MUX", SND_SOC_NOPM, INTERP_HPHL, 0,
 			   &rx_int1_2_mux, wcd934x_codec_enable_mix_path,
-			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU |
-			   SND_SOC_DAPM_POST_PMD),
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
 	SND_SOC_DAPM_MUX_E("RX INT2_2 MUX", SND_SOC_NOPM, INTERP_HPHR, 0,
 			   &rx_int2_2_mux, wcd934x_codec_enable_mix_path,
-			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU |
-			   SND_SOC_DAPM_POST_PMD),
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
 	SND_SOC_DAPM_MUX_E("RX INT3_2 MUX", SND_SOC_NOPM, INTERP_LO1, 0,
 			   &rx_int3_2_mux, wcd934x_codec_enable_mix_path,
-			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU |
-			   SND_SOC_DAPM_POST_PMD),
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
 	SND_SOC_DAPM_MUX_E("RX INT4_2 MUX", SND_SOC_NOPM, INTERP_LO2, 0,
 			   &rx_int4_2_mux, wcd934x_codec_enable_mix_path,
-			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU |
-			   SND_SOC_DAPM_POST_PMD),
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
 	SND_SOC_DAPM_MUX_E("RX INT7_2 MUX", SND_SOC_NOPM, INTERP_SPKR1, 0,
 			   &rx_int7_2_mux, wcd934x_codec_enable_mix_path,
-			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU |
-			   SND_SOC_DAPM_POST_PMD),
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
 	SND_SOC_DAPM_MUX_E("RX INT8_2 MUX", SND_SOC_NOPM, INTERP_SPKR2, 0,
 			   &rx_int8_2_mux, wcd934x_codec_enable_mix_path,
-			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU |
-			   SND_SOC_DAPM_POST_PMD),
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
 
 	SND_SOC_DAPM_MUX("RX INT0_1 MIX1 INP0", SND_SOC_NOPM, 0, 0,
 			 &rx_int0_1_mix_inp0_mux),
@@ -5389,41 +5352,20 @@ static const struct snd_soc_dapm_widget wcd934x_dapm_widgets[] = {
 	SND_SOC_DAPM_MUX("RX INT2 DEM MUX", SND_SOC_NOPM, 0, 0,
 			 &rx_int2_dem_inp_mux),
 
-	SND_SOC_DAPM_MUX_E("RX INT0_1 INTERP", SND_SOC_NOPM, INTERP_EAR, 0,
-			   &rx_int0_1_interp_mux,
-			   wcd934x_codec_enable_main_path,
-			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU |
-			   SND_SOC_DAPM_POST_PMD),
-	SND_SOC_DAPM_MUX_E("RX INT1_1 INTERP", SND_SOC_NOPM, INTERP_HPHL, 0,
-			   &rx_int1_1_interp_mux,
-			   wcd934x_codec_enable_main_path,
-			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU |
-			   SND_SOC_DAPM_POST_PMD),
-	SND_SOC_DAPM_MUX_E("RX INT2_1 INTERP", SND_SOC_NOPM, INTERP_HPHR, 0,
-			   &rx_int2_1_interp_mux,
-			   wcd934x_codec_enable_main_path,
-			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU |
-			   SND_SOC_DAPM_POST_PMD),
-	SND_SOC_DAPM_MUX_E("RX INT3_1 INTERP", SND_SOC_NOPM, INTERP_LO1, 0,
-			   &rx_int3_1_interp_mux,
-			   wcd934x_codec_enable_main_path,
-			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU |
-			   SND_SOC_DAPM_POST_PMD),
-	SND_SOC_DAPM_MUX_E("RX INT4_1 INTERP", SND_SOC_NOPM, INTERP_LO2, 0,
-			   &rx_int4_1_interp_mux,
-			   wcd934x_codec_enable_main_path,
-			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU |
-			   SND_SOC_DAPM_POST_PMD),
-	SND_SOC_DAPM_MUX_E("RX INT7_1 INTERP", SND_SOC_NOPM, INTERP_SPKR1, 0,
-			   &rx_int7_1_interp_mux,
-			   wcd934x_codec_enable_main_path,
-			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU |
-			   SND_SOC_DAPM_POST_PMD),
-	SND_SOC_DAPM_MUX_E("RX INT8_1 INTERP", SND_SOC_NOPM, INTERP_SPKR2, 0,
-			   &rx_int8_1_interp_mux,
-			   wcd934x_codec_enable_main_path,
-			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU |
-			   SND_SOC_DAPM_POST_PMD),
+	SND_SOC_DAPM_MUX("RX INT0_1 INTERP", SND_SOC_NOPM, INTERP_EAR, 0,
+			   &rx_int0_1_interp_mux),
+	SND_SOC_DAPM_MUX("RX INT1_1 INTERP", SND_SOC_NOPM, INTERP_HPHL, 0,
+			   &rx_int1_1_interp_mux),
+	SND_SOC_DAPM_MUX("RX INT2_1 INTERP", SND_SOC_NOPM, INTERP_HPHR, 0,
+			   &rx_int2_1_interp_mux),
+	SND_SOC_DAPM_MUX("RX INT3_1 INTERP", SND_SOC_NOPM, INTERP_LO1, 0,
+			   &rx_int3_1_interp_mux),
+	SND_SOC_DAPM_MUX("RX INT4_1 INTERP", SND_SOC_NOPM, INTERP_LO2, 0,
+			   &rx_int4_1_interp_mux),
+	SND_SOC_DAPM_MUX("RX INT7_1 INTERP", SND_SOC_NOPM, INTERP_SPKR1, 0,
+			   &rx_int7_1_interp_mux),
+	SND_SOC_DAPM_MUX("RX INT8_1 INTERP", SND_SOC_NOPM, INTERP_SPKR2, 0,
+			   &rx_int8_1_interp_mux),
 
 	SND_SOC_DAPM_MUX("RX INT0_2 INTERP", SND_SOC_NOPM, 0, 0,
 			 &rx_int0_2_interp_mux),
-- 
2.41.0

[PATCH 7/7] ASoC: codecs: msm8916-wcd-digital: drop dead gain hacks

[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 reads out the (cached) gain setting and
writes it back again on DAPM events.

Drop these incomplete and pointless hacks.

Note that seemingly related 10 ms delay after enabling the interpolator
is removed in the process.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 sound/soc/codecs/msm8916-wcd-digital.c | 26 +++-----------------------
 1 file changed, 3 insertions(+), 23 deletions(-)

diff --git a/sound/soc/codecs/msm8916-wcd-digital.c b/sound/soc/codecs/msm8916-wcd-digital.c
index 978c4d056e81..f73731aa4b58 100644
--- a/sound/soc/codecs/msm8916-wcd-digital.c
+++ b/sound/soc/codecs/msm8916-wcd-digital.c
@@ -228,17 +228,6 @@ struct msm8916_wcd_digital_priv {
 	struct clk *ahbclk, *mclk;
 };
 
-static const unsigned long rx_gain_reg[] = {
-	LPASS_CDC_RX1_VOL_CTL_B2_CTL,
-	LPASS_CDC_RX2_VOL_CTL_B2_CTL,
-	LPASS_CDC_RX3_VOL_CTL_B2_CTL,
-};
-
-static const unsigned long tx_gain_reg[] = {
-	LPASS_CDC_TX1_VOL_CTL_GAIN,
-	LPASS_CDC_TX2_VOL_CTL_GAIN,
-};
-
 static const char *const rx_mix1_text[] = {
 	"ZERO", "IIR1", "IIR2", "RX1", "RX2", "RX3"
 };
@@ -580,12 +569,6 @@ static int msm8916_wcd_digital_enable_interpolator(
 	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
 
 	switch (event) {
-	case SND_SOC_DAPM_POST_PMU:
-		/* apply the digital gain after the interpolator is enabled */
-		usleep_range(10000, 10100);
-		snd_soc_component_write(component, rx_gain_reg[w->shift],
-			      snd_soc_component_read(component, rx_gain_reg[w->shift]));
-		break;
 	case SND_SOC_DAPM_POST_PMD:
 		snd_soc_component_update_bits(component, LPASS_CDC_CLK_RX_RESET_CTL,
 					      1 << w->shift, 1 << w->shift);
@@ -630,9 +613,6 @@ static int msm8916_wcd_digital_enable_dec(struct snd_soc_dapm_widget *w,
 		snd_soc_component_update_bits(component, tx_mux_ctl_reg,
 				    TX_MUX_CTL_HPF_BP_SEL_MASK,
 				    TX_MUX_CTL_HPF_BP_SEL_NO_BYPASS);
-		/* apply the digital gain after the decimator is enabled */
-		snd_soc_component_write(component, tx_gain_reg[w->shift],
-			      snd_soc_component_read(component, tx_gain_reg[w->shift]));
 		snd_soc_component_update_bits(component, tx_vol_ctl_reg,
 				    TX_VOL_CTL_CFG_MUTE_EN_MASK, 0);
 		break;
@@ -739,13 +719,13 @@ static const struct snd_soc_dapm_widget msm8916_wcd_digital_dapm_widgets[] = {
 	/* Interpolator */
 	SND_SOC_DAPM_MIXER_E("RX1 INT", LPASS_CDC_CLK_RX_B1_CTL, 0, 0, NULL,
 			     0, msm8916_wcd_digital_enable_interpolator,
-			     SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_POST_PMD),
+			     SND_SOC_DAPM_POST_PMD),
 	SND_SOC_DAPM_MIXER_E("RX2 INT", LPASS_CDC_CLK_RX_B1_CTL, 1, 0, NULL,
 			     0, msm8916_wcd_digital_enable_interpolator,
-			     SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_POST_PMD),
+			     SND_SOC_DAPM_POST_PMD),
 	SND_SOC_DAPM_MIXER_E("RX3 INT", LPASS_CDC_CLK_RX_B1_CTL, 2, 0, NULL,
 			     0, msm8916_wcd_digital_enable_interpolator,
-			     SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_POST_PMD),
+			     SND_SOC_DAPM_POST_PMD),
 	SND_SOC_DAPM_MUX("RX1 MIX1 INP1", SND_SOC_NOPM, 0, 0,
 			 &rx_mix1_inp1_mux),
 	SND_SOC_DAPM_MUX("RX1 MIX1 INP2", SND_SOC_NOPM, 0, 0,
-- 
2.41.0

Re: [PATCH 2/7] ASoC: codecs: lpass-wsa-macro: fix compander volume hack

[Srinivas Kandagatla]

Thanks Johan for this patch,

On 16/01/2024 09:38, Johan Hovold wrote:
> 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 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.
> 
This is not a hack, wsa codec requires gain to be programmed after the 
clk is enabled for that particular interpolator.

However I agree with you on programming the gain that is different to 
what user set it.

This is because of unimplemented or half baked implementation of half-db 
feature of gain control in this codec.

We can clean that half baked implementation for now and still continue 
to program the gain values after the clks are enabled.

lets remove spkr_gain_offset and associated code paths in this codec, 
which should fix the issue that you have reported and still do the right 
thing of programming gain after clks are enabled.

thanks,
Srini


> Fixes: 2c4066e5d428 ("ASoC: codecs: lpass-wsa-macro: add dapm widgets and route")
> Cc: stable@vger.kernel.org      # 5.11
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>   sound/soc/codecs/lpass-wsa-macro.c | 10 ----------
>   1 file changed, 10 deletions(-)
> 
> diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
> index 7e21cec3c2fb..7de221464d47 100644
> --- a/sound/soc/codecs/lpass-wsa-macro.c
> +++ b/sound/soc/codecs/lpass-wsa-macro.c
> @@ -1583,8 +1583,6 @@ static int wsa_macro_enable_interpolator(struct snd_soc_dapm_widget *w,
>   	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
>   	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,11 +1621,7 @@ 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);
>   		break;
> @@ -1654,10 +1648,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);

Re: [PATCH 1/7] ASoC: qcom: sc8280xp: limit speaker volumes

[Srinivas Kandagatla]

On 16/01/2024 09:38, Johan Hovold wrote:
> 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: stable@vger.kernel.org	# 6.5
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

LGTM, We can get rid of this limit once we have Speaker protection inplace.


Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

--srini

> ---
>   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;

Re: [PATCH 7/7] ASoC: codecs: msm8916-wcd-digital: drop dead gain hacks

[Srinivas Kandagatla]

Hi Johan,

did you see any issue with this codec?

These are not really hacks, to make the gains effective it has to be 
applied in a particular order.


On 16/01/2024 09:39, Johan Hovold wrote:
> -		/* apply the digital gain after the interpolator is enabled */

As you noticed in the comments, the gains have to be reprogrammed after 
interpolator and its clks are enabled.


> -		usleep_range(10000, 10100);
> -		snd_soc_component_write(component, rx_gain_reg[w->shift],
> -			      snd_soc_component_read(component, rx_gain_reg[w->shift]));
> -		break;
>   	case SND_SOC_DAPM_POST_PMD:
>   		snd_soc_component_update_bits(component, LPASS_CDC_CLK_RX_RESET_CTL,
>   					      1 << w->shift, 1 << w->shift);
> @@ -630,9 +613,6 @@ static int msm8916_wcd_digital_enable_dec(struct snd_soc_dapm_widget *w,
>   		snd_soc_component_update_bits(component, tx_mux_ctl_reg,
>   				    TX_MUX_CTL_HPF_BP_SEL_MASK,
>   				    TX_MUX_CTL_HPF_BP_SEL_NO_BYPASS);
> -		/* apply the digital gain after the decimator is enabled */
same here.

> -		snd_soc_component_write(component, tx_gain_reg[w->shift],
> -			      snd_soc_component_read(component, tx_gain_reg[w->shift]));

Re: [PATCH 2/7] ASoC: codecs: lpass-wsa-macro: fix compander volume hack

[Johan Hovold]

On Tue, Jan 16, 2024 at 11:10:21AM +0000, Srinivas Kandagatla wrote:
> Thanks Johan for this patch,
> 
> On 16/01/2024 09:38, Johan Hovold wrote:
> > 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 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.
> > 
> This is not a hack, wsa codec requires gain to be programmed after the 
> clk is enabled for that particular interpolator.

Ok, but then it's also broken as, as I mentioned off-list, these
registers are cached so unless companding is enabled, the write on
enable will have no effect.

> However I agree with you on programming the gain that is different to 
> what user set it.
> 
> This is because of unimplemented or half baked implementation of half-db 
> feature of gain control in this codec.
> 
> We can clean that half baked implementation for now and still continue 
> to program the gain values after the clks are enabled.
> 
> lets remove spkr_gain_offset and associated code paths in this codec, 
> which should fix the issue that you have reported and still do the right 
> thing of programming gain after clks are enabled.

Removing the offset which can alter the gain, will cause both of these
writes to become no-ops as the registers are cached (e.g. just as for
the follow-on codec cleanups). So then we might as well just remove
this too.

How is the half-dB feature supposed to work?

And are you sure that you need to reprogram the gain value after
enabling the clock? Everything seems to work without doing so.

Johan

Re: [PATCH 1/7] ASoC: qcom: sc8280xp: limit speaker volumes

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 649 bytes --]

On Tue, Jan 16, 2024 at 11:11:47AM +0000, Srinivas Kandagatla wrote:
> On 16/01/2024 09:38, Johan Hovold wrote:
> > 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.

> LGTM, We can get rid of this limit once we have Speaker protection inplace.

There should be a userspace component for speaker protection so you'll
need to limit things when that's not running.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/7] ASoC: codecs: lpass-wsa-macro: fix compander volume hack

[Johan Hovold]

On Tue, Jan 16, 2024 at 02:10:19PM +0100, Johan Hovold wrote:
> On Tue, Jan 16, 2024 at 11:10:21AM +0000, Srinivas Kandagatla wrote:

> > This is not a hack, wsa codec requires gain to be programmed after the 
> > clk is enabled for that particular interpolator.
> 
> Ok, but then it's also broken as, as I mentioned off-list, these
> registers are cached so unless companding is enabled, the write on
> enable will have no effect.

I was obviously confused here, and indeed tests reveal that those writes
are needed for any prior updates to take effect (e.g. volume changes
while the codec is runtime suspended).

I've just sent a v2 here:

	https://lore.kernel.org/lkml/20240117090331.31111-1-johan+linaro@kernel.org/

Johan