On Friday, August 25, 2017 3:42:35 PM CEST Rafael J. Wysocki wrote: > On Thursday, August 24, 2017 11:50:40 PM CEST Rafael J. Wysocki wrote: > > On Thursday, August 24, 2017 6:35:49 PM CEST Rafael J. Wysocki wrote: > > > On Thursday, August 24, 2017 11:15:26 AM CEST Ulf Hansson wrote: > > > > [cut] > > > > > [BTW, it is not entirely clear to me why it ever is necessary to runtime resume > > > a device with direct_complete set after __device_suspend(), because it can only > > > have direct_complete set at that point if all of the hierarchy below it has > > > this flag set too and so runtime PM has to be disabled for all of those > > > devices as well.] > > > > Which makes me realize that we should take a step back and look at what > > problems there are. > > > > First, there are devices (I know about two examples so far and both are PCI) > > that may need to be runtime resumed during system suspend for reasons other > > than the ones checked by the ACPI PM domain (or the PCI bus type). There needs > > to be a way to indicate that from the driver side. > > > > However, it still may be valuable to check the power-related conditions for > > leaving the device in runtime suspend over system suspend/resume in case > > it actually doesn't need to be runtime resumed during system suspend after > > all. That's what the majority of my patch was about. > > > > The second problem is that the ACPI PM domain (and the PCI bus type) > > runtime resumes all devices unconditionally in its ->suspend callback, > > even though that may not be necessary for some devices. Therefore there > > needs to be a way to indicate that too. That still would be good to > > have *regardless* of the direct_complete mechanism, because the direct_complete > > flag may not be set very often due to dependencies and then the > > resume-during-suspend will take place unnecessarily. > > > > Accordingly, it looks like we need a "no need to resume me" flag in the first > > place. That would indicate to interested pieces of code that, from the > > driver perspective, the device doesn't need to be runtime resumed before > > invoking its system suspend callbacks. This should be clear enough to everyone > > IMO. > > > > [Note that if that flag is set for all devices, we may drop it along with > > direct_complete, but before that happens both are needed.] > > I think we are in agreement that direct_complete will not be necessary any > more when all drivers/bus types/PM domains and so on can do the "safe > suspend", but we're not there yet. :-) > > > To address the first issue I would add something like the flag in the patches > > I sent (but without the ACPI PM domain part which should be covered by the > > "no need to resume me" flag above), because that allows the device's ->suspend > > callback to run in principle and the driver may use that callback even to > > runtime resume the device if that's what it wants to do. So something like > > "run my ->suspend callback even though I might stay in runtime suspend". > > > > I would probably add driver_flags to dev_pm_info for that to set at the probe > > time (and I would make the core clear that on driver removal). > > > > The complexity concern is there, but honestly I don't see a better way at > > this point. > > So below is a prototype patch. It still is missing a documentation update, but > other than that it should be complete unless I missed something. > > The way it works is that the SAFE_SUSPEND flag is not looked at by the core > at all. The ACPI PM domain looks at it and the PCI bus type can be modified > to take it into account in the future. That is what causes the "runtime resume > during system suspend" to be skipped. > > In turn, the ALWAYS_SUSPEND flag is only looked at by the core and it causes > the decision on whether or not to use direct_complete to be deferred to the > __device_suspend_late() time. If you set it for a PCI device, the effect is > equivalent to "no direct_complete". If you set it for a device in the ACPI > PM domain, that depends on whether or not SAFE_SUSPEND is set. If it isn't > set, the effect is equivalent to "no direct_complete" too, but if it is set, > the core may still try to use direct_complete for the device, but it will > make the decision on it in __device_suspend_late() and then it will not invoke > the ->suspend_late callback for the device if it is still runtime suspended. > [Note that you cannot runtime resume and runtime suspend again a device during > system suspend, so if it is runtime suspended in __device_suspend_late(), it > has been runtime suspend all the way since device_prepare().] > > So say you point the ->suspend_late and ->resume_early callbacks of > the designware i2c driver to pm_runtime_force_suspend() and > pm_runtime_force_resume(), respectively, and set both the SAFE_SUSPEND > and ALWAYS_SUSPEND flags for the device. > > If system suspend is started and the device is not runtime suspended, > direct_complete is not set for it and everything works as usual, so say > the device is runtime suspended in device_prepare(). Then, the ACPI PM > domain checks the other conditions for leaving it in runtime suspend and > returns either 0 or a positive number from acpi_subsys_prepare(). > > If 0 is returned, direct_complete is not set by the core and > acpi_subsys_suspend() is called. It checks the SAFE_SUSPEND flag and sees > that the device need not be runtime resumed, so it invokes the driver's > ->suspend callback (which is not present, so it doesn't do anything). > Next, in __device_suspend_late(), acpi_subsys_suspend_late() is invoked > and it calls pm_runtime_force_suspend(), which executes the driver's > ->runtime_suspend() callback, and then (if successful) calls > acpi_dev_suspend_late() to put the device into a low-power state. The > resume path is a reverse of the above in this case. So far, so good. Well, not really, because if the device remains runtime suspended, ->runtime_suspend() will not be called by pm_runtime_force_suspend() and acpi_dev_suspend_late() should not be called then. So more changes in the ACPI PM domain are needed after all. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
