[alsa-devel] [PATCH] ASoC: rt286: fix headphone click/crack noise on Dell XPS 9343 I2S mode

[Kai-Heng Feng]

On Thu, Feb 16, 2017 at 2:53 PM, Bard Liao <bardliao at realtek.com> wrote:
>> -----Original Message-----
>> From: Kai-Heng Feng [mailto:kai.heng.feng at canonical.com]
>> Sent: Monday, January 23, 2017 1:35 PM
>> To: Bard Liao
>> Cc: Oder Chiou; lgirdwood at gmail.com; broonie at kernel.org;
>> alsa-devel at alsa-project.org; Kai-Heng Feng
>> Subject: [PATCH] ASoC: rt286: fix headphone click/crack noise on Dell XPS 9343
>> I2S mode
>>
>> HDA mode does not have these issues because necessary workarounds in
>> linux/sound/pci/hda/patch_realtek.c are already applied. So we can
>> leverage these workaournds into rt286.
>>
>> When jack is plugged, it rapidly generates I2C interrupts, which
>> triggers rt286_irq() and rt286_jack_detect(), which causes the noise.
>> alc_fixup_dell_xps13() in patch_realtek.c sets up a pin that can stop
>> the frantic interrupts, hence fixes the click noise.
>>
>> When rt286 is under powersaving state, play a sound with headphone or
>> plug a headphone in will produce a loud crack sound.
>> alc_shutup_dell_xps13() patch_realtek.c mutes the headphone at plugging.
>> Apply the same workaround to LDO2 power event can make the loud crack
>> sound to a more tolerable pop sound. I found that unconditionally apply
>> the workaround to all related power event can help a little further.
>>
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=112611
>> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1313434
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng at canonical.com>
>> ---
>>  sound/soc/codecs/rt286.c | 67
>> +++++++++++++++++++++++++++++++++++++++++++++---
>>  1 file changed, 63 insertions(+), 4 deletions(-)
>>
>> diff --git a/sound/soc/codecs/rt286.c b/sound/soc/codecs/rt286.c
>> index 74c0e4e..d041937 100644
>> --- a/sound/soc/codecs/rt286.c
>> +++ b/sound/soc/codecs/rt286.c
>> @@ -36,6 +36,8 @@
>>  #define RT286_VENDOR_ID 0x10ec0286
>>  #define RT288_VENDOR_ID 0x10ec0288
>>
>> +#define AMP_OUT_MUTE 0xb080
>> +
>>  struct rt286_priv {
>>       struct reg_default *index_cache;
>>       int index_cache_size;
>> @@ -47,6 +49,7 @@ struct rt286_priv {
>>       struct delayed_work jack_detect_work;
>>       int sys_clk;
>>       int clk_id;
>> +     void (*mute_hpo_func)(struct snd_soc_codec *codec);
>>  };
>>
>>  static const struct reg_default rt286_index_def[] = {
>> @@ -472,10 +475,51 @@ static int rt286_set_dmic1_event(struct
>> snd_soc_dapm_widget *w,
>>       return 0;
>>  }
>>
>> +/* Add HV, VREF and LDO1 event functions to workaround headphone crack
>> noise */
>> +static int rt286_hv_event(struct snd_soc_dapm_widget *w,
>> +                          struct snd_kcontrol *kcontrol, int event)
>> +{
>> +     struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
>> +     struct rt286_priv *rt286 = snd_soc_codec_get_drvdata(codec);
>> +
>> +     if (rt286->mute_hpo_func)
>> +             rt286->mute_hpo_func(codec);
>> +
>> +     return 0;
>> +}
>> +
>> +static int rt286_vref_event(struct snd_soc_dapm_widget *w,
>> +                          struct snd_kcontrol *kcontrol, int event)
>> +{
>> +     struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
>> +     struct rt286_priv *rt286 = snd_soc_codec_get_drvdata(codec);
>> +
>> +     if (rt286->mute_hpo_func)
>> +             rt286->mute_hpo_func(codec);
>> +
>> +     return 0;
>> +}
>> +
>> +static int rt286_ldo1_event(struct snd_soc_dapm_widget *w,
>> +                          struct snd_kcontrol *kcontrol, int event)
>> +{
>> +     struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
>> +     struct rt286_priv *rt286 = snd_soc_codec_get_drvdata(codec);
>> +
>> +     if (rt286->mute_hpo_func)
>> +             rt286->mute_hpo_func(codec);
>> +
>> +     return 0;
>> +}
>> +
>>  static int rt286_ldo2_event(struct snd_soc_dapm_widget *w,
>>                            struct snd_kcontrol *kcontrol, int event)
>>  {
>>       struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
>> +     struct rt286_priv *rt286 = snd_soc_codec_get_drvdata(codec);
>> +
>> +     if (rt286->mute_hpo_func)
>> +             rt286->mute_hpo_func(codec);
>>
>>       switch (event) {
>>       case SND_SOC_DAPM_POST_PMU:
>> @@ -516,16 +560,24 @@ static int rt286_mic1_event(struct
>> snd_soc_dapm_widget *w,
>>       return 0;
>>  }
>>
>> +static void dell_dino_mute_hpo(struct snd_soc_codec *codec)
>> +{
>> +     snd_soc_write(codec, RT286_SET_AMP_GAIN_HPO, AMP_OUT_MUTE);
>> +}
>> +
>
> I didn't see where will headphone be unmute. There is already a
> headphone mute control on the driver. See
>         SND_SOC_DAPM_SWITCH("HPO L", SND_SOC_NOPM, 0, 0,
>                 &hpol_enable_control),
>         SND_SOC_DAPM_SWITCH("HPO R", SND_SOC_NOPM, 0, 0,
>                 &hpor_enable_control),

This is a direct mirror to function alc_shutup_dell_xps13() in
sound/pci/hda/patch_realtek.c.

I'll try to use what you suggest here.

>
>
>>       SND_SOC_DAPM_SUPPLY_S("LDO2", 2, RT286_POWER_CTRL1,
>>               13, 1, rt286_ldo2_event, SND_SOC_DAPM_PRE_PMD |
>> -             SND_SOC_DAPM_POST_PMU),
>> +             SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU),
>
> Is POST_PMU really what you want?

I found that mute output before any power event has better result than
mute after.

>
>> @@ -1190,6 +1242,11 @@ static int rt286_i2c_probe(struct i2c_client *i2c,
>>               regmap_update_bits(rt286->regmap,
>>                                       RT286_CBJ_CTRL1, 0xf000, 0xb000);
>>       } else {
>> +             /* Fix headphone click noise */
>> +             if (dmi_check_system(dmi_dell_dino))
>> +                     regmap_write(rt286->regmap,
>> +                                     RT286_MIC1_DET_CTRL, 0x0020);
>> +
>
> I need to figure out how 0x0020 works.

It's from commit 3e1b0c4a9d563d7fc6e22dc92613cd3237bb5ce0

>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel