[PATCH 0/2] ACPI / PCI / PM: Delay resuming runtine-suspended devices during system-suspend

[PATCH 0/2] ACPI / PCI / PM: Delay resuming runtine-suspended devices during system-suspend

[Rafael J. Wysocki]

Hi All,

The PCI bus type and the ACPI PM domain resume runtime-suspended devices during
their .prepare() callbacks, which is suboptimal.

One problem caused by that is increasing the time of system suspend if there
are many runtime-suspended devices to resume during it (because the .prepare()
callbacks are executed sequentially).

The following two patches are intended to improve that one aspect slightly
in those two subsystems.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH 1/2] ACPI / PM: Resume runtime-suspended devices later during system suspend

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Runtime-suspended devices are resumed during system suspend by
acpi_subsys_prepare() for two reasons: First, because they may need
to be reprogrammed in order to change their wakeup settings and,
second, because they may need to be operatonal for their children
to be successfully suspended.  That is a problem, though, if there
are many runtime-suspended devices that need to be resumed this
way during system suspend, because the .prepare() PM callbacks of
devices are executed sequentially and the times taken by them
accumulate, which may increase the total system suspend time quite
a bit.

For this reason, move the resume of runtime-suspended devices up
to the next phase of device suspend (during system suspend), except
for the ones that have power.ignore_children set.  The exception is
made, because the devices with power.ignore_children set may still
be necessary for their children to be successfully suspended (during
system suspend) and they won't be resumed automatically as a result
of the runtime resume of their children.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/device_pm.c |   41 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 3 deletions(-)

Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -901,15 +901,30 @@ EXPORT_SYMBOL_GPL(acpi_dev_resume_early)
 int acpi_subsys_prepare(struct device *dev)
 {
 	/*
-	 * Follow PCI and resume devices suspended at run time before running
-	 * their system suspend callbacks.
+	 * Devices having power.ignore_children set may still be necessary for
+	 * suspending their children in the next phase of device suspend.
 	 */
-	pm_runtime_resume(dev);
+	if (dev->power.ignore_children)
+		pm_runtime_resume(dev);
+
 	return pm_generic_prepare(dev);
 }
 EXPORT_SYMBOL_GPL(acpi_subsys_prepare);
 
 /**
+ * acpi_subsys_suspend - Run the device driver's suspend callback.
+ * @dev: Device to handle.
+ *
+ * Follow PCI and resume devices suspended at run time before running their
+ * system suspend callbacks.
+ */
+int acpi_subsys_suspend(struct device *dev)
+{
+	pm_runtime_resume(dev);
+	return pm_generic_suspend(dev);
+}
+
+/**
  * acpi_subsys_suspend_late - Suspend device using ACPI.
  * @dev: Device to suspend.
  *
@@ -937,6 +952,23 @@ int acpi_subsys_resume_early(struct devi
 	return ret ? ret : pm_generic_resume_early(dev);
 }
 EXPORT_SYMBOL_GPL(acpi_subsys_resume_early);
+
+/**
+ * acpi_subsys_freeze - Run the device driver's freeze callback.
+ * @dev: Device to handle.
+ */
+int acpi_subsys_freeze(struct device *dev)
+{
+	/*
+	 * This used to be done in acpi_subsys_prepare() for all devices and
+	 * some drivers may depend on it, so do it here.  Ideally, however,
+	 * runtime-suspended devices should not be touched during freeze/thaw
+	 * transitions.
+	 */
+	pm_runtime_resume(dev);
+	return pm_generic_freeze(dev);
+}
+
 #endif /* CONFIG_PM_SLEEP */
 
 static struct dev_pm_domain acpi_general_pm_domain = {
@@ -947,8 +979,11 @@ static struct dev_pm_domain acpi_general
 #endif
 #ifdef CONFIG_PM_SLEEP
 		.prepare = acpi_subsys_prepare,
+		.suspend = acpi_subsys_suspend,
 		.suspend_late = acpi_subsys_suspend_late,
 		.resume_early = acpi_subsys_resume_early,
+		.freeze = acpi_subsys_freeze,
+		.poweroff = acpi_subsys_suspend,
 		.poweroff_late = acpi_subsys_suspend_late,
 		.restore_early = acpi_subsys_resume_early,
 #endif

[PATCH 2/2] PCI / PM: Resume runtime-suspended devices later during system suspend

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Runtime-suspended devices are resumed during system suspend by
pci_pm_prepare() for two reasons: First, because they may need
to be reprogrammed in order to change their wakeup settings and,
second, because they may need to be operatonal for their children
to be successfully suspended.  That is a problem, though, if there
are many runtime-suspended devices that need to be resumed this
way during system suspend, because the .prepare() PM callbacks of
devices are executed sequentially and the times taken by them
accumulate, which may increase the total system suspend time quite
a bit.

For this reason, move the resume of runtime-suspended devices up
to the next phase of device suspend (during system suspend), except
for the ones that have power.ignore_children set.  The exception is
made, because the devices with power.ignore_children set may still
be necessary for their children to be successfully suspended (during
system suspend) and they won't be resumed automatically as a result
of the runtime resume of their children.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/pci-driver.c |   33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

Index: linux-pm/drivers/pci/pci-driver.c
===================================================================
--- linux-pm.orig/drivers/pci/pci-driver.c
+++ linux-pm/drivers/pci/pci-driver.c
@@ -616,15 +616,11 @@ static int pci_pm_prepare(struct device
 	int error = 0;
 
 	/*
-	 * PCI devices suspended at run time need to be resumed at this
-	 * point, because in general it is necessary to reconfigure them for
-	 * system suspend.  Namely, if the device is supposed to wake up the
-	 * system from the sleep state, we may need to reconfigure it for this
-	 * purpose.  In turn, if the device is not supposed to wake up the
-	 * system from the sleep state, we'll have to prevent it from signaling
-	 * wake-up.
+	 * Devices having power.ignore_children set may still be necessary for
+	 * suspending their children in the next phase of device suspend.
 	 */
-	pm_runtime_resume(dev);
+	if (dev->power.ignore_children)
+		pm_runtime_resume(dev);
 
 	if (drv && drv->pm && drv->pm->prepare)
 		error = drv->pm->prepare(dev);
@@ -654,6 +650,16 @@ static int pci_pm_suspend(struct device
 		goto Fixup;
 	}
 
+	/*
+	 * PCI devices suspended at run time need to be resumed at this point,
+	 * because in general it is necessary to reconfigure them for system
+	 * suspend.  Namely, if the device is supposed to wake up the system
+	 * from the sleep state, we may need to reconfigure it for this purpose.
+	 * In turn, if the device is not supposed to wake up the system from the
+	 * sleep state, we'll have to prevent it from signaling wake-up.
+	 */
+	pm_runtime_resume(dev);
+
 	pci_dev->state_saved = false;
 	if (pm->suspend) {
 		pci_power_t prev = pci_dev->current_state;
@@ -808,6 +814,14 @@ static int pci_pm_freeze(struct device *
 		return 0;
 	}
 
+	/*
+	 * This used to be done in pci_pm_prepare() for all devices and some
+	 * drivers may depend on it, so do it here.  Ideally, runtime-suspended
+	 * devices should not be touched during freeze/thaw transitions,
+	 * however.
+	 */
+	pm_runtime_resume(dev);
+
 	pci_dev->state_saved = false;
 	if (pm->freeze) {
 		int error;
@@ -915,6 +929,9 @@ static int pci_pm_poweroff(struct device
 		goto Fixup;
 	}
 
+	/* The reason to do that is the same as in pci_pm_suspend(). */
+	pm_runtime_resume(dev);
+
 	pci_dev->state_saved = false;
 	if (pm->poweroff) {
 		int error;

Re: [PATCH 2/2] PCI / PM: Resume runtime-suspended devices later during system suspend

[Bjorn Helgaas]

On Sun, Feb 23, 2014 at 4:21 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Runtime-suspended devices are resumed during system suspend by
> pci_pm_prepare() for two reasons: First, because they may need
> to be reprogrammed in order to change their wakeup settings and,
> second, because they may need to be operatonal for their children
> to be successfully suspended.  That is a problem, though, if there
> are many runtime-suspended devices that need to be resumed this
> way during system suspend, because the .prepare() PM callbacks of
> devices are executed sequentially and the times taken by them
> accumulate, which may increase the total system suspend time quite
> a bit.
>
> For this reason, move the resume of runtime-suspended devices up
> to the next phase of device suspend (during system suspend), except
> for the ones that have power.ignore_children set.  The exception is
> made, because the devices with power.ignore_children set may still
> be necessary for their children to be successfully suspended (during
> system suspend) and they won't be resumed automatically as a result
> of the runtime resume of their children.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

You can merge these two via your tree if you want.  I don't have any
changes queued up for pci-driver.c.

Bjorn

> ---
>  drivers/pci/pci-driver.c |   33 +++++++++++++++++++++++++--------
>  1 file changed, 25 insertions(+), 8 deletions(-)
>
> Index: linux-pm/drivers/pci/pci-driver.c
> ===================================================================
> --- linux-pm.orig/drivers/pci/pci-driver.c
> +++ linux-pm/drivers/pci/pci-driver.c
> @@ -616,15 +616,11 @@ static int pci_pm_prepare(struct device
>         int error = 0;
>
>         /*
> -        * PCI devices suspended at run time need to be resumed at this
> -        * point, because in general it is necessary to reconfigure them for
> -        * system suspend.  Namely, if the device is supposed to wake up the
> -        * system from the sleep state, we may need to reconfigure it for this
> -        * purpose.  In turn, if the device is not supposed to wake up the
> -        * system from the sleep state, we'll have to prevent it from signaling
> -        * wake-up.
> +        * Devices having power.ignore_children set may still be necessary for
> +        * suspending their children in the next phase of device suspend.
>          */
> -       pm_runtime_resume(dev);
> +       if (dev->power.ignore_children)
> +               pm_runtime_resume(dev);
>
>         if (drv && drv->pm && drv->pm->prepare)
>                 error = drv->pm->prepare(dev);
> @@ -654,6 +650,16 @@ static int pci_pm_suspend(struct device
>                 goto Fixup;
>         }
>
> +       /*
> +        * PCI devices suspended at run time need to be resumed at this point,
> +        * because in general it is necessary to reconfigure them for system
> +        * suspend.  Namely, if the device is supposed to wake up the system
> +        * from the sleep state, we may need to reconfigure it for this purpose.
> +        * In turn, if the device is not supposed to wake up the system from the
> +        * sleep state, we'll have to prevent it from signaling wake-up.
> +        */
> +       pm_runtime_resume(dev);
> +
>         pci_dev->state_saved = false;
>         if (pm->suspend) {
>                 pci_power_t prev = pci_dev->current_state;
> @@ -808,6 +814,14 @@ static int pci_pm_freeze(struct device *
>                 return 0;
>         }
>
> +       /*
> +        * This used to be done in pci_pm_prepare() for all devices and some
> +        * drivers may depend on it, so do it here.  Ideally, runtime-suspended
> +        * devices should not be touched during freeze/thaw transitions,
> +        * however.
> +        */
> +       pm_runtime_resume(dev);
> +
>         pci_dev->state_saved = false;
>         if (pm->freeze) {
>                 int error;
> @@ -915,6 +929,9 @@ static int pci_pm_poweroff(struct device
>                 goto Fixup;
>         }
>
> +       /* The reason to do that is the same as in pci_pm_suspend(). */
> +       pm_runtime_resume(dev);
> +
>         pci_dev->state_saved = false;
>         if (pm->poweroff) {
>                 int error;
>

Re: [PATCH 2/2] PCI / PM: Resume runtime-suspended devices later during system suspend

[Rafael J. Wysocki]

On Monday, February 24, 2014 01:58:05 PM Bjorn Helgaas wrote:
> On Sun, Feb 23, 2014 at 4:21 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Runtime-suspended devices are resumed during system suspend by
> > pci_pm_prepare() for two reasons: First, because they may need
> > to be reprogrammed in order to change their wakeup settings and,
> > second, because they may need to be operatonal for their children
> > to be successfully suspended.  That is a problem, though, if there
> > are many runtime-suspended devices that need to be resumed this
> > way during system suspend, because the .prepare() PM callbacks of
> > devices are executed sequentially and the times taken by them
> > accumulate, which may increase the total system suspend time quite
> > a bit.
> >
> > For this reason, move the resume of runtime-suspended devices up
> > to the next phase of device suspend (during system suspend), except
> > for the ones that have power.ignore_children set.  The exception is
> > made, because the devices with power.ignore_children set may still
> > be necessary for their children to be successfully suspended (during
> > system suspend) and they won't be resumed automatically as a result
> > of the runtime resume of their children.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> You can merge these two via your tree if you want.  I don't have any
> changes queued up for pci-driver.c.

I will, thanks!

Rafael