[alsa-devel] Pop noise on startup when headphones are plugged in (Dell XPS13 9333)
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?
Regards, Gabriele
At Sun, 19 Apr 2015 19:26:58 +0200, 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?
Are you sure that the commit is the culprit? There are lots of other changes like the widget-based power saving.
In anyway, please give alsa-info.sh output.
thanks,
Takashi
At Mon, 20 Apr 2015 07:58:15 +0200, Takashi Iwai wrote:
At Sun, 19 Apr 2015 19:26:58 +0200, 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?
Are you sure that the commit is the culprit? There are lots of other changes like the widget-based power saving.
FYI, the widget power-saving can be disabled by the following. (It can be done by hints, too.)
If this fixes the problem, we likely need more tweaks at path power activation. Or, as a last resort, put this to the fixup for your model.
Takashi
--- diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 231d0e4b9a95..637ff2b70f6e 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -5584,7 +5584,7 @@ static int patch_alc269(struct hda_codec *codec)
spec = codec->spec; spec->gen.shared_mic_vref_pin = 0x18; - codec->power_save_node = 1; + /* codec->power_save_node = 1; */
snd_hda_pick_fixup(codec, alc269_fixup_models, alc269_fixup_tbl, alc269_fixups);
On Monday 20 April 2015 09:18:40 Takashi Iwai wrote:
At Mon, 20 Apr 2015 07:58:15 +0200, Takashi Iwai wrote:
At Sun, 19 Apr 2015 19:26:58 +0200, 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?
Are you sure that the commit is the culprit? There are lots of other changes like the widget-based power saving.
FYI, the widget power-saving can be disabled by the following. (It can be done by hints, too.)
If this fixes the problem, we likely need more tweaks at path power activation. Or, as a last resort, put this to the fixup for your model.
Unfortunately the change here below has no effect on my machine. patch_alc662() is the right function.
Takashi
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 231d0e4b9a95..637ff2b70f6e 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -5584,7 +5584,7 @@ static int patch_alc269(struct hda_codec *codec)
spec = codec->spec; spec->gen.shared_mic_vref_pin = 0x18;
- codec->power_save_node = 1;
/* codec->power_save_node = 1; */
snd_hda_pick_fixup(codec, alc269_fixup_models, alc269_fixup_tbl, alc269_fixups);
On Monday 20 April 2015 07:58:15 Takashi Iwai wrote:
At Sun, 19 Apr 2015 19:26:58 +0200, 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?
Are you sure that the commit is the culprit? There are lots of other changes like the widget-based power saving.
Quite sure. I did a bisection to get it. 5e56bcea5017 ("ALSA: hda - Allow driver to add vendor-specific verbs for regmap") is OK. Also, if I revert a551d91473 in the current linux-next tree, the startup noise goes away.
In anyway, please give alsa-info.sh output.
http://www.alsa-project.org/db/?f=7d283ebf64ca750482ec5ed6174a1b860fa70631
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?
Are you sure that the commit is the culprit? There are lots of other changes like the widget-based power saving.
Quite sure. I did a bisection to get it. 5e56bcea5017 ("ALSA: hda - Allow driver to add vendor-specific verbs for regmap") is OK. Also, if I revert a551d91473 in the current linux-next tree, the startup noise goes away.
In anyway, please give alsa-info.sh output.
http://www.alsa-project.org/db/?f=7d283ebf64ca750482ec5ed6174a1b860fa70631
Did you use headset or headphone since the capture source control is used to determine the function of multi-role combo jack when the codec cannot dinstingush headset, headphone and mic jack ?
2015-04-21 4:13 GMT+02:00 Raymond Yau superquad.vortex2@gmail.com:
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?
Are you sure that the commit is the culprit? There are lots of other changes like the widget-based power saving.
Quite sure. I did a bisection to get it. 5e56bcea5017 ("ALSA: hda - Allow driver to add vendor-specific verbs for regmap") is OK. Also, if I revert a551d91473 in the current linux-next tree, the startup noise goes away.
In anyway, please give alsa-info.sh output.
http://www.alsa-project.org/db/?f=7d283ebf64ca750482ec5ed6174a1b860fa70631
Did you use headset or headphone since the capture source control is used to determine the function of multi-role combo jack when the codec cannot dinstingush headset, headphone and mic jack ?
I've used simple headphones.
2015-04-19 19:26 GMT+02:00 Gabriele Mazzotta gabriele.mzt@gmail.com:
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?
Small update: I found that if I add a printk in alc_shutup_dell_xps13() or I increase the delay from 20ms to 200ms, I get the pop noise on start, with or without a551d91473 reverted. I will try to find out what this delay is exactly affecting.
At Tue, 21 Apr 2015 11:58:54 +0200, Gabriele Mazzotta wrote:
2015-04-19 19:26 GMT+02:00 Gabriele Mazzotta gabriele.mzt@gmail.com:
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?
Small update: I found that if I add a printk in alc_shutup_dell_xps13() or I increase the delay from 20ms to 200ms, I get the pop noise on start, with or without a551d91473 reverted. I will try to find out what this delay is exactly affecting.
Interesting. I didn't think that increasing the delay affects (not decreasing).
Maybe you'd be better to trace the verbs that are issued around the headphone jack plugging. The tracepoints are your friend. See HD-audio.txt for a brief instruction.
thanks,
Takashi
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?
--- sound/pci/hda/hda_generic.c | 3 ++- sound/pci/hda/patch_realtek.c | 1 + 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c index 3d2597b..adf84ab 100644 --- a/sound/pci/hda/hda_generic.c +++ b/sound/pci/hda/hda_generic.c @@ -3257,7 +3257,8 @@ static int create_input_ctls(struct hda_codec *codec) continue;
val = PIN_IN; - if (cfg->inputs[i].type == AUTO_PIN_MIC) + if (cfg->inputs[i].type == AUTO_PIN_MIC && + !spec->suppress_hp_mic_detect) val |= snd_hda_get_default_vref(codec, pin); if (pin != spec->hp_mic_pin) set_pin_target(codec, pin, val, false); diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 231d0e4..3d854e7 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -4062,6 +4062,7 @@ static void alc_fixup_headset_mode(struct hda_codec *codec,
switch (action) { case HDA_FIXUP_ACT_PRE_PROBE: + spec->gen.suppress_hp_mic_detect = 1; spec->parse_flags |= HDA_PINCFG_HEADSET_MIC | HDA_PINCFG_HEADPHONE_MIC; break; case HDA_FIXUP_ACT_PROBE:
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
2015-04-24 8:13 GMT+02:00 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
The patch did no harm, but didn't solve the problem.
Gabriele
At Fri, 24 Apr 2015 17:14:24 +0200, Gabriele Mazzotta wrote:
2015-04-24 8:13 GMT+02:00 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
The patch did no harm, but didn't solve the problem.
OK, so the problem doesn't seem relevant with the runtime PM, which was the usual suspect.
My patch should reduce the actual verb writes, so it would be nice to have even if it doesn't fix your problem. But maybe I'll postpone it as a 4.2 material.
thanks,
Takashi
On Friday 24 April 2015 17:34:57 Takashi Iwai wrote:
At Fri, 24 Apr 2015 17:14:24 +0200, Gabriele Mazzotta wrote:
2015-04-24 8:13 GMT+02:00 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
The patch did no harm, but didn't solve the problem.
OK, so the problem doesn't seem relevant with the runtime PM, which was the usual suspect.
My patch should reduce the actual verb writes, so it would be nice to have even if it doesn't fix your problem. But maybe I'll postpone it as a 4.2 material.
thanks,
Takashi
Hi,
I think this the patch here below is simpler than the other I sent. snd_hda_get_default_vref() guesses the vref input pins. We know that some of these are for headphones/headset mics, so we could set the vref to HIZ for these.
Are there systems that would misbehave with this change?
Gabriele
--- diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c index 3d2597b..081db8b 100644 --- a/sound/pci/hda/hda_generic.c +++ b/sound/pci/hda/hda_generic.c @@ -3257,7 +3257,9 @@ static int create_input_ctls(struct hda_codec *codec) continue;
val = PIN_IN; - if (cfg->inputs[i].type == AUTO_PIN_MIC) + if (cfg->inputs[i].type == AUTO_PIN_MIC && + !cfg->inputs[i].is_headset_mic && + !cfg->inputs[i].is_headphone_mic) val |= snd_hda_get_default_vref(codec, pin); if (pin != spec->hp_mic_pin) set_pin_target(codec, pin, val, false);
On Saturday 25 April 2015 13:51:33 Gabriele Mazzotta wrote:
Hi,
I think this the patch here below is simpler than the other I sent. snd_hda_get_default_vref() guesses the vref input pins. We know that some of these are for headphones/headset mics, so we could set the vref to HIZ for these.
Are there systems that would misbehave with this change?
Gabriele
diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c index 3d2597b..081db8b 100644 --- a/sound/pci/hda/hda_generic.c +++ b/sound/pci/hda/hda_generic.c @@ -3257,7 +3257,9 @@ static int create_input_ctls(struct hda_codec *codec) continue;
val = PIN_IN;
if (cfg->inputs[i].type == AUTO_PIN_MIC)
if (cfg->inputs[i].type == AUTO_PIN_MIC &&!cfg->inputs[i].is_headset_mic &&!cfg->inputs[i].is_headphone_mic) val |= snd_hda_get_default_vref(codec, pin);
Sorry, I think I misunderstood the meaning of the two flags and they can't be used like that.
At Sat, 25 Apr 2015 15:57:15 +0200, Gabriele Mazzotta wrote:
On Saturday 25 April 2015 13:51:33 Gabriele Mazzotta wrote:
Hi,
I think this the patch here below is simpler than the other I sent. snd_hda_get_default_vref() guesses the vref input pins. We know that some of these are for headphones/headset mics, so we could set the vref to HIZ for these.
Are there systems that would misbehave with this change?
Gabriele
diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c index 3d2597b..081db8b 100644 --- a/sound/pci/hda/hda_generic.c +++ b/sound/pci/hda/hda_generic.c @@ -3257,7 +3257,9 @@ static int create_input_ctls(struct hda_codec *codec) continue;
val = PIN_IN;
if (cfg->inputs[i].type == AUTO_PIN_MIC)
if (cfg->inputs[i].type == AUTO_PIN_MIC &&!cfg->inputs[i].is_headset_mic &&!cfg->inputs[i].is_headphone_mic) val |= snd_hda_get_default_vref(codec, pin);Sorry, I think I misunderstood the meaning of the two flags and they can't be used like that.
Yeah, it's a slight abuse.
How about a patch like below? This doesn't add any flag but it sets the default pin target value explicitly by the driver beforehand, and let the generic parser setting up only when uninitialized.
Takashi
--- diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c index 3d2597b7037b..788f969b1a68 100644 --- a/sound/pci/hda/hda_generic.c +++ b/sound/pci/hda/hda_generic.c @@ -3259,7 +3259,8 @@ static int create_input_ctls(struct hda_codec *codec) val = PIN_IN; if (cfg->inputs[i].type == AUTO_PIN_MIC) val |= snd_hda_get_default_vref(codec, pin); - if (pin != spec->hp_mic_pin) + if (pin != spec->hp_mic_pin && + !snd_hda_codec_get_pin_target(codec, pin)) set_pin_target(codec, pin, val, false);
if (mixer) { diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 06199e4e930f..bd661ca62092 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -4190,11 +4190,15 @@ static void alc_shutup_dell_xps13(struct hda_codec *codec) static void alc_fixup_dell_xps13(struct hda_codec *codec, const struct hda_fixup *fix, int action) { - if (action == HDA_FIXUP_ACT_PROBE) { - struct alc_spec *spec = codec->spec; - struct hda_input_mux *imux = &spec->gen.input_mux; - int i; + struct alc_spec *spec = codec->spec; + struct hda_input_mux *imux = &spec->gen.input_mux; + int i;
+ switch (action) { + case HDA_FIXUP_ACT_PRE_PROBE: + snd_hda_codec_set_pin_target(codec, 0x19, PIN_VREFHIZ); + break; + case HDA_FIXUP_ACT_PROBE: spec->shutup = alc_shutup_dell_xps13;
/* Make the internal mic the default input source. */ @@ -4204,6 +4208,7 @@ static void alc_fixup_dell_xps13(struct hda_codec *codec, break; } } + break; } }
On Sunday 26 April 2015 18:26:59 Takashi Iwai wrote:
At Sat, 25 Apr 2015 15:57:15 +0200, Gabriele Mazzotta wrote:
On Saturday 25 April 2015 13:51:33 Gabriele Mazzotta wrote:
Hi,
I think this the patch here below is simpler than the other I sent. snd_hda_get_default_vref() guesses the vref input pins. We know that some of these are for headphones/headset mics, so we could set the vref to HIZ for these.
Are there systems that would misbehave with this change?
Gabriele
diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c index 3d2597b..081db8b 100644 --- a/sound/pci/hda/hda_generic.c +++ b/sound/pci/hda/hda_generic.c @@ -3257,7 +3257,9 @@ static int create_input_ctls(struct hda_codec *codec) continue;
val = PIN_IN;
if (cfg->inputs[i].type == AUTO_PIN_MIC)
if (cfg->inputs[i].type == AUTO_PIN_MIC &&!cfg->inputs[i].is_headset_mic &&!cfg->inputs[i].is_headphone_mic) val |= snd_hda_get_default_vref(codec, pin);Sorry, I think I misunderstood the meaning of the two flags and they can't be used like that.
Yeah, it's a slight abuse.
How about a patch like below? This doesn't add any flag but it sets the default pin target value explicitly by the driver beforehand, and let the generic parser setting up only when uninitialized.
It's working perfectly, thanks.
Gabriele
Takashi
diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c index 3d2597b7037b..788f969b1a68 100644 --- a/sound/pci/hda/hda_generic.c +++ b/sound/pci/hda/hda_generic.c @@ -3259,7 +3259,8 @@ static int create_input_ctls(struct hda_codec *codec) val = PIN_IN; if (cfg->inputs[i].type == AUTO_PIN_MIC) val |= snd_hda_get_default_vref(codec, pin);
if (pin != spec->hp_mic_pin)
if (pin != spec->hp_mic_pin &&!snd_hda_codec_get_pin_target(codec, pin)) set_pin_target(codec, pin, val, false);if (mixer) {
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 06199e4e930f..bd661ca62092 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -4190,11 +4190,15 @@ static void alc_shutup_dell_xps13(struct hda_codec *codec) static void alc_fixup_dell_xps13(struct hda_codec *codec, const struct hda_fixup *fix, int action) {
- if (action == HDA_FIXUP_ACT_PROBE) {
struct alc_spec *spec = codec->spec;struct hda_input_mux *imux = &spec->gen.input_mux;int i;
struct alc_spec *spec = codec->spec;
struct hda_input_mux *imux = &spec->gen.input_mux;
int i;
switch (action) {
case HDA_FIXUP_ACT_PRE_PROBE:
snd_hda_codec_set_pin_target(codec, 0x19, PIN_VREFHIZ);break;case HDA_FIXUP_ACT_PROBE: spec->shutup = alc_shutup_dell_xps13;
/* Make the internal mic the default input source. */
@@ -4204,6 +4208,7 @@ static void alc_fixup_dell_xps13(struct hda_codec *codec, break; } }
break;}
At Sun, 26 Apr 2015 19:05:18 +0200, Gabriele Mazzotta wrote:
On Sunday 26 April 2015 18:26:59 Takashi Iwai wrote:
At Sat, 25 Apr 2015 15:57:15 +0200, Gabriele Mazzotta wrote:
On Saturday 25 April 2015 13:51:33 Gabriele Mazzotta wrote:
Hi,
I think this the patch here below is simpler than the other I sent. snd_hda_get_default_vref() guesses the vref input pins. We know that some of these are for headphones/headset mics, so we could set the vref to HIZ for these.
Are there systems that would misbehave with this change?
Gabriele
diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c index 3d2597b..081db8b 100644 --- a/sound/pci/hda/hda_generic.c +++ b/sound/pci/hda/hda_generic.c @@ -3257,7 +3257,9 @@ static int create_input_ctls(struct hda_codec *codec) continue;
val = PIN_IN;
if (cfg->inputs[i].type == AUTO_PIN_MIC)
if (cfg->inputs[i].type == AUTO_PIN_MIC &&!cfg->inputs[i].is_headset_mic &&!cfg->inputs[i].is_headphone_mic) val |= snd_hda_get_default_vref(codec, pin);Sorry, I think I misunderstood the meaning of the two flags and they can't be used like that.
Yeah, it's a slight abuse.
How about a patch like below? This doesn't add any flag but it sets the default pin target value explicitly by the driver beforehand, and let the generic parser setting up only when uninitialized.
It's working perfectly, thanks.
Gabriele
Good to hear. I queued the patch below now.
thanks,
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Fix click noise at start on Dell XPS13
Dell XPS13 produces a click noise at boot up, and Gabriele spotted out that it's triggered by the initial pin control of the mic (NID 0x19). This has to be set to Hi-Z Vref while the driver initializes to Vref 80% as a normal mic.
This patch fixes the generic parser code not to override the target vref if it has been already set by the driver, and adds a proper initialization of the target vref for this pin in the Realtek driver side.
Reported-and-tested-by: Gabriele Mazzotta gabriele.mzt@gmail.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_generic.c | 3 ++- sound/pci/hda/patch_realtek.c | 16 ++++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c index 3d2597b7037b..788f969b1a68 100644 --- a/sound/pci/hda/hda_generic.c +++ b/sound/pci/hda/hda_generic.c @@ -3259,7 +3259,8 @@ static int create_input_ctls(struct hda_codec *codec) val = PIN_IN; if (cfg->inputs[i].type == AUTO_PIN_MIC) val |= snd_hda_get_default_vref(codec, pin); - if (pin != spec->hp_mic_pin) + if (pin != spec->hp_mic_pin && + !snd_hda_codec_get_pin_target(codec, pin)) set_pin_target(codec, pin, val, false);
if (mixer) { diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 06199e4e930f..e2afd53cc14c 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -4190,11 +4190,18 @@ static void alc_shutup_dell_xps13(struct hda_codec *codec) static void alc_fixup_dell_xps13(struct hda_codec *codec, const struct hda_fixup *fix, int action) { - if (action == HDA_FIXUP_ACT_PROBE) { - struct alc_spec *spec = codec->spec; - struct hda_input_mux *imux = &spec->gen.input_mux; - int i; + struct alc_spec *spec = codec->spec; + struct hda_input_mux *imux = &spec->gen.input_mux; + int i;
+ switch (action) { + case HDA_FIXUP_ACT_PRE_PROBE: + /* mic pin 0x19 must be initialized with Vref Hi-Z, otherwise + * it causes a click noise at start up + */ + snd_hda_codec_set_pin_target(codec, 0x19, PIN_VREFHIZ); + break; + case HDA_FIXUP_ACT_PROBE: spec->shutup = alc_shutup_dell_xps13;
/* Make the internal mic the default input source. */ @@ -4204,6 +4211,7 @@ static void alc_fixup_dell_xps13(struct hda_codec *codec, break; } } + break; } }
participants (3)
-
Gabriele Mazzotta -
Raymond Yau -
Takashi Iwai