diff mbox series

ASoC: codecs: wcd938x: fix dB range for HPHL and HPHR

Message ID 20230705125723.40464-1-srinivas.kandagatla@linaro.org
State Accepted
Commit c03226ba15fe3c42d13907ec7d8536396602557b
Headers show
Series ASoC: codecs: wcd938x: fix dB range for HPHL and HPHR | expand

Commit Message

Srinivas Kandagatla July 5, 2023, 12:57 p.m. UTC
dB range for HPHL and HPHR gains are from +6dB to -30dB in steps of
1.5dB with register values range from 0 to 24.

Current code maps these dB ranges incorrectly, fix them to allow proper
volume setting.

Fixes: e8ba1e05bdc0("ASoC: codecs: wcd938x: add basic controls")
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/codecs/wcd938x.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Takashi Iwai July 10, 2023, 8:19 a.m. UTC | #1
On Fri, 07 Jul 2023 17:06:24 +0200,
Mark Brown wrote:
> 
> On Fri, Jul 07, 2023 at 03:47:47PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > The ASoC generic control stuff supports inverting the value prior to
> > > presentation to userspace so it's masked there (instead of writing the
> > > number userspace sees to the register we subtract the number from the
> > > maximum value and write that to the register), pulling that up further
> > > to the ALSA core might be nice I guess?
> 
> > I believe yes.  Though, I'm still not sure how we can improve the
> > mismatch of dB min/max.  The dB values of those inverted controls
> > reflect the result of subtraction, no?
> 
> Yes, the dB scale presented to userspace is reversed relative to the
> ordering in the registers.

Right, the TLV min/max corresponds to the control values, and they
don't mean the raw register values.

BTW, this thread made me wonder whether it makes sense to give some
sanity checks (maybe with CONFIG_SND_DEBUG) in ALSA core.
e.g. read_tlv_buf() in sound/core/control.c can perform some tests
before actually passing to user-space.


Takashi
diff mbox series

Patch

diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c
index faa15a5ed2c8..3a3360711f8f 100644
--- a/sound/soc/codecs/wcd938x.c
+++ b/sound/soc/codecs/wcd938x.c
@@ -210,7 +210,7 @@  struct wcd938x_priv {
 };
 
 static const SNDRV_CTL_TLVD_DECLARE_DB_MINMAX(ear_pa_gain, 600, -1800);
-static const SNDRV_CTL_TLVD_DECLARE_DB_MINMAX(line_gain, 600, -3000);
+static const DECLARE_TLV_DB_SCALE(line_gain, -3000, 150, -3000);
 static const SNDRV_CTL_TLVD_DECLARE_DB_MINMAX(analog_gain, 0, 3000);
 
 struct wcd938x_mbhc_zdet_param {
@@ -2655,8 +2655,8 @@  static const struct snd_kcontrol_new wcd938x_snd_controls[] = {
 		       wcd938x_get_swr_port, wcd938x_set_swr_port),
 	SOC_SINGLE_EXT("DSD_R Switch", WCD938X_DSD_R, 0, 1, 0,
 		       wcd938x_get_swr_port, wcd938x_set_swr_port),
-	SOC_SINGLE_TLV("HPHL Volume", WCD938X_HPH_L_EN, 0, 0x18, 0, line_gain),
-	SOC_SINGLE_TLV("HPHR Volume", WCD938X_HPH_R_EN, 0, 0x18, 0, line_gain),
+	SOC_SINGLE_TLV("HPHL Volume", WCD938X_HPH_L_EN, 0, 0x18, 1, line_gain),
+	SOC_SINGLE_TLV("HPHR Volume", WCD938X_HPH_R_EN, 0, 0x18, 1, line_gain),
 	WCD938X_EAR_PA_GAIN_TLV("EAR_PA Volume", WCD938X_ANA_EAR_COMPANDER_CTL,
 				2, 0x10, 0, ear_pa_gain),
 	SOC_SINGLE_EXT("ADC1 Switch", WCD938X_ADC1, 1, 1, 0,