Re: [PATCH] cpufreq: suspend/resume governors with PM notifiers

["Rafael J. Wysocki" <rjw@rjwysocki.net>]

On Saturday, November 16, 2013 08:47:24 PM Viresh Kumar wrote:
> On 16 November 2013 19:59, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> 
> > Well, disabling it for the whole duration of suspend/resume and/or hibernation
> > may not be the right approach entirely, unless we force the pax perf of the
> 
> s/pax/max ?

Yes.

> > boot CPU at least in addition to that.  Otherwise the latency of suspend and
> > the subsequent resume will depend on what perf level the CPUs where before
> > disabling the governors, which is not desirable at al.
> 
> Well that is pretty much doable.

Not necessarily on all CPU models.

> >> And these are the notifications that we send:
> >> - PM_HIBERNATION_PREPARE
> >> - PM_POST_HIBERNATION
> >> - PM_RESTORE_PREPARE
> >> - PM_POST_RESTORE
> >>
> >> If I am not wrong I need to stop governors on PM_HIBERNATION_PREPARE and need to
> >> start them back on: PM_POST_HIBERNATION (I am a bit confused with this one. Does
> >> this POST_HIBERNATION one happens at the end of going into hibernation? or after
> >> booting back? I need a notifier at the end of restore)..
> >
> > You'd need both PM_POST_HIBERNATION and PM_POST_RESTORE, but I wouldn't really
> > like cpufreq governors to be disabled throughout the whole hibernation.
> 
> So PM_POST_HIBERNATION is called just before shutting off the system? And
> PM_POST_RESTORE is called after system is resumed from saved image?

PM_POST_HIBERNATION is only called if there's an error during hibernation.
PM_POST_RESTORE is called as you said.

Also you have to remember that the _PREPARE PM notifiers are called before the
freezing of tasks when user space is still running, so disabling governors at
that point may lead to some weird behavior.

> > Actually, we use CPU offline/online during system suspend/resume to avoid
> > having to do stuff like this from PM notifiers.
> 
> I didn't get the logic behind this one..

If we have to do special stuff from PM notifiers for CPU "suspend", we will be
better off by doing something entirely special instead of CPU offline down the
road.  Which we may end up doing given the problems with frozen/not frozen in
the cpufreq core.

We may introduce suspend_noirq and resume_noirq for cpu_subsys, for example,
and handle things from there.  Or something similar.  But slapping PM notifiers
on top of the existing code just because it appears to be easy (and making that
code even more overdesigned than it already is this way) doesn't seem quite
right.

Now, the Tianyu's patch extends the Srivatsa's approach to governors, which
actually should have been done from the outset, so it is within the scope of
what we have already.  It may not solve all of the problems, but it still makes
some progress and has a little chance to introduce *new* problems at the same
time.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.