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