Suspend and hibernation patchset against -rc6

Suspend and hibernation patchset against -rc6

[Rafael J. Wysocki]

On Wednesday, 12 of March 2008, Rafael J. Wysocki wrote:
>
> For everyone interested, I have put together a patchset containing some "fresh"
> patches related to suspend and hibernation, on top of 2.6.25-rc5, located at:
> 
> http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.25-rc5/patches/

There's a new patchset against 2.6.25-rc6 at:

http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.25-rc6/patches/

It contains some more patches, specifically the "legacy PM removal" series, the
fixed "PM: make wakeup flags available whenever CONFIG_PM is set" series posted
recently by Alan (the second patch rebased by me), the Johannes' apm-emulation
patch and the patches introducing the new suspend/hibernation callbacks I sent
yesterday.

These remarks still apply:

> I have not yet decided what to do with patches 01-03.  Patch 03 will probably
> be sent upstream soon.
> 
> Patch 04 is currently in the "test" branch of the ACPI tree and has been
> tested already for some time in -mm and linux-next.

Also, patches 05-06 are upstream (in the Greg's tree).

Everything has been compiled and (lightly) tested on HP nx6325 and Asus L5D
(both 64-bit).

Thanks,
Rafael

Re: Suspend and hibernation patchset against -rc6

[Pavel Machek]

Hi!

> > For everyone interested, I have put together a patchset containing some "fresh"
> > patches related to suspend and hibernation, on top of 2.6.25-rc5, located at:
> > 
> > http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.25-rc5/patches/
> 
> There's a new patchset against 2.6.25-rc6 at:
> 
> http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.25-rc6/patches/
> 
> It contains some more patches, specifically the "legacy PM removal" series, the
> fixed "PM: make wakeup flags available whenever CONFIG_PM is set" series posted
> recently by Alan (the second patch rebased by me), the Johannes' apm-emulation
> patch and the patches introducing the new suspend/hibernation callbacks I sent
> yesterday.

Ok, I ran basic tests on thinkpad x60, and they seem to work.

...one strange thing is I'm seeing some delays, I'll investigate a bit
more.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les:  http://www.ujezdskystrom.info/

suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Pavel Machek]

Confirmed, suspend slowness is in 2.6.25-rc6, too. It takes 15 seconds
to suspend/resume, while 2.6.24 takes 9. Thinkpad x60.

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les:  http://www.ujezdskystrom.info/

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Pavel Machek]

On Tue 2008-03-18 14:06:42, Pavel Machek wrote:
> 
> Confirmed, suspend slowness is in 2.6.25-rc6, too. It takes 15 seconds
> to suspend/resume, while 2.6.24 takes 9. Thinkpad x60.

IIRC 2.6.25-rc5 was "fast".
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les:  http://www.ujezdskystrom.info/

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Pavel Machek]

On Tue 2008-03-18 14:07:10, Pavel Machek wrote:
> On Tue 2008-03-18 14:06:42, Pavel Machek wrote:
> > 
> > Confirmed, suspend slowness is in 2.6.25-rc6, too. It takes 15 seconds
> > to suspend/resume, while 2.6.24 takes 9. Thinkpad x60.
> 
> IIRC 2.6.25-rc5 was "fast".

rc6:

CPU0 attaching sched-domain:
 domain 0: span 3
  groups: 1 2
CPU1 attaching sched-domain:
 domain 0: span 3
  groups: 2 1
CPU1 is up
ACPI: EC: missing OBF confirmation, don't expect it any longer.


(there's ~.5 sec delay here)

ACPI: EC: missing write data confirmation, don't expect it any longer.

(and > 3 sec delay here)

PM: Writing back config space on device 0000:00:02.0 at offset 1 (was
900003, writing 900007)
PCI: Setting latency timer of device 0000:00:02.0 to 64
PM: Writing back config space on device 0000:00:02.1 at offset 1 (was
900000, writing 900003)
PM: Writing back config space on device 0000:00:1b.0 at offset 1 (was
100106, writing 100102)
PCI: Setting latency timer of device 0000:00:1b.0 to 64
PCI: Setting latency timer of device 0000:00:1c.0 to 64
PCI: Setting latency timer of device 0000:00:1c.1 to 64


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les:  http://www.ujezdskystrom.info/

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Alexey Starikovskiy]

Pavel Machek wrote:
> On Tue 2008-03-18 14:07:10, Pavel Machek wrote:
>> On Tue 2008-03-18 14:06:42, Pavel Machek wrote:
>>> Confirmed, suspend slowness is in 2.6.25-rc6, too. It takes 15 seconds
>>> to suspend/resume, while 2.6.24 takes 9. Thinkpad x60.
>> IIRC 2.6.25-rc5 was "fast".
> 
> rc6:
> 
> CPU0 attaching sched-domain:
>  domain 0: span 3
>   groups: 1 2
> CPU1 attaching sched-domain:
>  domain 0: span 3
>   groups: 2 1
> CPU1 is up
> ACPI: EC: missing OBF confirmation, don't expect it any longer.
This means that either ACPI interrupt or EC GPE is already disabled,
but readings from EC continue. 
> 
> 
> (there's ~.5 sec delay here)
> 
> ACPI: EC: missing write data confirmation, don't expect it any longer.
> 
> (and > 3 sec delay here)
> 
> PM: Writing back config space on device 0000:00:02.0 at offset 1 (was
> 900003, writing 900007)
> PCI: Setting latency timer of device 0000:00:02.0 to 64
> PM: Writing back config space on device 0000:00:02.1 at offset 1 (was
> 900000, writing 900003)
> PM: Writing back config space on device 0000:00:1b.0 at offset 1 (was
> 100106, writing 100102)
> PCI: Setting latency timer of device 0000:00:1b.0 to 64
> PCI: Setting latency timer of device 0000:00:1c.0 to 64
> PCI: Setting latency timer of device 0000:00:1c.1 to 64
> 
> 

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Pavel Machek]

On Tue 2008-03-18 21:48:46, Alexey Starikovskiy wrote:
> Pavel Machek wrote:
>> On Tue 2008-03-18 14:07:10, Pavel Machek wrote:
>>> On Tue 2008-03-18 14:06:42, Pavel Machek wrote:
>>>> Confirmed, suspend slowness is in 2.6.25-rc6, too. It takes 15 seconds
>>>> to suspend/resume, while 2.6.24 takes 9. Thinkpad x60.
>>> IIRC 2.6.25-rc5 was "fast".
>>
>> rc6:
>>
>> CPU0 attaching sched-domain:
>>  domain 0: span 3
>>   groups: 1 2
>> CPU1 attaching sched-domain:
>>  domain 0: span 3
>>   groups: 2 1
>> CPU1 is up
>> ACPI: EC: missing OBF confirmation, don't expect it any longer.
> This means that either ACPI interrupt or EC GPE is already disabled,
> but readings from EC continue. 

...and I was getting it with "fast" suspends, too, so this may not be
the core reason.

I should get some sleep soon, perhaps intel has some x60 machines
around?

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Linus Torvalds]

On Tue, 18 Mar 2008, Pavel Machek wrote:
>
> On Tue 2008-03-18 14:06:42, Pavel Machek wrote:
> > 
> > Confirmed, suspend slowness is in 2.6.25-rc6, too. It takes 15 seconds
> > to suspend/resume, while 2.6.24 takes 9. Thinkpad x60.
> 
> IIRC 2.6.25-rc5 was "fast".

Can you bisect it? There's only 343 commits between -rc5 and -rc6, so it 
should not take too long to check which commit it is. Even if it should 
take 9 reboots to bisect it entirely, going just five or six will likely 
narrow it down sufficiently that we can probably guess fairly well what 
it's about.

		Linus

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Pavel Machek]

On Tue 2008-03-18 10:31:28, Linus Torvalds wrote:
> 
> 
> On Tue, 18 Mar 2008, Pavel Machek wrote:
> >
> > On Tue 2008-03-18 14:06:42, Pavel Machek wrote:
> > > 
> > > Confirmed, suspend slowness is in 2.6.25-rc6, too. It takes 15 seconds
> > > to suspend/resume, while 2.6.24 takes 9. Thinkpad x60.
> > 
> > IIRC 2.6.25-rc5 was "fast".
> 
> Can you bisect it? There's only 343 commits between -rc5 and -rc6,
> so it 

It will take a while. My .git directory is 7G, and I'd like to do a
backup before playing with git.

git gc does not exist here, so I guess I need to update git, too :-(.

> should not take too long to check which commit it is. Even if it should 
> take 9 reboots to bisect it entirely, going just five or six will likely 
> narrow it down sufficiently that we can probably guess fairly well what 
> it's about.

Hmm, it gets weirder. 2.6.25-rc5 was fast, and had small problem with
backlight hotkeys did not work.

2.6.25-rc6 has backlight hotkeys somehow working, but closing/opening lid in X
kills the backlight, making machine unusable.

Hmm...

commit 7c0ea45be4f114d85ee35caeead8e1660699c46f
tree 0822ef23606a733e00bbf75d3e218b1e92abdd78
parent 2f44bbb495dd3e6d0209eff2257438ab9c570e5b
author Zhao Yakui <yakui.zhao@intel.com> Tue, 11 Mar 2008 16:56:47
+0800
committer Len Brown <len.brown@intel.com> Tue, 11 Mar 2008 22:20:19
-0400

    ACPI: Ignore _BQC object when registering backlight device

    According to acpi spec , the objects of  _BCL and _BCM are
required if
    integrated LCD is present and supports brightness level .The _BQC
is
    the optional object. So the _BQC object is ignored when the
backlight device
    is registered in ACPI video driver.

    http://bugzilla.kernel.org/show_bug.cgi?id=10206

    Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
    Signed-off-by: Zhang Rui  <rui.zhang@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>


when did this go in? Pretty recently, right? And that means that ACPI
backlight driver is now used on machines where it was not used
before...?
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les:  http://www.ujezdskystrom.info/

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Pavel Machek]

Hi!

> > should not take too long to check which commit it is. Even if it should 
> > take 9 reboots to bisect it entirely, going just five or six will likely 
> > narrow it down sufficiently that we can probably guess fairly well what 
> > it's about.
> 
> Hmm, it gets weirder. 2.6.25-rc5 was fast, and had small problem with
> backlight hotkeys did not work.
> 
> 2.6.25-rc6 has backlight hotkeys somehow working, but closing/opening lid in X
> kills the backlight, making machine unusable.
> 
> Hmm...
> 
> commit 7c0ea45be4f114d85ee35caeead8e1660699c46f
> tree 0822ef23606a733e00bbf75d3e218b1e92abdd78
> parent 2f44bbb495dd3e6d0209eff2257438ab9c570e5b
> author Zhao Yakui <yakui.zhao@intel.com> Tue, 11 Mar 2008 16:56:47
> +0800
> committer Len Brown <len.brown@intel.com> Tue, 11 Mar 2008 22:20:19
> -0400
> 
>     ACPI: Ignore _BQC object when registering backlight device
> 
>     According to acpi spec , the objects of  _BCL and _BCM are
> required if
>     integrated LCD is present and supports brightness level .The _BQC
> is
>     the optional object. So the _BQC object is ignored when the
> backlight device
>     is registered in ACPI video driver.
> 
>     http://bugzilla.kernel.org/show_bug.cgi?id=10206
> 
>     Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
>     Signed-off-by: Zhang Rui  <rui.zhang@intel.com>
>     Signed-off-by: Len Brown <len.brown@intel.com>
> 
> 
> when did this go in? Pretty recently, right? And that means that ACPI
> backlight driver is now used on machines where it was not used
> before...?

No, this commit was not responsible, but CONFIG_ACPI_VIDEO *is*
responsible for the "X has no backlight after lid close/open".
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les:  http://www.ujezdskystrom.info/

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Linus Torvalds]

On Tue, 18 Mar 2008, Pavel Machek wrote:
> 
> It will take a while. My .git directory is 7G, and I'd like to do a
> backup before playing with git.

Ouch, what have you done? It should be about 200MB, not 7GB.

> git gc does not exist here, so I guess I need to update git, too :-(.

Well, even with old git you can just do 

	git repack -a -d

and in your case you'll probably have to leave it running overnight, 
because it's going to take hours since you've clearly not ever repacked it 
before and as a result it's going to do lots and lots of IO (all that 7GB 
and then read much of it twice).

But yeah, upgrading git first is probably a good idea regardless.

> commit 7c0ea45be4f114d85ee35caeead8e1660699c46f
> 
>     ACPI: Ignore _BQC object when registering backlight device
> 
> when did this go in? Pretty recently, right?

Yes:

	[torvalds@woody linux]$ git describe 7c0ea45be4f114d85ee35caeead8e1660699c46f
	v2.6.25-rc5-89-g7c0ea45

so it happened after -rc5.

> And that means that ACPI backlight driver is now used on machines where 
> it was not used before...?

Somebody more into ACPI needs to judge how likely this is to be an issue..

		Linus

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Pavel Machek]

Hi!

> > It will take a while. My .git directory is 7G, and I'd like to do a
> > backup before playing with git.
> 
> Ouch, what have you done? It should be about 200MB, not 7GB.
> 
> > git gc does not exist here, so I guess I need to update git, too :-(.
> 
> Well, even with old git you can just do 
> 
> 	git repack -a -d
> 
> and in your case you'll probably have to leave it running overnight, 
> because it's going to take hours since you've clearly not ever repacked it 
> before and as a result it's going to do lots and lots of IO (all that 7GB 
> and then read much of it twice).

And 10 minutes later, .git directory is 200MB. Thanks!


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les:  http://www.ujezdskystrom.info/

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Linus Torvalds]

On Wed, 19 Mar 2008, Pavel Machek wrote:
>
> > > It will take a while. My .git directory is 7G, and I'd like to do a
> > > backup before playing with git.
> > 
> > Ouch, what have you done? It should be about 200MB, not 7GB.
> 
> And 10 minutes later, .git directory is 200MB. Thanks!

It strikes me that the only way I can think of that you could have gotten 
a 7GB .git directory without really working at it is if you use one of the 
so-called "dumb" git protocols that just copy whole packfiles from 
kernel.org when you pull.

So do you happen to perhaps use http:// or rsync:// when you fetch git 
data? That would not only be horribly slow occasionally (you'd fetch 
all-new packs and re-download about 200MB of data when I repack the kernel 
repo on kernel.org, which happens about once or twice every release 
cycle), but it would also explain how it ballooned to 7GB for you (because 
you have all these duplicate packs!).

Usign the native git:// protocol would be much faster and avoid the issue 
in the future.

		Linus

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Pavel Machek]

On Wed 2008-03-19 07:42:36, Linus Torvalds wrote:
> 
> 
> On Wed, 19 Mar 2008, Pavel Machek wrote:
> >
> > > > It will take a while. My .git directory is 7G, and I'd like to do a
> > > > backup before playing with git.
> > > 
> > > Ouch, what have you done? It should be about 200MB, not 7GB.
> > 
> > And 10 minutes later, .git directory is 200MB. Thanks!
> 
> It strikes me that the only way I can think of that you could have gotten 
> a 7GB .git directory without really working at it is if you use one of the 
> so-called "dumb" git protocols that just copy whole packfiles from 
> kernel.org when you pull.
> 
> So do you happen to perhaps use http:// or rsync:// when you fetch git 
> data? That would not only be horribly slow occasionally (you'd fetch 

I was using rsync... should be fixed now. Thanks!

> all-new packs and re-download about 200MB of data when I repack the kernel 
> repo on kernel.org, which happens about once or twice every release 
> cycle), but it would also explain how it ballooned to 7GB for you (because 
> you have all these duplicate packs!).

Well, I thought that it is pulling a bit too much, but I attributed
that to our fast development ;-).

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les:  http://www.ujezdskystrom.info/

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Pavel Machek]

Hi!

> > It will take a while. My .git directory is 7G, and I'd like to do a
> > backup before playing with git.
> 
> Ouch, what have you done? It should be about 200MB, not 7GB.

Nothing, just used git.

> > git gc does not exist here, so I guess I need to update git, too :-(.
> 
> Well, even with old git you can just do 
> 
> 	git repack -a -d
> 
> and in your case you'll probably have to leave it running overnight, 
> because it's going to take hours since you've clearly not ever repacked it 
> before and as a result it's going to do lots and lots of IO (all that 7GB 
> and then read much of it twice).

I actually _did_ repack before, and this repack was reasonably quick.

> > commit 7c0ea45be4f114d85ee35caeead8e1660699c46f
> > 
> >     ACPI: Ignore _BQC object when registering backlight device
> > 
> > when did this go in? Pretty recently, right?
> 
> Yes:
> 
> 	[torvalds@woody linux]$ git describe 7c0ea45be4f114d85ee35caeead8e1660699c46f
> 	v2.6.25-rc5-89-g7c0ea45
> 
> so it happened after -rc5.

Ok, so I confirmed that kernel-breaks-backlight-in-X-after-lid-reopen
is indeed regression between -rc5 and -rc6. Reverting drivers/acpi in
-rc6 fixes this regression.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Pavel Machek]

Hi!

> > commit 7c0ea45be4f114d85ee35caeead8e1660699c46f
> > 
> >     ACPI: Ignore _BQC object when registering backlight device
> > 
> > when did this go in? Pretty recently, right?
> 
> Yes:
> 
> 	[torvalds@woody linux]$ git describe 7c0ea45be4f114d85ee35caeead8e1660699c46f
> 	v2.6.25-rc5-89-g7c0ea45
> 
> so it happened after -rc5.
> 
> > And that means that ACPI backlight driver is now used on machines where 
> > it was not used before...?
> 
> Somebody more into ACPI needs to judge how likely this is to be an issue..

Ok, 7c0... is innocent. Reverting not only the mdelay, but also rest
of drivers/acpi/ec.c patch fixed the
"rc6-breaks-backlight-in-X-over-lid-close", good.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Linus Torvalds]

On Wed, 19 Mar 2008, Pavel Machek wrote:
> 
> Ok, 7c0... is innocent. Reverting not only the mdelay, but also rest
> of drivers/acpi/ec.c patch fixed the
> "rc6-breaks-backlight-in-X-over-lid-close", good.

Can you clarify a bit?

Do you mean that the full revert of 2c81ce4 that already got committed is 
sufficient for you, or do you need to totally undo everything we've done 
to ec.c since -rc5, and thus also to revert the _other_ revert we did (in 
4af8e10a6c57e7292862bd1703712f0565c7e429)?

In other words, what does "rest of drivers/acpi/ec.c patch" mean? Does it 
mean just the IRQ storm patch, or does it mean everything that happened in 
drivers/acpi/ec.c since 2.6.25-rc5?

			Linus

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Pavel Machek]

On Wed 2008-03-19 07:47:06, Linus Torvalds wrote:
> 
> 
> On Wed, 19 Mar 2008, Pavel Machek wrote:
> > 
> > Ok, 7c0... is innocent. Reverting not only the mdelay, but also rest
> > of drivers/acpi/ec.c patch fixed the
> > "rc6-breaks-backlight-in-X-over-lid-close", good.
> 
> Can you clarify a bit?
> 
> Do you mean that the full revert of 2c81ce4 that already got committed is 
> sufficient for you, or do you need to totally undo everything we've done 
> to ec.c since -rc5, and thus also to revert the _other_ revert we did (in 
> 4af8e10a6c57e7292862bd1703712f0565c7e429)?

I believe that current mainline is okay, and will test that theory
shortly.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les:  http://www.ujezdskystrom.info/

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Pavel Machek]

Hi!

> > Ok, 7c0... is innocent. Reverting not only the mdelay, but also rest
> > of drivers/acpi/ec.c patch fixed the
> > "rc6-breaks-backlight-in-X-over-lid-close", good.
> 
> Can you clarify a bit?
> 
> Do you mean that the full revert of 2c81ce4 that already got committed is 
> sufficient for you, or do you need to totally undo everything we've done 
> to ec.c since -rc5, and thus also to revert the _other_ revert we did (in 
> 4af8e10a6c57e7292862bd1703712f0565c7e429)?

I did some more testing, and realized I was wrong. We need to totally
undo everything we've done to ec.c since -rc5... (that has small
sideffect of brightness up/down keys no longer working -- regression
since 2.6.24, but saves us from "backlight gone after
closing/reopening lid" which is _NASTY_ regression from 2.6.25-rc5).

ACPI sucks.

I.e. this patch:


Revert ec.c to 2.6.25-rc5 state:

* this fixes backlight after closing/reopening the lid while in X on
thinkpad x60

* unfortunately it breaks fn-home/end keyboard brightness control

Signed-off-by: Pavel Machek <pavel@suse.cz>

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 7222a18..caf873c 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -943,7 +943,11 @@ int __init acpi_ec_ecdt_probe(void)
 		boot_ec->command_addr = ecdt_ptr->control.address;
 		boot_ec->data_addr = ecdt_ptr->data.address;
 		boot_ec->gpe = ecdt_ptr->gpe;
-		boot_ec->handle = ACPI_ROOT_OBJECT;
+		if (ACPI_FAILURE(acpi_get_handle(NULL, ecdt_ptr->id,
+				&boot_ec->handle))) {
+			pr_info("Failed to locate handle for boot EC\n");
+			boot_ec->handle = ACPI_ROOT_OBJECT;
+		}
 	} else {
 		/* This workaround is needed only on some broken machines,
 		 * which require early EC, but fail to provide ECDT */

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les:  http://www.ujezdskystrom.info/

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Alexey Starikovskiy]

Pavel Machek wrote:
> Hi!
> 
>>> Ok, 7c0... is innocent. Reverting not only the mdelay, but also rest
>>> of drivers/acpi/ec.c patch fixed the
>>> "rc6-breaks-backlight-in-X-over-lid-close", good.
>> Can you clarify a bit?
>>
>> Do you mean that the full revert of 2c81ce4 that already got committed is 
>> sufficient for you, or do you need to totally undo everything we've done 
>> to ec.c since -rc5, and thus also to revert the _other_ revert we did (in 
>> 4af8e10a6c57e7292862bd1703712f0565c7e429)?
> 
> I did some more testing, and realized I was wrong. We need to totally
> undo everything we've done to ec.c since -rc5... (that has small
> sideffect of brightness up/down keys no longer working -- regression
> since 2.6.24, but saves us from "backlight gone after
> closing/reopening lid" which is _NASTY_ regression from 2.6.25-rc5).
> 
> ACPI sucks.
> 
> I.e. this patch:
> 
> 
> Revert ec.c to 2.6.25-rc5 state:
> 
> * this fixes backlight after closing/reopening the lid while in X on
> thinkpad x60
> 
> * unfortunately it breaks fn-home/end keyboard brightness control
It should also break volume up/down/mute keys. Essentially all the keys on Thinkpad.
And it would also be regression to 2.6.24 level.

Regards,
Alex.

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Rafael J. Wysocki]

On Wednesday, 19 of March 2008, Alexey Starikovskiy wrote:
> Pavel Machek wrote:
> > Hi!
> > 
> >>> Ok, 7c0... is innocent. Reverting not only the mdelay, but also rest
> >>> of drivers/acpi/ec.c patch fixed the
> >>> "rc6-breaks-backlight-in-X-over-lid-close", good.
> >> Can you clarify a bit?
> >>
> >> Do you mean that the full revert of 2c81ce4 that already got committed is 
> >> sufficient for you, or do you need to totally undo everything we've done 
> >> to ec.c since -rc5, and thus also to revert the _other_ revert we did (in 
> >> 4af8e10a6c57e7292862bd1703712f0565c7e429)?
> > 
> > I did some more testing, and realized I was wrong. We need to totally
> > undo everything we've done to ec.c since -rc5... (that has small
> > sideffect of brightness up/down keys no longer working -- regression
> > since 2.6.24, but saves us from "backlight gone after
> > closing/reopening lid" which is _NASTY_ regression from 2.6.25-rc5).
> > 
> > ACPI sucks.
> > 
> > I.e. this patch:
> > 
> > 
> > Revert ec.c to 2.6.25-rc5 state:
> > 
> > * this fixes backlight after closing/reopening the lid while in X on
> > thinkpad x60
> > 
> > * unfortunately it breaks fn-home/end keyboard brightness control
> It should also break volume up/down/mute keys. Essentially all the keys on Thinkpad.
> And it would also be regression to 2.6.24 level.

Well, and what's the underlying issue?

Rafael

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Pavel Machek]

On Wed 2008-03-19 23:35:10, Alexey Starikovskiy wrote:
> Pavel Machek wrote:
>> Hi!
>>
>>>> Ok, 7c0... is innocent. Reverting not only the mdelay, but also rest
>>>> of drivers/acpi/ec.c patch fixed the
>>>> "rc6-breaks-backlight-in-X-over-lid-close", good.
>>> Can you clarify a bit?
>>>
>>> Do you mean that the full revert of 2c81ce4 that already got committed is 
>>> sufficient for you, or do you need to totally undo everything we've done 
>>> to ec.c since -rc5, and thus also to revert the _other_ revert we did (in 
>>> 4af8e10a6c57e7292862bd1703712f0565c7e429)?
>>
>> I did some more testing, and realized I was wrong. We need to totally
>> undo everything we've done to ec.c since -rc5... (that has small
>> sideffect of brightness up/down keys no longer working -- regression
>> since 2.6.24, but saves us from "backlight gone after
>> closing/reopening lid" which is _NASTY_ regression from 2.6.25-rc5).
>>
>> ACPI sucks.
>>
>> I.e. this patch:
>>
>>
>> Revert ec.c to 2.6.25-rc5 state:
>>
>> * this fixes backlight after closing/reopening the lid while in X on
>> thinkpad x60
>>
>> * unfortunately it breaks fn-home/end keyboard brightness control
> It should also break volume up/down/mute keys. Essentially all the keys on Thinkpad.
> And it would also be regression to 2.6.24 level.

As I explained above.

Unfortunately, applying this means I loose screen as soon as I
close/reopen lid. 2.6.24 did not behave like that. And it makes my
machine mostly unusable -- as soon as you try to carry it away, you
loose video. BAD.
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les:  http://www.ujezdskystrom.info/

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Alexey Starikovskiy]

Pavel Machek wrote:
> On Wed 2008-03-19 23:35:10, Alexey Starikovskiy wrote:
>> Pavel Machek wrote:
>>> Hi!
>>>
>>>>> Ok, 7c0... is innocent. Reverting not only the mdelay, but also rest
>>>>> of drivers/acpi/ec.c patch fixed the
>>>>> "rc6-breaks-backlight-in-X-over-lid-close", good.
>>>> Can you clarify a bit?
>>>>
>>>> Do you mean that the full revert of 2c81ce4 that already got committed is 
>>>> sufficient for you, or do you need to totally undo everything we've done 
>>>> to ec.c since -rc5, and thus also to revert the _other_ revert we did (in 
>>>> 4af8e10a6c57e7292862bd1703712f0565c7e429)?
>>> I did some more testing, and realized I was wrong. We need to totally
>>> undo everything we've done to ec.c since -rc5... (that has small
>>> sideffect of brightness up/down keys no longer working -- regression
>>> since 2.6.24, but saves us from "backlight gone after
>>> closing/reopening lid" which is _NASTY_ regression from 2.6.25-rc5).
>>>
>>> ACPI sucks.
>>>
>>> I.e. this patch:
>>>
>>>
>>> Revert ec.c to 2.6.25-rc5 state:
>>>
>>> * this fixes backlight after closing/reopening the lid while in X on
>>> thinkpad x60
>>>
>>> * unfortunately it breaks fn-home/end keyboard brightness control
>> It should also break volume up/down/mute keys. Essentially all the keys on Thinkpad.
>> And it would also be regression to 2.6.24 level.
> 
> As I explained above.
> 
> Unfortunately, applying this means I loose screen as soon as I
> close/reopen lid. 2.6.24 did not behave like that. And it makes my
> machine mostly unusable -- as soon as you try to carry it away, you
> loose video. BAD.
> 								Pavel
> 
Pavel,
You are reverting a revert of the patch, which was introduced _after_ 2.6.24. 
Thus, you regression to 2.6.24 should lay somewhere else.

Regards,
Alex.

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Rafael J. Wysocki]

On Wednesday, 19 of March 2008, Pavel Machek wrote:
> On Wed 2008-03-19 23:35:10, Alexey Starikovskiy wrote:
> > Pavel Machek wrote:
> >> Hi!
> >>
> >>>> Ok, 7c0... is innocent. Reverting not only the mdelay, but also rest
> >>>> of drivers/acpi/ec.c patch fixed the
> >>>> "rc6-breaks-backlight-in-X-over-lid-close", good.
> >>> Can you clarify a bit?
> >>>
> >>> Do you mean that the full revert of 2c81ce4 that already got committed is 
> >>> sufficient for you, or do you need to totally undo everything we've done 
> >>> to ec.c since -rc5, and thus also to revert the _other_ revert we did (in 
> >>> 4af8e10a6c57e7292862bd1703712f0565c7e429)?
> >>
> >> I did some more testing, and realized I was wrong. We need to totally
> >> undo everything we've done to ec.c since -rc5... (that has small
> >> sideffect of brightness up/down keys no longer working -- regression
> >> since 2.6.24, but saves us from "backlight gone after
> >> closing/reopening lid" which is _NASTY_ regression from 2.6.25-rc5).
> >>
> >> ACPI sucks.
> >>
> >> I.e. this patch:
> >>
> >>
> >> Revert ec.c to 2.6.25-rc5 state:
> >>
> >> * this fixes backlight after closing/reopening the lid while in X on
> >> thinkpad x60
> >>
> >> * unfortunately it breaks fn-home/end keyboard brightness control
> > It should also break volume up/down/mute keys. Essentially all the keys on Thinkpad.
> > And it would also be regression to 2.6.24 level.
> 
> As I explained above.
> 
> Unfortunately, applying this means I loose screen as soon as I
> close/reopen lid. 2.6.24 did not behave like that. And it makes my
> machine mostly unusable -- as soon as you try to carry it away, you
> loose video. BAD.

Pavel,

Can you text the current -git (preferably without anything on top) pretty please?

Thanks,
Rafael

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Rafael J. Wysocki]

On Wednesday, 19 of March 2008, Rafael J. Wysocki wrote:
> On Wednesday, 19 of March 2008, Pavel Machek wrote:
> > On Wed 2008-03-19 23:35:10, Alexey Starikovskiy wrote:
> > > Pavel Machek wrote:
> > >> Hi!
> > >>
> > >>>> Ok, 7c0... is innocent. Reverting not only the mdelay, but also rest
> > >>>> of drivers/acpi/ec.c patch fixed the
> > >>>> "rc6-breaks-backlight-in-X-over-lid-close", good.
> > >>> Can you clarify a bit?
> > >>>
> > >>> Do you mean that the full revert of 2c81ce4 that already got committed is 
> > >>> sufficient for you, or do you need to totally undo everything we've done 
> > >>> to ec.c since -rc5, and thus also to revert the _other_ revert we did (in 
> > >>> 4af8e10a6c57e7292862bd1703712f0565c7e429)?
> > >>
> > >> I did some more testing, and realized I was wrong. We need to totally
> > >> undo everything we've done to ec.c since -rc5... (that has small
> > >> sideffect of brightness up/down keys no longer working -- regression
> > >> since 2.6.24, but saves us from "backlight gone after
> > >> closing/reopening lid" which is _NASTY_ regression from 2.6.25-rc5).
> > >>
> > >> ACPI sucks.
> > >>
> > >> I.e. this patch:
> > >>
> > >>
> > >> Revert ec.c to 2.6.25-rc5 state:
> > >>
> > >> * this fixes backlight after closing/reopening the lid while in X on
> > >> thinkpad x60
> > >>
> > >> * unfortunately it breaks fn-home/end keyboard brightness control
> > > It should also break volume up/down/mute keys. Essentially all the keys on Thinkpad.
> > > And it would also be regression to 2.6.24 level.
> > 
> > As I explained above.
> > 
> > Unfortunately, applying this means I loose screen as soon as I
> > close/reopen lid. 2.6.24 did not behave like that. And it makes my
> > machine mostly unusable -- as soon as you try to carry it away, you
> > loose video. BAD.
> 
> Pavel,
> 
> Can you text the current -git (preferably without anything on top) pretty please?

s/text/test/  (sorry)

Rafael

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Pavel Machek]

> > >> Revert ec.c to 2.6.25-rc5 state:
> > >>
> > >> * this fixes backlight after closing/reopening the lid while in X on
> > >> thinkpad x60
> > >>
> > >> * unfortunately it breaks fn-home/end keyboard brightness control
> > > It should also break volume up/down/mute keys. Essentially all the keys on Thinkpad.
> > > And it would also be regression to 2.6.24 level.
> > 
> > As I explained above.
> > 
> > Unfortunately, applying this means I loose screen as soon as I
> > close/reopen lid. 2.6.24 did not behave like that. And it makes my
> > machine mostly unusable -- as soon as you try to carry it away, you
> > loose video. BAD.
> 
> Can you text the current -git (preferably without anything on top) pretty please?

Did anything relevant change in the last 8 hours or so? My scripts
tell me there's nothing to pull:

Done counting 441 objects.
Result has 304 objects.
Deltifying 304 objects.
 100% (304/304) done
Total 304, written 304 (delta 236), reused 121 (delta 75)
Fetching pack (head and objects)...
Fetching tags...
Up to date.

Applying changes...
Branch already fully merged.
pavel@amd:~$ 

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les:  http://www.ujezdskystrom.info/

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Rafael J. Wysocki]

On Wednesday, 19 of March 2008, Pavel Machek wrote:
> 
> > > >> Revert ec.c to 2.6.25-rc5 state:
> > > >>
> > > >> * this fixes backlight after closing/reopening the lid while in X on
> > > >> thinkpad x60
> > > >>
> > > >> * unfortunately it breaks fn-home/end keyboard brightness control
> > > > It should also break volume up/down/mute keys. Essentially all the keys on Thinkpad.
> > > > And it would also be regression to 2.6.24 level.
> > > 
> > > As I explained above.
> > > 
> > > Unfortunately, applying this means I loose screen as soon as I
> > > close/reopen lid. 2.6.24 did not behave like that. And it makes my
> > > machine mostly unusable -- as soon as you try to carry it away, you
> > > loose video. BAD.
> > 
> > Can you text the current -git (preferably without anything on top) pretty please?
> 
> Did anything relevant change in the last 8 hours or so?

No, it didn't.

> My scripts tell me there's nothing to pull:
> 
> Done counting 441 objects.
> Result has 304 objects.
> Deltifying 304 objects.
>  100% (304/304) done
> Total 304, written 304 (delta 236), reused 121 (delta 75)
> Fetching pack (head and objects)...
> Fetching tags...
> Up to date.
> 
> Applying changes...
> Branch already fully merged.
> pavel@amd:~$

Still, the current -git has the offending patch reverted and you're seeing a
regression nevertheless.

Is that correct?

Rafael

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Pavel Machek]

On Wed 2008-03-19 22:58:59, Rafael J. Wysocki wrote:
> On Wednesday, 19 of March 2008, Pavel Machek wrote:
> > 
> > > > >> Revert ec.c to 2.6.25-rc5 state:
> > > > >>
> > > > >> * this fixes backlight after closing/reopening the lid while in X on
> > > > >> thinkpad x60
> > > > >>
> > > > >> * unfortunately it breaks fn-home/end keyboard brightness control
> > > > > It should also break volume up/down/mute keys. Essentially all the keys on Thinkpad.
> > > > > And it would also be regression to 2.6.24 level.
> > > > 
> > > > As I explained above.
> > > > 
> > > > Unfortunately, applying this means I loose screen as soon as I
> > > > close/reopen lid. 2.6.24 did not behave like that. And it makes my
> > > > machine mostly unusable -- as soon as you try to carry it away, you
> > > > loose video. BAD.
> > > 
> > > Can you text the current -git (preferably without anything on top) pretty please?
> > 
> > Did anything relevant change in the last 8 hours or so?
> 
> No, it didn't.
> 
> > My scripts tell me there's nothing to pull:
> > 
> > Done counting 441 objects.
> > Result has 304 objects.
> > Deltifying 304 objects.
> >  100% (304/304) done
> > Total 304, written 304 (delta 236), reused 121 (delta 75)
> > Fetching pack (head and objects)...
> > Fetching tags...
> > Up to date.
> > 
> > Applying changes...
> > Branch already fully merged.
> > pavel@amd:~$
> 
> Still, the current -git has the offending patch reverted and you're seeing a
> regression nevertheless.
> 
> Is that correct?

Yes.

(Or more precisely, there are two regressions:

* s2ram 5 seconds slower -- fixed by current git

* backlight goes out in X -- not fixed by current git)

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les:  http://www.ujezdskystrom.info/

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Rafael J. Wysocki]

On Wednesday, 19 of March 2008, Pavel Machek wrote:
> On Wed 2008-03-19 22:58:59, Rafael J. Wysocki wrote:
> > On Wednesday, 19 of March 2008, Pavel Machek wrote:
> > > 
> > > > > >> Revert ec.c to 2.6.25-rc5 state:
> > > > > >>
> > > > > >> * this fixes backlight after closing/reopening the lid while in X on
> > > > > >> thinkpad x60
> > > > > >>
> > > > > >> * unfortunately it breaks fn-home/end keyboard brightness control
> > > > > > It should also break volume up/down/mute keys. Essentially all the keys on Thinkpad.
> > > > > > And it would also be regression to 2.6.24 level.
> > > > > 
> > > > > As I explained above.
> > > > > 
> > > > > Unfortunately, applying this means I loose screen as soon as I
> > > > > close/reopen lid. 2.6.24 did not behave like that. And it makes my
> > > > > machine mostly unusable -- as soon as you try to carry it away, you
> > > > > loose video. BAD.
> > > > 
> > > > Can you text the current -git (preferably without anything on top) pretty please?
> > > 
> > > Did anything relevant change in the last 8 hours or so?
> > 
> > No, it didn't.
> > 
> > > My scripts tell me there's nothing to pull:
> > > 
> > > Done counting 441 objects.
> > > Result has 304 objects.
> > > Deltifying 304 objects.
> > >  100% (304/304) done
> > > Total 304, written 304 (delta 236), reused 121 (delta 75)
> > > Fetching pack (head and objects)...
> > > Fetching tags...
> > > Up to date.
> > > 
> > > Applying changes...
> > > Branch already fully merged.
> > > pavel@amd:~$
> > 
> > Still, the current -git has the offending patch reverted and you're seeing a
> > regression nevertheless.
> > 
> > Is that correct?
> 
> Yes.
> 
> (Or more precisely, there are two regressions:
> 
> * s2ram 5 seconds slower -- fixed by current git

OK

> * backlight goes out in X -- not fixed by current git)

Can you please open a bugzilla entry for this one and make it block
bug #9832?  [That will make the tracking of it a bit easier.]

Thanks,
Rafael

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Pavel Machek]

Hi!

> > * backlight goes out in X -- not fixed by current git)
> 
> Can you please open a bugzilla entry for this one and make it block
> bug #9832?  [That will make the tracking of it a bit easier.]

Bug 10285 has been added to the database

								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les:  http://www.ujezdskystrom.info/

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Henrique de Moraes Holschuh]

On Tue, 18 Mar 2008, Pavel Machek wrote:
>     ACPI: Ignore _BQC object when registering backlight device

Thinkpads always have them all, or none at all.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Pavel Machek]

On Thu 2008-03-20 12:06:37, Henrique de Moraes Holschuh wrote:
> On Tue, 18 Mar 2008, Pavel Machek wrote:
> >     ACPI: Ignore _BQC object when registering backlight device
> 
> Thinkpads always have them all, or none at all.

Yep, this was not issue after all.

I tried to do bisect, but it looks like the X-and-lid-close/open  problem is present in
2.6.24, too. I'll try to do some meaningfull bisect tonight.
							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Pavel Machek]

On Tue 2008-03-18 10:31:28, Linus Torvalds wrote:
> 
> 
> On Tue, 18 Mar 2008, Pavel Machek wrote:
> >
> > On Tue 2008-03-18 14:06:42, Pavel Machek wrote:
> > > 
> > > Confirmed, suspend slowness is in 2.6.25-rc6, too. It takes 15 seconds
> > > to suspend/resume, while 2.6.24 takes 9. Thinkpad x60.
> > 
> > IIRC 2.6.25-rc5 was "fast".
> 
> Can you bisect it? There's only 343 commits between -rc5 and -rc6, so it 
> should not take too long to check which commit it is. Even if it should 
> take 9 reboots to bisect it entirely, going just five or six will likely 
> narrow it down sufficiently that we can probably guess fairly well what 
> it's about.

Slowdown is fixed by this patch:


diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index e7e197e..caf873c 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -129,7 +129,6 @@ static struct acpi_ec {
 	struct mutex lock;
 	wait_queue_head_t wait;
 	struct list_head list;
-	atomic_t irq_count;
 	u8 handlers_installed;
 } *boot_ec, *first_ec;
 
@@ -182,8 +181,6 @@ static int acpi_ec_wait(struct acpi_ec *
 {
 	int ret = 0;
 
-	atomic_set(&ec->irq_count, 0);
-
 	if (unlikely(event == ACPI_EC_EVENT_OBF_1 &&
 		     test_bit(EC_FLAGS_NO_OBF1_GPE, &ec->flags)))
 		force_poll = 1;
@@ -230,7 +227,6 @@ static int acpi_ec_wait(struct acpi_ec *
 		while (time_before(jiffies, delay)) {
 			if (acpi_ec_check_status(ec, event))
 				goto end;
-			msleep(5);
 		}
 	}
 	pr_err(PREFIX "acpi_ec_wait timeout,"
@@ -533,13 +529,6 @@ static u32 acpi_ec_gpe_handler(void *dat
 	struct acpi_ec *ec = data;
 
 	pr_debug(PREFIX "~~~> interrupt\n");
-	atomic_inc(&ec->irq_count);
-	if (atomic_read(&ec->irq_count) > 5) {
-		pr_err(PREFIX "GPE storm detected, disabling EC GPE\n");
-		acpi_disable_gpe(NULL, ec->gpe, ACPI_ISR);
-		clear_bit(EC_FLAGS_GPE_MODE, &ec->flags);
-		return ACPI_INTERRUPT_HANDLED;
-	}
 	clear_bit(EC_FLAGS_WAIT_GPE, &ec->flags);
 	if (test_bit(EC_FLAGS_GPE_MODE, &ec->flags))
 		wake_up(&ec->wait);
@@ -954,7 +943,11 @@ int __init acpi_ec_ecdt_probe(void)
 		boot_ec->command_addr = ecdt_ptr->control.address;
 		boot_ec->data_addr = ecdt_ptr->data.address;
 		boot_ec->gpe = ecdt_ptr->gpe;
-		boot_ec->handle = ACPI_ROOT_OBJECT;
+		if (ACPI_FAILURE(acpi_get_handle(NULL, ecdt_ptr->id,
+				&boot_ec->handle))) {
+			pr_info("Failed to locate handle for boot EC\n");
+			boot_ec->handle = ACPI_ROOT_OBJECT;
+		}
 	} else {
 		/* This workaround is needed only on some broken machines,
 		 * which require early EC, but fail to provide ECDT */

...so it seems to be "hello, Alexey!"

commit 2c81ce4c9c37b910210f2640c28e98a0c398dc26
tree e46ccf30836014d115e72791263697585f7943f6
parent 2f44bbb495dd3e6d0209eff2257438ab9c570e5b
author Alexey Starikovskiy <astarikovskiy@suse.de> Tue, 11 Mar 2008
13:30:00 -0400
committer Len Brown <len.brown@intel.com> Tue, 11 Mar 2008 13:30:00
-0400

    ACPI: EC: Handle IRQ storm on Acer laptops

    On some Acer systems, the HW fails to clear the GPE source,
    causing an interrupt storm.

    So in EC interrupt mode, we count how many interrupts we
    receive when waiting.  If we get more than 5, we give
    up on interrupt mode and revert to polling mode.

    Also, for polling mode to work on Acers, we need
    to insert a delay.

    Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
    Signed-off-by: Len Brown <len.brown@intel.com>



								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les:  http://www.ujezdskystrom.info/

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Alexey Starikovskiy]

[-- Attachment #1: Type: text/plain, Size: 1050 bytes --]

Pavel Machek wrote:
> On Tue 2008-03-18 10:31:28, Linus Torvalds wrote:
>>
>> On Tue, 18 Mar 2008, Pavel Machek wrote:
>>> On Tue 2008-03-18 14:06:42, Pavel Machek wrote:
>>>> Confirmed, suspend slowness is in 2.6.25-rc6, too. It takes 15 seconds
>>>> to suspend/resume, while 2.6.24 takes 9. Thinkpad x60.
>>> IIRC 2.6.25-rc5 was "fast".
>> Can you bisect it? There's only 343 commits between -rc5 and -rc6, so it 
>> should not take too long to check which commit it is. Even if it should 
>> take 9 reboots to bisect it entirely, going just five or six will likely 
>> narrow it down sufficiently that we can probably guess fairly well what 
>> it's about.
> 
> Slowdown is fixed by this patch:
I think only this part is relevant here. But yes, I agree that the whole 
patch should be reverted.
> @@ -230,7 +227,6 @@ static int acpi_ec_wait(struct acpi_ec *
>  		while (time_before(jiffies, delay)) {
>  			if (acpi_ec_check_status(ec, event))
>  				goto end;
> -			msleep(5);
>  		}
>  	}
>  	pr_err(PREFIX "acpi_ec_wait timeout,"
Thanks,
Alex.

[-- Attachment #2: x-rev.patch --]
[-- Type: text/x-patch, Size: 1674 bytes --]

ACPI: EC: Revert storm handler patch

From: Alexey Starikovskiy <astarikovskiy@suse.de>

Patch have caused several new troubles, so just revert it for now.

Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
---

 drivers/acpi/ec.c |   11 -----------
 1 files changed, 0 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index e7e197e..7222a18 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -129,7 +129,6 @@ static struct acpi_ec {
 	struct mutex lock;
 	wait_queue_head_t wait;
 	struct list_head list;
-	atomic_t irq_count;
 	u8 handlers_installed;
 } *boot_ec, *first_ec;
 
@@ -182,8 +181,6 @@ static int acpi_ec_wait(struct acpi_ec *ec, enum ec_event event, int force_poll)
 {
 	int ret = 0;
 
-	atomic_set(&ec->irq_count, 0);
-
 	if (unlikely(event == ACPI_EC_EVENT_OBF_1 &&
 		     test_bit(EC_FLAGS_NO_OBF1_GPE, &ec->flags)))
 		force_poll = 1;
@@ -230,7 +227,6 @@ static int acpi_ec_wait(struct acpi_ec *ec, enum ec_event event, int force_poll)
 		while (time_before(jiffies, delay)) {
 			if (acpi_ec_check_status(ec, event))
 				goto end;
-			msleep(5);
 		}
 	}
 	pr_err(PREFIX "acpi_ec_wait timeout,"
@@ -533,13 +529,6 @@ static u32 acpi_ec_gpe_handler(void *data)
 	struct acpi_ec *ec = data;
 
 	pr_debug(PREFIX "~~~> interrupt\n");
-	atomic_inc(&ec->irq_count);
-	if (atomic_read(&ec->irq_count) > 5) {
-		pr_err(PREFIX "GPE storm detected, disabling EC GPE\n");
-		acpi_disable_gpe(NULL, ec->gpe, ACPI_ISR);
-		clear_bit(EC_FLAGS_GPE_MODE, &ec->flags);
-		return ACPI_INTERRUPT_HANDLED;
-	}
 	clear_bit(EC_FLAGS_WAIT_GPE, &ec->flags);
 	if (test_bit(EC_FLAGS_GPE_MODE, &ec->flags))
 		wake_up(&ec->wait);

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Pavel Machek]

> Pavel Machek wrote:
> >On Tue 2008-03-18 10:31:28, Linus Torvalds wrote:
> >>
> >>On Tue, 18 Mar 2008, Pavel Machek wrote:
> >>>On Tue 2008-03-18 14:06:42, Pavel Machek wrote:
> >>>>Confirmed, suspend slowness is in 2.6.25-rc6, too. It takes 15 seconds
> >>>>to suspend/resume, while 2.6.24 takes 9. Thinkpad x60.
> >>>IIRC 2.6.25-rc5 was "fast".
> >>Can you bisect it? There's only 343 commits between -rc5 and -rc6, so it 
> >>should not take too long to check which commit it is. Even if it should 
> >>take 9 reboots to bisect it entirely, going just five or six will likely 
> >>narrow it down sufficiently that we can probably guess fairly well what 
> >>it's about.
> >
> >Slowdown is fixed by this patch:
> I think only this part is relevant here. But yes, I agree that the whole 
> patch should be reverted.
> >@@ -230,7 +227,6 @@ static int acpi_ec_wait(struct acpi_ec *
> > 		while (time_before(jiffies, delay)) {
> > 			if (acpi_ec_check_status(ec, event))
> > 				goto end;
> >-			msleep(5);
> > 		}
> > 	}
> > 	pr_err(PREFIX "acpi_ec_wait timeout,"
> Thanks,
> Alex.

> ACPI: EC: Revert storm handler patch
> 
> From: Alexey Starikovskiy <astarikovskiy@suse.de>
> 
> Patch have caused several new troubles, so just revert it for now.
> 
> Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>

Tested-by: Pavel Machek <pavel@suse.cz>
ACK.
									Pavel


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Henrique de Moraes Holschuh]

On Wed, 19 Mar 2008, Pavel Machek wrote:
>     Also, for polling mode to work on Acers, we need
>     to insert a delay.

Can we, in the future, always trigger any such performance damaging "fixups"
based on DMI white/black lists?  As a rule?

This is not the first time I see a vendor push broken crap, and everyone
else who did it right get the shaft, because people don't like to add
quirks to common code.  We get performance enhancement features disabled,
mdelays added...

And for the do-as-windows-does crowd, they do it on vendor­issued device
drivers, which obviously don't hork everyone else's devices.  Our equivalent
solution is to key things to DMI matches.   This is worth keeping in mind,
because the ACPI subsystem seems to be a common target for such bad
behaviour.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Alexey Starikovskiy]

Henrique de Moraes Holschuh wrote:
> On Wed, 19 Mar 2008, Pavel Machek wrote:
>   
>>     Also, for polling mode to work on Acers, we need
>>     to insert a delay.
>>     
>
> Can we, in the future, always trigger any such performance damaging "fixups"
> based on DMI white/black lists?  As a rule?
>   
Poll mode is not supposed to be entered on any non-broken HW.
The fact that it happens now with Thinkpads at suspend is a bug.
EC region should not be accessed with interrupts of GPEs disabled.
> This is not the first time I see a vendor push broken crap, and everyone
> else who did it right get the shaft, because people don't like to add
> quirks to common code.  We get performance enhancement features disabled,
> mdelays added...
>
> And for the do-as-windows-does crowd, they do it on vendor­issued device
> drivers, which obviously don't hork everyone else's devices.  Our equivalent
> solution is to key things to DMI matches.   This is worth keeping in mind,
> because the ACPI subsystem seems to be a common target for such bad
> behaviour.
>
>   

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Henrique de Moraes Holschuh]

On Thu, 20 Mar 2008, Alexey Starikovskiy wrote:
> Henrique de Moraes Holschuh wrote:
>> On Wed, 19 Mar 2008, Pavel Machek wrote:
>>   
>>>     Also, for polling mode to work on Acers, we need
>>>     to insert a delay.
>>
>> Can we, in the future, always trigger any such performance damaging "fixups"
>> based on DMI white/black lists?  As a rule?
>>   
> Poll mode is not supposed to be entered on any non-broken HW.

We have seen just how much we can trust that, haven't we?

The truth is that even secondary last-resort paths like this need to work
well, and as fast as possible.  As things stand, we DO excercise them a lot
more than we'd expect...

> The fact that it happens now with Thinkpads at suspend is a bug.

Sure, and one we should fix.  That doesn't change the fact that any broken
HW (or one hit by a kernel bug) that is not as broken as some Acers don't
need to take that 5ms hit on every EC transaction, in an EC mode that is
already way too slow without extra help.

> EC region should not be accessed with interrupts of GPEs disabled.

Looks like something we should tack a WARN_ONCE to, at least when in a debug
mode compile?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

[Pavel Machek]

On Tue 2008-03-18 10:31:28, Linus Torvalds wrote:
> 
> 
> On Tue, 18 Mar 2008, Pavel Machek wrote:
> >
> > On Tue 2008-03-18 14:06:42, Pavel Machek wrote:
> > > 
> > > Confirmed, suspend slowness is in 2.6.25-rc6, too. It takes 15 seconds
> > > to suspend/resume, while 2.6.24 takes 9. Thinkpad x60.
> > 
> > IIRC 2.6.25-rc5 was "fast".
> 
> Can you bisect it? There's only 343 commits between -rc5 and -rc6, so it 
> should not take too long to check which commit it is. Even if it should 
> take 9 reboots to bisect it entirely, going just five or six will likely 
> narrow it down sufficiently that we can probably guess fairly well what 
> it's about.

Minimal fix for slowness problem is:

Fix suspend slowness problem, introduced by:

| commit 2c81ce4c9c37b910210f2640c28e98a0c398dc26
| tree e46ccf30836014d115e72791263697585f7943f6
| parent 2f44bbb495dd3e6d0209eff2257438ab9c570e5b
| author Alexey Starikovskiy <astarikovskiy@suse.de> Tue, 11 Mar 2008
| 13:30:00 -0400
| committer Len Brown <len.brown@intel.com> Tue, 11 Mar 2008 13:30:00
| -0400
|
|    ACPI: EC: Handle IRQ storm on Acer laptops

Signed-off-by: Pavel Machek <pavel@suse.cz>

---
commit 2fc3b3069659f4d914cd6863f606c5d01d7d1644
tree 39e9fded0e00ba04947c24073873b5c7ff55af52
parent e094d39e372df9d3e360f2c2672f2860c0995f6a
author Pavel <pavel@amd.ucw.cz> Wed, 19 Mar 2008 00:43:14 +0100
committer Pavel <pavel@amd.ucw.cz> Wed, 19 Mar 2008 00:43:14 +0100

 drivers/acpi/ec.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index e7e197e..360adc0 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -230,7 +230,6 @@ static int acpi_ec_wait(struct acpi_ec *
 		while (time_before(jiffies, delay)) {
 			if (acpi_ec_check_status(ec, event))
 				goto end;
-			msleep(5);
 		}
 	}
 	pr_err(PREFIX "acpi_ec_wait timeout,"


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les:  http://www.ujezdskystrom.info/

Suspend and hibernation patchset against -rc7

[Rafael J. Wysocki]

On Monday, 17 of March 2008, Rafael J. Wysocki wrote:
> On Wednesday, 12 of March 2008, Rafael J. Wysocki wrote:
> >
> > For everyone interested, I have put together a patchset containing some "fresh"
> > patches related to suspend and hibernation, on top of 2.6.25-rc5, located at:
> > 
> > http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.25-rc5/patches/
> 
> There's a new patchset against 2.6.25-rc6 at:
> 
> http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.25-rc6/patches/

There's a new patchset against 2.6.25-rc7 at:

http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.25-rc7/patches/

It contains some more patches than the previous one.  Most importantly,
there are pm-remove-destroy_suspended_device.patch (from me) and
ACPI-force-acpi_new_pts_ordering-for-ASUS-A6VC-laptop.patch (from Shaohua) in
it.

I still have not decided what to do with patches 01-03.  Patch 03 will probably
be sent upstream soon, as well as patches 12 and 14.

Patch 04 is currently in the "test" branch of the ACPI tree and has been
tested already for some time in -mm and linux-next.

Patches 05-11 are in the Greg's tree and have been tested in -mm and
linux-next.

Patch 13 is in -mm.

Patches 15-17 are new, but they don't crash my box, so hopefully they're not
broken. ;-)
 
Everything has been compiled and (lightly) tested on HP nx6325 (64-bit).

Thanks,
Rafael

Suspend and hibernation patchset against -rc8

[Rafael J. Wysocki]

On Thursday, 27 of March 2008, Rafael J. Wysocki wrote:
> On Monday, 17 of March 2008, Rafael J. Wysocki wrote:
> > On Wednesday, 12 of March 2008, Rafael J. Wysocki wrote:
> > >
> > > For everyone interested, I have put together a patchset containing some "fresh"
> > > patches related to suspend and hibernation, on top of 2.6.25-rc5, located at:
> > > 
> > > http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.25-rc5/patches/
> > 
> > There's a new patchset against 2.6.25-rc6 at:
> > 
> > http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.25-rc6/patches/
> 
> There's a new patchset against 2.6.25-rc7 at:
> 
> http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.25-rc7/patches/

There's a new patchset against 2.6.25-rc8 at:
 
http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.25-rc8/patches/

It contains some changes with respect to the previous one.  Most importantly,
dropped ACPI-force-acpi_new_pts_ordering-for-ASUS-A6VC-laptop.patch (not needed
any more) and added patch 14, which is experimental.

I still have not decided what to do with patches 01-03.  Patch 03 will probably
be sent upstream soon, as well as patches 12 and 14.
 
Patch 04 is currently in the "test" branch of the ACPI tree and has been
tested already for some time in -mm and linux-next.
 
Patches 05-11 are in the Greg's tree and have been tested in -mm and
linux-next.
 
Patch 13 is in -mm.

Patches 15-17 are the 4th revision of the patches to introduce new suspend and
hibernation callbacks for drivers.

The series has been compiled and (lightly) tested on HP nx6325 (64-bit).

Thanks,
Rafael

Suspend and hibernation patchset against -rc9

[Rafael J. Wysocki]

Hi,

For everyone interested, there's a new suspend/hibernation patchset against
2.6.25-rc8 at:
  
http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.25-rc9/patches/
 
It contains one new patch picked up from -mm,
gregkh-driver-power_state-remove-it-from-driver-core.patch , that patch 16 is
based on.

I still have not decided what to do with patches 01-03.  Patch 03 will probably 
be sent upstream.

A new version of patch 04 has recently been included into the x86 git tree and
before it had been tested in -mm and linux-next for quite some time.

Patches 05-11 and 13 are in the Greg's tree and have been tested in -mm and
linux-next.
  
Patch 12 is in the ACPI test tree and has been tested in -mm and linux-next.

Patches 16-18 are the 5th revision of the patchset introducing new suspend and
hibernation callbacks for drivers.  They are in -mm and have just been resent to
Greg.

Thanks,
Rafael