[3/5] PM: Support for system-wide power transitions in generic power domains

Commit Message

Rafael Wysocki May 8, 2011, 9:24 p.m. UTC

From: Rafael J. Wysocki <[e-mail]>

Make generic power domains support system-wide power transitions
(system suspend and hibernation).  Add suspend, resume, freeze and
thaw callbacks to be associated with struct generic_power_domain
objects and make pm_genpd_init() use them as appropriate.

The new callbacks do nothing for devices belonging to power domains
that were powered down at run time (before the transition).  For the
other devices the action carried out depends on the type of the
transition.  During system suspend the power domain ->suspend()
callback executes pm_generic_suspend() for the device, while the
power domain ->suspend_noirq() callback stops the device and
eventually removes power from the power domain it belongs to (after
all devices in the power domain have been stopped).  During system
resume the power domain ->resume_noirq() callback powers up the power
domain (when executed for the first time for the given power domain)
and starts the device, while the ->resume() callback executes
pm_generic_resume() for the device.  Finally, the ->complete()
callback executes pm_runtime_idle() for the device which should put
it back into the suspended state if its runtime PM usage count is
equal to zero at that time.

The actions carried out during hibernation and resume from it are
analogous to the ones described above.

Signed-off-by: Rafael J. Wysocki <[e-mail]>
---
 drivers/base/power/domain.c |  292 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h   |    2 
 2 files changed, 294 insertions(+)

Comments

Alan Stern May 9, 2011, 2:36 p.m. UTC | #1

On Sun, 8 May 2011, Rafael J. Wysocki wrote:

> From: Rafael J. Wysocki <[e-mail]>
> 
> Make generic power domains support system-wide power transitions
> (system suspend and hibernation).  Add suspend, resume, freeze and
> thaw callbacks to be associated with struct generic_power_domain
> objects and make pm_genpd_init() use them as appropriate.

...

> Index: linux-2.6/drivers/base/power/domain.c
> ===================================================================
> --- linux-2.6.orig/drivers/base/power/domain.c
> +++ linux-2.6/drivers/base/power/domain.c
> @@ -273,6 +273,280 @@ static int pm_genpd_runtime_resume(struc
>  
>  #endif /* CONFIG_PM_RUNTIME */
>  
> +#ifdef CONFIG_PM_SLEEP
> +
> +/**
> + * pm_genpd_powered_down - Check if power has been removed from a power domain.
> + * @genpd: Power domain to check.
> + */
> +static bool pm_genpd_powered_down(struct generic_power_domain *genpd)
> +{
> +	bool ret;
> +
> +	mutex_lock(&genpd->lock);
> +	ret = genpd->power_is_off;
> +	mutex_unlock(&genpd->lock);
> +
> +	return ret;
> +}

What is the purpose of the mutex?  On the face of it, there's nothing 
to prevent another thread from changing the domain's power state in 
between the mutex_unlock() call and the return statement.

Alan Stern

Rafael Wysocki May 9, 2011, 7:20 p.m. UTC | #2

On Monday, May 09, 2011, Alan Stern wrote:
> On Sun, 8 May 2011, Rafael J. Wysocki wrote:
> 
> > From: Rafael J. Wysocki <[e-mail]>
> > 
> > Make generic power domains support system-wide power transitions
> > (system suspend and hibernation).  Add suspend, resume, freeze and
> > thaw callbacks to be associated with struct generic_power_domain
> > objects and make pm_genpd_init() use them as appropriate.
> 
> ...
> 
> > Index: linux-2.6/drivers/base/power/domain.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/base/power/domain.c
> > +++ linux-2.6/drivers/base/power/domain.c
> > @@ -273,6 +273,280 @@ static int pm_genpd_runtime_resume(struc
> >  
> >  #endif /* CONFIG_PM_RUNTIME */
> >  
> > +#ifdef CONFIG_PM_SLEEP
> > +
> > +/**
> > + * pm_genpd_powered_down - Check if power has been removed from a power domain.
> > + * @genpd: Power domain to check.
> > + */
> > +static bool pm_genpd_powered_down(struct generic_power_domain *genpd)
> > +{
> > +	bool ret;
> > +
> > +	mutex_lock(&genpd->lock);
> > +	ret = genpd->power_is_off;
> > +	mutex_unlock(&genpd->lock);
> > +
> > +	return ret;
> > +}
> 
> What is the purpose of the mutex?  On the face of it, there's nothing 
> to prevent another thread from changing the domain's power state in 
> between the mutex_unlock() call and the return statement.

Now, that I think of it, the mutex isn't really necessary.

genpd->power_is_off is only changed by the runtime PM code that will
never be called during system suspend if genpd->power_is_off is set
already.

I'm going to post an update shortly, I'll include a fix for that in it.

Thanks,
Rafael

Jonathan Corbet May 11, 2011, 5:17 p.m. UTC | #3

Hi, Rafael,

One small question that came to mind as I was looking at this patch:

> +/**
> + * pm_genpd_powered_down - Check if power has been removed from a power domain.
> + * @genpd: Power domain to check.
> + */
> +static bool pm_genpd_powered_down(struct generic_power_domain *genpd)
> +{
> +	bool ret;
> +
> +	mutex_lock(&genpd->lock);
> +	ret = genpd->power_is_off;
> +	mutex_unlock(&genpd->lock);
> +
> +	return ret;

I'm not quite sure why this function exists?  The lock doesn't really
change anything, since the power state can change before or after this
check regardless.  If you need the power state to be stable, it seems like
the lock needs to be taken further up the stack; otherwise simply checking
genpd->power_is_off directly would seem to be sufficient. Am I missing
something?

Thanks,

jon

Rafael Wysocki May 11, 2011, 7:11 p.m. UTC | #4

On Wednesday, May 11, 2011, Jonathan Corbet wrote:
> Hi, Rafael,
> 
> One small question that came to mind as I was looking at this patch:
> 
> > +/**
> > + * pm_genpd_powered_down - Check if power has been removed from a power domain.
> > + * @genpd: Power domain to check.
> > + */
> > +static bool pm_genpd_powered_down(struct generic_power_domain *genpd)
> > +{
> > +	bool ret;
> > +
> > +	mutex_lock(&genpd->lock);
> > +	ret = genpd->power_is_off;
> > +	mutex_unlock(&genpd->lock);
> > +
> > +	return ret;
> 
> I'm not quite sure why this function exists?  The lock doesn't really
> change anything, since the power state can change before or after this
> check regardless.  If you need the power state to be stable, it seems like
> the lock needs to be taken further up the stack; otherwise simply checking
> genpd->power_is_off directly would seem to be sufficient. Am I missing
> something?

No, you aren't. :-)

In fact, the function doesn't exist any more in the most recent version
of the patch: https://patchwork.kernel.org/patch/775412/

Thanks,
Rafael

Patch

Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -273,6 +273,280 @@  static int pm_genpd_runtime_resume(struc
 
 #endif /* CONFIG_PM_RUNTIME */
 
+#ifdef CONFIG_PM_SLEEP
+
+/**
+ * pm_genpd_powered_down - Check if power has been removed from a power domain.
+ * @genpd: Power domain to check.
+ */
+static bool pm_genpd_powered_down(struct generic_power_domain *genpd)
+{
+	bool ret;
+
+	mutex_lock(&genpd->lock);
+	ret = genpd->power_is_off;
+	mutex_unlock(&genpd->lock);
+
+	return ret;
+}
+
+/**
+ * pm_genpd_suspend - Suspend a device belonging to an I/O power domain.
+ * @dev: Device to suspend.
+ *
+ * Suspend a device under the assumption that its pwr_domain field points to the
+ * domain member of an object of type struct generic_power_domain representing
+ * a power domain consisting of I/O devices.
+ */
+static int pm_genpd_suspend(struct device *dev)
+{
+	struct generic_power_domain *genpd;
+
+	dev_dbg(dev, "%s()\n", __func__);
+
+	if (IS_ERR_OR_NULL(dev->pwr_domain))
+		return -EINVAL;
+
+	genpd = container_of(dev->pwr_domain,
+			     struct generic_power_domain, domain);
+
+	if (pm_genpd_powered_down(genpd))
+		return 0;
+
+	/*
+	 * If the device is in the (runtime) "suspended" state, call
+	 * ->start_device() for it, if defined.
+	 */
+	pm_runtime_resume(dev);
+
+	return pm_generic_suspend(dev);
+}
+
+/**
+ * pm_genpd_suspend_noirq - Late suspend of a device from an I/O power domain.
+ * @dev: Device to suspend.
+ *
+ * Carry out a late suspend of a device under the assumption that its
+ * pwr_domain field points to the domain member of an object of type
+ * struct generic_power_domain representing a power domain consisting of I/O
+ * devices.
+ */
+static int pm_genpd_suspend_noirq(struct device *dev)
+{
+	struct generic_power_domain *genpd;
+
+	dev_dbg(dev, "%s()\n", __func__);
+
+	if (IS_ERR_OR_NULL(dev->pwr_domain))
+		return -EINVAL;
+
+	genpd = container_of(dev->pwr_domain,
+			     struct generic_power_domain, domain);
+
+	if (pm_genpd_powered_down(genpd))
+		return 0;
+
+	if (genpd->stop_device)
+		genpd->stop_device(dev);
+
+	mutex_lock(&genpd->lock);
+	if (++genpd->suspended_count == genpd->device_count) {
+		if (genpd->power_off)
+			genpd->power_off(&genpd->domain);
+	}
+	mutex_unlock(&genpd->lock);
+
+	return 0;
+}
+
+/**
+ * pm_genpd_resume_noirq - Early resume of a device from an I/O power domain.
+ * @dev: Device to resume.
+ *
+ * Carry out an early resume of a device under the assumption that its
+ * pwr_domain field points to the domain member of an object of type
+ * struct generic_power_domain representing a power domain consisting of I/O
+ * devices.
+ */
+static int pm_genpd_resume_noirq(struct device *dev)
+{
+	struct generic_power_domain *genpd;
+
+	dev_dbg(dev, "%s()\n", __func__);
+
+	if (IS_ERR_OR_NULL(dev->pwr_domain))
+		return -EINVAL;
+
+	genpd = container_of(dev->pwr_domain,
+			     struct generic_power_domain, domain);
+
+	if (pm_genpd_powered_down(genpd))
+		return 0;
+
+	mutex_lock(&genpd->lock);
+	if (genpd->suspended_count == genpd->device_count) {
+		if (genpd->power_on) {
+			int ret = genpd->power_on(&genpd->domain);
+			if (ret) {
+				mutex_unlock(&genpd->lock);
+				return ret;
+			}
+		}
+	}
+	genpd->suspended_count--;
+	mutex_unlock(&genpd->lock);
+
+	if (genpd->start_device)
+		genpd->start_device(dev);
+
+	return 0;
+}
+
+/**
+ * pm_genpd_resume - Resume a device belonging to an I/O power domain.
+ * @dev: Device to resume.
+ *
+ * Resume a device under the assumption that its pwr_domain field points to the
+ * domain member of an object of type struct generic_power_domain representing
+ * a power domain consisting of I/O devices.
+ */
+static int pm_genpd_resume(struct device *dev)
+{
+	struct generic_power_domain *genpd;
+
+	dev_dbg(dev, "%s()\n", __func__);
+
+	if (IS_ERR_OR_NULL(dev->pwr_domain))
+		return -EINVAL;
+
+	genpd = container_of(dev->pwr_domain,
+			     struct generic_power_domain, domain);
+
+	return pm_genpd_powered_down(genpd) ? 0 : pm_generic_resume(dev);
+}
+
+/**
+ * pm_genpd_freeze - Freeze a device belonging to an I/O power domain.
+ * @dev: Device to freeze.
+ *
+ * Freeze a device under the assumption that its pwr_domain field points to the
+ * domain member of an object of type struct generic_power_domain representing
+ * a power domain consisting of I/O devices.
+ */
+static int pm_genpd_freeze(struct device *dev)
+{
+	struct generic_power_domain *genpd;
+
+	dev_dbg(dev, "%s()\n", __func__);
+
+	if (IS_ERR_OR_NULL(dev->pwr_domain))
+		return -EINVAL;
+
+	genpd = container_of(dev->pwr_domain,
+			     struct generic_power_domain, domain);
+
+	if (pm_genpd_powered_down(genpd))
+		return 0;
+
+	/*
+	 * If the device is in the (runtime) "suspended" state, call
+	 * ->start_device() for it, if defined.
+	 */
+	pm_runtime_resume(dev);
+
+	return pm_generic_freeze(dev);
+}
+
+/**
+ * pm_genpd_freeze_noirq - Late freeze of a device from an I/O power domain.
+ * @dev: Device to freeze.
+ *
+ * Carry out a late freeze of a device under the assumption that its
+ * pwr_domain field points to the domain member of an object of type
+ * struct generic_power_domain representing a power domain consisting of I/O
+ * devices.
+ */
+static int pm_genpd_freeze_noirq(struct device *dev)
+{
+	struct generic_power_domain *genpd;
+
+	dev_dbg(dev, "%s()\n", __func__);
+
+	if (IS_ERR_OR_NULL(dev->pwr_domain))
+		return -EINVAL;
+
+	genpd = container_of(dev->pwr_domain,
+			     struct generic_power_domain, domain);
+
+	if (!pm_genpd_powered_down(genpd) && genpd->stop_device)
+		genpd->stop_device(dev);
+
+	return 0;
+}
+
+/**
+ * pm_genpd_thaw_noirq - Early thaw of a device from an I/O power domain.
+ * @dev: Device to thaw.
+ *
+ * Carry out an early thaw of a device under the assumption that its
+ * pwr_domain field points to the domain member of an object of type
+ * struct generic_power_domain representing a power domain consisting of I/O
+ * devices.
+ */
+static int pm_genpd_thaw_noirq(struct device *dev)
+{
+	struct generic_power_domain *genpd;
+
+	dev_dbg(dev, "%s()\n", __func__);
+
+	if (IS_ERR_OR_NULL(dev->pwr_domain))
+		return -EINVAL;
+
+	genpd = container_of(dev->pwr_domain,
+			     struct generic_power_domain, domain);
+
+	if (!pm_genpd_powered_down(genpd) && genpd->start_device)
+		genpd->start_device(dev);
+
+	return 0;
+}
+
+/**
+ * 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 pwr_domain field points to the
+ * domain member of an object of type struct generic_power_domain representing
+ * a power domain consisting of I/O devices.
+ */
+static int pm_genpd_thaw(struct device *dev)
+{
+	struct generic_power_domain *genpd;
+
+	dev_dbg(dev, "%s()\n", __func__);
+
+	if (IS_ERR_OR_NULL(dev->pwr_domain))
+		return -EINVAL;
+
+	genpd = container_of(dev->pwr_domain,
+			     struct generic_power_domain, domain);
+
+	return pm_genpd_powered_down(genpd) ? 0 : pm_generic_thaw(dev);
+}
+
+#else
+
+#define pm_genpd_suspend	NULL
+#define pm_genpd_suspend_noirq	NULL
+#define pm_genpd_resume_noirq	NULL
+#define pm_genpd_resume		NULL
+#define pm_genpd_freeze		NULL
+#define pm_genpd_freeze_noirq	NULL
+#define pm_genpd_thaw_noirq	NULL
+#define pm_genpd_thaw		NULL
+
+#endif /* CONFIG_PM_SLEEP */
+
 /**
  * pm_genpd_add_device - Add a device to an I/O power domain.
  * @genpd: Power domain to add the device to.
@@ -304,6 +578,7 @@  int pm_genpd_add_device(struct generic_p
 
 	dle->dev = dev;
 	list_add_tail(&dle->node, &genpd->device_list);
+	genpd->device_count++;
 
 	spin_lock_irq(&dev->power.lock);
 	dev->pwr_domain = &genpd->domain;
@@ -341,6 +616,7 @@  int pm_genpd_remove_device(struct generi
 		dev->pwr_domain = NULL;
 		spin_unlock_irq(&dev->power.lock);
 
+		genpd->device_count--;
 		list_del(&dle->node);
 		kfree(dle);
 
@@ -438,7 +714,23 @@  void pm_genpd_init(struct generic_power_
 	genpd->gov = gov;
 	genpd->in_progress = 0;
 	genpd->power_is_off = is_off;
+	genpd->device_count = 0;
+	genpd->suspended_count = 0;
 	genpd->domain.ops.runtime_suspend = pm_genpd_runtime_suspend;
 	genpd->domain.ops.runtime_resume = pm_genpd_runtime_resume;
 	genpd->domain.ops.runtime_idle = pm_generic_runtime_idle;
+	genpd->domain.ops.prepare = pm_generic_prepare;
+	genpd->domain.ops.suspend = pm_genpd_suspend;
+	genpd->domain.ops.suspend_noirq = pm_genpd_suspend_noirq;
+	genpd->domain.ops.resume_noirq = pm_genpd_resume_noirq;
+	genpd->domain.ops.resume = pm_genpd_resume;
+	genpd->domain.ops.freeze = pm_genpd_freeze;
+	genpd->domain.ops.freeze_noirq = pm_genpd_freeze_noirq;
+	genpd->domain.ops.thaw_noirq = pm_genpd_thaw_noirq;
+	genpd->domain.ops.thaw = pm_genpd_thaw;
+	genpd->domain.ops.poweroff = pm_genpd_suspend;
+	genpd->domain.ops.poweroff_noirq = pm_genpd_suspend_noirq;
+	genpd->domain.ops.restore_noirq = pm_genpd_resume_noirq;
+	genpd->domain.ops.restore = pm_genpd_resume;
+	genpd->domain.ops.complete = pm_generic_complete;
 }
Index: linux-2.6/include/linux/pm_domain.h
===================================================================
--- linux-2.6.orig/include/linux/pm_domain.h
+++ linux-2.6/include/linux/pm_domain.h
@@ -25,6 +25,8 @@  struct generic_power_domain {
 	struct dev_power_governor *gov;
 	unsigned int in_progress;
 	bool power_is_off;
+	unsigned int device_count;
+	unsigned int suspended_count;
 	int (*power_off)(struct dev_power_domain *domain);
 	int (*power_on)(struct dev_power_domain *domain);
 	int (*start_device)(struct device *dev);