From patchwork Tue Nov 17 16:54:28 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kai-Heng Feng X-Patchwork-Id: 1401660 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=canonical.com Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4CbBrB6YcCz9sPB; Wed, 18 Nov 2020 03:54:50 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1kf4Fj-00085Q-64; Tue, 17 Nov 2020 16:54:47 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1kf4Fh-00084f-Sz for kernel-team@lists.ubuntu.com; Tue, 17 Nov 2020 16:54:45 +0000 Received: from 61-220-137-34.hinet-ip.hinet.net ([61.220.137.34] helo=localhost) by youngberry.canonical.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1kf4Fh-0007j3-1J for kernel-team@lists.ubuntu.com; Tue, 17 Nov 2020 16:54:45 +0000 From: Kai-Heng Feng To: kernel-team@lists.ubuntu.com Subject: [G/OEM-5.6] [PATCH 3/4] ALSA: hda: Separate runtime and system suspend Date: Wed, 18 Nov 2020 00:54:28 +0800 Message-Id: <20201117165429.1231318-4-kai.heng.feng@canonical.com> X-Mailer: git-send-email 2.28.0 In-Reply-To: <20201117165429.1231318-1-kai.heng.feng@canonical.com> References: <20201117165429.1231318-1-kai.heng.feng@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" BugLink: https://bugs.launchpad.net/bugs/1904595 Both pm_runtime_force_suspend() and pm_runtime_force_resume() have some implicit checks, so it can make code flow more straightforward if we separate runtime and system suspend callbacks. High Definition Audio Specification, 4.5.9.3 Codec Wake From System S3 states that codec can wake the system up from S3 if WAKEEN is toggled. Since HDA controller has different wakeup settings for runtime and system susend, we also need to explicitly disable direct-complete which can be enabled automatically by PCI core. In addition to that, avoid waking up codec if runtime resume is for system suspend, to not break direct-complete for codecs. While at it, also remove AZX_DCAPS_SUSPEND_SPURIOUS_WAKEUP, as the original bug commit a6630529aecb ("ALSA: hda: Workaround for spurious wakeups on some Intel platforms") solves doesn't happen with this patch. Signed-off-by: Kai-Heng Feng Link: https://lore.kernel.org/r/20201027130038.16463-3-kai.heng.feng@canonical.com Signed-off-by: Takashi Iwai (cherry picked from commit f5dac54d9d93826a776dffc848df76746f7135bb) Signed-off-by: Kai-Heng Feng --- sound/pci/hda/hda_controller.h | 3 +- sound/pci/hda/hda_intel.c | 62 +++++++++++++++++++--------------- 2 files changed, 36 insertions(+), 29 deletions(-) diff --git a/sound/pci/hda/hda_controller.h b/sound/pci/hda/hda_controller.h index a356fb0e5773..9da7a06d024f 100644 --- a/sound/pci/hda/hda_controller.h +++ b/sound/pci/hda/hda_controller.h @@ -41,7 +41,7 @@ /* 24 unused */ #define AZX_DCAPS_COUNT_LPIB_DELAY (1 << 25) /* Take LPIB as delay */ #define AZX_DCAPS_PM_RUNTIME (1 << 26) /* runtime PM support */ -#define AZX_DCAPS_SUSPEND_SPURIOUS_WAKEUP (1 << 27) /* Workaround for spurious wakeups after suspend */ +/* 27 unused */ #define AZX_DCAPS_CORBRP_SELF_CLEAR (1 << 28) /* CORBRP clears itself after reset */ #define AZX_DCAPS_NO_MSI64 (1 << 29) /* Stick to 32-bit MSIs */ #define AZX_DCAPS_SEPARATE_STREAM_TAG (1 << 30) /* capture and playback use separate stream tag */ @@ -143,6 +143,7 @@ struct azx { unsigned int align_buffer_size:1; unsigned int region_requested:1; unsigned int disabled:1; /* disabled by vga_switcheroo */ + unsigned int pm_prepared:1; /* GTS present */ unsigned int gts_present:1; diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index ca62cc2c9f67..031b8ba82fae 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -298,8 +298,7 @@ enum { /* PCH for HSW/BDW; with runtime PM */ /* no i915 binding for this as HSW/BDW has another controller for HDMI */ #define AZX_DCAPS_INTEL_PCH \ - (AZX_DCAPS_INTEL_PCH_BASE | AZX_DCAPS_PM_RUNTIME |\ - AZX_DCAPS_SUSPEND_SPURIOUS_WAKEUP) + (AZX_DCAPS_INTEL_PCH_BASE | AZX_DCAPS_PM_RUNTIME) /* HSW HDMI */ #define AZX_DCAPS_INTEL_HASWELL \ @@ -986,7 +985,7 @@ static void __azx_runtime_suspend(struct azx *chip) display_power(chip, false); } -static void __azx_runtime_resume(struct azx *chip, bool from_rt) +static void __azx_runtime_resume(struct azx *chip) { struct hda_intel *hda = container_of(chip, struct hda_intel, chip); struct hdac_bus *bus = azx_bus(chip); @@ -1003,7 +1002,8 @@ static void __azx_runtime_resume(struct azx *chip, bool from_rt) azx_init_pci(chip); hda_intel_init_chip(chip, true); - if (from_rt) { + /* Avoid codec resume if runtime resume is for system suspend */ + if (!chip->pm_prepared) { list_for_each_codec(codec, &chip->bus) { if (codec->relaxed_resume) continue; @@ -1019,6 +1019,29 @@ static void __azx_runtime_resume(struct azx *chip, bool from_rt) } #ifdef CONFIG_PM_SLEEP +static int azx_prepare(struct device *dev) +{ + struct snd_card *card = dev_get_drvdata(dev); + struct azx *chip; + + chip = card->private_data; + chip->pm_prepared = 1; + + /* HDA controller always requires different WAKEEN for runtime suspend + * and system suspend, so don't use direct-complete here. + */ + return 0; +} + +static void azx_complete(struct device *dev) +{ + struct snd_card *card = dev_get_drvdata(dev); + struct azx *chip; + + chip = card->private_data; + chip->pm_prepared = 0; +} + static int azx_suspend(struct device *dev) { struct snd_card *card = dev_get_drvdata(dev); @@ -1030,15 +1053,7 @@ static int azx_suspend(struct device *dev) chip = card->private_data; bus = azx_bus(chip); - snd_power_change_state(card, SNDRV_CTL_POWER_D3hot); - /* An ugly workaround: direct call of __azx_runtime_suspend() and - * __azx_runtime_resume() for old Intel platforms that suffer from - * spurious wakeups after S3 suspend - */ - if (chip->driver_caps & AZX_DCAPS_SUSPEND_SPURIOUS_WAKEUP) - __azx_runtime_suspend(chip); - else - pm_runtime_force_suspend(dev); + __azx_runtime_suspend(chip); if (bus->irq >= 0) { free_irq(bus->irq, chip); bus->irq = -1; @@ -1067,11 +1082,7 @@ static int azx_resume(struct device *dev) if (azx_acquire_irq(chip, 1) < 0) return -EIO; - if (chip->driver_caps & AZX_DCAPS_SUSPEND_SPURIOUS_WAKEUP) - __azx_runtime_resume(chip, false); - else - pm_runtime_force_resume(dev); - snd_power_change_state(card, SNDRV_CTL_POWER_D0); + __azx_runtime_resume(chip); trace_azx_resume(chip); return 0; @@ -1119,10 +1130,7 @@ static int azx_runtime_suspend(struct device *dev) chip = card->private_data; /* enable controller wake up event */ - if (snd_power_get_state(card) == SNDRV_CTL_POWER_D0) { - azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) | - STATESTS_INT_MASK); - } + azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) | STATESTS_INT_MASK); __azx_runtime_suspend(chip); trace_azx_runtime_suspend(chip); @@ -1133,18 +1141,14 @@ static int azx_runtime_resume(struct device *dev) { struct snd_card *card = dev_get_drvdata(dev); struct azx *chip; - bool from_rt = snd_power_get_state(card) == SNDRV_CTL_POWER_D0; if (!azx_is_pm_ready(card)) return 0; chip = card->private_data; - __azx_runtime_resume(chip, from_rt); + __azx_runtime_resume(chip); /* disable controller Wake Up event*/ - if (from_rt) { - azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) & - ~STATESTS_INT_MASK); - } + azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) & ~STATESTS_INT_MASK); trace_azx_runtime_resume(chip); return 0; @@ -1178,6 +1182,8 @@ static int azx_runtime_idle(struct device *dev) static const struct dev_pm_ops azx_pm = { SET_SYSTEM_SLEEP_PM_OPS(azx_suspend, azx_resume) #ifdef CONFIG_PM_SLEEP + .prepare = azx_prepare, + .complete = azx_complete, .freeze_noirq = azx_freeze_noirq, .thaw_noirq = azx_thaw_noirq, #endif