From patchwork Thu Aug 8 21:55:07 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rafael J. Wysocki" X-Patchwork-Id: 11084879 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 2580A1709 for ; Thu, 8 Aug 2019 21:59:08 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1175B28826 for ; Thu, 8 Aug 2019 21:59:08 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 059AF28BCF; Thu, 8 Aug 2019 21:59:08 +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=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A235228826 for ; Thu, 8 Aug 2019 21:59:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390296AbfHHV64 (ORCPT ); Thu, 8 Aug 2019 17:58:56 -0400 Received: from cloudserver094114.home.pl ([79.96.170.134]:52972 "EHLO cloudserver094114.home.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725785AbfHHV64 (ORCPT ); Thu, 8 Aug 2019 17:58:56 -0400 Received: from 79.184.254.29.ipv4.supernova.orange.pl (79.184.254.29) (HELO kreacher.localnet) by serwer1319399.home.pl (79.96.170.134) with SMTP (IdeaSmtpServer 0.83.275) id 2d19d8ad48c4b56a; Thu, 8 Aug 2019 23:58:53 +0200 From: "Rafael J. Wysocki" To: linux-nvme Cc: Keith Busch , Mario Limonciello , Kai-Heng Feng , Keith Busch , Christoph Hellwig , Sagi Grimberg , Linux PM , Linux Kernel Mailing List , Rajat Jain , Linux PCI , Bjorn Helgaas Subject: [PATCH v3 1/2] PCI: PCIe: ASPM: Introduce pcie_aspm_enabled() Date: Thu, 08 Aug 2019 23:55:07 +0200 Message-ID: <1618955.HVa0YQSOW5@kreacher> In-Reply-To: <2184247.yL3mcj2FRQ@kreacher> References: <4323ed84dd07474eab65699b4d007aaf@AUSX13MPC105.AMER.DELL.COM> <20190731221956.GB15795@localhost.localdomain> <2184247.yL3mcj2FRQ@kreacher> MIME-Version: 1.0 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Rafael J. Wysocki Add a function checking whether or not PCIe ASPM has been enabled for a given device. It will be used by the NVMe driver to decide how to handle the device during system suspend. Signed-off-by: Rafael J. Wysocki Acked-by: Bjorn Helgaas --- v2 -> v3: * Make the new function return bool. * Change its name back to pcie_aspm_enabled(). * Fix kerneldoc comment formatting. -> v2: * Move the PCI/PCIe ASPM changes to a separate patch. * Add the _mask suffix to the new function name. * Add EXPORT_SYMBOL_GPL() to the new function. * Avoid adding an unnecessary blank line. --- drivers/pci/pcie/aspm.c | 20 ++++++++++++++++++++ include/linux/pci.h | 3 +++ 2 files changed, 23 insertions(+) Index: linux-pm/drivers/pci/pcie/aspm.c =================================================================== --- linux-pm.orig/drivers/pci/pcie/aspm.c +++ linux-pm/drivers/pci/pcie/aspm.c @@ -1170,6 +1170,26 @@ static int pcie_aspm_get_policy(char *bu module_param_call(policy, pcie_aspm_set_policy, pcie_aspm_get_policy, NULL, 0644); +/** + * pcie_aspm_enabled - Check if PCIe ASPM has been enabled for a device. + * @pci_device: Target device. + */ +bool pcie_aspm_enabled(struct pci_dev *pci_device) +{ + struct pci_dev *bridge = pci_upstream_bridge(pci_device); + bool ret; + + if (!bridge) + return false; + + mutex_lock(&aspm_lock); + ret = bridge->link_state ? !!bridge->link_state->aspm_enabled : false; + mutex_unlock(&aspm_lock); + + return ret; +} +EXPORT_SYMBOL_GPL(pcie_aspm_enabled); + #ifdef CONFIG_PCIEASPM_DEBUG static ssize_t link_state_show(struct device *dev, struct device_attribute *attr, Index: linux-pm/include/linux/pci.h =================================================================== --- linux-pm.orig/include/linux/pci.h +++ linux-pm/include/linux/pci.h @@ -1567,8 +1567,11 @@ extern bool pcie_ports_native; #ifdef CONFIG_PCIEASPM bool pcie_aspm_support_enabled(void); +bool pcie_aspm_enabled(struct pci_dev *pci_device); #else static inline bool pcie_aspm_support_enabled(void) { return false; } +static inline bool pcie_aspm_enabled(struct pci_dev *pci_device) +{ return false; } #endif #ifdef CONFIG_PCIEAER From patchwork Thu Aug 8 21:58:38 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rafael J. Wysocki" X-Patchwork-Id: 11084875 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A346E14DB for ; Thu, 8 Aug 2019 21:58:57 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8CF8528826 for ; Thu, 8 Aug 2019 21:58:57 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7E0EB28BCF; Thu, 8 Aug 2019 21:58:57 +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=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B32F528826 for ; Thu, 8 Aug 2019 21:58:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1733140AbfHHV64 (ORCPT ); Thu, 8 Aug 2019 17:58:56 -0400 Received: from cloudserver094114.home.pl ([79.96.170.134]:50579 "EHLO cloudserver094114.home.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725535AbfHHV64 (ORCPT ); Thu, 8 Aug 2019 17:58:56 -0400 Received: from 79.184.254.29.ipv4.supernova.orange.pl (79.184.254.29) (HELO kreacher.localnet) by serwer1319399.home.pl (79.96.170.134) with SMTP (IdeaSmtpServer 0.83.275) id 198204becd846193; Thu, 8 Aug 2019 23:58:52 +0200 From: "Rafael J. Wysocki" To: linux-nvme Cc: Keith Busch , Mario Limonciello , Kai-Heng Feng , Keith Busch , Christoph Hellwig , Sagi Grimberg , Linux PM , Linux Kernel Mailing List , Rajat Jain , Linux PCI , Bjorn Helgaas Subject: [PATCH v3 2/2] nvme-pci: Allow PCI bus-level PM to be used if ASPM is disabled Date: Thu, 08 Aug 2019 23:58:38 +0200 Message-ID: <4856352.pThfM6cRGE@kreacher> In-Reply-To: <2184247.yL3mcj2FRQ@kreacher> References: <4323ed84dd07474eab65699b4d007aaf@AUSX13MPC105.AMER.DELL.COM> <20190731221956.GB15795@localhost.localdomain> <2184247.yL3mcj2FRQ@kreacher> MIME-Version: 1.0 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Rafael J. Wysocki One of the modifications made by commit d916b1be94b6 ("nvme-pci: use host managed power state for suspend") was adding a pci_save_state() call to nvme_suspend() so as to instruct the PCI bus type to leave devices handled by the nvme driver in D0 during suspend-to-idle. That was done with the assumption that ASPM would transition the device's PCIe link into a low-power state when the device became inactive. However, if ASPM is disabled for the device, its PCIe link will stay in L0 and in that case commit d916b1be94b6 is likely to cause the energy used by the system while suspended to increase. Namely, if the device in question works in accordance with the PCIe specification, putting it into D3hot causes its PCIe link to go to L1 or L2/L3 Ready, which is lower-power than L0. Since the energy used by the system while suspended depends on the state of its PCIe link (as a general rule, the lower-power the state of the link, the less energy the system will use), putting the device into D3hot during suspend-to-idle should be more energy-efficient that leaving it in D0 with disabled ASPM. For this reason, avoid leaving NVMe devices with disabled ASPM in D0 during suspend-to-idle. Instead, shut them down entirely and let the PCI bus type put them into D3. Fixes: d916b1be94b6 ("nvme-pci: use host managed power state for suspend") Link: https://lore.kernel.org/linux-pm/2763495.NmdaWeg79L@kreacher/T/#t Signed-off-by: Rafael J. Wysocki --- v2 -> v3: * Modify the changelog to describe the rationale for this patch in a less confusing and more convincing way. -> v2: * Move the PCI/PCIe ASPM changes to a separate patch. * Do not add a redundant ndev->last_ps == U32_MAX check in nvme_suspend(). --- drivers/nvme/host/pci.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) Index: linux-pm/drivers/nvme/host/pci.c =================================================================== --- linux-pm.orig/drivers/nvme/host/pci.c +++ linux-pm/drivers/nvme/host/pci.c @@ -2846,7 +2846,7 @@ static int nvme_resume(struct device *de struct nvme_dev *ndev = pci_get_drvdata(to_pci_dev(dev)); struct nvme_ctrl *ctrl = &ndev->ctrl; - if (pm_resume_via_firmware() || !ctrl->npss || + if (ndev->last_ps == U32_MAX || nvme_set_power_state(ctrl, ndev->last_ps) != 0) nvme_reset_ctrl(ctrl); return 0; @@ -2859,6 +2859,8 @@ static int nvme_suspend(struct device *d struct nvme_ctrl *ctrl = &ndev->ctrl; int ret = -EBUSY; + ndev->last_ps = U32_MAX; + /* * The platform does not remove power for a kernel managed suspend so * use host managed nvme power settings for lowest idle power if @@ -2866,8 +2868,14 @@ static int nvme_suspend(struct device *d * shutdown. But if the firmware is involved after the suspend or the * device does not support any non-default power states, shut down the * device fully. + * + * If ASPM is not enabled for the device, shut down the device and allow + * the PCI bus layer to put it into D3 in order to take the PCIe link + * down, so as to allow the platform to achieve its minimum low-power + * state (which may not be possible if the link is up). */ - if (pm_suspend_via_firmware() || !ctrl->npss) { + if (pm_suspend_via_firmware() || !ctrl->npss || + !pcie_aspm_enabled(pdev)) { nvme_dev_disable(ndev, true); return 0; } @@ -2880,7 +2888,6 @@ static int nvme_suspend(struct device *d ctrl->state != NVME_CTRL_ADMIN_ONLY) goto unfreeze; - ndev->last_ps = 0; ret = nvme_get_power_state(ctrl, &ndev->last_ps); if (ret < 0) goto unfreeze;