From patchwork Thu Feb 22 11:08:08 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Takashi Iwai X-Patchwork-Id: 10235141 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 2093760224 for ; Thu, 22 Feb 2018 11:08:18 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 24EDA28976 for ; Thu, 22 Feb 2018 11:08:18 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 193FD288E1; Thu, 22 Feb 2018 11:08:18 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 21BC6288E1 for ; Thu, 22 Feb 2018 11:08:16 +0000 (UTC) Received: from alsa0.perex.cz (localhost [127.0.0.1]) by alsa0.perex.cz (Postfix) with ESMTP id E64E42676BB; Thu, 22 Feb 2018 12:08:14 +0100 (CET) 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 5F26B2676BC; Thu, 22 Feb 2018 12:08:13 +0100 (CET) Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id CAB5F26768E for ; Thu, 22 Feb 2018 12:08:09 +0100 (CET) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id E7C4CADFD; Thu, 22 Feb 2018 11:08:08 +0000 (UTC) Date: Thu, 22 Feb 2018 12:08:08 +0100 Message-ID: From: Takashi Iwai To: "Hans de Goede" In-Reply-To: <20180222105352.5408-1-hdegoede@redhat.com> References: <20180222105352.5408-1-hdegoede@redhat.com> 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/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Cc: alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [RFC 0/1] ALSA: hda: Add a power_save blacklist 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 On Thu, 22 Feb 2018 11:53:51 +0100, Hans de Goede wrote: > > Hi All, > > On some boards setting power_save to a non 0 value leads to clicking / > popping sounds when ever we enter/leave powersaving mode. Ideally we would > figure out how to avoid these sounds, but that is not always feasible. > > This commit adds a blacklist for devices where powersaving is known to > cause problems and disables it on these devices. > > Note this patch ATM is a RFC because I still need to get test-feedback > that it actually works on affected boards. > > I tried to put this blacklist in userspace first: > https://github.com/systemd/systemd/pull/8128 > > But the systemd maintainers rightfully pointed out that it would be > impossible to then later remove entries once we actually find a way to > make power-saving work on listed boards without issues. Having this list > in the kernel will allow removal of the blacklist entry in the same commit > which fixes the clicks / plops. > > I've chosen to also apply the blacklist to changes made through > /sys/module/snd_hda_intel/parameters/power_save, since tools such as > powertop and TLP do this automatically. > > About the choice to also apply this to changes made through > /sys/module/snd_hda_intel/parameters/power_save, one could argue that this > will get in the way of users who want the power-saving anyways and are > willing to live with the clicks/plops. An alternative approach would be to > only apply this to the value of the module-parameter at boot and then only > when it matches CONFIG_SND_HDA_POWER_SAVE_DEFAULT. I'm open to changing this. IMO, we shouldn't prevent user to set the explicit power_save mode even if it's blacklisted. That is, if any, I prefer blacklist influencing only on the default value, a patch like below. But, before adding to the blacklist, it's better to investigate the cause. For example, Lenovo X1 is definitely a thing to be looked at. Often a simple turn-off of "Loopback Mixing" element is enough. We need alsa-info.sh output for further investigation for each case, in anyway. thanks, Takashi === diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index c71dcacea807..f21bf0add9f0 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -181,7 +181,7 @@ static const struct kernel_param_ops param_ops_xint = { }; #define param_check_xint param_check_int -static int power_save = CONFIG_SND_HDA_POWER_SAVE_DEFAULT; +static int power_save = -1; /* default */ module_param(power_save, xint, 0644); MODULE_PARM_DESC(power_save, "Automatic power-saving timeout " "(in second, 0 = disable)."); @@ -2198,6 +2198,7 @@ static int azx_probe_continue(struct azx *chip) struct hdac_bus *bus = azx_bus(chip); struct pci_dev *pci = chip->pci; int dev = chip->dev_index; + int val; int err; hda->probe_continued = 1; @@ -2278,7 +2279,11 @@ static int azx_probe_continue(struct azx *chip) chip->running = 1; azx_add_card_list(chip); - snd_hda_set_power_save(&chip->bus, power_save * 1000); + val = power_save; + if (val < 0 && !is_power_save_blacklisted(chip)) + val = CONFIG_SND_HDA_POWER_SAVE_DEFAULT; + if (val >= 0) + snd_hda_set_power_save(&chip->bus, val * 1000); if (azx_has_pm_runtime(chip) || hda->use_vga_switcheroo) pm_runtime_put_autosuspend(&pci->dev);