Grazvydas Ignotas <nota...@gmail.com> writes:
> On Thu, Apr 12, 2012 at 3:19 AM, Kevin Hilman <khil...@ti.com> wrote:
>> It would be helpful now to narrow down what are the big contributors to
>> the overhead in omap_sram_idle(). Most of the code there is skipped for
>> C1 because the next states for MPU and CORE are both ON.
>
> Ok I did some tests, all in mostly idle system with just init, busybox
> shell and dd doing a NAND read to /dev/null .
Hmm, I seem to get a hang using dd to read from NAND /dev/mtdX on my
Overo. I saw your patch 'mtd: omap2: fix resource leak in prefetch-busy
path' but that didn't seem to help my crash.
> MB/s is throughput that
> dd reports, mA and approx. current draw during the transfer, read from
> fuel gauge that's onboard.
>
> MB/s| mA|comment
> 3.7|218|mainline f549e088b80
> 3.8|224|nand qos PM_QOS_CPU_DMA_LATENCY 0 [1]
> 4.4|220|[1] + pwrdm_p*_transition commented [2]
> 3.8|225|[1] + omap34xx_do_sram_idle->cpu_do_idle [3]
> 4.2|210|[1] + pwrdm_set_next_pwrst(per_pd, PWRDM_POWER_ON) [4]
> 4.0|224|[1] + 'Deny idle' [5]
> 5.1|210|[2] + [4] + [5]
> 5.2|202|[5] + omap_sram_idle->cpu_do_idle [6]
> 5.5|243|!CONFIG_PM
> 6.1|282|busywait DMA end (for reference)
Thanks for the detailed experiments. This definitely confirms we have
some serious unwanted overhead for C1, and our C-state latency values
are clearly way off base, since they only account HW latency and not any
of the SW latency introduced in omap_sram_idle().
>> There are 2 primary differences that I see as possible causes. I list
>> them here with a couple more experiments for you to try to help us
>> narrow this down.
>>
>> 1) powerdomain accounting: pwrdm_pre_transition(), pwrdm_post_transition()
>>
>> Could you try using omap_sram_idle() and just commenting out those
>> calls? Does that help performance? Those iterate over all the
>> powerdomains, so defintely add some overhead, but I don't think it
>> would be as significant as what you're seeing.
>
> Seems to be taking good part of it.
>
>> Much more likely is...
>>
>> 2) jump to SRAM, SDRC self-refresh, SDRC errata workarounds
>
> Could not notice any difference.
>
> To me it looks like this results from many small things adding up..
> Idle is called so often that pwrdm_p*_transition() and those
> pwrdm_for_each_clkdm() walks start slowing everything down, perhaps
> because they access lots of registers on slow buses?
Yes PRCM register accesses are unfortunately rather slow, and we've
known that for some time, but haven't done any detailed analysis of the
overhead.
Using the function_graph tracer, I was able to see that the pre/post
transition are taking an enormous amount of time:
- pwrdm pre-transition: 1400+ us at 600MHz (4000+ us at 125MHz)
- pwrdm post-transtion: 1600+ us at 600MHz (6000+ us at 125MHz)
Notice the big difference between 600MHz OPP and 125MHz OPP. Are you
using CPUfreq at all in your tests? If using cpufreq + ondemand
governor, you're probably running at low OPP due to lack of CPU activity
which will also affect the latencies in the idle path.
> Maybe some register cache would help us there, or are those registers
> expected to be changed by hardware often?
Yes, we've known that some sort of register cache here would be useful
for some time, but haven't got to implementing it.
> Also trying to idle PER while transfer is ongoing (as reported in
> previous mail) doesn't sound like a good idea and is one of the
> reasons for slowdown. Seems to also causing more current drain,
> ironically.
Agreed. Again, using the function_graph tracer, I get some pretty big
latencies from the GPIO pre/post idling process:
- gpio_prepare_for_idle(): 2400+ us at 600MHz (8200+ us at 125MHz)
- gpio_resume_from_idle(): 2200+ us at 600MHz (7600+ us at 125MHz)
Removing PER transtions as you did will get rid of those.
I'm looking into this in more detail know, and will likely have a few
patches for you to experiment with.
Thanks again for digging into this with us,
Kevin
>
>
> changes (again, sorry for corrupted diffs, but they should be easy to
> reproduce):
> [2]:
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -307,7 +307,7 @@ void omap_sram_idle(void)
> omap3_enable_io_chain();
> }
>
> - pwrdm_pre_transition();
> +// pwrdm_pre_transition();
>
> /* PER */
> if (per_next_state < PWRDM_POWER_ON) {
> @@ -372,7 +373,7 @@ void omap_sram_idle(void)
> }
> omap3_intc_resume_idle();
>
> - pwrdm_post_transition();
> +// pwrdm_post_transition();
>
> /* PER */
> if (per_next_state < PWRDM_POWER_ON) {
> [3]:
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -347,7 +347,7 @@ void omap_sram_idle(void)
> if (save_state == 1 || save_state == 3)
> cpu_suspend(save_state, omap34xx_do_sram_idle);
> else
> - omap34xx_do_sram_idle(save_state);
> + cpu_do_idle();
>
> /* Restore normal SDRC POWER settings */
> if (cpu_is_omap3430() && omap_rev() >= OMAP3430_REV_ES3_0 &&
> [4]:
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -107,6 +107,7 @@ static int __omap3_enter_idle(struct cpuidle_device *dev,
> if (index == 0) {
> pwrdm_for_each_clkdm(mpu_pd, _cpuidle_deny_idle);
> pwrdm_for_each_clkdm(core_pd, _cpuidle_deny_idle);
> + pwrdm_set_next_pwrst(per_pd, PWRDM_POWER_ON);
> }
>
> /*
> [5]:
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -105,8 +105,7 @@ 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]);
> }
>
> /*
> @@ -128,8 +128,7 @@ 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]);
> }
>
> return_sleep_time:
> [6]:
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -117,7 +116,8 @@ static int __omap3_enter_idle(struct cpuidle_device *dev,
> cpu_pm_enter();
>
> /* Execute ARM wfi */
> - omap_sram_idle();
> + //omap_sram_idle();
> + cpu_do_idle();
>
> /*
> * Call idle CPU PM enter notifier chain to restore
--
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
