linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: robherring2@gmail.com (Rob Herring)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH v2 1/2] cpuidle: Add common init interface and idle functionality
Date: Wed, 04 Jan 2012 17:51:28 -0600	[thread overview]
Message-ID: <4F04E600.1040701@gmail.com> (raw)
In-Reply-To: <CAJOA=zNFmAHDPJRfeihCo08B86re1X1LH=uTimam0uemwK+TNg@mail.gmail.com>

On 01/04/2012 05:35 PM, Turquette, Mike wrote:
> On Wed, Jan 4, 2012 at 3:15 PM, Rob Lee <rob.lee@linaro.org> wrote:
>> On 22 December 2011 16:57, Rob Herring <robherring2@gmail.com> wrote:
>>> On 12/14/2011 01:02 AM, Robert Lee wrote:
>>>> +static struct cpuidle_driver exynos4_idle_driver = {
>>>> +     .name           = "exynos4_idle",
>>>> +     .owner          = THIS_MODULE,
>>>> +     .states[0] = {
>>>> +             .enter                  = cpuidle_def_idle,
>>>>               .exit_latency           = 1,
>>>>               .target_residency       = 100000,
>>>>               .flags                  = CPUIDLE_FLAG_TIME_VALID,
>>>>               .name                   = "IDLE",
>>>>               .desc                   = "ARM clock gating(WFI)",
>>>>       },
>>>
>>> As this is just plain wfi and shouldn't really be different per
>>> platform, it would be nice to get rid of all of this state info. Perhaps
>>> a macro with all the data since each driver needs to init each state.
>>> The target residency value looks kind of suspect. You should only go to
>>> wfi if you expect to be idle for 100ms?
>>>
>>
>> Ok, I'll look at add a ARM specific macro for a generic WFI state in v3.
>>
>> For the target_residency value, I don't understand why the values
>> being used are there other than some of the original ARM platform
>> implementations were based on the Intel implementations per their
>> comments, and the Intel value was used.  From the comments in
>> drivers/cpuidle/governors/menu.c: "state entry and exit have an energy
>> cost, and a certain amount of time in the  C state is required to
>> actually break even on this cost. CPUIDLE provides us this duration in
>> the target_residency field".  The governor code uses it accordingly.
>> I'm not aware that a pure WFI only state uses additional energy to
>> enter and exit compared to spinning in a loop, so I think the
>> "target_residency" of a pure WFI state value should 0 or 1.  If a
>> platform skips a pure WFI idle state and also implements additional
>> platform specific power savings in their "WFI" idle state, then they
>> may need to adjust exit_latency and target_residency accordingly.
> 
> Extra data point: on OMAP4 we chose .target_residency = 5 for simple WFI state:
> http://git.omapzoom.org/?p=kernel/omap.git;a=blob;f=arch/arm/mach-omap2/cpuidle44xx.c;h=383944076085f808b8764baf11df0589229010e5;hb=p-android-omap-3.0#l113

Is there any data behind that value is or is just a different made up
value?

The default/highest power state is always state 0, so that will be
picked unless states 1-N meet the requirements. There is no spin loop
unless a driver implements one. So it doesn't really matter what the
target_residency or exit_latency values are for state 0. I would use 1
for both.

Rob

  reply	other threads:[~2012-01-04 23:51 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-14  7:02 [RFC PATCH v2 0/2] Add common cpuidle code for consolidation Robert Lee
2011-12-14  7:02 ` [RFC PATCH v2 1/2] cpuidle: Add common init interface and idle functionality Robert Lee
2011-12-22 18:09   ` Mark Brown
2012-01-04 23:21     ` Rob Lee
2011-12-22 22:57   ` Rob Herring
2012-01-04 23:15     ` Rob Lee
2012-01-04 23:35       ` Turquette, Mike
2012-01-04 23:51         ` Rob Herring [this message]
2012-01-05 20:11           ` Turquette, Mike
2012-01-05  9:32   ` Russell King - ARM Linux
2012-01-05 16:42     ` Rob Lee
2011-12-14  7:02 ` [RFC PATCH v2 2/2] ARM: imx: Add mx5 cpuidle implmentation Robert Lee
2011-12-22 17:50   ` Mark Brown
2012-01-04 23:35     ` Rob Lee
2012-01-05  3:21       ` Shawn Guo
2012-01-05  5:55       ` Mark Brown
2012-01-06 21:10         ` Rob Lee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F04E600.1040701@gmail.com \
    --to=robherring2@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).