From a5bef810ad9816a3a8e500d8832be77d52903a12 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Sun, 29 Apr 2012 22:54:17 +0200 Subject: [PATCH 1/8] PM / Domains: Rework default device stop governor function, v2 The existing default device stop governor function for PM domains, default_stop_ok(), is supposed to check whether or not the device's PM QoS latency constraint will be violated if the device is stopped by pm_genpd_runtime_suspend(). However, the computations carried out by it don't reflect the definition of the PM QoS latency constrait in Documentation/ABI/testing/sysfs-devices-power. Make default_stop_ok() follow the definition of the PM QoS latency constrait. In particular, make it take the device's start and stop latencies correctly. Add a new field, effective_constraint_ns, to struct gpd_timing_data and use it to store the difference between the device's PM QoS constraint and its resume latency for use by the device's parent (the effective_constraint_ns values for the children are used for computing the parent's one along with its PM QoS constraint). Remove the break_even_ns field from struct gpd_timing_data, because it's not used any more. Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 1 + drivers/base/power/domain_governor.c | 53 +++++++++++++++++++++++++--- include/linux/pm_domain.h | 2 +- 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 73ce9fbe9839..3c6e94fe058a 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -506,6 +506,7 @@ static int pm_genpd_runtime_suspend(struct device *dev) if (dev_gpd_data(dev)->always_on) return -EBUSY; + dev_gpd_data(dev)->td.effective_constraint_ns = -1; stop_ok = genpd->gov ? genpd->gov->stop_ok : NULL; if (stop_ok && !stop_ok(dev)) return -EBUSY; diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c index 66a265bf5867..a67f157a7a74 100644 --- a/drivers/base/power/domain_governor.c +++ b/drivers/base/power/domain_governor.c @@ -14,6 +14,31 @@ #ifdef CONFIG_PM_RUNTIME +static int dev_update_qos_constraint(struct device *dev, void *data) +{ + s64 *constraint_ns_p = data; + s32 constraint_ns = -1; + + if (dev->power.subsys_data && dev->power.subsys_data->domain_data) + constraint_ns = dev_gpd_data(dev)->td.effective_constraint_ns; + + if (constraint_ns < 0) { + constraint_ns = dev_pm_qos_read_value(dev); + constraint_ns *= NSEC_PER_USEC; + } + if (constraint_ns == 0) + return 0; + + /* + * constraint_ns cannot be negative here, because the device has been + * suspended. + */ + if (constraint_ns < *constraint_ns_p || *constraint_ns_p == 0) + *constraint_ns_p = constraint_ns; + + return 0; +} + /** * default_stop_ok - Default PM domain governor routine for stopping devices. * @dev: Device to check. @@ -21,14 +46,34 @@ bool default_stop_ok(struct device *dev) { struct gpd_timing_data *td = &dev_gpd_data(dev)->td; + s64 constraint_ns; dev_dbg(dev, "%s()\n", __func__); - if (dev->power.max_time_suspended_ns < 0 || td->break_even_ns == 0) - return true; + constraint_ns = dev_pm_qos_read_value(dev); + if (constraint_ns < 0) + return false; - return td->stop_latency_ns + td->start_latency_ns < td->break_even_ns - && td->break_even_ns < dev->power.max_time_suspended_ns; + constraint_ns *= NSEC_PER_USEC; + /* + * We can walk the children without any additional locking, because + * they all have been suspended at this point. + */ + if (!dev->power.ignore_children) + device_for_each_child(dev, &constraint_ns, + dev_update_qos_constraint); + + if (constraint_ns > 0) { + constraint_ns -= td->start_latency_ns; + if (constraint_ns == 0) + return false; + } + td->effective_constraint_ns = constraint_ns; + /* + * The children have been suspended already, so we don't need to take + * their stop latencies into account here. + */ + return constraint_ns > td->stop_latency_ns || constraint_ns == 0; } /** diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index 91f8286106ea..9c25219458c2 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -93,7 +93,7 @@ struct gpd_timing_data { s64 start_latency_ns; s64 save_state_latency_ns; s64 restore_state_latency_ns; - s64 break_even_ns; + s64 effective_constraint_ns; }; struct generic_pm_domain_data { From dd8683e97f12609fb3f8c4318628f0d246542f89 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Sun, 29 Apr 2012 22:54:30 +0200 Subject: [PATCH 2/8] PM / Domains: Rework default domain power off governor function, v2 The existing default domain power down governor function for PM domains, default_power_down_ok(), is supposed to check whether or not the PM QoS latency constraints of the devices in the domain will be violated if the domain is turned off by pm_genpd_poweroff(). However, the computations carried out by it don't reflect the definition of the PM QoS latency constrait in Documentation/ABI/testing/sysfs-devices-power. Make default_power_down_ok() follow the definition of the PM QoS latency constrait. In particular, make it only take latencies into account, because it doesn't matter how much time has elapsed since the domain's devices were suspended for the computation. Remove the break_even_ns and power_off_time fields from struct generic_pm_domain, because they are not necessary any more. Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 2 +- drivers/base/power/domain_governor.c | 71 ++++++++++++++-------------- include/linux/pm_domain.h | 2 - 3 files changed, 36 insertions(+), 39 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 3c6e94fe058a..d03a8c7ad847 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -381,6 +381,7 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd) return 0; } + genpd->max_off_time_ns = -1; if (genpd->gov && genpd->gov->power_down_ok) { if (!genpd->gov->power_down_ok(&genpd->domain)) return -EAGAIN; @@ -443,7 +444,6 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd) } genpd->status = GPD_STATE_POWER_OFF; - genpd->power_off_time = ktime_get(); /* Update PM QoS information for devices in the domain. */ list_for_each_entry_reverse(pdd, &genpd->dev_list, list_node) { diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c index a67f157a7a74..2aae623fd840 100644 --- a/drivers/base/power/domain_governor.c +++ b/drivers/base/power/domain_governor.c @@ -89,7 +89,6 @@ static bool default_power_down_ok(struct dev_pm_domain *pd) struct pm_domain_data *pdd; s64 min_dev_off_time_ns; s64 off_on_time_ns; - ktime_t time_now = ktime_get(); off_on_time_ns = genpd->power_off_latency_ns + genpd->power_on_latency_ns; @@ -118,8 +117,6 @@ static bool default_power_down_ok(struct dev_pm_domain *pd) if (sd_max_off_ns < 0) continue; - sd_max_off_ns -= ktime_to_ns(ktime_sub(time_now, - sd->power_off_time)); /* * Check if the subdomain is allowed to be off long enough for * the current domain to turn off and on (that's how much time @@ -135,52 +132,54 @@ static bool default_power_down_ok(struct dev_pm_domain *pd) min_dev_off_time_ns = -1; list_for_each_entry(pdd, &genpd->dev_list, list_node) { struct gpd_timing_data *td; - struct device *dev = pdd->dev; - s64 dev_off_time_ns; + s64 constraint_ns; - if (!dev->driver || dev->power.max_time_suspended_ns < 0) + if (!pdd->dev->driver) continue; + /* + * Check if the device is allowed to be off long enough for the + * domain to turn off and on (that's how much time it will + * have to wait worst case). + */ td = &to_gpd_data(pdd)->td; - dev_off_time_ns = dev->power.max_time_suspended_ns - - (td->start_latency_ns + td->restore_state_latency_ns + - ktime_to_ns(ktime_sub(time_now, - dev->power.suspend_time))); - if (dev_off_time_ns <= off_on_time_ns) + constraint_ns = td->effective_constraint_ns; + /* default_stop_ok() need not be called before us. */ + if (constraint_ns < 0) { + constraint_ns = dev_pm_qos_read_value(pdd->dev); + constraint_ns *= NSEC_PER_USEC; + } + if (constraint_ns == 0) + continue; + + /* + * constraint_ns cannot be negative here, because the device has + * been suspended. + */ + constraint_ns -= td->restore_state_latency_ns; + if (constraint_ns <= off_on_time_ns) return false; - if (min_dev_off_time_ns > dev_off_time_ns + if (min_dev_off_time_ns > constraint_ns || min_dev_off_time_ns < 0) - min_dev_off_time_ns = dev_off_time_ns; + min_dev_off_time_ns = constraint_ns; } - if (min_dev_off_time_ns < 0) { - /* - * There are no latency constraints, so the domain can spend - * arbitrary time in the "off" state. - */ - genpd->max_off_time_ns = -1; + /* + * If the computed minimum device off time is negative, there are no + * latency constraints, so the domain can spend arbitrary time in the + * "off" state. + */ + if (min_dev_off_time_ns < 0) return true; - } /* - * The difference between the computed minimum delta and the time needed - * to turn the domain on is the maximum theoretical time this domain can - * spend in the "off" state. + * The difference between the computed minimum device off time and the + * time needed to turn the domain on is the maximum theoretical time + * this domain can spend in the "off" state. */ - min_dev_off_time_ns -= genpd->power_on_latency_ns; - - /* - * If the difference between the computed minimum delta and the time - * needed to turn the domain off and back on on is smaller than the - * domain's power break even time, removing power from the domain is not - * worth it. - */ - if (genpd->break_even_ns > - min_dev_off_time_ns - genpd->power_off_latency_ns) - return false; - - genpd->max_off_time_ns = min_dev_off_time_ns; + genpd->max_off_time_ns = min_dev_off_time_ns - + genpd->power_on_latency_ns; return true; } diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index 9c25219458c2..e7ada5ccdfc2 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -70,9 +70,7 @@ struct generic_pm_domain { int (*power_on)(struct generic_pm_domain *domain); s64 power_on_latency_ns; struct gpd_dev_ops dev_ops; - s64 break_even_ns; /* Power break even for the entire domain. */ s64 max_off_time_ns; /* Maximum allowed "suspended" time. */ - ktime_t power_off_time; struct device_node *of_node; /* Node in device tree */ }; From 76e267d822f2913893ad210ba431607aa8e2af94 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Sun, 29 Apr 2012 22:54:36 +0200 Subject: [PATCH 3/8] PM / Runtime: Remove device fields related to suspend time, v2 After the previous changes in default_stop_ok() and default_power_down_ok() for PM domains, there are two fields in struct dev_pm_info that aren't necessary any more, suspend_time and max_time_suspended_ns. Remove those fields along with all of the code that accesses them, which simplifies the runtime PM framework quite a bit. Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 13 ----- drivers/base/power/runtime.c | 103 +---------------------------------- include/linux/pm.h | 2 - include/linux/pm_runtime.h | 3 - 4 files changed, 2 insertions(+), 119 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index d03a8c7ad847..45c2b7f0fe3b 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -445,16 +445,6 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd) genpd->status = GPD_STATE_POWER_OFF; - /* Update PM QoS information for devices in the domain. */ - list_for_each_entry_reverse(pdd, &genpd->dev_list, list_node) { - struct gpd_timing_data *td = &to_gpd_data(pdd)->td; - - pm_runtime_update_max_time_suspended(pdd->dev, - td->start_latency_ns + - td->restore_state_latency_ns + - genpd->power_on_latency_ns); - } - list_for_each_entry(link, &genpd->slave_links, slave_node) { genpd_sd_counter_dec(link->master); genpd_queue_power_off_work(link->master); @@ -515,9 +505,6 @@ static int pm_genpd_runtime_suspend(struct device *dev) if (ret) return ret; - pm_runtime_update_max_time_suspended(dev, - dev_gpd_data(dev)->td.start_latency_ns); - /* * If power.irq_safe is set, this routine will be run with interrupts * off, so it can't use mutexes. diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index bd0f3949bcf9..59894873a3b3 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -282,47 +282,6 @@ static int rpm_callback(int (*cb)(struct device *), struct device *dev) return retval != -EACCES ? retval : -EIO; } -struct rpm_qos_data { - ktime_t time_now; - s64 constraint_ns; -}; - -/** - * rpm_update_qos_constraint - Update a given PM QoS constraint data. - * @dev: Device whose timing data to use. - * @data: PM QoS constraint data to update. - * - * Use the suspend timing data of @dev to update PM QoS constraint data pointed - * to by @data. - */ -static int rpm_update_qos_constraint(struct device *dev, void *data) -{ - struct rpm_qos_data *qos = data; - unsigned long flags; - s64 delta_ns; - int ret = 0; - - spin_lock_irqsave(&dev->power.lock, flags); - - if (dev->power.max_time_suspended_ns < 0) - goto out; - - delta_ns = dev->power.max_time_suspended_ns - - ktime_to_ns(ktime_sub(qos->time_now, dev->power.suspend_time)); - if (delta_ns <= 0) { - ret = -EBUSY; - goto out; - } - - if (qos->constraint_ns > delta_ns || qos->constraint_ns == 0) - qos->constraint_ns = delta_ns; - - out: - spin_unlock_irqrestore(&dev->power.lock, flags); - - return ret; -} - /** * rpm_suspend - Carry out runtime suspend of given device. * @dev: Device to suspend. @@ -349,7 +308,6 @@ static int rpm_suspend(struct device *dev, int rpmflags) { int (*callback)(struct device *); struct device *parent = NULL; - struct rpm_qos_data qos; int retval; trace_rpm_suspend(dev, rpmflags); @@ -445,38 +403,14 @@ static int rpm_suspend(struct device *dev, int rpmflags) goto out; } - qos.constraint_ns = __dev_pm_qos_read_value(dev); - if (qos.constraint_ns < 0) { - /* Negative constraint means "never suspend". */ + if (__dev_pm_qos_read_value(dev) < 0) { + /* Negative PM QoS constraint means "never suspend". */ retval = -EPERM; goto out; } - qos.constraint_ns *= NSEC_PER_USEC; - qos.time_now = ktime_get(); __update_runtime_status(dev, RPM_SUSPENDING); - if (!dev->power.ignore_children) { - if (dev->power.irq_safe) - spin_unlock(&dev->power.lock); - else - spin_unlock_irq(&dev->power.lock); - - retval = device_for_each_child(dev, &qos, - rpm_update_qos_constraint); - - if (dev->power.irq_safe) - spin_lock(&dev->power.lock); - else - spin_lock_irq(&dev->power.lock); - - if (retval) - goto fail; - } - - dev->power.suspend_time = qos.time_now; - dev->power.max_time_suspended_ns = qos.constraint_ns ? : -1; - if (dev->pm_domain) callback = dev->pm_domain->ops.runtime_suspend; else if (dev->type && dev->type->pm) @@ -529,8 +463,6 @@ static int rpm_suspend(struct device *dev, int rpmflags) fail: __update_runtime_status(dev, RPM_ACTIVE); - dev->power.suspend_time = ktime_set(0, 0); - dev->power.max_time_suspended_ns = -1; dev->power.deferred_resume = false; wake_up_all(&dev->power.wait_queue); @@ -704,9 +636,6 @@ static int rpm_resume(struct device *dev, int rpmflags) if (dev->power.no_callbacks) goto no_callback; /* Assume success. */ - dev->power.suspend_time = ktime_set(0, 0); - dev->power.max_time_suspended_ns = -1; - __update_runtime_status(dev, RPM_RESUMING); if (dev->pm_domain) @@ -1369,9 +1298,6 @@ void pm_runtime_init(struct device *dev) setup_timer(&dev->power.suspend_timer, pm_suspend_timer_fn, (unsigned long)dev); - dev->power.suspend_time = ktime_set(0, 0); - dev->power.max_time_suspended_ns = -1; - init_waitqueue_head(&dev->power.wait_queue); } @@ -1389,28 +1315,3 @@ void pm_runtime_remove(struct device *dev) if (dev->power.irq_safe && dev->parent) pm_runtime_put_sync(dev->parent); } - -/** - * pm_runtime_update_max_time_suspended - Update device's suspend time data. - * @dev: Device to handle. - * @delta_ns: Value to subtract from the device's max_time_suspended_ns field. - * - * Update the device's power.max_time_suspended_ns field by subtracting - * @delta_ns from it. The resulting value of power.max_time_suspended_ns is - * never negative. - */ -void pm_runtime_update_max_time_suspended(struct device *dev, s64 delta_ns) -{ - unsigned long flags; - - spin_lock_irqsave(&dev->power.lock, flags); - - if (delta_ns > 0 && dev->power.max_time_suspended_ns > 0) { - if (dev->power.max_time_suspended_ns > delta_ns) - dev->power.max_time_suspended_ns -= delta_ns; - else - dev->power.max_time_suspended_ns = 0; - } - - spin_unlock_irqrestore(&dev->power.lock, flags); -} diff --git a/include/linux/pm.h b/include/linux/pm.h index 715305e05123..f067e60a3832 100644 --- a/include/linux/pm.h +++ b/include/linux/pm.h @@ -544,8 +544,6 @@ struct dev_pm_info { unsigned long active_jiffies; unsigned long suspended_jiffies; unsigned long accounting_timestamp; - ktime_t suspend_time; - s64 max_time_suspended_ns; struct dev_pm_qos_request *pq_req; #endif struct pm_subsys_data *subsys_data; /* Owned by the subsystem. */ diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h index 609daae7a014..f271860c78d5 100644 --- a/include/linux/pm_runtime.h +++ b/include/linux/pm_runtime.h @@ -150,9 +150,6 @@ static inline void pm_runtime_set_autosuspend_delay(struct device *dev, static inline unsigned long pm_runtime_autosuspend_expiration( struct device *dev) { return 0; } -static inline void pm_runtime_update_max_time_suspended(struct device *dev, - s64 delta_ns) {} - #endif /* !CONFIG_PM_RUNTIME */ static inline int pm_runtime_idle(struct device *dev) From 23e0fc5ae64925e0ff1b6221b83dff1b217545df Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Sun, 29 Apr 2012 22:54:47 +0200 Subject: [PATCH 4/8] PM / QoS: Create device constraints objects on notifier registration The current behavior of dev_pm_qos_add_notifier() makes device PM QoS notifiers less than useful. Namely, it silently returns success when called before any PM QoS constraints are added for the device, so the caller will assume that the notifier has been registered, but when someone actually adds some nontrivial constraints for the device eventually, the previous callers of dev_pm_qos_add_notifier() will not know about that and their notifier routines will not be executed (contrary to their expectations). To address this problem make dev_pm_qos_add_notifier() create the constraints object for the device if it is not present when the routine is called. Signed-off-by: Rafael J. Wysocki Acked-by : markgross --- drivers/base/power/qos.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c index 71855570922d..fd849a2c4fa8 100644 --- a/drivers/base/power/qos.c +++ b/drivers/base/power/qos.c @@ -352,21 +352,26 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_remove_request); * * Will register the notifier into a notification chain that gets called * upon changes to the target value for the device. + * + * If the device's constraints object doesn't exist when this routine is called, + * it will be created (or error code will be returned if that fails). */ int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier) { - int retval = 0; + int ret = 0; mutex_lock(&dev_pm_qos_mtx); - /* Silently return if the constraints object is not present. */ - if (dev->power.constraints) - retval = blocking_notifier_chain_register( - dev->power.constraints->notifiers, - notifier); + if (!dev->power.constraints) + ret = dev->power.power_state.event != PM_EVENT_INVALID ? + dev_pm_qos_constraints_allocate(dev) : -ENODEV; + + if (!ret) + ret = blocking_notifier_chain_register( + dev->power.constraints->notifiers, notifier); mutex_unlock(&dev_pm_qos_mtx); - return retval; + return ret; } EXPORT_SYMBOL_GPL(dev_pm_qos_add_notifier); From efa6902501ffc87d69bfb10b8a09b7d6ee222d77 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 1 May 2012 21:33:53 +0200 Subject: [PATCH 5/8] PM / Domains: Make device removal more straightforward The removal of a device from a PM domain doesn't have to browse the domain's device list, because it can check directly if the device belongs to the given domain. Moreover, it should clear the domain_data pointer in dev->power.subsys_data, because dev_pm_put_subsys_data(dev) may not remove dev->power.subsys_data and the stale domain data pointer may cause problems to happen. Rework pm_genpd_remove_device() taking the above observations into account. Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 45c2b7f0fe3b..6ae5672c35ab 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1279,11 +1279,13 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd, struct device *dev) { struct pm_domain_data *pdd; - int ret = -EINVAL; + int ret = 0; dev_dbg(dev, "%s()\n", __func__); - if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev)) + if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev) + || IS_ERR_OR_NULL(dev->pm_domain) + || pd_to_genpd(dev->pm_domain) != genpd) return -EINVAL; genpd_acquire_lock(genpd); @@ -1293,21 +1295,14 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd, goto out; } - list_for_each_entry(pdd, &genpd->dev_list, list_node) { - if (pdd->dev != dev) - continue; + dev->pm_domain = NULL; + pdd = dev->power.subsys_data->domain_data; + list_del_init(&pdd->list_node); + dev->power.subsys_data->domain_data = NULL; + dev_pm_put_subsys_data(dev); + kfree(to_gpd_data(pdd)); - list_del_init(&pdd->list_node); - pdd->dev = NULL; - dev_pm_put_subsys_data(dev); - dev->pm_domain = NULL; - kfree(to_gpd_data(pdd)); - - genpd->device_count--; - - ret = 0; - break; - } + genpd->device_count--; out: genpd_release_lock(genpd); From 6ff7bb0d02f82968be13937c03e93b6c090229df Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 1 May 2012 21:34:07 +0200 Subject: [PATCH 6/8] PM / Domains: Cache device stop and domain power off governor results, v3 The results of the default device stop and domain power off governor functions for generic PM domains, default_stop_ok() and default_power_down_ok(), depend only on the timing data of devices, which are static, and on their PM QoS constraints. Thus, in theory, these functions only need to carry out their computations, which may be time consuming in general, when it is known that the PM QoS constraint of at least one of the devices in question has changed. Use the PM QoS notifiers of devices to implement that. First, introduce new fields, constraint_changed and max_off_time_changed, into struct gpd_timing_data and struct generic_pm_domain, respectively, and register a PM QoS notifier function when adding a device into a domain that will set those fields to 'true' whenever the device's PM QoS constraint is modified. Second, make default_stop_ok() and default_power_down_ok() use those fields to decide whether or not to carry out their computations from scratch. The device and PM domain hierarchies are taken into account in that and the expense is that the changes of PM QoS constraints of suspended devices will not be taken into account immediately, which isn't guaranteed anyway in general. Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 120 +++++++++++++++++++++++---- drivers/base/power/domain_governor.c | 45 +++++++++- include/linux/pm_domain.h | 7 ++ 3 files changed, 153 insertions(+), 19 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 6ae5672c35ab..cde5983de6c2 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -38,11 +39,13 @@ ktime_t __start = ktime_get(); \ type __retval = GENPD_DEV_CALLBACK(genpd, type, callback, dev); \ s64 __elapsed = ktime_to_ns(ktime_sub(ktime_get(), __start)); \ - struct generic_pm_domain_data *__gpd_data = dev_gpd_data(dev); \ - if (__elapsed > __gpd_data->td.field) { \ - __gpd_data->td.field = __elapsed; \ + struct gpd_timing_data *__td = &dev_gpd_data(dev)->td; \ + if (!__retval && __elapsed > __td->field) { \ + __td->field = __elapsed; \ dev_warn(dev, name " latency exceeded, new value %lld ns\n", \ __elapsed); \ + genpd->max_off_time_changed = true; \ + __td->constraint_changed = true; \ } \ __retval; \ }) @@ -211,6 +214,7 @@ int __pm_genpd_poweron(struct generic_pm_domain *genpd) elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start)); if (elapsed_ns > genpd->power_on_latency_ns) { genpd->power_on_latency_ns = elapsed_ns; + genpd->max_off_time_changed = true; if (genpd->name) pr_warning("%s: Power-on latency exceeded, " "new value %lld ns\n", genpd->name, @@ -247,6 +251,53 @@ int pm_genpd_poweron(struct generic_pm_domain *genpd) #ifdef CONFIG_PM_RUNTIME +static int genpd_dev_pm_qos_notifier(struct notifier_block *nb, + unsigned long val, void *ptr) +{ + struct generic_pm_domain_data *gpd_data; + struct device *dev; + + gpd_data = container_of(nb, struct generic_pm_domain_data, nb); + + mutex_lock(&gpd_data->lock); + dev = gpd_data->base.dev; + if (!dev) { + mutex_unlock(&gpd_data->lock); + return NOTIFY_DONE; + } + mutex_unlock(&gpd_data->lock); + + for (;;) { + struct generic_pm_domain *genpd; + struct pm_domain_data *pdd; + + spin_lock_irq(&dev->power.lock); + + pdd = dev->power.subsys_data ? + dev->power.subsys_data->domain_data : NULL; + if (pdd) { + to_gpd_data(pdd)->td.constraint_changed = true; + genpd = dev_to_genpd(dev); + } else { + genpd = ERR_PTR(-ENODATA); + } + + spin_unlock_irq(&dev->power.lock); + + if (!IS_ERR(genpd)) { + mutex_lock(&genpd->lock); + genpd->max_off_time_changed = true; + mutex_unlock(&genpd->lock); + } + + dev = dev->parent; + if (!dev || dev->power.ignore_children) + break; + } + + return NOTIFY_DONE; +} + /** * __pm_genpd_save_device - Save the pre-suspend state of a device. * @pdd: Domain data of the device to save the state of. @@ -381,7 +432,6 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd) return 0; } - genpd->max_off_time_ns = -1; if (genpd->gov && genpd->gov->power_down_ok) { if (!genpd->gov->power_down_ok(&genpd->domain)) return -EAGAIN; @@ -436,6 +486,7 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd) elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start)); if (elapsed_ns > genpd->power_off_latency_ns) { genpd->power_off_latency_ns = elapsed_ns; + genpd->max_off_time_changed = true; if (genpd->name) pr_warning("%s: Power-off latency exceeded, " "new value %lld ns\n", genpd->name, @@ -496,7 +547,6 @@ static int pm_genpd_runtime_suspend(struct device *dev) if (dev_gpd_data(dev)->always_on) return -EBUSY; - dev_gpd_data(dev)->td.effective_constraint_ns = -1; stop_ok = genpd->gov ? genpd->gov->stop_ok : NULL; if (stop_ok && !stop_ok(dev)) return -EBUSY; @@ -601,6 +651,12 @@ void pm_genpd_poweroff_unused(void) #else +static inline int genpd_dev_pm_qos_notifier(struct notifier_block *nb, + unsigned long val, void *ptr) +{ + return NOTIFY_DONE; +} + static inline void genpd_power_off_work_fn(struct work_struct *work) {} #define pm_genpd_runtime_suspend NULL @@ -1197,6 +1253,14 @@ int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev, if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev)) return -EINVAL; + gpd_data = kzalloc(sizeof(*gpd_data), GFP_KERNEL); + if (!gpd_data) + return -ENOMEM; + + mutex_init(&gpd_data->lock); + gpd_data->nb.notifier_call = genpd_dev_pm_qos_notifier; + dev_pm_qos_add_notifier(dev, &gpd_data->nb); + genpd_acquire_lock(genpd); if (genpd->status == GPD_STATE_POWER_OFF) { @@ -1215,26 +1279,35 @@ int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev, goto out; } - gpd_data = kzalloc(sizeof(*gpd_data), GFP_KERNEL); - if (!gpd_data) { - ret = -ENOMEM; - goto out; - } - genpd->device_count++; + genpd->max_off_time_changed = true; - dev->pm_domain = &genpd->domain; dev_pm_get_subsys_data(dev); + + mutex_lock(&gpd_data->lock); + spin_lock_irq(&dev->power.lock); + dev->pm_domain = &genpd->domain; dev->power.subsys_data->domain_data = &gpd_data->base; gpd_data->base.dev = dev; - gpd_data->need_restore = false; list_add_tail(&gpd_data->base.list_node, &genpd->dev_list); + gpd_data->need_restore = false; if (td) gpd_data->td = *td; + gpd_data->td.constraint_changed = true; + gpd_data->td.effective_constraint_ns = -1; + spin_unlock_irq(&dev->power.lock); + mutex_unlock(&gpd_data->lock); + + genpd_release_lock(genpd); + + return 0; + out: genpd_release_lock(genpd); + dev_pm_qos_remove_notifier(dev, &gpd_data->nb); + kfree(gpd_data); return ret; } @@ -1278,6 +1351,7 @@ int __pm_genpd_of_add_device(struct device_node *genpd_node, struct device *dev, int pm_genpd_remove_device(struct generic_pm_domain *genpd, struct device *dev) { + struct generic_pm_domain_data *gpd_data; struct pm_domain_data *pdd; int ret = 0; @@ -1295,14 +1369,27 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd, goto out; } + genpd->device_count--; + genpd->max_off_time_changed = true; + + spin_lock_irq(&dev->power.lock); dev->pm_domain = NULL; pdd = dev->power.subsys_data->domain_data; list_del_init(&pdd->list_node); dev->power.subsys_data->domain_data = NULL; - dev_pm_put_subsys_data(dev); - kfree(to_gpd_data(pdd)); + spin_unlock_irq(&dev->power.lock); - genpd->device_count--; + gpd_data = to_gpd_data(pdd); + mutex_lock(&gpd_data->lock); + pdd->dev = NULL; + mutex_unlock(&gpd_data->lock); + + genpd_release_lock(genpd); + + dev_pm_qos_remove_notifier(dev, &gpd_data->nb); + kfree(gpd_data); + dev_pm_put_subsys_data(dev); + return 0; out: genpd_release_lock(genpd); @@ -1673,6 +1760,7 @@ void pm_genpd_init(struct generic_pm_domain *genpd, genpd->resume_count = 0; genpd->device_count = 0; genpd->max_off_time_ns = -1; + genpd->max_off_time_changed = true; 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; diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c index 2aae623fd840..3a5c5346bc47 100644 --- a/drivers/base/power/domain_governor.c +++ b/drivers/base/power/domain_governor.c @@ -46,18 +46,34 @@ static int dev_update_qos_constraint(struct device *dev, void *data) bool default_stop_ok(struct device *dev) { struct gpd_timing_data *td = &dev_gpd_data(dev)->td; + unsigned long flags; s64 constraint_ns; dev_dbg(dev, "%s()\n", __func__); - constraint_ns = dev_pm_qos_read_value(dev); + spin_lock_irqsave(&dev->power.lock, flags); + + if (!td->constraint_changed) { + bool ret = td->cached_stop_ok; + + spin_unlock_irqrestore(&dev->power.lock, flags); + return ret; + } + td->constraint_changed = false; + td->cached_stop_ok = false; + td->effective_constraint_ns = -1; + constraint_ns = __dev_pm_qos_read_value(dev); + + spin_unlock_irqrestore(&dev->power.lock, flags); + if (constraint_ns < 0) return false; constraint_ns *= NSEC_PER_USEC; /* * We can walk the children without any additional locking, because - * they all have been suspended at this point. + * they all have been suspended at this point and their + * effective_constraint_ns fields won't be modified in parallel with us. */ if (!dev->power.ignore_children) device_for_each_child(dev, &constraint_ns, @@ -69,11 +85,13 @@ bool default_stop_ok(struct device *dev) return false; } td->effective_constraint_ns = constraint_ns; + td->cached_stop_ok = constraint_ns > td->stop_latency_ns || + constraint_ns == 0; /* * The children have been suspended already, so we don't need to take * their stop latencies into account here. */ - return constraint_ns > td->stop_latency_ns || constraint_ns == 0; + return td->cached_stop_ok; } /** @@ -90,6 +108,25 @@ static bool default_power_down_ok(struct dev_pm_domain *pd) s64 min_dev_off_time_ns; s64 off_on_time_ns; + if (genpd->max_off_time_changed) { + struct gpd_link *link; + + /* + * We have to invalidate the cached results for the masters, so + * use the observation that default_power_down_ok() is not + * going to be called for any master until this instance + * returns. + */ + list_for_each_entry(link, &genpd->slave_links, slave_node) + link->master->max_off_time_changed = true; + + genpd->max_off_time_changed = false; + genpd->cached_power_down_ok = false; + genpd->max_off_time_ns = -1; + } else { + return genpd->cached_power_down_ok; + } + off_on_time_ns = genpd->power_off_latency_ns + genpd->power_on_latency_ns; /* @@ -165,6 +202,8 @@ static bool default_power_down_ok(struct dev_pm_domain *pd) min_dev_off_time_ns = constraint_ns; } + genpd->cached_power_down_ok = true; + /* * If the computed minimum device off time is negative, there are no * latency constraints, so the domain can spend arbitrary time in the diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index e7ada5ccdfc2..1e994eeacdf3 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -14,6 +14,7 @@ #include #include #include +#include enum gpd_status { GPD_STATE_ACTIVE = 0, /* PM domain is active */ @@ -71,6 +72,8 @@ struct generic_pm_domain { s64 power_on_latency_ns; struct gpd_dev_ops dev_ops; s64 max_off_time_ns; /* Maximum allowed "suspended" time. */ + bool max_off_time_changed; + bool cached_power_down_ok; struct device_node *of_node; /* Node in device tree */ }; @@ -92,12 +95,16 @@ struct gpd_timing_data { s64 save_state_latency_ns; s64 restore_state_latency_ns; s64 effective_constraint_ns; + bool constraint_changed; + bool cached_stop_ok; }; struct generic_pm_domain_data { struct pm_domain_data base; struct gpd_dev_ops ops; struct gpd_timing_data td; + struct notifier_block nb; + struct mutex lock; bool need_restore; bool always_on; }; From 4fcac10d28e7a046120b51a106b19082d2e57401 Mon Sep 17 00:00:00 2001 From: Huang Ying Date: Mon, 7 May 2012 21:35:45 +0200 Subject: [PATCH 7/8] PM / Domains: Fix link checking when add subdomain Current pm_genpd_add_subdomain() will allow duplicated link between master and slave domain. This patch fixed it. Because when current pm_genpd_add_subdomain() checks whether the link between the master and slave generic PM domain already exists, slave_links instead of master_links of master domain is used. Signed-off-by: Huang Ying Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index cde5983de6c2..c3eaa08a8f96 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1448,7 +1448,7 @@ int pm_genpd_add_subdomain(struct generic_pm_domain *genpd, goto out; } - list_for_each_entry(link, &genpd->slave_links, slave_node) { + list_for_each_entry(link, &genpd->master_links, master_node) { if (link->slave == subdomain && link->master == genpd) { ret = -EINVAL; goto out; From b723b0eb91e08a0ee9a401c0b22c0d52966d9daa Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 7 May 2012 22:00:59 +0200 Subject: [PATCH 8/8] PM / Domains: Fix computation of maximum domain off time The default domain power off governor function for generic PM domains, default_power_down_ok(), may violate subdomain maximum off time limit by allowing the master domain to be off for too long. Namely, it only finds the minium of all device maximum off times over the domain's devices and uses that to compute the domain's maximum off time, but it should do the same for the subdomains. Fix this problem by modifying default_power_down_ok() to compute the given domain's maximum off time as the difference between the minimum off time over all devices and subdomains in the domain and its power on latency. Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain_governor.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c index 3a5c5346bc47..28dee3053f1f 100644 --- a/drivers/base/power/domain_governor.c +++ b/drivers/base/power/domain_governor.c @@ -105,7 +105,7 @@ static bool default_power_down_ok(struct dev_pm_domain *pd) struct generic_pm_domain *genpd = pd_to_genpd(pd); struct gpd_link *link; struct pm_domain_data *pdd; - s64 min_dev_off_time_ns; + s64 min_off_time_ns; s64 off_on_time_ns; if (genpd->max_off_time_changed) { @@ -142,6 +142,7 @@ static bool default_power_down_ok(struct dev_pm_domain *pd) to_gpd_data(pdd)->td.save_state_latency_ns; } + min_off_time_ns = -1; /* * Check if subdomains can be off for enough time. * @@ -161,12 +162,14 @@ static bool default_power_down_ok(struct dev_pm_domain *pd) */ if (sd_max_off_ns <= off_on_time_ns) return false; + + if (min_off_time_ns > sd_max_off_ns || min_off_time_ns < 0) + min_off_time_ns = sd_max_off_ns; } /* * Check if the devices in the domain can be off enough time. */ - min_dev_off_time_ns = -1; list_for_each_entry(pdd, &genpd->dev_list, list_node) { struct gpd_timing_data *td; s64 constraint_ns; @@ -197,9 +200,8 @@ static bool default_power_down_ok(struct dev_pm_domain *pd) if (constraint_ns <= off_on_time_ns) return false; - if (min_dev_off_time_ns > constraint_ns - || min_dev_off_time_ns < 0) - min_dev_off_time_ns = constraint_ns; + if (min_off_time_ns > constraint_ns || min_off_time_ns < 0) + min_off_time_ns = constraint_ns; } genpd->cached_power_down_ok = true; @@ -209,16 +211,15 @@ static bool default_power_down_ok(struct dev_pm_domain *pd) * latency constraints, so the domain can spend arbitrary time in the * "off" state. */ - if (min_dev_off_time_ns < 0) + if (min_off_time_ns < 0) return true; /* - * The difference between the computed minimum device off time and the - * time needed to turn the domain on is the maximum theoretical time - * this domain can spend in the "off" state. + * The difference between the computed minimum subdomain or device off + * time and the time needed to turn the domain on is the maximum + * theoretical time this domain can spend in the "off" state. */ - genpd->max_off_time_ns = min_dev_off_time_ns - - genpd->power_on_latency_ns; + genpd->max_off_time_ns = min_off_time_ns - genpd->power_on_latency_ns; return true; }