PM / Domains: Generic PM domains for CPUs
Subsystems (11)
v2 cover PM / Domains: Generic PM domains for CPUs
Changes since v1 [1]:
- Address review comments on v1.
- Incorporate Kevin's arch/arm/domain.c changes
- Drop drivers/base/power/cpu_domain.c
- Rebase on top of linux-next (to date)
- Reference implemantation added.
This patchset fashions CPU clusters as generic PM domains. CPUs in most new SOCs are grouped together as a cluster, along with some supporting hardware, GIC, L2 caches etc. When all these devices in a cluster is powered off, the power domain may also be powered off.
GenPD framework provides the necessary backend to build a cluster hierarchy through devices, domains and nested domains. When devices and sub-domains of a genpd are suspended, the genpd may also be suspended and resumed before the first of the devices resumes. This works well for most devices and domains. However, not all devices operate in a process context. Devices that suspend and resume when IRQs are disabled, are defined as IRQ safe devices and domains that have IRQ safe devices are never powered off. The reason being, genpd uses mutexes to lock domains and IRQ safe devices may operate in atomic contexts, therefore cannot sleep. So even when all the devices are suspended, the domain remains powered on.
Since CPUIdle frameworks calls the CPU's idle functions with IRQs disabled, the CPU devices have to be IRQ safe. A cluster fashioned as genpd be attached to IRQ safe CPU devices and possibly other devices that may or may not be IRQ safe. However, even when all these devices are suspended, the genpd would never be powered off. This is a significant part of the cluster's idle power consumption.
These patches attempt to save some runtime idle power of CPU clusters by allowing the genpd to suspend even when the attached devices are IRQ safe. The approach here is simple, allow genpd domains to specify the type of locking the domain would use. A genpd that can suspend and resume in an IRQ safe context, would initialize a spinlock as the genpd lock instead of a mutex. Therefore, IRQ safe devices may initiate a genpd suspend when the last active device goes idle. In a CPU domain, the last CPU powering down, may now program the domain hardware to suspend, when the CPU enters idle. Thus when all the CPUs, caches, peripheral cluster hardware are in idle, the cluster domain would also be in a low power state.
These patches also bring in a generalized way to represent CPU clusters on an ARM platform. Platform drivers specify the CPU domain providers in their DT and a genpd is initialized for every domain provider specified for the CPU. Optionally, the platform may register their callback to perform SoC specific functions when the domains are suspended/resumed. Currently, only CPU devices are attached to the CPU genpd by default. Devices representing L2, GIC, peripheral hardware that depend on the PM domain, may also be attached to the genpd in the future.
This RFC is based on Ulf's clean up of genpd intermediate states [2] and Kevin's WIP on generic PM domains for ARM CPU cluster. CPU domain definition for the QCOM DB platform is provided as reference (patch 11/16 onwards).
Thanks,
Lina
[1]. http://www.spinics.net/lists/arm-kernel/msg423430.html
[2]. https://patches.linaro.org/50047/
Kevin Hilman (1):
WIP: ARM: PM domains for CPUs/clusters
Lina Iyer (15):
PM / Domains: Allocate memory outside domain locks
PM / Domains: Remove dev->driver check for runtime PM
PM / Domains: Support IRQ safe PM domains
arm: domain: Remove hack to add dev->driver.
arm: domain: Make CPU genpd IRQ safe
arm: domain: Synchronize CPU device runtime PM usage with idle state
arm: domain: Handle CPU online reference counting
arm: domain: Add platform callbacks for domain power on/off
drivers: cpuidle: Add runtime PM support for CPUIdle
drivers: qcom: spm: Support cache and coherency SPMs
drivers: qcom: spm: Enable runtime suspend/resume of CPU PM domain
drivers: qcom: spm: Add 8084 L2 SPM register data
arm: dts: Add L2 power-controller device bindings for APQ8084
arm: dts: Add power domain device bindings for APQ8084
drivers: qcom: Enable genpd on selecting QCOM_PM
.../devicetree/bindings/arm/msm/qcom,saw2.txt | 1 + Documentation/power/devices.txt | 11 +- arch/arm/boot/dts/exynos5420.dtsi | 18 ++ arch/arm/boot/dts/qcom-apq8084.dtsi | 10 +- arch/arm/include/asm/cpu.h | 1 - arch/arm/include/asm/pm_domain.h | 27 +++ arch/arm/kernel/Makefile | 1 + arch/arm/kernel/domains.c | 243 +++++++++++++++++++++ drivers/base/power/domain.c | 230 ++++++++++++++----- drivers/cpuidle/cpuidle-arm.c | 11 + drivers/soc/qcom/Kconfig | 3 + drivers/soc/qcom/spm.c | 156 ++++++++++++- include/linux/pm_domain.h | 11 +- 13 files changed, 649 insertions(+), 74 deletions(-) create mode 100644 arch/arm/include/asm/pm_domain.h create mode 100644 arch/arm/kernel/domains.c
Signature · 1 line
2.1.4
RFC PATCH v2 1/16 PM / Domains: Allocate memory outside domain locks
In preparation for supporting IRQ-safe domains, allocate domain data outside the domain locks. These functions are not called in an atomic context, so we can always allocate memory using GFP_KERNEL. By allocating memory before the locks, we can safely lock the domain using spinlocks instead of mutexes.
---
Changes in v2:
- Modify commit text
--- drivers/base/power/domain.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-)
drivers/base/power/domain.c +13 -14
@@ -1379,13 +1379,17 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd,int pm_genpd_add_subdomain (struct generic_pm_domain *genpd,struct generic_pm_domain *subdomain) { -struct gpd_link *link; +struct gpd_link *link, *itr;int ret =0 ;if (IS_ERR_OR_NULL (genpd) ||IS_ERR_OR_NULL (subdomain) || genpd == subdomain)return -EINVAL ; + link =kzalloc (sizeof (*link),GFP_KERNEL ); +if (!link) +return -ENOMEM ; +mutex_lock (&genpd->lock);mutex_lock_nested (&subdomain->lock,SINGLE_DEPTH_NESTING );
@@ -1395,18 +1399,13 @@ int pm_genpd_add_subdomain(struct generic_pm_domain *genpd, goto out; } -list_for_each_entry (link, &genpd->master_links, master_node) { -if (link->slave == subdomain && link->master == genpd) { +list_for_each_entry (itr, &genpd->master_links, master_node) { +if (itr->slave == subdomain && itr->master == genpd) { ret = -EINVAL ; goto out; } } - link =kzalloc (sizeof (*link),GFP_KERNEL ); -if (!link) { - ret = -ENOMEM ; - goto out; - } link->master = genpd;list_add_tail (&link->master_node, &genpd->master_links); link->slave = subdomain;
@@ -1508,17 +1507,17 @@ int pm_genpd_attach_cpuidle(struct generic_pm_domain *genpd, int state)if (IS_ERR_OR_NULL (genpd) || state <0 )return -EINVAL ; + cpuidle_data =kzalloc (sizeof (*cpuidle_data),GFP_KERNEL ); +if (!cpuidle_data) +return -ENOMEM ; +mutex_lock (&genpd->lock);if (genpd->cpuidle_data) { ret = -EEXIST ; - goto out; - } - cpuidle_data =kzalloc (sizeof (*cpuidle_data),GFP_KERNEL ); -if (!cpuidle_data) { - ret = -ENOMEM ; - goto out; + goto err_drv; } + cpuidle_drv =cpuidle_driver_ref ();if (!cpuidle_drv) { ret = -ENODEV ;
Signature · 1 line
2.1.4
RFC PATCH v2 1/16 PM / Domains: Allocate memory outside domain locks
Quoted 41 lines · Lina Iyer
In preparation for supporting IRQ-safe domains, allocate domain data outside the domain locks. These functions are not called in an atomic context, so we can always allocate memory using
GFP_KERNEL. By allocating memory before the locks, we can safely lock the domain using spinlocks instead of mutexes.Cc Ulf Hansson <[email protected]>Cc Rafael J. Wysocki <[email protected]>Cc Kevin Hilman <[email protected]>Signed-off-by Lina Iyer <[email protected]>---
Changes in v2:
- Modify commit text--- drivers/base/power/domain.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-)drivers/base/power/domain.c +5
-1@@ -1379,13 +1379,17 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd,int pm_genpd_add_subdomain (struct generic_pm_domain *genpd,struct generic_pm_domain *subdomain) { -struct gpd_link *link; +struct gpd_link *link, *itr;int ret =0 ;if (IS_ERR_OR_NULL (genpd) ||IS_ERR_OR_NULL (subdomain) || genpd == subdomain)return -EINVAL ; + link =kzalloc (sizeof (*link),GFP_KERNEL ); +if (!link) +return -ENOMEM ; +mutex_lock (&genpd->lock);mutex_lock_nested (&subdomain->lock,SINGLE_DEPTH_NESTING );
drivers/base/power/domain.c int pm_genpd_add_subdomain(struct generic_pm_domain *genpd, · line 1405
Quoted 9 lines
drivers/base/power/domain.c +2
-2@@ -1395,18 +1399,13 @@ int pm_genpd_add_subdomain(struct generic_pm_domain *genpd, goto out; } -list_for_each_entry (link, &genpd->master_links, master_node) { -if (link->slave == subdomain && link->master == genpd) { +list_for_each_entry (itr, &genpd->master_links, master_node) { +if (itr->slave == subdomain && itr->master == genpd) { ret = -EINVAL ; goto out;
The same comment as in previous version - where is the kfree()? You moved the allocation so all error paths have to be checked and adjusted. Not just one that I pointed in previous review.
Quoted 20 lines
drivers/base/power/domain.c +4
-5} } - link =kzalloc (sizeof (*link),GFP_KERNEL ); -if (!link) { - ret = -ENOMEM ; - goto out; - } link->master = genpd;list_add_tail (&link->master_node, &genpd->master_links); link->slave = subdomain;@@ -1508,17 +1507,17 @@ int pm_genpd_attach_cpuidle(struct generic_pm_domain *genpd, int state)if (IS_ERR_OR_NULL (genpd) || state <0 )return -EINVAL ; + cpuidle_data =kzalloc (sizeof (*cpuidle_data),GFP_KERNEL ); +if (!cpuidle_data) +return -ENOMEM ; +mutex_lock (&genpd->lock);
drivers/base/power/domain.c int pm_genpd_attach_cpuidle(struct generic_pm_domain *genpd, int state) · line 1518
Quoted 10 lines
drivers/base/power/domain.c +1
-6if (genpd->cpuidle_data) { ret = -EEXIST ; - goto out; - } - cpuidle_data =kzalloc (sizeof (*cpuidle_data),GFP_KERNEL ); -if (!cpuidle_data) { - ret = -ENOMEM ; - goto out; + goto err_drv;
Is the mutex freed in this exit path?
Best regards,
Krzysztof
Quoted 6 lines
drivers/base/power/domain.c +1
-0} + cpuidle_drv =cpuidle_driver_ref ();if (!cpuidle_drv) { ret = -ENODEV ;
RFC PATCH v2 1/16 PM / Domains: Allocate memory outside domain locks
Quoted 45 lines · Krzysztof Kozlowski
Quoted 44 lines · Lina Iyer
In preparation for supporting IRQ-safe domains, allocate domain data outside the domain locks. These functions are not called in an atomic context, so we can always allocate memory using
GFP_KERNEL. By allocating memory before the locks, we can safely lock the domain using spinlocks instead of mutexes.Cc Ulf Hansson <[email protected]>Cc Rafael J. Wysocki <[email protected]>Cc Kevin Hilman <[email protected]>Signed-off-by Lina Iyer <[email protected]>---
Changes in v2:
- Modify commit text--- drivers/base/power/domain.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-)drivers/base/power/domain.c +5
-1@@ -1379,13 +1379,17 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd,int pm_genpd_add_subdomain (struct generic_pm_domain *genpd,struct generic_pm_domain *subdomain) { -struct gpd_link *link; +struct gpd_link *link, *itr;int ret =0 ;if (IS_ERR_OR_NULL (genpd) ||IS_ERR_OR_NULL (subdomain) || genpd == subdomain)return -EINVAL ; + link =kzalloc (sizeof (*link),GFP_KERNEL ); +if (!link) +return -ENOMEM ; +mutex_lock (&genpd->lock);mutex_lock_nested (&subdomain->lock,SINGLE_DEPTH_NESTING );@@ -1395,18 +1399,13 @@ int pm_genpd_add_subdomain(struct generic_pm_domain *genpd, goto out; }
Quoted 11 lines
drivers/base/power/domain.c int pm_genpd_add_subdomain(struct generic_pm_domain *genpd, · line 1405
Quoted 6 lines
drivers/base/power/domain.c +2
-2-list_for_each_entry (link, &genpd->master_links, master_node) { -if (link->slave == subdomain && link->master == genpd) { +list_for_each_entry (itr, &genpd->master_links, master_node) { +if (itr->slave == subdomain && itr->master == genpd) { ret = -EINVAL ; goto out;The same comment as in previous version - where is the
kfree()? You moved the allocation so all error paths have to be checked and adjusted. Not just one that I pointed in previous review.
Sorry, didnt notice that, will fix this. I seem to have addressed the other fone.
Quoted 21 lines
Quoted 21 lines
drivers/base/power/domain.c +4
-5} } - link =kzalloc (sizeof (*link),GFP_KERNEL ); -if (!link) { - ret = -ENOMEM ; - goto out; - } link->master = genpd;list_add_tail (&link->master_node, &genpd->master_links); link->slave = subdomain;@@ -1508,17 +1507,17 @@ int pm_genpd_attach_cpuidle(struct generic_pm_domain *genpd, int state)if (IS_ERR_OR_NULL (genpd) || state <0 )return -EINVAL ; + cpuidle_data =kzalloc (sizeof (*cpuidle_data),GFP_KERNEL ); +if (!cpuidle_data) +return -ENOMEM ; +mutex_lock (&genpd->lock);
Quoted 12 lines
drivers/base/power/domain.c int pm_genpd_attach_cpuidle(struct generic_pm_domain *genpd, int state) · line 1518
Quoted 9 lines
drivers/base/power/domain.c +1
-6if (genpd->cpuidle_data) { ret = -EEXIST ; - goto out; - } - cpuidle_data =kzalloc (sizeof (*cpuidle_data),GFP_KERNEL ); -if (!cpuidle_data) { - ret = -ENOMEM ; - goto out; + goto err_drv;Is the mutex freed in this exit path?
Yes, err_drv jumps to out, which frees the mutex.
Thank you so much for the review.
-- Lina
Quoted 7 lines
Quoted 6 lines
drivers/base/power/domain.c +1
-0} + cpuidle_drv =cpuidle_driver_ref ();if (!cpuidle_drv) { ret = -ENODEV ;
RFC PATCH v2 2/16 PM / Domains: Remove dev->driver check for runtime PM
In reply to Lina IyerRemove check for driver of a device, for runtime PM. Device may be suspended without an explicit driver. This check seems to be vestigial and incorrect in the current context.
--- drivers/base/power/domain.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/base/power/domain.c +1 -2
@@ -391,8 +391,7 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)if (stat >PM_QOS_FLAGS_NONE )return -EBUSY ; -if (pdd->dev->driver && (!pm_runtime_suspended (pdd->dev) - || pdd->dev->power.irq_safe)) +if (!pm_runtime_suspended (pdd->dev) || pdd->dev->power.irq_safe) not_suspended++; }
Signature · 1 line
2.1.4
RFC PATCH v2 2/16 PM / Domains: Remove dev->driver check for runtime PM
Quoted 9 lines · Lina Iyer
Remove check for driver of a device, for runtime PM. Device may be suspended without an explicit driver. This check seems to be vestigial and incorrect in the current context.
Signed-off-by Kevin Hilman <[email protected]>Signed-off-by Lina Iyer <[email protected]>--- drivers/base/power/domain.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Makes sense,
Best regards,
Krzysztof
Quoted 16 lines
drivers/base/power/domain.c +1
-2@@ -391,8 +391,7 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)if (stat >PM_QOS_FLAGS_NONE )return -EBUSY ; -if (pdd->dev->driver && (!pm_runtime_suspended (pdd->dev) - || pdd->dev->power.irq_safe)) +if (!pm_runtime_suspended (pdd->dev) || pdd->dev->power.irq_safe) not_suspended++; }
RFC PATCH v2 3/16 PM / Domains: Support IRQ safe PM domains
In reply to Lina IyerGeneric Power Domains currently support turning on/off only in process context. This prevents the usage of PM domains for domains that could be powered on/off in a context where IRQs are disabled. Many such domains exist today and do not get powered off, when the IRQ safe devices in that domain are powered off, because of this limitation.
However, not all domains can operate in IRQ safe contexts. Genpd therefore, has to support both cases where the domain may or may not operate in IRQ safe contexts. Configuring genpd to use an appropriate lock for that domain, would allow domains that have IRQ safe devices to runtime suspend and resume, in atomic context.
To achieve domain specific locking, set the domain's ->flag to GENPD_FLAG_IRQ_SAFE while defining the domain. This indicates that genpd should use a spinlock instead of a mutex for locking the domain. Locking is abstracted through genpd_lock() and genpd_unlock() functions that use the flag to determine the appropriate lock to be used for that domain. Domains that have lower latency to suspend and resume and can operate with IRQs disabled may now be able to save power, when the component devices and sub-domains are idle at runtime.
The restriction this imposes on the domain hierarchy is that sub-domains and all devices in the IRQ safe domain's hierarchy also have to be IRQ safe, so that we dont try to lock a mutex, while holding a spinlock. Non-IRQ safe domains may continue to have devices and sub-domains that may or may not be IRQ safe.
---
Changes in v2:
- Modify commit text
- Fix an issue with IRQ safe check
- Rename genpd_lock_domain() and genpd_unlock_domain names
- Inlined functions instead of macros
- Rebase on top of V4 of Ulf's patch [1].
[1]. https://patches.linaro.org/50047/
--- Documentation/power/devices.txt | 11 ++- drivers/base/power/domain.c | 201 +++++++++++++++++++++++++++++++--------- include/linux/pm_domain.h | 11 ++- 3 files changed, 178 insertions(+), 45 deletions(-)
Documentation/power/devices.txt +10 -1
@@ -600,7 +600,16 @@ individually. Instead, a set of devices sharing a power resource can be put into a low-power state together at the same time by turning off the shared power resource. Of course, they also need to be put into the full-power state together, by turning the shared power resource on. A set of devices with this -property is often referred to as a power domain. +property is often referred to as a power domain. A power domain may also be +nested inside another domain. + +Devices, by default, operate in process context and if a device can operate in +IRQ safe context, has to be explicitly set as IRQ safe. PM domains by default, +operate in process context but could have devices that are IRQ safe. Such PM +domains cannot be powered on/off during runtime PM. On the other hand, an IRQ +safe PM domain that can be powered on/off and suspend or resumed in an atomic +context, may contain IRQ safe devices. Such domain may only contain only IRQ +safe devices or IRQ safe sub-domains. Support for power domains is provided through the pm_domain field of struct device. This field is a pointer to an object of type struct dev_pm_domain,
drivers/base/power/domain.c +158 -43
420 lines not on this page · raw message
include/linux/pm_domain.h +10 -1
@@ -16,9 +16,11 @@#include <linux/of.h> #include <linux/notifier.h> #include <linux/cpuidle.h> +#include <linux/spinlock.h> /* Defines used for the flags field in the struct generic_pm_domain */ #define GENPD_FLAG_PM_CLK (1U << 0)/* PM domain uses PM clk */ +#define GENPD_FLAG_IRQ_SAFE (1U << 1)/* PM domain operates in atomic */ enum gpd_status {GPD_STATE_ACTIVE =0 ,/* PM domain is active */
@@ -49,7 +51,6 @@ struct generic_pm_domain {struct list_head master_links;/* Links with PM domain as a master */ struct list_head slave_links;/* Links with PM domain as a slave */ struct list_head dev_list;/* List of devices */ -struct mutex lock;struct dev_power_governor *gov;struct work_struct power_off_work;const char *name;
@@ -74,6 +75,14 @@ struct generic_pm_domain {void (*detach_dev)(struct generic_pm_domain *domain,struct device *dev);unsigned int flags;/* Bit field of configs for genpd */ +bool irq_safe; +union { +struct mutex mlock; +struct { +spinlock_t slock; +unsigned long lock_flags; + }; + }; };static inline struct generic_pm_domain *pd_to_genpd (struct dev_pm_domain *pd)
Signature · 1 line
2.1.4
RFC PATCH v2 3/16 PM / Domains: Support IRQ safe PM domains
Hi Lina,
Quoted 5 lines · Lina Iyer
Generic Power Domains currently support turning on/off only in process context. This prevents the usage of PM domains for domains that could be powered on/off in a context where IRQs are disabled. Many such domains exist today and do not get powered off, when the IRQ safe devices in that domain are powered off, because of this limitation.
[...]
Thanks for your patch!
Documentation/power/devices.txt individually. Instead, a set of devices sharing a power resource can be put · line 604
Quoted 9 lines
Documentation/power/devices.txt +2
-1@@ -600,7 +600,16 @@ individually. Instead, a set of devices sharing a power resource can be put into a low-power state together at the same time by turning off the shared power resource. Of course, they also need to be put into the full-power state together, by turning the shared power resource on. A set of devices with this -property is often referred to as a power domain. +property is often referred to as a power domain. A power domain may also be +nested inside another domain.
"... another power domain"?
Documentation/power/devices.txt individually. Instead, a set of devices sharing a power resource can be put · line 607
Quoted 2 lines
Documentation/power/devices.txt +2
-0+Devices, by default, operate in process context and if a device can operate in +IRQ safe context, has to be explicitly set as IRQ safe. PM domains by default,
This isn't exactly your fault, but this section is the only one inside this document that talks about "power domains". All other sections talk about "PM domains".
You're adding a paragraph about "PM domains" inside this section.
Quoted 13 lines
Documentation/power/devices.txt +5
-0+operate in process context but could have devices that are IRQ safe. Such PM +domains cannot be powered on/off during runtime PM. On the other hand, an IRQ +safe PM domain that can be powered on/off and suspend or resumed in an atomic +context, may contain IRQ safe devices. Such domain may only contain only IRQ +safe devices or IRQ safe sub-domains. Support for power domains is provided through the pm_domain field of struct device. This field is a pointer to an object of type struct dev_pm_domain,drivers/base/power/domain.c +0
-0
drivers/base/power/domain.c line 59
Quoted 10 lines
drivers/base/power/domain.c +7
-0@@ -50,6 +50,74 @@static LIST_HEAD (gpd_list);static DEFINE_MUTEX (gpd_list_lock); +static inline int genpd_lock_noirq (struct generic_pm_domain *genpd, +unsigned int subclass) + __acquires(&genpd->slock) +{ +unsigned long flags; + +if (unlikely (subclass >0 ))
In general, using "unlikely" is frowned upon.
And in this case, the compiler already knows the exact value of subclass, as it's passed as a constant from one of the functions below.
drivers/base/power/domain.c line 78
Quoted 5 lines
drivers/base/power/domain.c +5
-0+static inline int genpd_lock_irq (struct generic_pm_domain *genpd, +unsigned int subclass) + __acquires(&genpd->mlock) +{ +if (unlikely (subclass >0 ))
Same here.
drivers/base/power/domain.c int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev, · line 1359
Quoted 5 lines
drivers/base/power/domain.c +1
-0@@ -1277,11 +1356,18 @@ 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 ; +/* Devices in an IRQ safe PM domain have to be IRQ safe too */
Do you really need this comment? It's identical to the error message below.
Quoted 6 lines
drivers/base/power/domain.c +3
-0+if (genpd->irq_safe && !dev->power.irq_safe) { +dev_err (dev, +"Devices in an IRQ safe domain have to be IRQ safe.\n" );@@ -1385,12 +1471,23 @@ int pm_genpd_add_subdomain(struct generic_pm_domain *genpd, || genpd == subdomain)
drivers/base/power/domain.c int pm_genpd_add_subdomain(struct generic_pm_domain *genpd, · line 1481
Quoted 10 lines
drivers/base/power/domain.c +8
-0return -EINVAL ; +/* +* If the domain can be powered on/off in an IRQ safe +* context, ensure that the subdomain can also be +* powered on/off in that context. +*/ +if (genpd->irq_safe && !subdomain->irq_safe) { +WARN ("Incompatible sub-domain %s of IRQ safe domain %s\n" , + subdomain->name, genpd->name);
If you change the error message to "Subdomains in an IRQ safe domain have to be IRQ safe", it's (a) consistent with the one for devices, and (b) you can remove the comment.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
RFC PATCH v2 3/16 PM / Domains: Support IRQ safe PM domains
In reply to Lina IyerSorry, I forgot a few comments...
Documentation/power/devices.txt
Quoted 4 lines · Lina Iyer
Documentation/power/devices.txt +1
-0+context, may contain IRQ safe devices. Such domain may only contain only IRQ
Double "only".
Documentation/power/devices.txt static int pm_genpd_runtime_resume(struct device *dev) · line 612
Quoted 10 lines
Documentation/power/devices.txt +3
-2+safe devices or IRQ safe sub-domains.@@ -532,15 +608,18 @@ static int pm_genpd_runtime_resume(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - /* If power.irq_safe, the PM domain is never powered off. */ - if (dev->power.irq_safe) { + /* + * As we dont power off a non IRQ safe domain, which holds a
Bogus "a" at the end.
Documentation/power/devices.txt static int pm_genpd_runtime_resume(struct device *dev) · line 614
Quoted 2 lines
Documentation/power/devices.txt +2
-0+ * an IRQ safe device, we dont need to restore power to it. + */
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
RFC PATCH v2 4/16 WIP: ARM: PM domains for CPUs/clusters
In reply to Lina IyerFrom: Kevin Hilman <[email protected]>
arch/arm/boot/dts/exynos5420.dtsi +18 -0
@@ -61,6 +61,7 @@ reg = <0x0>; clock-frequency = <1800000000>; cci-control-port = <&cci_control1>; + power-domains = <&big_cluster_pd>; }; cpu1: cpu at 1 {
@@ -69,6 +70,7 @@ reg = <0x1>; clock-frequency = <1800000000>; cci-control-port = <&cci_control1>; + power-domains = <&big_cluster_pd>; }; cpu2: cpu at 2 {
@@ -77,6 +79,7 @@ reg = <0x2>; clock-frequency = <1800000000>; cci-control-port = <&cci_control1>; + power-domains = <&big_cluster_pd>; }; cpu3: cpu at 3 {
@@ -85,6 +88,7 @@ reg = <0x3>; clock-frequency = <1800000000>; cci-control-port = <&cci_control1>; + power-domains = <&big_cluster_pd>; }; cpu4: cpu at 100 {
@@ -93,6 +97,7 @@ reg = <0x100>; clock-frequency = <1000000000>; cci-control-port = <&cci_control0>; + power-domains = <&little_cluster_pd>; }; cpu5: cpu at 101 {
@@ -101,6 +106,7 @@ reg = <0x101>; clock-frequency = <1000000000>; cci-control-port = <&cci_control0>; + power-domains = <&little_cluster_pd>; }; cpu6: cpu at 102 {
@@ -109,6 +115,7 @@ reg = <0x102>; clock-frequency = <1000000000>; cci-control-port = <&cci_control0>; + power-domains = <&little_cluster_pd>; }; cpu7: cpu at 103 {
@@ -117,6 +124,7 @@ reg = <0x103>; clock-frequency = <1000000000>; cci-control-port = <&cci_control0>; + power-domains = <&little_cluster_pd>; }; };
@@ -249,6 +257,16 @@ }; }; + big_cluster_pd: big_cluster { + compatible = "arm,pd"; + #power-domain-cells = <0>; + }; + + little_cluster_pd: little_cluster { + compatible = "arm,pd"; + #power-domain-cells = <0>; + }; + gsc_pd: power-domain at 10044000 { compatible = "samsung,exynos4210-pd"; reg = <0x10044000 0x20>;
arch/arm/kernel/Makefile +1 -0
@@ -78,6 +78,7 @@ CFLAGS_pj4-cp0.o := -marm AFLAGS_iwmmxt.o := -Wa,-mcpu=iwmmxt obj-$(CONFIG_ARM_CPU_TOPOLOGY) += topology.o obj-$(CONFIG_VDSO) += vdso.o +obj-$(CONFIG_PM_GENERIC_DOMAINS) += domains.o ifneq ($(CONFIG_ARCH_EBSA110),y) obj-y += io.o
arch/arm/kernel/domains.c new file +122 -0
122 lines not on this page · raw message
drivers/base/power/domain.c +1 -0
@@ -5,6 +5,7 @@* * This file is released under the GPLv2. */ +#define DEBUG #include <linux/kernel.h> #include <linux/io.h>
Signature · 1 line
2.1.4
RFC PATCH v2 5/16 arm: domain: Remove hack to add dev->driver.
In reply to Lina IyerFixed in Generic PM domains framework.
--- arch/arm/kernel/domains.c | 20 -------------------- 1 file changed, 20 deletions(-)
arch/arm/kernel/domains.c +0 -20
@@ -48,26 +48,6 @@ static int arm_domain_cpu_init(void)return -ENODEV ; } -/* -* HACK: genpd checks if devices are runtime_suspended -* before doing a poweroff of the domain. However, that check -* assumes that that device has a driver. Since CPU devices don't -* have a driver, genpd assumes that the device is runtime_suspended -* and will power off the domain as soon as the any device -* in the domain does a runtime_suspend. -* -* c.f. the following code in pm_genpd_poweroff(): -* -* if (pdd->dev->driver && (!pm_runtime_suspended(pdd->dev) -* || pdd->dev->power.irq_safe)) -* not_suspended++; -* -* Just removing the pdd->dev->driver check would also work, -* but not sure if that's right in the general case. -*/ - cpu_dev->driver =kzalloc (sizeof (struct device_driver ),GFP_KERNEL ); -WARN_ON (!cpu_dev->driver); -if (cpu_online (cpuid)) {pm_runtime_set_active (cpu_dev);pm_runtime_get_noresume (cpu_dev);
Signature · 1 line
2.1.4
RFC PATCH v2 6/16 arm: domain: Make CPU genpd IRQ safe
In reply to Lina IyerGeneric PM domains that support runtime suspend/resume in IRQ safe contexts should define GENPD_FLAG_IRQ_SAFE flag for the genpd.
CPUIdle powers off the CPUs in an IRQ safe context and therefore CPU domains that use CPUIlde for runtime suspend/resume would need to operate as IRQ safe domains. Define GENPD_FLAG_IRQ_SAFE for CPU domains.
--- arch/arm/kernel/domains.c | 1 + 1 file changed, 1 insertion(+)
arch/arm/kernel/domains.c +1 -0
@@ -90,6 +90,7 @@ static int arm_domain_init(void) pd->genpd.name =kstrdup (np->name,GFP_KERNEL ); pd->genpd.power_off = arm_pd_power_down; pd->genpd.power_on = arm_pd_power_up; + pd->genpd.flags |=GENPD_FLAG_IRQ_SAFE ;platform_set_drvdata (pdev, pd);dev_dbg (dev,"adding as generic power domain.\n" );
Signature · 1 line
2.1.4
RFC PATCH v2 7/16 arm: domain: Synchronize CPU device runtime PM usage with idle state
In reply to Lina IyerWhen a CPU is running, the runtime PM usage count should be incremented and decremented when the CPU is powers down. To ensure that this happens correctly, increment the runtime usage from that CPU.
--- arch/arm/kernel/domains.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)
arch/arm/kernel/domains.c +23 -1
@@ -33,6 +33,14 @@ static int arm_pd_power_up(struct generic_pm_domain *genpd)return 0 ; } +static void run_cpu (void *unused) +{ +struct device *cpu_dev =get_cpu_device (smp_processor_id ()); + +/* We are running, increment the usage count */ +pm_runtime_get_noresume (cpu_dev); +} +static int arm_domain_cpu_init (void ) {int cpuid, ret =0 ;
@@ -50,13 +58,27 @@ static int arm_domain_cpu_init(void)if (cpu_online (cpuid)) {pm_runtime_set_active (cpu_dev); -pm_runtime_get_noresume (cpu_dev); +/* +* Execute the below on that 'cpu' to ensure that the reference +* counting is correct. Its possible that while this code is +* executing, the 'cpu' may be powerd down, but we may incorrectly +* increment the usage. By executing the get_cpu on the 'cpu', +* we can ensure that the 'cpu' and its usage count are matched. +*/ +smp_call_function_single (cpuid, run_cpu,NULL , true); }else {pm_runtime_set_suspended (cpu_dev); }pm_runtime_irq_safe (cpu_dev);pm_runtime_enable (cpu_dev); +/* +* We attempt to attach the device to genpd again. We would +* have failed in our earlier attempt to attach to the domain +* provider as the CPU device would not have been IRQ safe, +* while the domain is defined as IRQ safe. IRQ safe domains +* can only have IRQ safe devices. +*/ ret =genpd_dev_pm_attach (cpu_dev);if (ret) {dev_warn (cpu_dev,"%s: Unable to attach to power-domain: %d\n" , __func__, ret);
Signature · 1 line
2.1.4
RFC PATCH v2 7/16 arm: domain: Synchronize CPU device runtime PM usage with idle state
Quoted 2 lines · Lina Iyer
When a CPU is running, the runtime PM usage count should be incremented and decremented when the CPU is powers down. To ensure that this happens
powered down
Quoted 1 line
correctly, increment the runtime usage from that CPU.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
RFC PATCH v2 8/16 arm: domain: Handle CPU online reference counting
In reply to Lina IyerEnsure that the CPU device usage count is incremented for a CPU that just came online. CPUs coming online do not follow the same code path as CPUs warm booting back to CPUIdle. Register for hotplug notifier and ensure the reference count is incremented correctly.
--- arch/arm/kernel/domains.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
arch/arm/kernel/domains.c +39 -0
@@ -41,10 +41,49 @@ static void run_cpu(void *unused)pm_runtime_get_noresume (cpu_dev); } +static int cpu_online_notifier (struct notifier_block *n, +unsigned long action,void *hcpu) +{ +int cpu = (unsigned long )hcpu; +struct device *dev =get_cpu_device (cpu); + +switch (action) { +case CPU_STARTING : +case CPU_STARTING_FROZEN : +/* +* Attach the cpu to its domain if the cpu is coming up +* for the first time. +* Called from the cpu that is coming up. +*/ +pm_runtime_set_active (dev); +pm_runtime_get_noresume (dev); +break ; + +default : +return NOTIFY_DONE ; + } + +return NOTIFY_OK ; +} + +static struct notifier_block hotplug_notifier = { + .notifier_call = cpu_online_notifier, +}; +static int arm_domain_cpu_init (void ) {int cpuid, ret =0 ; +/* +* Register for hotplug notifier, because not all CPUs +* may be online at this time. Also, hotplug entry path is not +* the same as warm boot. So the reference counting would have +* to be adjusted here accordingly. +* Register early, incase, a core comes online, +* while we are executing this. +*/ +register_cpu_notifier (&hotplug_notifier); +/* Find any CPU nodes with a phandle to this power domain */ for_each_possible_cpu (cpuid) {struct device *cpu_dev;
Signature · 1 line
2.1.4
RFC PATCH v2 9/16 arm: domain: Add platform callbacks for domain power on/off
In reply to Lina IyerPlatform drivers may have additional setup inorder before the domain can be powered off. Allow, platform drivers to register power on/off callbacks against a domain provider.
While registering the callback ensure that the domain is neither in power on/off state. The domain should be active. To ensure that the platform callback registration doesntrace with genpd power on/off, execute the registration from a CPU on that domain.
--- arch/arm/include/asm/cpu.h | 1 - arch/arm/include/asm/pm_domain.h | 27 ++++++++++++++ arch/arm/kernel/domains.c | 81 +++++++++++++++++++++++++++++++++++++++- 3 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 arch/arm/include/asm/pm_domain.h
arch/arm/include/asm/cpu.h +0 -1
@@ -22,5 +22,4 @@ struct cpuinfo_arm { }; DECLARE_PER_CPU(struct cpuinfo_arm, cpu_data); -#endif
arch/arm/include/asm/pm_domain.h new file +27 -0
@@ -0,0 +1,27 @@ +/* +* arch/arm/include/asm/pm_domain.h +* +* Copyright (C) 2015 Linaro Ltd. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +*/ +#ifndef __ASM_ARM_PM_DOMAIN_H +#define __ASM_ARM_PM_DOMAIN_H + +#include <linux/pm_domain.h> + +#if IS_ENABLED (CONFIG_PM_GENERIC_DOMAINS ) +extern int register_platform_domain_handlers (struct of_phandle_args *args, +int (*pd_down)(struct generic_pm_domain *), +int (*pd_up)(struct generic_pm_domain *)); +#else +static inline +int register_platform_domain_handlers (struct of_phandle_args *args, +int (*pd_down)(struct generic_pm_domain *), +int (*pd_up)(struct generic_pm_domain *)) +{return -ENODEV ; } +#endif + +#endif
arch/arm/kernel/domains.c +80 -1
119 lines not on this page · raw message
Signature · 1 line
2.1.4
RFC PATCH v2 9/16 arm: domain: Add platform callbacks for domain power on/off
Hi Lina, Kevin,
Quoted 2 lines · Lina Iyer
Platform drivers may have additional setup inorder before the domain can be powered off. Allow, platform drivers to register power on/off
Bogus comma.
Quoted 5 lines
callbacks against a domain provider.
While registering the callback ensure that the domain is neither in power on/off state. The domain should be active. To ensure that the platform callback registration doesntrace with genpd power on/off,
doesn't race
Quoted 15 lines
execute the registration from a CPU on that domain.
Signed-off-by Lina Iyer <[email protected]>arch/arm/include/asm/pm_domain.h new file +8
-0@@ -0,0 +1,27 @@ +/* +* arch/arm/include/asm/pm_domain.h +* +* Copyright (C) 2015 Linaro Ltd. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation.
arch/arm/include/asm/pm_domain.h line 18
Quoted 10 lines
arch/arm/include/asm/pm_domain.h new file +10
-0+*/ +#ifndef __ASM_ARM_PM_DOMAIN_H +#define __ASM_ARM_PM_DOMAIN_H + +#include <linux/pm_domain.h> + +#if IS_ENABLED (CONFIG_PM_GENERIC_DOMAINS ) +extern int register_platform_domain_handlers (struct of_phandle_args *args, +int (*pd_down)(struct generic_pm_domain *), +int (*pd_up)(struct generic_pm_domain *));
This looks a bit convoluted to me...
Quoted 47 lines
arch/arm/kernel/domains.c +19
-0@@ -9,10 +9,19 @@#include <linux/module.h> #include <linux/platform_device.h> +#include <asm/pm_domain.h> +#define NAME_MAX 16 +struct platform_cb { +int (*power_off)(struct generic_pm_domain *); +int (*power_on)(struct generic_pm_domain *); +}; +struct arm_pm_domain {struct generic_pm_domain genpd; +struct platform_cb plat_handler; +struct spinlock_t lock; }; static inline@@ -23,16 +32,85 @@ struct arm_pm_domain *to_arm_pd(struct generic_pm_domain *d)static int arm_pd_power_down (struct generic_pm_domain *genpd) { +struct arm_pm_domain *arm_pd =to_arm_pd (genpd); + +if (arm_pd->plat_handler.power_off) +return arm_pd->plat_handler.power_off (genpd); +/* pr_info("KJH: %s: %s\n", __func__, genpd->name); */ return 0 ; }static int arm_pd_power_up (struct generic_pm_domain *genpd) { +struct arm_pm_domain *arm_pd =to_arm_pd (genpd); + +if (arm_pd->plat_handler.power_on) +return arm_pd->plat_handler.power_on (genpd); +/* pr_info("KJH: %s: %s\n", __func__, genpd->name); */ return 0 ; }
arch/arm/kernel/domains.c static int arm_domain_init(void) · line 231
Quoted 2 lines
arch/arm/kernel/domains.c +0
-0@@ -152,6 +230,7 @@ static int arm_domain_init(void) pd->genpd.power_off = arm_pd_power_down; pd->genpd.power_on = arm_pd_power_up;
Shouldn't these .power_off() and .power_on() be set up from platform code instead, in the platform-specific code that creates the PM domain?
The PM Domain containing the CPU may contain other devices, in which case it's already set up from platform-specific code, which would conflict with arm_domain_init()?
Cfr. arch/arm/mach-shmobile/pm-rmobile.c, which handles all PM domains (both for devices and CPUs) on R-Mobile, and arch/arm/boot/dts/{r8a73a4,r8a7740,sh73a0}.dtsi.
R8a73a4 is the most advanced of these three: it has 2 big.LITTLE clusters, with separate PM Domains for L2/SCU and sub-domains for the CPUs. Unfortunately we don't have SMP support for it, so currently dtsi describes the first cpu core only. The full structure should look like this
cpus {
cpu0: cpu at 0 {
compatible = "arm,cortex-a15";
power-domains = <&pd_a2sl>;
next-level-cache = <&L2_CA15>;
};
cpu1: cpu at 1 {
compatible = "arm,cortex-a15";
power-domains = <&pd_a2sl>;
next-level-cache = <&L2_CA15>;
};
cpu2: cpu at 2 {
compatible = "arm,cortex-a15";
power-domains = <&pd_a2sl>;
next-level-cache = <&L2_CA15>;
};
cpu3: cpu at 3 {
compatible = "arm,cortex-a15";
power-domains = <&pd_a2sl>;
next-level-cache = <&L2_CA15>;
};
cpu4: cpu at 4 {
compatible = "arm,cortex-a7";
power-domains = <&pd_a2kl>;
next-level-cache = <&L2_CA7>;
};
cpu5: cpu at 5 {
compatible = "arm,cortex-a7";
power-domains = <&pd_a2kl>;
next-level-cache = <&L2_CA7>;
};
cpu6: cpu at 6 {
compatible = "arm,cortex-a7";
power-domains = <&pd_a2kl>;
next-level-cache = <&L2_CA7>;
};
cpu7: cpu at 7 {
compatible = "arm,cortex-a7";
power-domains = <&pd_a2kl>;
next-level-cache = <&L2_CA7>;
};
};
L2_CA15: cache-controller at 0 {
compatible = "cache";
power-domains = <&pd_a3sm>;
};
L2_CA7: cache-controller at 1 {
compatible = "cache";
power-domains = <&pd_a3km>;
};And the PM Domain part (which is complete in upstream):
pd_c4: c4 at 0 {
#power-domain-cells = <0>;
pd_a3sm: a3sm at 20 {
reg = <20>;
#power-domain-cells = <0>;
pd_a2sl: a2sl at 21 {
reg = <21>;
#power-domain-cells = <0>;
};
};
pd_a3km: a3km at 22 {
reg = <22>;
#size-cells = <0>;
#power-domain-cells = <0>;
pd_a2kl: a2kl at 23 {
reg = <23>;
#power-domain-cells = <0>;
};
};
};Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
RFC PATCH v2 9/16 arm: domain: Add platform callbacks for domain power on/off
Quoted 8 lines · Geert Uytterhoeven
Hi Lina, Kevin,
Quoted 2 lines · Lina Iyer
Platform drivers may have additional setup inorder before the domain can be powered off. Allow, platform drivers to register power on/off
Bogus comma.
Will fix.
Quoted 8 lines
Quoted 5 lines
callbacks against a domain provider.
While registering the callback ensure that the domain is neither in power on/off state. The domain should be active. To ensure that the platform callback registration doesntrace with genpd power on/off,
doesn't race
Argh, thanks.
Quoted 81 lines
Quoted 15 lines
execute the registration from a CPU on that domain.
Signed-off-by Lina Iyer <[email protected]>arch/arm/include/asm/pm_domain.h new file +8
-0@@ -0,0 +1,27 @@ +/* +* arch/arm/include/asm/pm_domain.h +* +* Copyright (C) 2015 Linaro Ltd. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. arch/arm/include/asm/pm_domain.h line 18
Quoted 10 lines
arch/arm/include/asm/pm_domain.h new file +10
-0+*/ +#ifndef __ASM_ARM_PM_DOMAIN_H +#define __ASM_ARM_PM_DOMAIN_H + +#include <linux/pm_domain.h> + +#if IS_ENABLED (CONFIG_PM_GENERIC_DOMAINS ) +extern int register_platform_domain_handlers (struct of_phandle_args *args, +int (*pd_down)(struct generic_pm_domain *), +int (*pd_up)(struct generic_pm_domain *));This looks a bit convoluted to me...
Quoted 47 lines
arch/arm/kernel/domains.c +19
-0@@ -9,10 +9,19 @@#include <linux/module.h> #include <linux/platform_device.h> +#include <asm/pm_domain.h> +#define NAME_MAX 16 +struct platform_cb { +int (*power_off)(struct generic_pm_domain *); +int (*power_on)(struct generic_pm_domain *); +}; +struct arm_pm_domain {struct generic_pm_domain genpd; +struct platform_cb plat_handler; +struct spinlock_t lock; }; static inline@@ -23,16 +32,85 @@ struct arm_pm_domain *to_arm_pd(struct generic_pm_domain *d)static int arm_pd_power_down (struct generic_pm_domain *genpd) { +struct arm_pm_domain *arm_pd =to_arm_pd (genpd); + +if (arm_pd->plat_handler.power_off) +return arm_pd->plat_handler.power_off (genpd); +/* pr_info("KJH: %s: %s\n", __func__, genpd->name); */ return 0 ; }static int arm_pd_power_up (struct generic_pm_domain *genpd) { +struct arm_pm_domain *arm_pd =to_arm_pd (genpd); + +if (arm_pd->plat_handler.power_on) +return arm_pd->plat_handler.power_on (genpd); +/* pr_info("KJH: %s: %s\n", __func__, genpd->name); */ return 0 ; }arch/arm/kernel/domains.c static int arm_domain_init(void) · line 231
Quoted 2 lines
arch/arm/kernel/domains.c +0
-0@@ -152,6 +230,7 @@ static int arm_domain_init(void) pd->genpd.power_off = arm_pd_power_down; pd->genpd.power_on = arm_pd_power_up;Shouldn't these .power_off() and .power_on() be set up from platform code instead, in the platform-specific code that creates the PM domain?
Quoted 4 lines
The PM Domain containing the CPU may contain other devices, in which case it's already set up from platform-specific code, which would conflict with
arm_domain_init()?
In my first RFC, the platform code was creating a domain. In this RFC, we are flaunting an idea that a generic code could setup the domains without any intervention from platform code. It will do everything common on most ARM platforms. Architectures that do not have anything specific in their domain, would benefit from such an initialization.
If we were to export this genpd and then platform code could attach more devices to the genpd, or make it a sub of another genpd, would that work?
Quoted 6 lines
Cfr.
arch/arm/mach-shmobile/pm-rmobile.c, which handles all PM domains (both for devices and CPUs) on R-Mobile, andarch/arm/boot/dts/{r8a73a4,r8a7740,sh73a0}.dtsi.R8a73a4 is the most advanced of these three: it has 2 big.LITTLE clusters, with separate PM Domains for L2/SCU and sub-domains for the CPUs.
If you could get the CPU genpd from the ARM common code, you could embed that in the L2 domain.
On QCOM's big.LITTLE SoC, there would a PM domain for each of the CPUs and then there would be a coherency PM domain that would contain the big and LITTLE cluster domains. In that case, the platform code would create and initialize the coherency domain and make the CPU domains a sub of the coherency domain. Would something like that work?
Quoted 81 lines
Unfortunately we don't have SMP support for it, so currently dtsi describes the first cpu core only. The full structure should look like this
cpus { cpu0: cpu at 0 { compatible = "arm,cortex-a15"; power-domains = <&pd_a2sl>; next-level-cache = <&L2_CA15>; }; cpu1: cpu at 1 { compatible = "arm,cortex-a15"; power-domains = <&pd_a2sl>; next-level-cache = <&L2_CA15>; }; cpu2: cpu at 2 { compatible = "arm,cortex-a15"; power-domains = <&pd_a2sl>; next-level-cache = <&L2_CA15>; }; cpu3: cpu at 3 { compatible = "arm,cortex-a15"; power-domains = <&pd_a2sl>; next-level-cache = <&L2_CA15>; }; cpu4: cpu at 4 { compatible = "arm,cortex-a7"; power-domains = <&pd_a2kl>; next-level-cache = <&L2_CA7>; }; cpu5: cpu at 5 { compatible = "arm,cortex-a7"; power-domains = <&pd_a2kl>; next-level-cache = <&L2_CA7>; }; cpu6: cpu at 6 { compatible = "arm,cortex-a7"; power-domains = <&pd_a2kl>; next-level-cache = <&L2_CA7>; }; cpu7: cpu at 7 { compatible = "arm,cortex-a7"; power-domains = <&pd_a2kl>; next-level-cache = <&L2_CA7>; }; }; L2_CA15: cache-controller at 0 { compatible = "cache"; power-domains = <&pd_a3sm>; }; L2_CA7: cache-controller at 1 { compatible = "cache"; power-domains = <&pd_a3km>; };And the PM Domain part (which is complete in upstream):
pd_c4: c4 at 0 { #power-domain-cells = <0>; pd_a3sm: a3sm at 20 { reg = <20>; #power-domain-cells = <0>; pd_a2sl: a2sl at 21 { reg = <21>; #power-domain-cells = <0>; }; }; pd_a3km: a3km at 22 { reg = <22>; #size-cells = <0>;
Quoted 9 lines
#power-domain-cells = <0>;
pd_a2kl: a2kl at 23 {
reg = <23>;
#power-domain-cells = <0>;
};
};
};Thanks for the example. Would it work, if the platform code initalizes the pd_a3sm, pd_a3km and pd_c4 and set up the hierarchy and to add the CPU domains, you could query the ARM and then add the domains to the pd_a3sm and pd_a3km?
Thanks for looking through the patch.
-- Lina
RFC PATCH v2 9/16 arm: domain: Add platform callbacks for domain power on/off
Hi Lina,
Quoted 206 lines · Lina Iyer
Quoted 82 lines · Geert Uytterhoeven
Quoted 20 lines
arch/arm/include/asm/pm_domain.h new file +16
-0@@ -0,0 +1,27 @@ +/* +* arch/arm/include/asm/pm_domain.h +* +* Copyright (C) 2015 Linaro Ltd. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +*/ +#ifndef __ASM_ARM_PM_DOMAIN_H +#define __ASM_ARM_PM_DOMAIN_H + +#include <linux/pm_domain.h> + +#if IS_ENABLED (CONFIG_PM_GENERIC_DOMAINS ) +extern int register_platform_domain_handlers(struct of_phandle_args*args,
arch/arm/include/asm/pm_domain.h line 18
Quoted 2 lines
arch/arm/include/asm/pm_domain.h new file +2
-0+int (*pd_down)(struct generic_pm_domain *), + int (*pd_up)(struct generic_pm_domain *));This looks a bit convoluted to me...
Quoted 49 lines
arch/arm/kernel/domains.c +19
-0@@ -9,10 +9,19 @@#include <linux/module.h> #include <linux/platform_device.h> +#include <asm/pm_domain.h> +#define NAME_MAX 16 +struct platform_cb { +int (*power_off)(struct generic_pm_domain *); +int (*power_on)(struct generic_pm_domain *); +}; +struct arm_pm_domain {struct generic_pm_domain genpd; +struct platform_cb plat_handler; +struct spinlock_t lock; }; static inline@@ -23,16 +32,85 @@ struct arm_pm_domain *to_arm_pd(struct generic_pm_domain *d)static int arm_pd_power_down (struct generic_pm_domain *genpd) { +struct arm_pm_domain *arm_pd =to_arm_pd (genpd); + +if (arm_pd->plat_handler.power_off) +return arm_pd->plat_handler.power_off (genpd); +/* pr_info("KJH: %s: %s\n", __func__, genpd->name); */ return 0 ; }static int arm_pd_power_up (struct generic_pm_domain *genpd) { +struct arm_pm_domain *arm_pd =to_arm_pd (genpd); + +if (arm_pd->plat_handler.power_on) +return arm_pd->plat_handler.power_on (genpd); +/* pr_info("KJH: %s: %s\n", __func__, genpd->name); */ return 0 ; }arch/arm/kernel/domains.c static int arm_domain_init(void) · line 231
Quoted 2 lines
arch/arm/kernel/domains.c +0
-0@@ -152,6 +230,7 @@ static int arm_domain_init(void) pd->genpd.power_off = arm_pd_power_down; pd->genpd.power_on = arm_pd_power_up;Shouldn't these .power_off() and .power_on() be set up from platform code instead, in the platform-specific code that creates the PM domain?
Quoted 4 lines
The PM Domain containing the CPU may contain other devices, in which case it's already set up from platform-specific code, which would conflict with
arm_domain_init()?In my first RFC, the platform code was creating a domain. In this RFC, we are flaunting an idea that a generic code could setup the domains without any intervention from platform code. It will do everything common on most ARM platforms. Architectures that do not have anything specific in their domain, would benefit from such an initialization.
If we were to export this genpd and then platform code could attach more devices to the genpd, or make it a sub of another genpd, would that work?
Quoted 6 lines
Cfr.
arch/arm/mach-shmobile/pm-rmobile.c, which handles all PM domains (both for devices and CPUs) on R-Mobile, andarch/arm/boot/dts/{r8a73a4,r8a7740,sh73a0}.dtsi.R8a73a4 is the most advanced of these three: it has 2 big.LITTLE clusters, with separate PM Domains for L2/SCU and sub-domains for the CPUs.
If you could get the CPU genpd from the ARM common code, you could embed that in the L2 domain.
On QCOM's big.LITTLE SoC, there would a PM domain for each of the CPUs and then there would be a coherency PM domain that would contain the big and LITTLE cluster domains. In that case, the platform code would create and initialize the coherency domain and make the CPU domains a sub of the coherency domain. Would something like that work?
Quoted 82 lines
Unfortunately we don't have SMP support for it, so currently dtsi describes
the first cpu core only. The full structure should look like thiscpus { cpu0: cpu at 0 { compatible = "arm,cortex-a15"; power-domains = <&pd_a2sl>; next-level-cache = <&L2_CA15>; }; cpu1: cpu at 1 { compatible = "arm,cortex-a15"; power-domains = <&pd_a2sl>; next-level-cache = <&L2_CA15>; }; cpu2: cpu at 2 { compatible = "arm,cortex-a15"; power-domains = <&pd_a2sl>; next-level-cache = <&L2_CA15>; }; cpu3: cpu at 3 { compatible = "arm,cortex-a15"; power-domains = <&pd_a2sl>; next-level-cache = <&L2_CA15>; }; cpu4: cpu at 4 { compatible = "arm,cortex-a7"; power-domains = <&pd_a2kl>; next-level-cache = <&L2_CA7>; }; cpu5: cpu at 5 { compatible = "arm,cortex-a7"; power-domains = <&pd_a2kl>; next-level-cache = <&L2_CA7>; }; cpu6: cpu at 6 { compatible = "arm,cortex-a7"; power-domains = <&pd_a2kl>; next-level-cache = <&L2_CA7>; }; cpu7: cpu at 7 { compatible = "arm,cortex-a7"; power-domains = <&pd_a2kl>; next-level-cache = <&L2_CA7>; }; }; L2_CA15: cache-controller at 0 { compatible = "cache"; power-domains = <&pd_a3sm>; }; L2_CA7: cache-controller at 1 { compatible = "cache"; power-domains = <&pd_a3km>; };And the PM Domain part (which is complete in upstream):
pd_c4: c4 at 0 { #power-domain-cells = <0>; pd_a3sm: a3sm at 20 { reg = <20>; #power-domain-cells = <0>; pd_a2sl: a2sl at 21 { reg = <21>; #power-domain-cells = <0>; }; }; pd_a3km: a3km at 22 { reg = <22>; #size-cells = <0>;Quoted 9 lines
#power-domain-cells = <0>; pd_a2kl: a2kl at 23 { reg = <23>; #power-domain-cells = <0>; }; }; };
Quoted 4 lines
Thanks for the example. Would it work, if the platform code initalizes the
pd_a3sm,pd_a3kmandpd_c4and set up the hierarchy and to add the CPU domains, you could query the ARM and then add the domains to thepd_a3smandpd_a3km?
Yes, it could work. But then I have to add _more_ code to ignore the a2sl and a2kl code in pm-rmobile.c, and call register_platform_domain_handlers(), as in the end it's pm-rmobile that knows how to power up those PM domains.
I'm a bit reluctant to split responsibility across two drivers: a platform-specific
one handling PM domains with non-cpu devices, and a generic one handling PM domains with cpu-devices.
Perhaps the generic one can be optional, and provide helpers for common CPU operations? Then the platform-specific driver can handle all PM domains, and delegate to the generic CPU helpers where appropriate.
Does that make sense?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
RFC PATCH v2 9/16 arm: domain: Add platform callbacks for domain power on/off
Quoted 11 lines · Geert Uytterhoeven
Hi Lina,
Quoted 1 line · Lina Iyer
On Mon, Jun 29 2015 at 07:36 -0600, Geert Uytterhoeven wrote:
Perhaps the generic one can be optional, and provide helpers for common CPU operations? Then the platform-specific driver can handle all PM domains, and delegate to the generic CPU helpers where appropriate.
Does that make sense?
Well it does. Thanks Geert. My RFC v1 [1] did exacly that. But, it didnt fit into the big picture of things well.
Here is where we wanted to head towards, in the long run. Today, we have CPU_PM for CPU runtime and we have runtime PM for others, we want to unify and move to a generic runtime PM for the CPUs as well. To that effort, we want to bring in generic code all into the fold of CPU runtime PM and CPU domain runtime PM. A generic CPU PM domain with its own genpd callback for ->power_on() and ->power_off() would help handle the common power on/off stuff there and possibly call into GIC and others that currently use CPU_PM from there. So the common node needs to be the handle of power on/off callbacks from the genpd, when all the CPUs are entering idle or resuming.
With what you have suggested, the platform driver creates the genpd and would pass the CPU genpd to the common code for common operations. (This was what was done in [1]). The platform driver would set the power_on() and power_off() callbacks and that would have to be overriden in order handle common CPU domain suspend/resume activities. Overwriting members of an object allocated by the platform driver, is something we should avoid.
Or instead of allocating the memory in your platform driver for the CPU genpd, you could query and get the genpd and then add your additions on top. You could add other flags like GENPD_FLAG_PM_CLK, but *not* overwrite the power on/off callbacks in the genpd. They still have to be registered separately like in this patch. Again, not every elegant, IMO.
Another option, that might be cleaner, is that we could have a PM domain for CPUs that would set up the compatibility flag to "arm,pd" and you could nest that domain inside pd_a2sl and pd_a2kl.
pd_c4: c4 at 0 {
[...]
pd_a3sm: a3sm at 20 {
[...]
pd_a2sl: a2sl at 21 {
reg = <21>;
#power-domain-cells = <0>;
pd_cpu_sl: pd1 { <-- Virtual PM domain
#power-domain-cells = <0>;
};
};
};
};cpus {
cpu0: cpu at 0 {
compatible = "arm,cortex-a15";
power-domains = <&pd_cpu_sl>; <-- here we refer to the
virtual PM domain
next-level-cache = <&L2_CA15>;
};
[...]
};This the common code would get its own callbacks and when that genpd powers off, the platform genpd would power down. But no new code is needed in your platform driver. The benefit is that platform code and generic CPU domain code may exist and act in parallel and may only be related if specified in the DT and the problem with that approach is that this virtual PM domain is not a h/w domain, hence specifying in DT is questionable.
What do you think?
Thanks,
Lina
RFC PATCH v2 9/16 arm: domain: Add platform callbacks for domain power on/off
Hi Lina,
Quoted 14 lines · Lina Iyer
Quoted 10 lines · Geert Uytterhoeven
Quoted 1 line · Lina Iyer
On Mon, Jun 29 2015 at 07:36 -0600, Geert Uytterhoeven wrote:
Perhaps the generic one can be optional, and provide helpers for common CPU operations? Then the platform-specific driver can handle all PM domains,
and delegate to the generic CPU helpers where appropriate.Does that make sense?
Well it does. Thanks Geert. My RFC v1 [1] did exacly that. But, it didnt fit into the big picture of things well.
Quoted 10 lines
Here is where we wanted to head towards, in the long run. Today, we have
CPU_PMfor CPU runtime and we have runtime PM for others, we want to unify and move to a generic runtime PM for the CPUs as well. To that effort, we want to bring in generic code all into the fold of CPU runtime PM and CPU domain runtime PM. A generic CPU PM domain with its own genpd callback for ->power_on() and ->power_off() would help handle the common power on/off stuff there and possibly call into GIC and others that currently useCPU_PMfrom there. So the common node needs to be the handle of power on/off callbacks from the genpd, when all the CPUs are entering idle or resuming.
Agreed.
Quoted 7 lines
With what you have suggested, the platform driver creates the genpd and would pass the CPU genpd to the common code for common operations. (This was what was done in [1]). The platform driver would set the
power_on()andpower_off()callbacks and that would have to be overriden in order handle common CPU domain suspend/resume activities. Overwriting members of an object allocated by the platform driver, is something we should avoid.
Instead of letting the generic code override the .power_{on,off}() callbacks, the platform code could call the generic CPU-related methods from its own .power_{on,off}() callbacks?
struct rmobile_pm_domain already has .suspend() and .resume() methods. The former is used to e.g. prevent the PM domains containing CPUs to be powered down (in the absence of cpuidle integration). That requires scanning the DT for CPUs, and it would indeed be good to have that scanning support in generic code.
The latter became unused after the removal of sh7372 support, which did have some cpuidle integration.
Quoted 5 lines
Or instead of allocating the memory in your platform driver for the CPU genpd, you could query and get the genpd and then add your additions on top. You could add other flags like
GENPD_FLAG_PM_CLK, but *not* overwrite the power on/off callbacks in the genpd. They still have to be registered separately like in this patch. Again, not every elegant, IMO.
Just wondering, can I set up the .attach_dev() and .detach_dev()?
Actually, for R-Mobile hardware I don't have a need to set GENPD_FLAG_PM_CLK or .attach_dev() and .detach_dev() for CPU PM domains, as they are only needed for devices with MSTP clocks. CPU and L2 cache don't have these, and there are no other devices in e.g. a3sm and a2sl.
The GIC has an MSTP clock, but it's part of a different power domain.
Quoted 28 lines
Another option, that might be cleaner, is that we could have a PM domain for CPUs that would set up the compatibility flag to "arm,pd" and you could nest that domain inside
pd_a2slandpd_a2kl.pd_c4: c4 at 0 { [...] pd_a3sm: a3sm at 20 { [...] pd_a2sl: a2sl at 21 { reg = <21>; #power-domain-cells = <0>; pd_cpu_sl: pd1 { <-- Virtual PM domain #power-domain-cells = <0>; }; }; }; };cpus { cpu0: cpu at 0 { compatible = "arm,cortex-a15"; power-domains = <&pd_cpu_sl>; <-- here we refer to the virtual PM domain next-level-cache = <&L2_CA15>; }; [...] };
Quoted 7 lines
This the common code would get its own callbacks and when that genpd powers off, the platform genpd would power down. But no new code is needed in your platform driver. The benefit is that platform code and generic CPU domain code may exist and act in parallel and may only be related if specified in the DT and the problem with that approach is that this virtual PM domain is not a h/w domain, hence specifying in DT is questionable.
Indeed, I don't like this option, as the DT would no longer describe HW, but the Linux implementation.
So let's continue with your approach, and see how it turns out. We can always change and improvide code, while changing DT is more complicated.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
RFC PATCH v2 9/16 arm: domain: Add platform callbacks for domain power on/off
Quoted 6 lines · Geert Uytterhoeven
Hi Lina,
Quoted 3 lines · Lina Iyer
Quoted 2 lines · Geert Uytterhoeven
Quoted 1 line · Lina Iyer
On Mon, Jun 29 2015 at 07:36 -0600, Geert Uytterhoeven wrote:
[...]
Quoted 8 lines
Quoted 7 lines
With what you have suggested, the platform driver creates the genpd and would pass the CPU genpd to the common code for common operations. (This was what was done in [1]). The platform driver would set the
power_on()andpower_off()callbacks and that would have to be overriden in order handle common CPU domain suspend/resume activities. Overwriting members of an object allocated by the platform driver, is something we should avoid.
Quoted 4 lines
Instead of letting the generic code override the .power_{on,off}() callbacks, the platform code could call the generic CPU-related methods from its own .power_{on,off}() callbacks?
We dont want to impose on platform drivers to call back into the CPU domain core code to do common domain power down activities. On a generic ARM CPU domain, we may not even need a platform specific domain callbacks. IMHO, this is not desirable.
Quoted 8 lines
struct
rmobile_pm_domainalready has .suspend() and .resume() methods. The former is used to e.g. prevent the PM domains containing CPUs to be powered down (in the absence of cpuidle integration). That requires scanning the DT for CPUs, and it would indeed be good to have that scanning support in generic code.
The latter became unused after the removal of sh7372 support, which did have some cpuidle integration.
The CPUs may still be attached to domain, even if they are not powered off by CPUIdle. The code increases the reference count for every online CPU and any CPUs that may come online. The reference count, only goes down when the CPU powers down through CPUIdle or hotplug. So, if a CPU is not integrated with CPUIdle, it would remain in use (unless powered off by hotplug) and the domain would not be powered down.
Quoted 8 lines
Quoted 5 lines
Or instead of allocating the memory in your platform driver for the CPU genpd, you could query and get the genpd and then add your additions on top. You could add other flags like
GENPD_FLAG_PM_CLK, but *not* overwrite the power on/off callbacks in the genpd. They still have to be registered separately like in this patch. Again, not every elegant, IMO.Just wondering, can I set up the .attach_dev() and .detach_dev()?
I am presuming, you are concerned with the fore mentioned case of CPUs not participating in CPUIdle. With pm_runtime_put_sync() not happening for CPUs that do not power down, you should not have to worry about the domain being powered down.
Please correct me if I misunderstood your point.
Quoted 7 lines
Actually, for R-Mobile hardware I don't have a need to set
GENPD_FLAG_PM_CLKor .attach_dev() and .detach_dev() for CPU PM domains, as they are only needed for devices with MSTP clocks. CPU and L2 cache don't have these, and there are no other devices in e.g. a3sm and a2sl.The GIC has an MSTP clock, but it's part of a different power domain.
Okay.
[...]
Quoted 8 lines
Quoted 7 lines
This the common code would get its own callbacks and when that genpd powers off, the platform genpd would power down. But no new code is needed in your platform driver. The benefit is that platform code and generic CPU domain code may exist and act in parallel and may only be related if specified in the DT and the problem with that approach is that this virtual PM domain is not a h/w domain, hence specifying in DT is questionable.
Quoted 6 lines
Indeed, I don't like this option, as the DT would no longer describe HW, but the Linux implementation.
So let's continue with your approach, and see how it turns out. We can always change and improvide code, while changing DT is more complicated.
Agreed.
Thanks for your time, Geert.
-- Lina
RFC PATCH v2 9/16 arm: domain: Add platform callbacks for domain power on/off
Quoted 18 lines · Lina Iyer
Quoted 6 lines · Geert Uytterhoeven
Hi Lina,
Quoted 3 lines · Lina Iyer
Quoted 2 lines · Geert Uytterhoeven
Quoted 1 line · Lina Iyer
On Mon, Jun 29 2015 at 07:36 -0600, Geert Uytterhoeven wrote:
[...]
Quoted 8 lines
Quoted 7 lines
With what you have suggested, the platform driver creates the genpd and would pass the CPU genpd to the common code for common operations. (This was what was done in [1]). The platform driver would set the
power_on()andpower_off()callbacks and that would have to be overriden in order handle common CPU domain suspend/resume activities. Overwriting members of an object allocated by the platform driver, is something we should avoid.
Quoted 9 lines
Quoted 4 lines
Instead of letting the generic code override the .power_{on,off}() callbacks, the platform code could call the generic CPU-related methods from its own .power_{on,off}() callbacks?
We dont want to impose on platform drivers to call back into the CPU domain core code to do common domain power down activities. On a generic ARM CPU domain, we may not even need a platform specific domain callbacks. IMHO, this is not desirable.
Any thoughts on this?
Quoted 60 lines
Quoted 8 lines
struct
rmobile_pm_domainalready has .suspend() and .resume() methods. The former is used to e.g. prevent the PM domains containing CPUs to be powered down (in the absence of cpuidle integration). That requires scanning the DT for CPUs, and it would indeed be good to have that scanning support in generic code.
The latter became unused after the removal of sh7372 support, which did have some cpuidle integration.The CPUs may still be attached to domain, even if they are not powered off by CPUIdle. The code increases the reference count for every online CPU and any CPUs that may come online. The reference count, only goes down when the CPU powers down through CPUIdle or hotplug. So, if a CPU is not integrated with CPUIdle, it would remain in use (unless powered off by hotplug) and the domain would not be powered down.
Quoted 8 lines
Quoted 5 lines
Or instead of allocating the memory in your platform driver for the CPU genpd, you could query and get the genpd and then add your additions on top. You could add other flags like
GENPD_FLAG_PM_CLK, but *not* overwrite the power on/off callbacks in the genpd. They still have to be registered separately like in this patch. Again, not every elegant, IMO.Just wondering, can I set up the .attach_dev() and .detach_dev()?
I am presuming, you are concerned with the fore mentioned case of CPUs not participating in CPUIdle. With
pm_runtime_put_sync()not happening for CPUs that do not power down, you should not have to worry about the domain being powered down.Please correct me if I misunderstood your point.
Quoted 7 lines
Actually, for R-Mobile hardware I don't have a need to set
GENPD_FLAG_PM_CLKor .attach_dev() and .detach_dev() for CPU PM domains, as they are only needed for devices with MSTP clocks. CPU and L2 cache don't have these, and there are no other devices in e.g. a3sm and a2sl.The GIC has an MSTP clock, but it's part of a different power domain.
Okay.
[...]
Quoted 8 lines
Quoted 7 lines
This the common code would get its own callbacks and when that genpd powers off, the platform genpd would power down. But no new code is needed in your platform driver. The benefit is that platform code and generic CPU domain code may exist and act in parallel and may only be related if specified in the DT and the problem with that approach is that this virtual PM domain is not a h/w domain, hence specifying in DT is questionable.
Quoted 6 lines
Indeed, I don't like this option, as the DT would no longer describe HW, but the Linux implementation.
So let's continue with your approach, and see how it turns out. We can always change and improvide code, while changing DT is more complicated.
Agreed.
Thanks for your time, Geert.
-- Lina
RFC PATCH v2 10/16 drivers: cpuidle: Add runtime PM support for CPUIdle
In reply to Lina IyerNotify runtime PM when the CPU is going to be powered off in the idle state. This allows for runtime PM suspend/resume of the CPU's PM domain.
--- drivers/cpuidle/cpuidle-arm.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
drivers/cpuidle/cpuidle-arm.c +11 -0
@@ -11,12 +11,14 @@#define pr_fmt (fmt) "CPUidle arm: " fmt +#include <linux/cpu.h> #include <linux/cpuidle.h> #include <linux/cpumask.h> #include <linux/cpu_pm.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/of.h> +#include <linux/pm_runtime.h> #include <linux/slab.h> #include <asm/cpuidle.h>
@@ -45,12 +47,20 @@ static int arm_enter_idle_state(struct cpuidle_device *dev, ret =cpu_pm_enter ();if (!ret) { +struct device *cpu_dev =get_cpu_device (dev->cpu); + +/* +* Notify runtime PM as well of this cpu powering down +* TODO: Merge CPU_PM and runtime PM. +*/ +pm_runtime_put_sync (cpu_dev);/* * Pass idle state index to cpu_suspend which in turn will * call the CPU ops suspend protocol with idle index as a * parameter. */ arm_cpuidle_suspend (idx); +pm_runtime_get_sync (cpu_dev);cpu_pm_exit (); }
@@ -146,6 +156,7 @@ static int __init arm_idle_init(void)kfree (dev); goto out_fail; } + }return 0 ;
Signature · 1 line
2.1.4
RFC PATCH v2 11/16 drivers: qcom: spm: Support cache and coherency SPMs
In reply to Lina IyerRecognize non-CPU SPM devices defined in the DT and configure the corresponding SPM hardware. SPM controllers for L2 controls the cache's idle low power state and may also be used to turn off the cluster's power rail.
On multi-cluster SoCs, each cluster would have an SPM for the cache and an additional coherency level SPM. The coherency SPM turns off or puts the coherency hardware in idle and any caches present at that level.
--- drivers/soc/qcom/spm.c | 75 +++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 65 insertions(+), 10 deletions(-)
drivers/soc/qcom/spm.c +65 -10
@@ -68,9 +68,17 @@ struct spm_reg_data {u8 start_index[PM_SLEEP_MODE_NR ]; }; +struct cluster ; +struct spm_driver_data {void __iomem *reg_base;const struct spm_reg_data *reg_data; +struct cluster *domain; +}; + +/* Group for domain entities */ +struct cluster { +struct spm_driver_data *domain_spm; };static const u8 spm_reg_offset_v2_1[SPM_REG_NR ] = {
@@ -116,6 +124,9 @@ static const struct spm_reg_data spm_reg_8064_cpu = { static DEFINE_PER_CPU(struct spm_driver_data *, cpu_spm_drv); +/* 3 instances: little, big and coherency (cluster of clusters) */ +static struct cluster cpu_domain[3 ]; +typedef int (*idle_fn )(int ); static DEFINE_PER_CPU(idle_fn*, qcom_idle_ops);
@@ -282,14 +293,26 @@ static struct cpuidle_ops qcom_cpuidle_ops __initdata = {CPUIDLE_METHOD_OF_DECLARE (qcom_idle_v1,"qcom,kpss-acc-v1" , &qcom_cpuidle_ops);CPUIDLE_METHOD_OF_DECLARE (qcom_idle_v2,"qcom,kpss-acc-v2" , &qcom_cpuidle_ops); +/* Match L2 SPM with their affinity level */ +static const struct of_device_id cache_spm_table[] = { + { }, +}; +static struct spm_driver_data *spm_get_drv (struct platform_device *pdev, -int *spm_cpu) +int *index,bool *is_domain_spm) {struct spm_driver_data *drv =NULL ; -struct device_node *cpu_node, *saw_node; +struct device_node *cpu_node, *saw_node, *cache_node;int cpu;bool found; +const struct of_device_id *match_id; +int idx; + drv =devm_kzalloc (&pdev->dev,sizeof (*drv),GFP_KERNEL ); +if (!drv) +return ERR_PTR (-ENOMEM ); + +/* Check for a CPU SPM, if found we are done */ for_each_possible_cpu (cpu) { cpu_node =of_cpu_device_node_get (cpu);if (!cpu_node)
@@ -302,10 +325,37 @@ static struct spm_driver_data *spm_get_drv(struct platform_device *pdev,break ; } +/* +* If found, we have a CPU SPM, if not, +* check if we have a cache SPM +*/ if (found) { - drv =devm_kzalloc (&pdev->dev,sizeof (*drv),GFP_KERNEL ); -if (drv) - *spm_cpu = cpu; +/* +* Now that we have our CPU, find the associated L2 +* SAW and bind the CPU with the domain that contains +* the L2 SAW. +*/ + cache_node =of_parse_phandle (cpu_node, +"next-level-cache" ,0 ); + saw_node =of_parse_phandle (cache_node,"qcom,saw" ,0 ); + match_id =of_match_node (cache_spm_table, saw_node); +if (match_id) { + idx = (int ) match_id->data; + drv->domain = &cpu_domain[idx]; + } +of_node_put (saw_node); +of_node_put (cache_node); + *index = cpu; + *is_domain_spm = false; + }else { +/* Check if this is a cache SPM */ + match_id =of_match_node (cache_spm_table, pdev->dev.of_node); +if (!match_id) { +devm_kfree (&pdev->dev, drv); +return ERR_PTR (-ENODEV ); + } + *index = (int ) match_id->data; + *is_domain_spm = true; }return drv;
@@ -327,11 +377,12 @@ static int spm_dev_probe(struct platform_device *pdev)struct resource *res;const struct of_device_id *match_id;void __iomem *addr; -int cpu; +int index; +bool is_domain_spm; - drv =spm_get_drv (pdev, &cpu); -if (!drv) -return -EINVAL ; + drv =spm_get_drv (pdev, &index, &is_domain_spm); +if (IS_ERR (drv)) +return PTR_ERR (drv); res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); drv->reg_base =devm_ioremap_resource (&pdev->dev, res);
@@ -366,7 +417,11 @@ static int spm_dev_probe(struct platform_device *pdev)/* Set up Standby as the default low power mode */ spm_set_low_power_mode (drv,PM_SLEEP_MODE_STBY ); -per_cpu (cpu_spm_drv, cpu) = drv; +/* We are ready to use the CPU/Cache SPM. */ +if (is_domain_spm) + cpu_domain[index].domain_spm = drv; +else +per_cpu (cpu_spm_drv, index) = drv;return 0 ; }
Signature · 1 line
2.1.4
RFC PATCH v2 12/16 drivers: qcom: spm: Enable runtime suspend/resume of CPU PM domain
In reply to Lina IyerOn APQ8084 QCOM SoC's, the CPUs are powered by a single rail controlled by the L2 cache power controller (L2 SPM). The L2 power domain supplies power to all the CPUs and L2. It is safe to power down the domain when all the CPUs and the L2 are powered down.
Powering down of the domain is done through the finite state machine on the L2 SAW. The L2 SPM can be configured to enter an idle state, when all CPUs enter their idle state. The L2 SPM state machine would turn off the cache and possibly power off the power domain as well. The SPM also guarantees that the h/w is ready for the CPU to resume, when woken up by an interrupt.
Define a cluster that holds the SPM and possibly other common cluster elements. The L2 SAW is also the genpd domain provider and the CPUs are the devices attached to the domain. When CPUIdle powers down each CPU, the ARM domain framework would callback to notify that the domain may be powered off. Configure the L2 SPM at that time to flush the L2 cache and turn off the CPU power rail.
--- drivers/soc/qcom/spm.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 2 deletions(-)
drivers/soc/qcom/spm.c +61 -2
@@ -24,9 +24,12 @@#include <linux/platform_device.h> #include <linux/cpuidle.h> #include <linux/cpu_pm.h> +#include <linux/pm_domain.h> #include <linux/qcom_scm.h> +#include <asm/cacheflush.h> #include <asm/cpuidle.h> +#include <asm/pm_domain.h> #include <asm/proc-fns.h> #include <asm/suspend.h>
@@ -79,6 +82,7 @@ struct spm_driver_data {/* Group for domain entities */ struct cluster {struct spm_driver_data *domain_spm; +bool domain_off; };static const u8 spm_reg_offset_v2_1[SPM_REG_NR ] = {
@@ -181,7 +185,23 @@ static void spm_set_low_power_mode(struct spm_driver_data *drv,static int qcom_pm_collapse (unsigned long int unused) { -qcom_scm_cpu_power_down (QCOM_SCM_CPU_PWR_DOWN_L2_ON ); +int flags =QCOM_SCM_CPU_PWR_DOWN_L2_ON ; +int cpu =smp_processor_id (); +bool domain_off; + +/* +* Check if the CPU domain will power off after this CPU +* enters idle. +* L2 cache may be turned off when the domain powers off, +* flush the non-secure cache before calling into secure. +*/ + domain_off =per_cpu (cpu_spm_drv, cpu)->domain->domain_off; +if (domain_off) { + flags =QCOM_SCM_CPU_PWR_DOWN_L2_OFF ; +flush_cache_all (); + } + +qcom_scm_cpu_power_down (flags);/* * Returns here only if there was a pending interrupt and we did not
@@ -293,6 +313,35 @@ static struct cpuidle_ops qcom_cpuidle_ops __initdata = {CPUIDLE_METHOD_OF_DECLARE (qcom_idle_v1,"qcom,kpss-acc-v1" , &qcom_cpuidle_ops);CPUIDLE_METHOD_OF_DECLARE (qcom_idle_v2,"qcom,kpss-acc-v2" , &qcom_cpuidle_ops); +static int pd_power_on (struct generic_pm_domain *domain) +{ +struct spm_driver_data *drv =per_cpu (cpu_spm_drv,smp_processor_id ()); +struct cluster *pmd = drv->domain; + +if (!pmd || !pmd->domain_spm) +return 0 ; + + pmd->domain_off = false; +spm_set_low_power_mode (pmd->domain_spm,PM_SLEEP_MODE_STBY ); + +return 0 ; +} + +static int pd_power_off (struct generic_pm_domain *domain) +{ +struct spm_driver_data *drv =per_cpu (cpu_spm_drv,smp_processor_id ()); +struct cluster *pmd = drv->domain; + +if (!pmd || !pmd->domain_spm) +return 0 ; + + pmd->domain_off = true; +spm_set_low_power_mode (pmd->domain_spm,PM_SLEEP_MODE_SPC ); + +return 0 ; +} + +/* Match L2 SPM with their affinity level */ static const struct of_device_id cache_spm_table[] = { { },
@@ -418,8 +467,18 @@ static int spm_dev_probe(struct platform_device *pdev)spm_set_low_power_mode (drv,PM_SLEEP_MODE_STBY );/* We are ready to use the CPU/Cache SPM. */ -if (is_domain_spm) +if (is_domain_spm) { +struct of_phandle_args args; +int ret; + + args.np = pdev->dev.of_node; + args.args_count =0 ; + ret =register_platform_domain_handlers (&args, + pd_power_on, pd_power_off); +if (ret) +dev_dbg (&pdev->dev,"Domain callback not registered\n" ); cpu_domain[index].domain_spm = drv; + }else per_cpu (cpu_spm_drv, index) = drv;
Signature · 1 line
2.1.4
RFC PATCH v2 13/16 drivers: qcom: spm: Add 8084 L2 SPM register data
In reply to Lina IyerAdd register data and configure L2 SAW to support voltage control and L2 idle states for QCOM APQ8084 SoC.
--- .../devicetree/bindings/arm/msm/qcom,saw2.txt | 1 + drivers/soc/qcom/spm.c | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+)
Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt +1 -0
@@ -27,6 +27,7 @@ PROPERTIES "qcom,apq8064-saw2-v1.1-cpu" "qcom,msm8974-saw2-v2.1-cpu" "qcom,apq8084-saw2-v2.1-cpu" + "qcom,apq8084-saw2-v2.1-l2" - reg: Usage: required
drivers/soc/qcom/spm.c +20 -0
@@ -90,6 +90,8 @@ static const u8 spm_reg_offset_v2_1[SPM_REG_NR] = { [SPM_REG_SPM_CTL] = 0x30, [SPM_REG_DLY] = 0x34, [SPM_REG_SEQ_ENTRY] = 0x80, + [SPM_REG_PMIC_DATA_0] = 0x40, + [SPM_REG_PMIC_DATA_1] = 0x44, };/* SPM register data for 8974, 8084 */
@@ -104,6 +106,20 @@ static const struct spm_reg_data spm_reg_8974_8084_cpu = { .start_index[PM_SLEEP_MODE_SPC] = 3, }; +static const struct spm_reg_data spm_reg_8084_l2 = { + .reg_offset = spm_reg_offset_v2_1, + .spm_cfg =0x14 , + .spm_dly =0x3C102800 , + .pmic_data[0 ] =0x02030080 , + .pmic_data[1 ] =0x00030000 , + .seq = {0x1F ,0x00 ,0x20 ,0x03 ,0x22 ,0x00 ,0x0F ,0x00 ,0x10 ,0x22 , +0x12 ,0x32 ,0x60 ,0x70 ,0x80 ,0xB0 ,0x11 ,0x42 ,0x03 ,0x01 , +0xB0 ,0x78 ,0x80 ,0x12 ,0x22 ,0x44 ,0x50 ,0x3B ,0x60 ,0x02 , +0x32 ,0x50 ,0x0F }, + .start_index[PM_SLEEP_MODE_STBY ] =0 , + .start_index[PM_SLEEP_MODE_SPC ] =7 , +}; +static const u8 spm_reg_offset_v1_1[SPM_REG_NR ] = { [SPM_REG_CFG ] =0x08 , [SPM_REG_SPM_CTL ] =0x20 ,
@@ -216,6 +232,7 @@ static int qcom_cpu_spc(int cpu)struct spm_driver_data *drv =per_cpu (cpu_spm_drv, cpu);spm_set_low_power_mode (drv,PM_SLEEP_MODE_SPC ); + ret =cpu_suspend (0 , qcom_pm_collapse);/* * ARM common code executes WFI without calling into our driver and
@@ -344,6 +361,7 @@ static int pd_power_off(struct generic_pm_domain *domain)/* Match L2 SPM with their affinity level */ static const struct of_device_id cache_spm_table[] = { + { .compatible ="qcom,apq8084-saw2-v2.1-l2" , .data = (void *)0 }, { }, };
@@ -417,6 +435,8 @@ static const struct of_device_id spm_match_table[] = { .data = &spm_reg_8974_8084_cpu }, { .compatible = "qcom,apq8064-saw2-v1.1-cpu", .data = &spm_reg_8064_cpu }, + { .compatible = "qcom,apq8084-saw2-v2.1-l2", + .data = &spm_reg_8084_l2 }, { }, };
Signature · 1 line
2.1.4
RFC PATCH v2 14/16 arm: dts: Add L2 power-controller device bindings for APQ8084
In reply to Lina IyerAdd power controller (SAW) device nodes for L2 caches. L2 SAW enable L2 to enter idle states and be powered off. Also, on 8084 the L2 SAW may be used to regulate the active voltage for the cpu and L2.
--- arch/arm/boot/dts/qcom-apq8084.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm/boot/dts/qcom-apq8084.dtsi +2 -2
@@ -59,7 +59,7 @@ }; L2: l2-cache { - compatible = "qcom,arch-cache"; + compatible = "cache"; cache-level = <2>; qcom,saw = <&saw_l2>; };
@@ -183,7 +183,7 @@ }; saw_l2: power-controller at f9012000 { - compatible = "qcom,saw2"; + compatible = "qcom,apq8084-saw2-v2.1-l2", "qcom,saw2"; reg = <0xf9012000 0x1000>; regulator; };
Signature · 1 line
2.1.4
RFC PATCH v2 15/16 arm: dts: Add power domain device bindings for APQ8084
In reply to Lina IyerDefine L2 SAW as the power domain provider and individual cpus are the power domain consumers.
--- arch/arm/boot/dts/qcom-apq8084.dtsi | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
arch/arm/boot/dts/qcom-apq8084.dtsi +7 -1
@@ -23,6 +23,7 @@ qcom,acc = <&acc0>; qcom,saw = <&saw0>; cpu-idle-states = <&CPU_SPC>; + power-domains = <&saw_l2>; }; cpu at 1 {
@@ -34,6 +35,7 @@ qcom,acc = <&acc1>; qcom,saw = <&saw1>; cpu-idle-states = <&CPU_SPC>; + power-domains = <&saw_l2>; }; cpu at 2 {
@@ -45,6 +47,7 @@ qcom,acc = <&acc2>; qcom,saw = <&saw2>; cpu-idle-states = <&CPU_SPC>; + power-domains = <&saw_l2>; }; cpu at 3 {
@@ -56,6 +59,7 @@ qcom,acc = <&acc3>; qcom,saw = <&saw3>; cpu-idle-states = <&CPU_SPC>; + power-domains = <&saw_l2>; }; L2: l2-cache {
@@ -183,9 +187,11 @@ }; saw_l2: power-controller at f9012000 { - compatible = "qcom,apq8084-saw2-v2.1-l2", "qcom,saw2"; + compatible = "qcom,apq8084-saw2-v2.1-l2", "qcom,saw2", + "arm,pd"; reg = <0xf9012000 0x1000>; regulator; + #power-domain-cells = <0>; }; acc0: clock-controller at f9088000 {
Signature · 1 line
2.1.4
RFC PATCH v2 16/16 drivers: qcom: Enable genpd on selecting QCOM_PM
In reply to Lina IyerEnable PM_CPU_DOMAIN and its PM_GENERIC_DOMAINS dependenciesd to provide cpu domain support for QCOM SoCs.
--- drivers/soc/qcom/Kconfig | 3 +++ 1 file changed, 3 insertions(+)
drivers/soc/qcom/Kconfig +3 -0
@@ -15,6 +15,9 @@ config QCOM_PM depends on ARCH_QCOM && !ARM64 select QCOM_SCM select ARM_CPU_SUSPEND + select PM_GENERIC_DOMAINS + select PM_GENERIC_DOMAINS_SLEEP + select PM_GENERIC_DOMAINS_OF help QCOM Platform specific power driver to manage cores and L2 low power modes. It interface with various system drivers to put the cores in
Signature · 1 line
2.1.4