Message-ID: <4184911.zoJM7jZeH4@aspire.rjw.lan>
Date:   Sun, 12 Nov 2017 01:34:12 +0100
From:   "Rafael J. Wysocki" <rjw@...ysocki.net>
To:     Linux PM <linux-pm@...r.kernel.org>
Cc:     Bjorn Helgaas <bhelgaas@...gle.com>,
        Alan Stern <stern@...land.harvard.edu>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Linux ACPI <linux-acpi@...r.kernel.org>,
        Linux PCI <linux-pci@...r.kernel.org>,
        Linux Documentation <linux-doc@...r.kernel.org>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Kevin Hilman <khilman@...nel.org>
Subject: [PATCH v3 0/6] PM / sleep: Driver flags for system suspend/resume (part 2)

Hi All,

The following still applies:

On Wednesday, November 8, 2017 1:41:35 AM CET Rafael J. Wysocki wrote:
>
> This is a follow-up for the first part of the PM driver flags series
> sent previously some time ago with an intro as follows:
> 
> On Saturday, October 28, 2017 12:11:55 AM CET Rafael J. Wysocki wrote:
> > The following part of the original cover letter still applies:
> > 
> > On Monday, October 16, 2017 3:12:35 AM CEST Rafael J. Wysocki wrote:
> > > 
> > > This work was triggered by attempts to fix and optimize PM in the
> > > i2c-designware-platdev driver that ended up with adding a couple of
> > > flags to the driver's internal data structures for the tracking of
> > > device state (https://marc.info/?l=linux-acpi&m=150629646805636&w=2).
> > > That approach is sort of suboptimal, though, because other drivers will
> > > probably want to do similar things and if all of them need to use internal
> > > flags for that, quite a bit of code duplication may ensue at least.
> > > 
> > > That can be avoided in a couple of ways and one of them is to provide a means
> > > for drivers to tell the core what to do and to make the core take care of it
> > > if told to do so.  Hence, the idea to use driver flags for system-wide PM
> > > that was briefly discussed during the LPC in LA last month.
> > 
> > [...]
> > 
> > > What can work (and this is the only strategy that can work AFAICS) is to
> > > point different callback pointers *in* *a* *driver* to the same routine
> > > if the driver wants to reuse that code.  That actually will work for PCI
> > > and USB drivers today, at least most of the time, but unfortunately there
> > > are problems with it for, say, platform devices.
> > > 
> > > The first problem is the requirement to track the status of the device
> > > (suspended vs not suspended) in the callbacks, because the system-wide PM
> > > code in the PM core doesn't do that.  The runtime PM framework does it, so
> > > this means adding some extra code which isn't necessary for runtime PM to
> > > the callback routines and that is not particularly nice.
> > > 
> > > The second problem is that, if the driver wants to do anything in its
> > > ->suspend callback, it generally has to prevent runtime suspend of the
> > > device from taking place in parallel with that, which is quite cumbersome.
> > > Usually, that is taken care of by resuming the device from runtime suspend
> > > upfront, but generally doing that is wasteful (there may be no real need to
> > > resume the device except for the fact that the code is designed this way).
> > > 
> > > On top of the above, there are optimizations to be made, like leaving certain
> > > devices in suspend after system resume to avoid wasting time on waiting for
> > > them to resume before user space can run again and similar.
> > > 
> > > This patch series focuses on addressing those problems so as to make it
> > > easier to reuse callback routines by pointing different callback pointers
> > > to them in device drivers.  The flags introduced here are to instruct the
> > > PM core and middle layers (whatever they are) on how the driver wants the
> > > device to be handled and then the driver has to provide callbacks to match
> > > these instructions and the rest should be taken care of by the code above it.
> > > 
> > > The flags are introduced one by one to avoid making too many changes in
> > > one go and to allow things to be explained better (hopefully).  They mostly
> > > are mutually independent with some clearly documented exceptions.
> > 
> > but I had to rework the core patches to address the problem pointed with the
> > generic power domains (genpd) framework pointed out by Ulf.
> > 
> > Namely, genpd expects its "noirq" callbacks to be invoked for devices in
> > runtime suspend too and it has valid reasons for that, so its "noirq"
> > callbacks can never be skipped, even for devices with the SMART_SUSPEND
> > flag set.  For this reason, the logic related to DPM_FLAG_SMART_SUSPEND
> > had to be moved from the core to the PCI bus type and the ACPI PM domain
> > which are mostly affected by it anyway.  The code after the changes looks
> > more straightforward to me, but it generally is more code and some patterns
> > had to be repeated in a few places.
> 
> I promised to send the rest of the series then:
> 
> > I will send the core patches for the remaining two flags introduced by the
> > original series separately and the intel-lpss and i2c-designware ones will
> > be posted when the core patches have been reviewed and agreed on.
> 
> and here it goes.
> 
> It actually only adds support for one additional flag, namely for
> DPM_FLAG_LEAVE_SUSPENDED, to the PM core (basic bits), PCI bus type and the
> ACPI PM domain.
> 
> That part of the series (patches [1-3/6]) is rather straightforward and, as PCI
> and the ACPI PM domain are concerned, it should be functionally equivalent to
> the previous version of the set, so I retained the Greg's ACKs on these patches.
> 
> The other part (patches [4-6/6]) is sort of new, as it makes the PM core
> carry out optimizations for devices with DPM_FLAG_LEAVE_SUSPENDED and/or
> DPM_FLAG_SMART_SUSPEND set where the "noirq", "early" and "late" system-wide
> PM callbacks provided by the drivers are invoked by the core directly.  That
> part basically allows platform drivers, for instance, to reuse runtime PM
> callbacks (by pointing ->suspend_late and ->resume_early to them) without
> adding extra checks to them, as long as they are called directly by the core
> (or the ACPI PM domain).

And on top of that, while replying to Ulf's comments I realized that devices
with nonzero runtime PM usage_count reference counters cannot be left in suspend
during system resume, because that would confuse the runtime PM framework going
forward.  Patches [1/6] and [5/6] have to be updated to avoid that, so here
goes a new revision.

It should apply on top of the current linux-next.

Thanks,
Rafael

Powered by blists - more mailing lists