diff mbox series

[RFC,2/4] cpuidle: menu: add idle_time to cpuidle_state

Message ID 1619123448-10138-3-git-send-email-skomatineni@nvidia.com
State New
Headers show
Series None | expand

Commit Message

Sowjanya Komatineni April 22, 2021, 8:30 p.m. UTC
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 <skomatineni@nvidia.com>
---
 drivers/cpuidle/governors/menu.c | 7 ++++++-
 include/linux/cpuidle.h          | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

Comments

Rafael J. Wysocki April 23, 2021, 12:22 p.m. UTC | #1
On Thu, Apr 22, 2021 at 10:31 PM Sowjanya Komatineni
<skomatineni@nvidia.com> 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@nvidia.com>
> ---
>  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,
> --
diff mbox series

Patch

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,