From 8a62ffe2753a845272f4f2100b5fca0b6053ff6f Mon Sep 17 00:00:00 2001 From: Vincent Guittot Date: Fri, 21 Dec 2018 11:33:54 +0100 Subject: [PATCH 1/5] PM-runtime: Add new interface to get accounted time Some drivers (like i915/drm) needs to get the accounted suspended time. pm_runtime_suspended_time() will return the suspended accounted time in ns unit. Reviewed-by: Ulf Hansson Signed-off-by: Vincent Guittot Signed-off-by: Rafael J. Wysocki --- drivers/base/power/runtime.c | 15 +++++++++++++++ include/linux/pm_runtime.h | 2 ++ 2 files changed, 17 insertions(+) diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 457be03b744d..a453090c9449 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -88,6 +88,21 @@ static void __update_runtime_status(struct device *dev, enum rpm_status status) dev->power.runtime_status = status; } +u64 pm_runtime_suspended_time(struct device *dev) +{ + unsigned long flags, time; + + spin_lock_irqsave(&dev->power.lock, flags); + + update_pm_runtime_accounting(dev); + time = dev->power.suspended_jiffies; + + spin_unlock_irqrestore(&dev->power.lock, flags); + + return jiffies_to_nsecs(time); +} +EXPORT_SYMBOL_GPL(pm_runtime_suspended_time); + /** * pm_runtime_deactivate_timer - Deactivate given device's suspend timer. * @dev: Device to handle. diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h index 54af4eef169f..a370006921c0 100644 --- a/include/linux/pm_runtime.h +++ b/include/linux/pm_runtime.h @@ -113,6 +113,8 @@ static inline bool pm_runtime_is_irq_safe(struct device *dev) return dev->power.irq_safe; } +extern u64 pm_runtime_suspended_time(struct device *dev); + #else /* !CONFIG_PM */ static inline bool queue_pm_work(struct work_struct *work) { return false; } From 3b4ed2e2eb5583774a1ed4aa4596a5a3c55f2567 Mon Sep 17 00:00:00 2001 From: Vincent Guittot Date: Fri, 21 Dec 2018 11:33:55 +0100 Subject: [PATCH 2/5] drm/i915: Move on the new pm runtime interface Use the new PM-runtime interface to get the accounted suspended time: pm_runtime_suspended_time(). This new interface helps to simplify and cleanup the code that computes __I915_SAMPLE_RC6_ESTIMATED and to remove direct access to internals of PM-runtime. Signed-off-by: Vincent Guittot Reviewed-by: Ulf Hansson Reviewed-by: Tvrtko Ursulin Signed-off-by: Rafael J. Wysocki --- drivers/gpu/drm/i915/i915_pmu.c | 16 ++++++---------- drivers/gpu/drm/i915/i915_pmu.h | 4 ++-- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index d6c8f8fdfda5..3f76f605ac31 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -5,6 +5,7 @@ */ #include +#include #include "i915_pmu.h" #include "intel_ringbuffer.h" #include "i915_drv.h" @@ -478,7 +479,6 @@ static u64 get_rc6(struct drm_i915_private *i915) * counter value. */ spin_lock_irqsave(&i915->pmu.lock, flags); - spin_lock(&kdev->power.lock); /* * After the above branch intel_runtime_pm_get_if_in_use failed @@ -491,16 +491,13 @@ static u64 get_rc6(struct drm_i915_private *i915) * suspended and if not we cannot do better than report the last * known RC6 value. */ - if (kdev->power.runtime_status == RPM_SUSPENDED) { + if (pm_runtime_status_suspended(kdev)) { + val = pm_runtime_suspended_time(kdev); + if (!i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur) - i915->pmu.suspended_jiffies_last = - kdev->power.suspended_jiffies; + i915->pmu.suspended_time_last = val; - val = kdev->power.suspended_jiffies - - i915->pmu.suspended_jiffies_last; - val += jiffies - kdev->power.accounting_timestamp; - - val = jiffies_to_nsecs(val); + val -= i915->pmu.suspended_time_last; val += i915->pmu.sample[__I915_SAMPLE_RC6].cur; i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur = val; @@ -510,7 +507,6 @@ static u64 get_rc6(struct drm_i915_private *i915) val = i915->pmu.sample[__I915_SAMPLE_RC6].cur; } - spin_unlock(&kdev->power.lock); spin_unlock_irqrestore(&i915->pmu.lock, flags); } diff --git a/drivers/gpu/drm/i915/i915_pmu.h b/drivers/gpu/drm/i915/i915_pmu.h index 7f164ca3db12..3dc2a30188e7 100644 --- a/drivers/gpu/drm/i915/i915_pmu.h +++ b/drivers/gpu/drm/i915/i915_pmu.h @@ -95,9 +95,9 @@ struct i915_pmu { */ struct i915_pmu_sample sample[__I915_NUM_PMU_SAMPLERS]; /** - * @suspended_jiffies_last: Cached suspend time from PM core. + * @suspended_time_last: Cached suspend time from PM core. */ - unsigned long suspended_jiffies_last; + u64 suspended_time_last; /** * @i915_attr: Memory block holding device attributes. */ From 40619f7dd3ef05ae7861bc60d401585d316e1374 Mon Sep 17 00:00:00 2001 From: Aditya Pakki Date: Sat, 5 Jan 2019 13:58:45 -0600 Subject: [PATCH 3/5] PM: clock_ops: fix missing clk_prepare() return value check clk_prepare() can fail, so check its status and if it fails, issue an error message and change the clock_entry_status to PCE_STATUS_ERROR. Signed-off-by: Aditya Pakki [ rjw: Subject ] Signed-off-by: Rafael J. Wysocki --- drivers/base/power/clock_ops.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c index 5a42ae4078c2..365ad751ce0f 100644 --- a/drivers/base/power/clock_ops.c +++ b/drivers/base/power/clock_ops.c @@ -65,10 +65,15 @@ static void pm_clk_acquire(struct device *dev, struct pm_clock_entry *ce) if (IS_ERR(ce->clk)) { ce->status = PCE_STATUS_ERROR; } else { - clk_prepare(ce->clk); - ce->status = PCE_STATUS_ACQUIRED; - dev_dbg(dev, "Clock %pC con_id %s managed by runtime PM.\n", - ce->clk, ce->con_id); + if (clk_prepare(ce->clk)) { + ce->status = PCE_STATUS_ERROR; + dev_err(dev, "clk_prepare() failed\n"); + } else { + ce->status = PCE_STATUS_ACQUIRED; + dev_dbg(dev, + "Clock %pC con_id %s managed by runtime PM.\n", + ce->clk, ce->con_id); + } } } From 58456488e0e3793f2f95a3d2e2a78b6eba0ad0aa Mon Sep 17 00:00:00 2001 From: Vincent Guittot Date: Wed, 23 Jan 2019 08:50:13 +0100 Subject: [PATCH 4/5] PM-runtime: update accounting_timestamp on enable Initializing accounting_timestamp to something different from 0 during pm_runtime_init() doesn't make sense and puts an artificial ordering constraint between timekeeping_init() and pm_runtime_init(). PM-runtime should start time accounting only when it is enabled and discard the period when disabled. Set accounting_timestamp to now when enabling PM-runtime. Suggested-by: "Rafael J. Wysocki" Signed-off-by: Vincent Guittot Reviewed-by: Ulf Hansson [ rjw: Subject & changelog ] Signed-off-by: Rafael J. Wysocki --- drivers/base/power/runtime.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index a453090c9449..f23b7ecfce3b 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1309,10 +1309,15 @@ void pm_runtime_enable(struct device *dev) spin_lock_irqsave(&dev->power.lock, flags); - if (dev->power.disable_depth > 0) + if (dev->power.disable_depth > 0) { dev->power.disable_depth--; - else + + /* About to enable runtime pm, set accounting_timestamp to now */ + if (!dev->power.disable_depth) + dev->power.accounting_timestamp = jiffies; + } else { dev_warn(dev, "Unbalanced %s!\n", __func__); + } WARN(!dev->power.disable_depth && dev->power.runtime_status == RPM_SUSPENDED && @@ -1509,7 +1514,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; + dev->power.accounting_timestamp = 0; INIT_WORK(&dev->power.work, pm_runtime_work); dev->power.timer_expires = 0; From a08c2a5a31941131c41feaa0429e4c8854cf48f2 Mon Sep 17 00:00:00 2001 From: Thara Gopinath Date: Wed, 23 Jan 2019 08:50:14 +0100 Subject: [PATCH 5/5] PM-runtime: Replace jiffies-based accounting with ktime-based accounting Replace jiffies-based accounting for runtime_active_time and runtime_suspended_time with ktime-based accounting. This makes the runtime debug counters inline with genpd and other PM subsytems which use ktime-based accounting. Timekeeping is initialized before driver_init(). It's only at that time that PM-runtime can be enabled. Signed-off-by: Thara Gopinath [switch from ktime to raw nsec] Signed-off-by: Vincent Guittot Reviewed-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/runtime.c | 17 +++++++++-------- drivers/base/power/sysfs.c | 11 ++++++++--- include/linux/pm.h | 6 +++--- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index f23b7ecfce3b..eb1a3b878e1e 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -66,8 +66,8 @@ static int rpm_suspend(struct device *dev, int rpmflags); */ void update_pm_runtime_accounting(struct device *dev) { - unsigned long now = jiffies; - unsigned long delta; + u64 now = ktime_to_ns(ktime_get()); + u64 delta; delta = now - dev->power.accounting_timestamp; @@ -77,9 +77,9 @@ void update_pm_runtime_accounting(struct device *dev) return; if (dev->power.runtime_status == RPM_SUSPENDED) - dev->power.suspended_jiffies += delta; + dev->power.suspended_time += delta; else - dev->power.active_jiffies += delta; + dev->power.active_time += delta; } static void __update_runtime_status(struct device *dev, enum rpm_status status) @@ -90,16 +90,17 @@ static void __update_runtime_status(struct device *dev, enum rpm_status status) u64 pm_runtime_suspended_time(struct device *dev) { - unsigned long flags, time; + u64 time; + unsigned long flags; spin_lock_irqsave(&dev->power.lock, flags); update_pm_runtime_accounting(dev); - time = dev->power.suspended_jiffies; + time = dev->power.suspended_time; spin_unlock_irqrestore(&dev->power.lock, flags); - return jiffies_to_nsecs(time); + return time; } EXPORT_SYMBOL_GPL(pm_runtime_suspended_time); @@ -1314,7 +1315,7 @@ void pm_runtime_enable(struct device *dev) /* About to enable runtime pm, set accounting_timestamp to now */ if (!dev->power.disable_depth) - dev->power.accounting_timestamp = jiffies; + dev->power.accounting_timestamp = ktime_to_ns(ktime_get()); } else { dev_warn(dev, "Unbalanced %s!\n", __func__); } diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c index d713738ce796..96c8a227610a 100644 --- a/drivers/base/power/sysfs.c +++ b/drivers/base/power/sysfs.c @@ -125,9 +125,12 @@ static ssize_t runtime_active_time_show(struct device *dev, struct device_attribute *attr, char *buf) { int ret; + u64 tmp; spin_lock_irq(&dev->power.lock); update_pm_runtime_accounting(dev); - ret = sprintf(buf, "%i\n", jiffies_to_msecs(dev->power.active_jiffies)); + tmp = dev->power.active_time; + do_div(tmp, NSEC_PER_MSEC); + ret = sprintf(buf, "%llu\n", tmp); spin_unlock_irq(&dev->power.lock); return ret; } @@ -138,10 +141,12 @@ static ssize_t runtime_suspended_time_show(struct device *dev, struct device_attribute *attr, char *buf) { int ret; + u64 tmp; spin_lock_irq(&dev->power.lock); update_pm_runtime_accounting(dev); - ret = sprintf(buf, "%i\n", - jiffies_to_msecs(dev->power.suspended_jiffies)); + tmp = dev->power.suspended_time; + do_div(tmp, NSEC_PER_MSEC); + ret = sprintf(buf, "%llu\n", tmp); spin_unlock_irq(&dev->power.lock); return ret; } diff --git a/include/linux/pm.h b/include/linux/pm.h index 0bd9de116826..3d2cbf947768 100644 --- a/include/linux/pm.h +++ b/include/linux/pm.h @@ -633,9 +633,9 @@ struct dev_pm_info { int runtime_error; int autosuspend_delay; u64 last_busy; - unsigned long active_jiffies; - unsigned long suspended_jiffies; - unsigned long accounting_timestamp; + u64 active_time; + u64 suspended_time; + u64 accounting_timestamp; #endif struct pm_subsys_data *subsys_data; /* Owned by the subsystem. */ void (*set_latency_tolerance)(struct device *, s32);