[PATCH] nvme-pci: Add sleep quirk for Samsung 990 Evo

[PATCH] nvme-pci: Add sleep quirk for Samsung 990 Evo

[Werner Sembach]

From: Georg Gottleuber <ggo@tuxedocomputers.com>

On some TUXEDO platforms, a Samsung 990 Evo NVMe leads to a high
power consumption in s2idle sleep (2-3 watts).

This patch applies 'Force No Simple Suspend' quirk to achieve a
sleep with a lower power consumption, typically around 0.5 watts.

Signed-off-by: Georg Gottleuber <ggo@tuxedocomputers.com>
Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
Cc: <stable@vger.kernel.org>
---
 drivers/nvme/host/pci.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index e6267a6aa3801..63f4947c960f9 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2917,6 +2917,17 @@ static unsigned long check_vendor_combination_bug(struct pci_dev *pdev)
 		    dmi_match(DMI_BOARD_NAME, "NS5x_7xPU") ||
 		    dmi_match(DMI_BOARD_NAME, "PH4PRX1_PH6PRX1"))
 			return NVME_QUIRK_FORCE_NO_SIMPLE_SUSPEND;
+	} else if (pdev->vendor == 0x144d && pdev->device == 0xa80d) {
+		/*
+		 * Exclude Samsung 990 Evo from NVME_QUIRK_SIMPLE_SUSPEND
+		 * because of high power consumption (> 2 Watt) in s2idle
+		 * sleep. Only some boards with Intel CPU are affected.
+		 */
+		if (dmi_match(DMI_BOARD_NAME, "GMxPXxx") ||
+		    dmi_match(DMI_BOARD_NAME, "PH4PG31") ||
+		    dmi_match(DMI_BOARD_NAME, "PH4PRX1_PH6PRX1") ||
+		    dmi_match(DMI_BOARD_NAME, "PH6PG01_PH6PG71"))
+			return NVME_QUIRK_FORCE_NO_SIMPLE_SUSPEND;
 	}
 
 	return 0;
-- 
2.34.1

Re: [PATCH] nvme-pci: Add sleep quirk for Samsung 990 Evo

[Christoph Hellwig]

On Thu, Mar 28, 2024 at 02:09:22PM +0100, Werner Sembach wrote:
> From: Georg Gottleuber <ggo@tuxedocomputers.com>
> 
> On some TUXEDO platforms, a Samsung 990 Evo NVMe leads to a high
> power consumption in s2idle sleep (2-3 watts).
> 
> This patch applies 'Force No Simple Suspend' quirk to achieve a
> sleep with a lower power consumption, typically around 0.5 watts.

Does this only apply to a specific SSD or all SSDs on this platform?
How do these platforms even get into the conditional?  Probably
through acpi_storage_d3 setting, which probably is set incorrectly
for the platform?  Any chance to just fix that?

Re: [PATCH] nvme-pci: Add sleep quirk for Samsung 990 Evo

[Georg Gottleuber]

Am 02.04.24 um 15:16 schrieb Christoph Hellwig:
> On Thu, Mar 28, 2024 at 02:09:22PM +0100, Werner Sembach wrote:
>> From: Georg Gottleuber <ggo@tuxedocomputers.com>
>>
>> On some TUXEDO platforms, a Samsung 990 Evo NVMe leads to a high
>> power consumption in s2idle sleep (2-3 watts).
>>
>> This patch applies 'Force No Simple Suspend' quirk to achieve a
>> sleep with a lower power consumption, typically around 0.5 watts.
> 
> Does this only apply to a specific SSD or all SSDs on this platform?
> How do these platforms even get into the conditional?  Probably
> through acpi_storage_d3 setting, which probably is set incorrectly
> for the platform?  Any chance to just fix that?

Yes, this only apply to a specific SSD. I tested these SSDs (on 
PH4PRX1_PH6PRX1):
* Kingston NV1, SNVS250G
* Samsung 980, MZ-V8V500
* Samsung 970 Evo, S46DNX0K900454D
* Samsung 980 Pro, S69ENX0T709932L

S2idle consumes around 0.4 watts with these SSDs. But with a Samsung 990 
Evo s2idle on this platform consumes 3.7 to 4.4 watts (6.8 vs 6.5 kernel).

With my quirk s2idle sleep consumption is at the same level of all other 
SSDs tested.

Other boards have different values (a bit less drastic: factor 3 to 8).

All measurements were taken with the battery disconnected and a
modified adapter plug.

Because of the isolated problems with this SSD I have not debugged 
acpi_storage_d3. Do you think that would make sense?

Kind regards,
Georg Gottleuber

Re: [PATCH] nvme-pci: Add sleep quirk for Samsung 990 Evo

[Keith Busch]

On Tue, Apr 02, 2024 at 05:13:48PM +0200, Georg Gottleuber wrote:
> Am 02.04.24 um 15:16 schrieb Christoph Hellwig:
> > On Thu, Mar 28, 2024 at 02:09:22PM +0100, Werner Sembach wrote:
> > > From: Georg Gottleuber <ggo@tuxedocomputers.com>
> > > 
> > > On some TUXEDO platforms, a Samsung 990 Evo NVMe leads to a high
> > > power consumption in s2idle sleep (2-3 watts).
> > > 
> > > This patch applies 'Force No Simple Suspend' quirk to achieve a
> > > sleep with a lower power consumption, typically around 0.5 watts.
> > 
> > Does this only apply to a specific SSD or all SSDs on this platform?
> > How do these platforms even get into the conditional?  Probably
> > through acpi_storage_d3 setting, which probably is set incorrectly
> > for the platform?  Any chance to just fix that?
> 
> Yes, this only apply to a specific SSD. I tested these SSDs (on
> PH4PRX1_PH6PRX1):
> * Kingston NV1, SNVS250G
> * Samsung 980, MZ-V8V500
> * Samsung 970 Evo, S46DNX0K900454D
> * Samsung 980 Pro, S69ENX0T709932L
> 
> S2idle consumes around 0.4 watts with these SSDs. But with a Samsung 990 Evo
> s2idle on this platform consumes 3.7 to 4.4 watts (6.8 vs 6.5 kernel).

For all these different SSDs you tested in this platform, do you see the
"platform quirk: setting simple suspend" in the dmesg? I just want to
confirm if the platform is changing the reported acpi_storage_d3 value
for different SSD models or if they're all the same.

Re: [PATCH] nvme-pci: Add sleep quirk for Samsung 990 Evo

[Georg Gottleuber]

Am 02.04.24 um 18:08 schrieb Keith Busch:
> On Tue, Apr 02, 2024 at 05:13:48PM +0200, Georg Gottleuber wrote:
>> Am 02.04.24 um 15:16 schrieb Christoph Hellwig:
>>> On Thu, Mar 28, 2024 at 02:09:22PM +0100, Werner Sembach wrote:
>>>> From: Georg Gottleuber <ggo@tuxedocomputers.com>
>>>>
>>>> On some TUXEDO platforms, a Samsung 990 Evo NVMe leads to a high
>>>> power consumption in s2idle sleep (2-3 watts).
>>>>
>>>> This patch applies 'Force No Simple Suspend' quirk to achieve a
>>>> sleep with a lower power consumption, typically around 0.5 watts.
>>>
>>> Does this only apply to a specific SSD or all SSDs on this platform?
>>> How do these platforms even get into the conditional?  Probably
>>> through acpi_storage_d3 setting, which probably is set incorrectly
>>> for the platform?  Any chance to just fix that?
>>
>> Yes, this only apply to a specific SSD. I tested these SSDs (on
>> PH4PRX1_PH6PRX1):
>> * Kingston NV1, SNVS250G
>> * Samsung 980, MZ-V8V500
>> * Samsung 970 Evo, S46DNX0K900454D
>> * Samsung 980 Pro, S69ENX0T709932L
>>
>> S2idle consumes around 0.4 watts with these SSDs. But with a Samsung 990 Evo
>> s2idle on this platform consumes 3.7 to 4.4 watts (6.8 vs 6.5 kernel).
> 
> For all these different SSDs you tested in this platform, do you see the
> "platform quirk: setting simple suspend" in the dmesg? I just want to
> confirm if the platform is changing the reported acpi_storage_d3 value
> for different SSD models or if they're all the same.

Yes, without my quirk for all tested SSDs (including Samsung 990 Evo) 
"platform quirk: setting simple suspend" is printed in dmesg.

Re: [PATCH] nvme-pci: Add sleep quirk for Samsung 990 Evo

[Werner Sembach]

Hi,

(hope Thunderbird supports In-Reply-To via mailto links)

wanted to give this thread a gentle push

Kind regards,

Werner