* [PATCH 1/2] PM / Domains: Allow genpd to power on during the system PM phase
2016-04-26 12:23 [PATCH 0/2] PM / Domains: Second step in improving system PM code in genpd Ulf Hansson
@ 2016-04-26 12:23 ` Ulf Hansson
2016-05-05 10:31 ` Kevin Hilman
2016-04-26 12:23 ` [PATCH 2/2] PM / Domains: Remove redundant wrapper functions for system PM Ulf Hansson
2016-04-27 21:29 ` [PATCH 0/2] PM / Domains: Second step in improving system PM code in genpd Rafael J. Wysocki
2 siblings, 1 reply; 6+ messages in thread
From: Ulf Hansson @ 2016-04-26 12:23 UTC (permalink / raw)
To: Rafael J. Wysocki, Kevin Hilman, Ulf Hansson, linux-pm
Cc: Len Brown, Pavel Machek, Geert Uytterhoeven, Lina Iyer,
Axel Haslam, Marek Szyprowski, Jon Hunter, Andy Gross,
Laurent Pinchart
If the PM domain is powered off when the first device in the domain starts
its system PM prepare phase, genpd prevents any further attempts to power
on the PM domain during the system PM phase. This constraint affects not
only the current device which is being prepared, but all devices within
the same PM domain.
This behaviour needs to be changed, as a subsystem/driver for a device in
the same PM domain may still need to be able to serve requests in the
system PM phase. Accordingly, it may need to runtime resume its device and
thus also request the corresponding PM domain to be powered on.
To deal with these scenarios, let's make the device operational in the
system PM prepare phase by runtime resuming it, no matter if the PM domain
was powered on or not. Additionally allow the PM domain to be powered on
via runtime PM during the system PM phase.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/base/power/domain.c | 84 +++++++++++++++++----------------------------
include/linux/pm_domain.h | 1 -
2 files changed, 31 insertions(+), 54 deletions(-)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index de23b64..a608f52 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -187,8 +187,7 @@ static int genpd_poweron(struct generic_pm_domain *genpd, unsigned int depth)
struct gpd_link *link;
int ret = 0;
- if (genpd->status == GPD_STATE_ACTIVE
- || (genpd->prepared_count > 0 && genpd->suspend_power_off))
+ if (genpd->status == GPD_STATE_ACTIVE)
return 0;
/*
@@ -735,21 +734,22 @@ static int pm_genpd_prepare(struct device *dev)
mutex_lock(&genpd->lock);
- if (genpd->prepared_count++ == 0) {
+ if (genpd->prepared_count++ == 0)
genpd->suspended_count = 0;
- genpd->suspend_power_off = genpd->status == GPD_STATE_POWER_OFF;
- }
mutex_unlock(&genpd->lock);
- if (genpd->suspend_power_off)
- return 0;
-
/*
- * The PM domain must be in the GPD_STATE_ACTIVE state at this point,
- * so genpd_poweron() will return immediately, but if the device
- * is suspended (e.g. it's been stopped by genpd_stop_dev()), we need
- * to make it operational.
+ * Even if the PM domain is powered off at this point, we can't expect
+ * it to remain in that state during the entire system PM suspend
+ * phase. Any subsystem/driver for a device in the PM domain, may still
+ * need to serve a request which may require the device to be runtime
+ * resumed and its PM domain to be powered.
+ *
+ * As we are disabling runtime PM at this point, we are preventing the
+ * subsystem/driver to decide themselves. For that reason, we need to
+ * make sure the device is operational as it may be required in some
+ * cases.
*/
pm_runtime_resume(dev);
__pm_runtime_disable(dev, false);
@@ -758,8 +758,7 @@ static int pm_genpd_prepare(struct device *dev)
if (ret) {
mutex_lock(&genpd->lock);
- if (--genpd->prepared_count == 0)
- genpd->suspend_power_off = false;
+ genpd->prepared_count--;
mutex_unlock(&genpd->lock);
pm_runtime_enable(dev);
@@ -786,7 +785,7 @@ static int pm_genpd_suspend(struct device *dev)
if (IS_ERR(genpd))
return -EINVAL;
- return genpd->suspend_power_off ? 0 : pm_generic_suspend(dev);
+ return pm_generic_suspend(dev);
}
/**
@@ -807,7 +806,7 @@ static int pm_genpd_suspend_late(struct device *dev)
if (IS_ERR(genpd))
return -EINVAL;
- return genpd->suspend_power_off ? 0 : pm_generic_suspend_late(dev);
+ return pm_generic_suspend_late(dev);
}
/**
@@ -827,8 +826,7 @@ static int pm_genpd_suspend_noirq(struct device *dev)
if (IS_ERR(genpd))
return -EINVAL;
- if (genpd->suspend_power_off
- || (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)))
+ if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))
return 0;
genpd_stop_dev(genpd, dev);
@@ -860,8 +858,7 @@ static int pm_genpd_resume_noirq(struct device *dev)
if (IS_ERR(genpd))
return -EINVAL;
- if (genpd->suspend_power_off
- || (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)))
+ if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))
return 0;
/*
@@ -894,7 +891,7 @@ static int pm_genpd_resume_early(struct device *dev)
if (IS_ERR(genpd))
return -EINVAL;
- return genpd->suspend_power_off ? 0 : pm_generic_resume_early(dev);
+ return pm_generic_resume_early(dev);
}
/**
@@ -915,7 +912,7 @@ static int pm_genpd_resume(struct device *dev)
if (IS_ERR(genpd))
return -EINVAL;
- return genpd->suspend_power_off ? 0 : pm_generic_resume(dev);
+ return pm_generic_resume(dev);
}
/**
@@ -936,7 +933,7 @@ static int pm_genpd_freeze(struct device *dev)
if (IS_ERR(genpd))
return -EINVAL;
- return genpd->suspend_power_off ? 0 : pm_generic_freeze(dev);
+ return pm_generic_freeze(dev);
}
/**
@@ -958,7 +955,7 @@ static int pm_genpd_freeze_late(struct device *dev)
if (IS_ERR(genpd))
return -EINVAL;
- return genpd->suspend_power_off ? 0 : pm_generic_freeze_late(dev);
+ return pm_generic_freeze_late(dev);
}
/**
@@ -980,7 +977,7 @@ static int pm_genpd_freeze_noirq(struct device *dev)
if (IS_ERR(genpd))
return -EINVAL;
- return genpd->suspend_power_off ? 0 : genpd_stop_dev(genpd, dev);
+ return genpd_stop_dev(genpd, dev);
}
/**
@@ -1000,8 +997,7 @@ static int pm_genpd_thaw_noirq(struct device *dev)
if (IS_ERR(genpd))
return -EINVAL;
- return genpd->suspend_power_off ?
- 0 : genpd_start_dev(genpd, dev);
+ return genpd_start_dev(genpd, dev);
}
/**
@@ -1023,7 +1019,7 @@ static int pm_genpd_thaw_early(struct device *dev)
if (IS_ERR(genpd))
return -EINVAL;
- return genpd->suspend_power_off ? 0 : pm_generic_thaw_early(dev);
+ return pm_generic_thaw_early(dev);
}
/**
@@ -1044,7 +1040,7 @@ static int pm_genpd_thaw(struct device *dev)
if (IS_ERR(genpd))
return -EINVAL;
- return genpd->suspend_power_off ? 0 : pm_generic_thaw(dev);
+ return pm_generic_thaw(dev);
}
/**
@@ -1072,26 +1068,13 @@ static int pm_genpd_restore_noirq(struct device *dev)
* At this point suspended_count == 0 means we are being run for the
* first time for the given domain in the present cycle.
*/
- if (genpd->suspended_count++ == 0) {
+ if (genpd->suspended_count++ == 0)
/*
* The boot kernel might put the domain into arbitrary state,
* so make it appear as powered off to pm_genpd_sync_poweron(),
* so that it tries to power it on in case it was really off.
*/
genpd->status = GPD_STATE_POWER_OFF;
- if (genpd->suspend_power_off) {
- /*
- * If the domain was off before the hibernation, make
- * sure it will be off going forward.
- */
- genpd_power_off(genpd, true);
-
- return 0;
- }
- }
-
- if (genpd->suspend_power_off)
- return 0;
pm_genpd_sync_poweron(genpd, true);
@@ -1110,7 +1093,6 @@ static int pm_genpd_restore_noirq(struct device *dev)
static void pm_genpd_complete(struct device *dev)
{
struct generic_pm_domain *genpd;
- bool run_complete;
dev_dbg(dev, "%s()\n", __func__);
@@ -1120,18 +1102,14 @@ static void pm_genpd_complete(struct device *dev)
mutex_lock(&genpd->lock);
- run_complete = !genpd->suspend_power_off;
- if (--genpd->prepared_count == 0)
- genpd->suspend_power_off = false;
+ genpd->prepared_count--;
mutex_unlock(&genpd->lock);
- if (run_complete) {
- pm_generic_complete(dev);
- pm_runtime_set_active(dev);
- pm_runtime_enable(dev);
- pm_request_idle(dev);
- }
+ pm_generic_complete(dev);
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+ pm_request_idle(dev);
}
/**
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 39285c7..dd5b044 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -57,7 +57,6 @@ struct generic_pm_domain {
unsigned int device_count; /* Number of devices */
unsigned int suspended_count; /* System suspend device counter */
unsigned int prepared_count; /* Suspend counter of prepared devices */
- bool suspend_power_off; /* Power status before system suspend */
int (*power_off)(struct generic_pm_domain *domain);
int (*power_on)(struct generic_pm_domain *domain);
struct gpd_dev_ops dev_ops;
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] PM / Domains: Remove redundant wrapper functions for system PM
2016-04-26 12:23 [PATCH 0/2] PM / Domains: Second step in improving system PM code in genpd Ulf Hansson
2016-04-26 12:23 ` [PATCH 1/2] PM / Domains: Allow genpd to power on during the system PM phase Ulf Hansson
@ 2016-04-26 12:23 ` Ulf Hansson
2016-05-05 10:42 ` Kevin Hilman
2016-04-27 21:29 ` [PATCH 0/2] PM / Domains: Second step in improving system PM code in genpd Rafael J. Wysocki
2 siblings, 1 reply; 6+ messages in thread
From: Ulf Hansson @ 2016-04-26 12:23 UTC (permalink / raw)
To: Rafael J. Wysocki, Kevin Hilman, Ulf Hansson, linux-pm
Cc: Len Brown, Pavel Machek, Geert Uytterhoeven, Lina Iyer,
Axel Haslam, Marek Szyprowski, Jon Hunter, Andy Gross,
Laurent Pinchart
Previous changes to genpd enables us to assign several of the system PM
callbacks to the pm_generic* helper functions, instead of invoking them
via wrapper functions.
While removing the wrapper functions, it became clear that some of the
current assignment actually was causing the wrong system PM callback to
be invoked. For example, the ->restore() callback ended up invoking
pm_generic_resume(), which thus may invoke the wrong callback in the
driver. Let's fix this here as well.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/base/power/domain.c | 203 +++-----------------------------------------
1 file changed, 12 insertions(+), 191 deletions(-)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index a608f52..658eb1b 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -768,48 +768,6 @@ static int pm_genpd_prepare(struct device *dev)
}
/**
- * pm_genpd_suspend - Suspend a device belonging to an I/O PM domain.
- * @dev: Device to suspend.
- *
- * Suspend a device under the assumption that its pm_domain field points to the
- * domain member of an object of type struct generic_pm_domain representing
- * a PM domain consisting of I/O devices.
- */
-static int pm_genpd_suspend(struct device *dev)
-{
- struct generic_pm_domain *genpd;
-
- dev_dbg(dev, "%s()\n", __func__);
-
- genpd = dev_to_genpd(dev);
- if (IS_ERR(genpd))
- return -EINVAL;
-
- return pm_generic_suspend(dev);
-}
-
-/**
- * pm_genpd_suspend_late - Late suspend of a device from an I/O PM domain.
- * @dev: Device to suspend.
- *
- * Carry out a late suspend of a device under the assumption that its
- * pm_domain field points to the domain member of an object of type
- * struct generic_pm_domain representing a PM domain consisting of I/O devices.
- */
-static int pm_genpd_suspend_late(struct device *dev)
-{
- struct generic_pm_domain *genpd;
-
- dev_dbg(dev, "%s()\n", __func__);
-
- genpd = dev_to_genpd(dev);
- if (IS_ERR(genpd))
- return -EINVAL;
-
- return pm_generic_suspend_late(dev);
-}
-
-/**
* pm_genpd_suspend_noirq - Completion of suspend of device in an I/O PM domain.
* @dev: Device to suspend.
*
@@ -873,92 +831,6 @@ static int pm_genpd_resume_noirq(struct device *dev)
}
/**
- * pm_genpd_resume_early - Early resume of a device in an I/O PM domain.
- * @dev: Device to resume.
- *
- * Carry out an early resume of a device under the assumption that its
- * pm_domain field points to the domain member of an object of type
- * struct generic_pm_domain representing a power domain consisting of I/O
- * devices.
- */
-static int pm_genpd_resume_early(struct device *dev)
-{
- struct generic_pm_domain *genpd;
-
- dev_dbg(dev, "%s()\n", __func__);
-
- genpd = dev_to_genpd(dev);
- if (IS_ERR(genpd))
- return -EINVAL;
-
- return pm_generic_resume_early(dev);
-}
-
-/**
- * pm_genpd_resume - Resume of device in an I/O PM domain.
- * @dev: Device to resume.
- *
- * Resume a device under the assumption that its pm_domain field points to the
- * domain member of an object of type struct generic_pm_domain representing
- * a power domain consisting of I/O devices.
- */
-static int pm_genpd_resume(struct device *dev)
-{
- struct generic_pm_domain *genpd;
-
- dev_dbg(dev, "%s()\n", __func__);
-
- genpd = dev_to_genpd(dev);
- if (IS_ERR(genpd))
- return -EINVAL;
-
- return pm_generic_resume(dev);
-}
-
-/**
- * pm_genpd_freeze - Freezing a device in an I/O PM domain.
- * @dev: Device to freeze.
- *
- * Freeze a device under the assumption that its pm_domain field points to the
- * domain member of an object of type struct generic_pm_domain representing
- * a power domain consisting of I/O devices.
- */
-static int pm_genpd_freeze(struct device *dev)
-{
- struct generic_pm_domain *genpd;
-
- dev_dbg(dev, "%s()\n", __func__);
-
- genpd = dev_to_genpd(dev);
- if (IS_ERR(genpd))
- return -EINVAL;
-
- return pm_generic_freeze(dev);
-}
-
-/**
- * pm_genpd_freeze_late - Late freeze of a device in an I/O PM domain.
- * @dev: Device to freeze.
- *
- * Carry out a late freeze of a device under the assumption that its
- * pm_domain field points to the domain member of an object of type
- * struct generic_pm_domain representing a power domain consisting of I/O
- * devices.
- */
-static int pm_genpd_freeze_late(struct device *dev)
-{
- struct generic_pm_domain *genpd;
-
- dev_dbg(dev, "%s()\n", __func__);
-
- genpd = dev_to_genpd(dev);
- if (IS_ERR(genpd))
- return -EINVAL;
-
- return pm_generic_freeze_late(dev);
-}
-
-/**
* pm_genpd_freeze_noirq - Completion of freezing a device in an I/O PM domain.
* @dev: Device to freeze.
*
@@ -1001,49 +873,6 @@ static int pm_genpd_thaw_noirq(struct device *dev)
}
/**
- * pm_genpd_thaw_early - Early thaw of device in an I/O PM domain.
- * @dev: Device to thaw.
- *
- * Carry out an early thaw of a device under the assumption that its
- * pm_domain field points to the domain member of an object of type
- * struct generic_pm_domain representing a power domain consisting of I/O
- * devices.
- */
-static int pm_genpd_thaw_early(struct device *dev)
-{
- struct generic_pm_domain *genpd;
-
- dev_dbg(dev, "%s()\n", __func__);
-
- genpd = dev_to_genpd(dev);
- if (IS_ERR(genpd))
- return -EINVAL;
-
- return pm_generic_thaw_early(dev);
-}
-
-/**
- * pm_genpd_thaw - Thaw a device belonging to an I/O power domain.
- * @dev: Device to thaw.
- *
- * Thaw a device under the assumption that its pm_domain field points to the
- * domain member of an object of type struct generic_pm_domain representing
- * a power domain consisting of I/O devices.
- */
-static int pm_genpd_thaw(struct device *dev)
-{
- struct generic_pm_domain *genpd;
-
- dev_dbg(dev, "%s()\n", __func__);
-
- genpd = dev_to_genpd(dev);
- if (IS_ERR(genpd))
- return -EINVAL;
-
- return pm_generic_thaw(dev);
-}
-
-/**
* pm_genpd_restore_noirq - Start of restore of device in an I/O PM domain.
* @dev: Device to resume.
*
@@ -1151,18 +980,10 @@ EXPORT_SYMBOL_GPL(pm_genpd_syscore_poweron);
#else /* !CONFIG_PM_SLEEP */
#define pm_genpd_prepare NULL
-#define pm_genpd_suspend NULL
-#define pm_genpd_suspend_late NULL
#define pm_genpd_suspend_noirq NULL
-#define pm_genpd_resume_early NULL
#define pm_genpd_resume_noirq NULL
-#define pm_genpd_resume NULL
-#define pm_genpd_freeze NULL
-#define pm_genpd_freeze_late NULL
#define pm_genpd_freeze_noirq NULL
-#define pm_genpd_thaw_early NULL
#define pm_genpd_thaw_noirq NULL
-#define pm_genpd_thaw NULL
#define pm_genpd_restore_noirq NULL
#define pm_genpd_complete NULL
@@ -1454,24 +1275,24 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
genpd->domain.ops.runtime_suspend = genpd_runtime_suspend;
genpd->domain.ops.runtime_resume = genpd_runtime_resume;
genpd->domain.ops.prepare = pm_genpd_prepare;
- genpd->domain.ops.suspend = pm_genpd_suspend;
- genpd->domain.ops.suspend_late = pm_genpd_suspend_late;
+ genpd->domain.ops.suspend = pm_generic_suspend;
+ genpd->domain.ops.suspend_late = pm_generic_suspend_late;
genpd->domain.ops.suspend_noirq = pm_genpd_suspend_noirq;
genpd->domain.ops.resume_noirq = pm_genpd_resume_noirq;
- genpd->domain.ops.resume_early = pm_genpd_resume_early;
- genpd->domain.ops.resume = pm_genpd_resume;
- genpd->domain.ops.freeze = pm_genpd_freeze;
- genpd->domain.ops.freeze_late = pm_genpd_freeze_late;
+ genpd->domain.ops.resume_early = pm_generic_resume_early;
+ genpd->domain.ops.resume = pm_generic_resume;
+ genpd->domain.ops.freeze = pm_generic_freeze;
+ genpd->domain.ops.freeze_late = pm_generic_freeze_late;
genpd->domain.ops.freeze_noirq = pm_genpd_freeze_noirq;
genpd->domain.ops.thaw_noirq = pm_genpd_thaw_noirq;
- genpd->domain.ops.thaw_early = pm_genpd_thaw_early;
- genpd->domain.ops.thaw = pm_genpd_thaw;
- genpd->domain.ops.poweroff = pm_genpd_suspend;
- genpd->domain.ops.poweroff_late = pm_genpd_suspend_late;
+ genpd->domain.ops.thaw_early = pm_generic_thaw_early;
+ genpd->domain.ops.thaw = pm_generic_thaw;
+ genpd->domain.ops.poweroff = pm_generic_poweroff;
+ genpd->domain.ops.poweroff_late = pm_generic_poweroff_late;
genpd->domain.ops.poweroff_noirq = pm_genpd_suspend_noirq;
genpd->domain.ops.restore_noirq = pm_genpd_restore_noirq;
- genpd->domain.ops.restore_early = pm_genpd_resume_early;
- genpd->domain.ops.restore = pm_genpd_resume;
+ genpd->domain.ops.restore_early = pm_generic_restore_early;
+ genpd->domain.ops.restore = pm_generic_restore;
genpd->domain.ops.complete = pm_genpd_complete;
if (genpd->flags & GENPD_FLAG_PM_CLK) {
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/2] PM / Domains: Second step in improving system PM code in genpd
2016-04-26 12:23 [PATCH 0/2] PM / Domains: Second step in improving system PM code in genpd Ulf Hansson
2016-04-26 12:23 ` [PATCH 1/2] PM / Domains: Allow genpd to power on during the system PM phase Ulf Hansson
2016-04-26 12:23 ` [PATCH 2/2] PM / Domains: Remove redundant wrapper functions for system PM Ulf Hansson
@ 2016-04-27 21:29 ` Rafael J. Wysocki
2 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2016-04-27 21:29 UTC (permalink / raw)
To: Ulf Hansson, Kevin Hilman
Cc: linux-pm, Len Brown, Pavel Machek, Geert Uytterhoeven, Lina Iyer,
Axel Haslam, Marek Szyprowski, Jon Hunter, Andy Gross,
Laurent Pinchart
On Tuesday, April 26, 2016 02:23:29 PM Ulf Hansson wrote:
> This is the second step in improving the system PM code in genpd. The first
> patch is actually fixing a bug while it also enables for further clean-ups,
> which is done in the second patch.
>
> The next step of improving system PM code in genpd will move it to the part of
> trying to optimize the behaviour, such as avoid waking up devices unless it's
> really needed.
>
> Note, these two patches is based upon "[PATCH v2 0/2] PM / Domains: Initial
> improvements of system PM code in genpd", which was recently posted. Probably
> it's a good idea to combine these series into one, if/when new re-spins are
> needed.
>
> Ulf Hansson (2):
> PM / Domains: Allow genpd to power on during the system PM phase
> PM / Domains: Remove redundant wrapper functions for system PM
>
> drivers/base/power/domain.c | 271 ++++++--------------------------------------
> include/linux/pm_domain.h | 1 -
> 2 files changed, 35 insertions(+), 237 deletions(-)
Kevin?
^ permalink raw reply [flat|nested] 6+ messages in thread