Re: [PATCH 2/3] cpuidle,menu: use interactivity_req to disable polling
From: Rafael J. Wysocki
Date: Wed Jan 13 2016 - 17:07:21 EST
On Wednesday, January 13, 2016 10:58:13 PM Rafael J. Wysocki wrote:
> Hi,
>
> On Wed, Jan 13, 2016 at 6:27 PM, Sudeep Holla <sudeep.holla@xxxxxxx> wrote:
> > Hi Rik,
[cut]
> >
> > diff --git i/drivers/cpuidle/governors/menu.c w/drivers/cpuidle/governors/menu.c
> > index 7b0971d97cc3..7c7f4059705a 100644
> > --- i/drivers/cpuidle/governors/menu.c
> > +++ w/drivers/cpuidle/governors/menu.c
> > @@ -330,7 +330,8 @@ static int menu_select(struct cpuidle_driver *drv,
> > struct cpuidle_device *dev)
> > * We want to default to C1 (hlt), not to busy polling
> > * unless the timer is happening really really soon.
> > */
> > - if (interactivity_req > 20 &&
> > + if (((interactivity_req && interactivity_req > 20) ||
>
> Well, if interactivity_req > 20, then it also is different from 0, so
> the first check should not be necessary here.
>
> > + data->next_timer_us > 20) &&
>
> I guess that this simply restores the previous behavior on the
> platforms in question.
>
> Now, the reason why it may matter is because
> CPUIDLE_DRIVER_STATE_START is 0 and so data->last_state_idx may end up
> as -1 on them. So I think this piece of code only ever makes sense if
> CPUIDLE_DRIVER_STATE_START is 1.
>
> > !drv->states[CPUIDLE_DRIVER_STATE_START].disabled &&
> > dev->states_usage[CPUIDLE_DRIVER_STATE_START].disable == 0)
> > data->last_state_idx = CPUIDLE_DRIVER_STATE_START;
> > --
So I'm wondering if the appended patch helps by any chance?
Rafael
---
drivers/cpuidle/governors/menu.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
Index: linux-pm/drivers/cpuidle/governors/menu.c
===================================================================
--- linux-pm.orig/drivers/cpuidle/governors/menu.c
+++ linux-pm/drivers/cpuidle/governors/menu.c
@@ -294,8 +294,6 @@ static int menu_select(struct cpuidle_dr
data->needs_update = 0;
}
- data->last_state_idx = CPUIDLE_DRIVER_STATE_START - 1;
-
/* Special case when user has set very strict latency requirement */
if (unlikely(latency_req == 0))
return 0;
@@ -326,14 +324,19 @@ static int menu_select(struct cpuidle_dr
if (latency_req > interactivity_req)
latency_req = interactivity_req;
- /*
- * We want to default to C1 (hlt), not to busy polling
- * unless the timer is happening really really soon.
- */
- if (interactivity_req > 20 &&
- !drv->states[CPUIDLE_DRIVER_STATE_START].disabled &&
- dev->states_usage[CPUIDLE_DRIVER_STATE_START].disable == 0)
+ if (CPUIDLE_DRIVER_STATE_START > 0) {
+ data->last_state_idx = CPUIDLE_DRIVER_STATE_START - 1;
+ /*
+ * We want to default to C1 (hlt), not to busy polling
+ * unless the timer is happening really really soon.
+ */
+ if (interactivity_req > 20 &&
+ !drv->states[CPUIDLE_DRIVER_STATE_START].disabled &&
+ dev->states_usage[CPUIDLE_DRIVER_STATE_START].disable == 0)
+ data->last_state_idx = CPUIDLE_DRIVER_STATE_START;
+ } else {
data->last_state_idx = CPUIDLE_DRIVER_STATE_START;
+ }
/*
* Find the idle state with the lowest power while satisfying