| Message ID | 56E69B3F.8090306@mcsplus.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Timur, On 03/14/16 13:06, Timur Karaldin wrote: > Hi, colleagues! > > I'm newbie with ALSA devel. I need to get extra functionality from codec > through ALSA mixer. > Task: 1. making possibility to set-up gain for each input (XXX_2_LADCCTRL and > XXX_2_RADCCTRL registers) This part is a bit tricky as in the registers: value 0..8 is gain from 0dB to -12dB in steps of -1.5dB and value 0xf means basically mute/disconnect. > 2. making possibility to set-up debounce for button and headset detection ( > HEADSET_DETECT_CTRL_A register) > 3. making possibility to enable high-power AC-coupled output > (HEADSET_DETECT_CTRL_A register) > 4. making possibility to enable stereo pseudo differential output > (HEADSET_DETECT_CTRL_B register) > 5. enable headset detection (HEADSET_DETECT_CTRL_A) > 6. getting headset detect status (HEADSET_DETECT_CTRL_A) > > Well, my patch works in generally, but I don't like it very much, because: > 1. All controls for input amplifiers' gain are shown in Alsamixer-Playback > section. But not only mine, capture controls from base codec's version are > shown in playback section too, for example (Right or Left) PGA Mixer Line1(L > or R)/2(L or R)/MIC3(L or R), which is definitely capture settings. Should I > worried about it? because, as I understand, it could not be setted correctly > during changing playback/capture mode for power-saving reason. > 2. "Right PGA Mixer Line1R" control (and similar) using the same bits in > LINE1R_2_RADCCTRL register as my "Line1R RADC Gain" control (and similar) and > put there b"1111" when user switch disconnect it from ADC. So my control is > not shown when user disconnect it from ADC. > 3. When I start/stop arecord some of my options were reset (debounces, headset > enable, high-power AC-coupled, pseudo differential output). When I set up it > in shell and then start arecord, it's reset again. Is it the reason because of > displaying controls in playback mixer section? > > I will be very pleased for any advise, thank you! > > > here is my patch for tlv320aic3x.c > --- > sound/soc/codecs/tlv320aic3x.c | 55 +++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 52 insertions(+), 3 deletions(-) > > diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c > index d7349bc..27d031e 100644 > --- a/sound/soc/codecs/tlv320aic3x.c > +++ b/sound/soc/codecs/tlv320aic3x.c > @@ -122,11 +122,24 @@ static const struct reg_default aic3x_reg[] = { > { 108, 0x00 }, { 109, 0x00 }, > }; > > +static bool aic3106_volatile(struct device *dev, unsigned int reg) > +{ > + switch(reg) > + { > + case AIC3X_HEADSET_DETECT_CTRL_A: > + case AIC3X_HEADSET_DETECT_CTRL_B: > + return true; > + default: > + return false; > + } > +} > + > static const struct regmap_config aic3x_regmap = { > .reg_bits = 8, > .val_bits = 8, > > .max_register = DAC_ICC_ADJ, > + .volatile_reg = aic3106_volatile, > .reg_defaults = aic3x_reg, > .num_reg_defaults = ARRAY_SIZE(aic3x_reg), > .cache_type = REGCACHE_RBTREE, > @@ -196,7 +209,7 @@ static int mic_bias_event(struct snd_soc_dapm_widget *w, > struct snd_kcontrol *kcontrol, int event) > { > struct snd_soc_codec *codec = w->codec; > - struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec); > + struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec); You have couple of these nop changes in the patch... > > switch (event) { > case SND_SOC_DAPM_POST_PMU: > @@ -270,6 +283,26 @@ static const struct soc_enum aic3x_agc_decay_enum[] = { > SOC_ENUM_SINGLE(RAGC_CTRL_A, 0, 4, aic3x_agc_decay), > }; > > +static const char *aic3x_adc_gain[] = { "0dB", "-1.5dB", "-3dB", "-4.5dB", > "-6dB", "-7.5dB", "-9dB", "-10.5dB", "-12dB" }; > +static const struct soc_enum aic3x_adc_gain_enum[] = { > + SOC_ENUM_SINGLE(MIC3LR_2_LADC_CTRL, 4, 9, aic3x_adc_gain), > + SOC_ENUM_SINGLE(MIC3LR_2_RADC_CTRL, 0, 9, aic3x_adc_gain), > + SOC_ENUM_SINGLE(LINE1L_2_LADC_CTRL, 3, 9, aic3x_adc_gain), > + SOC_ENUM_SINGLE(LINE1R_2_RADC_CTRL, 3, 9, aic3x_adc_gain), > + SOC_ENUM_SINGLE(LINE2L_2_LADC_CTRL, 3, 9, aic3x_adc_gain), > + SOC_ENUM_SINGLE(LINE2R_2_RADC_CTRL, 3, 9, aic3x_adc_gain), > +}; So the issue is that we have the DAPM switches controlling exactly the same registers. I believe if you would set the gain in a way that bit0 is 1, then DAPM will think that the path is disconnected. Also if you would set the gain and then mute and unmute the path you would have lost the gain you wanted to have.. The only way I can think of implementing these mixers is to have two sets of custom callbacks. one set is to set/get the gain and the other is to set/get the mute/disconnect on these. When the path is disconnected you should not write the gain change to the chip, but cache it and if the path is unmuted, you write the cached gain. When you mute the path you should take the set gain first, cache it, then disconnect the path. For these gains you should have DECLARE_TLV_DB_SCALE() and use SOC_SINGLE_TLV(). Make sure that the control name matches with the corresponding DAPM widget's name so ALSA can match them correctly. > +static const char *aic3x_headset_debounce[] = { "16ms", "32ms", "64ms", > "128ms", "256ms", "512ms" }; > +static const struct soc_enum aic3x_headset_debounce_enum[] = { > + SOC_ENUM_SINGLE(AIC3X_HEADSET_DETECT_CTRL_A, 2, 6, aic3x_headset_debounce), > +}; > +static const char *aic3x_button_debounce[] = { "0ms", "8ms", "16ms", "32ms" }; > +static const struct soc_enum aic3x_button_debounce_enum[] = { > + SOC_ENUM_SINGLE(AIC3X_HEADSET_DETECT_CTRL_A, 0, 4, aic3x_button_debounce), > +}; > + > + > + > /* > * DAC digital volumes. From -63.5 to 0 dB in 0.5 dB steps > */ > @@ -399,6 +432,19 @@ static const struct snd_kcontrol_new aic3x_snd_controls[] > = { > SOC_DOUBLE_R("PGA Capture Switch", LADC_VOL, RADC_VOL, 7, 0x01, 1), > > SOC_ENUM("ADC HPF Cut-off", aic3x_enum[ADC_HPF_ENUM]), > + /* Additional controls */ > + SOC_ENUM("Mic3L LADC Gain", aic3x_adc_gain_enum[0]), > + SOC_ENUM("Mic3R RADC Gain", aic3x_adc_gain_enum[1]), > + SOC_ENUM("Line1L LADC Gain", aic3x_adc_gain_enum[2]), > + SOC_ENUM("Line1R RADC Gain", aic3x_adc_gain_enum[3]), > + SOC_ENUM("Line2L LADC Gain", aic3x_adc_gain_enum[4]), > + SOC_ENUM("Line2R RADC Gain", aic3x_adc_gain_enum[5]), Please avoid using arrays as the driver no longer use these anymore. > + SOC_ENUM("Headset Debounce", aic3x_headset_debounce_enum), > + SOC_ENUM("Button Debounce", aic3x_button_debounce_enum), > + SOC_SINGLE("Headset Detected", AIC3X_HEADSET_DETECT_CTRL_A, > AIC3X_HEADSET_DETECT_SHIFT, AIC3X_HEADSET_DETECT_MASK, 0), > + SOC_SINGLE("Headset Detect Enable", AIC3X_HEADSET_DETECT_CTRL_A, 7, 1, 0), > + SOC_SINGLE("High power output Ac-coupled", AIC3X_HEADSET_DETECT_CTRL_B, > 7, 1, 0), > + SOC_SINGLE("Stereo pseudodifferential output", > AIC3X_HEADSET_DETECT_CTRL_B, 3, 1, 0), > }; > > static const struct snd_kcontrol_new aic3x_mono_controls[] = { > @@ -614,7 +660,7 @@ static const struct snd_soc_dapm_widget > aic3x_dapm_widgets[] = { > SND_SOC_DAPM_REG(snd_soc_dapm_micbias, "DMic Rate 32", > AIC3X_ASD_INTF_CTRLA, 0, 3, 3, 0), > > - /* Mic Bias */ > + /* Mic Bias */ nop change > SND_SOC_DAPM_SUPPLY("Mic Bias", MICBIAS_CTRL, 6, 0, > mic_bias_event, > SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD), > @@ -692,7 +738,7 @@ static const struct snd_soc_dapm_route intercon[] = { > {"Left Line2L Mux", "single-ended", "LINE2L"}, > {"Left Line2L Mux", "differential", "LINE2L"}, > > - {"Left PGA Mixer", "Line1L Switch", "Left Line1L Mux"}, > + {"Left PGA Mixer", "Line1L Switch", "Left Line1L Mux"}, nop change > {"Left PGA Mixer", "Line1R Switch", "Left Line1R Mux"}, > {"Left PGA Mixer", "Line2L Switch", "Left Line2L Mux"}, > {"Left PGA Mixer", "Mic3L Switch", "MIC3L"}, > @@ -814,6 +860,7 @@ static const struct snd_soc_dapm_route intercon[] = { > {"Right HPCOM Mux", "external feedback", "Right HPCOM Mixer"}, > {"Right HP Com", NULL, "Right HPCOM Mux"}, > {"HPRCOM", NULL, "Right HP Com"}, > + why? > }; > > static const struct snd_soc_dapm_route intercon_mono[] = { > @@ -918,7 +965,9 @@ static int aic3x_hw_params(struct snd_pcm_substream > *substream, > data = (LDAC2LCH | RDAC2RCH); > data |= (fsref == 44100) ? FSREF_44100 : FSREF_48000; > if (params_rate(params) >= 64000) > + { > data |= DUAL_RATE_MODE; > + } why? > snd_soc_write(codec, AIC3X_CODEC_DATAPATH_REG, data); > > /* codec sample rate select */ > -- > and tlv320aic3x.h > --- > sound/soc/codecs/tlv320aic3x.h | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/sound/soc/codecs/tlv320aic3x.h b/sound/soc/codecs/tlv320aic3x.h > index e521ac3..8ff42d2 100644 > --- a/sound/soc/codecs/tlv320aic3x.h > +++ b/sound/soc/codecs/tlv320aic3x.h > @@ -271,6 +271,12 @@ enum { > AIC3X_BUTTON_DEBOUNCE_32MS = 3 > }; > > +typedef struct { > + struct platform_device* pdev; > + struct proc_dir_entry *proc_value; > + struct snd_soc_codec *codec; > +} aic3106_detect_t; > + > #define AIC3X_HEADSET_DETECT_ENABLED 0x80 > #define AIC3X_HEADSET_DETECT_SHIFT 5 > #define AIC3X_HEADSET_DETECT_MASK 3 > -- When submitting please separate the patches by functionality. It is easier to review the changes.
On 03/14/2016 04:57 PM, Timur Karaldin wrote: > Hi, Peter! > What's about placing capture controls in the Playback section of Alsamixer > view? Yeah, it is annoying. They can be fixed by: "Line1L Switch" -> "Line1L Capture Switch" and so on. On the other hand we had these unfortunate control names I think since the beginning and it is kind of an ABI for existing products. I don't have issue changing them, but existing setups relying on the control names might break... > Does resetting of some registers during start/stop of recording > connect with it? Not sure what you mean.
14.03.2016 19:38, Peter Ujfalusi ?????: > On 03/14/2016 04:57 PM, Timur Karaldin wrote: >> Hi, Peter! >> What's about placing capture controls in the Playback section of Alsamixer >> view? > Yeah, it is annoying. They can be fixed by: > "Line1L Switch" -> "Line1L Capture Switch" and so on. On the other hand we had > these unfortunate control names I think since the beginning and it is kind of > an ABI for existing products. I don't have issue changing them, but existing > setups relying on the control names might break... > >> Does resetting of some registers during start/stop of recording >> connect with it? > Not sure what you mean. > I mean current situation: #i2cdump -f -y -r 13-13 1 0x1b Result: 0x00 #arecord test.wav & #i2cdump -f -y -r 13-13 1 0x1b Result: 0x00 # amixer sset 'Headset Detect Enable' *on* #i2cdump -f -y -r 13-13 1 0x1b Result:*0x80* #killall arecord #i2cdump -f -y -r 13-13 1 0x1b Result: 0x00 #arecord test.wav & #i2cdump -f -y -r 13-13 1 0x1b Result:*0x00 *#killall arecord* *amixer sset 'Headset Detect Enable'*on** *#i2cdump -f -y -r 13-13 1 0x1b Result:**0x00 **#amixer sset 'Headset Detect Enable' off *amixer: Invalid command!* So state of 'Headset Detect Enable' and others of my controls were not stored during arecord/aplay restart (open and close device).
On 03/15/16 12:17, Timur Karaldin wrote: > 14.03.2016 19:38, Peter Ujfalusi ?????: >> On 03/14/2016 04:57 PM, Timur Karaldin wrote: >>> Hi, Peter! >>> What's about placing capture controls in the Playback section of Alsamixer >>> view? >> Yeah, it is annoying. They can be fixed by: >> "Line1L Switch" -> "Line1L Capture Switch" and so on. On the other hand we had >> these unfortunate control names I think since the beginning and it is kind of >> an ABI for existing products. I don't have issue changing them, but existing >> setups relying on the control names might break... >> >>> Does resetting of some registers during start/stop of recording >>> connect with it? >> Not sure what you mean. >> > I mean current situation: > > #i2cdump -f -y -r 13-13 1 0x1b > Result: 0x00 > #arecord test.wav & > #i2cdump -f -y -r 13-13 1 0x1b > Result: 0x00 > # amixer sset 'Headset Detect Enable' *on* > #i2cdump -f -y -r 13-13 1 0x1b > Result:*0x80* > #killall arecord > #i2cdump -f -y -r 13-13 1 0x1b > Result: 0x00 > #arecord test.wav & > #i2cdump -f -y -r 13-13 1 0x1b > Result:*0x00 > *#killall arecord* > *amixer sset 'Headset Detect Enable'*on** > *#i2cdump -f -y -r 13-13 1 0x1b > Result:**0x00 > **#amixer sset 'Headset Detect Enable' off > *amixer: Invalid command!* > > So state of 'Headset Detect Enable' and others of my controls were not stored > during arecord/aplay restart (open and close device). aic3x_set_power() will soft reset the codec on power off which resets also the headset detect registers.
Hi Peter! 14.03.2016 17:47, Peter Ujfalusi ?????: > So the issue is that we have the DAPM switches controlling exactly the > same registers. I believe if you would set the gain in a way that bit0 > is 1, then DAPM will think that the path is disconnected. Also if you > would set the gain and then mute and unmute the path you would have > lost the gain you wanted to have.. The only way I can think of > implementing these mixers is to have two sets of custom callbacks. one > set is to set/get the gain and the other is to set/get the > mute/disconnect on these. When the path is disconnected you should not > write the gain change to the chip, but cache it and if the path is > unmuted, you write the cached gain. When you mute the path you should > take the set gain first, cache it, then disconnect the path. For these > gains you should have DECLARE_TLV_DB_SCALE() and use SOC_SINGLE_TLV(). > Make sure that the control name matches with the corresponding DAPM > widget's name so ALSA can match them correctly. That's not clear for me. As I understand, I need to create SOC_SINGLE_TLV() for each gain I would like to change, with the same name as SOC_DAPM_SINGLE_AIC3x (for example "Line1L Switch" for Line1) and make callbacks with cached gain values. Am I right? May be the better sollution is to change SOC_DAPM_SINGLE_AIC3x on/off switch to other type with switch and gain control together? So it could have some connected states with different gains and disconnected state (Something like volume/mute control). Cheers, Tim
On 03/15/16 14:12, Timur Karaldin wrote: > Hi Peter! > > 14.03.2016 17:47, Peter Ujfalusi ?????: >> So the issue is that we have the DAPM switches controlling exactly the same >> registers. I believe if you would set the gain in a way that bit0 is 1, then >> DAPM will think that the path is disconnected. Also if you would set the >> gain and then mute and unmute the path you would have lost the gain you >> wanted to have.. The only way I can think of implementing these mixers is to >> have two sets of custom callbacks. one set is to set/get the gain and the >> other is to set/get the mute/disconnect on these. When the path is >> disconnected you should not write the gain change to the chip, but cache it >> and if the path is unmuted, you write the cached gain. When you mute the >> path you should take the set gain first, cache it, then disconnect the path. >> For these gains you should have DECLARE_TLV_DB_SCALE() and use >> SOC_SINGLE_TLV(). Make sure that the control name matches with the >> corresponding DAPM widget's name so ALSA can match them correctly. > That's not clear for me. As I understand, I need to create SOC_SINGLE_TLV() > for each gain I would like to change, with the same name as > SOC_DAPM_SINGLE_AIC3x (for example "Line1L Switch" for Line1) and make > callbacks with cached gain values. Am I right? Mostly yes, but you need to name the controls: existing switch + new gain control "Line1L Switch" + "Line1L Volume" "Line1R Switch" + "Line1R Volume" In this way alsamixer can combine them. > May be the better sollution is to change SOC_DAPM_SINGLE_AIC3x on/off switch > to other type with switch and gain control together? So it could have some > connected states with different gains and disconnected state (Something like > volume/mute control). Something based on SOC_DAPM_SINGLE_TLV should work. But that means again change in Kcontrol names.
Ok, I choose the way #1 without changing kcontrol names. Another thing I don't understand is the way for caching gain volume if I have switches muted. Is there any proper way for storing cached values between gain and switch handlers includes soft reset or I need manually check kcontrol name in handler and cache values in extended private structure? The same question for switch handler, because I need to share cached values between these handlers. 15.03.2016 15:40, Peter Ujfalusi ?????: > On 03/15/16 14:12, Timur Karaldin wrote: >> Hi Peter! >> >> 14.03.2016 17:47, Peter Ujfalusi ?????: >>> So the issue is that we have the DAPM switches controlling exactly the same >>> registers. I believe if you would set the gain in a way that bit0 is 1, then >>> DAPM will think that the path is disconnected. Also if you would set the >>> gain and then mute and unmute the path you would have lost the gain you >>> wanted to have.. The only way I can think of implementing these mixers is to >>> have two sets of custom callbacks. one set is to set/get the gain and the >>> other is to set/get the mute/disconnect on these. When the path is >>> disconnected you should not write the gain change to the chip, but cache it >>> and if the path is unmuted, you write the cached gain. When you mute the >>> path you should take the set gain first, cache it, then disconnect the path. >>> For these gains you should have DECLARE_TLV_DB_SCALE() and use >>> SOC_SINGLE_TLV(). Make sure that the control name matches with the >>> corresponding DAPM widget's name so ALSA can match them correctly. >> That's not clear for me. As I understand, I need to create SOC_SINGLE_TLV() >> for each gain I would like to change, with the same name as >> SOC_DAPM_SINGLE_AIC3x (for example "Line1L Switch" for Line1) and make >> callbacks with cached gain values. Am I right? > Mostly yes, but you need to name the controls: > existing switch + new gain control > "Line1L Switch" + "Line1L Volume" > "Line1R Switch" + "Line1R Volume" > > In this way alsamixer can combine them. > >> May be the better sollution is to change SOC_DAPM_SINGLE_AIC3x on/off switch >> to other type with switch and gain control together? So it could have some >> connected states with different gains and disconnected state (Something like >> volume/mute control). > Something based on SOC_DAPM_SINGLE_TLV should work. But that means again > change in Kcontrol names. >
Hi Timur, please avoid top posting. On 03/15/16 18:13, Timur Karaldin wrote: > Ok, I choose the way #1 without changing kcontrol names. > Another thing I don't understand is the way for caching gain volume if I have > switches muted. Is there any proper way for storing cached values between gain > and switch handlers includes soft reset or I need manually check kcontrol name > in handler and cache values in extended private structure? The same question > for switch handler, because I need to share cached values between these handlers. Since the problematic registers are: 17, 18, .. 24. I would have an array inside struct aic3x_priv to store the configured gain. You could store the mute as well, but since regmap is caching the content I would simply read back the gain value and if it is 0xf, it is muted, so update only the local shadow, if it is not muted, write the gain. In the mute/unmute controls I would then save the gain to shadow when muting the path and write the cached gain to the register when unmuting it. You should also return correct 0/1 for the unmute state and not the actual register value. > > 15.03.2016 15:40, Peter Ujfalusi ?????: >> On 03/15/16 14:12, Timur Karaldin wrote: >>> Hi Peter! >>> >>> 14.03.2016 17:47, Peter Ujfalusi ?????: >>>> So the issue is that we have the DAPM switches controlling exactly the same >>>> registers. I believe if you would set the gain in a way that bit0 is 1, then >>>> DAPM will think that the path is disconnected. Also if you would set the >>>> gain and then mute and unmute the path you would have lost the gain you >>>> wanted to have.. The only way I can think of implementing these mixers is to >>>> have two sets of custom callbacks. one set is to set/get the gain and the >>>> other is to set/get the mute/disconnect on these. When the path is >>>> disconnected you should not write the gain change to the chip, but cache it >>>> and if the path is unmuted, you write the cached gain. When you mute the >>>> path you should take the set gain first, cache it, then disconnect the path. >>>> For these gains you should have DECLARE_TLV_DB_SCALE() and use >>>> SOC_SINGLE_TLV(). Make sure that the control name matches with the >>>> corresponding DAPM widget's name so ALSA can match them correctly. >>> That's not clear for me. As I understand, I need to create SOC_SINGLE_TLV() >>> for each gain I would like to change, with the same name as >>> SOC_DAPM_SINGLE_AIC3x (for example "Line1L Switch" for Line1) and make >>> callbacks with cached gain values. Am I right? >> Mostly yes, but you need to name the controls: >> existing switch + new gain control >> "Line1L Switch" + "Line1L Volume" >> "Line1R Switch" + "Line1R Volume" >> >> In this way alsamixer can combine them. >> >>> May be the better sollution is to change SOC_DAPM_SINGLE_AIC3x on/off switch >>> to other type with switch and gain control together? So it could have some >>> connected states with different gains and disconnected state (Something like >>> volume/mute control). >> Something based on SOC_DAPM_SINGLE_TLV should work. But that means again >> change in Kcontrol names. >> > >
Hi Peter! 16.03.2016 11:57, Peter Ujfalusi ?????: > Hi Timur, > > please avoid top posting. > > On 03/15/16 18:13, Timur Karaldin wrote: >> Ok, I choose the way #1 without changing kcontrol names. >> Another thing I don't understand is the way for caching gain volume if I have >> switches muted. Is there any proper way for storing cached values between gain >> and switch handlers includes soft reset or I need manually check kcontrol name >> in handler and cache values in extended private structure? The same question >> for switch handler, because I need to share cached values between these handlers. > Since the problematic registers are: 17, 18, .. 24. I would have an array > inside struct aic3x_priv to store the configured gain. You could store the > mute as well, but since regmap is caching the content I would simply read back > the gain value and if it is 0xf, it is muted, so update only the local shadow, > if it is not muted, write the gain. > In the mute/unmute controls I would then save the gain to shadow when muting > the path and write the cached gain to the register when unmuting it. You > should also return correct 0/1 for the unmute state and not the actual > register value. Actually I try to avoid complex handler, with switch-case which depends on reg number or kcontrol name inside of it. I have found similar discussion with you about one year ago here http://mailman.alsa-project.org/pipermail/alsa-devel/2015-February/087430.html. There is a proposal to use TLV_DB_RANGE_HEAD() and TLV_DB_SCALE_ITEM() because of invert with SOC_DAPM_SINGLE_TLV. As I understand we could make tlv with using 0 as mute, skip values 1-6, use values 7-15 (7 = -12dB and 15=0dB) then declare SOC_DAPM_SINGLE_TLV as "Line1L Volume" for example. Is it still actual? Few things which I didn't understand: - should I use "Line1L Switch" together with "Line1L Volume" in this case. I think no, because DAPM_SINGLE_TLV should replace switch. - i don't understand how declare tlv with skipping values 1-6 Cheers, Tim
On 03/16/2016 12:26 PM, Timur Karaldin wrote: > Hi Peter! > > 16.03.2016 11:57, Peter Ujfalusi ?????: >> Hi Timur, >> >> please avoid top posting. >> >> On 03/15/16 18:13, Timur Karaldin wrote: >>> Ok, I choose the way #1 without changing kcontrol names. >>> Another thing I don't understand is the way for caching gain volume if I have >>> switches muted. Is there any proper way for storing cached values between gain >>> and switch handlers includes soft reset or I need manually check kcontrol name >>> in handler and cache values in extended private structure? The same question >>> for switch handler, because I need to share cached values between these >>> handlers. >> Since the problematic registers are: 17, 18, .. 24. I would have an array >> inside struct aic3x_priv to store the configured gain. You could store the >> mute as well, but since regmap is caching the content I would simply read back >> the gain value and if it is 0xf, it is muted, so update only the local shadow, >> if it is not muted, write the gain. >> In the mute/unmute controls I would then save the gain to shadow when muting >> the path and write the cached gain to the register when unmuting it. You >> should also return correct 0/1 for the unmute state and not the actual >> register value. your mail client does not seem to wrap the lines correctly, can you check that. > Actually I try to avoid complex handler, with switch-case which depends on reg > number or kcontrol name inside of it. > I have found similar discussion with you about one year ago here > http://mailman.alsa-project.org/pipermail/alsa-devel/2015-February/087430.html. There > is a proposal to use TLV_DB_RANGE_HEAD() and TLV_DB_SCALE_ITEM() because of > invert with SOC_DAPM_SINGLE_TLV. As I recall there were some issues with that back then, but can not recall. At least I don't see followup patches regarding to that. > As I understand we could make tlv with using > 0 as mute, skip values 1-6, use values 7-15 (7 = -12dB and 15=0dB) then > declare SOC_DAPM_SINGLE_TLV as "Line1L Volume" for example. Is it still > actual? Worth a try ;) The naming needs to be different, take a look at twl4030 codec's twl4030_dapm_dbypass_tlv and how it is used. > Few things which I didn't understand: > - should I use "Line1L Switch" together with "Line1L Volume" in this case. I > think no, because DAPM_SINGLE_TLV should replace switch. Since we will have the gain and mute with volume control we will have only one control. > - i don't understand how declare tlv with skipping values 1-6 Yeah, I'm not sure how leaving 'hole' between the TLV_DB_SCALE_ITEMs will be handled if it works. > > Cheers, Tim
16.03.2016 18:23, Peter Ujfalusi ?????: > On 03/16/2016 12:26 PM, Timur Karaldin wrote: >> Hi Peter! >> >> 16.03.2016 11:57, Peter Ujfalusi ?????: >>> Hi Timur, >>> >>> please avoid top posting. >>> >>> On 03/15/16 18:13, Timur Karaldin wrote: >>>> Ok, I choose the way #1 without changing kcontrol names. >>>> Another thing I don't understand is the way for caching gain volume if I have >>>> switches muted. Is there any proper way for storing cached values between gain >>>> and switch handlers includes soft reset or I need manually check kcontrol name >>>> in handler and cache values in extended private structure? The same question >>>> for switch handler, because I need to share cached values between these >>>> handlers. >>> Since the problematic registers are: 17, 18, .. 24. I would have an array >>> inside struct aic3x_priv to store the configured gain. You could store the >>> mute as well, but since regmap is caching the content I would simply read back >>> the gain value and if it is 0xf, it is muted, so update only the local shadow, >>> if it is not muted, write the gain. >>> In the mute/unmute controls I would then save the gain to shadow when muting >>> the path and write the cached gain to the register when unmuting it. You >>> should also return correct 0/1 for the unmute state and not the actual >>> register value. > your mail client does not seem to wrap the lines correctly, can you check that. I have no idea how these lines should looks, so it's very hard for me to see what's wrong. Could you point me how it looks in original? > >> Actually I try to avoid complex handler, with switch-case which depends on reg >> number or kcontrol name inside of it. >> I have found similar discussion with you about one year ago here >> http://mailman.alsa-project.org/pipermail/alsa-devel/2015-February/087430.html. There >> is a proposal to use TLV_DB_RANGE_HEAD() and TLV_DB_SCALE_ITEM() because of >> invert with SOC_DAPM_SINGLE_TLV. > As I recall there were some issues with that back then, but can not recall. At > least I don't see followup patches regarding to that. > >> As I understand we could make tlv with using >> 0 as mute, skip values 1-6, use values 7-15 (7 = -12dB and 15=0dB) then >> declare SOC_DAPM_SINGLE_TLV as "Line1L Volume" for example. Is it still >> actual? > Worth a try ;) > The naming needs to be different, take a look at twl4030 codec's > twl4030_dapm_dbypass_tlv and how it is used. > >> Few things which I didn't understand: >> - should I use "Line1L Switch" together with "Line1L Volume" in this case. I >> think no, because DAPM_SINGLE_TLV should replace switch. > Since we will have the gain and mute with volume control we will have only one > control. > >> - i don't understand how declare tlv with skipping values 1-6 > Yeah, I'm not sure how leaving 'hole' between the TLV_DB_SCALE_ITEMs will be > handled if it works. > >> Cheers, Tim Ok, now it's much more clear for me. Another question is register behaviour during soft reset. There is "aic3x_set_power" handle. In this handle kernel makes SOFT_RESET, markes cache as dirty, then power down the codec for handle power down request. But as I could see main volumes like "HP DAC" and "PCM" stores values between close and open in mixer and I could not see any code to handle it. On the other hand my controls do not save states, as you mentioned because of SOFT_RESET, could you explain such different behaviour? Cheers, Tim
On 03/16/16 18:53, Timur Karaldin wrote: >> your mail client does not seem to wrap the lines correctly, can you check that. > I have no idea how these lines should looks, so it's very hard for me to see > what's wrong. Could you point me how it looks in original? They are looong. see: https://wiki.openstack.org/wiki/MailingListEtiquette > Ok, now it's much more clear for me. > Another question is register behaviour during soft reset. There is > "aic3x_set_power" handle. In this handle kernel makes SOFT_RESET, markes cache > as dirty, then power down the codec for handle power down request. > But as I could see main volumes like "HP DAC" and "PCM" stores values between > close and open in mixer and I could not see any code to handle it. On the > other hand my controls do not save states, as you mentioned because of > SOFT_RESET, could you explain such different behaviour? All cached registers are going to be restored after power on with exception of volatile registers. You need to restore the bits in a volatile registers in the driver.
Hi Peter! 17.03.2016 12:26, Peter Ujfalusi ?????: >> Ok, now it's much more clear for me. >> Another question is register behaviour during soft reset. There is >> "aic3x_set_power" handle. In this handle kernel makes SOFT_RESET, markes cache >> as dirty, then power down the codec for handle power down request. >> But as I could see main volumes like "HP DAC" and "PCM" stores values between >> close and open in mixer and I could not see any code to handle it. On the >> other hand my controls do not save states, as you mentioned because of >> SOFT_RESET, could you explain such different behaviour? > All cached registers are going to be restored after power on with exception of > volatile registers. You need to restore the bits in a volatile registers in > the driver. > I'm very close to patch v2, but I have some problems with misunderstanding the key moment with dapm handler snd_soc_dapm_put_volsw_aic3x I think. I successfuly wrote put/get handlers for DOUBLE_R_EXT_TLV and SINGLE_EXT_TLV volume controls using snd_kcontrol_chip and snd_soc_codec_get_drvdata function for accessing codec's private data. It's work. But any attempts in snd_soc_dapm_put_volsw_aic3x to get priv data aix3x_priv* make kernel error: Unable to handle kernel NULL pointer dereference at virtual address 0000009e. If I remove snd_soc_codec_get_drvdata call and accessing to cached regs it's become normal again. Currently I use kernel 3.15 and there is some example in wm8903.c static int wm8903_class_w_put(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { struct snd_soc_codec *codec = snd_soc_dapm_kcontrol_codec(kcontrol); struct wm8903_priv *wm8903 = snd_soc_codec_get_drvdata(codec); ... and it's look ok, and I hope it's working, but not good for me in snd_soc_dapm_put_volsw_aic3x handler. Do you have any idea or any tips I should keep in mind, when working in DAPM context? Cheers, Tim
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index d7349bc..27d031e 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -122,11 +122,24 @@ static const struct reg_default aic3x_reg[] = { { 108, 0x00 }, { 109, 0x00 }, }; +static bool aic3106_volatile(struct device *dev, unsigned int reg) +{ + switch(reg) + { + case AIC3X_HEADSET_DETECT_CTRL_A: + case AIC3X_HEADSET_DETECT_CTRL_B: + return true; + default: + return false; + } +} + static const struct regmap_config aic3x_regmap = { .reg_bits = 8, .val_bits = 8, .max_register = DAC_ICC_ADJ, + .volatile_reg = aic3106_volatile, .reg_defaults = aic3x_reg, .num_reg_defaults = ARRAY_SIZE(aic3x_reg), .cache_type = REGCACHE_RBTREE, @@ -196,7 +209,7 @@ static int mic_bias_event(struct snd_soc_dapm_widget *w, struct snd_kcontrol *kcontrol, int event) { struct snd_soc_codec *codec = w->codec; - struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec); + struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec); switch (event) { case SND_SOC_DAPM_POST_PMU: @@ -270,6 +283,26 @@ static const struct soc_enum aic3x_agc_decay_enum[] = { SOC_ENUM_SINGLE(RAGC_CTRL_A, 0, 4, aic3x_agc_decay), }; +static const char *aic3x_adc_gain[] = { "0dB", "-1.5dB", "-3dB", "-4.5dB", "-6dB", "-7.5dB", "-9dB", "-10.5dB", "-12dB" }; +static const struct soc_enum aic3x_adc_gain_enum[] = { + SOC_ENUM_SINGLE(MIC3LR_2_LADC_CTRL, 4, 9, aic3x_adc_gain), + SOC_ENUM_SINGLE(MIC3LR_2_RADC_CTRL, 0, 9, aic3x_adc_gain), + SOC_ENUM_SINGLE(LINE1L_2_LADC_CTRL, 3, 9, aic3x_adc_gain), + SOC_ENUM_SINGLE(LINE1R_2_RADC_CTRL, 3, 9, aic3x_adc_gain), + SOC_ENUM_SINGLE(LINE2L_2_LADC_CTRL, 3, 9, aic3x_adc_gain), + SOC_ENUM_SINGLE(LINE2R_2_RADC_CTRL, 3, 9, aic3x_adc_gain), +}; +static const char *aic3x_headset_debounce[] = { "16ms", "32ms", "64ms", "128ms", "256ms", "512ms" }; +static const struct soc_enum aic3x_headset_debounce_enum[] = { + SOC_ENUM_SINGLE(AIC3X_HEADSET_DETECT_CTRL_A, 2, 6, aic3x_headset_debounce), +}; +static const char *aic3x_button_debounce[] = { "0ms", "8ms", "16ms", "32ms" }; +static const struct soc_enum aic3x_button_debounce_enum[] = { + SOC_ENUM_SINGLE(AIC3X_HEADSET_DETECT_CTRL_A, 0, 4, aic3x_button_debounce), +}; + + + /* * DAC digital volumes. From -63.5 to 0 dB in 0.5 dB steps */ @@ -399,6 +432,19 @@ static const struct snd_kcontrol_new aic3x_snd_controls[] = { SOC_DOUBLE_R("PGA Capture Switch", LADC_VOL, RADC_VOL, 7, 0x01, 1), SOC_ENUM("ADC HPF Cut-off", aic3x_enum[ADC_HPF_ENUM]), + /* Additional controls */ + SOC_ENUM("Mic3L LADC Gain", aic3x_adc_gain_enum[0]), + SOC_ENUM("Mic3R RADC Gain", aic3x_adc_gain_enum[1]), + SOC_ENUM("Line1L LADC Gain", aic3x_adc_gain_enum[2]), + SOC_ENUM("Line1R RADC Gain", aic3x_adc_gain_enum[3]), + SOC_ENUM("Line2L LADC Gain", aic3x_adc_gain_enum[4]), + SOC_ENUM("Line2R RADC Gain", aic3x_adc_gain_enum[5]), + SOC_ENUM("Headset Debounce", aic3x_headset_debounce_enum), + SOC_ENUM("Button Debounce", aic3x_button_debounce_enum), + SOC_SINGLE("Headset Detected", AIC3X_HEADSET_DETECT_CTRL_A, AIC3X_HEADSET_DETECT_SHIFT, AIC3X_HEADSET_DETECT_MASK, 0), + SOC_SINGLE("Headset Detect Enable", AIC3X_HEADSET_DETECT_CTRL_A, 7, 1, 0), + SOC_SINGLE("High power output Ac-coupled", AIC3X_HEADSET_DETECT_CTRL_B, 7, 1, 0), + SOC_SINGLE("Stereo pseudodifferential output", AIC3X_HEADSET_DETECT_CTRL_B, 3, 1, 0), }; static const struct snd_kcontrol_new aic3x_mono_controls[] = { @@ -614,7 +660,7 @@ static const struct snd_soc_dapm_widget aic3x_dapm_widgets[] = { SND_SOC_DAPM_REG(snd_soc_dapm_micbias, "DMic Rate 32", AIC3X_ASD_INTF_CTRLA, 0, 3, 3, 0), - /* Mic Bias */ + /* Mic Bias */ SND_SOC_DAPM_SUPPLY("Mic Bias", MICBIAS_CTRL, 6, 0, mic_bias_event, SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD), @@ -692,7 +738,7 @@ static const struct snd_soc_dapm_route intercon[] = { {"Left Line2L Mux", "single-ended", "LINE2L"}, {"Left Line2L Mux", "differential", "LINE2L"}, - {"Left PGA Mixer", "Line1L Switch", "Left Line1L Mux"}, + {"Left PGA Mixer", "Line1L Switch", "Left Line1L Mux"}, {"Left PGA Mixer", "Line1R Switch", "Left Line1R Mux"}, {"Left PGA Mixer", "Line2L Switch", "Left Line2L Mux"}, {"Left PGA Mixer", "Mic3L Switch", "MIC3L"}, @@ -814,6 +860,7 @@ static const struct snd_soc_dapm_route intercon[] = { {"Right HPCOM Mux", "external feedback", "Right HPCOM Mixer"}, {"Right HP Com", NULL, "Right HPCOM Mux"}, {"HPRCOM", NULL, "Right HP Com"}, + }; static const struct snd_soc_dapm_route intercon_mono[] = { @@ -918,7 +965,9 @@ static int aic3x_hw_params(struct snd_pcm_substream *substream, data = (LDAC2LCH | RDAC2RCH); data |= (fsref == 44100) ? FSREF_44100 : FSREF_48000; if (params_rate(params) >= 64000) + { data |= DUAL_RATE_MODE; + } snd_soc_write(codec, AIC3X_CODEC_DATAPATH_REG, data); /* codec sample rate select */ -- and tlv320aic3x.h --- sound/soc/codecs/tlv320aic3x.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sound/soc/codecs/tlv320aic3x.h b/sound/soc/codecs/tlv320aic3x.h index e521ac3..8ff42d2 100644 --- a/sound/soc/codecs/tlv320aic3x.h +++ b/sound/soc/codecs/tlv320aic3x.h @@ -271,6 +271,12 @@ enum { AIC3X_BUTTON_DEBOUNCE_32MS = 3 }; +typedef struct { + struct platform_device* pdev; + struct proc_dir_entry *proc_value; + struct snd_soc_codec *codec; +} aic3106_detect_t; + #define AIC3X_HEADSET_DETECT_ENABLED 0x80 #define AIC3X_HEADSET_DETECT_SHIFT 5 #define AIC3X_HEADSET_DETECT_MASK 3