| Message ID | s5hh9s6kq8p.wl-tiwai@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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