PM / Domains / cpuidle: Preliminary cpuidle support for generic PM domains

RFC PATCH In review Rafael J. Wysocki 4 participants 19 messages May 9, 2012 – May 31, 2012

Subsystems (6)

Driver core Suspend to RAM Hibernation Power management core +2 more subsystems

lore

Share & tools

lore

Rafael J. Wysocki

author

2012-05-09 21:38 UTC #0 lore raw

cover PM / Domains / cpuidle: Preliminary cpuidle support for generic PM domains

Hi all,

Since people have been spending quite some time on cpuidle recently, I thought it might be a good time to show some ideas about how to make cpuidle work along with the generic PM domains framework (and the other way around).

The first patch is just an addition that's used by the second one. The second patch is not functional code and untested, so it is for comments only right now (no signoff), although I did my best to make is as close to functional as possible.

The details are in the changelogs, please tell me what you think.

Thanks,
Rafael

Rafael J. Wysocki

author

2012-05-09 21:38 UTC #1 lore raw

RFC PATCH 1/2 PM / cpuidle: Add driver reference counter

From: Rafael J. Wysocki <[email protected]>

Add a reference counter for the cpuidle driver, so that it can't be unregistered when it is in use.

Signed-off-by Rafael J. Wysocki <[email protected]>

---
 drivers/cpuidle/driver.c |   29 ++++++++++++++++++++++++++++-
 include/linux/cpuidle.h  |    6 +++++-
 2 files changed, 33 insertions(+), 2 deletions(-)

drivers/cpuidle/driver.c +28 -1

Index: linux/drivers/cpuidle/driver.c
===================================================================
@@ -16,6 +16,7 @@
 
 static struct cpuidle_driver *cpuidle_curr_driver;
 DEFINE_SPINLOCK(cpuidle_driver_lock);
+int cpuidle_driver_refcount;
 
 static void __cpuidle_register_driver(struct cpuidle_driver *drv)
 {

@@ -89,8 +90,34 @@ void cpuidle_unregister_driver(struct cp
 	}
 
 	spin_lock(&cpuidle_driver_lock);
-	cpuidle_curr_driver = NULL;
+
+	if (!WARN_ON(cpuidle_driver_refcount > 0))
+		cpuidle_curr_driver = NULL;
+
 	spin_unlock(&cpuidle_driver_lock);
 }
 
 EXPORT_SYMBOL_GPL(cpuidle_unregister_driver);
+
+struct cpuidle_driver *cpuidle_driver_ref(void)
+{
+	struct cpuidle_driver *drv;
+
+	spin_lock(&cpuidle_driver_lock);
+
+	drv = cpuidle_curr_driver;
+	cpuidle_driver_refcount++;
+
+	spin_unlock(&cpuidle_driver_lock);
+	return drv;
+}
+
+void cpuidle_driver_unref(void)
+{
+	spin_lock(&cpuidle_driver_lock);
+
+	if (!WARN_ON(cpuidle_driver_refcount <= 0))
+		cpuidle_driver_refcount--;
+
+	spin_unlock(&cpuidle_driver_lock);
+}

include/linux/cpuidle.h +5 -1

Index: linux/include/linux/cpuidle.h
===================================================================
@@ -136,7 +136,9 @@ struct cpuidle_driver {
 extern void disable_cpuidle(void);
 extern int cpuidle_idle_call(void);
 extern int cpuidle_register_driver(struct cpuidle_driver *drv);
-struct cpuidle_driver *cpuidle_get_driver(void);
+extern struct cpuidle_driver *cpuidle_get_driver(void);
+extern struct cpuidle_driver *cpuidle_driver_ref(void);
+extern void cpuidle_driver_unref(void);
 extern void cpuidle_unregister_driver(struct cpuidle_driver *drv);
 extern int cpuidle_register_device(struct cpuidle_device *dev);
 extern void cpuidle_unregister_device(struct cpuidle_device *dev);

@@ -157,6 +159,8 @@ static inline int cpuidle_idle_call(void
 static inline int cpuidle_register_driver(struct cpuidle_driver *drv)
 {return -ENODEV; }
 static inline struct cpuidle_driver *cpuidle_get_driver(void) {return NULL; }
+static inline struct cpuidle_driver *cpuidle_driver_ref(void) {return NULL; }
+static inline void cpuidle_driver_unref(void) {}
 static inline void cpuidle_unregister_driver(struct cpuidle_driver *drv) { }
 static inline int cpuidle_register_device(struct cpuidle_device *dev)
 {return -ENODEV; }

Rafael J. Wysocki

author

2012-05-09 21:38 UTC #2 lore raw

RFC PATCH 2/2 PM / Domains: Add preliminary cpuidle support

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

On some systems there are CPU cores located in the same power domains as I/O devices. Then, power can only be removed from the domain if all I/O devices in it are not in use and the CPU core is idle. Add preliminary support for that to the generic PM domains framework.

This assumes that there is only one CPU core in the system and it is supposed to be set up in the following way.

First, the platform is expected to provide a cpuidle driver with one extra state designated for the generic PM domains code to handle. This state should be initially disabled and its exit_latency value should be set to whatever time is needed to bring up the CPU core itself after restoring power to it, not including the domain's power on latency. Its .enter() callback should point to a procedure that will save the CPU core's state as appropriate before power removal. On success, it should return the same value as it has been passed as its third argument, but it shouldn't put the CPU core into a C-state. If it is about to return the index of a different cpuidle state, however, it should make sure that the CPU be put into that state before it returns.

The remaining characteristics of the extra cpuidle state, referred to as the "domain" cpuidle state below, (e.g. power usage, target residency) should be populated in accordance with the properties of the hardware.

Next, the platform should execute genpd_attach_cpuidle() on the PM domain containing the CPU core. That will cause the generic PM domains framework to treat that domain in a special way such that:

* When all devices in the domain have been suspended and it is about to be turned off, the states of the devices will be saved, but power will not be removed from the domain. Instead, the "domain" cpuidle state will be enabled so that power can be removed from the domain when the CPU core is idle and the state has been chosen as the target by the cpuidle governor. In that case, before removing power from the domain, the framework will execute the .enter() callback initially defined for the "domain" state.

* When the first I/O device in the domain is resumed and __pm_genpd_poweron(() is called for the first time after power has been removed from the domain, the "domain" cpuidle state will be disabled to avoid subsequent surprise power removals via cpuidle.

---
 drivers/base/power/domain.c |  152 +++++++++++++++++++++++++++++++++++++++++++-
 include/linux/cpuidle.h     |    2 
 include/linux/pm_domain.h   |   20 +++++
 3 files changed, 173 insertions(+), 1 deletion(-)

include/linux/pm_domain.h +20 -0

Index: linux/include/linux/pm_domain.h
===================================================================
@@ -15,6 +15,7 @@
 #include <linux/err.h>
 #include <linux/of.h>
 #include <linux/notifier.h>
+#include <linux/cpuidle.h>
 
 enum gpd_status {
 	GPD_STATE_ACTIVE = 0,	/* PM domain is active */

@@ -45,6 +46,14 @@ struct gpd_dev_ops {
 	bool (*active_wakeup)(struct device *dev);
 };
 
+struct gpd_cpu_data {
+	unsigned int saved_exit_latency;
+	struct cpuidle_state *idle_state;
+	int (*idle_enter)(struct cpuidle_device *dev,
+			  struct cpuidle_driver *drv,
+			  int index);
+};
+
 struct generic_pm_domain {
 	struct dev_pm_domain domain;	/* PM domain operations */
 	struct list_head gpd_list_node;	/* Node in the global PM domains list */

@@ -75,6 +84,7 @@ struct generic_pm_domain {
 	bool max_off_time_changed;
 	bool cached_power_down_ok;
 	struct device_node *of_node; /* Node in device tree */
+	struct gpd_cpu_data *cpu_data;
 };
 
 static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)

@@ -154,6 +164,8 @@ extern int pm_genpd_add_callbacks(struct
 				  struct gpd_dev_ops *ops,
 				  struct gpd_timing_data *td);
 extern int __pm_genpd_remove_callbacks(struct device *dev, bool clear_td);
+extern int genpd_attach_cpuidle(struct generic_pm_domain *genpd, int state);
+extern int genpd_detach_cpuidle(struct generic_pm_domain *genpd);
 extern void pm_genpd_init(struct generic_pm_domain *genpd,
 			  struct dev_power_governor *gov, bool is_off);

@@ -209,6 +221,14 @@ static inline int __pm_genpd_remove_call
 {
 	return -ENOSYS;
 }
+static inline int genpd_attach_cpuidle(struct generic_pm_domain *genpd, int st)
+{
+	return -ENOSYS;
+}
+static inline int genpd_detach_cpuidle(struct generic_pm_domain *genpd)
+{
+	return -ENOSYS;
+}
 static inline void pm_genpd_init(struct generic_pm_domain *genpd,
 				 struct dev_power_governor *gov, bool is_off)
 {

drivers/base/power/domain.c +151 -1

182 lines not on this page · raw message

include/linux/cpuidle.h +2 -0

Index: linux/include/linux/cpuidle.h
===================================================================
@@ -53,6 +53,8 @@ struct cpuidle_state {
 			int index);
 
 	int (*enter_dead) (struct cpuidle_device *dev, int index);
+
+	void		*platform_data;
 };
 
 /* Idle State Flags */

Santosh Shilimkar

2012-05-10 10:10 UTC #3 lore raw

Re: [RFC][PATCH 2/2] PM / Domains: Add preliminary cpuidle support

Rafael,

Quoted 8 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>
On some systems there are CPU cores located in the same power domains as I/O devices. Then, power can only be removed from the domain if all I/O devices in it are not in use and the CPU core is idle. Add preliminary support for that to the generic PM domains framework.

I am just curious to know, what kind of IO devices, you are talking here? And also how those devices linked with CPU low power states apart from being part of same power domain. And is it the power domain or more of voltage domain, we are talking here.

Quoted 34 lines

This assumes that there is only one CPU core in the system and it is supposed to be set up in the following way.
First, the platform is expected to provide a cpuidle driver with one extra state designated for the generic PM domains code to handle. This state should be initially disabled and its exit_latency value should be set to whatever time is needed to bring up the CPU core itself after restoring power to it, not including the domain's power on latency. Its .enter() callback should point to a procedure that will save the CPU core's state as appropriate before power removal. On success, it should return the same value as it has been passed as its third argument, but it shouldn't put the CPU core into a C-state. If it is about to return the index of a different cpuidle state, however, it should make sure that the CPU be put into that state before it returns.
The remaining characteristics of the extra cpuidle state, referred to as the "domain" cpuidle state below, (e.g. power usage, target residency) should be populated in accordance with the properties of the hardware.
Next, the platform should execute genpd_attach_cpuidle() on the PM domain containing the CPU core. That will cause the generic PM domains framework to treat that domain in a special way such that:
* When all devices in the domain have been suspended and it is about to be turned off, the states of the devices will be saved, but power will not be removed from the domain. Instead, the "domain" cpuidle state will be enabled so that power can be removed from the domain when the CPU core is idle and the state has been chosen as the target by the cpuidle governor. In that case, before removing power from the domain, the framework will execute the .enter() callback initially defined for the "domain" state.

Quoted 6 lines

* When the first I/O device in the domain is resumed and __pm_genpd_poweron(() is called for the first time after power has been removed from the domain, the "domain" cpuidle state will be disabled to avoid subsequent surprise power removals via cpuidle.

If these are CPU cluster/package specific IO's like interrupt controller, cache controller, Coherency interconnect etc and if the intention is to ensure that these devices context is saved/restored in cpuidle entry/exit, it can be handled with CPU PM notifiers. We already do that for ARM SOCs.

Quoted 1 line

From the patch description it seems, they are general purpose

peripherals. We had one thermal sensor on OMAP which wrongly clocked from the CPU clock source and needed some idle notifier infrastructure to prepare/resume this device for idle entry/exit.

Regards
Santosh

Rafael J. Wysocki

author

2012-05-10 18:36 UTC #4 lore raw

Re: [RFC][PATCH 2/2] PM / Domains: Add preliminary cpuidle support

Quoted 5 lines · Santosh Shilimkar

Rafael,
Quoted 2 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>

Quoted 8 lines

Quoted 6 lines
On some systems there are CPU cores located in the same power domains as I/O devices. Then, power can only be removed from the domain if all I/O devices in it are not in use and the CPU core is idle. Add preliminary support for that to the generic PM domains framework.
I am just curious to know, what kind of IO devices, you are talking here?

Nothing specific, really. It can be any kind of I/O devices that happen to be in the same power domain. This includes USB, SDHI, MMCIF controllers on the SoC I have in mind in particular.

Quoted 3 lines

And also how those devices linked with CPU low power states apart from being part of same power domain. And is it the power domain or more of voltage domain, we are talking here.

Depending on the definitions I guess. How do you define a power domain and a voltage domain?

Quoted 41 lines

Quoted 34 lines
This assumes that there is only one CPU core in the system and it is supposed to be set up in the following way.
First, the platform is expected to provide a cpuidle driver with one extra state designated for the generic PM domains code to handle. This state should be initially disabled and its exit_latency value should be set to whatever time is needed to bring up the CPU core itself after restoring power to it, not including the domain's power on latency. Its .enter() callback should point to a procedure that will save the CPU core's state as appropriate before power removal. On success, it should return the same value as it has been passed as its third argument, but it shouldn't put the CPU core into a C-state. If it is about to return the index of a different cpuidle state, however, it should make sure that the CPU be put into that state before it returns.
The remaining characteristics of the extra cpuidle state, referred to as the "domain" cpuidle state below, (e.g. power usage, target residency) should be populated in accordance with the properties of the hardware.
Next, the platform should execute genpd_attach_cpuidle() on the PM domain containing the CPU core. That will cause the generic PM domains framework to treat that domain in a special way such that:
* When all devices in the domain have been suspended and it is about to be turned off, the states of the devices will be saved, but power will not be removed from the domain. Instead, the "domain" cpuidle state will be enabled so that power can be removed from the domain when the CPU core is idle and the state has been chosen as the target by the cpuidle governor. In that case, before removing power from the domain, the framework will execute the .enter() callback initially defined for the "domain" state.
Quoted 6 lines
* When the first I/O device in the domain is resumed and __pm_genpd_poweron(() is called for the first time after power has been removed from the domain, the "domain" cpuidle state will be disabled to avoid subsequent surprise power removals via cpuidle.

Quoted 5 lines

If these are CPU cluster/package specific IO's like interrupt controller, cache controller, Coherency interconnect etc and if the intention is to ensure that these devices context is saved/restored in cpuidle entry/exit, it can be handled with CPU PM notifiers.

Maybe it can, but I'm not so sure of that in general.

Quoted 1 line

We already do that for ARM SOCs.

Surely not all of them? I know of a few at least where this isn't done.

Quoted 2 lines

From the patch description it seems, they are general purpose peripherals.

Yes, they are.

Quoted 4 lines

We had one thermal sensor on OMAP which wrongly clocked from the CPU clock source and needed some idle notifier infrastructure to prepare/resume this device for idle entry/exit.

The system I have in mind is designed in such a way that there is a power domain with three subdomains, one of which contains the CPU core and the remaining two contain I/O devices of various kinds. General purpose as well as "core".

Thanks,
Rafael

Santosh Shilimkar

2012-05-11 08:23 UTC #5 lore raw

Re: [RFC][PATCH 2/2] PM / Domains: Add preliminary cpuidle support

Quoted 12 lines · Rafael J. Wysocki

Quoted 11 lines · Santosh Shilimkar
Rafael,
Quoted 8 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
On some systems there are CPU cores located in the same power domains as I/O devices. Then, power can only be removed from the domain if all I/O devices in it are not in use and the CPU core is idle. Add preliminary support for that to the generic PM domains framework.

Quoted 7 lines

Quoted 2 lines
I am just curious to know, what kind of IO devices, you are talking here?
Nothing specific, really. It can be any kind of I/O devices that happen to be in the same power domain. This includes USB, SDHI, MMCIF controllers on the SoC I have in mind in particular.

OK.
These are more of generic devices and actually not related to CPU/CPU clusters as such.

Quoted 7 lines

Quoted 3 lines
And also how those devices linked with CPU low power states apart from being part of same power domain. And is it the power domain or more of voltage domain, we are talking here.
Depending on the definitions I guess. How do you define a power domain and a voltage domain?

A voltage domain can be a section of the device supplied by a dedicated voltage rail. A voltage domain can have many power-domains like CPU cluster domain, Interconnect domain, peripheral domains. And each power domain then can have many sub-modules like UART, SPI, USB etc

Quoted 47 lines

Quoted 41 lines
Quoted 34 lines
This assumes that there is only one CPU core in the system and it is supposed to be set up in the following way.
First, the platform is expected to provide a cpuidle driver with one extra state designated for the generic PM domains code to handle. This state should be initially disabled and its exit_latency value should be set to whatever time is needed to bring up the CPU core itself after restoring power to it, not including the domain's power on latency. Its .enter() callback should point to a procedure that will save the CPU core's state as appropriate before power removal. On success, it should return the same value as it has been passed as its third argument, but it shouldn't put the CPU core into a C-state. If it is about to return the index of a different cpuidle state, however, it should make sure that the CPU be put into that state before it returns.
The remaining characteristics of the extra cpuidle state, referred to as the "domain" cpuidle state below, (e.g. power usage, target residency) should be populated in accordance with the properties of the hardware.
Next, the platform should execute genpd_attach_cpuidle() on the PM domain containing the CPU core. That will cause the generic PM domains framework to treat that domain in a special way such that:
* When all devices in the domain have been suspended and it is about to be turned off, the states of the devices will be saved, but power will not be removed from the domain. Instead, the "domain" cpuidle state will be enabled so that power can be removed from the domain when the CPU core is idle and the state has been chosen as the target by the cpuidle governor. In that case, before removing power from the domain, the framework will execute the .enter() callback initially defined for the "domain" state.
Quoted 6 lines
* When the first I/O device in the domain is resumed and __pm_genpd_poweron(() is called for the first time after power has been removed from the domain, the "domain" cpuidle state will be disabled to avoid subsequent surprise power removals via cpuidle.
Quoted 5 lines
If these are CPU cluster/package specific IO's like interrupt controller, cache controller, Coherency interconnect etc and if the intention is to ensure that these devices context is saved/restored in cpuidle entry/exit, it can be handled with CPU PM notifiers.

Quoted 6 lines

Maybe it can, but I'm not so sure of that in general.
Quoted 1 line
We already do that for ARM SOCs.
Surely not all of them? I know of a few at least where this isn't done.

You are right these are not for general purpose IO's

Quoted 10 lines

Quoted 2 lines
From the patch description it seems, they are general purpose peripherals.
Yes, they are.
Quoted 4 lines
We had one thermal sensor on OMAP which wrongly clocked from the CPU clock source and needed some idle notifier infrastructure to prepare/resume this device for idle entry/exit.

Quoted 5 lines

The system I have in mind is designed in such a way that there is a power domain with three subdomains, one of which contains the CPU core and the remaining two contain I/O devices of various kinds. General purpose as well as "core".

I am not sure CPUIDLE is suppose to take care of these kind of general purpose IO's. CPUIDLE should take care of CPU and CPU cluster power management. Any other peripherals as you mentioned should be already have some sort of device drivers and they should be using runtime PM for it, no? And for the constraints, PM-Qos can be used. So far CPUIDLE core code has maintained that distinction and all the C-state latencies are of the CPU clusters rather than the SOC.

If you have a voltage rail dependency then that should be handled in the voltage layer/regulator layer. If there is a power domain dependency then the power domain framework should do the use counting yo handle such scenarios.

Please correct me but, IIUC, your proposal wants to use CPUIDLE for the SOC level power management.
Will you be able to expand your requirements and explain why can't you manage PM for the general purpose devices like MMC, USB etc in their own device drivers ?

Regards
Santosh

Magnus Damm

2012-05-11 08:35 UTC #6 lore raw

Re: [RFC][PATCH 2/2] PM / Domains: Add preliminary cpuidle support

Hi Santosh,

Quoted 32 lines · Santosh Shilimkar

Quoted 12 lines · Rafael J. Wysocki
Quoted 11 lines · Santosh Shilimkar
Rafael,
Quoted 8 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
On some systems there are CPU cores located in the same power domains as I/O devices. Then, power can only be removed from the domain if all I/O devices in it are not in use and the CPU core is idle. Add preliminary support for that to the generic PM domains framework.
Quoted 7 lines
Quoted 2 lines
I am just curious to know, what kind of IO devices, you are talking here?
Nothing specific, really. It can be any kind of I/O devices that happen to be in the same power domain. This includes USB, SDHI, MMCIF controllers on the SoC I have in mind in particular.
OK.
These are more of generic devices and actually not related to CPU/CPU clusters as such.
Quoted 7 lines
Quoted 3 lines
And also how those devices linked with CPU low power states apart from being part of same power domain. And is it the power domain or more of voltage domain, we are talking here.
Depending on the definitions I guess. How do you define a power domain and a voltage domain?

Quoted 5 lines

A voltage domain can be a section of the device supplied by a dedicated voltage rail. A voltage domain can have many power-domains like CPU cluster domain, Interconnect domain, peripheral domains. And each power domain then can have many sub-modules like UART, SPI, USB etc

There are no software controllable voltage domains on the sh7372/Mackerel SoC/board that Rafael is using. The SoC does however have multiple hierarchical power domains, and in the case when we want to have transparent runtime power management of the root power domain then we will have to take both I/O devices and CPU cores into consideration. I suppose exactly how to do that is up for discussion, but CPUIdle is certainly a natural step in some direction - this since the CPU-only power domains are already managed by CPUIdle.

Thanks,

/ magnus

Rafael J. Wysocki

author

2012-05-11 18:54 UTC #7 lore raw

Re: [RFC][PATCH 2/2] PM / Domains: Add preliminary cpuidle support

In reply to Santosh Shilimkar

Quoted 32 lines · Santosh Shilimkar

Quoted 12 lines · Rafael J. Wysocki
Quoted 11 lines · Santosh Shilimkar
Rafael,
Quoted 8 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
On some systems there are CPU cores located in the same power domains as I/O devices. Then, power can only be removed from the domain if all I/O devices in it are not in use and the CPU core is idle. Add preliminary support for that to the generic PM domains framework.
Quoted 7 lines
Quoted 2 lines
I am just curious to know, what kind of IO devices, you are talking here?
Nothing specific, really. It can be any kind of I/O devices that happen to be in the same power domain. This includes USB, SDHI, MMCIF controllers on the SoC I have in mind in particular.
OK.
These are more of generic devices and actually not related to CPU/CPU clusters as such.
Quoted 7 lines
Quoted 3 lines
And also how those devices linked with CPU low power states apart from being part of same power domain. And is it the power domain or more of voltage domain, we are talking here.
Depending on the definitions I guess. How do you define a power domain and a voltage domain?

Quoted 5 lines

A voltage domain can be a section of the device supplied by a dedicated voltage rail. A voltage domain can have many power-domains like CPU cluster domain, Interconnect domain, peripheral domains. And each power domain then can have many sub-modules like UART, SPI, USB etc

OK, so this is not the level of detail the code in question is about.

In my terminology a power domain is a set of devices such that it only is possible to remove power from (and restore power to) all of them together.

[...]

Quoted 11 lines

Quoted 6 lines
The system I have in mind is designed in such a way that there is a power domain with three subdomains, one of which contains the CPU core and the remaining two contain I/O devices of various kinds. General purpose as well as "core".
I am not sure CPUIDLE is suppose to take care of these kind of general purpose IO's. CPUIDLE should take care of CPU and CPU cluster power management. Any other peripherals as you mentioned should be already have some sort of device drivers and they should be using runtime PM for it, no?

Yes.

Quoted 3 lines

And for the constraints, PM-Qos can be used. So far CPUIDLE core code has maintained that distinction and all the C-state latencies are of the CPU clusters rather than the SOC.

Well, that need not be the case and my patch is about that.

Quoted 4 lines

If you have a voltage rail dependency then that should be handled in the voltage layer/regulator layer. If there is a power domain dependency then the power domain framework should do the use counting yo handle such scenarios.

The power domain framework, though, only covers I/O devices at the moment and this is an attempt to extend it to cover domains containing CPU cores as well as I/O devices.

Quoted 2 lines

Please correct me but, IIUC, your proposal wants to use CPUIDLE for the SOC level power management.

That depends on what you mean by SoC-level.

Quoted 3 lines

Will you be able to expand your requirements and explain why can't you manage PM for the general purpose devices like MMC, USB etc in their own device drivers ?

Because it is not possible to remove power from those devices individually.

Say you have a bunch of I/O devices such that you can only remove power from all of them simultaneously (a power domain, that is). Suppose that there is a register such that if a specific value is written to it, power is cut from all of those devices at the same time (and there's an analogous value for restoring power). Then, you can use the generic PM domains framework for power management of those devices.

Suppose, however, that if you write the "cut power" value to the register, your CPU core will lose power too. This case is beyond the scope of the existing generic PM domains framework, because it has to take the CPU power management into account, which is cpuidle in this particular case.

Thanks,
Rafael

Shilimkar, Santosh

2012-05-12 06:32 UTC #8 lore raw

Re: [RFC][PATCH 2/2] PM / Domains: Add preliminary cpuidle support

Quoted 92 lines · Rafael J. Wysocki

Quoted 32 lines · Santosh Shilimkar
Quoted 12 lines · Rafael J. Wysocki
Quoted 11 lines · Santosh Shilimkar
Rafael,
Quoted 8 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
On some systems there are CPU cores located in the same power domains as I/O devices. Then, power can only be removed from the domain if all I/O devices in it are not in use and the CPU core is idle. Add preliminary support for that to the generic PM domains framework.
Quoted 7 lines
Quoted 2 lines
I am just curious to know, what kind of IO devices, you are talking here?
Nothing specific, really. It can be any kind of I/O devices that happen to be in the same power domain. This includes USB, SDHI, MMCIF controllers on the SoC I have in mind in particular.
OK.
These are more of generic devices and actually not related to CPU/CPU clusters as such.
Quoted 7 lines
Quoted 3 lines
And also how those devices linked with CPU low power states apart from being part of same power domain. And is it the power domain or more of voltage domain, we are talking here.
Depending on the definitions I guess. How do you define a power domain and a voltage domain?
Quoted 5 lines
A voltage domain can be a section of the device supplied by a dedicated voltage rail. A voltage domain can have many power-domains like CPU cluster domain, Interconnect domain, peripheral domains. And each power domain then can have many sub-modules like UART, SPI, USB etc
OK, so this is not the level of detail the code in question is about.
In my terminology a power domain is a set of devices such that it only is possible to remove power from (and restore power to) all of them together.
[...]
Quoted 11 lines
Quoted 6 lines
The system I have in mind is designed in such a way that there is a power domain with three subdomains, one of which contains the CPU core and the remaining two contain I/O devices of various kinds. General purpose as well as "core".
I am not sure CPUIDLE is suppose to take care of these kind of general purpose IO's. CPUIDLE should take care of CPU and CPU cluster power management. Any other peripherals as you mentioned should be already have some sort of device drivers and they should be using runtime PM for it, no?
Yes.
Quoted 3 lines
And for the constraints, PM-Qos can be used. So far CPUIDLE core code has maintained that distinction and all the C-state latencies are of the CPU clusters rather than the SOC.
Well, that need not be the case and my patch is about that.
Quoted 4 lines
If you have a voltage rail dependency then that should be handled in the voltage layer/regulator layer. If there is a power domain dependency then the power domain framework should do the use counting yo handle such scenarios.
The power domain framework, though, only covers I/O devices at the moment and this is an attempt to extend it to cover domains containing CPU cores as well as I/O devices.
Quoted 2 lines
Please correct me but, IIUC, your proposal wants to use CPUIDLE for the SOC level power management.
That depends on what you mean by SoC-level.
Quoted 3 lines
Will you be able to expand your requirements and explain why can't you manage PM for the general purpose devices like MMC, USB etc in their own device drivers ?
Because it is not possible to remove power from those devices individually.
Say you have a bunch of I/O devices such that you can only remove power from all of them simultaneously (a power domain, that is). Suppose that there is a register such that if a specific value is written to it, power is cut from all of those devices at the same time (and there's an analogous value for restoring power). Then, you can use the generic PM domains framework for power management of those devices.

Quoted 5 lines

Suppose, however, that if you write the "cut power" value to the register, your CPU core will lose power too. This case is beyond the scope of the existing generic PM domains framework, because it has to take the CPU power management into account, which is cpuidle in this particular case.

Thanks for the explanation and now I understand your hardware and the patch-set intention bit better. Multiple devices in a power domain is very common design and OMAP also has similar concepts. If there are more than one module, a powerdomain can maintain an usecount. And whenever individual modules idle, they will decrements it. And when all module in a power-domain idle, the power-domain can do low power transition.

Your case is bit special because CPU is also part of the same power domain as IO's. Since CPU is part of the root power domain along with peripherals, you want CPU to be the device to cut the power in the end(idle) and all the notifiers prepares the IO's for the 'cut power' which CPU can do in the cpuidle. And then on the reverse chain, notifier will help devices to restore the context so that can resume without any issues.

Well if CPU is added as one more device along with other peripherals, you can still make it work at power domain framework level. This can be debated further but I think, we need an agreement on CPUIDLE exclusivity.

Based on your hardware, say the USB is doing a huge DMA transfer and CPU is not doing anything so it will hit idle thread and can idle. At least your CPU sub-domain can idle because it is quite independent even if you can't cut the power for whole domain. Even with your patchset its not possible to cut the power because one of the device in the domain is busy. There can be scenario, where say SPI is not used for thousands of CPU idle entry/exit so there is no need to worry about its context save/restore for every idle entry/exit.

The way I am looking at your issue is, every device in a power domain including CPU will decrement the powerdomain usecount. This can be handled through runtime PM. When they idle, they can save the context and the context can be restored only when next time they needs to be used. CPU can also decrements the usecount in the idle entry and then check whether the usecound of the common PD is zero. If it is, it can cut the power else just do CPU sub-domain idle.

I might be off track if above can't be managed for your hardware.

Regards
Santosh

Rafael J. Wysocki

author

2012-05-12 19:29 UTC #9 lore raw

Re: [RFC][PATCH 2/2] PM / Domains: Add preliminary cpuidle support

Quoted 4 lines · Santosh

Quoted 3 lines · Rafael J. Wysocki
Quoted 2 lines · Santosh Shilimkar
Quoted 1 line · Rafael J. Wysocki
On Thursday, May 10, 2012, Santosh Shilimkar wrote:

[...]

Quoted 5 lines

Your case is bit special because CPU is also part of the same power domain as IO's. Since CPU is part of the root power domain along with peripherals, you want CPU to be the device to cut the power in the end(idle) and all the notifiers prepares the IO's for the 'cut power' which CPU can do in the cpuidle.

No, no notifiers, please, they are clearly suboptimal.

Quoted 2 lines

And then on the reverse chain, notifier will help devices to restore the context so that can resume without any issues.

The idea here is to avoid using notifiers.

Quoted 3 lines

Well if CPU is added as one more device along with other peripherals, you can still make it work at power domain framework level.

What exactly do you mean by "power domain framework level"? My patch is at the power domain framework level as far as I can say.

Quoted 2 lines

This can be debated further but I think, we need an agreement on CPUIDLE exclusivity.

What do you mean by cpuidle exclusivity? And what agreement?

Quoted 2 lines

Based on your hardware, say the USB is doing a huge DMA transfer and CPU is not doing anything so it will hit idle thread and can idle.

Sure, it will.

Quoted 2 lines

At least your CPU sub-domain can idle because it is quite independent even if you can't cut the power for whole domain.

That's correct. The CPU sub-domain is beyond the scope of my patch.

Quoted 5 lines

Even with your patchset its not possible to cut the power because one of the device in the domain is busy. There can be scenario, where say SPI is not used for thousands of CPU idle entry/exit so there
is no need to worry about its context save/restore for every idle entry/exit.

The CPU idle state I'm referring to is not the only CPU idle state. It is an extra state allowing the CPU to go to even deeper idle that usually if all of the devices in its domain happen to be idle.

Quite obviously there are other idle states along this one that will be used if this extra state is not enabled.

Quoted 2 lines

The way I am looking at your issue is, every device in a power domain including CPU will decrement the powerdomain usecount.

The CPU will not do that, only devices.

Quoted 1 line

This can be handled through runtime PM.

But this patchset is a part of runtime PM! What exactly do you mean?

Quoted 4 lines

When they idle, they can save the context
and the context can be restored only when next time they needs to be used. CPU can also decrements the usecount in the idle entry and then check whether the usecound of the common PD is zero.

This is not how it is supposed to work.

It doesn't make sense for cpuidle to even try to use the "domain" state if it is known that some I/O devices in the domain aren't idle. That's why that state is disabled in my code whenever one (the first) of the I/O devices is resumed. It is then enabled when the last I/O device in the domain goes idle (the states of all devices are saved at this point, to allow cpuidle to use the "domain" state without worrying of the devices).

I'll post a new version of the $subject patch shortly, because I found that the previous one wasn't correct.

Thanks,
Rafael

Shilimkar, Santosh

2012-05-14 06:14 UTC #10 lore raw

Re: [RFC][PATCH 2/2] PM / Domains: Add preliminary cpuidle support

Quoted 12 lines · Rafael J. Wysocki

Quoted 4 lines · Santosh
Quoted 3 lines · Rafael J. Wysocki
Quoted 2 lines · Santosh Shilimkar
Quoted 1 line · Rafael J. Wysocki
On Thursday, May 10, 2012, Santosh Shilimkar wrote:
[...]
Quoted 5 lines
Your case is bit special because CPU is also part of the same power domain as IO's. Since CPU is part of the root power domain along with peripherals, you want CPU to be the device to cut the power in the end(idle) and all the notifiers prepares the IO's for the 'cut power' which CPU can do in the cpuidle.

Quoted 7 lines

No, no notifiers, please, they are clearly suboptimal.
Quoted 2 lines
And then on the reverse chain, notifier will help devices to restore the context so that can resume without any issues.
The idea here is to avoid using notifiers.

Thanks for clarifying that. Somehow I miss-understood this part in first place.

Quoted 28 lines

Quoted 3 lines
Well if CPU is added as one more device along with other peripherals, you can still make it work at power domain framework level.
What exactly do you mean by "power domain framework level"? My patch is at the power domain framework level as far as I can say.
Quoted 2 lines
This can be debated further but I think, we need an agreement on CPUIDLE exclusivity.
What do you mean by cpuidle exclusivity? And what agreement?
Quoted 2 lines
Based on your hardware, say the USB is doing a huge DMA transfer and CPU is not doing anything so it will hit idle thread and can idle.
Sure, it will.
Quoted 2 lines
At least your CPU sub-domain can idle because it is quite independent even if you can't cut the power for whole domain.
That's correct. The CPU sub-domain is beyond the scope of my patch.
Quoted 5 lines
Even with your patchset its not possible to cut the power because one of the device in the domain is busy. There can be scenario, where say SPI is not used for thousands of CPU idle entry/exit so there
is no need to worry about its context save/restore for every idle entry/exit.

Quoted 7 lines

The CPU idle state I'm referring to is not the only CPU idle state. It is an extra state allowing the CPU to go to even deeper idle that usually if all of the devices in its domain happen to be idle.
Quite obviously there are other idle states along this one that will be used if this extra state is not enabled.

That make sense.

Quoted 9 lines

Quoted 2 lines
The way I am looking at your issue is, every device in a power domain including CPU will decrement the powerdomain usecount.
The CPU will not do that, only devices.
Quoted 1 line
This can be handled through runtime PM.
But this patchset is a part of runtime PM! What exactly do you mean?

As I said for some reason I thought you were proposing the idle notifiers which is not the case.

Quoted 5 lines

Quoted 4 lines
When they idle, they can save the context
and the context can be restored only when next time they needs to be used. CPU can also decrements the usecount in the idle entry and then check whether the usecound of the common PD is zero.

Quoted 9 lines

This is not how it is supposed to work.
It doesn't make sense for cpuidle to even try to use the "domain" state if it is known that some I/O devices in the domain aren't idle. That's why that state is disabled in my code whenever one (the first) of the I/O devices is resumed. It is then enabled when the last I/O device in the domain goes idle (the states of all devices are saved at this point, to allow cpuidle to use the "domain" state without worrying of the devices).

Now since the idle notifier confusion ( at least for me) is out, what you proposed seems to be reasonable considering the power domain is shared between multiple devices including CPU.

Regards
Santosh

Rafael J. Wysocki

author

2012-05-12 19:35 UTC #11 lore raw

RFC PATCH 2/2 PM / Domains: Add preliminary cpuidle support

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

This assumes that all CPU cores in the system will be located in the same power domain is supposed to be set up in the following way.

First, the platform is expected to provide a cpuidle driver with one extra state designated for use with the generic PM domains code. This state should be initially disabled and its exit_latency value should be set to whatever time is needed to bring up the CPU core itself after restoring power to it, not including the domain's power on latency. Its .enter() callback should point to a procedure that will remove power from the domain containing the CPU core at the end of the CPU power transition.

Next, the platform should execute genpd_attach_cpuidle() on the PM domain containing the CPU core. That will cause the generic PM domains framework to treat that domain in a special way such that:

* When all devices in the domain have been suspended and it is about to be turned off, the states of the devices will be saved, but power will not be removed from the domain. Instead, the "domain" cpuidle state will be enabled so that power can be removed from the domain when the CPU core is idle and the "domain" state has been chosen as the target by the cpuidle governor. The "domain" state's .enter() callback is then reposible for removing power from the domain.

---
 drivers/base/power/domain.c |  115 +++++++++++++++++++++++++++++++++++++++++++-
 include/linux/pm_domain.h   |   17 ++++++
 2 files changed, 131 insertions(+), 1 deletion(-)

include/linux/pm_domain.h +17 -0

Index: linux/include/linux/pm_domain.h
===================================================================
@@ -15,6 +15,7 @@
 #include <linux/err.h>
 #include <linux/of.h>
 #include <linux/notifier.h>
+#include <linux/cpuidle.h>
 
 enum gpd_status {
 	GPD_STATE_ACTIVE = 0,	/* PM domain is active */

@@ -45,6 +46,11 @@ struct gpd_dev_ops {
 	bool (*active_wakeup)(struct device *dev);
 };
 
+struct gpd_cpu_data {
+	unsigned int saved_exit_latency;
+	struct cpuidle_state *idle_state;
+};
+
 struct generic_pm_domain {
 	struct dev_pm_domain domain;	/* PM domain operations */
 	struct list_head gpd_list_node;	/* Node in the global PM domains list */

@@ -75,6 +81,7 @@ struct generic_pm_domain {
 	bool max_off_time_changed;
 	bool cached_power_down_ok;
 	struct device_node *of_node; /* Node in device tree */
+	struct gpd_cpu_data *cpu_data;
 };
 
 static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)

@@ -155,6 +162,8 @@ extern int pm_genpd_add_callbacks(struct
 				  struct gpd_dev_ops *ops,
 				  struct gpd_timing_data *td);
 extern int __pm_genpd_remove_callbacks(struct device *dev, bool clear_td);
+extern int genpd_attach_cpuidle(struct generic_pm_domain *genpd, int state);
+extern int genpd_detach_cpuidle(struct generic_pm_domain *genpd);
 extern void pm_genpd_init(struct generic_pm_domain *genpd,
 			  struct dev_power_governor *gov, bool is_off);

@@ -211,6 +220,14 @@ static inline int __pm_genpd_remove_call
 {
 	return -ENOSYS;
 }
+static inline int genpd_attach_cpuidle(struct generic_pm_domain *genpd, int st)
+{
+	return -ENOSYS;
+}
+static inline int genpd_detach_cpuidle(struct generic_pm_domain *genpd)
+{
+	return -ENOSYS;
+}
 static inline void pm_genpd_init(struct generic_pm_domain *genpd,
 				 struct dev_power_governor *gov, bool is_off)
 {

drivers/base/power/domain.c +114 -1

145 lines not on this page · raw message

Rafael J. Wysocki

author

2012-05-16 20:24 UTC #12 lore raw

RFC PATCH 2/2 PM / Domains: Add preliminary cpuidle support

From: Rafael J. Wysocki <[email protected]>

Next, the platform should execute genpd_attach_cpuidle() on the PM domain containing the CPU core. That will cause the generic PM domains framework to treat that domain in a special way such that:

---

Since pm_genpd_poweroff() doesn't modify the masters' subdomain reference counters if genpd->cpu_data is set, __pm_genpd_poweron() should avoid modifying them too in that case.

Thanks,
Rafael

---
 drivers/base/power/domain.c |  117 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h   |   17 ++++++
 2 files changed, 134 insertions(+)

include/linux/pm_domain.h +17 -0

Index: linux/include/linux/pm_domain.h
===================================================================
@@ -15,6 +15,7 @@
 #include <linux/err.h>
 #include <linux/of.h>
 #include <linux/notifier.h>
+#include <linux/cpuidle.h>
 
 enum gpd_status {
 	GPD_STATE_ACTIVE = 0,	/* PM domain is active */

@@ -45,6 +46,11 @@ struct gpd_dev_ops {
 	bool (*active_wakeup)(struct device *dev);
 };
 
+struct gpd_cpu_data {
+	unsigned int saved_exit_latency;
+	struct cpuidle_state *idle_state;
+};
+
 struct generic_pm_domain {
 	struct dev_pm_domain domain;	/* PM domain operations */
 	struct list_head gpd_list_node;	/* Node in the global PM domains list */

@@ -75,6 +81,7 @@ struct generic_pm_domain {
 	bool max_off_time_changed;
 	bool cached_power_down_ok;
 	struct device_node *of_node; /* Node in device tree */
+	struct gpd_cpu_data *cpu_data;
 };
 
 static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)

@@ -155,6 +162,8 @@ extern int pm_genpd_add_callbacks(struct
 				  struct gpd_dev_ops *ops,
 				  struct gpd_timing_data *td);
 extern int __pm_genpd_remove_callbacks(struct device *dev, bool clear_td);
+extern int genpd_attach_cpuidle(struct generic_pm_domain *genpd, int state);
+extern int genpd_detach_cpuidle(struct generic_pm_domain *genpd);
 extern void pm_genpd_init(struct generic_pm_domain *genpd,
 			  struct dev_power_governor *gov, bool is_off);

@@ -211,6 +220,14 @@ static inline int __pm_genpd_remove_call
 {
 	return -ENOSYS;
 }
+static inline int genpd_attach_cpuidle(struct generic_pm_domain *genpd, int st)
+{
+	return -ENOSYS;
+}
+static inline int genpd_detach_cpuidle(struct generic_pm_domain *genpd)
+{
+	return -ENOSYS;
+}
 static inline void pm_genpd_init(struct generic_pm_domain *genpd,
 				 struct dev_power_governor *gov, bool is_off)
 {

drivers/base/power/domain.c +117 -0

153 lines not on this page · raw message

Kevin Hilman

2012-05-23 22:23 UTC #13 lore raw

Re: [Update 2x][RFC][PATCH 2/2] PM / Domains: Add preliminary cpuidle support

Hi Rafael,

Quoted 33 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>
On some systems there are CPU cores located in the same power domains as I/O devices. Then, power can only be removed from the domain if all I/O devices in it are not in use and the CPU core is idle. Add preliminary support for that to the generic PM domains framework.
First, the platform is expected to provide a cpuidle driver with one extra state designated for use with the generic PM domains code. This state should be initially disabled and its exit_latency value should be set to whatever time is needed to bring up the CPU core itself after restoring power to it, not including the domain's power on latency. Its .enter() callback should point to a procedure that will remove power from the domain containing the CPU core at the end of the CPU power transition.
The remaining characteristics of the extra cpuidle state, referred to as the "domain" cpuidle state below, (e.g. power usage, target residency) should be populated in accordance with the properties of the hardware.
Next, the platform should execute genpd_attach_cpuidle() on the PM domain containing the CPU core. That will cause the generic PM domains framework to treat that domain in a special way such that:
* When all devices in the domain have been suspended and it is about to be turned off, the states of the devices will be saved, but power will not be removed from the domain. Instead, the "domain" cpuidle state will be enabled so that power can be removed from the domain when the CPU core is idle and the state has been chosen as the target by the cpuidle governor.

Quoted 5 lines

* When the first I/O device in the domain is resumed and __pm_genpd_poweron(() is called for the first time after power has been removed from the domain, the "domain" cpuidle state will be disabled to avoid subsequent surprise power removals via cpuidle.

This looks like a good approach. I like that it keeps a pretty clean separation between CPUidle and PM domains.

My only comment would be that the recalc of the exit_latency should be described a bit more. Specifically, I'm not sure why it's adjused at every genpd poweron. At first I thought it was just supposed to be adjusted upon attach, then adjusted back on detatch, but with the recalc also in every poweron, I'm a little confused. Care to clarify?

Thanks,

Kevin

Rafael J. Wysocki

author

2012-05-24 16:12 UTC #14 lore raw

Re: [Update 2x][RFC][PATCH 2/2] PM / Domains: Add preliminary cpuidle support

Quoted 43 lines · Kevin Hilman

Hi Rafael,
Quoted 33 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
On some systems there are CPU cores located in the same power domains as I/O devices. Then, power can only be removed from the domain if all I/O devices in it are not in use and the CPU core is idle. Add preliminary support for that to the generic PM domains framework.
First, the platform is expected to provide a cpuidle driver with one extra state designated for use with the generic PM domains code. This state should be initially disabled and its exit_latency value should be set to whatever time is needed to bring up the CPU core itself after restoring power to it, not including the domain's power on latency. Its .enter() callback should point to a procedure that will remove power from the domain containing the CPU core at the end of the CPU power transition.
The remaining characteristics of the extra cpuidle state, referred to as the "domain" cpuidle state below, (e.g. power usage, target residency) should be populated in accordance with the properties of the hardware.
Next, the platform should execute genpd_attach_cpuidle() on the PM domain containing the CPU core. That will cause the generic PM domains framework to treat that domain in a special way such that:
* When all devices in the domain have been suspended and it is about to be turned off, the states of the devices will be saved, but power will not be removed from the domain. Instead, the "domain" cpuidle state will be enabled so that power can be removed from the domain when the CPU core is idle and the state has been chosen as the target by the cpuidle governor.
Quoted 5 lines
* When the first I/O device in the domain is resumed and __pm_genpd_poweron(() is called for the first time after power has been removed from the domain, the "domain" cpuidle state will be disabled to avoid subsequent surprise power removals via cpuidle.

Quoted 8 lines

This looks like a good approach. I like that it keeps a pretty clean separation between CPUidle and PM domains.
My only comment would be that the recalc of the exit_latency should be described a bit more. Specifically, I'm not sure why it's adjused at every genpd poweron. At first I thought it was just supposed to be adjusted upon attach, then adjusted back on detatch, but with the recalc also in every poweron, I'm a little confused. Care to clarify?

The problem is that the PM domains code measures the time it takes to power off a domain and updates its power on latency parameter if the measured time is greater. This is done for PM QoS to operate on realistic numbers (most of the time at least).

Of course, this also affects the CPU wakeup latency if the wakeup involves turning a domain on.

Thanks,
Rafael

Kevin Hilman

2012-05-24 23:59 UTC #15 lore raw

Re: [Update 2x][RFC][PATCH 2/2] PM / Domains: Add preliminary cpuidle support

Quoted 53 lines · Rafael J. Wysocki

Quoted 43 lines · Kevin Hilman
Hi Rafael,
Quoted 33 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
On some systems there are CPU cores located in the same power domains as I/O devices. Then, power can only be removed from the domain if all I/O devices in it are not in use and the CPU core is idle. Add preliminary support for that to the generic PM domains framework.
First, the platform is expected to provide a cpuidle driver with one extra state designated for use with the generic PM domains code. This state should be initially disabled and its exit_latency value should be set to whatever time is needed to bring up the CPU core itself after restoring power to it, not including the domain's power on latency. Its .enter() callback should point to a procedure that will remove power from the domain containing the CPU core at the end of the CPU power transition.
The remaining characteristics of the extra cpuidle state, referred to as the "domain" cpuidle state below, (e.g. power usage, target residency) should be populated in accordance with the properties of the hardware.
Next, the platform should execute genpd_attach_cpuidle() on the PM domain containing the CPU core. That will cause the generic PM domains framework to treat that domain in a special way such that:
* When all devices in the domain have been suspended and it is about to be turned off, the states of the devices will be saved, but power will not be removed from the domain. Instead, the "domain" cpuidle state will be enabled so that power can be removed from the domain when the CPU core is idle and the state has been chosen as the target by the cpuidle governor.
Quoted 5 lines
* When the first I/O device in the domain is resumed and __pm_genpd_poweron(() is called for the first time after power has been removed from the domain, the "domain" cpuidle state will be disabled to avoid subsequent surprise power removals via cpuidle.
Quoted 8 lines
This looks like a good approach. I like that it keeps a pretty clean separation between CPUidle and PM domains.
My only comment would be that the recalc of the exit_latency should be described a bit more. Specifically, I'm not sure why it's adjused at every genpd poweron. At first I thought it was just supposed to be adjusted upon attach, then adjusted back on detatch, but with the recalc also in every poweron, I'm a little confused. Care to clarify?

Quoted 4 lines

The problem is that the PM domains code measures the time it takes to power off a domain and updates its power on latency parameter if the measured time is greater. This is done for PM QoS to operate on realistic numbers (most of the time at least).

OK, I see. Maybe clarifying that in the changelog would help make that clearer.

Quoted 2 lines

Of course, this also affects the CPU wakeup latency if the wakeup involves turning a domain on.

Right.

Kevin

Rafael J. Wysocki

author

2012-05-28 21:45 UTC #16 lore raw

PATCH 2/2 PM / Domains: Add preliminary support for cpuidle

Quoted 55 lines · Kevin Hilman

Quoted 53 lines · Rafael J. Wysocki
Quoted 51 lines · Kevin Hilman
Hi Rafael,
Quoted 33 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
On some systems there are CPU cores located in the same power domains as I/O devices. Then, power can only be removed from the domain if all I/O devices in it are not in use and the CPU core is idle. Add preliminary support for that to the generic PM domains framework.
First, the platform is expected to provide a cpuidle driver with one extra state designated for use with the generic PM domains code. This state should be initially disabled and its exit_latency value should be set to whatever time is needed to bring up the CPU core itself after restoring power to it, not including the domain's power on latency. Its .enter() callback should point to a procedure that will remove power from the domain containing the CPU core at the end of the CPU power transition.
The remaining characteristics of the extra cpuidle state, referred to as the "domain" cpuidle state below, (e.g. power usage, target residency) should be populated in accordance with the properties of the hardware.
Next, the platform should execute genpd_attach_cpuidle() on the PM domain containing the CPU core. That will cause the generic PM domains framework to treat that domain in a special way such that:
* When all devices in the domain have been suspended and it is about to be turned off, the states of the devices will be saved, but power will not be removed from the domain. Instead, the "domain" cpuidle state will be enabled so that power can be removed from the domain when the CPU core is idle and the state has been chosen as the target by the cpuidle governor.
Quoted 5 lines
* When the first I/O device in the domain is resumed and __pm_genpd_poweron(() is called for the first time after power has been removed from the domain, the "domain" cpuidle state will be disabled to avoid subsequent surprise power removals via cpuidle.
This looks like a good approach. I like that it keeps a pretty clean separation between CPUidle and PM domains.
My only comment would be that the recalc of the exit_latency should be described a bit more. Specifically, I'm not sure why it's adjused at every genpd poweron. At first I thought it was just supposed to be adjusted upon attach, then adjusted back on detatch, but with the recalc also in every poweron, I'm a little confused. Care to clarify?

Quoted 7 lines

Quoted 4 lines
The problem is that the PM domains code measures the time it takes to power off a domain and updates its power on latency parameter if the measured time is greater. This is done for PM QoS to operate on realistic numbers (most of the time at least).
OK, I see. Maybe clarifying that in the changelog would help make that clearer.

Sure. I hope the updated changelog below is better.

Thanks,
Rafael

---
From: Rafael J. Wysocki <[email protected]>
Subject: PM / Domains: Add preliminary support for cpuidle

Next, the platform should execute genpd_attach_cpuidle() on the PM domain containing the CPU core. That will cause the generic PM domains framework to treat that domain in a special way such that:

The effective exit_latency value of the "domain" cpuidle state depends on the time needed to bring up the CPU core itself after restoring power to it as well as on the power on latency of the domain containing the CPU core. Thus the "domain" cpuidle state's exit_latency has to be recomputed every time the domain's power on latency is updated, which may happen every time power is restored to the domain, if the measured power on latency is greater than the latency stored in the corresponding generic_pm_domain structure.

Signed-off-by Rafael J. Wysocki <[email protected]>

---
 drivers/base/power/domain.c |  117 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h   |   17 ++++++
 2 files changed, 134 insertions(+)

include/linux/pm_domain.h +17 -0

Index: linux/include/linux/pm_domain.h
===================================================================
@@ -15,6 +15,7 @@
 #include <linux/err.h>
 #include <linux/of.h>
 #include <linux/notifier.h>
+#include <linux/cpuidle.h>
 
 enum gpd_status {
 	GPD_STATE_ACTIVE = 0,	/* PM domain is active */

@@ -45,6 +46,11 @@ struct gpd_dev_ops {
 	bool (*active_wakeup)(struct device *dev);
 };
 
+struct gpd_cpu_data {
+	unsigned int saved_exit_latency;
+	struct cpuidle_state *idle_state;
+};
+
 struct generic_pm_domain {
 	struct dev_pm_domain domain;	/* PM domain operations */
 	struct list_head gpd_list_node;	/* Node in the global PM domains list */

@@ -75,6 +81,7 @@ struct generic_pm_domain {
 	bool max_off_time_changed;
 	bool cached_power_down_ok;
 	struct device_node *of_node; /* Node in device tree */
+	struct gpd_cpu_data *cpu_data;
 };
 
 static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)

@@ -155,6 +162,8 @@ extern int pm_genpd_add_callbacks(struct
 				  struct gpd_dev_ops *ops,
 				  struct gpd_timing_data *td);
 extern int __pm_genpd_remove_callbacks(struct device *dev, bool clear_td);
+extern int genpd_attach_cpuidle(struct generic_pm_domain *genpd, int state);
+extern int genpd_detach_cpuidle(struct generic_pm_domain *genpd);
 extern void pm_genpd_init(struct generic_pm_domain *genpd,
 			  struct dev_power_governor *gov, bool is_off);

@@ -211,6 +220,14 @@ static inline int __pm_genpd_remove_call
 {
 	return -ENOSYS;
 }
+static inline int genpd_attach_cpuidle(struct generic_pm_domain *genpd, int st)
+{
+	return -ENOSYS;
+}
+static inline int genpd_detach_cpuidle(struct generic_pm_domain *genpd)
+{
+	return -ENOSYS;
+}
 static inline void pm_genpd_init(struct generic_pm_domain *genpd,
 				 struct dev_power_governor *gov, bool is_off)
 {

drivers/base/power/domain.c +117 -0

153 lines not on this page · raw message

Kevin Hilman

Reviewed-by on v1 2/2

2012-05-30 22:18 UTC #17 lore raw

Re: [Update 3x][PATCH 2/2] PM / Domains: Add preliminary support for cpuidle

Quoted 61 lines · Rafael J. Wysocki

Quoted 60 lines · Kevin Hilman
Quoted 57 lines · Rafael J. Wysocki
Quoted 43 lines · Kevin Hilman
Hi Rafael,
Quoted 33 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
On some systems there are CPU cores located in the same power domains as I/O devices. Then, power can only be removed from the domain if all I/O devices in it are not in use and the CPU core is idle. Add preliminary support for that to the generic PM domains framework.
First, the platform is expected to provide a cpuidle driver with one extra state designated for use with the generic PM domains code. This state should be initially disabled and its exit_latency value should be set to whatever time is needed to bring up the CPU core itself after restoring power to it, not including the domain's power on latency. Its .enter() callback should point to a procedure that will remove power from the domain containing the CPU core at the end of the CPU power transition.
The remaining characteristics of the extra cpuidle state, referred to as the "domain" cpuidle state below, (e.g. power usage, target residency) should be populated in accordance with the properties of the hardware.
Next, the platform should execute genpd_attach_cpuidle() on the PM domain containing the CPU core. That will cause the generic PM domains framework to treat that domain in a special way such that:
* When all devices in the domain have been suspended and it is about to be turned off, the states of the devices will be saved, but power will not be removed from the domain. Instead, the "domain" cpuidle state will be enabled so that power can be removed from the domain when the CPU core is idle and the state has been chosen as the target by the cpuidle governor.
Quoted 5 lines
* When the first I/O device in the domain is resumed and __pm_genpd_poweron(() is called for the first time after power has been removed from the domain, the "domain" cpuidle state will be disabled to avoid subsequent surprise power removals via cpuidle.
Quoted 8 lines
This looks like a good approach. I like that it keeps a pretty clean separation between CPUidle and PM domains.
My only comment would be that the recalc of the exit_latency should be described a bit more. Specifically, I'm not sure why it's adjused at every genpd poweron. At first I thought it was just supposed to be adjusted upon attach, then adjusted back on detatch, but with the recalc also in every poweron, I'm a little confused. Care to clarify?
The problem is that the PM domains code measures the time it takes to power off a domain and updates its power on latency parameter if the measured time is greater. This is done for PM QoS to operate on realistic numbers (most of the time at least).

Quoted 4 lines

Quoted 2 lines
OK, I see. Maybe clarifying that in the changelog would help make that clearer.
Sure. I hope the updated changelog below is better.

Perfect, thanks.

Reviewed-by Kevin Hilman <[email protected]>

Rafael J. Wysocki

author

2012-05-31 18:57 UTC #18 lore raw

Re: [Update 3x][PATCH 2/2] PM / Domains: Add preliminary support for cpuidle

Quoted 63 lines · Kevin Hilman

Quoted 61 lines · Rafael J. Wysocki
Quoted 60 lines · Kevin Hilman
Quoted 57 lines · Rafael J. Wysocki
Quoted 43 lines · Kevin Hilman
Hi Rafael,
Quoted 33 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
On some systems there are CPU cores located in the same power domains as I/O devices. Then, power can only be removed from the domain if all I/O devices in it are not in use and the CPU core is idle. Add preliminary support for that to the generic PM domains framework.
First, the platform is expected to provide a cpuidle driver with one extra state designated for use with the generic PM domains code. This state should be initially disabled and its exit_latency value should be set to whatever time is needed to bring up the CPU core itself after restoring power to it, not including the domain's power on latency. Its .enter() callback should point to a procedure that will remove power from the domain containing the CPU core at the end of the CPU power transition.
The remaining characteristics of the extra cpuidle state, referred to as the "domain" cpuidle state below, (e.g. power usage, target residency) should be populated in accordance with the properties of the hardware.
Next, the platform should execute genpd_attach_cpuidle() on the PM domain containing the CPU core. That will cause the generic PM domains framework to treat that domain in a special way such that:
* When all devices in the domain have been suspended and it is about to be turned off, the states of the devices will be saved, but power will not be removed from the domain. Instead, the "domain" cpuidle state will be enabled so that power can be removed from the domain when the CPU core is idle and the state has been chosen as the target by the cpuidle governor.
Quoted 5 lines
* When the first I/O device in the domain is resumed and __pm_genpd_poweron(() is called for the first time after power has been removed from the domain, the "domain" cpuidle state will be disabled to avoid subsequent surprise power removals via cpuidle.
Quoted 8 lines
This looks like a good approach. I like that it keeps a pretty clean separation between CPUidle and PM domains.
My only comment would be that the recalc of the exit_latency should be described a bit more. Specifically, I'm not sure why it's adjused at every genpd poweron. At first I thought it was just supposed to be adjusted upon attach, then adjusted back on detatch, but with the recalc also in every poweron, I'm a little confused. Care to clarify?
The problem is that the PM domains code measures the time it takes to power off a domain and updates its power on latency parameter if the measured time is greater. This is done for PM QoS to operate on realistic numbers (most of the time at least).

Quoted 8 lines

Quoted 4 lines
Quoted 2 lines
OK, I see. Maybe clarifying that in the changelog would help make that clearer.
Sure. I hope the updated changelog below is better.
Perfect, thanks.
Reviewed-by Kevin Hilman <[email protected]>

Cool, thanks!