From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: linux-pm@vger.kernel.org
Cc: jwerner@chromium.org, khilman@ti.com, len.brown@intel.com,
g.trinabh@gmail.com, deepthi@linux.vnet.ibm.com,
linux-kernel@vger.kernel.org, rjw@sisk.pl,
akpm@linux-foundation.org, snanda@chromium.org,
linaro-dev@lists.linaro.org
Subject: [RFC] cpuidle - remove the power_specified field in the driver
Date: Mon, 12 Nov 2012 21:26:56 +0100 [thread overview]
Message-ID: <1352752016-3136-1-git-send-email-daniel.lezcano@linaro.org> (raw)
In-Reply-To: <50831CA3.2020602@linaro.org>
This patch follows the discussion about reinitializing the power usage
when a C-state is added/removed.
https://lkml.org/lkml/2012/10/16/518
We realized the power usage field is never filled and when it is
filled for tegra, the power_specified flag is not set making all these
values to be resetted when the driver is initialized with the set_power_state
function.
Julius and I feel this is over-engineered and the power_specified
flag could be simply removed and continue assuming the states are
backward sorted.
The menu governor select function is simplified as the power is ordered.
Actually the condition is always true with the current code.
The cpuidle_play_dead function is also simplified by doing a reverse lookup
on the array.
The set_power_states function is removed as it does no make sense anymore.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
drivers/cpuidle/cpuidle.c | 17 ++++-------------
drivers/cpuidle/driver.c | 25 -------------------------
| 8 ++------
include/linux/cpuidle.h | 2 +-
4 files changed, 7 insertions(+), 45 deletions(-)
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 711dd83..f983262 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -69,24 +69,15 @@ int cpuidle_play_dead(void)
{
struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
- int i, dead_state = -1;
- int power_usage = -1;
+ int i;
if (!drv)
return -ENODEV;
/* Find lowest-power state that supports long-term idle */
- for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) {
- struct cpuidle_state *s = &drv->states[i];
-
- if (s->power_usage < power_usage && s->enter_dead) {
- power_usage = s->power_usage;
- dead_state = i;
- }
- }
-
- if (dead_state != -1)
- return drv->states[dead_state].enter_dead(dev, dead_state);
+ for (i = drv->state_count; i >= CPUIDLE_DRIVER_STATE_START; i--)
+ if (drv->states[i].play_dead)
+ return drv->states[i].enter_dead(dev, i);
return -ENODEV;
}
diff --git a/drivers/cpuidle/driver.c b/drivers/cpuidle/driver.c
index 3af841f..bb045f1 100644
--- a/drivers/cpuidle/driver.c
+++ b/drivers/cpuidle/driver.c
@@ -19,34 +19,9 @@ DEFINE_SPINLOCK(cpuidle_driver_lock);
static void __cpuidle_set_cpu_driver(struct cpuidle_driver *drv, int cpu);
static struct cpuidle_driver * __cpuidle_get_cpu_driver(int cpu);
-static void set_power_states(struct cpuidle_driver *drv)
-{
- int i;
-
- /*
- * cpuidle driver should set the drv->power_specified bit
- * before registering if the driver provides
- * power_usage numbers.
- *
- * If power_specified is not set,
- * we fill in power_usage with decreasing values as the
- * cpuidle code has an implicit assumption that state Cn
- * uses less power than C(n-1).
- *
- * With CONFIG_ARCH_HAS_CPU_RELAX, C0 is already assigned
- * an power value of -1. So we use -2, -3, etc, for other
- * c-states.
- */
- for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++)
- drv->states[i].power_usage = -1 - i;
-}
-
static void __cpuidle_driver_init(struct cpuidle_driver *drv)
{
drv->refcnt = 0;
-
- if (!drv->power_specified)
- set_power_states(drv);
}
static int __cpuidle_register_driver(struct cpuidle_driver *drv, int cpu)
--git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
index 2efee27..14eb11f 100644
--- a/drivers/cpuidle/governors/menu.c
+++ b/drivers/cpuidle/governors/menu.c
@@ -312,7 +312,6 @@ 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 power_usage = -1;
int i;
int multiplier;
struct timespec t;
@@ -383,11 +382,8 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
if (s->exit_latency * multiplier > data->predicted_us)
continue;
- if (s->power_usage < power_usage) {
- power_usage = s->power_usage;
- data->last_state_idx = i;
- data->exit_us = s->exit_latency;
- }
+ data->last_state_idx = i;
+ data->exit_us = s->exit_latency;
}
/* not deepest C-state chosen for low predicted residency */
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 3711b34..24cd103 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -126,9 +126,9 @@ struct cpuidle_driver {
struct module *owner;
int refcnt;
- unsigned int power_specified:1;
/* set to 1 to use the core cpuidle time keeping (for all states). */
unsigned int en_core_tk_irqen:1;
+ /* states array must be ordered in decreasing power consumption */
struct cpuidle_state states[CPUIDLE_STATE_MAX];
int state_count;
int safe_state_index;
--
1.7.5.4
next prev parent reply other threads:[~2012-11-12 20:27 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-16 22:39 [PATCH] cpuidle: reinitialize power_usage values when adding/removing C-states Julius Werner
2012-10-17 10:31 ` Daniel Lezcano
2012-10-17 10:44 ` Daniel Lezcano
2012-10-17 18:43 ` Julius Werner
2012-10-18 8:21 ` Daniel Lezcano
2012-10-19 21:50 ` [PATCH] acpi/cpuidle: " Julius Werner
2012-10-20 21:50 ` Daniel Lezcano
2012-10-22 17:13 ` Julius Werner
2012-10-22 17:21 ` Daniel Lezcano
2012-11-12 20:26 ` Daniel Lezcano [this message]
2012-11-12 21:09 ` [RFC] cpuidle - remove the power_specified field in the driver Julius Werner
2012-11-12 22:08 ` Daniel Lezcano
2012-11-18 8:40 ` Francesco Lavra
2012-11-18 9:17 ` Daniel Lezcano
2012-12-10 19:09 ` Julius Werner
2012-12-10 22:41 ` Rafael J. Wysocki
2012-12-11 9:46 ` Daniel Lezcano
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=1352752016-3136-1-git-send-email-daniel.lezcano@linaro.org \
--to=daniel.lezcano@linaro.org \
--cc=akpm@linux-foundation.org \
--cc=deepthi@linux.vnet.ibm.com \
--cc=g.trinabh@gmail.com \
--cc=jwerner@chromium.org \
--cc=khilman@ti.com \
--cc=len.brown@intel.com \
--cc=linaro-dev@lists.linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rjw@sisk.pl \
--cc=snanda@chromium.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).