diff mbox series

[v2,5/9] PM / ACPI: Provide option to disable direct_complete for ACPI devices

Message ID 1503499329-28834-6-git-send-email-ulf.hansson@linaro.org
State New
Headers show
Series PM / ACPI / i2c: Deploy runtime PM centric path for system sleep | expand

Commit Message

Ulf Hansson Aug. 23, 2017, 2:42 p.m. UTC 
In some cases a driver for an ACPI device needs to be able to prevent the
ACPI PM domain from using the direct_complete path during system sleep.

One typical case is when the driver for the device needs its device to stay
runtime enabled, during the __device_suspend phase. This isn't the case
when the direct_complete path is being executed by the PM core, as it then
disables runtime PM for the device in __device_suspend(). Any following
attempts to runtime resume the device after that point, just fails.

A workaround to this problem is to let the driver runtime resume its device
from its ->prepare() callback, as that would prevent the direct_complete
path from being executed. However, that may often be a waste, especially if
it turned out that no one really needed the device.

For this reason, invent acpi_dev_disable|enable_direct_complete(), to allow
drivers to inform the ACPI PM domain to change its default behaviour during
system sleep, and thus control whether it may use the direct_complete path
or not.

Typically a driver should call acpi_dev_disable_direct_comlete() during
->probe() and acpi_dev_enable_direct_complete() in ->remove().

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

---

Changes in v2:
	- Moved the no_direct_complete flag to the struct acpi_device_power.

---
 drivers/acpi/device_pm.c | 38 +++++++++++++++++++++++++++++++++++++-
 include/acpi/acpi_bus.h  |  1 +
 include/linux/acpi.h     |  4 ++++
 3 files changed, 42 insertions(+), 1 deletion(-)

-- 
2.7.4

Comments

Rafael J. Wysocki Aug. 24, 2017, 12:13 a.m. UTC | #1 
On Thursday, August 24, 2017 1:39:44 AM CEST Rafael J. Wysocki wrote:
> On Wed, Aug 23, 2017 at 4:42 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:

> > In some cases a driver for an ACPI device needs to be able to prevent the

> > ACPI PM domain from using the direct_complete path during system sleep.

> >

> > One typical case is when the driver for the device needs its device to stay

> > runtime enabled, during the __device_suspend phase. This isn't the case

> > when the direct_complete path is being executed by the PM core, as it then

> > disables runtime PM for the device in __device_suspend(). Any following

> > attempts to runtime resume the device after that point, just fails.

> 

> OK, that is a problem.

> 

> > A workaround to this problem is to let the driver runtime resume its device

> > from its ->prepare() callback, as that would prevent the direct_complete

> > path from being executed. However, that may often be a waste, especially if

> > it turned out that no one really needed the device.

> >

> > For this reason, invent acpi_dev_disable|enable_direct_complete(), to allow

> > drivers to inform the ACPI PM domain to change its default behaviour during

> > system sleep, and thus control whether it may use the direct_complete path

> > or not.

> 

> But I'm not sure this is the right place to address it as it very well

> may affect a PCI device too.

> 

> Also, this is about the device and not about its ACPI companion

> object, so putting the flag in there is somewhat unclean, so to speak.

> 

> It looks like we need a flag in dev_pm_info saying something along the

> lines of "my system suspend callback can deal with runtime PM" that

> will cause the direct_complete update to occur in

> __device_suspend_late() instead of __device_suspend().


IOW, something like the patch below.

It actually should work with the ACPI PM domain code as is except that it
will cause the device to be runtime resumed every time during suspend.

But acpi_subsys_suspend() can be changed to avoid resuming the device if
power.force_suspend is set.

---
 drivers/base/power/main.c |   11 +++++++++--
 include/linux/pm.h        |    1 +
 2 files changed, 10 insertions(+), 2 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.htmlIndex: linux-pm/drivers/base/power/main.c
===================================================================
--- linux-pm.orig/drivers/base/power/main.c
+++ linux-pm/drivers/base/power/main.c
@@ -1271,9 +1271,16 @@ static int __device_suspend_late(struct
 		goto Complete;
 	}
 
-	if (dev->power.syscore || dev->power.direct_complete)
+	if (dev->power.syscore)
 		goto Complete;
 
+	if (dev->power.direct_complete) {
+		if (pm_runtime_status_suspended(dev))
+			goto Complete;
+
+		dev->power.direct_complete = false;
+	}
+
 	if (dev->pm_domain) {
 		info = "late power domain ";
 		callback = pm_late_early_op(&dev->pm_domain->ops, state);
@@ -1482,7 +1489,7 @@ static int __device_suspend(struct devic
 	if (dev->power.syscore)
 		goto Complete;
 
-	if (dev->power.direct_complete) {
+	if (dev->power.direct_complete && !dev->power.force_suspend) {
 		if (pm_runtime_status_suspended(dev)) {
 			pm_runtime_disable(dev);
 			if (pm_runtime_status_suspended(dev))
Index: linux-pm/include/linux/pm.h
===================================================================
--- linux-pm.orig/include/linux/pm.h
+++ linux-pm/include/linux/pm.h
@@ -554,6 +554,7 @@ struct dev_pm_info {
 	pm_message_t		power_state;
 	unsigned int		can_wakeup:1;
 	unsigned int		async_suspend:1;
+	unsigned int		force_suspend:1;
 	bool			in_dpm_list:1;	/* Owned by the PM core */
 	bool			is_prepared:1;	/* Owned by the PM core */
 	bool			is_suspended:1;	/* Ditto */
Rafael J. Wysocki Aug. 24, 2017, 12:20 a.m. UTC | #2 
On Thursday, August 24, 2017 2:13:55 AM CEST Rafael J. Wysocki wrote:
> On Thursday, August 24, 2017 1:39:44 AM CEST Rafael J. Wysocki wrote:

> > On Wed, Aug 23, 2017 at 4:42 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:

> > > In some cases a driver for an ACPI device needs to be able to prevent the

> > > ACPI PM domain from using the direct_complete path during system sleep.

> > >

> > > One typical case is when the driver for the device needs its device to stay

> > > runtime enabled, during the __device_suspend phase. This isn't the case

> > > when the direct_complete path is being executed by the PM core, as it then

> > > disables runtime PM for the device in __device_suspend(). Any following

> > > attempts to runtime resume the device after that point, just fails.

> > 

> > OK, that is a problem.

> > 

> > > A workaround to this problem is to let the driver runtime resume its device

> > > from its ->prepare() callback, as that would prevent the direct_complete

> > > path from being executed. However, that may often be a waste, especially if

> > > it turned out that no one really needed the device.

> > >

> > > For this reason, invent acpi_dev_disable|enable_direct_complete(), to allow

> > > drivers to inform the ACPI PM domain to change its default behaviour during

> > > system sleep, and thus control whether it may use the direct_complete path

> > > or not.

> > 

> > But I'm not sure this is the right place to address it as it very well

> > may affect a PCI device too.

> > 

> > Also, this is about the device and not about its ACPI companion

> > object, so putting the flag in there is somewhat unclean, so to speak.

> > 

> > It looks like we need a flag in dev_pm_info saying something along the

> > lines of "my system suspend callback can deal with runtime PM" that

> > will cause the direct_complete update to occur in

> > __device_suspend_late() instead of __device_suspend().

> 

> IOW, something like the patch below.

> 

> It actually should work with the ACPI PM domain code as is except that it

> will cause the device to be runtime resumed every time during suspend.

> 

> But acpi_subsys_suspend() can be changed to avoid resuming the device if

> power.force_suspend is set.


Or better yet, if power.direct_complete is not set, because that means the
device needs to be resumed anyway.

And if power.direct_complete is set at that point, power.force_suspend has to
be set too.


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
index 5181057..f7bf596 100644
--- a/drivers/acpi/device_pm.c
+++ b/drivers/acpi/device_pm.c
@@ -933,6 +933,41 @@  EXPORT_SYMBOL_GPL(acpi_subsys_runtime_resume);
 
 #ifdef CONFIG_PM_SLEEP
 /**
+ * acpi_dev_disable_direct_complete - Disable the direct_complete path for ACPI.
+ * @dev: Device to disable the path for.
+ *
+ * Per default the ACPI PM domain tries to use the direct_complete path for its
+ * devices during system sleep. This function allows a user, typically a driver
+ * during probe, to disable the direct_complete path from being used by ACPI.
+ */
+void acpi_dev_disable_direct_complete(struct device *dev)
+{
+	struct acpi_device *adev = ACPI_COMPANION(dev);
+
+	if (adev)
+		adev->power.no_direct_complete = true;
+}
+EXPORT_SYMBOL_GPL(acpi_dev_disable_direct_complete);
+
+/**
+ * acpi_dev_enable_direct_complete - Enable the direct_complete path for ACPI.
+ * @dev: Device to enable the path for.
+ *
+ * Enable the direct_complete path to be used during system suspend for the ACPI
+ * PM domain, which is the default option. Typically a driver that disabled the
+ * path during ->probe(), must call this function during ->remove() to re-enable
+ * the direct_complete path to be used by ACPI.
+ */
+void acpi_dev_enable_direct_complete(struct device *dev)
+{
+	struct acpi_device *adev = ACPI_COMPANION(dev);
+
+	if (adev)
+		adev->power.no_direct_complete = false;
+}
+EXPORT_SYMBOL_GPL(acpi_dev_enable_direct_complete);
+
+/**
  * acpi_dev_suspend_late - Put device into a low-power state using ACPI.
  * @dev: Device to put into a low-power state.
  *
@@ -1023,7 +1058,8 @@  int acpi_subsys_prepare(struct device *dev)
 	if (ret < 0)
 		return ret;
 
-	if (!adev || !pm_runtime_suspended(dev))
+	if (!adev || adev->power.no_direct_complete ||
+	    !pm_runtime_suspended(dev))
 		return 0;
 
 	return !acpi_dev_needs_resume(dev, adev);
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index dedf9d7..bdec5f2 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -285,6 +285,7 @@  struct acpi_device_power_state {
 
 struct acpi_device_power {
 	int state;		/* Current state */
+	bool no_direct_complete;
 	struct acpi_device_power_flags flags;
 	struct acpi_device_power_state states[ACPI_D_STATE_COUNT];	/* Power states (D0-D3Cold) */
 };
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index c1a5213..ec33c41 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -867,6 +867,8 @@  static inline int acpi_dev_pm_attach(struct device *dev, bool power_on)
 #endif
 
 #if defined(CONFIG_ACPI) && defined(CONFIG_PM_SLEEP)
+void acpi_dev_disable_direct_complete(struct device *dev);
+void acpi_dev_enable_direct_complete(struct device *dev);
 int acpi_dev_suspend_late(struct device *dev);
 int acpi_dev_resume_early(struct device *dev);
 int acpi_subsys_prepare(struct device *dev);
@@ -876,6 +878,8 @@  int acpi_subsys_resume_early(struct device *dev);
 int acpi_subsys_suspend(struct device *dev);
 int acpi_subsys_freeze(struct device *dev);
 #else
+static inline void acpi_dev_disable_direct_complete(struct device *dev) {}
+static inline void acpi_dev_enable_direct_complete(struct device *dev) {}
 static inline int acpi_dev_suspend_late(struct device *dev) { return 0; }
 static inline int acpi_dev_resume_early(struct device *dev) { return 0; }
 static inline int acpi_subsys_prepare(struct device *dev) { return 0; }