From: Georg Gottleuber <[email protected]>
Some Kingston NV1 and A2000 are wasting a lot of power on specific TUXEDO
platforms in s2idle sleep if 'Simple Suspend' is used.
This patch applies a new quirk 'Force No Simple Suspend' to achieve a
low power sleep without 'Simple Suspend'.
Signed-off-by: Werner Sembach <[email protected]>
Signed-off-by: Georg Gottleuber <[email protected]>
Cc: <[email protected]>
---
drivers/nvme/host/nvme.h | 5 +++++
drivers/nvme/host/pci.c | 16 +++++++++++++++-
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index f35647c470afa..74f74b459f5fa 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -156,6 +156,11 @@ enum nvme_quirks {
* No temperature thresholds for channels other than 0 (Composite).
*/
NVME_QUIRK_NO_SECONDARY_TEMP_THRESH = (1 << 19),
+
+ /*
+ * Disables simple suspend/resume path.
+ */
+ NVME_QUIRK_FORCE_NO_SIMPLE_SUSPEND = (1 << 20),
};
/*
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 347cb5daebc3c..b4e6f0d21a44c 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2903,6 +2903,18 @@ static unsigned long check_vendor_combination_bug(struct pci_dev *pdev)
if ((dmi_match(DMI_BOARD_VENDOR, "LENOVO")) &&
dmi_match(DMI_BOARD_NAME, "LNVNB161216"))
return NVME_QUIRK_SIMPLE_SUSPEND;
+ } else if (pdev->vendor == 0x2646 && (pdev->device == 0x2263 ||
+ pdev->device == 0x500f)) {
+ /*
+ * Exclude some Kingston NV1 and A2000 devices from
+ * NVME_QUIRK_SIMPLE_SUSPEND. Do a full suspend to save a
+ * lot fo energy with s2idle sleep on some TUXEDO platforms.
+ */
+ if (dmi_match(DMI_BOARD_NAME, "NS5X_NS7XAU") ||
+ dmi_match(DMI_BOARD_NAME, "NS5x_7xAU") ||
+ dmi_match(DMI_BOARD_NAME, "NS5x_7xPU") ||
+ dmi_match(DMI_BOARD_NAME, "PH4PRX1_PH6PRX1"))
+ return NVME_QUIRK_FORCE_NO_SIMPLE_SUSPEND;
}
return 0;
@@ -2933,7 +2945,9 @@ static struct nvme_dev *nvme_pci_alloc_dev(struct pci_dev *pdev,
dev->dev = get_device(&pdev->dev);
quirks |= check_vendor_combination_bug(pdev);
- if (!noacpi && acpi_storage_d3(&pdev->dev)) {
+ if (!noacpi &&
+ !(quirks & NVME_QUIRK_FORCE_NO_SIMPLE_SUSPEND) &&
+ acpi_storage_d3(&pdev->dev)) {
/*
* Some systems use a bios work around to ask for D3 on
* platforms that support kernel managed suspend.
--
2.34.1
Hi,
Am 20.09.23 um 10:52 schrieb Werner Sembach:
> From: Georg Gottleuber <[email protected]>
>
> Some Kingston NV1 and A2000 are wasting a lot of power on specific TUXEDO
> platforms in s2idle sleep if 'Simple Suspend' is used.
>
> This patch applies a new quirk 'Force No Simple Suspend' to achieve a
> low power sleep without 'Simple Suspend'.
>
> Signed-off-by: Werner Sembach <[email protected]>
> Signed-off-by: Georg Gottleuber <[email protected]>
> Cc: <[email protected]>
Want to leave a gentle bump here. Any issues with this patch?
Kind regards,
Werner Sembach
> ---
> drivers/nvme/host/nvme.h | 5 +++++
> drivers/nvme/host/pci.c | 16 +++++++++++++++-
> 2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index f35647c470afa..74f74b459f5fa 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -156,6 +156,11 @@ enum nvme_quirks {
> * No temperature thresholds for channels other than 0 (Composite).
> */
> NVME_QUIRK_NO_SECONDARY_TEMP_THRESH = (1 << 19),
> +
> + /*
> + * Disables simple suspend/resume path.
> + */
> + NVME_QUIRK_FORCE_NO_SIMPLE_SUSPEND = (1 << 20),
> };
>
> /*
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 347cb5daebc3c..b4e6f0d21a44c 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -2903,6 +2903,18 @@ static unsigned long check_vendor_combination_bug(struct pci_dev *pdev)
> if ((dmi_match(DMI_BOARD_VENDOR, "LENOVO")) &&
> dmi_match(DMI_BOARD_NAME, "LNVNB161216"))
> return NVME_QUIRK_SIMPLE_SUSPEND;
> + } else if (pdev->vendor == 0x2646 && (pdev->device == 0x2263 ||
> + pdev->device == 0x500f)) {
> + /*
> + * Exclude some Kingston NV1 and A2000 devices from
> + * NVME_QUIRK_SIMPLE_SUSPEND. Do a full suspend to save a
> + * lot fo energy with s2idle sleep on some TUXEDO platforms.
> + */
> + if (dmi_match(DMI_BOARD_NAME, "NS5X_NS7XAU") ||
> + dmi_match(DMI_BOARD_NAME, "NS5x_7xAU") ||
> + dmi_match(DMI_BOARD_NAME, "NS5x_7xPU") ||
> + dmi_match(DMI_BOARD_NAME, "PH4PRX1_PH6PRX1"))
> + return NVME_QUIRK_FORCE_NO_SIMPLE_SUSPEND;
> }
>
> return 0;
> @@ -2933,7 +2945,9 @@ static struct nvme_dev *nvme_pci_alloc_dev(struct pci_dev *pdev,
> dev->dev = get_device(&pdev->dev);
>
> quirks |= check_vendor_combination_bug(pdev);
> - if (!noacpi && acpi_storage_d3(&pdev->dev)) {
> + if (!noacpi &&
> + !(quirks & NVME_QUIRK_FORCE_NO_SIMPLE_SUSPEND) &&
> + acpi_storage_d3(&pdev->dev)) {
> /*
> * Some systems use a bios work around to ask for D3 on
> * platforms that support kernel managed suspend.
What does save a lot of power mean, and why does it only apply to
some platforms? I suspect the more likely case is that APST is
simply completely broken on this device?
Am 06.12.23 um 10:24 schrieb Christoph Hellwig:
> What does save a lot of power mean, and why does it only apply to
> some platforms? I suspect the more likely case is that APST is
> simply completely broken on this device?
Hello,
power consumption in s0ix varies depending on mainboard and NVMe SSD.
Some examples:
Board, nvme(id), mW w/o patch, mW with patch, kernel
NS5X_SN7XAU, 0x500f, 2691mW, 487mW, 6.2.0
NS5X_SN7XAU, 0x2263, 2262mW, 741mW, 6.2.0
PH4PRX1_PH6PRX1, 0x2263, 3042mW, 760mW, 6.2.0
NS5X_SN7XPU, 0x500f, 3135mW, 779mW, 6.5.0
All measurements were taken with the battery disconnected and a
modified adapter plug.
I don't have a deeper insight into APST to judge whether something is
not working. But all other NVMe SSDs we have tested sleep with a power
consumption below 2W (Samsung 970 Evo, Samsung 980, Corsair MP600 PRO
NH, other Kingston NV1 with ID != 0x500f).
We originally noticed that newer Kingston NV1 (with id 500f) led to
higher standby consumption. With some trial and error I came to the
conclusion that the problem lies with 'Simple Suspend'.
Kind regards,
Georg Gottleuber
Ok. I guess we have to live with this quirk then :(
Reviewed-by: Christoph Hellwig <[email protected]>