linux-nvme.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] nvme: Favor D3cold for suspend if NVMe device supports it
@ 2021-04-16  9:13 Kai-Heng Feng
  2021-04-19  6:50 ` Christoph Hellwig
  0 siblings, 1 reply; 5+ messages in thread
From: Kai-Heng Feng @ 2021-04-16  9:13 UTC (permalink / raw)
  To: kbusch, axboe, hch, sagi
  Cc: Kai-Heng Feng, Alex Deucher, Prike Liang, Shyam Sundar S K,
	open list:NVM EXPRESS DRIVER, open list

On AMD platforms that use s2idle, NVMe timeouts on s2idle resume,
because their SMU FW may cut off NVMe power during sleep.

Unlike Intel platforms where the power resources are generally
controlled by root port, the power resources is controlled by NVMe
device itself on recent AMD platforms:
...
    Scope (\_SB.PCI0.GP24.NVME)
    {
        Name (_PR0, Package (0x01)  // _PR0: Power Resources for D0
        {
            P0NV
        })
        Name (_PR2, Package (0x01)  // _PR2: Power Resources for D2
        {
            P0NV
        })
        Name (_PR3, Package (0x01)  // _PR3: Power Resources for D3hot
        {
            P0NV
        })
        Method (_PS0, 0, NotSerialized)  // _PS0: Power State 0
        {
        }

        Method (_PS3, 0, NotSerialized)  // _PS3: Power State 3
        {
        }
    }
...
And it's a great indication that the NVMe should use D3cold for sleep
instead of staying at D0.

So use NVME_QUIRK_SIMPLE_SUSPEND if the ACPI counterpart of NVMe device
supports D3cold.

Tested on HP EliteBook 845 G7/G8.

BugLink: https://gitlab.freedesktop.org/drm/amd/-/issues/1230
References: https://lore.kernel.org/linux-nvme/1618458725-17164-1-git-send-email-Prike.Liang@amd.com/
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Prike Liang <Prike.Liang@amd.com>
Cc: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v2:
- Typo. It's EliteBook 845, not EliteBook 840.

 drivers/nvme/host/pci.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 7249ae74f71f..cc190324a919 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2840,6 +2840,13 @@ static bool nvme_acpi_storage_d3(struct pci_dev *dev)
 	acpi_status status;
 	u8 val;
 
+	/*
+	 * If the device itself supports D3cold, use that instead of D0 ASPM +
+	 * NVMe APST.
+	 */
+	if (pci_pr3_present(dev))
+		return true;
+
 	/*
 	 * Look for _DSD property specifying that the storage device on the port
 	 * must use D3 to support deep platform power savings during
-- 
2.30.2


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] nvme: Favor D3cold for suspend if NVMe device supports it
  2021-04-16  9:13 [PATCH v2] nvme: Favor D3cold for suspend if NVMe device supports it Kai-Heng Feng
@ 2021-04-19  6:50 ` Christoph Hellwig
  2021-04-19  6:57   ` Kai-Heng Feng
  0 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2021-04-19  6:50 UTC (permalink / raw)
  To: Kai-Heng Feng
  Cc: kbusch, axboe, hch, sagi, Alex Deucher, Prike Liang,
	Shyam Sundar S K, open list:NVM EXPRESS DRIVER, open list

On Fri, Apr 16, 2021 at 05:13:44PM +0800, Kai-Heng Feng wrote:
> On AMD platforms that use s2idle, NVMe timeouts on s2idle resume,
> because their SMU FW may cut off NVMe power during sleep.

We're already have a discussion on a proper quirk for thse broken
platforms on the linux-nvme list, please take part in that discussion.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] nvme: Favor D3cold for suspend if NVMe device supports it
  2021-04-19  6:50 ` Christoph Hellwig
@ 2021-04-19  6:57   ` Kai-Heng Feng
  2021-04-21  8:41     ` Liang, Prike
  0 siblings, 1 reply; 5+ messages in thread
From: Kai-Heng Feng @ 2021-04-19  6:57 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Keith Busch, Jens Axboe, Sagi Grimberg, Alex Deucher,
	Prike Liang, Shyam Sundar S K, open list:NVM EXPRESS DRIVER,
	open list

On Mon, Apr 19, 2021 at 2:50 PM Christoph Hellwig <hch@lst.de> wrote:
>
> On Fri, Apr 16, 2021 at 05:13:44PM +0800, Kai-Heng Feng wrote:
> > On AMD platforms that use s2idle, NVMe timeouts on s2idle resume,
> > because their SMU FW may cut off NVMe power during sleep.
>
> We're already have a discussion on a proper quirk for thse broken
> platforms on the linux-nvme list, please take part in that discussion.

Thanks. I didn't notice v5 was sent the to mailing list.
As of now, AMD folks are also reviewing this, and I believe this
approach is less quirky.

Kai-Heng

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH v2] nvme: Favor D3cold for suspend if NVMe device supports it
  2021-04-19  6:57   ` Kai-Heng Feng
@ 2021-04-21  8:41     ` Liang, Prike
  2021-04-21 17:38       ` Kai-Heng Feng
  0 siblings, 1 reply; 5+ messages in thread
From: Liang, Prike @ 2021-04-21  8:41 UTC (permalink / raw)
  To: Kai-Heng Feng, Christoph Hellwig
  Cc: Keith Busch, Jens Axboe, Sagi Grimberg, Deucher, Alexander, S-k,
	Shyam-sundar, open list:NVM EXPRESS DRIVER, open list

[AMD Public Use]

According to BIOS guys the _PR3 should be always implemented for NVMe device on the onwards ASIC. This solution seems more simple and looks good to me.

Reviewed-by: Prike Liang <Prike.Liang@amd.com>

Thanks,
Prike
> -----Original Message-----
> From: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Sent: Monday, April 19, 2021 2:58 PM
> To: Christoph Hellwig <hch@lst.de>
> Cc: Keith Busch <kbusch@kernel.org>; Jens Axboe <axboe@fb.com>; Sagi
> Grimberg <sagi@grimberg.me>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Liang, Prike <Prike.Liang@amd.com>; S-k,
> Shyam-sundar <Shyam-sundar.S-k@amd.com>; open list:NVM EXPRESS
> DRIVER <linux-nvme@lists.infradead.org>; open list <linux-
> kernel@vger.kernel.org>
> Subject: Re: [PATCH v2] nvme: Favor D3cold for suspend if NVMe device
> supports it
>
> On Mon, Apr 19, 2021 at 2:50 PM Christoph Hellwig <hch@lst.de> wrote:
> >
> > On Fri, Apr 16, 2021 at 05:13:44PM +0800, Kai-Heng Feng wrote:
> > > On AMD platforms that use s2idle, NVMe timeouts on s2idle resume,
> > > because their SMU FW may cut off NVMe power during sleep.
> >
> > We're already have a discussion on a proper quirk for thse broken
> > platforms on the linux-nvme list, please take part in that discussion.
>
> Thanks. I didn't notice v5 was sent the to mailing list.
> As of now, AMD folks are also reviewing this, and I believe this approach is
> less quirky.
>
> Kai-Heng
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] nvme: Favor D3cold for suspend if NVMe device supports it
  2021-04-21  8:41     ` Liang, Prike
@ 2021-04-21 17:38       ` Kai-Heng Feng
  0 siblings, 0 replies; 5+ messages in thread
From: Kai-Heng Feng @ 2021-04-21 17:38 UTC (permalink / raw)
  To: Liang, Prike
  Cc: Christoph Hellwig, Keith Busch, Jens Axboe, Sagi Grimberg,
	Deucher, Alexander, S-k, Shyam-sundar,
	open list:NVM EXPRESS DRIVER, open list

On Wed, Apr 21, 2021 at 4:42 PM Liang, Prike <Prike.Liang@amd.com> wrote:
>
> [AMD Public Use]
>
> According to BIOS guys the _PR3 should be always implemented for NVMe device on the onwards ASIC. This solution seems more simple and looks good to me.

Turns out ASUS laptops don't have _PR3 implement in ACPI, so this
patch doesn't work on them.
For now, please use the patch from AMD instead, thanks!

Kai-Heng

>
> Reviewed-by: Prike Liang <Prike.Liang@amd.com>
>
> Thanks,
> Prike
> > -----Original Message-----
> > From: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > Sent: Monday, April 19, 2021 2:58 PM
> > To: Christoph Hellwig <hch@lst.de>
> > Cc: Keith Busch <kbusch@kernel.org>; Jens Axboe <axboe@fb.com>; Sagi
> > Grimberg <sagi@grimberg.me>; Deucher, Alexander
> > <Alexander.Deucher@amd.com>; Liang, Prike <Prike.Liang@amd.com>; S-k,
> > Shyam-sundar <Shyam-sundar.S-k@amd.com>; open list:NVM EXPRESS
> > DRIVER <linux-nvme@lists.infradead.org>; open list <linux-
> > kernel@vger.kernel.org>
> > Subject: Re: [PATCH v2] nvme: Favor D3cold for suspend if NVMe device
> > supports it
> >
> > On Mon, Apr 19, 2021 at 2:50 PM Christoph Hellwig <hch@lst.de> wrote:
> > >
> > > On Fri, Apr 16, 2021 at 05:13:44PM +0800, Kai-Heng Feng wrote:
> > > > On AMD platforms that use s2idle, NVMe timeouts on s2idle resume,
> > > > because their SMU FW may cut off NVMe power during sleep.
> > >
> > > We're already have a discussion on a proper quirk for thse broken
> > > platforms on the linux-nvme list, please take part in that discussion.
> >
> > Thanks. I didn't notice v5 was sent the to mailing list.
> > As of now, AMD folks are also reviewing this, and I believe this approach is
> > less quirky.
> >
> > Kai-Heng

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-04-21 17:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-16  9:13 [PATCH v2] nvme: Favor D3cold for suspend if NVMe device supports it Kai-Heng Feng
2021-04-19  6:50 ` Christoph Hellwig
2021-04-19  6:57   ` Kai-Heng Feng
2021-04-21  8:41     ` Liang, Prike
2021-04-21 17:38       ` Kai-Heng Feng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).