Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Thu, 12 Jan 2023 18:25:04 +0100 | Subject | Re: [PATCH v2 2/4] powercap: idle_inject: Add prepare/complete callbacks |
| |
On Thu, Jan 12, 2023 at 3:53 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Wed, Nov 30, 2022 at 12:34 AM Srinivas Pandruvada > <srinivas.pandruvada@linux.intel.com> wrote: > > > > The actual idle percentage can be less than the desired because of > > interrupts. > > This is somewhat unclear. > > > Since the objective for CPU Idle injection is for thermal > > control, there should be some way to compensate for lost idle percentage. > > What does "lost idle percentage" mean here? > > > Some architectures provide interface to get actual idle percent observed > > by the hardware. So, the idle percent can be adjusted using the hardware > > feedback. For example, Intel CPUs provides package idle counters, which > > is currently used by intel powerclamp driver to adjust idle time. > > > > The only way this can be done currently is by monitoring hardware idle > > percent from a different software thread. This can be avoided by adding > > callbacks. > > > > Add a capability to register a prepare and complete callback during idle > > inject registry. Add a new register function idle_inject_register_full() > > which also allows to register callbacks. > > > > If they are not NULL, then prepare callback is called before calling > > play_idle_precise() and complete callback is called after calling > > play_idle_precise(). > > > > If prepare callback is present and returns non 0 value then > > play_idle_precise() is not called to avoid over compensation. > > This mechanism isn't particularly straightforward, but maybe there's > no better way. > > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > > --- > > v2 > > - Replace begin/end with prepare/complete > > - Add new interface idle_inject_register_full with callbacks > > - Update kernel doc > > - Update commit description > > > > drivers/powercap/idle_inject.c | 62 +++++++++++++++++++++++++++++++--- > > include/linux/idle_inject.h | 4 +++ > > 2 files changed, 62 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/powercap/idle_inject.c b/drivers/powercap/idle_inject.c > > index dfa989182e71..f48e71501429 100644 > > --- a/drivers/powercap/idle_inject.c > > +++ b/drivers/powercap/idle_inject.c > > @@ -63,13 +63,31 @@ struct idle_inject_thread { > > * @idle_duration_us: duration of CPU idle time to inject > > * @run_duration_us: duration of CPU run time to allow > > * @latency_us: max allowed latency > > + * @prepare: Callback function which is called before calling > > + * play_idle_precise() > > + * @complete: Callback function which is called after calling > > + * play_idle_precise() > > What about: > > @prepare: Optional callback deciding whether or not to skip idle > injection in the given cycle. > @complete: Optional callback updating the state after idle injection.
One more thing: ->complete() is not even used by powerclamp AFAICS, so I wouldn't add it at this time, because it isn't clear if it's going to be useful at all.
| |