[alsa-devel] [PATCH 0/2] ASoC: TWL4030: Cosmetic and PM related changes
Hello,
I'm not sure, if the first patch is acceptable by ASoC standards, but: Before the patch the analog capture path strings ended up like this at the end: "Analog Left Capture Route Main mic Switch" These switches showed up in the playback tab of alsamixer, which was not nice. After the patch the strings will be like this: "Analog Left Main mic Capture Route" Which show up correctly in alsamixer as "Analog Left Main mic" under the capture tab.
The second patch cleans up the output amplifier mute/un-mute by adding PGA_E to all output pins, which were handled by the twl4030_codec_mute function. Since those gain controls attached to the outputs are not in DAPM domain (yet?) and they have to be muted/turned off when they are not needed. Previously all of these (Earpiece PreDrivL/R, CarkitL/R) muted/un-muted at the same time without checking if the output actually is needed, there could be cases when the output amplifier is enabled but no actual audio data is routed there, which could leak some noise on the output.
I know about the changes in core that makes this hassle with the mute unnecesary (the core sets the BIAS_ON also when any bypass path is active), I'll be looking at that next.
--- Peter Ujfalusi (2): ASoC: TWL4030: Fix for capture mixer strings ASoC: TWL4030: Introduce PGAs for outputs
sound/soc/codecs/twl4030.c | 155 ++++++++++++++++++++++++-------------------- 1 files changed, 85 insertions(+), 70 deletions(-)
Change the strings related to capture in order to be interpreted correctly by alsamixer and possible other UI based mixer applications.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/twl4030.c | 46 +++++++++++++++++++++++-------------------- 1 files changed, 25 insertions(+), 21 deletions(-)
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index 818fb37..c64e314 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -443,16 +443,20 @@ SOC_DAPM_ENUM("Route", twl4030_vibrapath_enum);
/* Left analog microphone selection */ static const struct snd_kcontrol_new twl4030_dapm_analoglmic_controls[] = { - SOC_DAPM_SINGLE("Main mic", TWL4030_REG_ANAMICL, 0, 1, 0), - SOC_DAPM_SINGLE("Headset mic", TWL4030_REG_ANAMICL, 1, 1, 0), - SOC_DAPM_SINGLE("AUXL", TWL4030_REG_ANAMICL, 2, 1, 0), - SOC_DAPM_SINGLE("Carkit mic", TWL4030_REG_ANAMICL, 3, 1, 0), + SOC_DAPM_SINGLE("Main mic Capture Route", + TWL4030_REG_ANAMICL, 0, 1, 0), + SOC_DAPM_SINGLE("Headset mic Capture Route", + TWL4030_REG_ANAMICL, 1, 1, 0), + SOC_DAPM_SINGLE("AUXL Capture Route", + TWL4030_REG_ANAMICL, 2, 1, 0), + SOC_DAPM_SINGLE("Carkit mic Capture Route", + TWL4030_REG_ANAMICL, 3, 1, 0), };
/* Right analog microphone selection */ static const struct snd_kcontrol_new twl4030_dapm_analogrmic_controls[] = { - SOC_DAPM_SINGLE("Sub mic", TWL4030_REG_ANAMICR, 0, 1, 0), - SOC_DAPM_SINGLE("AUXR", TWL4030_REG_ANAMICR, 2, 1, 0), + SOC_DAPM_SINGLE("Sub mic Capture Route", TWL4030_REG_ANAMICR, 0, 1, 0), + SOC_DAPM_SINGLE("AUXR Capture Route", TWL4030_REG_ANAMICR, 2, 1, 0), };
/* TX1 L/R Analog/Digital microphone selection */ @@ -1327,11 +1331,11 @@ static const struct snd_soc_dapm_widget twl4030_dapm_widgets[] = { SND_SOC_DAPM_POST_REG),
/* Analog input mixers for the capture amplifiers */ - SND_SOC_DAPM_MIXER("Analog Left Capture Route", + SND_SOC_DAPM_MIXER("Analog Left", TWL4030_REG_ANAMICL, 4, 0, &twl4030_dapm_analoglmic_controls[0], ARRAY_SIZE(twl4030_dapm_analoglmic_controls)), - SND_SOC_DAPM_MIXER("Analog Right Capture Route", + SND_SOC_DAPM_MIXER("Analog Right", TWL4030_REG_ANAMICR, 4, 0, &twl4030_dapm_analogrmic_controls[0], ARRAY_SIZE(twl4030_dapm_analogrmic_controls)), @@ -1435,16 +1439,16 @@ static const struct snd_soc_dapm_route intercon[] = { {"VIBRA", NULL, "Vibra Route"},
/* Capture path */ - {"Analog Left Capture Route", "Main mic", "MAINMIC"}, - {"Analog Left Capture Route", "Headset mic", "HSMIC"}, - {"Analog Left Capture Route", "AUXL", "AUXL"}, - {"Analog Left Capture Route", "Carkit mic", "CARKITMIC"}, + {"Analog Left", "Main mic Capture Route", "MAINMIC"}, + {"Analog Left", "Headset mic Capture Route", "HSMIC"}, + {"Analog Left", "AUXL Capture Route", "AUXL"}, + {"Analog Left", "Carkit mic Capture Route", "CARKITMIC"},
- {"Analog Right Capture Route", "Sub mic", "SUBMIC"}, - {"Analog Right Capture Route", "AUXR", "AUXR"}, + {"Analog Right", "Sub mic Capture Route", "SUBMIC"}, + {"Analog Right", "AUXR Capture Route", "AUXR"},
- {"ADC Physical Left", NULL, "Analog Left Capture Route"}, - {"ADC Physical Right", NULL, "Analog Right Capture Route"}, + {"ADC Physical Left", NULL, "Analog Left"}, + {"ADC Physical Right", NULL, "Analog Right"},
{"Digimic0 Enable", NULL, "DIGIMIC0"}, {"Digimic1 Enable", NULL, "DIGIMIC1"}, @@ -1468,11 +1472,11 @@ static const struct snd_soc_dapm_route intercon[] = { {"ADC Virtual Right2", NULL, "TX2 Capture Route"},
/* Analog bypass routes */ - {"Right1 Analog Loopback", "Switch", "Analog Right Capture Route"}, - {"Left1 Analog Loopback", "Switch", "Analog Left Capture Route"}, - {"Right2 Analog Loopback", "Switch", "Analog Right Capture Route"}, - {"Left2 Analog Loopback", "Switch", "Analog Left Capture Route"}, - {"Voice Analog Loopback", "Switch", "Analog Left Capture Route"}, + {"Right1 Analog Loopback", "Switch", "Analog Right"}, + {"Left1 Analog Loopback", "Switch", "Analog Left"}, + {"Right2 Analog Loopback", "Switch", "Analog Right"}, + {"Left2 Analog Loopback", "Switch", "Analog Left"}, + {"Voice Analog Loopback", "Switch", "Analog Left"},
{"Analog R1 Playback Mixer", NULL, "Right1 Analog Loopback"}, {"Analog L1 Playback Mixer", NULL, "Left1 Analog Loopback"},
Dynamically control and control only the needed output amplifier muting/un-muting.
The original code was muting and un-muting the following output amplifiers: Earpiece PreDrivL/R, CarkitL/R at the same time regardless which pin is actually in use at the given moment.
Move these as separate PGA so only the needed amplifier will be touched.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/twl4030.c | 109 ++++++++++++++++++++++++-------------------- 1 files changed, 60 insertions(+), 49 deletions(-)
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index c64e314..4902226 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -225,55 +225,11 @@ static void twl4030_codec_mute(struct snd_soc_codec *codec, int mute) return;
if (mute) { - /* Bypass the reg_cache and mute the volumes - * Headset mute is done in it's own event handler - * Things to mute: Earpiece, PreDrivL/R, CarkitL/R - */ - reg_val = twl4030_read_reg_cache(codec, TWL4030_REG_EAR_CTL); - twl4030_i2c_write_u8(TWL4030_MODULE_AUDIO_VOICE, - reg_val & (~TWL4030_EAR_GAIN), - TWL4030_REG_EAR_CTL); - - reg_val = twl4030_read_reg_cache(codec, TWL4030_REG_PREDL_CTL); - twl4030_i2c_write_u8(TWL4030_MODULE_AUDIO_VOICE, - reg_val & (~TWL4030_PREDL_GAIN), - TWL4030_REG_PREDL_CTL); - reg_val = twl4030_read_reg_cache(codec, TWL4030_REG_PREDR_CTL); - twl4030_i2c_write_u8(TWL4030_MODULE_AUDIO_VOICE, - reg_val & (~TWL4030_PREDR_GAIN), - TWL4030_REG_PREDL_CTL); - - reg_val = twl4030_read_reg_cache(codec, TWL4030_REG_PRECKL_CTL); - twl4030_i2c_write_u8(TWL4030_MODULE_AUDIO_VOICE, - reg_val & (~TWL4030_PRECKL_GAIN), - TWL4030_REG_PRECKL_CTL); - reg_val = twl4030_read_reg_cache(codec, TWL4030_REG_PRECKR_CTL); - twl4030_i2c_write_u8(TWL4030_MODULE_AUDIO_VOICE, - reg_val & (~TWL4030_PRECKR_GAIN), - TWL4030_REG_PRECKR_CTL); - /* Disable PLL */ reg_val = twl4030_read_reg_cache(codec, TWL4030_REG_APLL_CTL); reg_val &= ~TWL4030_APLL_EN; twl4030_write(codec, TWL4030_REG_APLL_CTL, reg_val); } else { - /* Restore the volumes - * Headset mute is done in it's own event handler - * Things to restore: Earpiece, PreDrivL/R, CarkitL/R - */ - twl4030_write(codec, TWL4030_REG_EAR_CTL, - twl4030_read_reg_cache(codec, TWL4030_REG_EAR_CTL)); - - twl4030_write(codec, TWL4030_REG_PREDL_CTL, - twl4030_read_reg_cache(codec, TWL4030_REG_PREDL_CTL)); - twl4030_write(codec, TWL4030_REG_PREDR_CTL, - twl4030_read_reg_cache(codec, TWL4030_REG_PREDR_CTL)); - - twl4030_write(codec, TWL4030_REG_PRECKL_CTL, - twl4030_read_reg_cache(codec, TWL4030_REG_PRECKL_CTL)); - twl4030_write(codec, TWL4030_REG_PRECKR_CTL, - twl4030_read_reg_cache(codec, TWL4030_REG_PRECKR_CTL)); - /* Enable PLL */ reg_val = twl4030_read_reg_cache(codec, TWL4030_REG_APLL_CTL); reg_val |= TWL4030_APLL_EN; @@ -564,6 +520,41 @@ static int micpath_event(struct snd_soc_dapm_widget *w, return 0; }
+/* + * Output PGA builder: + * Handle the muting and unmuting of the given output (turning off the + * amplifier associated with the output pin) + * On mute bypass the reg_cache and mute the volume + * On unmute: restore the register content + * Outputs handled in this way: Earpiece, PreDrivL/R, CarkitL/R + */ +#define TWL4030_OUTPUT_PGA(pin_name, reg, mask) \ +static int pin_name##pga_event(struct snd_soc_dapm_widget *w, \ + struct snd_kcontrol *kcontrol, int event) \ +{ \ + u8 reg_val; \ + \ + switch (event) { \ + case SND_SOC_DAPM_POST_PMU: \ + twl4030_write(w->codec, reg, \ + twl4030_read_reg_cache(w->codec, reg)); \ + break; \ + case SND_SOC_DAPM_POST_PMD: \ + reg_val = twl4030_read_reg_cache(w->codec, reg); \ + twl4030_i2c_write_u8(TWL4030_MODULE_AUDIO_VOICE, \ + reg_val & (~mask), \ + reg); \ + break; \ + } \ + return 0; \ +} + +TWL4030_OUTPUT_PGA(earpiece, TWL4030_REG_EAR_CTL, TWL4030_EAR_GAIN); +TWL4030_OUTPUT_PGA(predrivel, TWL4030_REG_PREDL_CTL, TWL4030_PREDL_GAIN); +TWL4030_OUTPUT_PGA(predriver, TWL4030_REG_PREDR_CTL, TWL4030_PREDR_GAIN); +TWL4030_OUTPUT_PGA(carkitl, TWL4030_REG_PRECKL_CTL, TWL4030_PRECKL_GAIN); +TWL4030_OUTPUT_PGA(carkitr, TWL4030_REG_PRECKR_CTL, TWL4030_PRECKR_GAIN); + static void handsfree_ramp(struct snd_soc_codec *codec, int reg, int ramp) { unsigned char hs_ctl; @@ -1257,13 +1248,22 @@ static const struct snd_soc_dapm_widget twl4030_dapm_widgets[] = { SND_SOC_DAPM_MIXER("Earpiece Mixer", SND_SOC_NOPM, 0, 0, &twl4030_dapm_earpiece_controls[0], ARRAY_SIZE(twl4030_dapm_earpiece_controls)), + SND_SOC_DAPM_PGA_E("Earpiece PGA", SND_SOC_NOPM, + 0, 0, NULL, 0, earpiecepga_event, + SND_SOC_DAPM_POST_PMU|SND_SOC_DAPM_POST_PMD), /* PreDrivL/R */ SND_SOC_DAPM_MIXER("PredriveL Mixer", SND_SOC_NOPM, 0, 0, &twl4030_dapm_predrivel_controls[0], ARRAY_SIZE(twl4030_dapm_predrivel_controls)), + SND_SOC_DAPM_PGA_E("PredriveL PGA", SND_SOC_NOPM, + 0, 0, NULL, 0, predrivelpga_event, + SND_SOC_DAPM_POST_PMU|SND_SOC_DAPM_POST_PMD), SND_SOC_DAPM_MIXER("PredriveR Mixer", SND_SOC_NOPM, 0, 0, &twl4030_dapm_predriver_controls[0], ARRAY_SIZE(twl4030_dapm_predriver_controls)), + SND_SOC_DAPM_PGA_E("PredriveR PGA", SND_SOC_NOPM, + 0, 0, NULL, 0, predriverpga_event, + SND_SOC_DAPM_POST_PMU|SND_SOC_DAPM_POST_PMD), /* HeadsetL/R */ SND_SOC_DAPM_MIXER("HeadsetL Mixer", SND_SOC_NOPM, 0, 0, &twl4030_dapm_hsol_controls[0], @@ -1281,9 +1281,15 @@ static const struct snd_soc_dapm_widget twl4030_dapm_widgets[] = { SND_SOC_DAPM_MIXER("CarkitL Mixer", SND_SOC_NOPM, 0, 0, &twl4030_dapm_carkitl_controls[0], ARRAY_SIZE(twl4030_dapm_carkitl_controls)), + SND_SOC_DAPM_PGA_E("CarkitL PGA", SND_SOC_NOPM, + 0, 0, NULL, 0, carkitlpga_event, + SND_SOC_DAPM_POST_PMU|SND_SOC_DAPM_POST_PMD), SND_SOC_DAPM_MIXER("CarkitR Mixer", SND_SOC_NOPM, 0, 0, &twl4030_dapm_carkitr_controls[0], ARRAY_SIZE(twl4030_dapm_carkitr_controls)), + SND_SOC_DAPM_PGA_E("CarkitR PGA", SND_SOC_NOPM, + 0, 0, NULL, 0, carkitrpga_event, + SND_SOC_DAPM_POST_PMU|SND_SOC_DAPM_POST_PMD),
/* Output MUX controls */ /* HandsfreeL/R */ @@ -1375,16 +1381,19 @@ static const struct snd_soc_dapm_route intercon[] = { {"Earpiece Mixer", "AudioL1", "Analog L1 Playback Mixer"}, {"Earpiece Mixer", "AudioL2", "Analog L2 Playback Mixer"}, {"Earpiece Mixer", "AudioR1", "Analog R1 Playback Mixer"}, + {"Earpiece PGA", NULL, "Earpiece Mixer"}, /* PreDrivL */ {"PredriveL Mixer", "Voice", "Analog Voice Playback Mixer"}, {"PredriveL Mixer", "AudioL1", "Analog L1 Playback Mixer"}, {"PredriveL Mixer", "AudioL2", "Analog L2 Playback Mixer"}, {"PredriveL Mixer", "AudioR2", "Analog R2 Playback Mixer"}, + {"PredriveL PGA", NULL, "PredriveL Mixer"}, /* PreDrivR */ {"PredriveR Mixer", "Voice", "Analog Voice Playback Mixer"}, {"PredriveR Mixer", "AudioR1", "Analog R1 Playback Mixer"}, {"PredriveR Mixer", "AudioR2", "Analog R2 Playback Mixer"}, {"PredriveR Mixer", "AudioL2", "Analog L2 Playback Mixer"}, + {"PredriveR PGA", NULL, "PredriveR Mixer"}, /* HeadsetL */ {"HeadsetL Mixer", "Voice", "Analog Voice Playback Mixer"}, {"HeadsetL Mixer", "AudioL1", "Analog L1 Playback Mixer"}, @@ -1399,10 +1408,12 @@ static const struct snd_soc_dapm_route intercon[] = { {"CarkitL Mixer", "Voice", "Analog Voice Playback Mixer"}, {"CarkitL Mixer", "AudioL1", "Analog L1 Playback Mixer"}, {"CarkitL Mixer", "AudioL2", "Analog L2 Playback Mixer"}, + {"CarkitL PGA", NULL, "CarkitL Mixer"}, /* CarkitR */ {"CarkitR Mixer", "Voice", "Analog Voice Playback Mixer"}, {"CarkitR Mixer", "AudioR1", "Analog R1 Playback Mixer"}, {"CarkitR Mixer", "AudioR2", "Analog R2 Playback Mixer"}, + {"CarkitR PGA", NULL, "CarkitR Mixer"}, /* HandsfreeL */ {"HandsfreeL Mux", "Voice", "Analog Voice Playback Mixer"}, {"HandsfreeL Mux", "AudioL1", "Analog L1 Playback Mixer"}, @@ -1426,13 +1437,13 @@ static const struct snd_soc_dapm_route intercon[] = { /* outputs */ {"OUTL", NULL, "Analog L2 Playback Mixer"}, {"OUTR", NULL, "Analog R2 Playback Mixer"}, - {"EARPIECE", NULL, "Earpiece Mixer"}, - {"PREDRIVEL", NULL, "PredriveL Mixer"}, - {"PREDRIVER", NULL, "PredriveR Mixer"}, + {"EARPIECE", NULL, "Earpiece PGA"}, + {"PREDRIVEL", NULL, "PredriveL PGA"}, + {"PREDRIVER", NULL, "PredriveR PGA"}, {"HSOL", NULL, "HeadsetL PGA"}, {"HSOR", NULL, "HeadsetR PGA"}, - {"CARKITL", NULL, "CarkitL Mixer"}, - {"CARKITR", NULL, "CarkitR Mixer"}, + {"CARKITL", NULL, "CarkitL PGA"}, + {"CARKITR", NULL, "CarkitR PGA"}, {"HFL", NULL, "HandsfreeL PGA"}, {"HFR", NULL, "HandsfreeR PGA"}, {"Vibra Route", "Audio", "Vibra Mux"},
On Thu, Aug 13, 2009 at 03:59:34PM +0300, Peter Ujfalusi wrote:
Dynamically control and control only the needed output amplifier muting/un-muting.
The original code was muting and un-muting the following output amplifiers: Earpiece PreDrivL/R, CarkitL/R at the same time regardless which pin is actually in use at the given moment.
Move these as separate PGA so only the needed amplifier will be touched.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com
Applied, thanks.
On Thu, Aug 13, 2009 at 03:59:33PM +0300, Peter Ujfalusi wrote:
static const struct snd_kcontrol_new twl4030_dapm_analoglmic_controls[] = {
- SOC_DAPM_SINGLE("Main mic", TWL4030_REG_ANAMICL, 0, 1, 0),
- SOC_DAPM_SINGLE("Headset mic", TWL4030_REG_ANAMICL, 1, 1, 0),
- SOC_DAPM_SINGLE("AUXL", TWL4030_REG_ANAMICL, 2, 1, 0),
- SOC_DAPM_SINGLE("Carkit mic", TWL4030_REG_ANAMICL, 3, 1, 0),
- SOC_DAPM_SINGLE("Main mic Capture Route",
TWL4030_REG_ANAMICL, 0, 1, 0),- SOC_DAPM_SINGLE("Headset mic Capture Route",
TWL4030_REG_ANAMICL, 1, 1, 0),- SOC_DAPM_SINGLE("AUXL Capture Route",
TWL4030_REG_ANAMICL, 2, 1, 0),- SOC_DAPM_SINGLE("Carkit mic Capture Route",
TWL4030_REG_ANAMICL, 3, 1, 0),
These should really be "foo Switch" - a switch is an on/off control. If there's a volume control associated with the path then alsamixer will show the pair as a combined volume/mute control if they have the same name but with Volume and Switch as the last word.
I'd also spell "mic" "Mic".
participants (2)
-
Mark Brown -
Peter Ujfalusi