[Pm-utils] [PATCH] parse video quirks in uswsusp sleep module

[Victor Lowther]

On Mon, 2008-03-17 at 03:11 +0100, Michael Biebl wrote:
> 2008/3/17, Michael Biebl <mbiebl at gmail.com>:
> > 2008/3/17, Victor Lowther <victor.lowther at gmail.com>:
> >  > On Mon, 2008-03-17 at 01:55 +0100, Michael Biebl wrote:
> >  >  > 2008/3/17, Victor Lowther <victor.lowther at gmail.com>:
> >  >
> >  > > > Not really. :)  The behaviour that 99video uses is to set the acpi_sleep
> >  >  > >  flag based on quirks no matter the state of QUIRK_NONE.  This patch
> >  >  > >  brings the uswsusp code in line with that logic.
> >  >  >
> >  >  > acpi flags are quirks just like vbe post. QUIRK_NONE should skip all quirks.
> >  >  >
> >  >  > >  Whether this is correct behaviour is another question, but the logic
> >  >  > >  99video uses has been that way for a long time and I don't want to mess
> >  >  > >  with it without good cause.
> >  >  >
> >  >  > Not really that long.
> >  >  > The last officially release version (0.99.4) didn't have support for
> >  >  > QUIRK_NONE yet.
> >  >  > You added support for QUIRK_NONE on 28.02.
> >  >  > Your initial version skipped 99video completely.
> >  >  > On 01.03 (two weeks ago), you restructured 99video and since then acpi
> >  >  > flags were not skipped any more (the commit log doesn't tell why)
> >  >
> >  >
> >  > I stand corrected. I did not get the quirk_none logic from preexisting
> >  >  pm-utils code. I apoligise for the confusion.
> >  >
> >  >
> >  >  > There is imho no good reason, why we should treat acpi flags special.
> >  >  > Imho we should fix 99video.
> >  >
> >  >
> >  > After a bit more thinking, I remember where I got that logic -- from
> >  >  s2ram-x86.c in the uswsusp source code.  Specifically, this bit:
> >
> >
> > Color me stupid, but why does the s2ram code imply this logic?
> >  s2ram simply has no --quirk-none option.
> >
> >  no quirks, stupid as it sounds, means no quirks for me.
> 
> Actually, I think the QUIRKS_NONE option has a slightly different meaning:
> If no quirks are passed to pm-suspend, pm-suspend does not know if that is
> a) because the machine doesn't require quirks
> b) because the machine hasn't been tested yet.

huh?  In the case where hal is invoking pm-utils, I would think that it
would simply not invoke pm-utils at all if the machine is not in the
database.  

> QUIRKS_NONE=true simply documents that it is safe to suspend the
> machine if no --quirks-* options are given. It doesn't mean, that
> other --quirks-* should be cleared.
> A command line like
> pm-suspend --quirk-none --quirk-vbe-post is simply invalid.
> 
> QUIRK_NONE thus would only be interersting within do_suspend.
> Atm. we unconditionally do "echo mem > /sys/power/state".
> QUIRK_NONE would allow us to check if the machine has been
> successfully tested or not and only do the suspend in that case.
> We don't do that, so strictly speaking we could just ignore QUIRK_NONE
> (and I guess that's the reason why I ignored it in uswsusp).
> 
> I actually think, that the QUIRK_NONE tests in 99video could/should
> also be removed.

Well, I speak from experience when I say that s2ram and HAL get it wrong
when figuring out what quirks to apply on my system.  I suspect this is
true on every laptop which can purchased with different models of video
card or anything which has binary drivers as well as open-source
drivers.  

We need a mechanism that (at the least) allows an end-user to easily
override the parameters that HAL (or whoever) is passing pm-utils.
Adding --quirk-none via a seperate parameters file my first step to such
a solution.  It is not an optimal solution, but if you want to remove it
then you will need to provide a solution that does not involve an
end-user writing their own hook.

/sigh If it weren't for video quirks, this whole suspend/resume thing
would be much easier.

> As said, combinations like --quirk-none --quirk-vbe-mode are simply invalid.

Well, they should be invalid.  I have no idea whether HAL could actually
invoke pm-utils with such a combination of flags.

> Cheers,
> Michael
> 
> 
> 
> 
> 
-- 
Victor Lowther
Ubuntu Certified Professional