2008-03-12 00:30:33

by Rafael J. Wysocki

[permalink] [raw]
Subject: Suspend and hibernation patchset

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/

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.

Patches 05-08, being mainly fixes, have just been sent upstream.

Patch 09, which I'm considering as the most important one, is the first of
a series that will introduce a new suspend and hibernation framework for
device drivers. It introduces the highest-level structures and documents
them, so if you are planning any modifications in this area, have a look at it
and please let me know if there are any conflicts, so that we can avoid them.

Thanks,
Rafael


--
"Premature optimization is the root of all evil." - Donald Knuth


2008-03-17 22:18:38

by Rafael J. Wysocki

[permalink] [raw]
Subject: Suspend and hibernation patchset against -rc6

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

2008-03-18 13:31:45

by Pavel Machek

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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/

2008-03-18 13:32:04

by Pavel Machek

[permalink] [raw]
Subject: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)


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/

2008-03-18 13:32:21

by Pavel Machek

[permalink] [raw]
Subject: Re: Suspend and hibernation patchset against -rc6

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/

2008-03-19 19:26:15

by Linus Torvalds

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)



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

2008-03-19 19:28:33

by Linus Torvalds

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)



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

2008-03-19 19:49:29

by Linus Torvalds

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)



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

2008-03-19 20:46:55

by Linus Torvalds

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)



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

2008-03-19 21:02:16

by Pavel Machek

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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

2008-03-19 21:11:46

by Pavel Machek

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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/

2008-03-19 21:12:59

by Pavel Machek

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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

2008-03-19 21:13:52

by Pavel Machek

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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 <[email protected]> Tue, 11 Mar 2008 16:56:47
> +0800
> committer Len Brown <[email protected]> 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 <[email protected]>
> Signed-off-by: Zhang Rui <[email protected]>
> Signed-off-by: Len Brown <[email protected]>
>
>
> 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/

2008-03-19 21:15:31

by Pavel Machek

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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 <[email protected]> Tue, 11 Mar 2008
| 13:30:00 -0400
| committer Len Brown <[email protected]> Tue, 11 Mar 2008 13:30:00
| -0400
|
| ACPI: EC: Handle IRQ storm on Acer laptops

Signed-off-by: Pavel Machek <[email protected]>

---
commit 2fc3b3069659f4d914cd6863f606c5d01d7d1644
tree 39e9fded0e00ba04947c24073873b5c7ff55af52
parent e094d39e372df9d3e360f2c2672f2860c0995f6a
author Pavel <[email protected]> Wed, 19 Mar 2008 00:43:14 +0100
committer Pavel <[email protected]> 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/

2008-03-19 21:16:08

by Pavel Machek

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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/

2008-03-19 21:16:52

by Pavel Machek

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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/

2008-03-19 21:17:58

by Pavel Machek

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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 <[email protected]> Tue, 11 Mar 2008 16:56:47
+0800
committer Len Brown <[email protected]> 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 <[email protected]>
Signed-off-by: Zhang Rui <[email protected]>
Signed-off-by: Len Brown <[email protected]>


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/

2008-03-19 21:18:38

by Pavel Machek

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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 <[email protected]> Tue, 11 Mar 2008
13:30:00 -0400
committer Len Brown <[email protected]> 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 <[email protected]>
Signed-off-by: Len Brown <[email protected]>



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/

2008-03-19 21:38:40

by Pavel Machek

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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

2008-03-19 21:39:00

by Pavel Machek

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

> 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 <[email protected]>
>
> Patch have caused several new troubles, so just revert it for now.
>
> Signed-off-by: Alexey Starikovskiy <[email protected]>

Tested-by: Pavel Machek <[email protected]>
ACK.
Pavel


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

2008-03-19 21:58:58

by Pavel Machek

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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 <[email protected]>

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/

2008-03-19 22:01:47

by Pavel Machek

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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/

2008-03-19 22:17:10

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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

2008-03-19 22:18:31

by Alexey Starikovskiy

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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
>
>

2008-03-19 22:21:47

by Pavel Machek

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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/

2008-03-19 22:31:28

by Alexey Starikovskiy

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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.

2008-03-19 22:36:59

by Alexey Starikovskiy

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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.


2008-03-19 23:03:44

by Alexey Starikovskiy

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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.


Attachments:
x-rev.patch (1.63 kB)

2008-03-19 23:17:57

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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

2008-03-19 23:22:33

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [linux-pm] suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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

2008-03-19 23:22:56

by Pavel Machek

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)


> > >> 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/

2008-03-19 23:25:40

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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

2008-03-19 23:30:29

by Pavel Machek

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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/

2008-03-19 23:46:19

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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

2008-03-19 23:51:13

by Pavel Machek

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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/

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

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

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

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

2008-03-20 15:51:11

by Alexey Starikovskiy

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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.
>
>

2008-03-20 15:54:20

by Pavel Machek

[permalink] [raw]
Subject: Re: suspend slow in 2.6.25-rc6 (was Re: Suspend and hibernation patchset against -rc6)

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

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

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

2008-03-26 23:52:00

by Rafael J. Wysocki

[permalink] [raw]
Subject: Suspend and hibernation patchset against -rc7

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