Re: PM related performance degradation on OMAP3

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Grazvydas, Kevin,

I did some gather some performance measurements and statistics using
custom tracepoints in __omap3_enter_idle.
All the details are at
http://www.omappedia.org/wiki/Power_Management_Device_Latencies_Measurement#C1_performance_problem:_analysis
.

The setup is:
- Beagleboard (OMAP3530) at 500MHz,
- l-o master kernel + functional power states + per-device PM QoS. It
has been checked that the changes from l-o master do not have an
impact on the performance.
- The data transfer is performed using dd from a file in JFFS2 to
/dev/null: 'dd if=/tmp/mnt/a of=/dev/null bs=1M count=32'.

On Tue, Apr 17, 2012 at 4:30 PM, Kevin Hilman <khilman@xxxxxx> wrote:
> Grazvydas Ignotas <notasas@xxxxxxxxx> writes:
>
>> On Thu, Apr 12, 2012 at 3:19 AM, Kevin Hilman <khilman@xxxxxx> 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 .
>
...
>
>> 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)

Here are the results (BW in MB/s) on Beagleboard:
- 4.7: without using DMA,

- Using DMA
  2.1: [0]
  2.1: [1] only C1
  2.6: [1]+[2] no pre_ post_
  2.3: [1]+[5] no pwrdm_for_each_clkdm
  2.8: [1]+[5]+[2]
  3.1: [1]+[5]+[6] no omap_sram_idle
  3.1: No IDLE, no omap_sram_idle, all pwrdms to ON

So indeed this shows there is some serious performance issue with the
C1 C-state.

> 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?

>From the list of contributors, the main ones are:
    (140us) pwrdm_pre_transition and pwrdm_post_transition,
    (105us) omap2_gpio_prepare_for_idle and
omap2_gpio_resume_after_idle. This could be avoided if PER stays ON in
the latency-critical C-states,
    (78us) pwrdm_for_each_clkdm(mpu, core, deny_idle/allow_idle),
    (33us estimated) omap_set_pwrdm_state(mpu, core, neon),
    (11 us) clkdm_allow_idle(mpu). Is this needed?

Here are a few questions and suggestions:
- In case of latency critical C-states could the high-latency code be
bypassed in favor of a much simpler version? Pushing the concept a bit
farther one could have a C1 state that just relaxes the cpu (no WFI),
a C2 state which bypasses a lot of code in __omap3_enter_idle, and the
rest of the C-states as we have today,
- Is it needed to iterate through all the power and clock domains in
order to keep them active?
- Trying to idle some non related power domains (e.g. PER) causes a
performance hit. How to link all the power domains states to the
cpuidle C-state? The per-device PM QoS framework could be used to
constraint some power domains, but this is highly dependent on the use
case.

> 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.
That would be worth doing the analysis. A lot of read accesses to the
current, next and previous power states are performed in the idle
code.

> 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.
I can try some proof of concept code, just to prove its usefulness.

>> 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
>

Any thoughts?

Regards,
Jean

>>
>>
>> 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 majordomo@xxxxxxxxxxxxxxx
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux