Re: [PATCH 3/3] ARM: OMAP3: PM: cpuidle: optimize the clkdm idle latency in C1 state
Jean Pihet jean.pi...@newoldbits.com writes: Hi Rajendra, On Wed, Jun 20, 2012 at 10:57 AM, Rajendra Nayak rna...@ti.com wrote: Jean, On Wednesday 20 June 2012 02:22 PM, Rajendra Nayak wrote: Hi Jean, On Wednesday 20 June 2012 02:16 PM, Rajendra Nayak wrote: On Wednesday 20 June 2012 02:01 PM, Jean Pihet wrote: Hi Rajendra, On Wed, Jun 20, 2012 at 10:19 AM, Rajendra Nayakrna...@ti.com wrote: Hi Jean, On Friday 01 June 2012 08:41 PM, Jean Pihet wrote: For a power domain to idle all the clock domains in it must idle. This patch implements an optimization of the cpuidle code by denying and later allowing only the first registered clock domain of a power domain, and so optimizes the latency of the low power code. How much do we really save doing this? I understand what you are doing by looking at the patch but the changelog seems very confusing. The gain is on the registers accesses and the internal PRCM state machine. If needed the changelog can be updated. Can you explain a bit more on which register accesses are you talking about? and some more on the PRCM state machine. never mind, I looked at the patch again and then the cpuidle code and figured what you are doing. Makes sense to me now :-) Ok! How do you like this updated changelog, I just added one more line. For a power domain to idle all the clock domains in it must idle. Denying just *one* clkdm in a pwrdm from idling should have the same effect as denying *all*. This patch implements an optimization of the cpuidle code by denying and later allowing only the first registered clock domain of a power domain, and so optimizes the latency of the low power code. That looks great! Kevin, Can you take this change still in your for_3.6/pm/performance branch? Sorry, too late. Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] ARM: OMAP3: PM: cpuidle: optimize the clkdm idle latency in C1 state
Hi Jean,
On Friday 01 June 2012 08:41 PM, Jean Pihet wrote:
For a power domain to idle all the clock domains in it must idle.
This patch implements an optimization of the cpuidle code by
denying and later allowing only the first registered clock domain
of a power domain, and so optimizes the latency of the low power code.
How much do we really save doing this? I understand what you are doing
by looking at the patch but the changelog seems very confusing.
What the patch does is get rid of a few function indirection
and assumes there is only *one* clkdm for mpu and core. Is that right?
regards,
Rajendra
The functions _cpuidle_allow_idle and _cpuidle_deny_idle are
not used anymore and so are removed.
Signed-off-by: Jean Pihetj-pi...@ti.com
---
arch/arm/mach-omap2/cpuidle34xx.c | 22 --
1 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c
b/arch/arm/mach-omap2/cpuidle34xx.c
index 2e2f1c6..e6ae3fe 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -77,20 +77,6 @@ static struct omap3_idle_statedata omap3_idle_data[] = {
static struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
-static int _cpuidle_allow_idle(struct powerdomain *pwrdm,
- struct clockdomain *clkdm)
-{
- clkdm_allow_idle(clkdm);
- return 0;
-}
-
-static int _cpuidle_deny_idle(struct powerdomain *pwrdm,
- struct clockdomain *clkdm)
-{
- clkdm_deny_idle(clkdm);
- return 0;
-}
-
static int __omap3_enter_idle(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
@@ -108,8 +94,8 @@ static int __omap3_enter_idle(struct cpuidle_device *dev,
/* Deny idle for C1 */
if (index == 0) {
- pwrdm_for_each_clkdm(mpu_pd, _cpuidle_deny_idle);
- pwrdm_for_each_clkdm(core_pd, _cpuidle_deny_idle);
+ clkdm_deny_idle(mpu_pd-pwrdm_clkdms[0]);
+ clkdm_deny_idle(core_pd-pwrdm_clkdms[0]);
}
/*
@@ -131,8 +117,8 @@ static int __omap3_enter_idle(struct cpuidle_device *dev,
/* Re-allow idle for C1 */
if (index == 0) {
- pwrdm_for_each_clkdm(mpu_pd, _cpuidle_allow_idle);
- pwrdm_for_each_clkdm(core_pd, _cpuidle_allow_idle);
+ clkdm_allow_idle(mpu_pd-pwrdm_clkdms[0]);
+ clkdm_allow_idle(core_pd-pwrdm_clkdms[0]);
}
return_sleep_time:
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] ARM: OMAP3: PM: cpuidle: optimize the clkdm idle latency in C1 state
Hi Rajendra,
On Wed, Jun 20, 2012 at 10:19 AM, Rajendra Nayak rna...@ti.com wrote:
Hi Jean,
On Friday 01 June 2012 08:41 PM, Jean Pihet wrote:
For a power domain to idle all the clock domains in it must idle.
This patch implements an optimization of the cpuidle code by
denying and later allowing only the first registered clock domain
of a power domain, and so optimizes the latency of the low power code.
How much do we really save doing this? I understand what you are doing
by looking at the patch but the changelog seems very confusing.
The gain is on the registers accesses and the internal PRCM state machine.
If needed the changelog can be updated.
What the patch does is get rid of a few function indirection
and assumes there is only *one* clkdm for mpu and core. Is that right?
Not exactly. The patch does not assume that there is only one clkdm
per power domain. It just allows or denies only one clkdm to idle,
which has the same effect at the power domain level.
Thanks for reviewing,
Jean
regards,
Rajendra
The functions _cpuidle_allow_idle and _cpuidle_deny_idle are
not used anymore and so are removed.
Signed-off-by: Jean Pihetj-pi...@ti.com
---
arch/arm/mach-omap2/cpuidle34xx.c | 22 --
1 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c
b/arch/arm/mach-omap2/cpuidle34xx.c
index 2e2f1c6..e6ae3fe 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -77,20 +77,6 @@ static struct omap3_idle_statedata omap3_idle_data[] =
{
static struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
-static int _cpuidle_allow_idle(struct powerdomain *pwrdm,
- struct clockdomain *clkdm)
-{
- clkdm_allow_idle(clkdm);
- return 0;
-}
-
-static int _cpuidle_deny_idle(struct powerdomain *pwrdm,
- struct clockdomain *clkdm)
-{
- clkdm_deny_idle(clkdm);
- return 0;
-}
-
static int __omap3_enter_idle(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
@@ -108,8 +94,8 @@ static int __omap3_enter_idle(struct cpuidle_device
*dev,
/* Deny idle for C1 */
if (index == 0) {
- pwrdm_for_each_clkdm(mpu_pd, _cpuidle_deny_idle);
- pwrdm_for_each_clkdm(core_pd, _cpuidle_deny_idle);
+ clkdm_deny_idle(mpu_pd-pwrdm_clkdms[0]);
+ clkdm_deny_idle(core_pd-pwrdm_clkdms[0]);
}
/*
@@ -131,8 +117,8 @@ static int __omap3_enter_idle(struct cpuidle_device
*dev,
/* Re-allow idle for C1 */
if (index == 0) {
- pwrdm_for_each_clkdm(mpu_pd, _cpuidle_allow_idle);
- pwrdm_for_each_clkdm(core_pd, _cpuidle_allow_idle);
+ clkdm_allow_idle(mpu_pd-pwrdm_clkdms[0]);
+ clkdm_allow_idle(core_pd-pwrdm_clkdms[0]);
}
return_sleep_time:
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] ARM: OMAP3: PM: cpuidle: optimize the clkdm idle latency in C1 state
On Wednesday 20 June 2012 02:01 PM, Jean Pihet wrote:
Hi Rajendra,
On Wed, Jun 20, 2012 at 10:19 AM, Rajendra Nayakrna...@ti.com wrote:
Hi Jean,
On Friday 01 June 2012 08:41 PM, Jean Pihet wrote:
For a power domain to idle all the clock domains in it must idle.
This patch implements an optimization of the cpuidle code by
denying and later allowing only the first registered clock domain
of a power domain, and so optimizes the latency of the low power code.
How much do we really save doing this? I understand what you are doing
by looking at the patch but the changelog seems very confusing.
The gain is on the registers accesses and the internal PRCM state machine.
If needed the changelog can be updated.
Can you explain a bit more on which register accesses are you talking
about? and some more on the PRCM state machine.
What the patch does is get rid of a few function indirection
and assumes there is only *one* clkdm for mpu and core. Is that right?
Not exactly. The patch does not assume that there is only one clkdm
per power domain. It just allows or denies only one clkdm to idle,
which has the same effect at the power domain level.
Ok, so lets assume mpu on OMAP3 has 2 clkdms, and you allow only one
of them to idle. Will that have the same effect at the power domain
level?
The first line of the change log says For a power domain to idle *all*
the clock domains in it must idle and now you say allowing only *one*
clkdm to idle should have the same effect at the power domain level.
I am confused.
Thanks for reviewing,
Jean
regards,
Rajendra
The functions _cpuidle_allow_idle and _cpuidle_deny_idle are
not used anymore and so are removed.
Signed-off-by: Jean Pihetj-pi...@ti.com
---
arch/arm/mach-omap2/cpuidle34xx.c | 22 --
1 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c
b/arch/arm/mach-omap2/cpuidle34xx.c
index 2e2f1c6..e6ae3fe 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -77,20 +77,6 @@ static struct omap3_idle_statedata omap3_idle_data[] =
{
static struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
-static int _cpuidle_allow_idle(struct powerdomain *pwrdm,
- struct clockdomain *clkdm)
-{
- clkdm_allow_idle(clkdm);
- return 0;
-}
-
-static int _cpuidle_deny_idle(struct powerdomain *pwrdm,
- struct clockdomain *clkdm)
-{
- clkdm_deny_idle(clkdm);
- return 0;
-}
-
static int __omap3_enter_idle(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
@@ -108,8 +94,8 @@ static int __omap3_enter_idle(struct cpuidle_device
*dev,
/* Deny idle for C1 */
if (index == 0) {
- pwrdm_for_each_clkdm(mpu_pd, _cpuidle_deny_idle);
- pwrdm_for_each_clkdm(core_pd, _cpuidle_deny_idle);
+ clkdm_deny_idle(mpu_pd-pwrdm_clkdms[0]);
+ clkdm_deny_idle(core_pd-pwrdm_clkdms[0]);
}
/*
@@ -131,8 +117,8 @@ static int __omap3_enter_idle(struct cpuidle_device
*dev,
/* Re-allow idle for C1 */
if (index == 0) {
- pwrdm_for_each_clkdm(mpu_pd, _cpuidle_allow_idle);
- pwrdm_for_each_clkdm(core_pd, _cpuidle_allow_idle);
+ clkdm_allow_idle(mpu_pd-pwrdm_clkdms[0]);
+ clkdm_allow_idle(core_pd-pwrdm_clkdms[0]);
}
return_sleep_time:
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] ARM: OMAP3: PM: cpuidle: optimize the clkdm idle latency in C1 state
Hi Jean,
On Wednesday 20 June 2012 02:16 PM, Rajendra Nayak wrote:
On Wednesday 20 June 2012 02:01 PM, Jean Pihet wrote:
Hi Rajendra,
On Wed, Jun 20, 2012 at 10:19 AM, Rajendra Nayakrna...@ti.com wrote:
Hi Jean,
On Friday 01 June 2012 08:41 PM, Jean Pihet wrote:
For a power domain to idle all the clock domains in it must idle.
This patch implements an optimization of the cpuidle code by
denying and later allowing only the first registered clock domain
of a power domain, and so optimizes the latency of the low power code.
How much do we really save doing this? I understand what you are doing
by looking at the patch but the changelog seems very confusing.
The gain is on the registers accesses and the internal PRCM state
machine.
If needed the changelog can be updated.
Can you explain a bit more on which register accesses are you talking
about? and some more on the PRCM state machine.
never mind, I looked at the patch again and then the cpuidle code and
figured what you are doing. Makes sense to me now :-)
regards,
Rajendra
What the patch does is get rid of a few function indirection
and assumes there is only *one* clkdm for mpu and core. Is that right?
Not exactly. The patch does not assume that there is only one clkdm
per power domain. It just allows or denies only one clkdm to idle,
which has the same effect at the power domain level.
Ok, so lets assume mpu on OMAP3 has 2 clkdms, and you allow only one
of them to idle. Will that have the same effect at the power domain
level?
The first line of the change log says For a power domain to idle *all*
the clock domains in it must idle and now you say allowing only *one*
clkdm to idle should have the same effect at the power domain level.
I am confused.
Thanks for reviewing,
Jean
regards,
Rajendra
The functions _cpuidle_allow_idle and _cpuidle_deny_idle are
not used anymore and so are removed.
Signed-off-by: Jean Pihetj-pi...@ti.com
---
arch/arm/mach-omap2/cpuidle34xx.c | 22 --
1 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c
b/arch/arm/mach-omap2/cpuidle34xx.c
index 2e2f1c6..e6ae3fe 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -77,20 +77,6 @@ static struct omap3_idle_statedata
omap3_idle_data[] =
{
static struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
-static int _cpuidle_allow_idle(struct powerdomain *pwrdm,
- struct clockdomain *clkdm)
-{
- clkdm_allow_idle(clkdm);
- return 0;
-}
-
-static int _cpuidle_deny_idle(struct powerdomain *pwrdm,
- struct clockdomain *clkdm)
-{
- clkdm_deny_idle(clkdm);
- return 0;
-}
-
static int __omap3_enter_idle(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
@@ -108,8 +94,8 @@ static int __omap3_enter_idle(struct cpuidle_device
*dev,
/* Deny idle for C1 */
if (index == 0) {
- pwrdm_for_each_clkdm(mpu_pd, _cpuidle_deny_idle);
- pwrdm_for_each_clkdm(core_pd, _cpuidle_deny_idle);
+ clkdm_deny_idle(mpu_pd-pwrdm_clkdms[0]);
+ clkdm_deny_idle(core_pd-pwrdm_clkdms[0]);
}
/*
@@ -131,8 +117,8 @@ static int __omap3_enter_idle(struct cpuidle_device
*dev,
/* Re-allow idle for C1 */
if (index == 0) {
- pwrdm_for_each_clkdm(mpu_pd, _cpuidle_allow_idle);
- pwrdm_for_each_clkdm(core_pd, _cpuidle_allow_idle);
+ clkdm_allow_idle(mpu_pd-pwrdm_clkdms[0]);
+ clkdm_allow_idle(core_pd-pwrdm_clkdms[0]);
}
return_sleep_time:
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] ARM: OMAP3: PM: cpuidle: optimize the clkdm idle latency in C1 state
Jean, On Wednesday 20 June 2012 02:22 PM, Rajendra Nayak wrote: Hi Jean, On Wednesday 20 June 2012 02:16 PM, Rajendra Nayak wrote: On Wednesday 20 June 2012 02:01 PM, Jean Pihet wrote: Hi Rajendra, On Wed, Jun 20, 2012 at 10:19 AM, Rajendra Nayakrna...@ti.com wrote: Hi Jean, On Friday 01 June 2012 08:41 PM, Jean Pihet wrote: For a power domain to idle all the clock domains in it must idle. This patch implements an optimization of the cpuidle code by denying and later allowing only the first registered clock domain of a power domain, and so optimizes the latency of the low power code. How much do we really save doing this? I understand what you are doing by looking at the patch but the changelog seems very confusing. The gain is on the registers accesses and the internal PRCM state machine. If needed the changelog can be updated. Can you explain a bit more on which register accesses are you talking about? and some more on the PRCM state machine. never mind, I looked at the patch again and then the cpuidle code and figured what you are doing. Makes sense to me now :-) How do you like this updated changelog, I just added one more line. For a power domain to idle all the clock domains in it must idle. Denying just *one* clkdm in a pwrdm from idling should have the same effect as denying *all*. This patch implements an optimization of the cpuidle code by denying and later allowing only the first registered clock domain of a power domain, and so optimizes the latency of the low power code. regards, Rajendra -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] ARM: OMAP3: PM: cpuidle: optimize the clkdm idle latency in C1 state
Hi Rajendra, On Wed, Jun 20, 2012 at 10:57 AM, Rajendra Nayak rna...@ti.com wrote: Jean, On Wednesday 20 June 2012 02:22 PM, Rajendra Nayak wrote: Hi Jean, On Wednesday 20 June 2012 02:16 PM, Rajendra Nayak wrote: On Wednesday 20 June 2012 02:01 PM, Jean Pihet wrote: Hi Rajendra, On Wed, Jun 20, 2012 at 10:19 AM, Rajendra Nayakrna...@ti.com wrote: Hi Jean, On Friday 01 June 2012 08:41 PM, Jean Pihet wrote: For a power domain to idle all the clock domains in it must idle. This patch implements an optimization of the cpuidle code by denying and later allowing only the first registered clock domain of a power domain, and so optimizes the latency of the low power code. How much do we really save doing this? I understand what you are doing by looking at the patch but the changelog seems very confusing. The gain is on the registers accesses and the internal PRCM state machine. If needed the changelog can be updated. Can you explain a bit more on which register accesses are you talking about? and some more on the PRCM state machine. never mind, I looked at the patch again and then the cpuidle code and figured what you are doing. Makes sense to me now :-) Ok! How do you like this updated changelog, I just added one more line. For a power domain to idle all the clock domains in it must idle. Denying just *one* clkdm in a pwrdm from idling should have the same effect as denying *all*. This patch implements an optimization of the cpuidle code by denying and later allowing only the first registered clock domain of a power domain, and so optimizes the latency of the low power code. That looks great! Kevin, Can you take this change still in your for_3.6/pm/performance branch? Thanks regards, Jean regards, Rajendra -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] ARM: OMAP3: PM: cpuidle: optimize the clkdm idle latency in C1 state
For a power domain to idle all the clock domains in it must idle.
This patch implements an optimization of the cpuidle code by
denying and later allowing only the first registered clock domain
of a power domain, and so optimizes the latency of the low power code.
The functions _cpuidle_allow_idle and _cpuidle_deny_idle are
not used anymore and so are removed.
Signed-off-by: Jean Pihet j-pi...@ti.com
---
arch/arm/mach-omap2/cpuidle34xx.c | 22 --
1 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c
b/arch/arm/mach-omap2/cpuidle34xx.c
index 2e2f1c6..e6ae3fe 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -77,20 +77,6 @@ static struct omap3_idle_statedata omap3_idle_data[] = {
static struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
-static int _cpuidle_allow_idle(struct powerdomain *pwrdm,
- struct clockdomain *clkdm)
-{
- clkdm_allow_idle(clkdm);
- return 0;
-}
-
-static int _cpuidle_deny_idle(struct powerdomain *pwrdm,
- struct clockdomain *clkdm)
-{
- clkdm_deny_idle(clkdm);
- return 0;
-}
-
static int __omap3_enter_idle(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
@@ -108,8 +94,8 @@ static int __omap3_enter_idle(struct cpuidle_device *dev,
/* Deny idle for C1 */
if (index == 0) {
- pwrdm_for_each_clkdm(mpu_pd, _cpuidle_deny_idle);
- pwrdm_for_each_clkdm(core_pd, _cpuidle_deny_idle);
+ clkdm_deny_idle(mpu_pd-pwrdm_clkdms[0]);
+ clkdm_deny_idle(core_pd-pwrdm_clkdms[0]);
}
/*
@@ -131,8 +117,8 @@ static int __omap3_enter_idle(struct cpuidle_device *dev,
/* Re-allow idle for C1 */
if (index == 0) {
- pwrdm_for_each_clkdm(mpu_pd, _cpuidle_allow_idle);
- pwrdm_for_each_clkdm(core_pd, _cpuidle_allow_idle);
+ clkdm_allow_idle(mpu_pd-pwrdm_clkdms[0]);
+ clkdm_allow_idle(core_pd-pwrdm_clkdms[0]);
}
return_sleep_time:
--
1.7.7.6
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
