[SeaBIOS] [PATCH 3/6] vgabios: vbe 15h func returns constructed EDID

[Kevin O'Connor]

On Sat, Sep 01, 2012 at 03:11:40PM +0900, Hiroshi Miura wrote:
> 
>     The Bochs VGA ROM now return an information
>     about a recent WIDE monitor.

Thanks.  See my comments below.

[...]
> -    { 0x187, { MM_DIRECT, 1920, 1200, 16, 8, 16, SEG_GRAPH } },
> -    { 0x188, { MM_DIRECT, 1920, 1200, 24, 8, 16, SEG_GRAPH } },
> -    { 0x189, { MM_DIRECT, 1920, 1200, 32, 8, 16, SEG_GRAPH } },
> -    { 0x18a, { MM_DIRECT, 2560, 1600, 16, 8, 16, SEG_GRAPH } },
> -    { 0x18b, { MM_DIRECT, 2560, 1600, 24, 8, 16, SEG_GRAPH } },
> -    { 0x18c, { MM_DIRECT, 2560, 1600, 32, 8, 16, SEG_GRAPH } },
> +    { 0x187, { MM_DIRECT, 1920, 1080, 16, 8, 16, SEG_GRAPH } },
> +    { 0x188, { MM_DIRECT, 1920, 1080, 24, 8, 16, SEG_GRAPH } },
> +    { 0x189, { MM_DIRECT, 1920, 1080, 32, 8, 16, SEG_GRAPH } },
> +    { 0x18a, { MM_DIRECT, 1920, 1200, 16, 8, 16, SEG_GRAPH } },
> +    { 0x18b, { MM_DIRECT, 1920, 1200, 24, 8, 16, SEG_GRAPH } },
> +    { 0x18c, { MM_DIRECT, 1920, 1200, 32, 8, 16, SEG_GRAPH } },
> +    { 0x18d, { MM_DIRECT, 2560, 1600, 16, 8, 16, SEG_GRAPH } },
> +    { 0x18e, { MM_DIRECT, 2560, 1600, 24, 8, 16, SEG_GRAPH } },
> +    { 0x18f, { MM_DIRECT, 2560, 1600, 32, 8, 16, SEG_GRAPH } },

This appears to be adding three new 1080 height modes.  Unless this
is somehow tied to the edid support, I suggest moving this change to a
separate patch.

[...]
> -/****************************************************************
> - * EDID
> - ****************************************************************/
> -
> -u8 bochsvga_edid[128] VAR16 = {
> -    0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, /* 8-byte header */

This removes a bunch of code added in patch 2.  This makes patch
review harder.  I suggest you merge this patch with patch 2 (and patch
4) to make review easier.

[...]
> --- /dev/null
> +++ b/vgasrc/vbe_edid.h
> @@ -0,0 +1,134 @@
> +#ifndef __VBE_EDID_H
> +#define __VBE_EDID_H
> +
> +#define WORDBE(x) ((((x) & 0xff) << 8 ) | (((x) >> 8) & 0xff))
> +#define DWORDBE(x) (((x) & 0xff) << 24) | ((((x) >> 8) & 0xff) << 16 ) | ((((x) >> 16) & 0xff) << 8 ) | ((((x) >> 24) & 0xff))

We don't need more macros that perform byte swabbing.  Use (and/or
enhance) the code in byteorder.h.

-Kevin