2007-06-19 16:17:43

by Jan Beulich

[permalink] [raw]
Subject: [PATCH] retrieve VBE EDID/DDC info independent of used video mode

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

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




2007-06-30 22:27:08

by Daniel Drake

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

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 <[email protected]>
>
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2007-06-30 22:44:18

by Andrew Morton

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

On Sat, 30 Jun 2007 18:25:31 -0400 Daniel Drake <[email protected]> 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 <[email protected]>
> >
> > 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 [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
> >

2007-08-02 03:10:42

by Antonino A. Daplas

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

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

2007-08-02 03:58:21

by H. Peter Anvin

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

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

2007-08-02 10:17:04

by Antonino A. Daplas

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

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

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

2007-08-02 13:30:39

by H. Peter Anvin

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

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

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

2007-08-03 02:37:34

by Daniel Drake

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

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

2007-08-03 03:34:18

by Antonino A. Daplas

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

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