On Tue, 2010-09-14 at 10:23 -0600, Tim Gardner wrote:
> On 09/13/2010 12:31 AM, yong.s...@linaro.org wrote:
> > From: Arjan van de Ven<ar...@linux.intel.com>
> >
> > In order for PowerTOP to be able to report how well the new runtime PM is
> > working for the various drivers, the kernel needs to export some basic
> > statistics in sysfs.
> >
> > This patch adds two sysfs files in the runtime PM domain that expose the
> > total time a device has been active, and the time a device has been
> > suspended.
> >
> > With this PowerTOP can compute the activity percentage
> >
> > Active %age = 100 * (delta active) / (delta active + delta suspended)
> >
> > and present the information to the user.
> >
> > I've written the PowerTOP code (slated for version 1.12) already, and the
> > output looks like this:
> >
> > Runtime Device Power Management statistics
> > Active Device name
> > 10.0% 06:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd.
> > RTL8101E/RTL8102E PCI Express Fast Ethernet controller
> >
> > [version 2: fix stat update bugs noticed by Alan Stern]
> > [version 3: rebase to -next and move the sysfs declaration]
> >
> > Signed-off-by: Arjan van de Ven<ar...@linux.intel.com>
> > Signed-off-by: Rafael J. Wysocki<r...@sisk.pl>
> > Signed-off-by: Shen Yong<yong.s...@linaro.org>
> > ---
> > drivers/base/power/runtime.c | 54 ++++++++++++++++++++++++----
> > drivers/base/power/sysfs.c | 83
> > ++++++++++++++++++++++++++++++++----------
> > include/linux/pm.h | 6 +++
> > 3 files changed, 116 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> > index b0ec0e9..b78c401 100644
> > --- a/drivers/base/power/runtime.c
> > +++ b/drivers/base/power/runtime.c
> > @@ -123,6 +123,45 @@ int pm_runtime_idle(struct device *dev)
> > }
> > EXPORT_SYMBOL_GPL(pm_runtime_idle);
> >
> > +
> > +/**
> > + * update_pm_runtime_accounting - Update the time accounting of power
> > states
> > + * @dev: Device to update the accounting for
> > + *
> > + * In order to be able to have time accounting of the various power states
> > + * (as used by programs such as PowerTOP to show the effectiveness of
> > runtime
> > + * PM), we need to track the time spent in each state.
> > + * update_pm_runtime_accounting must be called each time before the
> > + * runtime_status field is updated, to account the time in the old state
> > + * correctly.
> > + */
> > +void update_pm_runtime_accounting(struct device *dev)
> > +{
> > + unsigned long now = jiffies;
> > + int delta;
> > +
> > + delta = now - dev->power.accounting_timestamp;
> > +
> > + if (delta< 0)
> > + delta = 0;
> > +
> > + dev->power.accounting_timestamp = now;
> > +
> > + if (dev->power.disable_depth> 0)
> > + return;
> > +
> > + if (dev->power.runtime_status == RPM_SUSPENDED)
> > + dev->power.suspended_jiffies += delta;
> > + else
> > + dev->power.active_jiffies += delta;
> > +}
> > +
> > +static void __update_runtime_status(struct device *dev, enum rpm_status
> > status)
> > +{
> > + update_pm_runtime_accounting(dev);
> > + dev->power.runtime_status = status;
> > +}
> > +
> > /**
> > * __pm_runtime_suspend - Carry out run-time suspend of given device.
> > * @dev: Device to suspend.
> > @@ -197,7 +236,7 @@ int __pm_runtime_suspend(struct device *dev, bool
> > from_wq)
> > goto repeat;
> > }
> >
> > - dev->power.runtime_status = RPM_SUSPENDING;
> > + __update_runtime_status(dev, RPM_SUSPENDING);
> > dev->power.deferred_resume = false;
> >
> > if (dev->bus&& dev->bus->pm&& dev->bus->pm->runtime_suspend) {
> > @@ -228,7 +267,7 @@ int __pm_runtime_suspend(struct device *dev, bool
> > from_wq)
> > }
> >
> > if (retval) {
> > - dev->power.runtime_status = RPM_ACTIVE;
> > + __update_runtime_status(dev, RPM_ACTIVE);
> > if (retval == -EAGAIN || retval == -EBUSY) {
> > if (dev->power.timer_expires == 0)
> > notify = true;
> > @@ -237,7 +276,7 @@ int __pm_runtime_suspend(struct device *dev, bool
> > from_wq)
> > pm_runtime_cancel_pending(dev);
> > }
> > } else {
> > - dev->power.runtime_status = RPM_SUSPENDED;
> > + __update_runtime_status(dev, RPM_SUSPENDED);
> > pm_runtime_deactivate_timer(dev);
> >
> > if (dev->parent) {
> > @@ -381,7 +420,7 @@ int __pm_runtime_resume(struct device *dev, bool
> > from_wq)
> > goto repeat;
> > }
> >
> > - dev->power.runtime_status = RPM_RESUMING;
> > + __update_runtime_status(dev, RPM_RESUMING);
> >
> > if (dev->bus&& dev->bus->pm&& dev->bus->pm->runtime_resume) {
> > spin_unlock_irq(&dev->power.lock);
> > @@ -411,10 +450,10 @@ int __pm_runtime_resume(struct device *dev, bool
> > from_wq)
> > }
> >
> > if (retval) {
> > - dev->power.runtime_status = RPM_SUSPENDED;
> > + __update_runtime_status(dev, RPM_SUSPENDED);
> > pm_runtime_cancel_pending(dev);
> > } else {
> > - dev->power.runtime_status = RPM_ACTIVE;
> > + __update_runtime_status(dev, RPM_ACTIVE);
> > if (parent)
> > atomic_inc(&parent->power.child_count);
> > }
> > @@ -848,7 +887,7 @@ int __pm_runtime_set_status(struct device *dev,
> > unsigned int status)
> > }
> >
> > out_set:
> > - dev->power.runtime_status = status;
> > + __update_runtime_status(dev, status);
> > dev->power.runtime_error = 0;
> > out:
> > spin_unlock_irqrestore(&dev->power.lock, flags);
> > @@ -1077,6 +1116,7 @@ void pm_runtime_init(struct device *dev)
> > dev->power.request_pending = false;
> > dev->power.request = RPM_REQ_NONE;
> > dev->power.deferred_resume = false;
> > + dev->power.accounting_timestamp = jiffies;
> > INIT_WORK(&dev->power.work, pm_runtime_work);
> >
> > dev->power.timer_expires = 0;
> > diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
> > index a4c33bc..b831ec5 100644
> > --- a/drivers/base/power/sysfs.c
> > +++ b/drivers/base/power/sysfs.c
> > @@ -6,6 +6,7 @@
> > #include<linux/string.h>
> > #include<linux/pm_runtime.h>
> > #include<asm/atomic.h>
> > +#include<linux/jiffies.h>
> > #include "power.h"
> >
> > /*
> > @@ -108,6 +109,65 @@ static ssize_t control_store(struct device * dev,
> > struct device_attribute *attr,
> > }
> >
> > static DEVICE_ATTR(control, 0644, control_show, control_store);
> > +
> > +static ssize_t rtpm_active_time_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + int ret;
> > + spin_lock_irq(&dev->power.lock);
> > + update_pm_runtime_accounting(dev);
> > + ret = sprintf(buf, "%i\n", jiffies_to_msecs(dev->power.active_jiffies));
> > + spin_unlock_irq(&dev->power.lock);
> > + return ret;
> > +}
> > +
> > +static DEVICE_ATTR(runtime_active_time, 0444, rtpm_active_time_show, NULL);
> > +
> > +static ssize_t rtpm_suspended_time_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + int ret;
> > + spin_lock_irq(&dev->power.lock);
> > + update_pm_runtime_accounting(dev);
> > + ret = sprintf(buf, "%i\n",
> > + jiffies_to_msecs(dev->power.suspended_jiffies));
> > + spin_unlock_irq(&dev->power.lock);
> > + return ret;
> > +}
> > +
> > +static DEVICE_ATTR(runtime_suspended_time, 0444, rtpm_suspended_time_show,
> > NULL);
> > +
> > +static ssize_t rtpm_status_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + const char *p;
> > +
> > + if (dev->power.runtime_error) {
> > + p = "error\n";
> > + } else if (dev->power.disable_depth) {
> > + p = "unsupported\n";
> > + } else {
> > + switch (dev->power.runtime_status) {
> > + case RPM_SUSPENDED:
> > + p = "suspended\n";
> > + break;
> > + case RPM_SUSPENDING:
> > + p = "suspending\n";
> > + break;
> > + case RPM_RESUMING:
> > + p = "resuming\n";
> > + break;
> > + case RPM_ACTIVE:
> > + p = "active\n";
> > + break;
> > + default:
> > + return -EIO;
> > + }
> > + }
> > + return sprintf(buf, p);
> > +}
> > +
> > +static DEVICE_ATTR(runtime_status, 0444, rtpm_status_show, NULL);
> > #endif
> >
> > static ssize_t
> > @@ -172,27 +232,8 @@ static ssize_t rtpm_enabled_show(struct device *dev,
> > return sprintf(buf, "enabled\n");
> > }
> >
> > -static ssize_t rtpm_status_show(struct device *dev,
> > - struct device_attribute *attr, char *buf)
> > -{
> > - if (dev->power.runtime_error)
> > - return sprintf(buf, "error\n");
> > - switch (dev->power.runtime_status) {
> > - case RPM_SUSPENDED:
> > - return sprintf(buf, "suspended\n");
> > - case RPM_SUSPENDING:
> > - return sprintf(buf, "suspending\n");
> > - case RPM_RESUMING:
> > - return sprintf(buf, "resuming\n");
> > - case RPM_ACTIVE:
> > - return sprintf(buf, "active\n");
> > - }
> > - return -EIO;
> > -}
> > -
Completely removing rtpm_status_show() doesn't appear correct, as this
is not removed in the upstream version of the patch.
> > static DEVICE_ATTR(runtime_usage, 0444, rtpm_usagecount_show, NULL);
> > static DEVICE_ATTR(runtime_active_kids, 0444, rtpm_children_show, NULL);
> > -static DEVICE_ATTR(runtime_status, 0444, rtpm_status_show, NULL);
> > static DEVICE_ATTR(runtime_enabled, 0444, rtpm_enabled_show, NULL);
> >
> > #endif
> > @@ -228,6 +269,9 @@ static DEVICE_ATTR(async, 0644, async_show,
> > async_store);
> > static struct attribute * power_attrs[] = {
> > #ifdef CONFIG_PM_RUNTIME
> > &dev_attr_control.attr,
> > + &dev_attr_runtime_status.attr,
> > + &dev_attr_runtime_suspended_time.attr,
> > + &dev_attr_runtime_active_time.attr,
> > #endif
> > &dev_attr_wakeup.attr,
> > #ifdef CONFIG_PM_ADVANCED_DEBUG
> > @@ -235,7 +279,6 @@ static struct attribute * power_attrs[] = {
> > #ifdef CONFIG_PM_RUNTIME
> > &dev_attr_runtime_usage.attr,
> > &dev_attr_runtime_active_kids.attr,
> > - &dev_attr_runtime_status.attr,
> > &dev_attr_runtime_enabled.attr,
> > #endif
> > #endif
> > diff --git a/include/linux/pm.h b/include/linux/pm.h
> > index 8e258c7..dca597f 100644
> > --- a/include/linux/pm.h
> > +++ b/include/linux/pm.h
> > @@ -476,9 +476,15 @@ struct dev_pm_info {
> > enum rpm_request request;
> > enum rpm_status runtime_status;
> > int runtime_error;
> > + unsigned long active_jiffies;
> > + unsigned long suspended_jiffies;
> > + unsigned long accounting_timestamp;
> > #endif
> > };
> >
> > +extern void update_pm_runtime_accounting(struct device *dev);
> > +
> > +
> > /*
> > * The PM_EVENT_ messages are also used by drivers implementing the legacy
> > * suspend framework, based on the ->suspend() and ->resume() callbacks
> > common
>
> This isn't a faithful backport of
> 8d4b9d1bfef117862a2889dec4dac227068544c9. What happened to
> rtpm_status_show() ? Even if its not used I'd prefer to keep that
> function in the patch so as to cause fewer conflicts if there are
> upstream stable patches.
>
> I prefer the attached version.
Tim's patch was on the right track to provide an updated
rtpm_status_show() routine from what we currently have in Maverick, but
this still didn't originate from upstream commit 8d4b9d1b.
I've thus taken the path of least resistance and cleanly cherry-picked
the following for the Ubuntu Maverick kernel:
commit 0fcb4eef8294492c8f1de8236b1ed81f09e42922
Author: Alan Stern <st...@rowland.harvard.edu>
Date: Thu Jul 8 00:05:37 2010 +0200
PM / Runtime: Make runtime_status attribute not debug-only (v. 2)
commit 8d4b9d1bfef117862a2889dec4dac227068544c9
Author: Arjan van de Ven <ar...@linux.intel.com>
Date: Mon Jul 19 02:01:06 2010 +0200
PM / Runtime: Add runtime PM statistics (v3)
Thanks,
Leann
_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
