On Thu, Apr 22, 2021 at 10:31 PM Sowjanya Komatineni
<skomatineni@xxxxxxxxxx> 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 <skomatineni@xxxxxxxxxx>
> ---
> 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,
> --
