[PATCH] retrieve VBE EDID/DDC info independent of used video mode

[PATCH] retrieve VBE EDID/DDC info independent of used video mode

[Jan Beulich]

The code to retrieve this information was (a) inside a CONFIG_VIDEO_SELECT
section and (b) protected by a check of a variable (vbe_version) that
would get initialized only when a VESA mode was selected on the command
line.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

 arch/i386/boot/video.S |   28 ++++++++++++++++++----------
 1 files changed, 18 insertions(+), 10 deletions(-)

--- linux-2.6.22-rc5/arch/i386/boot/video.S	2007-04-26 05:08:32.000000000 +0200
+++ 2.6.22-rc5-edid-no-vesa-mode/arch/i386/boot/video.S	2007-06-19 14:34:50.000000000 +0200
@@ -96,6 +96,7 @@
 #define PARAM_LFB_PAGES		0x32
 #define PARAM_VESA_ATTRIB	0x34
 #define PARAM_CAPABILITIES	0x36
+#define PARAM_EDID_INFO		0x140
 
 /* Define DO_STORE according to CONFIG_VIDEO_RETAIN */
 #ifdef CONFIG_VIDEO_RETAIN
@@ -132,8 +133,8 @@ vid1:
 #ifdef CONFIG_VIDEO_RETAIN
 	call	restore_screen			# Restore screen contents
 #endif /* CONFIG_VIDEO_RETAIN */
-	call	store_edid
 #endif /* CONFIG_VIDEO_SELECT */
+	call	store_edid
 	call	mode_params			# Store mode parameters
 	popw	%ds				# Restore original DS
 	ret
@@ -571,16 +572,12 @@ setr1:	lodsw
 	jmp	_m_s
 
 check_vesa:
-#ifdef CONFIG_FIRMWARE_EDID
 	leaw	modelist+1024, %di
 	movw	$0x4f00, %ax
 	int	$0x10
 	cmpw	$0x004f, %ax
 	jnz	setbad
 
-	movw	4(%di), %ax
-	movw	%ax, vbe_version
-#endif
 	leaw	modelist+1024, %di
 	subb	$VIDEO_FIRST_VESA>>8, %bh
 	movw	%bx, %cx			# Get mode information structure
@@ -1935,6 +1932,7 @@ skip10:	movb	%ah, %al
 	popw	%cx
 	popw	%ax
 	ret
+#endif /* CONFIG_VIDEO_SELECT */
 
 store_edid:
 #ifdef CONFIG_FIRMWARE_EDID
@@ -1945,18 +1943,28 @@ store_edid:
 	pushw	%dx
 	pushw   %di
 
+	pushw	%ds
+	popw    %es
+	leaw	modelist, %di
+	movw	$0x4f00, %ax
+	int	$0x10
+	cmpw	$0x004f, %ax
+	setne	%dl
+	cmpw	$0x0200, 4(%di)			# only do EDID on >= VBE2.0
+	adc	%dl, %dl
+
 	pushw	%fs
 	popw    %es
 
 	movl	$0x13131313, %eax		# memset block with 0x13
 	movw    $32, %cx
-	movw	$0x140, %di
+	movw	$PARAM_EDID_INFO, %di
 	cld
 	rep
 	stosl
 
-	cmpw	$0x0200, vbe_version		# only do EDID on >= VBE2.0
-	jl	no_edid
+	testb	%dl, %dl
+	jnz	no_edid
 
 	pushw   %es				# save ES
 	xorw    %di, %di                        # Report Capability
@@ -1978,7 +1986,7 @@ store_edid:
 	movw	$0x01, %bx
 	movw	$0x00, %cx
 	movw    $0x00, %dx
-	movw	$0x140, %di
+	movw	$PARAM_EDID_INFO, %di
 	int	$0x10
 
 no_edid:
@@ -1991,6 +1999,7 @@ no_edid:
 #endif
 	ret
 
+#ifdef CONFIG_VIDEO_SELECT
 # VIDEO_SELECT-only variables
 mt_end:		.word	0	# End of video mode table if built
 edit_buf:	.space	6	# Line editor buffer
@@ -2000,7 +2009,6 @@ do_restore:	.byte	0	# Screen contents al
 svga_prefix:	.byte	VIDEO_FIRST_BIOS>>8	# Default prefix for BIOS modes
 graphic_mode:	.byte	0	# Graphic mode with a linear frame buffer
 dac_size:	.byte	6	# DAC bit depth
-vbe_version:	.word	0	# VBE bios version
 
 # Status messages
 keymsg:		.ascii	"Press <RETURN> to see video modes available, "

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[Daniel Drake]

Jan Beulich wrote:
> The code to retrieve this information was (a) inside a CONFIG_VIDEO_SELECT
> section and (b) protected by a check of a variable (vbe_version) that
> would get initialized only when a VESA mode was selected on the command
> line.

This patch solves a 2.6.20.11 (and 2.6.21) regression added by the patch 
titled "x86: Don't probe for DDC on VBE1.2"

The regression caused the screen resolution to be incorrectly adjusted 
by 6 pixels. This patch makes it go back to normal again.

https://bugs.gentoo.org/show_bug.cgi?id=181067

Please apply this patch or discuss further :)

> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> 
>  arch/i386/boot/video.S |   28 ++++++++++++++++++----------
>  1 files changed, 18 insertions(+), 10 deletions(-)
> 
> --- linux-2.6.22-rc5/arch/i386/boot/video.S	2007-04-26 05:08:32.000000000 +0200
> +++ 2.6.22-rc5-edid-no-vesa-mode/arch/i386/boot/video.S	2007-06-19 14:34:50.000000000 +0200
> @@ -96,6 +96,7 @@
>  #define PARAM_LFB_PAGES		0x32
>  #define PARAM_VESA_ATTRIB	0x34
>  #define PARAM_CAPABILITIES	0x36
> +#define PARAM_EDID_INFO		0x140
>  
>  /* Define DO_STORE according to CONFIG_VIDEO_RETAIN */
>  #ifdef CONFIG_VIDEO_RETAIN
> @@ -132,8 +133,8 @@ vid1:
>  #ifdef CONFIG_VIDEO_RETAIN
>  	call	restore_screen			# Restore screen contents
>  #endif /* CONFIG_VIDEO_RETAIN */
> -	call	store_edid
>  #endif /* CONFIG_VIDEO_SELECT */
> +	call	store_edid
>  	call	mode_params			# Store mode parameters
>  	popw	%ds				# Restore original DS
>  	ret
> @@ -571,16 +572,12 @@ setr1:	lodsw
>  	jmp	_m_s
>  
>  check_vesa:
> -#ifdef CONFIG_FIRMWARE_EDID
>  	leaw	modelist+1024, %di
>  	movw	$0x4f00, %ax
>  	int	$0x10
>  	cmpw	$0x004f, %ax
>  	jnz	setbad
>  
> -	movw	4(%di), %ax
> -	movw	%ax, vbe_version
> -#endif
>  	leaw	modelist+1024, %di
>  	subb	$VIDEO_FIRST_VESA>>8, %bh
>  	movw	%bx, %cx			# Get mode information structure
> @@ -1935,6 +1932,7 @@ skip10:	movb	%ah, %al
>  	popw	%cx
>  	popw	%ax
>  	ret
> +#endif /* CONFIG_VIDEO_SELECT */
>  
>  store_edid:
>  #ifdef CONFIG_FIRMWARE_EDID
> @@ -1945,18 +1943,28 @@ store_edid:
>  	pushw	%dx
>  	pushw   %di
>  
> +	pushw	%ds
> +	popw    %es
> +	leaw	modelist, %di
> +	movw	$0x4f00, %ax
> +	int	$0x10
> +	cmpw	$0x004f, %ax
> +	setne	%dl
> +	cmpw	$0x0200, 4(%di)			# only do EDID on >= VBE2.0
> +	adc	%dl, %dl
> +
>  	pushw	%fs
>  	popw    %es
>  
>  	movl	$0x13131313, %eax		# memset block with 0x13
>  	movw    $32, %cx
> -	movw	$0x140, %di
> +	movw	$PARAM_EDID_INFO, %di
>  	cld
>  	rep
>  	stosl
>  
> -	cmpw	$0x0200, vbe_version		# only do EDID on >= VBE2.0
> -	jl	no_edid
> +	testb	%dl, %dl
> +	jnz	no_edid
>  
>  	pushw   %es				# save ES
>  	xorw    %di, %di                        # Report Capability
> @@ -1978,7 +1986,7 @@ store_edid:
>  	movw	$0x01, %bx
>  	movw	$0x00, %cx
>  	movw    $0x00, %dx
> -	movw	$0x140, %di
> +	movw	$PARAM_EDID_INFO, %di
>  	int	$0x10
>  
>  no_edid:
> @@ -1991,6 +1999,7 @@ no_edid:
>  #endif
>  	ret
>  
> +#ifdef CONFIG_VIDEO_SELECT
>  # VIDEO_SELECT-only variables
>  mt_end:		.word	0	# End of video mode table if built
>  edit_buf:	.space	6	# Line editor buffer
> @@ -2000,7 +2009,6 @@ do_restore:	.byte	0	# Screen contents al
>  svga_prefix:	.byte	VIDEO_FIRST_BIOS>>8	# Default prefix for BIOS modes
>  graphic_mode:	.byte	0	# Graphic mode with a linear frame buffer
>  dac_size:	.byte	6	# DAC bit depth
> -vbe_version:	.word	0	# VBE bios version
>  
>  # Status messages
>  keymsg:		.ascii	"Press <RETURN> to see video modes available, "
> 
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[Andrew Morton]

On Sat, 30 Jun 2007 18:25:31 -0400 Daniel Drake <dsd@gentoo.org> wrote:

> Jan Beulich wrote:
> > The code to retrieve this information was (a) inside a CONFIG_VIDEO_SELECT
> > section and (b) protected by a check of a variable (vbe_version) that
> > would get initialized only when a VESA mode was selected on the command
> > line.
> 
> This patch solves a 2.6.20.11 (and 2.6.21) regression added by the patch 
> titled "x86: Don't probe for DDC on VBE1.2"
> 
> The regression caused the screen resolution to be incorrectly adjusted 
> by 6 pixels. This patch makes it go back to normal again.
> 
> https://bugs.gentoo.org/show_bug.cgi?id=181067
> 
> Please apply this patch or discuss further :)

Well drat.  I didn't merge the patch because it conflicts with
git-newsetup, and Peter believes that git-newsetup already contains an
equivalent fix.  Testing 2.6.22-rc6-mm1 would confirm that.  Please.

So what do we think?  Should we merge Jan's patch into 2.6.22?  And 2.6.21?
And do we feel that it is safe enough for this?

Or can we afford to leave this bug in place in 2.6.22, which would suck a
bit?


> > Signed-off-by: Jan Beulich <jbeulich@novell.com>
> > 
> >  arch/i386/boot/video.S |   28 ++++++++++++++++++----------
> >  1 files changed, 18 insertions(+), 10 deletions(-)
> > 
> > --- linux-2.6.22-rc5/arch/i386/boot/video.S	2007-04-26 05:08:32.000000000 +0200
> > +++ 2.6.22-rc5-edid-no-vesa-mode/arch/i386/boot/video.S	2007-06-19 14:34:50.000000000 +0200
> > @@ -96,6 +96,7 @@
> >  #define PARAM_LFB_PAGES		0x32
> >  #define PARAM_VESA_ATTRIB	0x34
> >  #define PARAM_CAPABILITIES	0x36
> > +#define PARAM_EDID_INFO		0x140
> >  
> >  /* Define DO_STORE according to CONFIG_VIDEO_RETAIN */
> >  #ifdef CONFIG_VIDEO_RETAIN
> > @@ -132,8 +133,8 @@ vid1:
> >  #ifdef CONFIG_VIDEO_RETAIN
> >  	call	restore_screen			# Restore screen contents
> >  #endif /* CONFIG_VIDEO_RETAIN */
> > -	call	store_edid
> >  #endif /* CONFIG_VIDEO_SELECT */
> > +	call	store_edid
> >  	call	mode_params			# Store mode parameters
> >  	popw	%ds				# Restore original DS
> >  	ret
> > @@ -571,16 +572,12 @@ setr1:	lodsw
> >  	jmp	_m_s
> >  
> >  check_vesa:
> > -#ifdef CONFIG_FIRMWARE_EDID
> >  	leaw	modelist+1024, %di
> >  	movw	$0x4f00, %ax
> >  	int	$0x10
> >  	cmpw	$0x004f, %ax
> >  	jnz	setbad
> >  
> > -	movw	4(%di), %ax
> > -	movw	%ax, vbe_version
> > -#endif
> >  	leaw	modelist+1024, %di
> >  	subb	$VIDEO_FIRST_VESA>>8, %bh
> >  	movw	%bx, %cx			# Get mode information structure
> > @@ -1935,6 +1932,7 @@ skip10:	movb	%ah, %al
> >  	popw	%cx
> >  	popw	%ax
> >  	ret
> > +#endif /* CONFIG_VIDEO_SELECT */
> >  
> >  store_edid:
> >  #ifdef CONFIG_FIRMWARE_EDID
> > @@ -1945,18 +1943,28 @@ store_edid:
> >  	pushw	%dx
> >  	pushw   %di
> >  
> > +	pushw	%ds
> > +	popw    %es
> > +	leaw	modelist, %di
> > +	movw	$0x4f00, %ax
> > +	int	$0x10
> > +	cmpw	$0x004f, %ax
> > +	setne	%dl
> > +	cmpw	$0x0200, 4(%di)			# only do EDID on >= VBE2.0
> > +	adc	%dl, %dl
> > +
> >  	pushw	%fs
> >  	popw    %es
> >  
> >  	movl	$0x13131313, %eax		# memset block with 0x13
> >  	movw    $32, %cx
> > -	movw	$0x140, %di
> > +	movw	$PARAM_EDID_INFO, %di
> >  	cld
> >  	rep
> >  	stosl
> >  
> > -	cmpw	$0x0200, vbe_version		# only do EDID on >= VBE2.0
> > -	jl	no_edid
> > +	testb	%dl, %dl
> > +	jnz	no_edid
> >  
> >  	pushw   %es				# save ES
> >  	xorw    %di, %di                        # Report Capability
> > @@ -1978,7 +1986,7 @@ store_edid:
> >  	movw	$0x01, %bx
> >  	movw	$0x00, %cx
> >  	movw    $0x00, %dx
> > -	movw	$0x140, %di
> > +	movw	$PARAM_EDID_INFO, %di
> >  	int	$0x10
> >  
> >  no_edid:
> > @@ -1991,6 +1999,7 @@ no_edid:
> >  #endif
> >  	ret
> >  
> > +#ifdef CONFIG_VIDEO_SELECT
> >  # VIDEO_SELECT-only variables
> >  mt_end:		.word	0	# End of video mode table if built
> >  edit_buf:	.space	6	# Line editor buffer
> > @@ -2000,7 +2009,6 @@ do_restore:	.byte	0	# Screen contents al
> >  svga_prefix:	.byte	VIDEO_FIRST_BIOS>>8	# Default prefix for BIOS modes
> >  graphic_mode:	.byte	0	# Graphic mode with a linear frame buffer
> >  dac_size:	.byte	6	# DAC bit depth
> > -vbe_version:	.word	0	# VBE bios version
> >  
> >  # Status messages
> >  keymsg:		.ascii	"Press <RETURN> to see video modes available, "
> > 
> > 
> > 
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> > 

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[Chuck Ebbert]

On 06/30/2007 06:25 PM, Daniel Drake wrote:
> Jan Beulich wrote:
>> The code to retrieve this information was (a) inside a
>> CONFIG_VIDEO_SELECT
>> section and (b) protected by a check of a variable (vbe_version) that
>> would get initialized only when a VESA mode was selected on the command
>> line.
> 
> This patch solves a 2.6.20.11 (and 2.6.21) regression added by the patch
> titled "x86: Don't probe for DDC on VBE1.2"
> 
> The regression caused the screen resolution to be incorrectly adjusted
> by 6 pixels. This patch makes it go back to normal again.
> 
> https://bugs.gentoo.org/show_bug.cgi?id=181067
> 
> 

Well, that explains this too I'd guess:

	https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=245741

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[Daniel Drake]

Andrew Morton wrote:
> On Sat, 30 Jun 2007 18:25:31 -0400 Daniel Drake <dsd@gentoo.org> wrote:
>> This patch solves a 2.6.20.11 (and 2.6.21) regression added by the patch 
>> titled "x86: Don't probe for DDC on VBE1.2"
>>
>> The regression caused the screen resolution to be incorrectly adjusted 
>> by 6 pixels. This patch makes it go back to normal again.
>>
>> https://bugs.gentoo.org/show_bug.cgi?id=181067
>>
>> Please apply this patch or discuss further :)
> 
> Well drat.  I didn't merge the patch because it conflicts with
> git-newsetup, and Peter believes that git-newsetup already contains an
> equivalent fix.  Testing 2.6.22-rc6-mm1 would confirm that.  Please.

2.6.22-rc6-mm1 has the same problem (it is not fixed there).

Daniel

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[Andrew Morton]

On Sun, 08 Jul 2007 09:27:55 -0400 Daniel Drake <dsd@gentoo.org> wrote:

> Andrew Morton wrote:
> > On Sat, 30 Jun 2007 18:25:31 -0400 Daniel Drake <dsd@gentoo.org> wrote:
> >> This patch solves a 2.6.20.11 (and 2.6.21) regression added by the patch 
> >> titled "x86: Don't probe for DDC on VBE1.2"
> >>
> >> The regression caused the screen resolution to be incorrectly adjusted 
> >> by 6 pixels. This patch makes it go back to normal again.
> >>
> >> https://bugs.gentoo.org/show_bug.cgi?id=181067
> >>
> >> Please apply this patch or discuss further :)
> > 
> > Well drat.  I didn't merge the patch because it conflicts with
> > git-newsetup, and Peter believes that git-newsetup already contains an
> > equivalent fix.  Testing 2.6.22-rc6-mm1 would confirm that.  Please.
> 
> 2.6.22-rc6-mm1 has the same problem (it is not fixed there).
> 

Well damn, we've let this slide for too long.

Guys, 2.6.22 is days away.  Do we think that the below is safe to merge
now?

Add Daniel, this does fix things for you, doesn't it?


From: "Jan Beulich" <jbeulich@novell.com>

The code to retrieve this information was (a) inside a CONFIG_VIDEO_SELECT
section and (b) protected by a check of a variable (vbe_version) that would
get initialized only when a VESA mode was selected on the command line.

Signed-off-by: Jan Beulich <jbeulich@novell.com>
Cc: Daniel Drake <dsd@gentoo.org>
Cc: Andi Kleen <ak@suse.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: "Antonino A. Daplas" <adaplas@pol.net>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 arch/i386/boot/video.S |   28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff -puN arch/i386/boot/video.S~retrieve-vbe-edid-ddc-info-independent-of-used-video arch/i386/boot/video.S
--- a/arch/i386/boot/video.S~retrieve-vbe-edid-ddc-info-independent-of-used-video
+++ a/arch/i386/boot/video.S
@@ -96,6 +96,7 @@
 #define PARAM_LFB_PAGES		0x32
 #define PARAM_VESA_ATTRIB	0x34
 #define PARAM_CAPABILITIES	0x36
+#define PARAM_EDID_INFO		0x140
 
 /* Define DO_STORE according to CONFIG_VIDEO_RETAIN */
 #ifdef CONFIG_VIDEO_RETAIN
@@ -132,8 +133,8 @@ vid1:
 #ifdef CONFIG_VIDEO_RETAIN
 	call	restore_screen			# Restore screen contents
 #endif /* CONFIG_VIDEO_RETAIN */
-	call	store_edid
 #endif /* CONFIG_VIDEO_SELECT */
+	call	store_edid
 	call	mode_params			# Store mode parameters
 	popw	%ds				# Restore original DS
 	ret
@@ -571,16 +572,12 @@ setr1:	lodsw
 	jmp	_m_s
 
 check_vesa:
-#ifdef CONFIG_FIRMWARE_EDID
 	leaw	modelist+1024, %di
 	movw	$0x4f00, %ax
 	int	$0x10
 	cmpw	$0x004f, %ax
 	jnz	setbad
 
-	movw	4(%di), %ax
-	movw	%ax, vbe_version
-#endif
 	leaw	modelist+1024, %di
 	subb	$VIDEO_FIRST_VESA>>8, %bh
 	movw	%bx, %cx			# Get mode information structure
@@ -1935,6 +1932,7 @@ skip10:	movb	%ah, %al
 	popw	%cx
 	popw	%ax
 	ret
+#endif /* CONFIG_VIDEO_SELECT */
 
 store_edid:
 #ifdef CONFIG_FIRMWARE_EDID
@@ -1945,18 +1943,28 @@ store_edid:
 	pushw	%dx
 	pushw   %di
 
+	pushw	%ds
+	popw    %es
+	leaw	modelist, %di
+	movw	$0x4f00, %ax
+	int	$0x10
+	cmpw	$0x004f, %ax
+	setne	%dl
+	cmpw	$0x0200, 4(%di)			# only do EDID on >= VBE2.0
+	adc	%dl, %dl
+
 	pushw	%fs
 	popw    %es
 
 	movl	$0x13131313, %eax		# memset block with 0x13
 	movw    $32, %cx
-	movw	$0x140, %di
+	movw	$PARAM_EDID_INFO, %di
 	cld
 	rep
 	stosl
 
-	cmpw	$0x0200, vbe_version		# only do EDID on >= VBE2.0
-	jl	no_edid
+	testb	%dl, %dl
+	jnz	no_edid
 
 	pushw   %es				# save ES
 	xorw    %di, %di                        # Report Capability
@@ -1978,7 +1986,7 @@ store_edid:
 	movw	$0x01, %bx
 	movw	$0x00, %cx
 	movw    $0x00, %dx
-	movw	$0x140, %di
+	movw	$PARAM_EDID_INFO, %di
 	int	$0x10
 
 no_edid:
@@ -1991,6 +1999,7 @@ no_edid:
 #endif
 	ret
 
+#ifdef CONFIG_VIDEO_SELECT
 # VIDEO_SELECT-only variables
 mt_end:		.word	0	# End of video mode table if built
 edit_buf:	.space	6	# Line editor buffer
@@ -2000,7 +2009,6 @@ do_restore:	.byte	0	# Screen contents al
 svga_prefix:	.byte	VIDEO_FIRST_BIOS>>8	# Default prefix for BIOS modes
 graphic_mode:	.byte	0	# Graphic mode with a linear frame buffer
 dac_size:	.byte	6	# DAC bit depth
-vbe_version:	.word	0	# VBE bios version
 
 # Status messages
 keymsg:		.ascii	"Press <RETURN> to see video modes available, "
_

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[H. Peter Anvin]

Andrew Morton wrote:
> 
> Well damn, we've let this slide for too long.
> 
> Guys, 2.6.22 is days away.  Do we think that the below is safe to merge
> now?
> 
> Add Daniel, this does fix things for you, doesn't it?
> 

The patch should be fine for 2.6.22, but for -mm, the code this patches
doesn't even exist with git-newsetup; nor is there a CONFIG_VIDEO_SELECT
which could impede the production of the VESA version.

What does your .config look like?

	-hpa

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[Andi Kleen]

On Sunday 08 July 2007 20:14:04 Andrew Morton wrote:
> On Sun, 08 Jul 2007 09:27:55 -0400 Daniel Drake <dsd@gentoo.org> wrote:
> 
> > Andrew Morton wrote:
> > > On Sat, 30 Jun 2007 18:25:31 -0400 Daniel Drake <dsd@gentoo.org> wrote:
> > >> This patch solves a 2.6.20.11 (and 2.6.21) regression added by the patch 
> > >> titled "x86: Don't probe for DDC on VBE1.2"
> > >>
> > >> The regression caused the screen resolution to be incorrectly adjusted 
> > >> by 6 pixels. This patch makes it go back to normal again.
> > >>
> > >> https://bugs.gentoo.org/show_bug.cgi?id=181067
> > >>
> > >> Please apply this patch or discuss further :)
> > > 
> > > Well drat.  I didn't merge the patch because it conflicts with
> > > git-newsetup, and Peter believes that git-newsetup already contains an
> > > equivalent fix.  Testing 2.6.22-rc6-mm1 would confirm that.  Please.
> > 
> > 2.6.22-rc6-mm1 has the same problem (it is not fixed there).
> > 
> 
> Well damn, we've let this slide for too long.
> 
> Guys, 2.6.22 is days away.  Do we think that the below is safe to merge
> now?

I feel uneasy with the patch that late and i don't think the bug is 
particularly critical.

-Andi

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[H. Peter Anvin]

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

Daniel Drake wrote:
> 
> 2.6.22-rc6-mm1 has the same problem (it is not fixed there).
> 

I believe this patch should fix it for 2.6.22-rc6-mm1.  I will check
this into the newsetup tree.

	-hpa

[-- Attachment #2: diff --]
[-- Type: text/plain, Size: 892 bytes --]

diff --git a/arch/i386/boot/video-vesa.c b/arch/i386/boot/video-vesa.c
index 3c21bd7..e6aa9eb 100644
--- a/arch/i386/boot/video-vesa.c
+++ b/arch/i386/boot/video-vesa.c
@@ -28,7 +28,7 @@ static void vesa_store_mode_params_graphics(void);
 
 static int vesa_probe(void)
 {
-#ifdef CONFIG_VIDEO_VESA
+#if defined(CONFIG_VIDEO_VESA) || defined(CONFIG_FIRMWARE_EDID)
 	u16 ax;
 	u16 mode;
 	addr_t mode_ptr;
@@ -47,7 +47,8 @@ static int vesa_probe(void)
 	    vginfo.signature != VESA_MAGIC ||
 	    vginfo.version < 0x0102)
 		return 0;	/* Not present */
-
+#endif /* CONFIG_VIDEO_VESA || CONFIG_FIRMWARE_EDID */
+#ifdef CONFIG_VIDEO_VESA
 	set_fs(vginfo.video_mode_ptr.seg);
 	mode_ptr = vginfo.video_mode_ptr.off;
 
@@ -96,7 +97,7 @@ static int vesa_probe(void)
 	return nmodes;
 #else
 	return 0;
-#endif
+#endif /* CONFIG_VIDEO_VESA */
 }
 
 static int vesa_set_mode(struct mode_info *mode)

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[Daniel Drake]

Andrew Morton wrote:
>>> Well drat.  I didn't merge the patch because it conflicts with
>>> git-newsetup, and Peter believes that git-newsetup already contains an
>>> equivalent fix.  Testing 2.6.22-rc6-mm1 would confirm that.  Please.
>> 2.6.22-rc6-mm1 has the same problem (it is not fixed there).
>>
> 
> Well damn, we've let this slide for too long.
> 
> Guys, 2.6.22 is days away.  Do we think that the below is safe to merge
> now?
> 
> Add Daniel, this does fix things for you, doesn't it?

I don't have the issue, I'm just proxying for a bug reporter on the 
Gentoo bugzilla. According to him, Jan's patch does solve the problem:

https://bugs.gentoo.org/show_bug.cgi?id=181067#c8

Thanks,
Daniel

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[Daniel Drake]

H. Peter Anvin wrote:
> The patch should be fine for 2.6.22, but for -mm, the code this patches
> doesn't even exist with git-newsetup; nor is there a CONFIG_VIDEO_SELECT
> which could impede the production of the VESA version.
> 
> What does your .config look like?

https://bugs.gentoo.org/attachment.cgi?id=124339&action=view

Daniel

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[H. Peter Anvin]

Daniel Drake wrote:
> H. Peter Anvin wrote:
>> The patch should be fine for 2.6.22, but for -mm, the code this patches
>> doesn't even exist with git-newsetup; nor is there a CONFIG_VIDEO_SELECT
>> which could impede the production of the VESA version.
>>
>> What does your .config look like?
> 
> https://bugs.gentoo.org/attachment.cgi?id=124339&action=view
> 

User error.
Please replace user and press any key.

# CONFIG_FIRMWARE_EDID is not set

The user has *explicitly* disabled acquisition of EDID from the
firmware, so of course it doesn't probe for it.

	-hpa

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[Daniel Drake]

H. Peter Anvin wrote:
> User error.
> Please replace user and press any key.
> 
> # CONFIG_FIRMWARE_EDID is not set
> 
> The user has *explicitly* disabled acquisition of EDID from the
> firmware, so of course it doesn't probe for it.

I'm not versed with all this EDID and resolution stuff, but shouldn't 
the resolution be detected correctly under all configurations?

The original problem is that as of 2.6.20.11, the resolution is 
misdetected - it is wrong by 6 pixels. The same configuration with 
2.6.20.10 works fine.

With the 2.6.20.10/2.6.20.11 configuration, the user did have 
CONFIG_FIRMWARE_EDID set. I'm not sure why he changed this for the -mm 
testing, but then again I don't understand why it should matter.

Daniel

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[H. Peter Anvin]

Daniel Drake wrote:
> H. Peter Anvin wrote:
>> User error.
>> Please replace user and press any key.
>>
>> # CONFIG_FIRMWARE_EDID is not set
>>
>> The user has *explicitly* disabled acquisition of EDID from the
>> firmware, so of course it doesn't probe for it.
> 
> I'm not versed with all this EDID and resolution stuff, but shouldn't
> the resolution be detected correctly under all configurations?

Not if you actively disable detection!

Why would you want to be able to disable detection?  Well, your BIOS
might be buggy.

> The original problem is that as of 2.6.20.11, the resolution is
> misdetected - it is wrong by 6 pixels. The same configuration with
> 2.6.20.10 works fine.

That is because 2.6.20.11 incorrectly disabled EDID detection when a
non-VESA mode was chosen as the initial mode.  This is a bug.

> With the 2.6.20.10/2.6.20.11 configuration, the user did have
> CONFIG_FIRMWARE_EDID set. I'm not sure why he changed this for the -mm
> testing, but then again I don't understand why it should matter.

You're complaining that we didn't query EDID when you *explicitly* asked
us not to.  This is not a bug!

	-hpa

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[Daniel Drake]

H. Peter Anvin wrote:
> You're complaining that we didn't query EDID when you *explicitly* asked
> us not to.  This is not a bug!

I see -- I didn't realise CONFIG_FIRMWARE_EDID was one of options that 
you shouldn't disable unless you have a good reason. Thanks for the 
explanation.

So, 2.6.22-rc6-mm1 should work fine with CONFIG_FIRMWARE_EDID=y, or are 
further patches needed?

Thanks!
Daniel

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[H. Peter Anvin]

Daniel Drake wrote:
> H. Peter Anvin wrote:
>> You're complaining that we didn't query EDID when you *explicitly* asked
>> us not to.  This is not a bug!
> 
> I see -- I didn't realise CONFIG_FIRMWARE_EDID was one of options that
> you shouldn't disable unless you have a good reason. Thanks for the
> explanation.
> 
> So, 2.6.22-rc6-mm1 should work fine with CONFIG_FIRMWARE_EDID=y, or are
> further patches needed?
> 

It should, yes.

	-hpa

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[Daniel Drake]

Hi,

H. Peter Anvin wrote:
>> So, 2.6.22-rc6-mm1 should work fine with CONFIG_FIRMWARE_EDID=y, or are
>> further patches needed?
>>
> 
> It should, yes.

It didn't work, and the bug still exists in 2.6.23-rc1: the resolution 
is wrong by 6 pixels. The user does have CONFIG_FIRMWARE_EDID enabled.

So far the only known working setups since 2.6.20.11 are:
  1. Zwane's patch reverted
  2. Jan's patch applied (to 2.6.22 or lower, I guess it no longer works 
for 2.6.23)

Here is dmesg output from 2.6.23:
http://bugs.gentoo.org/attachment.cgi?id=126203&action=view

Let me know how we can help diagnose this further.

Thanks,
Daniel

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[Zwane Mwaikambo]

Hi Daniel,

On Tue, 31 Jul 2007, Daniel Drake wrote:

> Hi,
> 
> H. Peter Anvin wrote:
> > > So, 2.6.22-rc6-mm1 should work fine with CONFIG_FIRMWARE_EDID=y, or are
> > > further patches needed?
> > > 
> > 
> > It should, yes.
> 
> It didn't work, and the bug still exists in 2.6.23-rc1: the resolution is
> wrong by 6 pixels. The user does have CONFIG_FIRMWARE_EDID enabled.
> 
> So far the only known working setups since 2.6.20.11 are:
>  1. Zwane's patch reverted
>  2. Jan's patch applied (to 2.6.22 or lower, I guess it no longer works for
> 2.6.23)
> 
> Here is dmesg output from 2.6.23:
> http://bugs.gentoo.org/attachment.cgi?id=126203&action=view
> 
> Let me know how we can help diagnose this further.

Sorry if this has been hashed out before, but could you point me towards 
the gentoo bugzilla entry? I'm trying to understand how your setup broke. 
Which version VBE does your system have?

Thanks,
	Zwane

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[Antonino A. Daplas]

On Tue, 2007-07-31 at 19:58 -0400, Daniel Drake wrote:
> Hi,
> 
> H. Peter Anvin wrote:
> >> So, 2.6.22-rc6-mm1 should work fine with CONFIG_FIRMWARE_EDID=y, or are
> >> further patches needed?
> >>
> > 
> > It should, yes.
> 
> It didn't work, and the bug still exists in 2.6.23-rc1: the resolution 
> is wrong by 6 pixels. The user does have CONFIG_FIRMWARE_EDID enabled.
> 
> So far the only known working setups since 2.6.20.11 are:
>   1. Zwane's patch reverted
>   2. Jan's patch applied (to 2.6.22 or lower, I guess it no longer works 
> for 2.6.23)
> 
> Here is dmesg output from 2.6.23:
> http://bugs.gentoo.org/attachment.cgi?id=126203&action=view
> 

The dmesg ouput did show that intelfb was not able to acquire the EDID
from the BIOS, even if CONFIG_FIRMWARE_EDID=y I presume.  I don't think
it's the VBE version, the chipset is an Intel 830 which should at least
be a VBE 2.0.

Is it the same for the working kernel?  Do you have a link to the dmesg
ouput of a working kernel, if possible with #define DEBUG in
drivers/video/fbmon.c.  The output of read-edid or /var/log/X?.log will
also be helpful.

Tony 

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[Daniel Drake]

Zwane Mwaikambo wrote:
> Sorry if this has been hashed out before, but could you point me towards 
> the gentoo bugzilla entry? I'm trying to understand how your setup broke. 
> Which version VBE does your system have?

Here's the bug:
http://bugs.gentoo.org/show_bug.cgi?id=181067

How can we identify the VBE version?

Thanks,
Daniel

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[H. Peter Anvin]

Daniel Drake wrote:
> Zwane Mwaikambo wrote:
>> Sorry if this has been hashed out before, but could you point me 
>> towards the gentoo bugzilla entry? I'm trying to understand how your 
>> setup broke. Which version VBE does your system have?
> 
> Here's the bug:
> http://bugs.gentoo.org/show_bug.cgi?id=181067
> 
> How can we identify the VBE version?
> 
> Thanks,
> Daniel

Just put printf's in video-vesa.c

	-hpa

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[Antonino A. Daplas]

On Tue, 2007-07-31 at 21:17 -0400, Daniel Drake wrote:
> Zwane Mwaikambo wrote:
> > Sorry if this has been hashed out before, but could you point me towards 
> > the gentoo bugzilla entry? I'm trying to understand how your setup broke. 
> > Which version VBE does your system have?
> 
> Here's the bug:
> http://bugs.gentoo.org/show_bug.cgi?id=181067
> 

Looking at the dmesg output of the working and failing kernel, it does
seem that there's no EDID block available in the failing kernel.

> How can we identify the VBE version?

If vesafb works, then it is at least VBE 2.0. To confirm, you can use X
+ the 'vesa' driver and check /var/log/X*.log. 

Tony

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[Gabriel C]

Antonino A. Daplas wrote:
> On Tue, 2007-07-31 at 21:17 -0400, Daniel Drake wrote:
>> Zwane Mwaikambo wrote:
>>> Sorry if this has been hashed out before, but could you point me towards 
>>> the gentoo bugzilla entry? I'm trying to understand how your setup broke. 
>>> Which version VBE does your system have?
>> Here's the bug:
>> http://bugs.gentoo.org/show_bug.cgi?id=181067
>>
> 
> Looking at the dmesg output of the working and failing kernel, it does
> seem that there's no EDID block available in the failing kernel.
> 
>> How can we identify the VBE version?

You could use http://john.fremlin.de/programs/linux/read-edid/ and you get the EDID and VBE version


Regards,

Gabriel

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[Antonino A. Daplas]

On Wed, 2007-08-01 at 09:54 +0800, Antonino A. Daplas wrote:
> On Tue, 2007-07-31 at 21:17 -0400, Daniel Drake wrote:
> > Zwane Mwaikambo wrote:
> > > Sorry if this has been hashed out before, but could you point me towards 
> > > the gentoo bugzilla entry? I'm trying to understand how your setup broke. 
> > > Which version VBE does your system have?
> > 
> > Here's the bug:
> > http://bugs.gentoo.org/show_bug.cgi?id=181067
> > 
> 
> Looking at the dmesg output of the working and failing kernel, it does
> seem that there's no EDID block available in the failing kernel.
> 

BTW, I looked at the above bug report, it seems his last dmesg does not
have fbcon enabled.  Make sure that CONFIG_FRAMEBUFFER_CONSOLE=y before
doing more tests (the problem of lack of the EDID block in the failing
kernel still applies).

Tony

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[H. Peter Anvin]

Antonino A. Daplas wrote:
> On Wed, 2007-08-01 at 09:54 +0800, Antonino A. Daplas wrote:
>> On Tue, 2007-07-31 at 21:17 -0400, Daniel Drake wrote:
>>> Zwane Mwaikambo wrote:
>>>> Sorry if this has been hashed out before, but could you point me towards 
>>>> the gentoo bugzilla entry? I'm trying to understand how your setup broke. 
>>>> Which version VBE does your system have?
>>> Here's the bug:
>>> http://bugs.gentoo.org/show_bug.cgi?id=181067
>>>
>> Looking at the dmesg output of the working and failing kernel, it does
>> seem that there's no EDID block available in the failing kernel.
>>
> 
> BTW, I looked at the above bug report, it seems his last dmesg does not
> have fbcon enabled.  Make sure that CONFIG_FRAMEBUFFER_CONSOLE=y before
> doing more tests (the problem of lack of the EDID block in the failing
> kernel still applies).
> 

Okay, I'm royally puzzled why that would be.  I've gone over the code 
quite a few times, and I do not see any way (other than VESA < 2.0) that 
could cause that.

I look forward to getting the debug output; depending on what it is we 
might have to get some debugging output from the setup code.

We can printf in the new setup code, although obviously that requires 
leaving the screen in text mode.  However, EDID information should still 
be available.

	-hpa

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[Antonino A. Daplas]

H. Peter Anvin wrote:
> Antonino A. Daplas wrote:
>> On Wed, 2007-08-01 at 09:54 +0800, Antonino A. Daplas wrote:
>>> On Tue, 2007-07-31 at 21:17 -0400, Daniel Drake wrote:
>>>> Zwane Mwaikambo wrote:
>>>>> Sorry if this has been hashed out before, but could you point me
>>>>> towards the gentoo bugzilla entry? I'm trying to understand how
>>>>> your setup broke. Which version VBE does your system have?
>>>> Here's the bug:
>>>> http://bugs.gentoo.org/show_bug.cgi?id=181067
>>>>
>>> Looking at the dmesg output of the working and failing kernel, it does
>>> seem that there's no EDID block available in the failing kernel.
>>>
>>
>> BTW, I looked at the above bug report, it seems his last dmesg does not
>> have fbcon enabled.  Make sure that CONFIG_FRAMEBUFFER_CONSOLE=y before
>> doing more tests (the problem of lack of the EDID block in the failing
>> kernel still applies).
>>
> 
> Okay, I'm royally puzzled why that would be.  I've gone over the code
> quite a few times, and I do not see any way (other than VESA < 2.0) that
> could cause that.
> 
> I look forward to getting the debug output; depending on what it is we
> might have to get some debugging output from the setup code.
> 
> We can printf in the new setup code, although obviously that requires
> leaving the screen in text mode.  However, EDID information should still
> be available.
> 
 
How about this patch?

Tony
---

Subject: video setup: Fix VBE DDC reading

Add memory operand constraint and write-only modifier to the inline
assembly to effect the writing of the EDID block to boot_params.edid_info.

Signed-off-by: Antonino Daplas <adaplas@gmail.com>
---

 arch/i386/boot/video-vesa.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/i386/boot/video-vesa.c b/arch/i386/boot/video-vesa.c
index e6aa9eb..f1bc71e 100644
--- a/arch/i386/boot/video-vesa.c
+++ b/arch/i386/boot/video-vesa.c
@@ -268,7 +268,7 @@ #ifdef CONFIG_FIRMWARE_EDID
 	dx = 0;			/* EDID block number */
 	di =(size_t) &boot_params.edid_info; /* (ES:)Pointer to block */
 	asm(INT10
-	    : "+a" (ax), "+b" (bx), "+d" (dx)
+	    : "+a" (ax), "+b" (bx), "+d" (dx), "=m" (boot_params.edid_info)
 	    : "c" (cx), "D" (di)
 	    : "esi");
 #endif /* CONFIG_FIRMWARE_EDID */

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[H. Peter Anvin]

Antonino A. Daplas wrote:

>  
> How about this patch?
> 
> Tony
> ---
> 
> Subject: video setup: Fix VBE DDC reading
> 
> Add memory operand constraint and write-only modifier to the inline
> assembly to effect the writing of the EDID block to boot_params.edid_info.
> 
> Signed-off-by: Antonino Daplas <adaplas@gmail.com>

OK, now I feel dumb.  You're absolutely right; without that constraint, 
the entire EDID function after the memset is eliminated by gcc as being 
dead code.

Will queue the patch.

	-hpa

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[Daniel Drake]

Antonino A. Daplas wrote:
> How about this patch?
> 
> Tony
> ---
> 
> Subject: video setup: Fix VBE DDC reading
> 
> Add memory operand constraint and write-only modifier to the inline
> assembly to effect the writing of the EDID block to boot_params.edid_info.


Thanks, this patch works in that Linux now reports the correct resolution.

However, the TV output is now scrambled...
I suspect this might be a result of adding CONFIG_FRAMEBUFFER_CONSOLE in 
the most recent tests. I've asked the user to test without that option 
but with the patch:

https://bugs.gentoo.org/show_bug.cgi?id=181067#c27

Thanks!
Daniel

Re: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

[Antonino A. Daplas]

On Thu, 2007-08-02 at 22:33 -0400, Daniel Drake wrote:
> Antonino A. Daplas wrote:
> > How about this patch?
> > 
> > Tony
> > ---
> > 
> > Subject: video setup: Fix VBE DDC reading
> > 
> > Add memory operand constraint and write-only modifier to the inline
> > assembly to effect the writing of the EDID block to boot_params.edid_info.
> 
> 
> Thanks, this patch works in that Linux now reports the correct resolution.
> 
> However, the TV output is now scrambled...

The previous problem was just a display shift of 6 pixels, correct?  If
the display is now scrambled, then this is a new problem.

> I suspect this might be a result of adding CONFIG_FRAMEBUFFER_CONSOLE in 
> the most recent tests.

It's useless to unset CONFIG_FRAMEBUFFER_CONSOLE to 'n', he'll just end
up with a 640x400 stretched or windowed display.

Can he...

1. describe what 'scrambled' means?
2. Boot with video=intelfb:accel=0,<old options>?
3. post the output of 'fbset -i' and the latest dmesg?
4. change the color depth ('fbset -depth 16')?
5. If possible, run X using 'fbdev' as the driver at 16 bpp
   (I don't think 32 bpp works with X-fbdev)?

Tony