Re: [PATCH v2 RESEND] thermal core: add option to run PM_POST_SUSPEND asynchronously

[Radu Solea <radusolea@google.com>]

Thank you for taking the time to discuss this.
In the context of PM_POST_SUSPEND, there are not many guarantees. It
mainly just guarantees that the code is executed at a particular step
in the resume sequence and, implicitly, that the execution happens
before another power transition. There are no order or timing promises
made.
Doing work on the notification chain impacts at least two things
relevant to this change: triggering of the next items on the chain and
completion of the write() to /sys/power/state. In a multicore context,
both of these matter.
The system context is battery-powered embedded devices with thermal
zones on-board. These thermal zones are bus connected and operate
asynchronously to the executing core, introducing delays.
I proposed the change as optional because I cannot account for all
possible cases. However, in the specific context of these devices,
custom kernel configurations are the norm and it is expected that the
integration team validates system assumptions (such as system
unbounded queue load).
The ultimate reason for this change is that I want my 50ms back.
Joking aside, there are not many ways to determine that the system has
completed a resume cycle. The write() to /sys/power/state completing
is important in signaling that to the system at large.
I did not include numbers in the initial submission because I do not
expect that the numbers I see on a particular integration are easily
replicable in other situations. However, the 50ms I am currently
chasing are important in user-interactive systems and impact overall
power consumption.
Thank you,
Radu

On Wed, Nov 29, 2023 at 5:09 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Tue, Nov 21, 2023 at 12:40 AM Radu Solea <radusolea@google.com> wrote:
> >
> > Some thermal zones are bus connected and slow to resume, thus
> > delaying actions which depend on completion of PM_POST_SUSPEND.
>
> What actions in particular?
>
> > Add optional execution path to resume thermal zones on the system
> > unbounded workqueue.
>
> Why optional?
>
> This is only useful for people building their own custom kernels.
>
> > Signed-off-by: Radu Solea <radusolea@google.com>
> > ---
> >  drivers/thermal/Kconfig        | 11 +++++++
> >  drivers/thermal/thermal_core.c | 58 ++++++++++++++++++++++++++++++----
> >  2 files changed, 62 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > index c81a00fbca7d..148d6e9734c6 100644
> > --- a/drivers/thermal/Kconfig
> > +++ b/drivers/thermal/Kconfig
> > @@ -91,6 +91,17 @@ config THERMAL_WRITABLE_TRIPS
> >           Say 'Y' here if you would like to allow userspace tools to
> >           change trip temperatures.
> >
> > +config THERMAL_ASYNC_RESUME
> > +       bool "Thermal async init zones on system resume"
> > +       default n
> > +       help
> > +         Re-initialize thermal zones asynchronously on system resume.
> > +         Thermal zone sensors may be attached on slow buses, impacting
>
> "Slow" relative to what?  How can it be determined?
>
> > +         the duration of PM_POST_SUSPEND. If that is a concern enable
> > +         this switch.
> > +
> > +         If in doubt, say N.
> > +
> >  choice
> >         prompt "Default Thermal governor"
> >         default THERMAL_DEFAULT_GOV_STEP_WISE
>
> In the first place, I would like to know the exact motivation for this change.