diff mbox

ASoC: cs42l51: Fix SOC_DOUBLE_R_SX_TLV shift values for ADC, PCM, and Analog kcontrols

Message ID 1395243602-18012-1-git-send-email-brian.austin@cirrus.com (mailing list archive)
State Accepted
Commit 7272e051157ccd5871b5d939548d0ba5a94a2965
Headers show

Commit Message

Austin, Brian March 19, 2014, 3:40 p.m. UTC
The shift values for the ADC,PCM, and Analog kcontrols were wrong causing wrong values for the SOC_DOUBLE_R_SX_TLV macros
Fixed the TLV for aout_tlv to show -102dB correctly

Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Brian Austin <brian.austin@cirrus.com>
---
 sound/soc/codecs/cs42l51.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comments

Mark Brown March 19, 2014, 4:38 p.m. UTC | #1
On Wed, Mar 19, 2014 at 10:40:02AM -0500, Brian Austin wrote:
> The shift values for the ADC,PCM, and Analog kcontrols were wrong causing wrong values for the SOC_DOUBLE_R_SX_TLV macros
> Fixed the TLV for aout_tlv to show -102dB correctly

Applied, thanks.
Thomas Petazzoni April 17, 2014, 2:26 p.m. UTC | #2
Dear Brian Austin,

On Wed, 19 Mar 2014 10:40:02 -0500, Brian Austin wrote:
> The shift values for the ADC,PCM, and Analog kcontrols were wrong causing wrong values for the SOC_DOUBLE_R_SX_TLV macros
> Fixed the TLV for aout_tlv to show -102dB correctly
> 
> Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Signed-off-by: Brian Austin <brian.austin@cirrus.com>
> ---
>  sound/soc/codecs/cs42l51.c |    9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)

Thanks for this patch. Unfortunately, it still doesn't solve completely
the situation. When I start alsamixer, it shows that:

 * PCM is at 102 (so above the maximum 100 value)
 * Both ADC Mixers are at 102
 * Analog is at 174 (so way above the maximum 100 value)

Changing the value of PCM works (but of course I can then only change
it between 0 and 100, not up to 102). However, as soon as I change the
"Analog" value, even if I raise it up to 174, there isn't any sound
anymore. Probably because the alsamixer range 0-100 doesn't allow to
reach a high enough volume to be audible.

Here is the amixer output right after boot (without changing any
value) :

=================================================================
# amixer 
Simple mixer control 'Bass',0
  Capabilities: volume volume-joined
  Playback channels: Mono
  Capture channels: Mono
  Limits: 0 - 15
  Mono: 7 [47%] [0.00dB]
Simple mixer control 'Treble',0
  Capabilities: volume volume-joined
  Playback channels: Mono
  Capture channels: Mono
  Limits: 0 - 15
  Mono: 7 [47%] [0.00dB]
Simple mixer control 'PCM',0
  Capabilities: pvolume pswitch
  Playback channels: Front Left - Front Right
  Limits: Playback 0 - 102
  Mono:
  Front Left: Playback 103 [101%] [0.00dB] [off]
  Front Right: Playback 103 [101%] [0.00dB] [off]
Simple mixer control 'PCM channel mixer',0
  Capabilities: enum
  Items: 'L R' 'L+R' 'R L'
  Item0: 'L R'
Simple mixer control 'Mic Boost',0
  Capabilities: volume
  Playback channels: Front Left - Front Right
  Capture channels: Front Left - Front Right
  Limits: 0 - 1
  Front Left: 0 [0%] [16.00dB]
  Front Right: 0 [0%] [16.00dB]
Simple mixer control 'Playback Deemphasis',0
  Capabilities: pswitch pswitch-joined
  Playback channels: Mono
  Mono: Playback [off]
Simple mixer control 'ADC Mixer',0
  Capabilities: volume pswitch
  Playback channels: Front Left - Front Right
  Capture channels: Front Left - Front Right
  Limits: 0 - 102
  Front Left: 103 [101%] [0.00dB] Playback [off]
  Front Right: 103 [101%] [0.00dB] Playback [off]
Simple mixer control 'Analog',0
  Capabilities: pvolume
  Playback channels: Front Left - Front Right
  Limits: Playback 0 - 176
  Mono:
  Front Left: Playback 204 [116%] [0.00dB]
  Front Right: Playback 204 [116%] [0.00dB]
Simple mixer control 'Auto-Mute',0
  Capabilities: pswitch pswitch-joined
  Playback channels: Mono
  Mono: Playback [on]
Simple mixer control 'PGA-ADC Mux Left',0
  Capabilities: enum
  Items: 'AIN1 Left' 'AIN2 Left' 'MIC Left' 'MIC+preamp Left'
  Item0: 'AIN1 Left'
Simple mixer control 'PGA-ADC Mux Right',0
  Capabilities: enum
  Items: 'AIN1 Right' 'AIN2 Right' 'MIC Right' 'MIC+preamp Right'
  Item0: 'AIN1 Right'
Simple mixer control 'Soft Ramp',0
  Capabilities: pswitch pswitch-joined
  Playback channels: Mono
  Mono: Playback [off]
# 
=================================================================

See also an alsamixer screenshot at
http://free-electrons.com/~thomas/pub/alsamixer-issue.png.

I wanted to give you the raw state of the codec registers, but for some
reason, the codec_reg file is empty here (I'm on 3.15-rc1) :

=================================================================
# cat /sys/kernel/debug/asoc/a370db/cs42l51-codec.0-004a/codec_reg 
# 
=================================================================

Note that if I revert back the cs42l51 code to the state before
1d99f2436d0d1c7741d6dfd9d27b5376cdbbca40 ('ASoC: core: Rework
SOC_DOUBLE_R_SX_TLV add SOC_SINGLE_SX_TLV'), then the volume control
works normally.

Thanks!

Thomas
Austin, Brian April 17, 2014, 3 p.m. UTC | #3
On Thu, 17 Apr 2014, Thomas Petazzoni wrote:

>
> See also an alsamixer screenshot at
> http://free-electrons.com/~thomas/pub/alsamixer-issue.png.
>
> I wanted to give you the raw state of the codec registers, but for some
> reason, the codec_reg file is empty here (I'm on 3.15-rc1) :
>
> =================================================================
> # cat /sys/kernel/debug/asoc/a370db/cs42l51-codec.0-004a/codec_reg
> #

cat /sys/kernel/debug/regmap/0-004a/registers

It uses regmap. Yes a register dump  would be helpful

Thanks
Austin, Brian April 17, 2014, 3:08 p.m. UTC | #4
On Thu, 17 Apr 2014, Thomas Petazzoni wrote:

> Dear Brian Austin,
>
> On Wed, 19 Mar 2014 10:40:02 -0500, Brian Austin wrote:
>> The shift values for the ADC,PCM, and Analog kcontrols were wrong causing wrong values for the SOC_DOUBLE_R_SX_TLV macros
>> Fixed the TLV for aout_tlv to show -102dB correctly
>>
>> Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> Signed-off-by: Brian Austin <brian.austin@cirrus.com>
>> ---
>>  sound/soc/codecs/cs42l51.c |    9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> Thanks for this patch. Unfortunately, it still doesn't solve completely
> the situation. When I start alsamixer, it shows that:
>
> * PCM is at 102 (so above the maximum 100 value)
> * Both ADC Mixers are at 102
> * Analog is at 174 (so way above the maximum 100 value)
>
> Changing the value of PCM works (but of course I can then only change
> it between 0 and 100, not up to 102). However, as soon as I change the
> "Analog" value, even if I raise it up to 174, there isn't any sound
> anymore. Probably because the alsamixer range 0-100 doesn't allow to
> reach a high enough volume to be audible.
>
> Here is the amixer output right after boot (without changing any
> value) :
>
can you try this quick check please?

  static const struct snd_kcontrol_new cs42l51_snd_controls[] = {
         SOC_DOUBLE_R_SX_TLV("PCM Playback Volume",
                         CS42L51_PCMA_VOL, CS42L51_PCMB_VOL,
-                       0, 0x19, 0x7F, adc_pcm_tlv),
+                       0, 0x19, 0x80, adc_pcm_tlv),
         SOC_DOUBLE_R("PCM Playback Switch",
                         CS42L51_PCMA_VOL, CS42L51_PCMB_VOL, 7, 1, 1),
         SOC_DOUBLE_R_SX_TLV("Analog Playback Volume",
                         CS42L51_AOUTA_VOL, CS42L51_AOUTB_VOL,
-                       0, 0x34, 0xE4, aout_tlv),
+                       0, 0x34, 0xE5, aout_tlv),
         SOC_DOUBLE_R_SX_TLV("ADC Mixer Volume",
                         CS42L51_ADCA_VOL, CS42L51_ADCB_VOL,
-                       0, 0x19, 0x7F, adc_pcm_tlv),
+                       0, 0x19, 0x80, adc_pcm_tlv),
         SOC_DOUBLE_R("ADC Mixer Switch",
                         CS42L51_ADCA_VOL, CS42L51_ADCB_VOL, 7, 1, 1),
         SOC_SINGLE("Playback Deemphasis Switch", CS42L51_DAC_CTL, 3, 1, 
0),


-Brian
Austin, Brian April 18, 2014, 12:22 p.m. UTC | #5
On Apr 18, 2014, at 4:23 AM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> Dear Brian Austin,
> 
> On Thu, 17 Apr 2014 10:08:23 -0500, Brian Austin wrote:
> 
>> can you try this quick check please?
>> 
>>  static const struct snd_kcontrol_new cs42l51_snd_controls[] = {
>>         SOC_DOUBLE_R_SX_TLV("PCM Playback Volume",
>>                         CS42L51_PCMA_VOL, CS42L51_PCMB_VOL,
>> -                       0, 0x19, 0x7F, adc_pcm_tlv),
>> +                       0, 0x19, 0x80, adc_pcm_tlv),
>>         SOC_DOUBLE_R("PCM Playback Switch",
>>                         CS42L51_PCMA_VOL, CS42L51_PCMB_VOL, 7, 1, 1),
>>         SOC_DOUBLE_R_SX_TLV("Analog Playback Volume",
>>                         CS42L51_AOUTA_VOL, CS42L51_AOUTB_VOL,
>> -                       0, 0x34, 0xE4, aout_tlv),
>> +                       0, 0x34, 0xE5, aout_tlv),
>>         SOC_DOUBLE_R_SX_TLV("ADC Mixer Volume",
>>                         CS42L51_ADCA_VOL, CS42L51_ADCB_VOL,
>> -                       0, 0x19, 0x7F, adc_pcm_tlv),
>> +                       0, 0x19, 0x80, adc_pcm_tlv),
>>         SOC_DOUBLE_R("ADC Mixer Switch",
>>                         CS42L51_ADCA_VOL, CS42L51_ADCB_VOL, 7, 1, 1),
>>         SOC_SINGLE("Playback Deemphasis Switch", CS42L51_DAC_CTL, 3, 1, 
>> 0),
> 
> Unfortunately, it still doesn't work: at boot time the PCM and ADC
> mixer values are 100 which looks OK, but the Analog value is 170, which
> doesn't look ok. And inspecting the values set at the register level
> for the PCM control seems to reveal that things are still not working
> correctly (read on below).
> 
> But this time around, I've collected a lot of debugging informations,
> which hopefully will help understand the problem. My understanding is
> that:
> 
> 	SOC_DOUBLE_R_SX_TLV("Analog Playback Volume",
> 			CS42L51_AOUTA_VOL, CS42L51_AOUTB_VOL,
> 			0, 0x34, 0xE5, aout_tlv),
> 
> cannot work, because it assumes that the range -102.0 dB to 12.0 dB
> described in the datasheet is represented by using register values from
> 0x34 to 0xE5. Which misses the fact that when going from -102.0 dB to 0
> dB, the register value goes from 0x34 to 0, and then when going from 0
> to 12.0 dB, the register value should go from 0 to 0xE5.
> 
> 
Ok, Thanks for the output.
Something is for sure broken, but I could have sworn I checked this. I’ll hook up my L51 board today and see what the deal is.

Thanks,
Brian
Austin, Brian April 18, 2014, 2 p.m. UTC | #6
On Fri, 18 Apr 2014, Thomas Petazzoni wrote:


> Simple mixer control 'Analog',0
>  Capabilities: pvolume
>  Playback channels: Front Left - Front Right
>  Limits: Playback 0 - 177
This is wrong

>  Mono:
>  Front Left: Playback 204 [115%] [0.00dB]
>  Front Right: Playback 204 [115%] [0.00dB]

root@beagleboard:~# amixer sget 'Analog'
Simple mixer control 'Analog',0
   Capabilities: pvolume penum
   Playback channels: Front Left - Front Right
   Limits: Playback 0 - 229
   Mono:
   Front Left: Playback 204 [89%] [0.00dB]
   Front Right: Playback 204 [89%] [0.00dB]

root@beagleboard:~# i2cdump -y -f 1 0x4b
No size specified (using byte-data access)
      0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f    0123456789abcdef
00: 00 d9 7e ae 00 00 a0 00 60 44 00 00 00 00 84 84    .?~?..?.`D....??
10: 19 19 00 00 00 88 00 00 00 00 7f 00 01 3f 00 00    ??...?....?.??..
[...]

As you can see the output of amixer is wrong on your system. There are 229 
steps for gain on this control. At 89% we are at 0dB which is correct 
according to I2C Tools (0h)

root@beagleboard:~# amixer sset 'Analog' 0
Simple mixer control 'Analog',0
   Capabilities: pvolume penum
   Playback channels: Front Left - Front Right
   Limits: Playback 0 - 229
   Mono:
   Front Left: Playback 0 [0%] [-102.00dB]
   Front Right: Playback 0 [0%] [-102.00dB]
root@beagleboard:~# i2cdump -y -f 1 0x4b
No size specified (using byte-data access)
      0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f    0123456789abcdef
00: 00 d9 7e ae 00 00 a0 00 60 44 00 00 00 00 84 84    .?~?..?.`D....??
10: 19 19 00 00 00 88 34 34 00 00 7f 00 01 3f 00 00    ??...?44..?.??..

-102dB gives the correct output and register writes as well. (34h)

My regmap output maps correctly too. Weird.

So lets get a few versions of some things to see what and where things 
went wrong.

What kernel version?
Alsa-lib version?
What distro are you using?

Thanks,
Brian
Thomas Petazzoni April 18, 2014, 2:21 p.m. UTC | #7
Dear Brian Austin,

On Fri, 18 Apr 2014 09:00:35 -0500, Brian Austin wrote:

> My regmap output maps correctly too. Weird.

Indeed, weird. Thanks a lot for doing all this testing.

> So lets get a few versions of some things to see what and where things 
> went wrong.
> 
> What kernel version?

3.15-rc1

> Alsa-lib version?

1.0.26. This is not the latest, but certainly not that old either.

> What distro are you using?

Custom root filesystem built with Buildroot 2013.05.

Thomas
diff mbox

Patch

diff --git a/sound/soc/codecs/cs42l51.c b/sound/soc/codecs/cs42l51.c
index 3eab460..65fc37c 100644
--- a/sound/soc/codecs/cs42l51.c
+++ b/sound/soc/codecs/cs42l51.c
@@ -106,9 +106,8 @@  static int cs42l51_set_chan_mix(struct snd_kcontrol *kcontrol,
 
 static const DECLARE_TLV_DB_SCALE(adc_pcm_tlv, -5150, 50, 0);
 static const DECLARE_TLV_DB_SCALE(tone_tlv, -1050, 150, 0);
-/* This is a lie. after -102 db, it stays at -102 */
-/* maybe a range would be better */
-static const DECLARE_TLV_DB_SCALE(aout_tlv, -11550, 50, 0);
+
+static const DECLARE_TLV_DB_SCALE(aout_tlv, -10200, 50, 0);
 
 static const DECLARE_TLV_DB_SCALE(boost_tlv, 1600, 1600, 0);
 static const char *chan_mix[] = {
@@ -123,7 +122,7 @@  static const struct soc_enum cs42l51_chan_mix =
 static const struct snd_kcontrol_new cs42l51_snd_controls[] = {
 	SOC_DOUBLE_R_SX_TLV("PCM Playback Volume",
 			CS42L51_PCMA_VOL, CS42L51_PCMB_VOL,
-			6, 0x19, 0x7F, adc_pcm_tlv),
+			0, 0x19, 0x7F, adc_pcm_tlv),
 	SOC_DOUBLE_R("PCM Playback Switch",
 			CS42L51_PCMA_VOL, CS42L51_PCMB_VOL, 7, 1, 1),
 	SOC_DOUBLE_R_SX_TLV("Analog Playback Volume",
@@ -131,7 +130,7 @@  static const struct snd_kcontrol_new cs42l51_snd_controls[] = {
 			0, 0x34, 0xE4, aout_tlv),
 	SOC_DOUBLE_R_SX_TLV("ADC Mixer Volume",
 			CS42L51_ADCA_VOL, CS42L51_ADCB_VOL,
-			6, 0x19, 0x7F, adc_pcm_tlv),
+			0, 0x19, 0x7F, adc_pcm_tlv),
 	SOC_DOUBLE_R("ADC Mixer Switch",
 			CS42L51_ADCA_VOL, CS42L51_ADCB_VOL, 7, 1, 1),
 	SOC_SINGLE("Playback Deemphasis Switch", CS42L51_DAC_CTL, 3, 1, 0),