[alsa-devel] [PATCH 0/6] ASoC: TWL4030: Cleanups and analog loopback support
Hello, The following series - at the end - will add analog loopback/bypass support for the twl4030 codec.
The main changes: [1] ANAMICL register self cleaning bit handling [2] Simplified power up/down function [3] Move the Headset anti-pop/bias ramp enabling to be happend only if the headset is actually in use [4] Change the place for the DAC power switch [5] Preparation for the loopback implementation [6] Analog loopback/bypass implementation
You can find a bit more detailed notes in the actuall patches.
In short: if none of the loopbacks are enabled, the codec behaves in the same way as it used to. If the loopback is enabled the codec will be kept powered as long as the loopback is enabled.
P�ter
--- Peter Ujfalusi (6): ASoC: TWL4030: Syncronize the reg_cache for ANAMICL after the offset cancelation ASoC: TWL4030: Code clean up for codec power up and down ASoC: TWL4030: Enable Headset Left anti-pop/bias ramp only if the Headset Left is in use ASoC: TWL4030: Physical ADC and amplifier power switch change ASoC: TWL4030: Move the twl4030_power_up and _power_down function ASoC: TWL4030: Add analog loopback support
sound/soc/codecs/twl4030.c | 340 ++++++++++++++++++++++++++++++-------------- 1 files changed, 232 insertions(+), 108 deletions(-)
The offset cancelation bit in ANAMICL register is self cleanig. Make sure that the reg_cache holds the same value as the HW register.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/twl4030.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index f530c1e..df72a21 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -913,6 +913,9 @@ static void twl4030_power_up(struct snd_soc_codec *codec) ((byte & TWL4030_CNCL_OFFSET_START) == TWL4030_CNCL_OFFSET_START));
+ /* Make sure that the reg_cache has the same value as the HW */ + twl4030_write_reg_cache(codec, TWL4030_REG_ANAMICL, byte); + /* anti-pop when changing analog gain */ regmisc1 = twl4030_read_reg_cache(codec, TWL4030_REG_MISC_SET_1); twl4030_write(codec, TWL4030_REG_MISC_SET_1,
Merge the codec up and down functions to a simple one. Codec is powered down by default (reg_cache change).
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/twl4030.c | 43 +++++++++++++++++-------------------------- 1 files changed, 17 insertions(+), 26 deletions(-)
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index df72a21..7d35cd4 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -42,7 +42,7 @@ */ static const u8 twl4030_reg[TWL4030_CACHEREGNUM] = { 0x00, /* this register not used */ - 0x93, /* REG_CODEC_MODE (0x1) */ + 0x91, /* REG_CODEC_MODE (0x1) */ 0xc3, /* REG_OPTION (0x2) */ 0x00, /* REG_UNKNOWN (0x3) */ 0x00, /* REG_MICBIAS_CTL (0x4) */ @@ -154,26 +154,17 @@ static int twl4030_write(struct snd_soc_codec *codec, return twl4030_i2c_write_u8(TWL4030_MODULE_AUDIO_VOICE, value, reg); }
-static void twl4030_clear_codecpdz(struct snd_soc_codec *codec) +static void twl4030_codec_enable(struct snd_soc_codec *codec, int enable) { u8 mode;
mode = twl4030_read_reg_cache(codec, TWL4030_REG_CODEC_MODE); - twl4030_write(codec, TWL4030_REG_CODEC_MODE, - mode & ~TWL4030_CODECPDZ); + if (enable) + mode |= TWL4030_CODECPDZ; + else + mode &= ~TWL4030_CODECPDZ;
- /* REVISIT: this delay is present in TI sample drivers */ - /* but there seems to be no TRM requirement for it */ - udelay(10); -} - -static void twl4030_set_codecpdz(struct snd_soc_codec *codec) -{ - u8 mode; - - mode = twl4030_read_reg_cache(codec, TWL4030_REG_CODEC_MODE); - twl4030_write(codec, TWL4030_REG_CODEC_MODE, - mode | TWL4030_CODECPDZ); + twl4030_write(codec, TWL4030_REG_CODEC_MODE, mode);
/* REVISIT: this delay is present in TI sample drivers */ /* but there seems to be no TRM requirement for it */ @@ -185,7 +176,7 @@ static void twl4030_init_chip(struct snd_soc_codec *codec) int i;
/* clear CODECPDZ prior to setting register defaults */ - twl4030_clear_codecpdz(codec); + twl4030_codec_enable(codec, 0);
/* set all audio section registers to reasonable defaults */ for (i = TWL4030_REG_OPTION; i <= TWL4030_REG_MISC_SET_2; i++) @@ -895,7 +886,7 @@ static void twl4030_power_up(struct snd_soc_codec *codec) int i = 0;
/* set CODECPDZ to turn on codec */ - twl4030_set_codecpdz(codec); + twl4030_codec_enable(codec, 1);
/* initiate offset cancellation */ anamicl = twl4030_read_reg_cache(codec, TWL4030_REG_ANAMICL); @@ -922,8 +913,8 @@ static void twl4030_power_up(struct snd_soc_codec *codec) regmisc1 | TWL4030_SMOOTH_ANAVOL_EN);
/* toggle CODECPDZ as per TRM */ - twl4030_clear_codecpdz(codec); - twl4030_set_codecpdz(codec); + twl4030_codec_enable(codec, 0); + twl4030_codec_enable(codec, 1);
/* program anti-pop with bias ramp delay */ popn = twl4030_read_reg_cache(codec, TWL4030_REG_HS_POPN_SET); @@ -952,7 +943,7 @@ static void twl4030_power_down(struct snd_soc_codec *codec) twl4030_write(codec, TWL4030_REG_HS_POPN_SET, popn);
/* power down */ - twl4030_clear_codecpdz(codec); + twl4030_codec_enable(codec, 0); }
static int twl4030_set_bias_level(struct snd_soc_codec *codec, @@ -1030,7 +1021,7 @@ static int twl4030_hw_params(struct snd_pcm_substream *substream, if (mode != old_mode) { /* change rate and set CODECPDZ */ twl4030_write(codec, TWL4030_REG_CODEC_MODE, mode); - twl4030_set_codecpdz(codec); + twl4030_codec_enable(codec, 1); }
/* sample size */ @@ -1053,13 +1044,13 @@ static int twl4030_hw_params(struct snd_pcm_substream *substream, if (format != old_format) {
/* clear CODECPDZ before changing format (codec requirement) */ - twl4030_clear_codecpdz(codec); + twl4030_codec_enable(codec, 0);
/* change format */ twl4030_write(codec, TWL4030_REG_AUDIO_IF, format);
/* set CODECPDZ afterwards */ - twl4030_set_codecpdz(codec); + twl4030_codec_enable(codec, 1); } return 0; } @@ -1129,13 +1120,13 @@ static int twl4030_set_dai_fmt(struct snd_soc_dai *codec_dai, if (format != old_format) {
/* clear CODECPDZ before changing format (codec requirement) */ - twl4030_clear_codecpdz(codec); + twl4030_codec_enable(codec, 0);
/* change format */ twl4030_write(codec, TWL4030_REG_AUDIO_IF, format);
/* set CODECPDZ afterwards */ - twl4030_set_codecpdz(codec); + twl4030_codec_enable(codec, 1); }
return 0;
The Headset Left anti-pop and bias ramp does not need to be enabled, if the headset is not in use. Move the code to DAPM event handler for HeadsetL.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/twl4030.c | 71 ++++++++++++++++++++++++++++---------------- 1 files changed, 45 insertions(+), 26 deletions(-)
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index 7d35cd4..0e3198f 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -414,6 +414,47 @@ static int handsfree_event(struct snd_soc_dapm_widget *w, return 0; }
+static int headsetl_event(struct snd_soc_dapm_widget *w, + struct snd_kcontrol *kcontrol, int event) +{ + unsigned char hs_gain, hs_pop; + + /* Save the current volume */ + hs_gain = twl4030_read_reg_cache(w->codec, TWL4030_REG_HS_GAIN_SET); + + switch (event) { + case SND_SOC_DAPM_POST_PMU: + /* Do the anti-pop/bias ramp enable according to the TRM */ + hs_pop = TWL4030_RAMP_DELAY_645MS; + twl4030_write(w->codec, TWL4030_REG_HS_POPN_SET, hs_pop); + hs_pop |= TWL4030_VMID_EN; + twl4030_write(w->codec, TWL4030_REG_HS_POPN_SET, hs_pop); + /* Is this needed? Can we just use whatever gain here? */ + twl4030_write(w->codec, TWL4030_REG_HS_GAIN_SET, + (hs_gain & (~0x0f)) | 0x0a); + hs_pop |= TWL4030_RAMP_EN; + twl4030_write(w->codec, TWL4030_REG_HS_POPN_SET, hs_pop); + + /* Restore the original volume */ + twl4030_write(w->codec, TWL4030_REG_HS_GAIN_SET, hs_gain); + break; + case SND_SOC_DAPM_POST_PMD: + /* Do the anti-pop/bias ramp disable according to the TRM */ + hs_pop = twl4030_read_reg_cache(w->codec, + TWL4030_REG_HS_POPN_SET); + hs_pop &= ~TWL4030_RAMP_EN; + twl4030_write(w->codec, TWL4030_REG_HS_POPN_SET, hs_pop); + /* Bypass the reg_cache to mute the headset */ + twl4030_i2c_write_u8(TWL4030_MODULE_AUDIO_VOICE, + hs_gain & (~0x0f), + TWL4030_REG_HS_GAIN_SET); + hs_pop &= ~TWL4030_VMID_EN; + twl4030_write(w->codec, TWL4030_REG_HS_POPN_SET, hs_pop); + break; + } + return 0; +} + /* * Some of the gain controls in TWL (mostly those which are associated with * the outputs) are implemented in an interesting way: @@ -720,8 +761,9 @@ static const struct snd_soc_dapm_widget twl4030_dapm_widgets[] = { SND_SOC_DAPM_VALUE_MUX("PredriveR Mux", SND_SOC_NOPM, 0, 0, &twl4030_dapm_predriver_control), /* HeadsetL/R */ - SND_SOC_DAPM_MUX("HeadsetL Mux", SND_SOC_NOPM, 0, 0, - &twl4030_dapm_hsol_control), + SND_SOC_DAPM_MUX_E("HeadsetL Mux", SND_SOC_NOPM, 0, 0, + &twl4030_dapm_hsol_control, headsetl_event, + SND_SOC_DAPM_POST_PMU|SND_SOC_DAPM_POST_PMD), SND_SOC_DAPM_MUX("HeadsetR Mux", SND_SOC_NOPM, 0, 0, &twl4030_dapm_hsor_control), /* CarkitL/R */ @@ -882,7 +924,7 @@ static int twl4030_add_widgets(struct snd_soc_codec *codec)
static void twl4030_power_up(struct snd_soc_codec *codec) { - u8 anamicl, regmisc1, byte, popn; + u8 anamicl, regmisc1, byte; int i = 0;
/* set CODECPDZ to turn on codec */ @@ -915,33 +957,10 @@ static void twl4030_power_up(struct snd_soc_codec *codec) /* toggle CODECPDZ as per TRM */ twl4030_codec_enable(codec, 0); twl4030_codec_enable(codec, 1); - - /* program anti-pop with bias ramp delay */ - popn = twl4030_read_reg_cache(codec, TWL4030_REG_HS_POPN_SET); - popn &= TWL4030_RAMP_DELAY; - popn |= TWL4030_RAMP_DELAY_645MS; - twl4030_write(codec, TWL4030_REG_HS_POPN_SET, popn); - popn |= TWL4030_VMID_EN; - twl4030_write(codec, TWL4030_REG_HS_POPN_SET, popn); - - /* enable anti-pop ramp */ - popn |= TWL4030_RAMP_EN; - twl4030_write(codec, TWL4030_REG_HS_POPN_SET, popn); }
static void twl4030_power_down(struct snd_soc_codec *codec) { - u8 popn; - - /* disable anti-pop ramp */ - popn = twl4030_read_reg_cache(codec, TWL4030_REG_HS_POPN_SET); - popn &= ~TWL4030_RAMP_EN; - twl4030_write(codec, TWL4030_REG_HS_POPN_SET, popn); - - /* disable bias out */ - popn &= ~TWL4030_VMID_EN; - twl4030_write(codec, TWL4030_REG_HS_POPN_SET, popn); - /* power down */ twl4030_codec_enable(codec, 0); }
Change the power switches for the physical ADC and for the amplifiers for the analog capture path to map more closely the actual path inside of the codec.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/twl4030.c | 26 +++++++++++++------------- 1 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index 0e3198f..651be7f 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -802,16 +802,16 @@ static const struct snd_soc_dapm_widget twl4030_dapm_widgets[] = { SND_SOC_DAPM_POST_PMU|SND_SOC_DAPM_POST_PMD| SND_SOC_DAPM_POST_REG),
- /* Analog input muxes with power switch for the physical ADCL/R */ + /* Analog input muxes with switch for the capture amplifiers */ SND_SOC_DAPM_VALUE_MUX("Analog Left Capture Route", - TWL4030_REG_AVADC_CTL, 3, 0, &twl4030_dapm_analoglmic_control), + TWL4030_REG_ANAMICL, 4, 0, &twl4030_dapm_analoglmic_control), SND_SOC_DAPM_VALUE_MUX("Analog Right Capture Route", - TWL4030_REG_AVADC_CTL, 1, 0, &twl4030_dapm_analogrmic_control), + TWL4030_REG_ANAMICR, 4, 0, &twl4030_dapm_analogrmic_control),
- SND_SOC_DAPM_PGA("Analog Left Amplifier", - TWL4030_REG_ANAMICL, 4, 0, NULL, 0), - SND_SOC_DAPM_PGA("Analog Right Amplifier", - TWL4030_REG_ANAMICR, 4, 0, NULL, 0), + SND_SOC_DAPM_PGA("ADC Physical Left", + TWL4030_REG_AVADC_CTL, 3, 0, NULL, 0), + SND_SOC_DAPM_PGA("ADC Physical Right", + TWL4030_REG_AVADC_CTL, 1, 0, NULL, 0),
SND_SOC_DAPM_PGA("Digimic0 Enable", TWL4030_REG_ADCMICSEL, 1, 0, NULL, 0), @@ -885,23 +885,23 @@ static const struct snd_soc_dapm_route intercon[] = { {"Analog Right Capture Route", "Sub mic", "SUBMIC"}, {"Analog Right Capture Route", "AUXR", "AUXR"},
- {"Analog Left Amplifier", NULL, "Analog Left Capture Route"}, - {"Analog Right Amplifier", NULL, "Analog Right Capture Route"}, + {"ADC Physical Left", NULL, "Analog Left Capture Route"}, + {"ADC Physical Right", NULL, "Analog Right Capture Route"},
{"Digimic0 Enable", NULL, "DIGIMIC0"}, {"Digimic1 Enable", NULL, "DIGIMIC1"},
/* TX1 Left capture path */ - {"TX1 Capture Route", "Analog", "Analog Left Amplifier"}, + {"TX1 Capture Route", "Analog", "ADC Physical Left"}, {"TX1 Capture Route", "Digimic0", "Digimic0 Enable"}, /* TX1 Right capture path */ - {"TX1 Capture Route", "Analog", "Analog Right Amplifier"}, + {"TX1 Capture Route", "Analog", "ADC Physical Right"}, {"TX1 Capture Route", "Digimic0", "Digimic0 Enable"}, /* TX2 Left capture path */ - {"TX2 Capture Route", "Analog", "Analog Left Amplifier"}, + {"TX2 Capture Route", "Analog", "ADC Physical Left"}, {"TX2 Capture Route", "Digimic1", "Digimic1 Enable"}, /* TX2 Right capture path */ - {"TX2 Capture Route", "Analog", "Analog Right Amplifier"}, + {"TX2 Capture Route", "Analog", "ADC Physical Right"}, {"TX2 Capture Route", "Digimic1", "Digimic1 Enable"},
{"ADC Virtual Left1", NULL, "TX1 Capture Route"},
Move the twl4030_power_up and twl4030_power_down function earlier to facilitate the analog bypass implementation.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/twl4030.c | 85 +++++++++++++++++++++---------------------- 1 files changed, 42 insertions(+), 43 deletions(-)
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index 651be7f..8c77271 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -184,6 +184,48 @@ static void twl4030_init_chip(struct snd_soc_codec *codec)
}
+static void twl4030_power_up(struct snd_soc_codec *codec) +{ + u8 anamicl, regmisc1, byte; + int i = 0; + + /* set CODECPDZ to turn on codec */ + twl4030_codec_enable(codec, 1); + + /* initiate offset cancellation */ + anamicl = twl4030_read_reg_cache(codec, TWL4030_REG_ANAMICL); + twl4030_write(codec, TWL4030_REG_ANAMICL, + anamicl | TWL4030_CNCL_OFFSET_START); + + /* wait for offset cancellation to complete */ + do { + /* this takes a little while, so don't slam i2c */ + udelay(2000); + twl4030_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &byte, + TWL4030_REG_ANAMICL); + } while ((i++ < 100) && + ((byte & TWL4030_CNCL_OFFSET_START) == + TWL4030_CNCL_OFFSET_START)); + + /* Make sure that the reg_cache has the same value as the HW */ + twl4030_write_reg_cache(codec, TWL4030_REG_ANAMICL, byte); + + /* anti-pop when changing analog gain */ + regmisc1 = twl4030_read_reg_cache(codec, TWL4030_REG_MISC_SET_1); + twl4030_write(codec, TWL4030_REG_MISC_SET_1, + regmisc1 | TWL4030_SMOOTH_ANAVOL_EN); + + /* toggle CODECPDZ as per TRM */ + twl4030_codec_enable(codec, 0); + twl4030_codec_enable(codec, 1); +} + +static void twl4030_power_down(struct snd_soc_codec *codec) +{ + /* power down */ + twl4030_codec_enable(codec, 0); +} + /* Earpiece */ static const char *twl4030_earpiece_texts[] = {"Off", "DACL1", "DACL2", "DACR1"}; @@ -922,49 +964,6 @@ static int twl4030_add_widgets(struct snd_soc_codec *codec) return 0; }
-static void twl4030_power_up(struct snd_soc_codec *codec) -{ - u8 anamicl, regmisc1, byte; - int i = 0; - - /* set CODECPDZ to turn on codec */ - twl4030_codec_enable(codec, 1); - - /* initiate offset cancellation */ - anamicl = twl4030_read_reg_cache(codec, TWL4030_REG_ANAMICL); - twl4030_write(codec, TWL4030_REG_ANAMICL, - anamicl | TWL4030_CNCL_OFFSET_START); - - - /* wait for offset cancellation to complete */ - do { - /* this takes a little while, so don't slam i2c */ - udelay(2000); - twl4030_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &byte, - TWL4030_REG_ANAMICL); - } while ((i++ < 100) && - ((byte & TWL4030_CNCL_OFFSET_START) == - TWL4030_CNCL_OFFSET_START)); - - /* Make sure that the reg_cache has the same value as the HW */ - twl4030_write_reg_cache(codec, TWL4030_REG_ANAMICL, byte); - - /* anti-pop when changing analog gain */ - regmisc1 = twl4030_read_reg_cache(codec, TWL4030_REG_MISC_SET_1); - twl4030_write(codec, TWL4030_REG_MISC_SET_1, - regmisc1 | TWL4030_SMOOTH_ANAVOL_EN); - - /* toggle CODECPDZ as per TRM */ - twl4030_codec_enable(codec, 0); - twl4030_codec_enable(codec, 1); -} - -static void twl4030_power_down(struct snd_soc_codec *codec) -{ - /* power down */ - twl4030_codec_enable(codec, 0); -} - static int twl4030_set_bias_level(struct snd_soc_codec *codec, enum snd_soc_bias_level level) {
This patch adds the analog loopback/bypass support for twl4030 codec.
Details for the implementation: It seams that the analog loopback needs the DAC powered on on the channel, where the loopback is selected. The switch for the DACs has been moved from the DAPM_DAC to the "Analog XX Playback Mixer". In this way the DAC will be powered while the audio playback is used or/and the loopback is enabled for the channel.
After the loopback registers are changed, the event callback will update the bypass bitfield, so we will know if any of the bypass paths are enabled. If the codec is in STANDBY mode, the decision is made to keep/switch the codec off or keep/switch the codec on to allow the bypass. Also after the codec switches from ON to STANDBY the same check will be done.
In short: if none of the loopbacks are enabled, the codec behaves in the same way as it used to. If the loopback is enabled the codec will be kept powered as long as the bypass is enabled.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/twl4030.c | 132 ++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 122 insertions(+), 10 deletions(-)
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index 8c77271..7fdc959 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -117,6 +117,12 @@ static const u8 twl4030_reg[TWL4030_CACHEREGNUM] = { 0x00, /* REG_MISC_SET_2 (0x49) */ };
+/* codec private data */ +struct twl4030_priv { + unsigned int bypass_state; + unsigned int codec_powered; +}; + /* * read twl4030 register cache */ @@ -156,8 +162,13 @@ static int twl4030_write(struct snd_soc_codec *codec,
static void twl4030_codec_enable(struct snd_soc_codec *codec, int enable) { + struct twl4030_priv *twl4030 = + (struct twl4030_priv *)codec->private_data; u8 mode;
+ if (enable == twl4030->codec_powered) + return; + mode = twl4030_read_reg_cache(codec, TWL4030_REG_CODEC_MODE); if (enable) mode |= TWL4030_CODECPDZ; @@ -165,6 +176,7 @@ static void twl4030_codec_enable(struct snd_soc_codec *codec, int enable) mode &= ~TWL4030_CODECPDZ;
twl4030_write(codec, TWL4030_REG_CODEC_MODE, mode); + twl4030->codec_powered = enable;
/* REVISIT: this delay is present in TI sample drivers */ /* but there seems to be no TRM requirement for it */ @@ -186,9 +198,14 @@ static void twl4030_init_chip(struct snd_soc_codec *codec)
static void twl4030_power_up(struct snd_soc_codec *codec) { + struct twl4030_priv *twl4030 = + (struct twl4030_priv *)codec->private_data; u8 anamicl, regmisc1, byte; int i = 0;
+ if (twl4030->codec_powered) + return; + /* set CODECPDZ to turn on codec */ twl4030_codec_enable(codec, 1);
@@ -222,8 +239,15 @@ static void twl4030_power_up(struct snd_soc_codec *codec)
static void twl4030_power_down(struct snd_soc_codec *codec) { - /* power down */ - twl4030_codec_enable(codec, 0); + struct twl4030_priv *twl4030 = + (struct twl4030_priv *)codec->private_data; + + if (twl4030->bypass_state) + /* Power up, or leave the power on */ + twl4030_power_up(codec); + else + /* power down */ + twl4030_codec_enable(codec, 0); }
/* Earpiece */ @@ -402,6 +426,22 @@ static const struct soc_enum twl4030_micpathtx2_enum = static const struct snd_kcontrol_new twl4030_dapm_micpathtx2_control = SOC_DAPM_ENUM("Route", twl4030_micpathtx2_enum);
+/* Analog bypass for AudioR1 */ +static const struct snd_kcontrol_new twl4030_dapm_abypassr1_control = + SOC_DAPM_SINGLE("Switch", TWL4030_REG_ARXR1_APGA_CTL, 2, 1, 0); + +/* Analog bypass for AudioL1 */ +static const struct snd_kcontrol_new twl4030_dapm_abypassl1_control = + SOC_DAPM_SINGLE("Switch", TWL4030_REG_ARXL1_APGA_CTL, 2, 1, 0); + +/* Analog bypass for AudioR2 */ +static const struct snd_kcontrol_new twl4030_dapm_abypassr2_control = + SOC_DAPM_SINGLE("Switch", TWL4030_REG_ARXR2_APGA_CTL, 2, 1, 0); + +/* Analog bypass for AudioL2 */ +static const struct snd_kcontrol_new twl4030_dapm_abypassl2_control = + SOC_DAPM_SINGLE("Switch", TWL4030_REG_ARXL2_APGA_CTL, 2, 1, 0); + static int micpath_event(struct snd_soc_dapm_widget *w, struct snd_kcontrol *kcontrol, int event) { @@ -497,6 +537,29 @@ static int headsetl_event(struct snd_soc_dapm_widget *w, return 0; }
+static int bypass_event(struct snd_soc_dapm_widget *w, + struct snd_kcontrol *kcontrol, int event) +{ + struct soc_mixer_control *m = + (struct soc_mixer_control *)w->kcontrols->private_value; + struct twl4030_priv *twl4030 = + (struct twl4030_priv *)w->codec->private_data; + unsigned char reg; + + reg = twl4030_read_reg_cache(w->codec, m->reg); + if (reg & (1 << m->shift)) + twl4030->bypass_state |= + (1 << (m->reg - TWL4030_REG_ARXL1_APGA_CTL)); + else + twl4030->bypass_state &= + ~(1 << (m->reg - TWL4030_REG_ARXL1_APGA_CTL)); + + if (w->codec->bias_level == SND_SOC_BIAS_STANDBY) + twl4030_power_down(w->codec); + + return 0; +} + /* * Some of the gain controls in TWL (mostly those which are associated with * the outputs) are implemented in an interesting way: @@ -775,13 +838,13 @@ static const struct snd_soc_dapm_widget twl4030_dapm_widgets[] = {
/* DACs */ SND_SOC_DAPM_DAC("DAC Right1", "Right Front Playback", - TWL4030_REG_AVDAC_CTL, 0, 0), + SND_SOC_NOPM, 0, 0), SND_SOC_DAPM_DAC("DAC Left1", "Left Front Playback", - TWL4030_REG_AVDAC_CTL, 1, 0), + SND_SOC_NOPM, 0, 0), SND_SOC_DAPM_DAC("DAC Right2", "Right Rear Playback", - TWL4030_REG_AVDAC_CTL, 2, 0), + SND_SOC_NOPM, 0, 0), SND_SOC_DAPM_DAC("DAC Left2", "Left Rear Playback", - TWL4030_REG_AVDAC_CTL, 3, 0), + SND_SOC_NOPM, 0, 0),
/* Analog PGAs */ SND_SOC_DAPM_PGA("ARXR1_APGA", TWL4030_REG_ARXR1_APGA_CTL, @@ -793,6 +856,29 @@ static const struct snd_soc_dapm_widget twl4030_dapm_widgets[] = { SND_SOC_DAPM_PGA("ARXL2_APGA", TWL4030_REG_ARXL2_APGA_CTL, 0, 0, NULL, 0),
+ /* Analog bypasses */ + SND_SOC_DAPM_SWITCH_E("Right1 Analog Loopback", SND_SOC_NOPM, 0, 0, + &twl4030_dapm_abypassr1_control, bypass_event, + SND_SOC_DAPM_POST_REG), + SND_SOC_DAPM_SWITCH_E("Left1 Analog Loopback", SND_SOC_NOPM, 0, 0, + &twl4030_dapm_abypassl1_control, + bypass_event, SND_SOC_DAPM_POST_REG), + SND_SOC_DAPM_SWITCH_E("Right2 Analog Loopback", SND_SOC_NOPM, 0, 0, + &twl4030_dapm_abypassr2_control, + bypass_event, SND_SOC_DAPM_POST_REG), + SND_SOC_DAPM_SWITCH_E("Left2 Analog Loopback", SND_SOC_NOPM, 0, 0, + &twl4030_dapm_abypassl2_control, + bypass_event, SND_SOC_DAPM_POST_REG), + + SND_SOC_DAPM_MIXER("Analog R1 Playback Mixer", TWL4030_REG_AVDAC_CTL, + 0, 0, NULL, 0), + SND_SOC_DAPM_MIXER("Analog L1 Playback Mixer", TWL4030_REG_AVDAC_CTL, + 1, 0, NULL, 0), + SND_SOC_DAPM_MIXER("Analog R2 Playback Mixer", TWL4030_REG_AVDAC_CTL, + 2, 0, NULL, 0), + SND_SOC_DAPM_MIXER("Analog L2 Playback Mixer", TWL4030_REG_AVDAC_CTL, + 3, 0, NULL, 0), + /* Output MUX controls */ /* Earpiece */ SND_SOC_DAPM_VALUE_MUX("Earpiece Mux", SND_SOC_NOPM, 0, 0, @@ -866,10 +952,15 @@ static const struct snd_soc_dapm_widget twl4030_dapm_widgets[] = { };
static const struct snd_soc_dapm_route intercon[] = { - {"ARXL1_APGA", NULL, "DAC Left1"}, - {"ARXR1_APGA", NULL, "DAC Right1"}, - {"ARXL2_APGA", NULL, "DAC Left2"}, - {"ARXR2_APGA", NULL, "DAC Right2"}, + {"Analog L1 Playback Mixer", NULL, "DAC Left1"}, + {"Analog R1 Playback Mixer", NULL, "DAC Right1"}, + {"Analog L2 Playback Mixer", NULL, "DAC Left2"}, + {"Analog R2 Playback Mixer", NULL, "DAC Right2"}, + + {"ARXL1_APGA", NULL, "Analog L1 Playback Mixer"}, + {"ARXR1_APGA", NULL, "Analog R1 Playback Mixer"}, + {"ARXL2_APGA", NULL, "Analog L2 Playback Mixer"}, + {"ARXR2_APGA", NULL, "Analog R2 Playback Mixer"},
/* Internal playback routings */ /* Earpiece */ @@ -951,6 +1042,17 @@ static const struct snd_soc_dapm_route intercon[] = { {"ADC Virtual Left2", NULL, "TX2 Capture Route"}, {"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"}, + + {"Analog R1 Playback Mixer", NULL, "Right1 Analog Loopback"}, + {"Analog L1 Playback Mixer", NULL, "Left1 Analog Loopback"}, + {"Analog R2 Playback Mixer", NULL, "Right2 Analog Loopback"}, + {"Analog L2 Playback Mixer", NULL, "Left2 Analog Loopback"}, + };
static int twl4030_add_widgets(struct snd_soc_codec *codec) @@ -1038,6 +1140,7 @@ static int twl4030_hw_params(struct snd_pcm_substream *substream,
if (mode != old_mode) { /* change rate and set CODECPDZ */ + twl4030_codec_enable(codec, 0); twl4030_write(codec, TWL4030_REG_CODEC_MODE, mode); twl4030_codec_enable(codec, 1); } @@ -1258,11 +1361,19 @@ static int twl4030_probe(struct platform_device *pdev) { struct snd_soc_device *socdev = platform_get_drvdata(pdev); struct snd_soc_codec *codec; + struct twl4030_priv *twl4030;
codec = kzalloc(sizeof(struct snd_soc_codec), GFP_KERNEL); if (codec == NULL) return -ENOMEM;
+ twl4030 = kzalloc(sizeof(struct twl4030_priv), GFP_KERNEL); + if (twl4030 == NULL) { + kfree(codec); + return -ENOMEM; + } + + codec->private_data = twl4030; socdev->codec = codec; mutex_init(&codec->mutex); INIT_LIST_HEAD(&codec->dapm_widgets); @@ -1282,6 +1393,7 @@ static int twl4030_remove(struct platform_device *pdev) printk(KERN_INFO "TWL4030 Audio Codec remove\n"); snd_soc_free_pcms(socdev); snd_soc_dapm_free(socdev); + kfree(codec->private_data); kfree(codec);
return 0;
On Tue, Jan 27, 2009 at 11:29:44AM +0200, Peter Ujfalusi wrote:
It seams that the analog loopback needs the DAC powered on on the channel, where the loopback is selected. The switch for the DACs has been moved from
Oh, well...
After the loopback registers are changed, the event callback will update the bypass bitfield, so we will know if any of the bypass paths are enabled. If the codec is in STANDBY mode, the decision is made to keep/switch the codec off or keep/switch the codec on to allow the bypass. Also after the codec switches from ON to STANDBY the same check will be done.
The original implementation was done that way due to the lack of DAPM - it did *all* power management in the bias configuration. Now that you have DAPM support it should be possible for the driver to function more normally and avoid these checks. What most drivers do is bring all the chip wide power up when coming into standby then power on the rest under DAPM.
Support for turning the codec completely off should really be provided by the core as an optional thing that can be done when the subsystem has been idle for a while using the regular bias level stuff like the existing delay before falling back to standby. It needs to be optional since won't be appropriate for all applications since bringing the bias levels up cleanly may take too long for some systems.
A few more specific issues in the rest of the code below. I've also got a patch I'm going to push to Takashi later today which conflicts with this one, see the for-2.6.30 branch of:
git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git
static void twl4030_codec_enable(struct snd_soc_codec *codec, int enable) {
struct twl4030_priv *twl4030 =(struct twl4030_priv *)codec->private_data;
Casts away from void aren't needed, there's quite a few of those here and in the rest of the patch code. Please fix these - they're usually a sign that something is wrong if they're needed.
codec = kzalloc(sizeof(struct snd_soc_codec), GFP_KERNEL); if (codec == NULL) return -ENOMEM;
twl4030 = kzalloc(sizeof(struct twl4030_priv), GFP_KERNEL);if (twl4030 == NULL) {
Not essential but you could collapse these into a single allocation.
On Tuesday 27 January 2009 13:37:38 ext Mark Brown wrote:
On Tue, Jan 27, 2009 at 11:29:44AM +0200, Peter Ujfalusi wrote:
It seams that the analog loopback needs the DAC powered on on the channel, where the loopback is selected. The switch for the DACs has been moved from
Oh, well...
Yes, it took a while to figure out what is wrong...
After the loopback registers are changed, the event callback will update the bypass bitfield, so we will know if any of the bypass paths are enabled. If the codec is in STANDBY mode, the decision is made to keep/switch the codec off or keep/switch the codec on to allow the bypass. Also after the codec switches from ON to STANDBY the same check will be done.
The original implementation was done that way due to the lack of DAPM - it did *all* power management in the bias configuration. Now that you have DAPM support it should be possible for the driver to function more normally and avoid these checks. What most drivers do is bring all the chip wide power up when coming into standby then power on the rest under DAPM.
Well, I had the implementation like that. There were few 'issues' which resulted the current code - and that code is long gone :(. So the codec was powered at all time, except, if it would recieve the SND_SOC_BIAS_OFF event. To put things into context, let's say that the power consumption of the board when the codec turned off is 0 A. When the bypass was enabled (to external speaker) it jumped to 0.016 A, which is fine, since an external amp is powered also. Than if the bypass got disabled it jumped back to 0.01 A. After a bit of debugging I found the reason: We have outputs with dedicated gain controls (PreDriveL/R, HeadsetL/R, CarkitL/R, Earpiece), they have to be put to power-down mode (gain bits to 0). We do have volume controls for these, they are not in DAPM, but they can be modified. But now the problem is that something has to track the bypass states in userspace: if the bypass disabled, than has to mute all outputs, when enabled, it has to remember, what was the original gain for the outputs. So it's going to be a mess. Still after powering these down I have 0.006 A... Than I found another thing: PLL power. Swtiching it off APPL_CTL:APPL_EN bit resulted with 0.002 A. Now it is close enough. But than I played a bit with the registers, enabled the bypass (while the PLL was off) and the readings were: 0.020 A, in short: bypass on + PLL on = 0.016 A bypass on + PLL off = 0.020 A bypass off + PLL on = 0.006 A bypass off + PLL off = 0.002 A
Grrr, and I gave up. Oh, and we don't have control for the PLL (it is enabled all the time at the moment). I was thinking to use the SND_SOC_DAPM_PRE and SND_SOC_DAPM_POST for it but I don't know how to use those.
So I thought that I will write it in a way I have sent it and revisit later, when I (or someone else) have better idea to handle this.
Sorry for the long and most probably boring details...
Support for turning the codec completely off should really be provided by the core as an optional thing that can be done when the subsystem has been idle for a while using the regular bias level stuff like the existing delay before falling back to standby. It needs to be optional since won't be appropriate for all applications since bringing the bias levels up cleanly may take too long for some systems.
A few more specific issues in the rest of the code below. I've also got a patch I'm going to push to Takashi later today which conflicts with this one, see the for-2.6.30 branch of:
git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git
I can wait till it gets there and send the series on top of it.
static void twl4030_codec_enable(struct snd_soc_codec *codec, int enable) {
struct twl4030_priv *twl4030 =(struct twl4030_priv *)codec->private_data;Casts away from void aren't needed, there's quite a few of those here and in the rest of the patch code. Please fix these - they're usually a sign that something is wrong if they're needed.
codec = kzalloc(sizeof(struct snd_soc_codec), GFP_KERNEL); if (codec == NULL) return -ENOMEM;
twl4030 = kzalloc(sizeof(struct twl4030_priv), GFP_KERNEL);if (twl4030 == NULL) {Not essential but you could collapse these into a single allocation.
I'll fix these.
Thanks, Péter
On Tue, Jan 27, 2009 at 03:05:34PM +0200, Peter Ujfalusi wrote:
Than if the bypass got disabled it jumped back to 0.01 A. After a bit of debugging I found the reason: We have outputs with dedicated gain controls (PreDriveL/R, HeadsetL/R, CarkitL/R, Earpiece), they have to be put to power-down mode (gain bits to 0). We do have volume controls for these, they are not in DAPM, but they can be modified. But now the problem is that something has to track the bypass states in userspace: if the bypass disabled, than has to mute all outputs, when enabled, it has to remember, what was the original gain for the outputs. So it's going to be a mess.
This one should be dealable with in-kernel without any work by applications - WM8350 does a similar dance (for different reasons), providing a shadow volume control while some of the amplifiers are powered off.
But than I played a bit with the registers, enabled the bypass (while the PLL was off) and the readings were: 0.020 A, in short: bypass on + PLL on = 0.016 A bypass on + PLL off = 0.020 A
I suspect the increased power consumption here is due to the PLL being required to clock the DAC properly. Or you've transposed the figures :)
bypass off + PLL on = 0.006 A bypass off + PLL off = 0.002 A
Grrr, and I gave up. Oh, and we don't have control for the PLL (it is enabled
Normally all the PLL control is punted to the machine driver since the clocking configuration tends to be system dependant, especially when the codec is the clock master. Then again, the TI codecs seem to be fairly inflexible in this regard so perhaps it doesn't make sense there. If there's only one configuration possible then there's no sense bothering the machine drivers with it.
all the time at the moment). I was thinking to use the SND_SOC_DAPM_PRE and SND_SOC_DAPM_POST for it but I don't know how to use those.
Just declare them as widgets - the event function you supply will be called back when the state changes. The event function interface is the same as for the _E versions of normal DAPM controls, see WM8900 for one example of how to use them.
So I thought that I will write it in a way I have sent it and revisit later, when I (or someone else) have better idea to handle this.
I suspect that if the core were able to automatically push the codec down into BIAS_OFF as well as BIAS_STANDBY that'd probably be enough to deal with the issue? Probably in concert with fixing the core to bring the codec up to BIAS_ON when bypass paths are active. It looks like what you're implementing here is a combination of the fixes for those two issues.
My main concern is that it feels like this is breaking the model that the core has of what the codec driver should be doing - I think some (much?) of this is a case of the core needing more functionality and that if that is the case then we'd be better off not carrying driver specific workarounds in case they get broken when things are fixed in the core.
Sorry for the long and most probably boring details...
git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git
I can wait till it gets there and send the series on top of it.
Like I said, I've already applied all your other patches.
On Tuesday 27 January 2009 16:03:16 ext Mark Brown wrote:
This one should be dealable with in-kernel without any work by applications - WM8350 does a similar dance (for different reasons), providing a shadow volume control while some of the amplifiers are powered off.
I'm going to check the code. But I think the driver still needs to keep on track of the bypass switch states in order to do the right thing.
But than I played a bit with the registers, enabled the bypass (while the PLL was off) and the readings were: 0.020 A, in short: bypass on + PLL on = 0.016 A bypass on + PLL off = 0.020 A
I suspect the increased power consumption here is due to the PLL being required to clock the DAC properly. Or you've transposed the figures :)
I have checked it several times, the numbers are correct. Probably the DAC clocking, but how it is working without the PLL on - because the bypass was working...
bypass off + PLL on = 0.006 A bypass off + PLL off = 0.002 A
Grrr, and I gave up. Oh, and we don't have control for the PLL (it is enabled
all the time at the moment). I was thinking to use the SND_SOC_DAPM_PRE and SND_SOC_DAPM_POST for it but I don't know how to use those.
Just declare them as widgets - the event function you supply will be called back when the state changes. The event function interface is the same as for the _E versions of normal DAPM controls, see WM8900 for one example of how to use them.
I'll take a look.
So I thought that I will write it in a way I have sent it and revisit later, when I (or someone else) have better idea to handle this.
I suspect that if the core were able to automatically push the codec down into BIAS_OFF as well as BIAS_STANDBY that'd probably be enough to deal with the issue? Probably in concert with fixing the core to bring the codec up to BIAS_ON when bypass paths are active. It looks like what you're implementing here is a combination of the fixes for those two issues.
My main concern is that it feels like this is breaking the model that the core has of what the codec driver should be doing - I think some (much?) of this is a case of the core needing more functionality and that if that is the case then we'd be better off not carrying driver specific workarounds in case they get broken when things are fixed in the core.
Right, I agree. I'll send this last patch back to the drawing board ;)
Sorry for the long and most probably boring details...
git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git
I can wait till it gets there and send the series on top of it.
Like I said, I've already applied all your other patches.
I missunderstand that, sorry, I'll focus on the analog bypass redesign than...
Thanks, Péter
On Tue, Jan 27, 2009 at 04:28:35PM +0200, Peter Ujfalusi wrote:
On Tuesday 27 January 2009 16:03:16 ext Mark Brown wrote:
This one should be dealable with in-kernel without any work by applications - WM8350 does a similar dance (for different reasons), providing a shadow volume control while some of the amplifiers are powered off.
I'm going to check the code. But I think the driver still needs to keep on track of the bypass switch states in order to do the right thing.
I'd expet so - the main win would be to avoid having to have user space manually twiddle additional controls.
I suspect the increased power consumption here is due to the PLL being required to clock the DAC properly. Or you've transposed the figures :)
I have checked it several times, the numbers are correct. Probably the DAC clocking, but how it is working without the PLL on - because the bypass was working...
I'd imagine that it did actually have a clock (probably whatever clock would be input to the PLL), just not at the optimal rate.
On Tue, Jan 27, 2009 at 11:29:38AM +0200, Peter Ujfalusi wrote:
[1] ANAMICL register self cleaning bit handling [2] Simplified power up/down function [3] Move the Headset anti-pop/bias ramp enabling to be happend only if the headset is actually in use [4] Change the place for the DAC power switch [5] Preparation for the loopback implementation [6] Analog loopback/bypass implementation
Applied 1-5 - some comments on 6 in followup to that.
participants (2)
-
Mark Brown -
Peter Ujfalusi