Message-Id: <20190508185955.11406-1-kai.heng.feng@canonical.com>
Date:   Thu,  9 May 2019 02:59:55 +0800
From:   Kai-Heng Feng <kai.heng.feng@...onical.com>
To:     keith.busch@...el.com, axboe@...com, hch@....de, sagi@...mberg.me
Cc:     linux-nvme@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Kai-Heng Feng <kai.heng.feng@...onical.com>,
        Mario Limonciello <mario.limonciello@...l.com>
Subject: [PATCH] nvme-pci: Use non-operational power state instead of D3 on Suspend-to-Idle

Several NVMes consume lots of power during Suspend-to-Idle.

According to the NVMe vendors, APST should be used and there are two
things that should be prevented:
- Controller shutdown (CC.SHN)
- PCI D3

I think that's because the Windows Modern Standby design document [1]
states below as a requirement:
"
Akin to AHCI PCIe SSDs, NVMe SSDs need to provide the host with a
non-operational power state that is comparable to DEVSLP (<5mW draw,
<100ms exit latency) in order to allow the host to perform appropriate
transitions into Modern Standby. Should the NVMe SSD not expose such a
non-operational power state, autonomous power state transitions (APST)
is the only other option to enter Modern Standby successfully.
"

D3 wasn't mentioned at all, though for some vendors D3 still put the
device into a low power state, others disable APST after trasition to
D3.

So instead of disabling NVMe controller in suspend callback we do the
following instead:
- Make sure all IRQs are quiesced
- Stop all queues
- Prevent the device entering D3
- Use memory barrier to make sure DMA operations are flushed on HMB devices

This patch has been verified on several different NVMes:
- Intel
- Hynix
- LiteOn
- Micron
- WDC
- Samsung
- Tohiba

With the new suspend routine, the laptop I tested use roughly 0.8W
under Suspend-to-Idle, and resume time is faster than the original
D3 scheme.

The only one exception so far is Toshiba XG5, which works with the old
suspend routine, so introduce a new quirk for XG5.
We are working with Toshiba to root cause the issue on XG5.

[1] https://docs.microsoft.com/en-us/windows-hardware/design/device-experiences/part-selection#ssd-storage

Tested-by: Lucien Kao <Lucien_kao@...pal.com>
Tested-by: Mice Lin <mice_lin@...tron.com>
Tested-by: Jason Tan <LiangChuan.Tan@....com>
Tested-by: Cody Liu (codyliu) <codyliu@...ron.com>
Tested-by: Theodore Ts'o <tytso@....edu>
Signed-off-by: Mario Limonciello <mario.limonciello@...l.com>
Signed-off-by: Kai-Heng Feng <kai.heng.feng@...onical.com>
---
 drivers/nvme/host/core.c |   8 +++
 drivers/nvme/host/nvme.h |   8 +++
 drivers/nvme/host/pci.c  | 102 +++++++++++++++++++++++++++++++++++++--
 3 files changed, 115 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index a6644a2c3ef7..929db749da98 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3903,6 +3903,14 @@ static inline void _nvme_check_size(void)
 	BUILD_BUG_ON(sizeof(struct nvme_directive_cmd) != 64);
 }
 
+void nvme_enter_deepest(struct nvme_ctrl *ctrl)
+{
+	int ret = nvme_set_features(ctrl, NVME_FEAT_POWER_MGMT, ctrl->npss,
+				    NULL, 0, NULL);
+	if (ret)
+		dev_warn(ctrl->device, "failed to set deepest non-operational state (%d)\n", ret);
+}
+EXPORT_SYMBOL_GPL(nvme_enter_deepest);
 
 static int __init nvme_core_init(void)
 {
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 5ee75b5ff83f..ea40a83314ae 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -92,6 +92,11 @@ enum nvme_quirks {
 	 * Broken Write Zeroes.
 	 */
 	NVME_QUIRK_DISABLE_WRITE_ZEROES		= (1 << 9),
+
+	/*
+	 * Use D3 on S2I regardless of NPSS.
+	 */
+	NVME_QUIRK_USE_D3_ON_S2I		= (1 << 10),
 };
 
 /*
@@ -229,6 +234,7 @@ struct nvme_ctrl {
 	/* Power saving configuration */
 	u64 ps_max_latency_us;
 	bool apst_enabled;
+	bool suspend_to_idle;
 
 	/* PCIe only: */
 	u32 hmpre;
@@ -467,6 +473,8 @@ int nvme_delete_ctrl(struct nvme_ctrl *ctrl);
 int nvme_get_log(struct nvme_ctrl *ctrl, u32 nsid, u8 log_page, u8 lsp,
 		void *log, size_t size, u64 offset);
 
+void nvme_enter_deepest(struct nvme_ctrl *ctrl);
+
 extern const struct attribute_group *nvme_ns_id_attr_groups[];
 extern const struct block_device_operations nvme_ns_head_ops;
 
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 3e4fb891a95a..dea42b41f9a8 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -23,6 +23,7 @@
 #include <linux/io-64-nonatomic-lo-hi.h>
 #include <linux/sed-opal.h>
 #include <linux/pci-p2pdma.h>
+#include <linux/suspend.h>
 
 #include "trace.h"
 #include "nvme.h"
@@ -2828,12 +2829,98 @@ static void nvme_remove(struct pci_dev *pdev)
 }
 
 #ifdef CONFIG_PM_SLEEP
+static void nvme_do_suspend_to_idle(struct pci_dev *pdev)
+{
+	struct nvme_dev *ndev = pci_get_drvdata(pdev);
+
+	pdev->dev_flags |= PCI_DEV_FLAGS_NO_D3;
+	ndev->ctrl.suspend_to_idle = true;
+
+	nvme_start_freeze(&ndev->ctrl);
+	nvme_wait_freeze_timeout(&ndev->ctrl, NVME_IO_TIMEOUT);
+	nvme_stop_queues(&ndev->ctrl);
+
+	nvme_enter_deepest(&ndev->ctrl);
+
+	if (ndev->ctrl.queue_count > 0) {
+		nvme_disable_io_queues(ndev);
+		nvme_poll_irqdisable(&ndev->queues[0], -1);
+	}
+
+	nvme_suspend_io_queues(ndev);
+	nvme_suspend_queue(&ndev->queues[0]);
+	pci_free_irq_vectors(pdev);
+
+	blk_mq_tagset_busy_iter(&ndev->tagset, nvme_cancel_request, &ndev->ctrl);
+	blk_mq_tagset_busy_iter(&ndev->admin_tagset, nvme_cancel_request, &ndev->ctrl);
+
+	/* Make sure all HMB DMA operations are done */
+	mb();
+}
+
+static int nvme_do_resume_from_idle(struct pci_dev *pdev)
+{
+	struct nvme_dev *ndev = pci_get_drvdata(pdev);
+	int result;
+
+	pdev->dev_flags &= ~PCI_DEV_FLAGS_NO_D3;
+	ndev->ctrl.suspend_to_idle = false;
+
+	result = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
+	if (result < 0)
+		goto out;
+
+	result = nvme_pci_configure_admin_queue(ndev);
+	if (result)
+		goto out;
+
+	result = nvme_alloc_admin_tags(ndev);
+	if (result)
+		goto out;
+
+	ndev->ctrl.max_hw_sectors = NVME_MAX_KB_SZ << 1;
+	ndev->ctrl.max_segments = NVME_MAX_SEGS;
+	mutex_unlock(&ndev->shutdown_lock);
+
+	result = nvme_init_identify(&ndev->ctrl);
+	if (result)
+		goto out;
+
+	result = nvme_setup_io_queues(ndev);
+	if (result)
+		goto out;
+
+	if (ndev->online_queues < 2) {
+		dev_warn(ndev->ctrl.device, "IO queues not created\n");
+		nvme_kill_queues(&ndev->ctrl);
+		nvme_remove_namespaces(&ndev->ctrl);
+	} else {
+		nvme_start_queues(&ndev->ctrl);
+		nvme_wait_freeze(&ndev->ctrl);
+		nvme_dev_add(ndev);
+		nvme_unfreeze(&ndev->ctrl);
+	}
+
+	nvme_start_ctrl(&ndev->ctrl);
+
+	return 0;
+
+ out:
+	nvme_remove_dead_ctrl(ndev, result);
+	return result;
+}
+
 static int nvme_suspend(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct nvme_dev *ndev = pci_get_drvdata(pdev);
 
-	nvme_dev_disable(ndev, true);
+	if (IS_ENABLED(CONFIG_ACPI) && pm_suspend_via_s2idle() &&
+	    ndev->ctrl.npss && !(ndev->ctrl.quirks & NVME_QUIRK_USE_D3_ON_S2I))
+		nvme_do_suspend_to_idle(pdev);
+	else
+		nvme_dev_disable(ndev, true);
+
 	return 0;
 }
 
@@ -2841,9 +2928,16 @@ static int nvme_resume(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct nvme_dev *ndev = pci_get_drvdata(pdev);
+	int result = 0;
 
-	nvme_reset_ctrl(&ndev->ctrl);
-	return 0;
+	if (ndev->ctrl.suspend_to_idle) {
+		result = nvme_do_resume_from_idle(pdev);
+		if (result)
+			dev_warn(dev, "failed to resume from idle state (%d)\n", result);
+	} else
+		nvme_reset_ctrl(&ndev->ctrl);
+
+	return result;
 }
 #endif
 
@@ -2921,6 +3015,8 @@ static const struct pci_device_id nvme_id_table[] = {
 	{ PCI_VDEVICE(INTEL, 0x5845),	/* Qemu emulated controller */
 		.driver_data = NVME_QUIRK_IDENTIFY_CNS |
 				NVME_QUIRK_DISABLE_WRITE_ZEROES, },
+	{ PCI_DEVICE(0x1179, 0x0116),	/* Toshiba XG5 */
+		.driver_data = NVME_QUIRK_USE_D3_ON_S2I, },
 	{ PCI_DEVICE(0x1bb1, 0x0100),   /* Seagate Nytro Flash Storage */
 		.driver_data = NVME_QUIRK_DELAY_BEFORE_CHK_RDY, },
 	{ PCI_DEVICE(0x1c58, 0x0003),	/* HGST adapter */
-- 
2.17.1

Powered by blists - more mailing lists