ACPI video mode patch review

* ACPI video mode patch review
@ 2007-09-13 21:37 H. Peter Anvin
  2007-09-13 23:23 ` Jeff Chua
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: H. Peter Anvin @ 2007-09-13 21:37 UTC (permalink / raw)
  To: Pavel Machek; +Cc: Jeff Chua, Linux Kernel Mailing List

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

Pavel, want to look at the patch before sending it to Linus?

	--- THIS IS NOT (YET) A PULL REQUEST ---

	-hpa

[-- Attachment #2: msg --]
[-- Type: text/plain, Size: 6487 bytes --]

From: H. Peter Anvin <hpa@zytor.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Bcc: H. Peter Anvin <hpa+sent@zytor.com>
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 <hpa@zytor.com>
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 <pavel@ucw.cz>
    Signed-off-by: H. Peter Anvin <hpa@zytor.com>

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 <hpa@zytor.com>
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 <hpa@zytor.com>

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread