[alsa-devel] [PATCH] ASoC: sgtl5000: Fix VAG_POWER enabling/disabling order
The VAG_POWER must be enabled after all other bits in CHIP_ANA_POWER and disabled before any other bit in CHIP_ANA_POWER. See the SGTL5000 datasheet (Table 31, BIT 7, page 42-43). Failing to follow this order will result in ugly loud "POP" noise at the end of playback.
To achieve such order, use the _PRE and _POST DAPM widgets to trigger the power_vag_event, where the event type check has to be fixed accordingly as well.
Signed-off-by: Marek Vasut marex@denx.de Cc: Dong Aisheng dong.aisheng@linaro.org Cc: Eric Nelson eric.nelson@boundarydevices.com Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Mark Brown broonie@opensource.wolfsonmicro.com --- sound/soc/codecs/sgtl5000.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c index 92bbfec..2068c36 100644 --- a/sound/soc/codecs/sgtl5000.c +++ b/sound/soc/codecs/sgtl5000.c @@ -151,15 +151,15 @@ static int power_vag_event(struct snd_soc_dapm_widget *w, struct snd_kcontrol *kcontrol, int event) { switch (event) { - case SND_SOC_DAPM_PRE_PMU: + case SND_SOC_DAPM_POST_PMU: snd_soc_update_bits(w->codec, SGTL5000_CHIP_ANA_POWER, SGTL5000_VAG_POWERUP, SGTL5000_VAG_POWERUP); break;
- case SND_SOC_DAPM_POST_PMD: + case SND_SOC_DAPM_PRE_PMD: snd_soc_update_bits(w->codec, SGTL5000_CHIP_ANA_POWER, SGTL5000_VAG_POWERUP, 0); - msleep(400); + mdelay(400); break; default: break; @@ -217,12 +217,11 @@ static const struct snd_soc_dapm_widget sgtl5000_dapm_widgets[] = { 0, SGTL5000_CHIP_DIG_POWER, 1, 0),
- SND_SOC_DAPM_SUPPLY("VAG_POWER", SGTL5000_CHIP_ANA_POWER, 7, 0, - power_vag_event, - SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD), - SND_SOC_DAPM_ADC("ADC", "Capture", SGTL5000_CHIP_ANA_POWER, 1, 0), SND_SOC_DAPM_DAC("DAC", "Playback", SGTL5000_CHIP_ANA_POWER, 3, 0), + + SND_SOC_DAPM_PRE("VAG_POWER_PRE", power_vag_event), + SND_SOC_DAPM_POST("VAG_POWER_POST", power_vag_event), };
/* routes for sgtl5000 */ @@ -230,16 +229,13 @@ static const struct snd_soc_dapm_route sgtl5000_dapm_routes[] = { {"Capture Mux", "LINE_IN", "LINE_IN"}, /* line_in --> adc_mux */ {"Capture Mux", "MIC_IN", "MIC_IN"}, /* mic_in --> adc_mux */
- {"ADC", NULL, "VAG_POWER"}, {"ADC", NULL, "Capture Mux"}, /* adc_mux --> adc */ {"AIFOUT", NULL, "ADC"}, /* adc --> i2s_out */
- {"DAC", NULL, "VAG_POWER"}, {"DAC", NULL, "AIFIN"}, /* i2s-->dac,skip audio mux */ {"Headphone Mux", "DAC", "DAC"}, /* dac --> hp_mux */ {"LO", NULL, "DAC"}, /* dac --> line_out */
- {"LINE_IN", NULL, "VAG_POWER"}, {"Headphone Mux", "LINE_IN", "LINE_IN"},/* line_in --> hp_mux */ {"HP", NULL, "Headphone Mux"}, /* hp_mux --> hp */
Hi Marek,
On Fri, May 24, 2013 at 12:34 AM, Marek Vasut marex@denx.de wrote:
The VAG_POWER must be enabled after all other bits in CHIP_ANA_POWER and disabled before any other bit in CHIP_ANA_POWER. See the SGTL5000 datasheet (Table 31, BIT 7, page 42-43). Failing to follow this order will result in ugly loud "POP" noise at the end of playback.
To achieve such order, use the _PRE and _POST DAPM widgets to trigger the power_vag_event, where the event type check has to be fixed accordingly as well.
Signed-off-by: Marek Vasut marex@denx.de Cc: Dong Aisheng dong.aisheng@linaro.org Cc: Eric Nelson eric.nelson@boundarydevices.com Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Mark Brown broonie@opensource.wolfsonmicro.com
Thanks, tested on a mx51evk and it removes an annoying 'click' after an audio track is played.
case SND_SOC_DAPM_POST_PMD:
case SND_SOC_DAPM_PRE_PMD: snd_soc_update_bits(w->codec, SGTL5000_CHIP_ANA_POWER, SGTL5000_VAG_POWERUP, 0);
msleep(400);
mdelay(400);
What is the reasoning for this change?
Anyway:
Tested-by: Fabio Estevam fabio.estevam@freescale.com
Dear Fabio Estevam,
[Fixing Mark's address in the CC.]
Hi Marek,
On Fri, May 24, 2013 at 12:34 AM, Marek Vasut marex@denx.de wrote:
The VAG_POWER must be enabled after all other bits in CHIP_ANA_POWER and disabled before any other bit in CHIP_ANA_POWER. See the SGTL5000 datasheet (Table 31, BIT 7, page 42-43). Failing to follow this order will result in ugly loud "POP" noise at the end of playback.
To achieve such order, use the _PRE and _POST DAPM widgets to trigger the power_vag_event, where the event type check has to be fixed accordingly as well.
Signed-off-by: Marek Vasut marex@denx.de Cc: Dong Aisheng dong.aisheng@linaro.org Cc: Eric Nelson eric.nelson@boundarydevices.com Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Mark Brown broonie@opensource.wolfsonmicro.com
Thanks, tested on a mx51evk and it removes an annoying 'click' after an audio track is played.
That's good. But you still hear some noise when the headphone/lineout drivers are powered up/down, right?
case SND_SOC_DAPM_POST_PMD:
case SND_SOC_DAPM_PRE_PMD: snd_soc_update_bits(w->codec, SGTL5000_CHIP_ANA_POWER, SGTL5000_VAG_POWERUP, 0);
msleep(400);
mdelay(400);What is the reasoning for this change?
Me being a moron, that's the full reasoning. Sorry.
Best regards, Marek Vasut
On Tue, May 28, 2013 at 3:23 PM, Marek Vasut marex@denx.de wrote:
That's good. But you still hear some noise when the headphone/lineout drivers are powered up/down, right?
Only tested on mx51evk and I don't hear no more noises after your patch is applied.
Dear Fabio Estevam,
On Tue, May 28, 2013 at 3:23 PM, Marek Vasut marex@denx.de wrote:
That's good. But you still hear some noise when the headphone/lineout drivers are powered up/down, right?
Only tested on mx51evk and I don't hear no more noises after your patch is applied.
Crank the volume up, make it _REALLY_ loud and just before you start hearing the audio you're supposed to hear, the HP driver is enabled. This makes such a minor noise it seems. It's hard to hear when it's not loud enough ;-)
Best regards, Marek Vasut
On Fri, May 24, 2013 at 05:34:56AM +0200, Marek Vasut wrote:
- case SND_SOC_DAPM_POST_PMD:
- case SND_SOC_DAPM_PRE_PMD: snd_soc_update_bits(w->codec, SGTL5000_CHIP_ANA_POWER, SGTL5000_VAG_POWERUP, 0);
msleep(400);
mdelay(400);
This looks to be both unrelated and a regression - mdelay() should be more resource intensive than sleeping and if it's 400ms we shouldn't be that worried about delaying slightly longer.
Please note my updated mail address which I've been using for a while now, the Wolfson one will go bad at the end of the week.
Dear Mark Brown,
On Fri, May 24, 2013 at 05:34:56AM +0200, Marek Vasut wrote:
- case SND_SOC_DAPM_POST_PMD:
- case SND_SOC_DAPM_PRE_PMD: snd_soc_update_bits(w->codec, SGTL5000_CHIP_ANA_POWER,
SGTL5000_VAG_POWERUP, 0);
msleep(400);
mdelay(400);break;
This looks to be both unrelated and a regression - mdelay() should be more resource intensive than sleeping and if it's 400ms we shouldn't be that worried about delaying slightly longer.
Definitelly, this was not supposed to be part of the patch. Fixed in V2.
Please note my updated mail address which I've been using for a while now, the Wolfson one will go bad at the end of the week.
Roger. I hope you have a good time at your new position :)
Best regards, Marek Vasut
participants (3)
-
Fabio Estevam -
Marek Vasut -
Mark Brown