[alsa-devel] ASoC: TLV320AIC3x: Adding additional functionality for 3106 with [Patch] for discuss

Peter Ujfalusi peter.ujfalusi at ti.com
Mon Mar 14 15:47:05 CET 2016


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.

-- 
Péter


More information about the Alsa-devel mailing list