Kalle Jokiniemi <kalle.jokiniemi@xxxxxxxxx> writes:
> On Fri, 2009-10-30 at 13:26 +0200, Kalle Jokiniemi wrote:
>> From: Kalle Jokiniemi <ext-kalle.jokiniemi@xxxxxxxxx>
>>
>> The biggest source of latency in idle loop (omap_sram_idle
>> function) comes from updating the state counters for each
>> power domain. The two purposes of these counters are to
>> provide debug data in debugfs, and to keep track of context
>> losses occurring during idle transitions (off mode counters).
>>
>> I created new debugfs interface "enable_count" for
>> enabling the "count" interface, which exposes the debug
>> part of these counters. The counters are not updated
>> anymore for CORE ON idle transitions, when the count
>> interface is disabled. For deeper CORE states, counters
>> are still updated to preserve context loss tracking.
>>
>> This change decreases C1/C2 state latency over 100us at
>> OPP2.
>>
>> Signed-off-by: Kalle Jokiniemi <ext-kalle.jokiniemi@xxxxxxxxx>
[...]
>> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
>> index 01260ec..1e3041e 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -378,15 +378,16 @@ void omap_sram_idle(void)
>> return;
>> }
>>
>> - pwrdm_pre_transition();
>> + per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
>> + core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
>> +
>> + pwrdm_pre_transition(core_next_state != PWRDM_POWER_ON);
>
> I still left a little condition handling here, but it's nicer than the
> previous one, don't you think. Just passing this force_update parameter
> seemed like the most efficient way of doing it.
Yes, I'm OK with the 'force update' idea for now. You should just
update the changelog to talk about the force-update additon to the
transition hooks as well.
Other than that, looks fine with me for now.
Kevin
> I did not want to duplicate core state recording into powerdomain code,
> nor inefficient code looking for core pwrdm and then checking it's next
> state in powerdomain code. I think it's idle codes responsibility to
> know whether off-transitions will happen in general (do we need to force
> state counter update). Or at least that information is most efficiently
> available there now.
>
> We could consider cpuidle passing some "safe state" parameter, in case
> of C1/C2, which could be used instead of this core_next_state
> comparison. If you want to make it even nicer looking.
>
> - Kalle
>
>>
>> /* NEON control */
>> if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
>> pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
>>
>> /* PER */
>> - per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
>> - core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
>> if (per_next_state < PWRDM_POWER_ON) {
>> omap_uart_prepare_idle(2);
>> omap2_gpio_prepare_for_idle(per_next_state);
>> @@ -505,8 +506,7 @@ void omap_sram_idle(void)
>> omap3_disable_io_chain();
>> }
>>
>> -
>> - pwrdm_post_transition();
>> + pwrdm_post_transition(core_next_state != PWRDM_POWER_ON);
>>
>> omap2_clkdm_allow_idle(mpu_pwrdm->pwrdm_clkdms[0]);
>> }
>> diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
>> index f00289a..71af2b8 100644
>> --- a/arch/arm/mach-omap2/powerdomain.c
>> +++ b/arch/arm/mach-omap2/powerdomain.c
>> @@ -1216,15 +1216,17 @@ int pwrdm_clk_state_switch(struct clk *clk)
>> return -EINVAL;
>> }
>>
>> -int pwrdm_pre_transition(void)
>> +int pwrdm_pre_transition(int force_update)
>> {
>> - pwrdm_for_each(_pwrdm_pre_transition_cb, NULL);
>> + if (pm_dbg_count_is_active() || force_update)
>> + pwrdm_for_each(_pwrdm_pre_transition_cb, NULL);
>> return 0;
>> }
>>
>> -int pwrdm_post_transition(void)
>> +int pwrdm_post_transition(int force_update)
>> {
>> - pwrdm_for_each(_pwrdm_post_transition_cb, NULL);
>> + if (pm_dbg_count_is_active() || force_update)
>> + pwrdm_for_each(_pwrdm_post_transition_cb, NULL);
>> return 0;
>> }
>>
>> diff --git a/arch/arm/plat-omap/include/mach/powerdomain.h b/arch/arm/plat-omap/include/mach/powerdomain.h
>> index fa64614..cb93272 100644
>> --- a/arch/arm/plat-omap/include/mach/powerdomain.h
>> +++ b/arch/arm/plat-omap/include/mach/powerdomain.h
>> @@ -176,7 +176,7 @@ int pwrdm_wait_transition(struct powerdomain *pwrdm);
>>
>> int pwrdm_state_switch(struct powerdomain *pwrdm);
>> int pwrdm_clkdm_state_switch(struct clockdomain *clkdm);
>> -int pwrdm_pre_transition(void);
>> -int pwrdm_post_transition(void);
>> +int pwrdm_pre_transition(int force_update);
>> +int pwrdm_post_transition(int force_update);
>>
>> #endif
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
