public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Kai-Heng Feng <kai.heng.feng@canonical.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
	Christoph Hellwig <hch@lst.de>,
	Rafael Wysocki <rafael.j.wysocki@intel.com>,
	Mario Limonciello <Mario.Limonciello@dell.com>,
	Keith Busch <kbusch@kernel.org>,
	Keith Busch <keith.busch@intel.com>, Jens Axboe <axboe@fb.com>,
	Sagi Grimberg <sagi@grimberg.me>,
	linux-nvme <linux-nvme@lists.infradead.org>,
	Linux PM <linux-pm@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] nvme-pci: Use non-operational power state instead of D3 on Suspend-to-Idle
Date: Thu, 9 May 2019 11:56:01 +0200	[thread overview]
Message-ID: <20190509095601.GA19041@lst.de> (raw)
In-Reply-To: <A4DD2E9F-054E-4D4B-9F77-D69040EBE120@canonical.com>

On Thu, May 09, 2019 at 05:42:30PM +0800, Kai-Heng Feng wrote:
>> That would be a set of 6 new suspend and resume callbacks, mind you,
>> and there's quite a few of them already.  And the majority of drivers
>> would not need to use them anyway.
>
> I think suspend_to_idle() and resume_from_idle() should be enough?
> What are other 4 callbacks?
>
>>
>> Also, please note that, possibly apart from the device power state
>> setting, the S2I and S2R handling really aren't that different at all.
>> You basically need to carry out the same preparations during suspend
>> and reverse them during resume in both cases.
>
> But for this case, it’s quite different to the original suspend and 
> resume callbacks.

Let's think of what cases we needed.

The "classic" suspend in the nvme driver basically shuts down the
device entirely.  This is useful for:

 a) device that have no power management
 b) System power states that eventually power off the entire PCIe bus.
    I think that would:

     - suspend to disk (hibernate)
     - classic suspend to ram

The we have the sequence in your patch.  This seems to be related to
some of the MS wording, but I'm not sure what for example tearing down
the queues buys us.  Can you explain a bit more where those bits
make a difference?

Otherwise I think we should use a "no-op" suspend, just leaving the
power management to the device, or a simple setting the device to the
deepest power state for everything else, where everything else is
suspend, or suspend to idle.

And of course than we have windows modern standby actually mandating
runtime D3 in some case, and vague handwaving mentions of this being
forced on the platforms, which I'm not entirely sure how they fit
into the above picture.

  reply	other threads:[~2019-05-09  9:56 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-08 18:59 [PATCH] nvme-pci: Use non-operational power state instead of D3 on Suspend-to-Idle Kai-Heng Feng
2019-05-08 19:15 ` Chaitanya Kulkarni
2019-05-08 19:16 ` Keith Busch
2019-05-08 19:30   ` Kai-Heng Feng
2019-05-08 19:38     ` Mario.Limonciello
2019-05-08 19:51       ` Christoph Hellwig
2019-05-08 20:28         ` Mario.Limonciello
2019-05-09  6:12           ` Christoph Hellwig
2019-05-09  6:48             ` Kai-Heng Feng
2019-05-09  6:52               ` Christoph Hellwig
2019-05-09  9:19                 ` Rafael J. Wysocki
2019-05-09  9:25                   ` Christoph Hellwig
2019-05-09 20:48                     ` Rafael J. Wysocki
2019-05-09  9:07               ` Rafael J. Wysocki
2019-05-09  9:42                 ` Kai-Heng Feng
2019-05-09  9:56                   ` Christoph Hellwig [this message]
2019-05-09 10:28                     ` Kai-Heng Feng
2019-05-09 10:31                       ` Christoph Hellwig
2019-05-09 11:59                         ` Kai-Heng Feng
2019-05-09 18:57                           ` Mario.Limonciello
2019-05-09 19:28                             ` Keith Busch
2019-05-09 20:54                               ` Rafael J. Wysocki
2019-05-09 21:16                                 ` Keith Busch
2019-05-09 21:39                                   ` Rafael J. Wysocki
2019-05-09 21:37                               ` Mario.Limonciello
2019-05-09 21:54                                 ` Keith Busch
2019-05-09 22:19                                   ` Mario.Limonciello
2019-05-10  6:05                                     ` Kai-Heng Feng
2019-05-10  8:23                                       ` Rafael J. Wysocki
2019-05-10 13:52                                         ` Keith Busch
2019-05-10 15:15                                         ` Kai Heng Feng
2019-05-10 15:36                                           ` Keith Busch
2019-05-10 14:02                                       ` Keith Busch
2019-05-10 15:18                                         ` Kai Heng Feng
2019-05-10 15:49                                           ` hch
2019-05-10  5:30                               ` Christoph Hellwig
2019-05-10 13:51                                 ` Keith Busch
2019-05-09 16:20                       ` Keith Busch

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=20190509095601.GA19041@lst.de \
    --to=hch@lst.de \
    --cc=Mario.Limonciello@dell.com \
    --cc=axboe@fb.com \
    --cc=kai.heng.feng@canonical.com \
    --cc=kbusch@kernel.org \
    --cc=keith.busch@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=rafael@kernel.org \
    --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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox