Message-Id: <1177669279.7828.55.camel@johannes.berg>
Date:	Fri, 27 Apr 2007 12:21:19 +0200
From:	Johannes Berg <johannes@...solutions.net>
To:	Pavel Machek <pavel@....cz>
Cc:	"Rafael J. Wysocki" <rjw@...k.pl>, Nick Piggin <npiggin@...e.de>,
	Mike Galbraith <efault@....de>, linux-kernel@...r.kernel.org,
	Adrian Bunk <bunk@...sta.de>,
	Thomas Gleixner <tglx@...utronix.de>,
	Con Kolivas <kernel@...ivas.org>,
	suspend2-devel@...ts.suspend2.net, Ingo Molnar <mingo@...e.hu>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Arjan van de Ven <arjan@...radead.org>,
	linux-pm <linux-pm@...ts.linux-foundation.org>
Subject: driver power operations (was Re: suspend2 merge)

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.

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.

johannes

Download attachment "signature.asc" of type "application/pgp-signature" (191 bytes)

Powered by blists - more mailing lists