2021-04-22 20:32:58

by Sowjanya Komatineni

[permalink] [raw]
Subject: [RFC PATCH 2/4] cpuidle: menu: add idle_time to cpuidle_state

Some platforms use separate CPU firmware running in background to
handle state transitions which may need runtime idle time of the
corresponding target state from the kernel.

This patch adds idle_time to cpuidle state to expose to cpuidle driver the
idle time that the governor menu predicts based on next events and states
target residency for selecting proper idle state.

CPU idle driver passes this runtime state idle time to TF-A.

Signed-off-by: Sowjanya Komatineni <[email protected]>
---
drivers/cpuidle/governors/menu.c | 7 ++++++-
include/linux/cpuidle.h | 1 +
2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
index c3aa8d6..0da5bc5 100644
--- a/drivers/cpuidle/governors/menu.c
+++ b/drivers/cpuidle/governors/menu.c
@@ -382,8 +382,10 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
* stuck in the shallow one for too long.
*/
if (drv->states[idx].target_residency_ns < TICK_NSEC &&
- s->target_residency_ns <= delta_tick)
+ s->target_residency_ns <= delta_tick) {
+ drv->states[idx].idle_time = delta_tick / NSEC_PER_USEC;
idx = i;
+ }

return idx;
}
@@ -393,6 +395,7 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
idx = i;
}

+ drv->states[idx].idle_time = predicted_ns / NSEC_PER_USEC;
if (idx == -1)
idx = 0; /* No states enabled. Must use 0. */

@@ -419,6 +422,8 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
if (drv->states[i].target_residency_ns <= delta_tick)
break;
}
+
+ drv->states[idx].idle_time = delta_tick / NSEC_PER_USEC;
}
}

diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index fce4762..12db2e9 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -55,6 +55,7 @@ struct cpuidle_state {
unsigned int exit_latency; /* in US */
int power_usage; /* in mW */
unsigned int target_residency; /* in US */
+ unsigned int idle_time; /* in US */

int (*enter) (struct cpuidle_device *dev,
struct cpuidle_driver *drv,
--
2.7.4


2021-04-23 12:27:39

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [RFC PATCH 2/4] cpuidle: menu: add idle_time to cpuidle_state

On Thu, Apr 22, 2021 at 10:31 PM Sowjanya Komatineni
<[email protected]> wrote:
>
> Some platforms use separate CPU firmware running in background to
> handle state transitions which may need runtime idle time of the
> corresponding target state from the kernel.

How exactly does this work?

> This patch adds idle_time to cpuidle state to expose to cpuidle driver the
> idle time that the governor menu predicts based on next events and states
> target residency for selecting proper idle state.
>
> CPU idle driver passes this runtime state idle time to TF-A.
>
> Signed-off-by: Sowjanya Komatineni <[email protected]>
> ---
> drivers/cpuidle/governors/menu.c | 7 ++++++-
> include/linux/cpuidle.h | 1 +
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
> index c3aa8d6..0da5bc5 100644
> --- a/drivers/cpuidle/governors/menu.c
> +++ b/drivers/cpuidle/governors/menu.c
> @@ -382,8 +382,10 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
> * stuck in the shallow one for too long.
> */
> if (drv->states[idx].target_residency_ns < TICK_NSEC &&
> - s->target_residency_ns <= delta_tick)
> + s->target_residency_ns <= delta_tick) {
> + drv->states[idx].idle_time = delta_tick / NSEC_PER_USEC;
> idx = i;
> + }
>
> return idx;
> }
> @@ -393,6 +395,7 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
> idx = i;
> }
>
> + drv->states[idx].idle_time = predicted_ns / NSEC_PER_USEC;
> if (idx == -1)
> idx = 0; /* No states enabled. Must use 0. */
>
> @@ -419,6 +422,8 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
> if (drv->states[i].target_residency_ns <= delta_tick)
> break;
> }
> +
> + drv->states[idx].idle_time = delta_tick / NSEC_PER_USEC;
> }
> }
>
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index fce4762..12db2e9 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -55,6 +55,7 @@ struct cpuidle_state {
> unsigned int exit_latency; /* in US */
> int power_usage; /* in mW */
> unsigned int target_residency; /* in US */
> + unsigned int idle_time; /* in US */

No way.

This structure holds idle state properties of and not some random data
passed between cpuidle components. And it is not per-CPU, while the
governors work on the per-CPU basis.

state_usage might be more suitable, but that only if I'm convinced
that this is really necessary.

>
> int (*enter) (struct cpuidle_device *dev,
> struct cpuidle_driver *drv,
> --

2021-04-23 18:37:16

by Sowjanya Komatineni

[permalink] [raw]
Subject: Re: [RFC PATCH 2/4] cpuidle: menu: add idle_time to cpuidle_state


On 4/23/21 5:22 AM, Rafael J. Wysocki wrote:
> On Thu, Apr 22, 2021 at 10:31 PM Sowjanya Komatineni
> <[email protected]> wrote:
>> Some platforms use separate CPU firmware running in background to
>> handle state transitions which may need runtime idle time of the
>> corresponding target state from the kernel.
> How exactly does this work?
>
Explained this as part of other feedback in Patch-0

Thanks

Sowjanya