* [PATCH 1/2] ARM: OMAP3: PM: cpuidle: optimize the PER latency in C1 state
@ 2012-05-09 10:55 jean.pihet
2012-05-09 10:55 ` [PATCH 2/2] ARM: OMAP3: PM: cpuidle: optimize the clkdm idle " jean.pihet
0 siblings, 1 reply; 5+ messages in thread
From: jean.pihet @ 2012-05-09 10:55 UTC (permalink / raw)
To: Kevin Hilman, Grazvydas Ignotas, linux-omap; +Cc: Paul Walmsley, Jean Pihet
From: Jean Pihet <j-pihet@ti.com>
One of the main contributors of the low power code latency is
the PER power domain. To optimize the high-performance and
low-latency C1 state, prevent any PER state which is lower
than the CORE state in C1.
Reported and suggested by Kevin Hilman.
Reported-by: Kevin Hilman <khilman@ti.com>
Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
arch/arm/mach-omap2/cpuidle34xx.c | 38 +++++++++++++++++++-----------------
1 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 5358664..353dd8d 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -247,19 +247,18 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
int index)
{
int new_state_idx;
- u32 core_next_state, per_next_state = 0, per_saved_state = 0, cam_state;
+ u32 core_next_state, per_next_state = 0, per_saved_state = 0;
struct omap3_idle_statedata *cx;
int ret;
/*
- * Prevent idle completely if CAM is active.
+ * Use only C1 if CAM is active.
* CAM does not have wakeup capability in OMAP3.
*/
- cam_state = pwrdm_read_pwrst(cam_pd);
- if (cam_state == PWRDM_POWER_ON) {
+ if (pwrdm_read_pwrst(cam_pd) == PWRDM_POWER_ON)
new_state_idx = drv->safe_state_index;
- goto select_state;
- }
+ else
+ new_state_idx = next_valid_state(dev, drv, index);
/*
* FIXME: we currently manage device-specific idle states
@@ -269,24 +268,28 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
* its own code.
*/
- /*
- * Prevent PER off if CORE is not in retention or off as this
- * would disable PER wakeups completely.
- */
- cx = cpuidle_get_statedata(&dev->states_usage[index]);
+ /* Program PER state */
+ cx = cpuidle_get_statedata(&dev->states_usage[new_state_idx]);
core_next_state = cx->core_state;
per_next_state = per_saved_state = pwrdm_read_next_pwrst(per_pd);
- if ((per_next_state == PWRDM_POWER_OFF) &&
- (core_next_state > PWRDM_POWER_RET))
- per_next_state = PWRDM_POWER_RET;
+ if (new_state_idx == 0) {
+ /* In C1 do not allow PER state lower than CORE state */
+ if (per_next_state < core_next_state)
+ per_next_state = core_next_state;
+ } else {
+ /*
+ * Prevent PER OFF if CORE is not in RETention or OFF as this
+ * would disable PER wakeups completely.
+ */
+ if ((per_next_state == PWRDM_POWER_OFF) &&
+ (core_next_state > PWRDM_POWER_RET))
+ per_next_state = PWRDM_POWER_RET;
+ }
/* Are we changing PER target state? */
if (per_next_state != per_saved_state)
pwrdm_set_next_pwrst(per_pd, per_next_state);
- new_state_idx = next_valid_state(dev, drv, index);
-
-select_state:
ret = omap3_enter_idle(dev, drv, new_state_idx);
/* Restore original PER state if it was modified */
@@ -372,7 +375,6 @@ int __init omap3_idle_init(void)
/* C1 . MPU WFI + Core active */
_fill_cstate(drv, 0, "MPU ON + CORE ON");
- (&drv->states[0])->enter = omap3_enter_idle;
drv->safe_state_index = 0;
cx = _fill_cstate_usage(dev, 0);
cx->valid = 1; /* C1 is always valid */
--
1.7.7.6
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] ARM: OMAP3: PM: cpuidle: optimize the clkdm idle latency in C1 state
2012-05-09 10:55 [PATCH 1/2] ARM: OMAP3: PM: cpuidle: optimize the PER latency in C1 state jean.pihet
@ 2012-05-09 10:55 ` jean.pihet
2012-05-31 16:29 ` Kevin Hilman
0 siblings, 1 reply; 5+ messages in thread
From: jean.pihet @ 2012-05-09 10:55 UTC (permalink / raw)
To: Kevin Hilman, Grazvydas Ignotas, linux-omap; +Cc: Paul Walmsley, Jean Pihet
From: Jean Pihet <j-pihet@ti.com>
It is not needed to iterate through all the clock domains of a
power domain in order to allow or deny it to idle.
This patch allows or denies 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-pihet@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 353dd8d..d44b68a 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -73,20 +73,6 @@ struct omap3_idle_statedata omap3_idle_data[OMAP3_NUM_STATES];
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)
@@ -105,8 +91,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]);
}
/*
@@ -128,8 +114,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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] ARM: OMAP3: PM: cpuidle: optimize the clkdm idle latency in C1 state
2012-05-09 10:55 ` [PATCH 2/2] ARM: OMAP3: PM: cpuidle: optimize the clkdm idle " jean.pihet
@ 2012-05-31 16:29 ` Kevin Hilman
2012-06-01 15:14 ` Jean Pihet
0 siblings, 1 reply; 5+ messages in thread
From: Kevin Hilman @ 2012-05-31 16:29 UTC (permalink / raw)
To: jean.pihet; +Cc: Grazvydas Ignotas, linux-omap, Paul Walmsley, Jean Pihet
jean.pihet@newoldbits.com writes:
> From: Jean Pihet <j-pihet@ti.com>
>
> It is not needed to iterate through all the clock domains of a
> power domain in order to allow or deny it to idle.
Why? (I know the answer, but would like it answered here.)
> This patch allows or denies 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-pihet@ti.com>
Other than the changelog update, it looks good but also needs a rebase
like the previous patch.
After that, I'll add them to my for_3.6/pm/performance branch and queue
for v3.6.
Thanks!
Kevin
> ---
> 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 353dd8d..d44b68a 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -73,20 +73,6 @@ struct omap3_idle_statedata omap3_idle_data[OMAP3_NUM_STATES];
>
> 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)
> @@ -105,8 +91,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]);
> }
>
> /*
> @@ -128,8 +114,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:
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] ARM: OMAP3: PM: cpuidle: optimize the clkdm idle latency in C1 state
2012-05-31 16:29 ` Kevin Hilman
@ 2012-06-01 15:14 ` Jean Pihet
2012-06-01 16:13 ` Kevin Hilman
0 siblings, 1 reply; 5+ messages in thread
From: Jean Pihet @ 2012-06-01 15:14 UTC (permalink / raw)
To: Kevin Hilman; +Cc: Grazvydas Ignotas, linux-omap, Paul Walmsley, Jean Pihet
Hi Kevin,
On Thu, May 31, 2012 at 6:29 PM, Kevin Hilman <khilman@ti.com> wrote:
> jean.pihet@newoldbits.com writes:
>
>> From: Jean Pihet <j-pihet@ti.com>
>>
>> It is not needed to iterate through all the clock domains of a
>> power domain in order to allow or deny it to idle.
>
> Why? (I know the answer, but would like it answered here.)
>
>> This patch allows or denies 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-pihet@ti.com>
>
> Other than the changelog update, it looks good but also needs a rebase
> like the previous patch.
>
> After that, I'll add them to my for_3.6/pm/performance branch and queue
> for v3.6.
The new series has been sent as '[PATCH 0/3] ARM: OMAP3: PM: optimize
cpuidle C1 state latency' with the suggested changes (changelog
updated, rebased on for_3.6/pm/performance).
Note: with the code from the branch the CORE does not idle. The
optimization changes have been tested OK though.
>
> Thanks!
>
> Kevin
>
Regards,
Jean
>> ---
>> 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 353dd8d..d44b68a 100644
>> --- a/arch/arm/mach-omap2/cpuidle34xx.c
>> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
>> @@ -73,20 +73,6 @@ struct omap3_idle_statedata omap3_idle_data[OMAP3_NUM_STATES];
>>
>> 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)
>> @@ -105,8 +91,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]);
>> }
>>
>> /*
>> @@ -128,8 +114,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 majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] ARM: OMAP3: PM: cpuidle: optimize the clkdm idle latency in C1 state
2012-06-01 15:14 ` Jean Pihet
@ 2012-06-01 16:13 ` Kevin Hilman
0 siblings, 0 replies; 5+ messages in thread
From: Kevin Hilman @ 2012-06-01 16:13 UTC (permalink / raw)
To: Jean Pihet; +Cc: Grazvydas Ignotas, linux-omap, Paul Walmsley, Jean Pihet
Jean Pihet <jean.pihet@newoldbits.com> writes:
> Hi Kevin,
>
> On Thu, May 31, 2012 at 6:29 PM, Kevin Hilman <khilman@ti.com> wrote:
>> jean.pihet@newoldbits.com writes:
>>
>>> From: Jean Pihet <j-pihet@ti.com>
>>>
>>> It is not needed to iterate through all the clock domains of a
>>> power domain in order to allow or deny it to idle.
>>
>> Why? (I know the answer, but would like it answered here.)
>>
>>> This patch allows or denies 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-pihet@ti.com>
>>
>> Other than the changelog update, it looks good but also needs a rebase
>> like the previous patch.
>>
>> After that, I'll add them to my for_3.6/pm/performance branch and queue
>> for v3.6.
> The new series has been sent as '[PATCH 0/3] ARM: OMAP3: PM: optimize
> cpuidle C1 state latency' with the suggested changes (changelog
> updated, rebased on for_3.6/pm/performance).
>
> Note: with the code from the branch the CORE does not idle. The
> optimization changes have been tested OK though.
That's OK, I know the causes of the CORE idle retention problems and
have fixes for those queued up.
Thanks,
Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-06-01 16:13 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-09 10:55 [PATCH 1/2] ARM: OMAP3: PM: cpuidle: optimize the PER latency in C1 state jean.pihet
2012-05-09 10:55 ` [PATCH 2/2] ARM: OMAP3: PM: cpuidle: optimize the clkdm idle " jean.pihet
2012-05-31 16:29 ` Kevin Hilman
2012-06-01 15:14 ` Jean Pihet
2012-06-01 16:13 ` Kevin Hilman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.