From 5dc2f5a3030552f2501cb34ac6d979455630e486 Mon Sep 17 00:00:00 2001 From: "tuukka.tikkanen@linaro.org" Date: Mon, 24 Feb 2014 08:29:31 +0200 Subject: cpuidle: rename expected_us to next_timer_us in menu governor The field expected_us is used to store the time remaining until next timer expiry. The name is inaccurate, as we really do not expect all wakeups to be caused by timers. In addition, another field with a very similar name (predicted_us) is used to store the predicted time remaining until any wakeup source being active. This patch renames expected_us to next_timer_us in order to better reflect the contained information. Signed-off-by: Tuukka Tikkanen Acked-by: Nicolas Pitre Acked-by: Len Brown Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/governors/menu.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'drivers/cpuidle') diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index cf7f2f0e4ef5..e9a2a27134c1 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -122,7 +122,7 @@ struct menu_device { int last_state_idx; int needs_update; - unsigned int expected_us; + unsigned int next_timer_us; unsigned int predicted_us; unsigned int exit_us; unsigned int bucket; @@ -257,7 +257,7 @@ again: stddev = int_sqrt(stddev); if (((avg > stddev * 6) && (divisor * 4 >= INTERVALS * 3)) || stddev <= 20) { - if (data->expected_us > avg) + if (data->next_timer_us > avg) data->predicted_us = avg; return; } @@ -306,11 +306,11 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev) /* determine the expected residency time, round up */ t = ktime_to_timespec(tick_nohz_get_sleep_length()); - data->expected_us = + data->next_timer_us = t.tv_sec * USEC_PER_SEC + t.tv_nsec / NSEC_PER_USEC; - data->bucket = which_bucket(data->expected_us); + data->bucket = which_bucket(data->next_timer_us); multiplier = performance_multiplier(); @@ -326,7 +326,7 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev) * operands are 32 bits. * Make sure to round up for half microseconds. */ - data->predicted_us = div_round64((uint64_t)data->expected_us * + data->predicted_us = div_round64((uint64_t)data->next_timer_us * data->correction_factor[data->bucket], RESOLUTION * DECAY); @@ -336,7 +336,7 @@ 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 (data->expected_us > 5 && + if (data->next_timer_us > 5 && !drv->states[CPUIDLE_DRIVER_STATE_START].disabled && dev->states_usage[CPUIDLE_DRIVER_STATE_START].disable == 0) data->last_state_idx = CPUIDLE_DRIVER_STATE_START; @@ -401,7 +401,7 @@ static void menu_update(struct cpuidle_driver *drv, struct cpuidle_device *dev) * for the whole expected time. */ if (unlikely(!(target->flags & CPUIDLE_FLAG_TIME_VALID))) - last_idle_us = data->expected_us; + last_idle_us = data->next_timer_us; measured_us = last_idle_us; @@ -418,8 +418,8 @@ static void menu_update(struct cpuidle_driver *drv, struct cpuidle_device *dev) new_factor = data->correction_factor[data->bucket]; new_factor -= new_factor / DECAY; - if (data->expected_us > 0 && measured_us < MAX_INTERESTING) - new_factor += RESOLUTION * measured_us / data->expected_us; + if (data->next_timer_us > 0 && measured_us < MAX_INTERESTING) + new_factor += RESOLUTION * measured_us / data->next_timer_us; else /* * we were idle so long that we count it as a perfect -- cgit v1.2.3 From 22695ab6314083d9d045ce9276e9ae79eb889531 Mon Sep 17 00:00:00 2001 From: "tuukka.tikkanen@linaro.org" Date: Mon, 24 Feb 2014 08:29:32 +0200 Subject: cpuidle: Use actual state latency in menu governor Currently menu governor records the exit latency of the state it has chosen for the idle period. The stored latency value is then later used to calculate the actual length of the idle period. This value may however be incorrect, as the entered state may not be the one chosen by the governor. The entered state information is available, so we can use that to obtain the real exit latency. Signed-off-by: Tuukka Tikkanen Acked-by: Daniel Lezcano Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/governors/menu.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'drivers/cpuidle') diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index e9a2a27134c1..115386a46e9e 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -124,7 +124,6 @@ struct menu_device { unsigned int next_timer_us; unsigned int predicted_us; - unsigned int exit_us; unsigned int bucket; unsigned int correction_factor[BUCKETS]; unsigned int intervals[INTERVALS]; @@ -298,7 +297,6 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev) } data->last_state_idx = 0; - data->exit_us = 0; /* Special case when user has set very strict latency requirement */ if (unlikely(latency_req == 0)) @@ -359,7 +357,6 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev) continue; data->last_state_idx = i; - data->exit_us = s->exit_latency; } return data->last_state_idx; @@ -410,8 +407,8 @@ static void menu_update(struct cpuidle_driver *drv, struct cpuidle_device *dev) * We correct for the exit latency; we are assuming here that the * exit latency happens after the event that we're interested in. */ - if (measured_us > data->exit_us) - measured_us -= data->exit_us; + if (measured_us > target->exit_latency) + measured_us -= target->exit_latency; /* Update our correction ratio */ -- cgit v1.2.3 From 7ac26436677024ab5e57e25c2c83d4c3fc232106 Mon Sep 17 00:00:00 2001 From: "tuukka.tikkanen@linaro.org" Date: Mon, 24 Feb 2014 08:29:33 +0200 Subject: cpuidle: Ensure menu coefficients stay within domain The menu governor uses coefficients as one method of actual idle period length estimation. The coefficients are, as detailed below, multipliers giving expected idle period length from time until next timer expiry. The multipliers are supposed to have domain of (0..1]. The coefficients are fractions where only the numerators are stored and denominators are a shared constant RESOLUTION*DECAY. Since the value of the coefficient should always be greater than 0 and less than or equal to 1, the numerator must have a value greater than 0 and less than or equal to RESOLUTION*DECAY. If the coefficients are updated with measured idle durations exceeding timer length, the multiplier may reach values exceeding unity (i.e. the stored numerator exceeds RESOLUTION*DECAY). This patch ensures that the multipliers are updated with durations capped to timer length. Signed-off-by: Tuukka Tikkanen Acked-by: Nicolas Pitre Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/governors/menu.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/cpuidle') diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index 115386a46e9e..f0995dd2469f 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -410,6 +410,9 @@ static void menu_update(struct cpuidle_driver *drv, struct cpuidle_device *dev) if (measured_us > target->exit_latency) measured_us -= target->exit_latency; + /* Make sure our coefficients do not exceed unity */ + if (measured_us > data->next_timer_us) + measured_us = data->next_timer_us; /* Update our correction ratio */ new_factor = data->correction_factor[data->bucket]; -- cgit v1.2.3 From 61c66d6efa23759f1061d80ced668977fd28337d Mon Sep 17 00:00:00 2001 From: "tuukka.tikkanen@linaro.org" Date: Mon, 24 Feb 2014 08:29:34 +0200 Subject: cpuidle: Do not substract exit latency from assumed sleep length The menu governor statistics update function tries to determine the amount of time between entry to low power state and the occurrence of the wakeup event. However, the time measured by the framework includes exit latency on top of the desired value. This exit latency is substracted from the measured value to obtain the desired value. When measured value is not available, the menu governor assumes the wakeup was caused by the timer and the time is equal to remaining timer length. No exit latency should be substracted from this value. This patch prevents the erroneous substraction and clarifies the associated comment. It also removes one intermediate variable that serves no purpose. Signed-off-by: Tuukka Tikkanen Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/governors/menu.c | 44 ++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 18 deletions(-) (limited to 'drivers/cpuidle') diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index f0995dd2469f..b347c101c1f7 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -387,32 +387,40 @@ static void menu_update(struct cpuidle_driver *drv, struct cpuidle_device *dev) { struct menu_device *data = &__get_cpu_var(menu_devices); int last_idx = data->last_state_idx; - unsigned int last_idle_us = cpuidle_get_last_residency(dev); struct cpuidle_state *target = &drv->states[last_idx]; unsigned int measured_us; unsigned int new_factor; /* - * Ugh, this idle state doesn't support residency measurements, so we - * are basically lost in the dark. As a compromise, assume we slept - * for the whole expected time. + * Try to figure out how much time passed between entry to low + * power state and occurrence of the wakeup event. + * + * If the entered idle state didn't support residency measurements, + * we are basically lost in the dark how much time passed. + * As a compromise, assume we slept for the whole expected time. + * + * Any measured amount of time will include the exit latency. + * Since we are interested in when the wakeup begun, not when it + * was completed, we must substract the exit latency. However, if + * the measured amount of time is less than the exit latency, + * assume the state was never reached and the exit latency is 0. */ - if (unlikely(!(target->flags & CPUIDLE_FLAG_TIME_VALID))) - last_idle_us = data->next_timer_us; - + if (unlikely(!(target->flags & CPUIDLE_FLAG_TIME_VALID))) { + /* Use timer value as is */ + measured_us = data->next_timer_us; - measured_us = last_idle_us; + } else { + /* Use measured value */ + measured_us = cpuidle_get_last_residency(dev); - /* - * We correct for the exit latency; we are assuming here that the - * exit latency happens after the event that we're interested in. - */ - if (measured_us > target->exit_latency) - measured_us -= target->exit_latency; + /* Deduct exit latency */ + if (measured_us > target->exit_latency) + measured_us -= target->exit_latency; - /* Make sure our coefficients do not exceed unity */ - if (measured_us > data->next_timer_us) - measured_us = data->next_timer_us; + /* Make sure our coefficients do not exceed unity */ + if (measured_us > data->next_timer_us) + measured_us = data->next_timer_us; + } /* Update our correction ratio */ new_factor = data->correction_factor[data->bucket]; @@ -439,7 +447,7 @@ static void menu_update(struct cpuidle_driver *drv, struct cpuidle_device *dev) data->correction_factor[data->bucket] = new_factor; /* update the repeating-pattern data */ - data->intervals[data->interval_ptr++] = last_idle_us; + data->intervals[data->interval_ptr++] = measured_us; if (data->interval_ptr >= INTERVALS) data->interval_ptr = 0; } -- cgit v1.2.3 From 96e95182e95fd4e0069ff4d6ee1888fe9031d154 Mon Sep 17 00:00:00 2001 From: "tuukka.tikkanen@linaro.org" Date: Mon, 24 Feb 2014 08:29:35 +0200 Subject: cpuidle: Move perf multiplier calculation out of the selection loop The menu governor performance multiplier defines a minimum predicted idle duration to latency ratio. Instead of checking this separately in every iteration of the state selection loop, adjust the overall latency restriction for the whole loop if this restriction is tighter than what is set by the QoS subsystem. The original test s->exit_latency * multiplier > data->predicted_us becomes s->exit_latency > data->predicted_us / multiplier by dividing both sides of the comparison by "multiplier". While division is likely to be several times slower than multiplication, the minor performance hit allows making a generic sleep state selection function based on (sleep duration, maximum latency) tuple. Signed-off-by: Tuukka Tikkanen Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/governors/menu.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'drivers/cpuidle') diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index b347c101c1f7..71b523293354 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -288,7 +288,7 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev) struct menu_device *data = &__get_cpu_var(menu_devices); int latency_req = pm_qos_request(PM_QOS_CPU_DMA_LATENCY); int i; - int multiplier; + unsigned int interactivity_req; struct timespec t; if (data->needs_update) { @@ -310,8 +310,6 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev) data->bucket = which_bucket(data->next_timer_us); - multiplier = performance_multiplier(); - /* * if the correction factor is 0 (eg first time init or cpu hotplug * etc), we actually want to start out with a unity factor. @@ -330,6 +328,15 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev) get_typical_interval(data); + /* + * Performance multiplier defines a minimum predicted idle + * duration / latency ratio. Adjust the latency limit if + * necessary. + */ + interactivity_req = data->predicted_us / performance_multiplier(); + 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. @@ -353,8 +360,6 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev) continue; if (s->exit_latency > latency_req) continue; - if (s->exit_latency * multiplier > data->predicted_us) - continue; data->last_state_idx = i; } -- cgit v1.2.3 From 4b2f0b033a294e6c19d57c5d0a66c000f6299559 Mon Sep 17 00:00:00 2001 From: "tuukka.tikkanen@linaro.org" Date: Mon, 24 Feb 2014 08:29:37 +0200 Subject: cpuidle: poll state can measure residency For some platforms, a poll state is inserted in the cpuidle driver states. The flags for the state do not indicate that timekeeping is not affected. As the state does not do anything apart from calling cpu_relax(), the times returned by ktime_get should remain valid. Add the missing flag. Signed-off-by: Tuukka Tikkanen Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/cpuidle') diff --git a/drivers/cpuidle/driver.c b/drivers/cpuidle/driver.c index 06dbe7c86199..136d6a283e0a 100644 --- a/drivers/cpuidle/driver.c +++ b/drivers/cpuidle/driver.c @@ -209,7 +209,7 @@ static void poll_idle_init(struct cpuidle_driver *drv) state->exit_latency = 0; state->target_residency = 0; state->power_usage = -1; - state->flags = 0; + state->flags = CPUIDLE_FLAG_TIME_VALID; state->enter = poll_idle; state->disabled = false; } -- cgit v1.2.3 From 0b89e9aa28566cd3e96651dbdd769b026a476d49 Mon Sep 17 00:00:00 2001 From: Paul Burton Date: Thu, 6 Mar 2014 11:02:01 +0000 Subject: cpuidle: delay enabling interrupts until all coupled CPUs leave idle As described by a comment at the end of cpuidle_enter_state_coupled it can be inefficient for coupled idle states to return with IRQs enabled since they may proceed to service an interrupt instead of clearing the coupled idle state. Until they have finished & cleared the idle state all CPUs coupled with them will spin rather than being able to enter a safe idle state. Commits e1689795a784 "cpuidle: Add common time keeping and irq enabling" and 554c06ba3ee2 "cpuidle: remove en_core_tk_irqen flag" led to the cpuidle_enter_state enabling interrupts for all idle states, including coupled ones, making this inefficiency unavoidable by drivers & the local_irq_enable near the end of cpuidle_enter_state_coupled redundant. This patch avoids enabling interrupts in cpuidle_enter_state after a coupled state has been entered, allowing them to remain disabled until all coupled CPUs have exited the idle state and cpuidle_enter_state_coupled re-enables them. Cc: Daniel Lezcano Signed-off-by: Paul Burton Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/cpuidle.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/cpuidle') diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c index a55e68f2cfc8..366e6840ec46 100644 --- a/drivers/cpuidle/cpuidle.c +++ b/drivers/cpuidle/cpuidle.c @@ -85,7 +85,8 @@ int cpuidle_enter_state(struct cpuidle_device *dev, struct cpuidle_driver *drv, time_end = ktime_get(); - local_irq_enable(); + if (!cpuidle_state_is_coupled(dev, drv, entered_state)) + local_irq_enable(); diff = ktime_to_us(ktime_sub(time_end, time_start)); if (diff > INT_MAX) -- cgit v1.2.3