[alsa-devel] [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix
Hello,
The following series aims to fix the Headset power on/off pop noise problems.
Since the twl4030 codec internal implementation have this order in HW: DAC -> Analog PGA -> Mixer
While the ASoC framework expects the following order: DAC -> Mixer -> Analog PGA
I needed to move the APGA control from SND_SOC_DAPM_PGA to be handled with _MIXER. Than adding a PGA to the headset outputs results correct power sequences.
This patch also separates the analog and digital loopback paths, so when the analog loopback is enabled none of the DACs will be powered on, while in case of digital loopback the corresponding DACs will be powered.
These are internal changes, not visible for the user. I have tested it with my custom board with the headset soldered to HeadsetL, the pop noise is really small if any.
The user must also set the correct ramp delay for the given board. On my board: MCLK = 38.4MHz RAMP_DELAY = 2 (109/81/55 ms) works best (meaning 55ms ramp delay), if it is shorter, than it clicks, if it is longer, than it does not click, but the beep tone can be observed.
I hope that I have not missed anything with the patches, since these are forward ported from my working tree to sound-2.6:topic/asoc.
I have applied the patch from Misael on top of the current topic/asoc, so the head was: ASoC: TWL4030: Enable/disable voice digital filters
I believe that this should be able to minimize the pop noise on the headset outputs.
--- Peter Ujfalusi (2): ASoC: TWL4030: Change DAPM routings and controls for DACs and PGAs ASoC: TWL4030: Move the Headset pop-attenuation code to PGA event
sound/soc/codecs/twl4030.c | 257 ++++++++++++++++++++++++++++---------------- 1 files changed, 162 insertions(+), 95 deletions(-)
Restructuring the twl4030 codec's DAPM routing to be able to handle the power sequences correctly.
The twl4030 codec internal implementation have this order: DAC -> Analog PGA -> Mixer/Mux
While the ASoC framework expects the following order: DAC -> Mixer -> Analog PGA
This patch moves the Analog PGA handling from SND_SOC_DAPM_PGA to _MIXER and adds two levels of mixer to handle the digital and analog loopback functionality.
Now the analog loopback does not powers on any of the DACs.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/twl4030.c | 141 ++++++++++++++++++++++---------------------- 1 files changed, 71 insertions(+), 70 deletions(-)
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index e4d683d..990aa4a 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -1004,18 +1004,6 @@ static const struct snd_soc_dapm_widget twl4030_dapm_widgets[] = { SND_SOC_DAPM_DAC("DAC Voice", "Voice Playback", SND_SOC_NOPM, 0, 0),
- /* Analog PGAs */ - SND_SOC_DAPM_PGA("ARXR1_APGA", TWL4030_REG_ARXR1_APGA_CTL, - 0, 0, NULL, 0), - SND_SOC_DAPM_PGA("ARXL1_APGA", TWL4030_REG_ARXL1_APGA_CTL, - 0, 0, NULL, 0), - SND_SOC_DAPM_PGA("ARXR2_APGA", TWL4030_REG_ARXR2_APGA_CTL, - 0, 0, NULL, 0), - SND_SOC_DAPM_PGA("ARXL2_APGA", TWL4030_REG_ARXL2_APGA_CTL, - 0, 0, NULL, 0), - SND_SOC_DAPM_PGA("VDL_APGA", TWL4030_REG_VDL_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, @@ -1044,16 +1032,29 @@ static const struct snd_soc_dapm_widget twl4030_dapm_widgets[] = { &twl4030_dapm_dbypassv_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), - SND_SOC_DAPM_MIXER("Analog Voice Playback Mixer", TWL4030_REG_AVDAC_CTL, - 4, 0, NULL, 0), + /* Digital mixers, power control for the physical DACs */ + SND_SOC_DAPM_MIXER("Digital R1 Playback Mixer", + TWL4030_REG_AVDAC_CTL, 0, 0, NULL, 0), + SND_SOC_DAPM_MIXER("Digital L1 Playback Mixer", + TWL4030_REG_AVDAC_CTL, 1, 0, NULL, 0), + SND_SOC_DAPM_MIXER("Digital R2 Playback Mixer", + TWL4030_REG_AVDAC_CTL, 2, 0, NULL, 0), + SND_SOC_DAPM_MIXER("Digital L2 Playback Mixer", + TWL4030_REG_AVDAC_CTL, 3, 0, NULL, 0), + SND_SOC_DAPM_MIXER("Digital Voice Playback Mixer", + TWL4030_REG_AVDAC_CTL, 4, 0, NULL, 0), + + /* Analog mixers, power control for the physical PGAs */ + SND_SOC_DAPM_MIXER("Analog R1 Playback Mixer", + TWL4030_REG_ARXR1_APGA_CTL, 0, 0, NULL, 0), + SND_SOC_DAPM_MIXER("Analog L1 Playback Mixer", + TWL4030_REG_ARXL1_APGA_CTL, 0, 0, NULL, 0), + SND_SOC_DAPM_MIXER("Analog R2 Playback Mixer", + TWL4030_REG_ARXR2_APGA_CTL, 0, 0, NULL, 0), + SND_SOC_DAPM_MIXER("Analog L2 Playback Mixer", + TWL4030_REG_ARXL2_APGA_CTL, 0, 0, NULL, 0), + SND_SOC_DAPM_MIXER("Analog Voice Playback Mixer", + TWL4030_REG_VDL_APGA_CTL, 0, 0, NULL, 0),
/* Output MIXER controls */ /* Earpiece */ @@ -1147,60 +1148,60 @@ static const struct snd_soc_dapm_widget twl4030_dapm_widgets[] = { };
static const struct snd_soc_dapm_route intercon[] = { - {"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"}, - {"Analog Voice Playback Mixer", NULL, "DAC Voice"}, - - {"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"}, - {"VDL_APGA", NULL, "Analog Voice Playback Mixer"}, + {"Digital L1 Playback Mixer", NULL, "DAC Left1"}, + {"Digital R1 Playback Mixer", NULL, "DAC Right1"}, + {"Digital L2 Playback Mixer", NULL, "DAC Left2"}, + {"Digital R2 Playback Mixer", NULL, "DAC Right2"}, + {"Digital Voice Playback Mixer", NULL, "DAC Voice"}, + + {"Analog L1 Playback Mixer", NULL, "Digital L1 Playback Mixer"}, + {"Analog R1 Playback Mixer", NULL, "Digital R1 Playback Mixer"}, + {"Analog L2 Playback Mixer", NULL, "Digital L2 Playback Mixer"}, + {"Analog R2 Playback Mixer", NULL, "Digital R2 Playback Mixer"}, + {"Analog Voice Playback Mixer", NULL, "Digital Voice Playback Mixer"},
/* Internal playback routings */ /* Earpiece */ - {"Earpiece Mixer", "Voice", "VDL_APGA"}, - {"Earpiece Mixer", "AudioL1", "ARXL1_APGA"}, - {"Earpiece Mixer", "AudioL2", "ARXL2_APGA"}, - {"Earpiece Mixer", "AudioR1", "ARXR1_APGA"}, + {"Earpiece Mixer", "Voice", "Analog Voice Playback Mixer"}, + {"Earpiece Mixer", "AudioL1", "Analog L1 Playback Mixer"}, + {"Earpiece Mixer", "AudioL2", "Analog L2 Playback Mixer"}, + {"Earpiece Mixer", "AudioR1", "Analog R1 Playback Mixer"}, /* PreDrivL */ - {"PredriveL Mixer", "Voice", "VDL_APGA"}, - {"PredriveL Mixer", "AudioL1", "ARXL1_APGA"}, - {"PredriveL Mixer", "AudioL2", "ARXL2_APGA"}, - {"PredriveL Mixer", "AudioR2", "ARXR2_APGA"}, + {"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"}, /* PreDrivR */ - {"PredriveR Mixer", "Voice", "VDL_APGA"}, - {"PredriveR Mixer", "AudioR1", "ARXR1_APGA"}, - {"PredriveR Mixer", "AudioR2", "ARXR2_APGA"}, - {"PredriveR Mixer", "AudioL2", "ARXL2_APGA"}, + {"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"}, /* HeadsetL */ - {"HeadsetL Mixer", "Voice", "VDL_APGA"}, - {"HeadsetL Mixer", "AudioL1", "ARXL1_APGA"}, - {"HeadsetL Mixer", "AudioL2", "ARXL2_APGA"}, + {"HeadsetL Mixer", "Voice", "Analog Voice Playback Mixer"}, + {"HeadsetL Mixer", "AudioL1", "Analog L1 Playback Mixer"}, + {"HeadsetL Mixer", "AudioL2", "Analog L2 Playback Mixer"}, /* HeadsetR */ - {"HeadsetR Mixer", "Voice", "VDL_APGA"}, - {"HeadsetR Mixer", "AudioR1", "ARXR1_APGA"}, - {"HeadsetR Mixer", "AudioR2", "ARXR2_APGA"}, + {"HeadsetR Mixer", "Voice", "Analog Voice Playback Mixer"}, + {"HeadsetR Mixer", "AudioR1", "Analog R1 Playback Mixer"}, + {"HeadsetR Mixer", "AudioR2", "Analog R2 Playback Mixer"}, /* CarkitL */ - {"CarkitL Mixer", "Voice", "VDL_APGA"}, - {"CarkitL Mixer", "AudioL1", "ARXL1_APGA"}, - {"CarkitL Mixer", "AudioL2", "ARXL2_APGA"}, + {"CarkitL Mixer", "Voice", "Analog Voice Playback Mixer"}, + {"CarkitL Mixer", "AudioL1", "Analog L1 Playback Mixer"}, + {"CarkitL Mixer", "AudioL2", "Analog L2 Playback Mixer"}, /* CarkitR */ - {"CarkitR Mixer", "Voice", "VDL_APGA"}, - {"CarkitR Mixer", "AudioR1", "ARXR1_APGA"}, - {"CarkitR Mixer", "AudioR2", "ARXR2_APGA"}, + {"CarkitR Mixer", "Voice", "Analog Voice Playback Mixer"}, + {"CarkitR Mixer", "AudioR1", "Analog R1 Playback Mixer"}, + {"CarkitR Mixer", "AudioR2", "Analog R2 Playback Mixer"}, /* HandsfreeL */ - {"HandsfreeL Mux", "Voice", "VDL_APGA"}, - {"HandsfreeL Mux", "AudioL1", "ARXL1_APGA"}, - {"HandsfreeL Mux", "AudioL2", "ARXL2_APGA"}, - {"HandsfreeL Mux", "AudioR2", "ARXR2_APGA"}, + {"HandsfreeL Mux", "Voice", "Analog Voice Playback Mixer"}, + {"HandsfreeL Mux", "AudioL1", "Analog L1 Playback Mixer"}, + {"HandsfreeL Mux", "AudioL2", "Analog L2 Playback Mixer"}, + {"HandsfreeL Mux", "AudioR2", "Analog R2 Playback Mixer"}, /* HandsfreeR */ - {"HandsfreeR Mux", "Voice", "VDL_APGA"}, - {"HandsfreeR Mux", "AudioR1", "ARXR1_APGA"}, - {"HandsfreeR Mux", "AudioR2", "ARXR2_APGA"}, - {"HandsfreeR Mux", "AudioL2", "ARXL2_APGA"}, + {"HandsfreeR Mux", "Voice", "Analog Voice Playback Mixer"}, + {"HandsfreeR Mux", "AudioR1", "Analog R1 Playback Mixer"}, + {"HandsfreeR Mux", "AudioR2", "Analog R2 Playback Mixer"}, + {"HandsfreeR Mux", "AudioL2", "Analog L2 Playback Mixer"}, /* Vibra */ {"Vibra Mux", "AudioL1", "DAC Left1"}, {"Vibra Mux", "AudioR1", "DAC Right1"}, @@ -1208,8 +1209,8 @@ static const struct snd_soc_dapm_route intercon[] = { {"Vibra Mux", "AudioR2", "DAC Right2"},
/* outputs */ - {"OUTL", NULL, "ARXL2_APGA"}, - {"OUTR", NULL, "ARXR2_APGA"}, + {"OUTL", NULL, "Analog L2 Playback Mixer"}, + {"OUTR", NULL, "Analog R2 Playback Mixer"}, {"EARPIECE", NULL, "Earpiece Mixer"}, {"PREDRIVEL", NULL, "PredriveL Mixer"}, {"PREDRIVER", NULL, "PredriveR Mixer"}, @@ -1273,9 +1274,9 @@ static const struct snd_soc_dapm_route intercon[] = { {"Left Digital Loopback", "Volume", "TX1 Capture Route"}, {"Voice Digital Loopback", "Volume", "TX2 Capture Route"},
- {"Analog R2 Playback Mixer", NULL, "Right Digital Loopback"}, - {"Analog L2 Playback Mixer", NULL, "Left Digital Loopback"}, - {"Analog Voice Playback Mixer", NULL, "Voice Digital Loopback"}, + {"Digital R2 Playback Mixer", NULL, "Right Digital Loopback"}, + {"Digital L2 Playback Mixer", NULL, "Left Digital Loopback"}, + {"Digital Voice Playback Mixer", NULL, "Voice Digital Loopback"},
};
This patch adds SND_SOC_DAPM_PGA_E to the headset path, which handles the headset ramp up and down sequences needed for the pop noise removal.
With this patch the order of the internal components in the twl4030 codec is turned on and off in a correct order.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/twl4030.c | 116 ++++++++++++++++++++++++++++++++++--------- 1 files changed, 91 insertions(+), 25 deletions(-)
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index 990aa4a..afe7879 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -130,6 +130,12 @@ struct twl4030_priv { unsigned int rate; unsigned int sample_bits; unsigned int channels; + + unsigned int sysclk; + + /* Headset output state handling */ + unsigned int hsl_enabled; + unsigned int hsr_enabled; };
/* @@ -564,39 +570,85 @@ 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) +static void headset_ramp(struct snd_soc_codec *codec, int ramp) { unsigned char hs_gain, hs_pop; + struct twl4030_priv *twl4030 = codec->private_data; + /* Base values for ramp delay calculation: 2^19 - 2^26 */ + unsigned int ramp_base[] = {524288, 1048576, 2097152, 4194304, + 8388608, 16777216, 33554432, 67108864};
- /* Save the current volume */ - hs_gain = twl4030_read_reg_cache(w->codec, TWL4030_REG_HS_GAIN_SET); - hs_pop = twl4030_read_reg_cache(w->codec, TWL4030_REG_HS_POPN_SET); + hs_gain = twl4030_read_reg_cache(codec, TWL4030_REG_HS_GAIN_SET); + hs_pop = twl4030_read_reg_cache(codec, TWL4030_REG_HS_POPN_SET);
- switch (event) { - case SND_SOC_DAPM_POST_PMU: - /* Do the anti-pop/bias ramp enable according to the TRM */ + if (ramp) { + /* Headset ramp-up according to the TRM */ 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); + twl4030_write(codec, TWL4030_REG_HS_POPN_SET, hs_pop); + twl4030_write(codec, TWL4030_REG_HS_GAIN_SET, hs_gain); 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 */ + twl4030_write(codec, TWL4030_REG_HS_POPN_SET, hs_pop); + } else { + /* Headset ramp-down _not_ according to + * the TRM, but in a way that it is working */ hs_pop &= ~TWL4030_RAMP_EN; - twl4030_write(w->codec, TWL4030_REG_HS_POPN_SET, hs_pop); + twl4030_write(codec, TWL4030_REG_HS_POPN_SET, hs_pop); + /* Wait ramp delay time + 1, so the VMID can settle */ + mdelay((ramp_base[(hs_pop & TWL4030_RAMP_DELAY) >> 2] / + twl4030->sysclk) + 1); /* 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); + twl4030_write(codec, TWL4030_REG_HS_POPN_SET, hs_pop); + } +} + +static int headsetlpga_event(struct snd_soc_dapm_widget *w, + struct snd_kcontrol *kcontrol, int event) +{ + struct twl4030_priv *twl4030 = w->codec->private_data; + + switch (event) { + case SND_SOC_DAPM_POST_PMU: + /* Do the ramp-up only once */ + if (!twl4030->hsr_enabled) + headset_ramp(w->codec, 1); + + twl4030->hsl_enabled = 1; + break; + case SND_SOC_DAPM_POST_PMD: + /* Do the ramp-down only if both headsetL/R is disabled */ + if (!twl4030->hsr_enabled) + headset_ramp(w->codec, 0); + + twl4030->hsl_enabled = 0; + break; + } + return 0; +} + +static int headsetrpga_event(struct snd_soc_dapm_widget *w, + struct snd_kcontrol *kcontrol, int event) +{ + struct twl4030_priv *twl4030 = w->codec->private_data; + + switch (event) { + case SND_SOC_DAPM_POST_PMU: + /* Do the ramp-up only once */ + if (!twl4030->hsl_enabled) + headset_ramp(w->codec, 1); + + twl4030->hsr_enabled = 1; + break; + case SND_SOC_DAPM_POST_PMD: + /* Do the ramp-down only if both headsetL/R is disabled */ + if (!twl4030->hsl_enabled) + headset_ramp(w->codec, 0); + + twl4030->hsr_enabled = 0; break; } return 0; @@ -1069,13 +1121,18 @@ static const struct snd_soc_dapm_widget twl4030_dapm_widgets[] = { &twl4030_dapm_predriver_controls[0], ARRAY_SIZE(twl4030_dapm_predriver_controls)), /* HeadsetL/R */ - SND_SOC_DAPM_MIXER_E("HeadsetL Mixer", SND_SOC_NOPM, 0, 0, + SND_SOC_DAPM_MIXER("HeadsetL Mixer", SND_SOC_NOPM, 0, 0, &twl4030_dapm_hsol_controls[0], - ARRAY_SIZE(twl4030_dapm_hsol_controls), headsetl_event, + ARRAY_SIZE(twl4030_dapm_hsol_controls)), + SND_SOC_DAPM_PGA_E("HeadsetL PGA", SND_SOC_NOPM, + 0, 0, NULL, 0, headsetlpga_event, SND_SOC_DAPM_POST_PMU|SND_SOC_DAPM_POST_PMD), SND_SOC_DAPM_MIXER("HeadsetR Mixer", SND_SOC_NOPM, 0, 0, &twl4030_dapm_hsor_controls[0], ARRAY_SIZE(twl4030_dapm_hsor_controls)), + SND_SOC_DAPM_PGA_E("HeadsetR PGA", SND_SOC_NOPM, + 0, 0, NULL, 0, headsetrpga_event, + SND_SOC_DAPM_POST_PMU|SND_SOC_DAPM_POST_PMD), /* CarkitL/R */ SND_SOC_DAPM_MIXER("CarkitL Mixer", SND_SOC_NOPM, 0, 0, &twl4030_dapm_carkitl_controls[0], @@ -1180,10 +1237,12 @@ static const struct snd_soc_dapm_route intercon[] = { {"HeadsetL Mixer", "Voice", "Analog Voice Playback Mixer"}, {"HeadsetL Mixer", "AudioL1", "Analog L1 Playback Mixer"}, {"HeadsetL Mixer", "AudioL2", "Analog L2 Playback Mixer"}, + {"HeadsetL PGA", NULL, "HeadsetL Mixer"}, /* HeadsetR */ {"HeadsetR Mixer", "Voice", "Analog Voice Playback Mixer"}, {"HeadsetR Mixer", "AudioR1", "Analog R1 Playback Mixer"}, {"HeadsetR Mixer", "AudioR2", "Analog R2 Playback Mixer"}, + {"HeadsetR PGA", NULL, "HeadsetR Mixer"}, /* CarkitL */ {"CarkitL Mixer", "Voice", "Analog Voice Playback Mixer"}, {"CarkitL Mixer", "AudioL1", "Analog L1 Playback Mixer"}, @@ -1214,8 +1273,8 @@ static const struct snd_soc_dapm_route intercon[] = { {"EARPIECE", NULL, "Earpiece Mixer"}, {"PREDRIVEL", NULL, "PredriveL Mixer"}, {"PREDRIVER", NULL, "PredriveR Mixer"}, - {"HSOL", NULL, "HeadsetL Mixer"}, - {"HSOR", NULL, "HeadsetR Mixer"}, + {"HSOL", NULL, "HeadsetL PGA"}, + {"HSOR", NULL, "HeadsetR PGA"}, {"CARKITL", NULL, "CarkitL Mixer"}, {"CARKITR", NULL, "CarkitR Mixer"}, {"HFL", NULL, "HandsfreeL Mux"}, @@ -1554,17 +1613,21 @@ static int twl4030_set_dai_sysclk(struct snd_soc_dai *codec_dai, int clk_id, unsigned int freq, int dir) { struct snd_soc_codec *codec = codec_dai->codec; + struct twl4030_priv *twl4030 = codec->private_data; u8 infreq;
switch (freq) { case 19200000: infreq = TWL4030_APLL_INFREQ_19200KHZ; + twl4030->sysclk = 19200; break; case 26000000: infreq = TWL4030_APLL_INFREQ_26000KHZ; + twl4030->sysclk = 26000; break; case 38400000: infreq = TWL4030_APLL_INFREQ_38400KHZ; + twl4030->sysclk = 38400; break; default: printk(KERN_ERR "TWL4030 set sysclk: unknown rate %d\n", @@ -1953,6 +2016,9 @@ static int twl4030_probe(struct platform_device *pdev) kfree(codec); return -ENOMEM; } + /* Set default sysclk (used by the headsetl/rpga_event callback for + * pop-attenuation) */ + twl4030->sysclk = 26000;
codec->private_data = twl4030; socdev->card->codec = codec;
On Mon, May 18, 2009 at 04:02:03PM +0300, Peter Ujfalusi wrote:
The following series aims to fix the Headset power on/off pop noise problems.
Looks OK but given all the to and fro here I'd rather wait for some testing to make sure it works well on other system.s
I needed to move the APGA control from SND_SOC_DAPM_PGA to be handled with _MIXER. Than adding a PGA to the headset outputs results correct power sequences.
TBH it sounds like this is actually a mixer anyway - typically mixers do have some gain controls associated with them, the sort of PGA DAPM means is more an output stage driver. Typically these have much more control of the gain and only one input and output.
-----Original Message----- From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com] Sent: Monday, May 18, 2009 6:39 PM To: Peter Ujfalusi Cc: alsa-devel@alsa-project.org; sakoman@gmail.com; Aggarwal, Anuj; getarunks@gmail.com Subject: Re: [alsa-devel] [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix
On Mon, May 18, 2009 at 04:02:03PM +0300, Peter Ujfalusi wrote:
The following series aims to fix the Headset power on/off pop noise
problems.
Looks OK but given all the to and fro here I'd rather wait for some testing to make sure it works well on other system.s
[Aggarwal, Anuj] I tried this on OMAP3 EVM. Ramp delay values 0-3 produces minor glitches in the beginning & end, value 4 gave me the best result so far on my EVM, I will try with some other EVMs as well before finalizing it. 5 and others produce a beep in the beginning and end. Just one question, is there any side-effect of choosing a large ramp delay? For e.g. in my case, it could be 437ms (19.2MHz clock).
I needed to move the APGA control from SND_SOC_DAPM_PGA to be
handled with
_MIXER. Than adding a PGA to the headset outputs results correct power sequences.
TBH it sounds like this is actually a mixer anyway - typically mixers do have some gain controls associated with them, the sort of PGA DAPM means is more an output stage driver. Typically these have much more control of the gain and only one input and output.
The following series aims to fix the Headset power on/off pop noise problems.
Looks OK but given all the to and fro here I'd rather wait for some testing to make sure it works well on other system.s
[Aggarwal, Anuj] I tried this on OMAP3 EVM. Ramp delay values 0-3 produces minor glitches in the beginning & end, value 4 gave me the best result so far on my EVM, I will try with some other EVMs as well before finalizing it. 5 and others produce a beep in the beginning and end. Just one question, is there any side-effect of choosing a large ramp delay? For e.g. in my case, it could be 437ms (19.2MHz clock).
I tested on SDP3430, the best ramp delay value for my board seems to be 3. Lower values give glitches and higher values present the beep sound. In general, it has been reduced.
-Misa
I am facing problems on the capture side now, I don't know whether someone has tested it or not.
By default, capture was not working. I have to enable Analog Left Capture Route AUXL & Analog Right Capture Route AUXR to enable the capture paths for OMAP3 EVM. Even after enabling both the paths, I am not able to capture audio on both the channels. I am hearing audio only on either of the channel, the other one is playing only noise for me. (This channel swapping is random, sometimes it plays on left and sometimes on right.)
If I disable ' Analog Right Capture Route AUXR', I am not hearing any noise; audio is playing fine, but again on either of the channels, not consistently on the left one.
Moreover, in both the cases, I am getting large overrun errors (I am using NFS to store the recorded clips, will try with MMC etc. too):
root:~# arecord -f cd -d 30 rec05.dat Recording WAVE 'rec05.dat' : Signed 16 bit Little Endian, Rate 44100 Hz, Stereo overrun!!! (at least 8424.774 ms long) overrun!!! (at least 6465.943 ms long) overrun!!! (at least 3377.716 ms long) overrun!!! (at least 7982.147 ms long) overrun!!! (at least 8649.720 ms long)
I didn't face both the problems on my original code base (2.6.29-rc3) and hence wanted to confirm whether someone else has also faced the same issues. I am debugging at my end to see what difference is causing the problem but any pointers are welcome.
Thanks and Regards, Anuj Aggarwal
-----Original Message----- From: Lopez Cruz, Misael Sent: Tuesday, May 19, 2009 6:18 AM To: Aggarwal, Anuj; Mark Brown; Peter Ujfalusi Cc: sakoman@gmail.com; alsa-devel@alsa-project.org; getarunks@gmail.com Subject: RE: [alsa-devel] [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix
The following series aims to fix the Headset power on/off pop noise problems.
Looks OK but given all the to and fro here I'd rather wait for some testing to make sure it works well on other system.s
[Aggarwal, Anuj] I tried this on OMAP3 EVM. Ramp delay values 0-3 produces minor glitches in the beginning & end, value 4 gave me the best result so far on my EVM, I will try with some other EVMs as well before finalizing it. 5 and others produce a beep in the beginning and end. Just one question, is there any side-effect of choosing a large ramp delay? For e.g. in my case, it could be 437ms (19.2MHz clock).
I tested on SDP3430, the best ramp delay value for my board seems to be 3. Lower values give glitches and higher values present the beep sound. In general, it has been reduced.
-Misa
On Tuesday 19 May 2009 09:47:13 ext Aggarwal, Anuj wrote:
I am facing problems on the capture side now, I don't know whether someone has tested it or not.
By default, capture was not working. I have to enable Analog Left Capture Route AUXL & Analog Right Capture Route AUXR to enable the capture paths for OMAP3 EVM. Even after enabling both the paths, I am not able to capture audio on both the channels. I am hearing audio only on either of the channel, the other one is playing only noise for me. (This channel swapping is random, sometimes it plays on left and sometimes on right.)
If I disable ' Analog Right Capture Route AUXR', I am not hearing any noise; audio is playing fine, but again on either of the channels, not consistently on the left one.
Moreover, in both the cases, I am getting large overrun errors (I am using NFS to store the recorded clips, will try with MMC etc. too):
root:~# arecord -f cd -d 30 rec05.dat Recording WAVE 'rec05.dat' : Signed 16 bit Little Endian, Rate 44100 Hz, Stereo overrun!!! (at least 8424.774 ms long) overrun!!! (at least 6465.943 ms long) overrun!!! (at least 3377.716 ms long) overrun!!! (at least 7982.147 ms long) overrun!!! (at least 8649.720 ms long)
I didn't face both the problems on my original code base (2.6.29-rc3) and hence wanted to confirm whether someone else has also faced the same issues. I am debugging at my end to see what difference is causing the problem but any pointers are welcome.
I have sent a patch, which should get the AUXR working again.
Thanks and Regards, Anuj Aggarwal
-----Original Message----- From: Peter Ujfalusi [mailto:peter.ujfalusi@nokia.com] Sent: Tuesday, May 19, 2009 1:30 PM To: Aggarwal, Anuj Cc: Mark Brown; sakoman@gmail.com; alsa-devel@alsa-project.org; getarunks@gmail.com; Lopez Cruz, Misael Subject: Re: [alsa-devel] [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix
On Tuesday 19 May 2009 09:47:13 ext Aggarwal, Anuj wrote:
I am facing problems on the capture side now, I don't know whether
someone
has tested it or not.
By default, capture was not working. I have to enable Analog Left Capture Route AUXL & Analog Right Capture Route AUXR to enable the capture paths for OMAP3 EVM. Even after enabling both the paths, I am not able to capture audio on both the channels. I am hearing audio only on either of the channel, the other one is playing only noise for me. (This channel swapping is random, sometimes it plays on left and sometimes on right.)
If I disable ' Analog Right Capture Route AUXR', I am not hearing any noise; audio is playing fine, but again on either of the channels, not consistently on the left one.
Moreover, in both the cases, I am getting large overrun errors (I am using NFS to store the recorded clips, will try with MMC etc. too):
root:~# arecord -f cd -d 30 rec05.dat Recording WAVE 'rec05.dat' : Signed 16 bit Little Endian, Rate 44100 Hz, Stereo overrun!!! (at least 8424.774 ms long) overrun!!! (at least 6465.943 ms long) overrun!!! (at least 3377.716 ms long) overrun!!! (at least 7982.147 ms long) overrun!!! (at least 8649.720 ms long)
I didn't face both the problems on my original code base (2.6.29-rc3) and hence wanted to confirm whether someone else has also faced the same issues. I am debugging at my end to see what difference is causing the problem but any pointers are welcome.
I have sent a patch, which should get the AUXR working again.
[Aggarwal, Anuj] Yeah, it is working fine now. Thanks for this. I am still getting underrun errors, looking into this now.
Thanks and Regards, Anuj Aggarwal
-- Péter
On Monday 18 May 2009 18:28:52 ext Aggarwal, Anuj wrote:
[Aggarwal, Anuj] I tried this on OMAP3 EVM. Ramp delay values 0-3 produces minor glitches in the beginning & end, value 4 gave me the best result so far on my EVM, I will try with some other EVMs as well before finalizing it. 5 and others produce a beep in the beginning and end. Just one question, is there any side-effect of choosing a large ramp delay? For e.g. in my case, it could be 437ms (19.2MHz clock).
It is less than half a second ;) I'm not sure, but we could try later to add a timer to finish up the ramp- down. There could be (not foreseen) side effects... At this point I think this (series) is a good start and it is better than what we had.
BTW: I'll wait for the verdict on the series and I will send a fix for the Handsfree pop attenuation also.
On Tue, 19 May 2009 09:46:40 +0300 Peter Ujfalusi peter.ujfalusi@nokia.com wrote:
On Monday 18 May 2009 18:28:52 ext Aggarwal, Anuj wrote:
[Aggarwal, Anuj] I tried this on OMAP3 EVM. Ramp delay values 0-3 produces minor glitches in the beginning & end, value 4 gave me the best result so far on my EVM, I will try with some other EVMs as well before finalizing it. 5 and others produce a beep in the beginning and end. Just one question, is there any side-effect of choosing a large ramp delay? For e.g. in my case, it could be 437ms (19.2MHz clock).
It is less than half a second ;) I'm not sure, but we could try later to add a timer to finish up the ramp- down. There could be (not foreseen) side effects... At this point I think this (series) is a good start and it is better than what we had.
For my Beagle the power-up/-down low-frequency pop goes more silent as the ramp time increases. Sharp power-down snap is there with '27/20/14 ms' and '55/40/27 ms' but not times above.
Beep sound is here when ramp is '109/81/55 ms' and above. As ramp time increases, the beep sound frequency goes down. Codec HW issue?
I don't have any objections to these two patches and as the music still plays fine so probably these can be pushed in safely.
On Tue, May 19, 2009 at 10:33:16AM +0300, Jarkko Nikula wrote:
I don't have any objections to these two patches and as the music still plays fine so probably these can be pushed in safely.
It seems like the feedback has been pretty much OK so it should be OK apply the patches - Peter?
On Tuesday 19 May 2009 21:58:06 ext Mark Brown wrote:
On Tue, May 19, 2009 at 10:33:16AM +0300, Jarkko Nikula wrote:
I don't have any objections to these two patches and as the music still plays fine so probably these can be pushed in safely.
It seems like the feedback has been pretty much OK so it should be OK apply the patches - Peter?
I also think, that it should be OK to apply this series.
Thanks, Péter
-----Original Message----- From: Peter Ujfalusi [mailto:peter.ujfalusi@nokia.com] Sent: Wednesday, May 20, 2009 11:27 AM To: ext Mark Brown Cc: Jarkko Nikula; Aggarwal, Anuj; sakoman@gmail.com; alsa-devel@alsa- project.org; getarunks@gmail.com Subject: Re: [alsa-devel] [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix
On Tuesday 19 May 2009 21:58:06 ext Mark Brown wrote:
On Tue, May 19, 2009 at 10:33:16AM +0300, Jarkko Nikula wrote:
I don't have any objections to these two patches and as the music still plays fine so probably these can be pushed in safely.
It seems like the feedback has been pretty much OK so it should be OK apply the patches - Peter?
I also think, that it should be OK to apply this series.
[Aggarwal, Anuj] Since different platforms are using different ramp values, how should it be handled via the same codec source code? How should this be done, without modifying the code base? Or should it be done every time user wants to play an audio file?
Thanks, Péter
On Wed, May 20, 2009 at 01:51:02PM +0530, Aggarwal, Anuj wrote:
Peter Ujfalusi [mailto:peter.ujfalusi@nokia.com]:
On Tuesday 19 May 2009 21:58:06 ext Mark Brown wrote:
It seems like the feedback has been pretty much OK so it should be OK apply the patches - Peter?
I also think, that it should be OK to apply this series.
Applied - thanks for your hard work on this!
BTW, I don't know if you noticed the DAPM changes I pushed the other day. One commit in particular, 452c5eaa0d5162e02ffee742ea17540887bc2904, might be interesting for TWL4030 - it changes things so that we're in BIAS_ON whenever there's a bias path active, not only when there's a DAC or ADC path. I've got a feeling some of the pop/click stuff may mean that's not as useful as it might've been in the past.
[Aggarwal, Anuj] Since different platforms are using different ramp values, how should it be handled via the same codec source code? How should this be done, without modifying the code base? Or should it be done every time user wants to play an audio file?
Since it's just a change in one parameter the driver should be able to arrange to get that configuration value provided to it via platform data. It's still the same chip and needs the same driver, all that's changing is configuration information.
On Wednesday 20 May 2009 11:51:21 ext Mark Brown wrote:
Applied - thanks for your hard work on this!
BTW, I don't know if you noticed the DAPM changes I pushed the other day. One commit in particular, 452c5eaa0d5162e02ffee742ea17540887bc2904, might be interesting for TWL4030 - it changes things so that we're in BIAS_ON whenever there's a bias path active, not only when there's a DAC or ADC path. I've got a feeling some of the pop/click stuff may mean that's not as useful as it might've been in the past.
I will look at this change. BTW: I just got the Beagle board, it will take some time to get familiar with it, but than I can really use the latest and greatest of ASoC...
[Aggarwal, Anuj] Since different platforms are using different ramp values, how should it be handled via the same codec source code? How should this be done, without modifying the code base? Or should it be done every time user wants to play an audio file?
Since it's just a change in one parameter the driver should be able to arrange to get that configuration value provided to it via platform data. It's still the same chip and needs the same driver, all that's changing is configuration information.
As for now, storing the mixer settings and restoring those at boot time would be the way. I will look at the possibility to add platform data parameter to twl4030 codec to set the board specific initial ramp delay value.
-----Original Message----- From: Peter Ujfalusi [mailto:peter.ujfalusi@nokia.com] Sent: Wednesday, May 20, 2009 2:43 PM To: alsa-devel@alsa-project.org Cc: ext Mark Brown; Aggarwal, Anuj; sakoman@gmail.com; getarunks@gmail.com Subject: Re: [alsa-devel] [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix
On Wednesday 20 May 2009 11:51:21 ext Mark Brown wrote:
Applied - thanks for your hard work on this!
BTW, I don't know if you noticed the DAPM changes I pushed the other day. One commit in particular,
452c5eaa0d5162e02ffee742ea17540887bc2904,
might be interesting for TWL4030 - it changes things so that we're in BIAS_ON whenever there's a bias path active, not only when there's a DAC or ADC path. I've got a feeling some of the pop/click stuff may mean that's not as useful as it might've been in the past.
I will look at this change. BTW: I just got the Beagle board, it will take some time to get familiar with it, but than I can really use the latest and greatest of ASoC...
[Aggarwal, Anuj] Since different platforms are using different ramp values, how should it be handled via the same codec source code? How should this be done, without modifying the code base? Or should it be done every time user wants to play an audio file?
Since it's just a change in one parameter the driver should be able to arrange to get that configuration value provided to it via platform data. It's still the same chip and needs the same driver, all that's changing is configuration information.
As for now, storing the mixer settings and restoring those at boot time would be the way. I will look at the possibility to add platform data parameter to twl4030 codec to set the board specific initial ramp delay value.
[Aggarwal, Anuj] I tried to look into the soc-dapm.h for the appropriate platform/machine-domain specific widget which could do the needful but could not find any. What is the correct way of handling this? I want to pass the correct ramp value for my platform.
-- Péter
On Thu, May 21, 2009 at 04:17:45PM +0530, Aggarwal, Anuj wrote:
[Aggarwal, Anuj] I tried to look into the soc-dapm.h for the appropriate platform/machine-domain specific widget which could do the needful but could not find any. What is the correct way of handling this? I want to pass the correct ramp value for my platform.
Provide standard platform data for the twl4030, which would need the driver patching to use that (and hopefully converted to the standard device model). Or for the time being you can do a register write in the DAI init function, but the way forward is the platform data.
On Wednesday 20 May 2009 11:21:02 ext Aggarwal, Anuj wrote:
[Aggarwal, Anuj] Since different platforms are using different ramp values, how should it be handled via the same codec source code? How should this be done, without modifying the code base? Or should it be done every time user wants to play an audio file?
I think we should leave the ramp delay configurable, since it is easier to 'port' the codec to a new board. We can add - as Mark already suggested - a platform data parameter for the twl4030 codec driver to set the correct ramp delay value for the given board. The actual user will still have means to change it, but if the correct ramp delay has been found for the board, the pop attenuation would work out of box on the boards.
participants (5)
-
Aggarwal, Anuj -
Jarkko Nikula -
Lopez Cruz, Misael -
Mark Brown -
Peter Ujfalusi