RE: [PATCH] OMAP3: CPUIDLE & PM: check_bm fix.

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

 



Hi,

> Impact of this change depends a lot on what is happening on the
> system. HW triggered dma transfer comes to my mind first. In that case
> mpu could be kept in OFF state while core is active. Changing bm
> check this way would cause mpu to be kept in INACTIVE state. Any
> analysis of the impact?

Using C1 as a safe state will keep MPU up when CORE is INACTIVE.  You can use C2 or C3 just the same.  I found C1 to give best performance-per-power tradeoff in light tests.

Which state is best depends on activity of the system.  Unless you are sleeping for a good amount of time the pay back from RET/OFF is not as high.  Probably if you have a DMA or other going on the MPU will need to be active to service a completion in the near future.

There is a crossover point where calling OFF will burn more power than it will save because of context save and restore.

Anyway basic thought was BM activity failure probably means an interrupt sooner or later so take the biggest power drop and retain performance.  More characterization with end use cases would provide the best answer.

> >
> > Slowness on serial is there anytime you hit INACTIVE if you don't
> > have the UART-NO-IDLE-ON-TX workaround in tree.  This is not in
> > Tony's tree yet and probably should be if the UART is bothering you.
> > Choosing a non-WFI state penalty like your patch did is too
> > expensive from a power standpoint for the effect.
>
> Well I think there is very similiar workaround in workaround patch set
> (see omap_serial_can_sleep). Now non-WFI state is selected if there
> was activity in last 5 seconds. So get just hands off from the
> keyboard for 5 seconds and C state selection continues normally after
> that. I think 5 seconds without powersave from last character entered
> to serial-console is not too expensive. Benefit of working
> serial-console is bigger I think.

Maybe I should have commented in order (previous mail).

The omap_serial_can_sleep is similar to what I described before as being done to complete the work around.  We added this type of work around a couple years back on 2420 and adapted it for 3430 last year.

Probably the difference is if you use the UART SMART-FORCE method you can allow all C-States which don’t fall in the BM check.  If you abort the way you proposed you don't hit any idle states at all.

The major goal for _active_ use cases is to have the chip in INACTIVE state.  RET/OFF give most benefit for prolonged chip idle periods.

Code development (and test) will be a bit more true to end device performance if you allow a UART and your system hits INACTIVE.  You now hit and know your performance issues for active driver states before removing UART.  And you can debug them with the UART.

> > --- arch/arm/mach-omap2/pm_idle_34xx.c@@/LINUX-GIT-
> 2.6K_INT_FLOAT_4.0   2008-07-02 11:39:50.000000000 -0500
> > +++ arch/arm/mach-omap2/pm_idle_34xx.c  2008-07-04
> 11:54:15.000000000 -0500
> > @@ -706,32 +706,15 @@ return_sleep_time:
> >  static int omap3_enter_idle_bm(struct cpuidle_device *dev,
> >                                 struct cpuidle_state *state)
> >  {
> > -       struct cpuidle_state *new_state = NULL;
> > -       int i, j;
> > -
> >         if ((state->flags & CPUIDLE_FLAG_CHECK_BM) &&
> omap3_idle_bm_check()) {
> > -
> > -               /* Find current state in list */
> > -               for (i = 0; i < OMAP3_MAX_STATES; i++)
> > -                       if (state == &dev->states[i])
> > -                               break;
> > -               BUG_ON(i == OMAP3_MAX_STATES);
> > -
> > -               /* Back up to non 'CHECK_BM' state */
> > -               for (j = i - 1;  j > 0; j--) {
> > -                       struct cpuidle_state *s = &dev->states[j];
> > -
> > -                       if (!(s->flags & CPUIDLE_FLAG_CHECK_BM)) {
> > -                               new_state = s;
> > -                               break;
> > -                       }
> > +               if (dev->safe_state)
> > +                       return dev->safe_state->enter(dev, dev-
> >safe_state);
> > +               else {
> > +                       omap_sram_idle();
> > +                       return 0;
> >                 }
>
> How about adding that old loop into this else statement? Calling
> omap_sram_idle strictly here has an effect that last selected state is
> used (next states are not written). Also switching between safe_state
> mechanism and loop would be very easy.

Yes that probably is a safest thing.

Initially the thought was the non-safe state would never be called.  As the Intel code thought it necessary it seemed fine to do something similar.

I do agree that if it ever gets called there might be side effects.

Regards,
Richard W.

��.n��������+%������w��{.n�����{�������ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f


[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