Re: driver power operations (was Re: suspend2 merge)

Rafael J. Wysocki
2007-04-27T12:02:56Z

On Friday, 27 April 2007 12:21, Johannes Berg wrote:
> On Wed, 2007-04-25 at 22:31 +0200, Pavel Machek wrote:
>
> > > So, the "suspend" and "resume" for the functions being called for that are
> > > wrong, but then we call them with PMSG_FREEZE. ;-) Still, we could add
> > > .freeze() and .thaw() callbacks for hibernation just fine. This wouldn't even
> > > be that difficult ...
> >
> > It would be ugly big patch I'm afraid.
>
> It'd be a lot of code churn, but well worth it. And most of the changes
> would be trivial too. You need to start looking beyond "this is ugly in
> the short term" and realise that it's much more maintainable in the long
> term if driver writers know what they're supposed to do as opposed to
> just hacking at it until it mostly works or just doing a full device
> down/up cycle including resetting full driver state.
>
> Look at it now:
>
> * FREEZE Quiesce operations so that a consistent image can be saved;
> * but do NOT otherwise enter a low power device state, and do
> * NOT emit system wakeup events.
> *
> * PRETHAW Quiesce as if for FREEZE; additionally, prepare for restoring
> * the system from a snapshot taken after an earlier FREEZE.
> * Some drivers will need to reset their hardware state instead
> * of preserving it, to ensure that it's never mistaken for the
> * state which that earlier snapshot had set up.
>
> Why is prethaw even necessary? As far as I can tell it's only necessary
> because resume() can't tell you whether you just want to thaw or need to
> reset since it doesn't tell you at what point it's invoked.
>
> Having ->freeze(), ->thaw() and ->restart() (can somebody come up with a
> better name?) that are called at the appropriate places (with
> freeze/thaw around preparing the image and freeze/restart around
> restoring would go a long way of clearing up the confusion in all the
> drivers. Of course, it'd have to be documented that freeze/thaw isn't
> the only valid combination but that freeze/restart is used too, but
> that's not hard to do nor hard to understand.
>
> And, incidentally, it could possibly make both suspend and hibernate
> work much faster too. The comments there talk about "minimally power
> management aware" drivers which always do the wrong thing for suspend,
> in that they always reset everything... Of course, some drivers will
> actually need to do that, but if freeze/suspend and thaw/restart/resume
> have the same prototypes (probably just int <function>(void)) then
> drivers can trivially assign the same there.
> And hibernate would benefit since a lot of drivers could do a lot less
> work for freeze/thaw.

I violently agree with all of the above.

Moreover, for the hibernation we have two special cases that are of no interest
for the suspend:
1) drivers compiled as modules and not loaded before we restore the image
2) drivers that need to allocate much memory in .freeze()

> Or, if we don't want to have five calls and use 40 bytes (on 64-bit)
> just for these callback pointers for each device we could just as well
> have a single callback ->pm(what) and make "what" indicate which one of
> these five things... But then drivers can't make that code depend on the
> swsusp configuration which would be doable with five callbacks.

Five callbacks are fine by me, especially if we can define reasonable defaults
for the hibernation (and can we?).

Rafael