Re: Pop noise on startup when headphones are plugged in (Dell XPS13 9333)

[Takashi Iwai <tiwai@suse.de>]

At Thu, 23 Apr 2015 21:12:50 +0200,
Gabriele Mazzotta wrote:
> 
> On Sunday 19 April 2015 19:26:58 Gabriele Mazzotta wrote:
> > Hi,
> > 
> > I've recently found that commit a551d91473 ("ALSA: hda - Use regmap for
> > command verb caches, too") is somehow causing a pop noise on startup
> > when headphones are plugged in, but I couldn't figure out the exact
> > cause. Was this observed on other systems (mine is a Dell XPS13 9333,
> > Realtek ALC3661)? Does anyone have any idea of what the cause might be?
> 
> Hi,
> 
> I don't know why a551d91473 caused the issue, but I found the real
> cause of problem.
> 
> On init, create_input_ctls() sets the vref of nid 0x19 to 80 (as
> returned by snd_hda_get_default_vref()), but it should be set to HIZ.
> This is not so different from the issue addressed by f38663ab5c
> ("ALSA: hda - Set internal mic as default input source on Dell XPS 13 9333").
> 
> I made a patch to prevent this from happening.
> 
> Setting the vref is not necessary since alc_update_headset_mode() will
> take care of it.
> 
> Should I maybe add a new flag instead of using suppress_hp_mic_detect?

Yeah, that's better.  Although the flag is currently unused, it's
provided for a different purpose (to skip the headphone mic detection;
which is different from "headset" mic).

I wonder, though, whether the patch below improves anything.
A similar patch was in the development series in the past, but I had
to drop it because it caused behavior error.  But now I tried again,
and it seems working.


Takashi

---
diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index 2a8aa9dfb83d..8e47d9cc369f 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -79,6 +79,7 @@ struct hdac_device {
 	bool lazy_cache:1;	/* don't wake up for writes */
 	bool caps_overwriting:1; /* caps overwrite being in process */
 	bool cache_coef:1;	/* cache COEF read/write too */
+	bool cache_only:1;	/* don't write actually registers */
 };
 
 /* device/driver type used for matching */
diff --git a/sound/hda/hdac_regmap.c b/sound/hda/hdac_regmap.c
index 7371e0c3926f..c5b3dcbbacdd 100644
--- a/sound/hda/hdac_regmap.c
+++ b/sound/hda/hdac_regmap.c
@@ -265,6 +265,9 @@ static int hda_reg_write(void *context, unsigned int reg, unsigned int val)
 	unsigned int verb;
 	int i, bytes, err;
 
+	if (codec->cache_only)
+		return 0;
+
 	reg &= ~0x00080000U; /* drop GET bit */
 	reg |= (codec->addr << 28);
 	verb = get_verb(reg);
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index e70a7fb393dd..8f79d649975b 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -3156,28 +3156,29 @@ static void hda_call_codec_resume(struct hda_codec *codec)
 {
 	atomic_inc(&codec->core.in_pm);
 
-	if (codec->core.regmap)
-		regcache_mark_dirty(codec->core.regmap);
-
 	codec->power_jiffies = jiffies;
 
 	hda_set_power_state(codec, AC_PWRST_D0);
 	restore_shutup_pins(codec);
 	hda_exec_init_verbs(codec);
 	snd_hda_jack_set_dirty_all(codec);
+	if (codec->core.regmap) {
+		regcache_mark_dirty(codec->core.regmap);
+		codec->core.cache_only = true;
+	}
 	if (codec->patch_ops.resume)
 		codec->patch_ops.resume(codec);
-	else {
-		if (codec->patch_ops.init)
-			codec->patch_ops.init(codec);
-		if (codec->core.regmap)
-			regcache_sync(codec->core.regmap);
-	}
+	else if (codec->patch_ops.init)
+		codec->patch_ops.init(codec);
 
 	if (codec->jackpoll_interval)
 		hda_jackpoll_work(&codec->jackpoll_work.work);
 	else
 		snd_hda_jack_report_sync(codec);
+	if (codec->core.regmap) {
+		codec->core.cache_only = false;
+		regcache_sync(codec->core.regmap);
+	}
 	atomic_dec(&codec->core.in_pm);
 }
 
diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c
index 3d2597b7037b..844d5e6ae1a1 100644
--- a/sound/pci/hda/hda_generic.c
+++ b/sound/pci/hda/hda_generic.c
@@ -5780,8 +5780,6 @@ int snd_hda_gen_init(struct hda_codec *codec)
 	/* call init functions of standard auto-mute helpers */
 	update_automute_all(codec);
 
-	regcache_sync(codec->core.regmap);
-
 	if (spec->vmaster_mute.sw_kctl && spec->vmaster_mute.hook)
 		snd_hda_sync_vmaster_hook(&spec->vmaster_mute);
 
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 5f44f60a6389..e4a774ddb1fc 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -2211,7 +2211,6 @@ static int generic_hdmi_resume(struct hda_codec *codec)
 	int pin_idx;
 
 	codec->patch_ops.init(codec);
-	regcache_sync(codec->core.regmap);
 
 	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
 		struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index b18b9c67b262..34219aa6a40e 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -799,7 +799,6 @@ static int alc_resume(struct hda_codec *codec)
 	if (!spec->no_depop_delay)
 		msleep(150); /* to avoid pop noise */
 	codec->patch_ops.init(codec);
-	regcache_sync(codec->core.regmap);
 	hda_call_check_power_status(codec, 0x01);
 	return 0;
 }
@@ -3059,7 +3058,6 @@ static int alc269_resume(struct hda_codec *codec)
 		msleep(200);
 	}
 
-	regcache_sync(codec->core.regmap);
 	hda_call_check_power_status(codec, 0x01);
 
 	/* on some machine, the BIOS will clear the codec gpio data when enter