[alsa-devel] [PATCH] ASoC: rt5677: Avoid the pop sound that comes from the filter power
Anatol Pomazau
anatol at google.com
Wed Nov 11 18:29:03 CET 2015
Hi
On Tue, Nov 10, 2015 at 6:25 PM, Oder Chiou <oder_chiou at realtek.com> wrote:
> Hi Anatol,
>
>
>
> What these AUTODISABLE above for? Are they needed?
>
> They are needed for the power up sequence that make sure the un-mute
> should be done after the filter power up.
>
Thanks for the explanation.
Reported-by: Anatol Pomozov <anatol.pomozov at gmail.com>
Tested-by: Anatol Pomozov <anatol.pomozov at gmail.com>
For those of us who does not have access to the internal structure of the
> chip, could you please add a comment why this delay is needed here? What
> the chip is doing these 50ms?
>
> The delay is done before the mixer un-mute, and it should delay some time
> to prevent the pop sound.
>
I was looking for more technical information to understand this problem
better. Does this digital filter has an analog widget that needs time to
power up, some kind of charge pump for bias voltage? And please add the
comment to the rt5677_filter_power_event() function, future code readers
say "thank you" for having clear explanation in places like this one.
The 50ms delay is decided by our internal discussion, and it just waits the
> pop sound going away
>
Is there a way to reduce this 50ms delay? Maybe use some kind of a
precharger?
>
> *From:* Anatol Pomazau [mailto:anatol at google.com]
> *Sent:* Tuesday, November 10, 2015 2:05 AM
> *To:* Oder Chiou
> *Cc:* broonie at kernel.org; lgirdwood at gmail.com; alsa-devel at alsa-project.org;
> Flove; Bard Liao; John Lin; Jack Yu; Albert Chen
> *Subject:* Re: [PATCH] ASoC: rt5677: Avoid the pop sound that comes from
> the filter power
>
>
>
> Hi
>
>
>
> Thanks Oder into looking this.
>
>
>
> We've been working with awesome Realtek engineers on fixing pop in
> AIF1->AIF2 audio path and I confirm that this patch fixes the problem.
>
>
>
>
>
> On Mon, Nov 9, 2015 at 2:01 AM, Oder Chiou <oder_chiou at realtek.com> wrote:
>
> The patch changes the type of DACs mixer to AUTODISABLE and add the delay
> time after power up to avoid the pop sound that comes from the filter
> power.
>
> Signed-off-by: Oder Chiou <oder_chiou at realtek.com>
> ---
> sound/soc/codecs/rt5677.c | 100
> ++++++++++++++++++++++++++++------------------
> 1 file changed, 61 insertions(+), 39 deletions(-)
>
> diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
> index b4cd7e3..69d987a 100644
> --- a/sound/soc/codecs/rt5677.c
> +++ b/sound/soc/codecs/rt5677.c
> @@ -1386,90 +1386,90 @@ static const struct snd_kcontrol_new
> rt5677_dac_r_mix[] = {
> };
>
> static const struct snd_kcontrol_new rt5677_sto1_dac_l_mix[] = {
> - SOC_DAPM_SINGLE("ST L Switch", RT5677_STO1_DAC_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("ST L Switch", RT5677_STO1_DAC_MIXER,
> RT5677_M_ST_DAC1_L_SFT, 1, 1),
> - SOC_DAPM_SINGLE("DAC1 L Switch", RT5677_STO1_DAC_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("DAC1 L Switch", RT5677_STO1_DAC_MIXER,
> RT5677_M_DAC1_L_STO_L_SFT, 1, 1),
> - SOC_DAPM_SINGLE("DAC2 L Switch", RT5677_STO1_DAC_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("DAC2 L Switch", RT5677_STO1_DAC_MIXER,
> RT5677_M_DAC2_L_STO_L_SFT, 1, 1),
> - SOC_DAPM_SINGLE("DAC1 R Switch", RT5677_STO1_DAC_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("DAC1 R Switch", RT5677_STO1_DAC_MIXER,
> RT5677_M_DAC1_R_STO_L_SFT, 1, 1),
> };
>
> static const struct snd_kcontrol_new rt5677_sto1_dac_r_mix[] = {
> - SOC_DAPM_SINGLE("ST R Switch", RT5677_STO1_DAC_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("ST R Switch", RT5677_STO1_DAC_MIXER,
> RT5677_M_ST_DAC1_R_SFT, 1, 1),
> - SOC_DAPM_SINGLE("DAC1 R Switch", RT5677_STO1_DAC_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("DAC1 R Switch", RT5677_STO1_DAC_MIXER,
> RT5677_M_DAC1_R_STO_R_SFT, 1, 1),
> - SOC_DAPM_SINGLE("DAC2 R Switch", RT5677_STO1_DAC_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("DAC2 R Switch", RT5677_STO1_DAC_MIXER,
> RT5677_M_DAC2_R_STO_R_SFT, 1, 1),
> - SOC_DAPM_SINGLE("DAC1 L Switch", RT5677_STO1_DAC_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("DAC1 L Switch", RT5677_STO1_DAC_MIXER,
> RT5677_M_DAC1_L_STO_R_SFT, 1, 1),
> };
>
> static const struct snd_kcontrol_new rt5677_mono_dac_l_mix[] = {
> - SOC_DAPM_SINGLE("ST L Switch", RT5677_MONO_DAC_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("ST L Switch", RT5677_MONO_DAC_MIXER,
> RT5677_M_ST_DAC2_L_SFT, 1, 1),
> - SOC_DAPM_SINGLE("DAC1 L Switch", RT5677_MONO_DAC_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("DAC1 L Switch", RT5677_MONO_DAC_MIXER,
> RT5677_M_DAC1_L_MONO_L_SFT, 1, 1),
> - SOC_DAPM_SINGLE("DAC2 L Switch", RT5677_MONO_DAC_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("DAC2 L Switch", RT5677_MONO_DAC_MIXER,
> RT5677_M_DAC2_L_MONO_L_SFT, 1, 1),
> - SOC_DAPM_SINGLE("DAC2 R Switch", RT5677_MONO_DAC_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("DAC2 R Switch", RT5677_MONO_DAC_MIXER,
> RT5677_M_DAC2_R_MONO_L_SFT, 1, 1),
> };
>
> static const struct snd_kcontrol_new rt5677_mono_dac_r_mix[] = {
> - SOC_DAPM_SINGLE("ST R Switch", RT5677_MONO_DAC_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("ST R Switch", RT5677_MONO_DAC_MIXER,
> RT5677_M_ST_DAC2_R_SFT, 1, 1),
> - SOC_DAPM_SINGLE("DAC1 R Switch", RT5677_MONO_DAC_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("DAC1 R Switch", RT5677_MONO_DAC_MIXER,
> RT5677_M_DAC1_R_MONO_R_SFT, 1, 1),
> - SOC_DAPM_SINGLE("DAC2 R Switch", RT5677_MONO_DAC_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("DAC2 R Switch", RT5677_MONO_DAC_MIXER,
> RT5677_M_DAC2_R_MONO_R_SFT, 1, 1),
> - SOC_DAPM_SINGLE("DAC2 L Switch", RT5677_MONO_DAC_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("DAC2 L Switch", RT5677_MONO_DAC_MIXER,
> RT5677_M_DAC2_L_MONO_R_SFT, 1, 1),
> };
>
> static const struct snd_kcontrol_new rt5677_dd1_l_mix[] = {
> - SOC_DAPM_SINGLE("Sto DAC Mix L Switch", RT5677_DD1_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("Sto DAC Mix L Switch",
> RT5677_DD1_MIXER,
> RT5677_M_STO_L_DD1_L_SFT, 1, 1),
> - SOC_DAPM_SINGLE("Mono DAC Mix L Switch", RT5677_DD1_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("Mono DAC Mix L Switch",
> RT5677_DD1_MIXER,
> RT5677_M_MONO_L_DD1_L_SFT, 1, 1),
> - SOC_DAPM_SINGLE("DAC3 L Switch", RT5677_DD1_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("DAC3 L Switch", RT5677_DD1_MIXER,
> RT5677_M_DAC3_L_DD1_L_SFT, 1, 1),
> - SOC_DAPM_SINGLE("DAC3 R Switch", RT5677_DD1_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("DAC3 R Switch", RT5677_DD1_MIXER,
> RT5677_M_DAC3_R_DD1_L_SFT, 1, 1),
> };
>
> static const struct snd_kcontrol_new rt5677_dd1_r_mix[] = {
> - SOC_DAPM_SINGLE("Sto DAC Mix R Switch", RT5677_DD1_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("Sto DAC Mix R Switch",
> RT5677_DD1_MIXER,
> RT5677_M_STO_R_DD1_R_SFT, 1, 1),
> - SOC_DAPM_SINGLE("Mono DAC Mix R Switch", RT5677_DD1_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("Mono DAC Mix R Switch",
> RT5677_DD1_MIXER,
> RT5677_M_MONO_R_DD1_R_SFT, 1, 1),
> - SOC_DAPM_SINGLE("DAC3 R Switch", RT5677_DD1_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("DAC3 R Switch", RT5677_DD1_MIXER,
> RT5677_M_DAC3_R_DD1_R_SFT, 1, 1),
> - SOC_DAPM_SINGLE("DAC3 L Switch", RT5677_DD1_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("DAC3 L Switch", RT5677_DD1_MIXER,
> RT5677_M_DAC3_L_DD1_R_SFT, 1, 1),
> };
>
> static const struct snd_kcontrol_new rt5677_dd2_l_mix[] = {
> - SOC_DAPM_SINGLE("Sto DAC Mix L Switch", RT5677_DD2_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("Sto DAC Mix L Switch",
> RT5677_DD2_MIXER,
> RT5677_M_STO_L_DD2_L_SFT, 1, 1),
> - SOC_DAPM_SINGLE("Mono DAC Mix L Switch", RT5677_DD2_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("Mono DAC Mix L Switch",
> RT5677_DD2_MIXER,
> RT5677_M_MONO_L_DD2_L_SFT, 1, 1),
> - SOC_DAPM_SINGLE("DAC4 L Switch", RT5677_DD2_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("DAC4 L Switch", RT5677_DD2_MIXER,
> RT5677_M_DAC4_L_DD2_L_SFT, 1, 1),
> - SOC_DAPM_SINGLE("DAC4 R Switch", RT5677_DD2_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("DAC4 R Switch", RT5677_DD2_MIXER,
> RT5677_M_DAC4_R_DD2_L_SFT, 1, 1),
> };
>
> static const struct snd_kcontrol_new rt5677_dd2_r_mix[] = {
> - SOC_DAPM_SINGLE("Sto DAC Mix R Switch", RT5677_DD2_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("Sto DAC Mix R Switch",
> RT5677_DD2_MIXER,
> RT5677_M_STO_R_DD2_R_SFT, 1, 1),
> - SOC_DAPM_SINGLE("Mono DAC Mix R Switch", RT5677_DD2_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("Mono DAC Mix R Switch",
> RT5677_DD2_MIXER,
> RT5677_M_MONO_R_DD2_R_SFT, 1, 1),
> - SOC_DAPM_SINGLE("DAC4 R Switch", RT5677_DD2_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("DAC4 R Switch", RT5677_DD2_MIXER,
> RT5677_M_DAC4_R_DD2_R_SFT, 1, 1),
> - SOC_DAPM_SINGLE("DAC4 L Switch", RT5677_DD2_MIXER,
> + SOC_DAPM_SINGLE_AUTODISABLE("DAC4 L Switch", RT5677_DD2_MIXER,
> RT5677_M_DAC4_L_DD2_R_SFT, 1, 1),
> };
>
>
>
> I tested the kernel change with filter power delay only (the changes
> below) and it was enough for me to fix the pop sound.
>
>
>
> What these AUTODISABLE above for? Are they needed?
>
>
>
>
>
>
> @@ -2596,6 +2596,21 @@ static int rt5677_vref_event(struct
> snd_soc_dapm_widget *w,
> return 0;
> }
>
> +static int rt5677_filter_power_event(struct snd_soc_dapm_widget *w,
> + struct snd_kcontrol *kcontrol, int event)
> +{
> + switch (event) {
> + case SND_SOC_DAPM_POST_PMU:
> + msleep(50);
>
>
>
> For those of us who does not have access to the internal structure of the
> chip, could you please add a comment why this delay is needed here? What
> the chip is doing these 50ms?
>
>
>
>
>
> + break;
> +
> + default:
> + return 0;
> + }
> +
> + return 0;
> +}
> +
> static const struct snd_soc_dapm_widget rt5677_dapm_widgets[] = {
> SND_SOC_DAPM_SUPPLY("PLL1", RT5677_PWR_ANLG2, RT5677_PWR_PLL1_BIT,
> 0, rt5677_set_pll1_event, SND_SOC_DAPM_PRE_PMU |
> @@ -3072,19 +3087,26 @@ static const struct snd_soc_dapm_widget
> rt5677_dapm_widgets[] = {
>
> /* DAC Mixer */
> SND_SOC_DAPM_SUPPLY("dac stereo1 filter", RT5677_PWR_DIG2,
> - RT5677_PWR_DAC_S1F_BIT, 0, NULL, 0),
> + RT5677_PWR_DAC_S1F_BIT, 0, rt5677_filter_power_event,
> + SND_SOC_DAPM_POST_PMU),
> SND_SOC_DAPM_SUPPLY("dac mono2 left filter", RT5677_PWR_DIG2,
> - RT5677_PWR_DAC_M2F_L_BIT, 0, NULL, 0),
> + RT5677_PWR_DAC_M2F_L_BIT, 0, rt5677_filter_power_event,
> + SND_SOC_DAPM_POST_PMU),
> SND_SOC_DAPM_SUPPLY("dac mono2 right filter", RT5677_PWR_DIG2,
> - RT5677_PWR_DAC_M2F_R_BIT, 0, NULL, 0),
> + RT5677_PWR_DAC_M2F_R_BIT, 0, rt5677_filter_power_event,
> + SND_SOC_DAPM_POST_PMU),
> SND_SOC_DAPM_SUPPLY("dac mono3 left filter", RT5677_PWR_DIG2,
> - RT5677_PWR_DAC_M3F_L_BIT, 0, NULL, 0),
> + RT5677_PWR_DAC_M3F_L_BIT, 0, rt5677_filter_power_event,
> + SND_SOC_DAPM_POST_PMU),
> SND_SOC_DAPM_SUPPLY("dac mono3 right filter", RT5677_PWR_DIG2,
> - RT5677_PWR_DAC_M3F_R_BIT, 0, NULL, 0),
> + RT5677_PWR_DAC_M3F_R_BIT, 0, rt5677_filter_power_event,
> + SND_SOC_DAPM_POST_PMU),
> SND_SOC_DAPM_SUPPLY("dac mono4 left filter", RT5677_PWR_DIG2,
> - RT5677_PWR_DAC_M4F_L_BIT, 0, NULL, 0),
> + RT5677_PWR_DAC_M4F_L_BIT, 0, rt5677_filter_power_event,
> + SND_SOC_DAPM_POST_PMU),
> SND_SOC_DAPM_SUPPLY("dac mono4 right filter", RT5677_PWR_DIG2,
> - RT5677_PWR_DAC_M4F_R_BIT, 0, NULL, 0),
> + RT5677_PWR_DAC_M4F_R_BIT, 0, rt5677_filter_power_event,
> + SND_SOC_DAPM_POST_PMU),
>
> SND_SOC_DAPM_MIXER("Stereo DAC MIXL", SND_SOC_NOPM, 0, 0,
> rt5677_sto1_dac_l_mix, ARRAY_SIZE(rt5677_sto1_dac_l_mix)),
> --
> 1.8.1.1.439.g50a6b54
>
>
>
>
> ------Please consider the environment before printing this e-mail.
>
More information about the Alsa-devel
mailing list