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

[Bard Liao]

> -----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),


>  	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?

> @@ -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.