From: H. Peter Anvin <[email protected]>
To: Linus Torvalds <[email protected]>
Cc: Linux Kernel Mailing List <[email protected]>
Bcc: H. Peter Anvin <[email protected]>
Subject: [GIT PULL] Fix decoding of video mode numbers in acpi/wakeup.S
Hi Linus,
Please pull:
git://git.kernel.org/pub/scm/linux/kernel/git/hpa/linux-2.6-x86setup.git for-linus
H. Peter Anvin (2):
[x86 setup] Present the canonical video mode number to the kernel
[acpi] Correct the decoding of video mode numbers in wakeup.S
arch/i386/boot/video.c | 14 ++++++++---
arch/i386/kernel/acpi/wakeup.S | 41 ++++++++-------------------------
arch/x86_64/kernel/acpi/wakeup.S | 47 ++++++++++---------------------------
3 files changed, 33 insertions(+), 69 deletions(-)
[Log messages and full diffs follow]
commit 5178fb23e3e41500fd066ecd4be15a60dd373e75
Author: H. Peter Anvin <[email protected]>
Date: Thu Sep 13 14:16:37 2007 -0700
[acpi] Correct the decoding of video mode numbers in wakeup.S
wakeup.S looks at the video mode number from the setup header and
looks to see if it is a VESA mode. Unfortunately, the decoding is
done incorrectly and it will attempt to frob the VESA BIOS for any
mode number 0x0200 or larger. Correct this, and remove a bunch of #if
0'd code.
Massive thanks to Jeff Chua for reporting the bug, and suffering
though a large number of experiments in order to track this problem
down.
Cc: Pavel Machek <[email protected]>
Signed-off-by: H. Peter Anvin <[email protected]>
diff --git a/arch/i386/kernel/acpi/wakeup.S b/arch/i386/kernel/acpi/wakeup.S
index ed0a0f2..f22ba85 100644
--- a/arch/i386/kernel/acpi/wakeup.S
+++ b/arch/i386/kernel/acpi/wakeup.S
@@ -151,51 +151,30 @@ bogus_real_magic:
#define VIDEO_FIRST_V7 0x0900
# Setting of user mode (AX=mode ID) => CF=success
+
+# For now, we only handle VESA modes (0x0200..0x03ff). To handle other
+# modes, we should probably compile in the video code from the boot
+# directory.
mode_set:
movw %ax, %bx
-#if 0
- cmpb $0xff, %ah
- jz setalias
-
- testb $VIDEO_RECALC>>8, %ah
- jnz _setrec
-
- cmpb $VIDEO_FIRST_RESOLUTION>>8, %ah
- jnc setres
-
- cmpb $VIDEO_FIRST_SPECIAL>>8, %ah
- jz setspc
-
- cmpb $VIDEO_FIRST_V7>>8, %ah
- jz setv7
-#endif
-
- cmpb $VIDEO_FIRST_VESA>>8, %ah
- jnc check_vesa
-#if 0
- orb %ah, %ah
- jz setmenu
-#endif
-
- decb %ah
-# jz setbios Add bios modes later
+ subb $VIDEO_FIRST_VESA>>8, %bh
+ cmpb $2, %bh
+ jb check_vesa
-setbad: clc
+setbad:
+ clc
ret
check_vesa:
- subb $VIDEO_FIRST_VESA>>8, %bh
orw $0x4000, %bx # Use linear frame buffer
movw $0x4f02, %ax # VESA BIOS mode set call
int $0x10
cmpw $0x004f, %ax # AL=4f if implemented
- jnz _setbad # AH=0 if OK
+ jnz setbad # AH=0 if OK
stc
ret
-_setbad: jmp setbad
-
.code32
ALIGN
diff --git a/arch/x86_64/kernel/acpi/wakeup.S b/arch/x86_64/kernel/acpi/wakeup.S
index 13f1480..a06f2bc 100644
--- a/arch/x86_64/kernel/acpi/wakeup.S
+++ b/arch/x86_64/kernel/acpi/wakeup.S
@@ -81,7 +81,7 @@ wakeup_code:
testl $2, realmode_flags - wakeup_code
jz 1f
mov video_mode - wakeup_code, %ax
- call mode_seta
+ call mode_set
1:
movw $0xb800, %ax
@@ -291,52 +291,31 @@ no_longmode:
#define VIDEO_FIRST_V7 0x0900
# Setting of user mode (AX=mode ID) => CF=success
+
+# For now, we only handle VESA modes (0x0200..0x03ff). To handle other
+# modes, we should probably compile in the video code from the boot
+# directory.
.code16
-mode_seta:
+mode_set:
movw %ax, %bx
-#if 0
- cmpb $0xff, %ah
- jz setalias
-
- testb $VIDEO_RECALC>>8, %ah
- jnz _setrec
-
- cmpb $VIDEO_FIRST_RESOLUTION>>8, %ah
- jnc setres
-
- cmpb $VIDEO_FIRST_SPECIAL>>8, %ah
- jz setspc
-
- cmpb $VIDEO_FIRST_V7>>8, %ah
- jz setv7
-#endif
-
- cmpb $VIDEO_FIRST_VESA>>8, %ah
- jnc check_vesaa
-#if 0
- orb %ah, %ah
- jz setmenu
-#endif
-
- decb %ah
-# jz setbios Add bios modes later
+ subb $VIDEO_FIRST_VESA>>8, %bh
+ cmpb $2, %bh
+ jb check_vesa
-setbada: clc
+setbad:
+ clc
ret
-check_vesaa:
- subb $VIDEO_FIRST_VESA>>8, %bh
+check_vesa:
orw $0x4000, %bx # Use linear frame buffer
movw $0x4f02, %ax # VESA BIOS mode set call
int $0x10
cmpw $0x004f, %ax # AL=4f if implemented
- jnz _setbada # AH=0 if OK
+ jnz setbad # AH=0 if OK
stc
ret
-_setbada: jmp setbada
-
wakeup_stack_begin: # Stack grows down
.org 0xff0
commit f41b4d086c58ca531ad512e1e9d6712b2310969e
Author: H. Peter Anvin <[email protected]>
Date: Thu Sep 13 14:14:29 2007 -0700
[x86 setup] Present the canonical video mode number to the kernel
Canonicalize the video mode number as presented to the kernel. The
video mode number may be user-entered (e.g. ASK_VGA), an alias
(e.g. NORMAL_VGA), or a size specification, and that confuses the
suspend wakeup code.
Signed-off-by: H. Peter Anvin <[email protected]>
diff --git a/arch/i386/boot/video.c b/arch/i386/boot/video.c
index 693f20d..e4ba897 100644
--- a/arch/i386/boot/video.c
+++ b/arch/i386/boot/video.c
@@ -147,7 +147,7 @@ int mode_defined(u16 mode)
}
/* Set mode (without recalc) */
-static int raw_set_mode(u16 mode)
+static int raw_set_mode(u16 mode, u16 *real_mode)
{
int nmode, i;
struct card_info *card;
@@ -165,8 +165,10 @@ static int raw_set_mode(u16 mode)
if ((mode == nmode && visible) ||
mode == mi->mode ||
- mode == (mi->y << 8)+mi->x)
+ mode == (mi->y << 8)+mi->x) {
+ *real_mode = mi->mode;
return card->set_mode(mi);
+ }
if (visible)
nmode++;
@@ -178,7 +180,7 @@ static int raw_set_mode(u16 mode)
if (mode >= card->xmode_first &&
mode < card->xmode_first+card->xmode_n) {
struct mode_info mix;
- mix.mode = mode;
+ *real_mode = mix.mode = mode;
mix.x = mix.y = 0;
return card->set_mode(&mix);
}
@@ -223,6 +225,7 @@ static void vga_recalc_vertical(void)
static int set_mode(u16 mode)
{
int rv;
+ u16 real_mode;
/* Very special mode numbers... */
if (mode == VIDEO_CURRENT_MODE)
@@ -232,13 +235,16 @@ static int set_mode(u16 mode)
else if (mode == EXTENDED_VGA)
mode = VIDEO_8POINT;
- rv = raw_set_mode(mode);
+ rv = raw_set_mode(mode, &real_mode);
if (rv)
return rv;
if (mode & VIDEO_RECALC)
vga_recalc_vertical();
+ /* Save the canonical mode number for the kernel, not
+ an alias, size specification or menu position */
+ boot_params.hdr.vid_mode = real_mode;
return 0;
}
On 9/14/07, H. Peter Anvin <[email protected]> wrote:
> Pavel, want to look at the patch before sending it to Linus?
>
> [acpi] Correct the decoding of video mode numbers in wakeup.S
HPA,
After a day, still works, no video mess-up after s2ram resume. I guess
we can close this regression for -rc6 now. I'll test it again after
Linus release the next update.
Thanks for the patch. Great work!
Jeff.
Hi!
> Pavel, want to look at the patch before sending it to Linus?
Looks okay from a quick look. I'm on conference just now, will test
shortly.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Thursday, 13 September 2007 23:37, H. Peter Anvin wrote:
> Pavel, want to look at the patch before sending it to Linus?
Many thanks for taking care of this!
We already have a patch in -mm, s2ram-kill-old-debugging-junk.patch from Pavel
(http://marc.info/?l=linux-mm-commits&m=118737955611331&w=1), that removes the
#ifdefed blocks and it clashes with your first patch a bit.
FYI, I have rebased your first patch on top of the Pavel's patch:
http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.23-rc6/patches/39-acpi-video-mode-fix.patch
Greetings,
Rafael
Rafael J. Wysocki wrote:
> On Thursday, 13 September 2007 23:37, H. Peter Anvin wrote:
>> Pavel, want to look at the patch before sending it to Linus?
>
> Many thanks for taking care of this!
>
> We already have a patch in -mm, s2ram-kill-old-debugging-junk.patch from Pavel
> (http://marc.info/?l=linux-mm-commits&m=118737955611331&w=1), that removes the
> #ifdefed blocks and it clashes with your first patch a bit.
>
> FYI, I have rebased your first patch on top of the Pavel's patch:
> http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.23-rc6/patches/39-acpi-video-mode-fix.patch
>
Thanks Rafael,
However, I need to send something upstream to Linus for this kernel
cycle, so I don't want to base it on an -mm patch. There are two
alternatives, obviously: 1. send my patch in now based on the "change as
little as necessary to fix the immediate problem" and then rebase
Pavel's patch on top of mine, or 2. for me to send both Pavel's patch
and the rebased patch upstream.
Personally, I would prefer to avoid strategy 2 at this late stage in the
2.6.23-rc series.
Please let me know what you think.
-hpa
Hi!
> > Many thanks for taking care of this!
> >
> > We already have a patch in -mm, s2ram-kill-old-debugging-junk.patch from Pavel
> > (http://marc.info/?l=linux-mm-commits&m=118737955611331&w=1), that removes the
> > #ifdefed blocks and it clashes with your first patch a bit.
> >
> > FYI, I have rebased your first patch on top of the Pavel's patch:
> > http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.23-rc6/patches/39-acpi-video-mode-fix.patch
> >
>
> Thanks Rafael,
>
> However, I need to send something upstream to Linus for this kernel
> cycle, so I don't want to base it on an -mm patch. There are two
> alternatives, obviously: 1. send my patch in now based on the "change as
> little as necessary to fix the immediate problem" and then rebase
> Pavel's patch on top of mine, or 2. for me to send both Pavel's patch
> and the rebased patch upstream.
>
> Personally, I would prefer to avoid strategy 2 at this late stage in the
> 2.6.23-rc series.
Agreed we should have the fix in 2.6.23... Actually doing 2 does not
seem like a big problem, my patch is pretty straight cleanup (and may
even fix some machines, as we avoid touching video ram)... But
resolving conflict is not hard, either; I know, I did it :-).
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Sunday, 16 September 2007 22:59, Pavel Machek wrote:
> Hi!
>
> > > Many thanks for taking care of this!
> > >
> > > We already have a patch in -mm, s2ram-kill-old-debugging-junk.patch from Pavel
> > > (http://marc.info/?l=linux-mm-commits&m=118737955611331&w=1), that removes the
> > > #ifdefed blocks and it clashes with your first patch a bit.
> > >
> > > FYI, I have rebased your first patch on top of the Pavel's patch:
> > > http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.23-rc6/patches/39-acpi-video-mode-fix.patch
> > >
> >
> > Thanks Rafael,
> >
> > However, I need to send something upstream to Linus for this kernel
> > cycle, so I don't want to base it on an -mm patch. There are two
> > alternatives, obviously: 1. send my patch in now based on the "change as
> > little as necessary to fix the immediate problem" and then rebase
> > Pavel's patch on top of mine, or 2. for me to send both Pavel's patch
> > and the rebased patch upstream.
> >
> > Personally, I would prefer to avoid strategy 2 at this late stage in the
> > 2.6.23-rc series.
>
> Agreed we should have the fix in 2.6.23... Actually doing 2 does not
> seem like a big problem, my patch is pretty straight cleanup (and may
> even fix some machines, as we avoid touching video ram)... But
> resolving conflict is not hard, either; I know, I did it :-).
OK, let's take path 1, then.
Greetings,
Rafael
On Thu, 13 Sep 2007, H. Peter Anvin wrote:
> Pavel, want to look at the patch before sending it to Linus?
Hi,
sorry for this taking too long, I was physically located away from the
hardware.
Without your patch, after resume I got corrupted image (looked like
remains of BIOS POST graphics, in bad resolution, or something like that).
With your patch applied, I get no video at all after resume (I tried all
three possible combinations of acpi_sleep parameter).
--
Jiri Kosina
On 9/20/07, Jiri Kosina <[email protected]> wrote:
> With your patch applied, I get no video at all after resume (I tried all
> three possible combinations of acpi_sleep parameter).
Can you please try again with 2.6.23-rc7. The patch is already
included in -rc7 and it works for me on Lenono X60s.
Jeff.
On Thu, 20 Sep 2007, Jeff Chua wrote:
> > With your patch applied, I get no video at all after resume (I tried all
> > three possible combinations of acpi_sleep parameter).
> Can you please try again with 2.6.23-rc7. The patch is already
> included in -rc7 and it works for me on Lenono X60s.
With current -git, I still get the same behavior as with
previous 2.6.23-rcX kernels - i.e. after resume (with
acpi_sleep=s3_bios,s3_mode), I get corrupted image resembling the BIOS
POST graphics.
Previous kernels (I am currently pretty sure that
15028aad00ddf241581fbe74a02ec89cbb28d35d was OK, but I haven't done the
bisection yet) resumed perfectly with acpi_sleep=s3_bios,s3_mode on this
machine.
Please note that this is very different HW than you have - it is sort-of a
desktop machine, not a notebook at all.
--
Jiri Kosina
On 9/20/07, Jiri Kosina <[email protected]> wrote:
> On Thu, 20 Sep 2007, Jeff Chua wrote:
>
> With current -git, I still get the same behavior as with
> previous 2.6.23-rcX kernels - i.e. after resume (with
> acpi_sleep=s3_bios,s3_mode), I get corrupted image resembling the BIOS
> POST graphics.
>
> Previous kernels (I am currently pretty sure that
> 15028aad00ddf241581fbe74a02ec89cbb28d35d was OK, but I haven't done the
> bisection yet) resumed perfectly with acpi_sleep=s3_bios,s3_mode on this
> machine.
>
> Please note that this is very different HW than you have - it is sort-of a
> desktop machine, not a notebook at all.
Sorry, with -rc7, still needs the patch. Apparently it's still not
included in the -rc7 kernel.
It might already be in the latest git, but just want to be sure that's the case.
Jeff.