* [RFC PATCH] cpuidle: fix device->state_count handling
@ 2013-08-09 11:59 Bartlomiej Zolnierkiewicz
2013-08-09 15:47 ` Tomasz Figa
2013-08-09 16:01 ` Daniel Lezcano
0 siblings, 2 replies; 3+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2013-08-09 11:59 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Daniel Lezcano, Amit Daniel Kachhap, linux-pm, linux-samsung-soc
Use device->state_count instead of driver->state_count in
cpuidle_play_dead(), ladder_select_state() and menu_select().
This allows cpuidle drivers to override maximum allowable
state number for a given device, which is what some cpuidle
drivers would like to do (i.e. EXYNOS cpuidle driver).
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
Alternatively, we can make EXYNOS cpuidle driver work without
device->state_count (at a some tiny performance cost?). This
would allow us to remove device->state_count completely as it
is not needed by any other cpuidle driver currently (AFAICS).
drivers/cpuidle/cpuidle.c | 2 +-
drivers/cpuidle/governors/ladder.c | 2 +-
drivers/cpuidle/governors/menu.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
Index: b/drivers/cpuidle/cpuidle.c
===================================================================
--- a/drivers/cpuidle/cpuidle.c 2013-08-08 14:36:09.611488750 +0200
+++ b/drivers/cpuidle/cpuidle.c 2013-08-08 14:39:02.291486116 +0200
@@ -57,7 +57,7 @@ int cpuidle_play_dead(void)
return -ENODEV;
/* Find lowest-power state that supports long-term idle */
- for (i = drv->state_count - 1; i >= CPUIDLE_DRIVER_STATE_START; i--)
+ for (i = dev->state_count - 1; i >= CPUIDLE_DRIVER_STATE_START; i--)
if (drv->states[i].enter_dead)
return drv->states[i].enter_dead(dev, i);
Index: b/drivers/cpuidle/governors/ladder.c
===================================================================
--- a/drivers/cpuidle/governors/ladder.c 2013-08-08 14:36:09.611488750 +0200
+++ b/drivers/cpuidle/governors/ladder.c 2013-08-08 14:39:02.299486116 +0200
@@ -87,7 +87,7 @@ static int ladder_select_state(struct cp
last_residency = last_state->threshold.promotion_time + 1;
/* consider promotion */
- if (last_idx < drv->state_count - 1 &&
+ if (last_idx < dev->state_count - 1 &&
!drv->states[last_idx + 1].disabled &&
!dev->states_usage[last_idx + 1].disable &&
last_residency > last_state->threshold.promotion_time &&
Index: b/drivers/cpuidle/governors/menu.c
===================================================================
--- a/drivers/cpuidle/governors/menu.c 2013-08-08 14:36:09.611488750 +0200
+++ b/drivers/cpuidle/governors/menu.c 2013-08-08 14:39:02.299486116 +0200
@@ -312,7 +312,7 @@ static int menu_select(struct cpuidle_dr
* Find the idle state with the lowest power while satisfying
* our constraints.
*/
- for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) {
+ for (i = CPUIDLE_DRIVER_STATE_START; i < dev->state_count; i++) {
struct cpuidle_state *s = &drv->states[i];
struct cpuidle_state_usage *su = &dev->states_usage[i];
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [RFC PATCH] cpuidle: fix device->state_count handling
2013-08-09 11:59 [RFC PATCH] cpuidle: fix device->state_count handling Bartlomiej Zolnierkiewicz
@ 2013-08-09 15:47 ` Tomasz Figa
2013-08-09 16:01 ` Daniel Lezcano
1 sibling, 0 replies; 3+ messages in thread
From: Tomasz Figa @ 2013-08-09 15:47 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: Rafael J. Wysocki, Daniel Lezcano, Amit Daniel Kachhap, linux-pm,
linux-samsung-soc
Hi Bart,
On Friday 09 of August 2013 13:59:27 Bartlomiej Zolnierkiewicz wrote:
> Use device->state_count instead of driver->state_count in
> cpuidle_play_dead(), ladder_select_state() and menu_select().
>
> This allows cpuidle drivers to override maximum allowable
> state number for a given device, which is what some cpuidle
> drivers would like to do (i.e. EXYNOS cpuidle driver).
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
> Alternatively, we can make EXYNOS cpuidle driver work without
> device->state_count (at a some tiny performance cost?). This
> would allow us to remove device->state_count completely as it
> is not needed by any other cpuidle driver currently (AFAICS).
>
> drivers/cpuidle/cpuidle.c | 2 +-
> drivers/cpuidle/governors/ladder.c | 2 +-
> drivers/cpuidle/governors/menu.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> Index: b/drivers/cpuidle/cpuidle.c
> ===================================================================
> --- a/drivers/cpuidle/cpuidle.c 2013-08-08 14:36:09.611488750 +0200
> +++ b/drivers/cpuidle/cpuidle.c 2013-08-08 14:39:02.291486116 +0200
> @@ -57,7 +57,7 @@ int cpuidle_play_dead(void)
> return -ENODEV;
>
> /* Find lowest-power state that supports long-term idle */
> - for (i = drv->state_count - 1; i >= CPUIDLE_DRIVER_STATE_START; i--)
> + for (i = dev->state_count - 1; i >= CPUIDLE_DRIVER_STATE_START; i--)
> if (drv->states[i].enter_dead)
> return drv->states[i].enter_dead(dev, i);
>
> Index: b/drivers/cpuidle/governors/ladder.c
> ===================================================================
> --- a/drivers/cpuidle/governors/ladder.c 2013-08-08 14:36:09.611488750
> +0200 +++ b/drivers/cpuidle/governors/ladder.c 2013-08-08
> 14:39:02.299486116 +0200 @@ -87,7 +87,7 @@ static int
> ladder_select_state(struct cp
> last_residency = last_state->threshold.promotion_time + 1;
>
> /* consider promotion */
> - if (last_idx < drv->state_count - 1 &&
> + if (last_idx < dev->state_count - 1 &&
> !drv->states[last_idx + 1].disabled &&
> !dev->states_usage[last_idx + 1].disable &&
> last_residency > last_state->threshold.promotion_time &&
> Index: b/drivers/cpuidle/governors/menu.c
> ===================================================================
> --- a/drivers/cpuidle/governors/menu.c 2013-08-08 14:36:09.611488750
> +0200 +++ b/drivers/cpuidle/governors/menu.c 2013-08-08
> 14:39:02.299486116 +0200 @@ -312,7 +312,7 @@ static int
> menu_select(struct cpuidle_dr
> * Find the idle state with the lowest power while satisfying
> * our constraints.
> */
> - for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) {
> + for (i = CPUIDLE_DRIVER_STATE_START; i < dev->state_count; i++) {
> struct cpuidle_state *s = &drv->states[i];
> struct cpuidle_state_usage *su = &dev->states_usage[i];
>
Looks good to me. Have my
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [RFC PATCH] cpuidle: fix device->state_count handling
2013-08-09 11:59 [RFC PATCH] cpuidle: fix device->state_count handling Bartlomiej Zolnierkiewicz
2013-08-09 15:47 ` Tomasz Figa
@ 2013-08-09 16:01 ` Daniel Lezcano
1 sibling, 0 replies; 3+ messages in thread
From: Daniel Lezcano @ 2013-08-09 16:01 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: Rafael J. Wysocki, Amit Daniel Kachhap, linux-pm,
linux-samsung-soc
On 08/09/2013 01:59 PM, Bartlomiej Zolnierkiewicz wrote:
> Use device->state_count instead of driver->state_count in
> cpuidle_play_dead(), ladder_select_state() and menu_select().
>
> This allows cpuidle drivers to override maximum allowable
> state number for a given device, which is what some cpuidle
> drivers would like to do (i.e. EXYNOS cpuidle driver).
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
> Alternatively, we can make EXYNOS cpuidle driver work without
> device->state_count (at a some tiny performance cost?). This
> would allow us to remove device->state_count completely as it
> is not needed by any other cpuidle driver currently (AFAICS).
I am inclined for this solution.
The state_count was initially in the device structure, then it has been
moved to the driver structure and the code has been changed accordingly.
That was part of the cpuidle code consolidation made by Deepthi Dharwar
(46bcfad7).
This patch is one step back.
There is an alternate solution. May be you can use the multiple driver
support to register two cpuidle driver (one for each cpus) ? It was
initially made to support cpus with different latencies but that should
work.
> drivers/cpuidle/cpuidle.c | 2 +-
> drivers/cpuidle/governors/ladder.c | 2 +-
> drivers/cpuidle/governors/menu.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> Index: b/drivers/cpuidle/cpuidle.c
> ===================================================================
> --- a/drivers/cpuidle/cpuidle.c 2013-08-08 14:36:09.611488750 +0200
> +++ b/drivers/cpuidle/cpuidle.c 2013-08-08 14:39:02.291486116 +0200
> @@ -57,7 +57,7 @@ int cpuidle_play_dead(void)
> return -ENODEV;
>
> /* Find lowest-power state that supports long-term idle */
> - for (i = drv->state_count - 1; i >= CPUIDLE_DRIVER_STATE_START; i--)
> + for (i = dev->state_count - 1; i >= CPUIDLE_DRIVER_STATE_START; i--)
> if (drv->states[i].enter_dead)
> return drv->states[i].enter_dead(dev, i);
>
> Index: b/drivers/cpuidle/governors/ladder.c
> ===================================================================
> --- a/drivers/cpuidle/governors/ladder.c 2013-08-08 14:36:09.611488750 +0200
> +++ b/drivers/cpuidle/governors/ladder.c 2013-08-08 14:39:02.299486116 +0200
> @@ -87,7 +87,7 @@ static int ladder_select_state(struct cp
> last_residency = last_state->threshold.promotion_time + 1;
>
> /* consider promotion */
> - if (last_idx < drv->state_count - 1 &&
> + if (last_idx < dev->state_count - 1 &&
> !drv->states[last_idx + 1].disabled &&
> !dev->states_usage[last_idx + 1].disable &&
> last_residency > last_state->threshold.promotion_time &&
> Index: b/drivers/cpuidle/governors/menu.c
> ===================================================================
> --- a/drivers/cpuidle/governors/menu.c 2013-08-08 14:36:09.611488750 +0200
> +++ b/drivers/cpuidle/governors/menu.c 2013-08-08 14:39:02.299486116 +0200
> @@ -312,7 +312,7 @@ static int menu_select(struct cpuidle_dr
> * Find the idle state with the lowest power while satisfying
> * our constraints.
> */
> - for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) {
> + for (i = CPUIDLE_DRIVER_STATE_START; i < dev->state_count; i++) {
> struct cpuidle_state *s = &drv->states[i];
> struct cpuidle_state_usage *su = &dev->states_usage[i];
>
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
* English - detected
* English
* French
* English
* French
<javascript:void(0);>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-08-09 16:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-09 11:59 [RFC PATCH] cpuidle: fix device->state_count handling Bartlomiej Zolnierkiewicz
2013-08-09 15:47 ` Tomasz Figa
2013-08-09 16:01 ` Daniel Lezcano
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).