From: "David E. Box" <david.e.box@linux.intel.com> To: rjw@rjwysocki.net, lenb@kernel.org, bhelgaas@google.com, kbusch@kernel.org, axboe@fb.com, hch@lst.de, sagi@grimberg.me Cc: "David E. Box" <david.e.box@linux.intel.com>, linux-acpi@vger.kernel.org, linux-pci@vger.kernel.org, linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org, Dan Williams <dan.j.williams@intel.com> Subject: [PATCH 1/2] pci: Add ACPI StorageD3Enable _DSD support Date: Mon, 27 Apr 2020 17:32:13 -0700 [thread overview] Message-ID: <20200428003214.3764-2-david.e.box@linux.intel.com> (raw) In-Reply-To: <20200428003214.3764-1-david.e.box@linux.intel.com> NVMe storage power management during suspend-to-idle, particularly on laptops, has been inconsistent with some devices working with D3 while others must rely on NVMe APST in order for power savings to be realized. Currently the default is to use APST unless quirked to do otherwise. However newer platforms, like Intel Comet Lake systems, may require NVMe drives to use D3 in order for the PCIe ports to be properly power managed. To make it easier for drivers to choose, these platforms may supply a special "StorageD3Enable" _DSD property under the root port that the device is attached to. If supplied, the driver must use D3 in order for the platform to realize the deepest power savings in suspend-to-idle. Adds support for the _DSD to the pci/acpi layer. Acked-by: Dan Williams <dan.j.williams@intel.com> Link: https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/power-management-for-storage-hardware-devices-intro Signed-off-by: David E. Box <david.e.box@linux.intel.com> --- drivers/acpi/property.c | 3 +++ drivers/pci/pci-acpi.c | 47 +++++++++++++++++++++++++++++++++++++++++ drivers/pci/pci.c | 6 ++++++ drivers/pci/pci.h | 4 ++++ include/linux/pci.h | 1 + 5 files changed, 61 insertions(+) diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c index e601c4511a8b..f09375ab40e4 100644 --- a/drivers/acpi/property.c +++ b/drivers/acpi/property.c @@ -45,6 +45,9 @@ static const guid_t prp_guids[] = { /* Thunderbolt GUID for WAKE_SUPPORTED: 6c501103-c189-4296-ba72-9bf5a26ebe5d */ GUID_INIT(0x6c501103, 0xc189, 0x4296, 0xba, 0x72, 0x9b, 0xf5, 0xa2, 0x6e, 0xbe, 0x5d), + /* D3 Support for storage devivce: 5025030f-842f-4ab4-a561-99a5189762d0 */ + GUID_INIT(0x5025030f, 0x842f, 0x4ab4, + 0xa5, 0x61, 0x99, 0xa5, 0x18, 0x97, 0x62, 0xd0), }; /* ACPI _DSD data subnodes GUID: dbb8e3e6-5886-4ba6-8795-1319f52a966b */ diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index d21969fba6ab..5df249ebf022 100644 --- a/drivers/pci/pci-acpi.c +++ b/drivers/pci/pci-acpi.c @@ -972,6 +972,52 @@ static bool acpi_pci_bridge_d3(struct pci_dev *dev) return val == 1; } +static bool acpi_pci_storage_d3(struct pci_dev *dev) +{ + const struct fwnode_handle *fwnode; + struct acpi_device *adev; + struct pci_dev *root; + acpi_handle handle; + acpi_status status; + u8 val; + + /* + * Look for _DSD property specifying that the storage device on + * the port must use D3 to support deep platform power savings during + * suspend-to-idle + */ + root = pci_find_pcie_root_port(dev); + if (!root) + return false; + + adev = ACPI_COMPANION(&root->dev); + if (root == dev) { + /* + * It is possible that the ACPI companion is not yet bound + * for the root port so look it up manually here. + */ + if (!adev && !pci_dev_is_added(root)) + adev = acpi_pci_find_companion(&root->dev); + } + + if (!adev) + return false; + + status = acpi_get_handle(adev->handle, "PXSX", &handle); + if (ACPI_FAILURE(status)) + return false; + + adev = acpi_bus_get_acpi_device(handle); + if (!adev) + return false; + + fwnode = acpi_fwnode_handle(adev); + if (!fwnode_property_read_u8(fwnode, "StorageD3Enable", &val)) + return val == 1; + + return false; +} + static bool acpi_pci_power_manageable(struct pci_dev *dev) { struct acpi_device *adev = ACPI_COMPANION(&dev->dev); @@ -1100,6 +1146,7 @@ static bool acpi_pci_need_resume(struct pci_dev *dev) static const struct pci_platform_pm_ops acpi_pci_platform_pm = { .bridge_d3 = acpi_pci_bridge_d3, + .storage_d3 = acpi_pci_storage_d3, .is_manageable = acpi_pci_power_manageable, .set_state = acpi_pci_set_power_state, .get_state = acpi_pci_get_power_state, diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 595fcf59843f..217d89f31e8b 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -871,6 +871,11 @@ static inline bool platform_pci_bridge_d3(struct pci_dev *dev) return pci_platform_pm ? pci_platform_pm->bridge_d3(dev) : false; } +static inline bool platform_pci_storage_d3(struct pci_dev *dev) +{ + return pci_platform_pm->storage_d3(dev); +} + /** * pci_raw_set_power_state - Use PCI PM registers to set the power state of * given PCI device @@ -2903,6 +2908,7 @@ void pci_pm_init(struct pci_dev *dev) dev->d3_delay = PCI_PM_D3_WAIT; dev->d3cold_delay = PCI_PM_D3COLD_WAIT; dev->bridge_d3 = pci_bridge_d3_possible(dev); + dev->storage_d3 = platform_pci_storage_d3(dev); dev->d3cold_allowed = true; dev->d1_support = false; diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 6d3f75867106..4dc4d05da810 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -53,6 +53,9 @@ int pci_bus_error_reset(struct pci_dev *dev); * * @bridge_d3: Does the bridge allow entering into D3 * + * @storage_d3: 'true' if the bridge contains a storage device that must use D3 + * to support platform s2idle + * * @is_manageable: returns 'true' if given device is power manageable by the * platform firmware * @@ -77,6 +80,7 @@ int pci_bus_error_reset(struct pci_dev *dev); */ struct pci_platform_pm_ops { bool (*bridge_d3)(struct pci_dev *dev); + bool (*storage_d3)(struct pci_dev *dev); bool (*is_manageable)(struct pci_dev *dev); int (*set_state)(struct pci_dev *dev, pci_power_t state); pci_power_t (*get_state)(struct pci_dev *dev); diff --git a/include/linux/pci.h b/include/linux/pci.h index 83ce1cdf5676..cbba1555020b 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -346,6 +346,7 @@ struct pci_dev { unsigned int no_d1d2:1; /* D1 and D2 are forbidden */ unsigned int no_d3cold:1; /* D3cold is forbidden */ unsigned int bridge_d3:1; /* Allow D3 for bridge */ + unsigned int storage_d3:1; /* Storage dev must use D3 for s2idle */ unsigned int d3cold_allowed:1; /* D3cold is allowed by user */ unsigned int mmio_always_on:1; /* Disallow turning off io/mem decoding during BAR sizing */ -- 2.20.1
WARNING: multiple messages have this Message-ID (diff)
From: "David E. Box" <david.e.box@linux.intel.com> To: rjw@rjwysocki.net, lenb@kernel.org, bhelgaas@google.com, kbusch@kernel.org, axboe@fb.com, hch@lst.de, sagi@grimberg.me Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, linux-nvme@lists.infradead.org, linux-acpi@vger.kernel.org, "David E. Box" <david.e.box@linux.intel.com>, Dan Williams <dan.j.williams@intel.com> Subject: [PATCH 1/2] pci: Add ACPI StorageD3Enable _DSD support Date: Mon, 27 Apr 2020 17:32:13 -0700 [thread overview] Message-ID: <20200428003214.3764-2-david.e.box@linux.intel.com> (raw) In-Reply-To: <20200428003214.3764-1-david.e.box@linux.intel.com> NVMe storage power management during suspend-to-idle, particularly on laptops, has been inconsistent with some devices working with D3 while others must rely on NVMe APST in order for power savings to be realized. Currently the default is to use APST unless quirked to do otherwise. However newer platforms, like Intel Comet Lake systems, may require NVMe drives to use D3 in order for the PCIe ports to be properly power managed. To make it easier for drivers to choose, these platforms may supply a special "StorageD3Enable" _DSD property under the root port that the device is attached to. If supplied, the driver must use D3 in order for the platform to realize the deepest power savings in suspend-to-idle. Adds support for the _DSD to the pci/acpi layer. Acked-by: Dan Williams <dan.j.williams@intel.com> Link: https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/power-management-for-storage-hardware-devices-intro Signed-off-by: David E. Box <david.e.box@linux.intel.com> --- drivers/acpi/property.c | 3 +++ drivers/pci/pci-acpi.c | 47 +++++++++++++++++++++++++++++++++++++++++ drivers/pci/pci.c | 6 ++++++ drivers/pci/pci.h | 4 ++++ include/linux/pci.h | 1 + 5 files changed, 61 insertions(+) diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c index e601c4511a8b..f09375ab40e4 100644 --- a/drivers/acpi/property.c +++ b/drivers/acpi/property.c @@ -45,6 +45,9 @@ static const guid_t prp_guids[] = { /* Thunderbolt GUID for WAKE_SUPPORTED: 6c501103-c189-4296-ba72-9bf5a26ebe5d */ GUID_INIT(0x6c501103, 0xc189, 0x4296, 0xba, 0x72, 0x9b, 0xf5, 0xa2, 0x6e, 0xbe, 0x5d), + /* D3 Support for storage devivce: 5025030f-842f-4ab4-a561-99a5189762d0 */ + GUID_INIT(0x5025030f, 0x842f, 0x4ab4, + 0xa5, 0x61, 0x99, 0xa5, 0x18, 0x97, 0x62, 0xd0), }; /* ACPI _DSD data subnodes GUID: dbb8e3e6-5886-4ba6-8795-1319f52a966b */ diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index d21969fba6ab..5df249ebf022 100644 --- a/drivers/pci/pci-acpi.c +++ b/drivers/pci/pci-acpi.c @@ -972,6 +972,52 @@ static bool acpi_pci_bridge_d3(struct pci_dev *dev) return val == 1; } +static bool acpi_pci_storage_d3(struct pci_dev *dev) +{ + const struct fwnode_handle *fwnode; + struct acpi_device *adev; + struct pci_dev *root; + acpi_handle handle; + acpi_status status; + u8 val; + + /* + * Look for _DSD property specifying that the storage device on + * the port must use D3 to support deep platform power savings during + * suspend-to-idle + */ + root = pci_find_pcie_root_port(dev); + if (!root) + return false; + + adev = ACPI_COMPANION(&root->dev); + if (root == dev) { + /* + * It is possible that the ACPI companion is not yet bound + * for the root port so look it up manually here. + */ + if (!adev && !pci_dev_is_added(root)) + adev = acpi_pci_find_companion(&root->dev); + } + + if (!adev) + return false; + + status = acpi_get_handle(adev->handle, "PXSX", &handle); + if (ACPI_FAILURE(status)) + return false; + + adev = acpi_bus_get_acpi_device(handle); + if (!adev) + return false; + + fwnode = acpi_fwnode_handle(adev); + if (!fwnode_property_read_u8(fwnode, "StorageD3Enable", &val)) + return val == 1; + + return false; +} + static bool acpi_pci_power_manageable(struct pci_dev *dev) { struct acpi_device *adev = ACPI_COMPANION(&dev->dev); @@ -1100,6 +1146,7 @@ static bool acpi_pci_need_resume(struct pci_dev *dev) static const struct pci_platform_pm_ops acpi_pci_platform_pm = { .bridge_d3 = acpi_pci_bridge_d3, + .storage_d3 = acpi_pci_storage_d3, .is_manageable = acpi_pci_power_manageable, .set_state = acpi_pci_set_power_state, .get_state = acpi_pci_get_power_state, diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 595fcf59843f..217d89f31e8b 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -871,6 +871,11 @@ static inline bool platform_pci_bridge_d3(struct pci_dev *dev) return pci_platform_pm ? pci_platform_pm->bridge_d3(dev) : false; } +static inline bool platform_pci_storage_d3(struct pci_dev *dev) +{ + return pci_platform_pm->storage_d3(dev); +} + /** * pci_raw_set_power_state - Use PCI PM registers to set the power state of * given PCI device @@ -2903,6 +2908,7 @@ void pci_pm_init(struct pci_dev *dev) dev->d3_delay = PCI_PM_D3_WAIT; dev->d3cold_delay = PCI_PM_D3COLD_WAIT; dev->bridge_d3 = pci_bridge_d3_possible(dev); + dev->storage_d3 = platform_pci_storage_d3(dev); dev->d3cold_allowed = true; dev->d1_support = false; diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 6d3f75867106..4dc4d05da810 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -53,6 +53,9 @@ int pci_bus_error_reset(struct pci_dev *dev); * * @bridge_d3: Does the bridge allow entering into D3 * + * @storage_d3: 'true' if the bridge contains a storage device that must use D3 + * to support platform s2idle + * * @is_manageable: returns 'true' if given device is power manageable by the * platform firmware * @@ -77,6 +80,7 @@ int pci_bus_error_reset(struct pci_dev *dev); */ struct pci_platform_pm_ops { bool (*bridge_d3)(struct pci_dev *dev); + bool (*storage_d3)(struct pci_dev *dev); bool (*is_manageable)(struct pci_dev *dev); int (*set_state)(struct pci_dev *dev, pci_power_t state); pci_power_t (*get_state)(struct pci_dev *dev); diff --git a/include/linux/pci.h b/include/linux/pci.h index 83ce1cdf5676..cbba1555020b 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -346,6 +346,7 @@ struct pci_dev { unsigned int no_d1d2:1; /* D1 and D2 are forbidden */ unsigned int no_d3cold:1; /* D3cold is forbidden */ unsigned int bridge_d3:1; /* Allow D3 for bridge */ + unsigned int storage_d3:1; /* Storage dev must use D3 for s2idle */ unsigned int d3cold_allowed:1; /* D3cold is allowed by user */ unsigned int mmio_always_on:1; /* Disallow turning off io/mem decoding during BAR sizing */ -- 2.20.1 _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme
next prev parent reply other threads:[~2020-04-28 0:32 UTC|newest] Thread overview: 78+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-04-28 0:32 [PATCH 0/2] Add support for StorageD3Enable _DSD property David E. Box 2020-04-28 0:32 ` David E. Box 2020-04-28 0:32 ` David E. Box [this message] 2020-04-28 0:32 ` [PATCH 1/2] pci: Add ACPI StorageD3Enable _DSD support David E. Box 2020-05-18 12:34 ` Rafael J. Wysocki 2020-05-18 12:34 ` Rafael J. Wysocki 2020-05-19 17:10 ` David E. Box 2020-05-19 17:10 ` David E. Box 2020-04-28 0:32 ` [PATCH 2/2] drivers/nvme: Add support for ACPI StorageD3Enable property David E. Box 2020-04-28 0:32 ` David E. Box 2020-04-28 5:13 ` [PATCH 0/2] Add support for StorageD3Enable _DSD property Christoph Hellwig 2020-04-28 5:13 ` Christoph Hellwig 2020-04-28 14:09 ` David E. Box 2020-04-28 14:09 ` David E. Box 2020-04-28 14:22 ` Christoph Hellwig 2020-04-28 14:22 ` Christoph Hellwig 2020-04-28 15:27 ` David E. Box 2020-04-28 15:27 ` David E. Box 2020-04-29 5:20 ` Williams, Dan J 2020-04-29 5:20 ` Williams, Dan J 2020-04-29 15:10 ` Keith Busch 2020-04-29 15:10 ` Keith Busch 2020-04-29 16:11 ` David E. Box 2020-04-29 16:11 ` David E. Box 2020-05-01 13:12 ` hch 2020-05-01 13:12 ` hch 2020-05-01 15:54 ` David E. Box 2020-05-01 15:54 ` David E. Box 2020-05-01 13:10 ` hch 2020-05-01 13:10 ` hch 2020-05-18 13:51 ` David Woodhouse 2020-05-18 13:51 ` David Woodhouse 2020-05-18 17:20 ` Dan Williams 2020-05-18 17:20 ` Dan Williams 2020-06-12 20:48 ` [PATCH V2 0/2] nvme: Add support for ACPI StorageD3Enable property David E. Box 2020-06-12 20:48 ` David E. Box 2020-06-24 18:55 ` David E. Box 2020-06-24 18:55 ` David E. Box 2020-06-24 19:10 ` Dan Williams 2020-06-24 19:10 ` Dan Williams 2020-06-24 19:39 ` David E. Box 2020-06-24 19:39 ` David E. Box 2020-07-02 21:04 ` [PATCH v3] drivers/nvme: " David E. Box 2020-07-02 22:50 ` [PATCH v4] " David E. Box 2020-07-02 22:50 ` David E. Box 2020-07-03 7:18 ` kernel test robot 2020-07-03 7:18 ` kernel test robot 2020-07-06 14:57 ` Rafael J. Wysocki 2020-07-06 14:57 ` Rafael J. Wysocki 2020-07-07 21:24 ` David E. Box 2020-07-07 21:24 ` David E. Box 2020-07-07 7:09 ` Christoph Hellwig 2020-07-07 7:09 ` Christoph Hellwig 2020-07-09 18:43 ` [PATCH V5] " David E. Box 2020-07-09 18:43 ` David E. Box 2020-07-13 11:12 ` Rafael J. Wysocki 2020-07-13 11:12 ` Rafael J. Wysocki 2020-07-16 14:39 ` Christoph Hellwig 2020-07-16 14:39 ` Christoph Hellwig 2021-05-26 19:25 ` Raul E Rangel 2021-05-26 19:25 ` Raul E Rangel 2020-06-12 20:48 ` [PATCH V2 1/2] PCI: Add ACPI StorageD3Enable _DSD support David E. Box 2020-06-12 20:48 ` David E. Box 2020-06-24 21:15 ` Bjorn Helgaas 2020-06-24 21:15 ` Bjorn Helgaas 2020-06-25 11:30 ` Rafael J. Wysocki 2020-06-25 11:30 ` Rafael J. Wysocki 2020-06-25 12:16 ` Mika Westerberg 2020-06-25 12:16 ` Mika Westerberg 2020-06-25 17:07 ` David E. Box 2020-06-25 17:30 ` Bjorn Helgaas 2020-06-25 17:30 ` Bjorn Helgaas 2020-06-24 21:37 ` Bjorn Helgaas 2020-06-24 21:37 ` Bjorn Helgaas 2020-06-24 22:09 ` David E. Box 2020-06-24 22:09 ` David E. Box 2020-06-12 20:48 ` [PATCH V2 2/2] drivers/nvme: Add support for ACPI StorageD3Enable property David E. Box 2020-06-12 20:48 ` David E. Box
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200428003214.3764-2-david.e.box@linux.intel.com \ --to=david.e.box@linux.intel.com \ --cc=axboe@fb.com \ --cc=bhelgaas@google.com \ --cc=dan.j.williams@intel.com \ --cc=hch@lst.de \ --cc=kbusch@kernel.org \ --cc=lenb@kernel.org \ --cc=linux-acpi@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-nvme@lists.infradead.org \ --cc=linux-pci@vger.kernel.org \ --cc=rjw@rjwysocki.net \ --cc=sagi@grimberg.me \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.