From patchwork Fri Apr 24 06:13:58 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Takashi Iwai X-Patchwork-Id: 6266481 Return-Path: X-Original-To: patchwork-alsa-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id C3C13BF4A6 for ; Fri, 24 Apr 2015 06:14:19 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 9549020263 for ; Fri, 24 Apr 2015 06:14:18 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.kernel.org (Postfix) with ESMTP id EC75E20221 for ; Fri, 24 Apr 2015 06:14:16 +0000 (UTC) Received: by alsa0.perex.cz (Postfix, from userid 1000) id 9CF682604F3; Fri, 24 Apr 2015 08:14:15 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 Received: from alsa0.perex.cz (localhost [IPv6:::1]) by alsa0.perex.cz (Postfix) with ESMTP id 3A10826047A; Fri, 24 Apr 2015 08:14:08 +0200 (CEST) X-Original-To: alsa-devel@alsa-project.org Delivered-To: alsa-devel@alsa-project.org Received: by alsa0.perex.cz (Postfix, from userid 1000) id 5C3F92604AA; Fri, 24 Apr 2015 08:14:06 +0200 (CEST) Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 14496260455 for ; Fri, 24 Apr 2015 08:13:59 +0200 (CEST) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 93513ACBC; Fri, 24 Apr 2015 06:13:58 +0000 (UTC) Date: Fri, 24 Apr 2015 08:13:58 +0200 Message-ID: From: Takashi Iwai To: Gabriele Mazzotta In-Reply-To: <1799826.AYplPzq8AK@xps13> References: <23057263.IjMh4t6SYF@xps13> <1799826.AYplPzq8AK@xps13> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.4 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Cc: superquad.vortex2@gmail.com, alsa-devel@alsa-project.org Subject: Re: [alsa-devel] Pop noise on startup when headphones are plugged in (Dell XPS13 9333) X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org X-Virus-Scanned: ClamAV using ClamSMTP 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