[PATCH 0/7] vgabios improvements

[Kevin O'Connor]

This patch series refactors the vgabios graphics mode framebuffer
manipulation code.  It then adds in support for manipulating "direct"
framebuffers that modern vga modes use.

This series (along with the followup patches for coreboot native vga
vgabios) is also available at:

https://github.com/KevinOConnor/seabios/tree/testing

-Kevin

Kevin O'Connor (7):
  vgabios: Add option to control use of standard VGA IO ports.
  vgabios: Split vgafb_scroll() into separate move and clear functions.
  vgabios: Rewrite vgafb.c graphics operations to set of 4 standard
    operators.
  vgabios: Introduce text_address().
  vgabios: Add support for manipulating framebuffers in high memory.
  Set the color attribute when calling vgabios print character.
  vgabios: PMM scan was incorrectly depending on a zero %ds segment.

src/output.c     |   1 +
 vgasrc/Kconfig   |   8 +
 vgasrc/vgabios.c | 119 ++++++---
 vgasrc/vgabios.h |  34 ++-
 vgasrc/vgafb.c   | 762 +++++++++++++++++++++++++++++--------------------------
 vgasrc/vgainit.c |  15 +-
 6 files changed, 536 insertions(+), 403 deletions(-)

[PATCH 1/7] vgabios: Add option to control use of standard VGA IO ports.

[Kevin O'Connor]

Add option CONFIG_VGA_STDVGA_PORTS.  When this option is disabled, the
main BIOS code will not attempt to access any of the legacy VGA IO
ports.

Signed-off-by: Kevin O'Connor 
---
 vgasrc/Kconfig   |  8 ++++++++
 vgasrc/vgabios.c | 44 ++++++++++++++++++++++++++++++++------------
 vgasrc/vgafb.c   |  8 ++++++++
 vgasrc/vgainit.c |  3 ++-
 4 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/vgasrc/Kconfig b/vgasrc/Kconfig
index b442b3e..f04fca7 100644
--- a/vgasrc/Kconfig
+++ b/vgasrc/Kconfig
@@ -13,6 +13,7 @@ menu "VGA ROM"
         config VGA_STANDARD_VGA
             depends on QEMU
             bool "QEMU/Bochs Original IBM 256K VGA"
+            select VGA_STDVGA_PORTS
             help
                 Build basic VGA BIOS support (pre Super-VGA) for use
                 on emulators.
@@ -20,6 +21,7 @@ menu "VGA ROM"
         config VGA_CIRRUS
             depends on QEMU
             bool "QEMU/Bochs Cirrus SVGA"
+            select VGA_STDVGA_PORTS
             help
                 Build support for Cirrus VGA emulation found on QEMU
                 and Bochs emulators.  This is for emulators; it is not
@@ -28,17 +30,20 @@ menu "VGA ROM"
         config VGA_BOCHS
             depends on QEMU
             bool "QEMU/Bochs VBE SVGA"
+            select VGA_STDVGA_PORTS
             help
                 Build support for Bochs DISPI interface (a custom VBE
                 protocol) found on QEMU and Bochs emulators.

config VGA_GEODEGX2
             bool "GeodeGX2"
+            select VGA_STDVGA_PORTS
             help
                 Build support for Geode GX2 vga.

config VGA_GEODELX
             bool "GeodeLX"
+            select VGA_STDVGA_PORTS
             help
                 Build support for Geode LX vga.
     endchoice
@@ -68,6 +73,9 @@ menu "VGA ROM"
         bool
         default !NO_VGABIOS

+    config VGA_STDVGA_PORTS
+        bool
+
     config VGA_ALLOCATE_EXTRA_STACK
         depends on BUILD_VGABIOS
         bool "Allocate an internal stack for 16bit interrupt entry point"
diff --git a/vgasrc/vgabios.c b/vgasrc/vgabios.c
index 44502b3..040a7ae 100644
--- a/vgasrc/vgabios.c
+++ b/vgasrc/vgabios.c
@@ -59,6 +59,9 @@ set_cursor_shape(u8 start, u8 end)
     u16 curs = (start << 8) + end;
     SET_BDA(cursor_type, curs);

+    if (!CONFIG_VGA_STDVGA_PORTS)
+        return;
+
     u8 modeset_ctl = GET_BDA(modeset_ctl);
     u16 cheight = GET_BDA(char_height);
     if ((modeset_ctl & 0x01) && (cheight > 8) && (end < 8) && (start < 0x20)) {
@@ -92,6 +95,9 @@ set_cursor_pos(struct cursorpos cp)
     // Bios cursor pos
     SET_BDA(cursor_pos[page], (y << 8) | x);

+    if (!CONFIG_VGA_STDVGA_PORTS)
+        return;
+
     // Set the hardware cursor
     u8 current = GET_BDA(video_page);
     if (cp.page != current)
@@ -300,7 +306,7 @@ vga_set_mode(int mode, int flags)
         SET_BDA(cursor_type, 0x0000);
     }
     SET_BDA(video_pagesize, calc_page_size(memmodel, width, height));
-    SET_BDA(crtc_address, stdvga_get_crtc());
+    SET_BDA(crtc_address, CONFIG_VGA_STDVGA_PORTS ? stdvga_get_crtc() : 0);
     SET_BDA(char_height, cheight);
     SET_BDA(video_ctl, 0x60 | (flags & MF_NOCLEARMEM ? 0x80 : 0x00));
     SET_BDA(video_switches, 0xF9);
@@ -485,6 +491,10 @@ handle_100bXX(struct bregs *regs)
 static void
 handle_100b(struct bregs *regs)
 {
+    if (!CONFIG_VGA_STDVGA_PORTS) {
+        handle_100bXX(regs);
+        return;
+    }
     switch (regs->bh) {
     case 0x00: handle_100b00(regs); break;
     case 0x01: handle_100b01(regs); break;
@@ -641,6 +651,10 @@ handle_1010XX(struct bregs *regs)
 static void
 handle_1010(struct bregs *regs)
 {
+    if (!CONFIG_VGA_STDVGA_PORTS) {
+        handle_1010XX(regs);
+        return;
+    }
     switch (regs->al) {
     case 0x00: handle_101000(regs); break;
     case 0x01: handle_101001(regs); break;
@@ -838,16 +852,20 @@ handle_1011XX(struct bregs *regs)
 static void
 handle_1011(struct bregs *regs)
 {
+    if (CONFIG_VGA_STDVGA_PORTS) {
+        switch (regs->al) {
+        case 0x00: handle_101100(regs); break;
+        case 0x01: handle_101101(regs); break;
+        case 0x02: handle_101102(regs); break;
+        case 0x03: handle_101103(regs); break;
+        case 0x04: handle_101104(regs); break;
+        case 0x10: handle_101110(regs); break;
+        case 0x11: handle_101111(regs); break;
+        case 0x12: handle_101112(regs); break;
+        case 0x14: handle_101114(regs); break;
+        }
+    }
     switch (regs->al) {
-    case 0x00: handle_101100(regs); break;
-    case 0x01: handle_101101(regs); break;
-    case 0x02: handle_101102(regs); break;
-    case 0x03: handle_101103(regs); break;
-    case 0x04: handle_101104(regs); break;
-    case 0x10: handle_101110(regs); break;
-    case 0x11: handle_101111(regs); break;
-    case 0x12: handle_101112(regs); break;
-    case 0x14: handle_101114(regs); break;
     case 0x30: handle_101130(regs); break;
     case 0x20: handle_101120(regs); break;
     case 0x21: handle_101121(regs); break;
@@ -912,8 +930,10 @@ handle_101231(struct bregs *regs)
 static void
 handle_101232(struct bregs *regs)
 {
-    stdvga_enable_video_addressing(regs->al);
-    regs->al = 0x12;
+    if (CONFIG_VGA_STDVGA_PORTS) {
+        stdvga_enable_video_addressing(regs->al);
+        regs->al = 0x12;
+    }
 }

static void
diff --git a/vgasrc/vgafb.c b/vgasrc/vgafb.c
index 2efaab5..7c2570f 100644
--- a/vgasrc/vgafb.c
+++ b/vgasrc/vgafb.c
@@ -43,6 +43,8 @@ static void
 scroll_pl4(struct vgamode_s *vmode_g, int nblines, int attr
            , struct cursorpos ul, struct cursorpos lr)
 {
+    if (!CONFIG_VGA_STDVGA_PORTS)
+        return;
     int cheight = GET_BDA(char_height);
     int cwidth = 1;
     int stride = GET_BDA(video_cols) * cwidth;
@@ -225,6 +227,8 @@ static void
 write_gfx_char_pl4(struct vgamode_s *vmode_g
                    , struct cursorpos cp, struct carattr ca)
 {
+    if (!CONFIG_VGA_STDVGA_PORTS)
+        return;
     u16 nbcols = GET_BDA(video_cols);
     if (cp.x >= nbcols)
         return;
@@ -407,6 +411,8 @@ vgafb_write_pixel(u8 color, u16 x, u16 y)
     u8 *addr_far, mask, attr, data, i;
     switch (GET_GLOBAL(vmode_g->memmodel)) {
     case MM_PLANAR:
+        if (!CONFIG_VGA_STDVGA_PORTS)
+            return;
         addr_far = (void*)(x / 8 + y * GET_BDA(video_cols));
         mask = 0x80 >> (x & 0x07);
         for (i=0; i<4; i++) {
@@ -464,6 +470,8 @@ vgafb_read_pixel(u16 x, u16 y)
     u8 *addr_far, mask, attr=0, data, i;
     switch (GET_GLOBAL(vmode_g->memmodel)) {
     case MM_PLANAR:
+        if (!CONFIG_VGA_STDVGA_PORTS)
+            return 0;
         addr_far = (void*)(x / 8 + y * GET_BDA(video_cols));
         mask = 0x80 >> (x & 0x07);
         attr = 0x00;
diff --git a/vgasrc/vgainit.c b/vgasrc/vgainit.c
index 4692c34..33b8eb9 100644
--- a/vgasrc/vgainit.c
+++ b/vgasrc/vgainit.c
@@ -153,7 +153,8 @@ vga_post(struct bregs *regs)

SET_VGA(video_save_pointer_table.videoparam
             , SEGOFF(get_global_seg(), (u32)video_param_table));
-    stdvga_build_video_param();
+    if (CONFIG_VGA_STDVGA_PORTS)
+        stdvga_build_video_param();

extern void entry_10(void);
     SET_IVT(0x10, SEGOFF(get_global_seg(), (u32)entry_10));

[PATCH 2/7] vgabios: Split vgafb_scroll() into separate move and clear functions.

[Kevin O'Connor]

Rewrite the low-level scroll code so that it is implemented using two
basic operations: move text and clear text.  This simplifies the
low-level code as it no longer needs to handle up scrolling vs down
scrolling.  Determining the direction of the scroll is now done in the
higher level (vgabios.c) code.

Signed-off-by: Kevin O'Connor 
---
 vgasrc/vgabios.c |  63 ++++++++++++---
 vgasrc/vgabios.h |   6 +-
 vgasrc/vgafb.c   | 231 +++++++++++++++++++++++++++++--------------------------
 3 files changed, 178 insertions(+), 122 deletions(-)

diff --git a/vgasrc/vgabios.c b/vgasrc/vgabios.c
index 040a7ae..d0efc11 100644
--- a/vgasrc/vgabios.c
+++ b/vgasrc/vgabios.c
@@ -215,9 +215,20 @@ write_teletype(struct cursorpos *pcp, struct carattr ca)
     u16 nbrows = GET_BDA(video_rows);
     if (pcp->y > nbrows) {
         pcp->y--;
-        struct cursorpos ul = {0, 0, pcp->page};
-        struct cursorpos lr = {GET_BDA(video_cols)-1, nbrows, pcp->page};
-        vgafb_scroll(1, -1, ul, lr);
+
+        struct vgamode_s *vmode_g = get_current_mode();
+        if (!vmode_g)
+            return;
+
+        struct cursorpos dest = {0, 0, pcp->page};
+        struct cursorpos src = {0, 1, pcp->page};
+        struct cursorpos size = {GET_BDA(video_cols), nbrows};
+        vgafb_move_chars(vmode_g, dest, src, size);
+
+        struct cursorpos clr = {0, nbrows, pcp->page};
+        struct carattr attr = {' ', 0, 0};
+        struct cursorpos clrsize = {GET_BDA(video_cols), 1};
+        vgafb_clear_chars(vmode_g, clr, attr, clrsize);
     }
 }

@@ -415,18 +426,48 @@ verify_scroll(struct bregs *regs, int dir)
     u16 nbcols = GET_BDA(video_cols);
     if (lrx >= nbcols)
         lrx = nbcols - 1;
-
-    if (ulx > lrx || uly > lry)
+    int wincols = lrx - ulx + 1, winrows = lry - uly + 1;
+    if (wincols <= 0 || winrows <= 0)
         return;

-    int nblines = regs->al;
-    if (!nblines || nblines > lry - uly + 1)
-        nblines = lry - uly + 1;
+    struct vgamode_s *vmode_g = get_current_mode();
+    if (!vmode_g)
+        return;

u8 page = GET_BDA(video_page);
-    struct cursorpos ul = {ulx, uly, page};
-    struct cursorpos lr = {lrx, lry, page};
-    vgafb_scroll(dir * nblines, regs->bh, ul, lr);
+    int clearlines = regs->al, movelines = winrows - clearlines;
+    if (!clearlines || movelines <= 0) {
+        // Clear whole area.
+        struct cursorpos clr = {ulx, uly, page};
+        struct carattr attr = {' ', regs->bh, 1};
+        struct cursorpos clrsize = {wincols, winrows};
+        vgafb_clear_chars(vmode_g, clr, attr, clrsize);
+        return;
+    }
+
+    if (dir > 0) {
+        // Normal scroll
+        struct cursorpos dest = {ulx, uly, page};
+        struct cursorpos src = {ulx, uly + clearlines, page};
+        struct cursorpos size = {wincols, movelines};
+        vgafb_move_chars(vmode_g, dest, src, size);
+
+        struct cursorpos clr = {ulx, uly + movelines, page};
+        struct carattr attr = {' ', regs->bh, 1};
+        struct cursorpos clrsize = {wincols, clearlines};
+        vgafb_clear_chars(vmode_g, clr, attr, clrsize);
+    } else {
+        // Scroll down
+        struct cursorpos dest = {ulx, uly + clearlines, page};
+        struct cursorpos src = {ulx, uly, page};
+        struct cursorpos size = {wincols, movelines};
+        vgafb_move_chars(vmode_g, dest, src, size);
+
+        struct cursorpos clr = {ulx, uly, page};
+        struct carattr attr = {' ', regs->bh, 1};
+        struct cursorpos clrsize = {wincols, clearlines};
+        vgafb_clear_chars(vmode_g, clr, attr, clrsize);
+    }
 }

static void
diff --git a/vgasrc/vgabios.h b/vgasrc/vgabios.h
index 6949560..de6ae59 100644
--- a/vgasrc/vgabios.h
+++ b/vgasrc/vgabios.h
@@ -87,8 +87,10 @@ struct vgamode_s *get_current_mode(void);
 int vga_set_mode(int mode, int flags);

// vgafb.c
-void vgafb_scroll(int nblines, int attr
-                  , struct cursorpos ul, struct cursorpos lr);
+void vgafb_move_chars(struct vgamode_s *vmode_g, struct cursorpos dest
+                      , struct cursorpos src, struct cursorpos movesize);
+void vgafb_clear_chars(struct vgamode_s *vmode_g, struct cursorpos dest
+                       , struct carattr ca, struct cursorpos movesize);
 void vgafb_write_char(struct cursorpos cp, struct carattr ca);
 struct carattr vgafb_read_char(struct cursorpos cp);
 void vgafb_write_pixel(u8 color, u16 x, u16 y);
diff --git a/vgasrc/vgafb.c b/vgasrc/vgafb.c
index 7c2570f..5a2c4c8 100644
--- a/vgasrc/vgafb.c
+++ b/vgasrc/vgafb.c
@@ -17,12 +17,16 @@
  * Screen scrolling
  ****************************************************************/

-static inline void *
-memcpy_stride(u16 seg, void *dst, void *src, int copylen, int stride, int lines)
+static inline void
+memmove_stride(u16 seg, void *dst, void *src, int copylen, int stride, int lines)
 {
+    if (src < dst) {
+        dst += stride * (lines - 1);
+        src += stride * (lines - 1);
+        stride = -stride;
+    }
     for (; lines; lines--, dst+=stride, src+=stride)
         memcpy_far(seg, dst, seg, src, copylen);
-    return dst;
 }

static inline void
@@ -40,163 +44,172 @@ memset16_stride(u16 seg, void *dst, u16 val, int setlen, int stride, int lines)
 }

static void
-scroll_pl4(struct vgamode_s *vmode_g, int nblines, int attr
-           , struct cursorpos ul, struct cursorpos lr)
+planar_move_chars(struct vgamode_s *vmode_g, struct cursorpos dest
+                  , struct cursorpos src, struct cursorpos movesize)
 {
     if (!CONFIG_VGA_STDVGA_PORTS)
         return;
     int cheight = GET_BDA(char_height);
     int cwidth = 1;
     int stride = GET_BDA(video_cols) * cwidth;
-    void *src_far, *dest_far;
-    if (nblines >= 0) {
-        dest_far = (void*)(ul.y * cheight * stride + ul.x * cwidth);
-        src_far = dest_far + nblines * cheight * stride;
-    } else {
-        // Scroll down
-        nblines = -nblines;
-        dest_far = (void*)(lr.y * cheight * stride + ul.x * cwidth);
-        src_far = dest_far - nblines * cheight * stride;
-        stride = -stride;
+    void *dest_far = (void*)(dest.y * cheight * stride + dest.x * cwidth);
+    void *src_far = (void*)(src.y * cheight * stride + src.x * cwidth);
+    int i;
+    for (i=0; i<4; i++) {
+        stdvga_planar4_plane(i);
+        memmove_stride(SEG_GRAPH, dest_far, src_far
+                       , movesize.x * cwidth, stride, movesize.y * cheight);
     }
-    if (attr < 0)
-        attr = 0;
-    int cols = lr.x - ul.x + 1;
-    int rows = lr.y - ul.y + 1;
+    stdvga_planar4_plane(-1);
+}
+
+static void
+planar_clear_chars(struct vgamode_s *vmode_g, struct cursorpos dest
+                   , struct carattr ca, struct cursorpos clearsize)
+{
+    if (!CONFIG_VGA_STDVGA_PORTS)
+        return;
+    int cheight = GET_BDA(char_height);
+    int cwidth = 1;
+    int stride = GET_BDA(video_cols) * cwidth;
+    void *dest_far = (void*)(dest.y * cheight * stride + dest.x * cwidth);
     int i;
     for (i=0; i<4; i++) {
         stdvga_planar4_plane(i);
-        void *dest = dest_far;
-        if (nblines < rows)
-            dest = memcpy_stride(SEG_GRAPH, dest, src_far, cols * cwidth
-                                 , stride, (rows - nblines) * cheight);
-        u8 pixels = (attr & (1<<i)) ? 0xff : 0x00;
-        memset_stride(SEG_GRAPH, dest, pixels, cols * cwidth
-                      , stride, nblines * cheight);
+        u8 attr = (ca.attr & (1<<i)) ? 0xff : 0x00;
+        memset_stride(SEG_GRAPH, dest_far, attr
+                      , clearsize.x * cwidth, stride, clearsize.y * cheight);
     }
     stdvga_planar4_plane(-1);
 }

static void
-scroll_cga(struct vgamode_s *vmode_g, int nblines, int attr
-            , struct cursorpos ul, struct cursorpos lr)
+cga_move_chars(struct vgamode_s *vmode_g, struct cursorpos dest
+               , struct cursorpos src, struct cursorpos movesize)
 {
     int cheight = GET_BDA(char_height) / 2;
     int cwidth = GET_GLOBAL(vmode_g->depth);
     int stride = GET_BDA(video_cols) * cwidth;
-    void *src_far, *dest_far;
-    if (nblines >= 0) {
-        dest_far = (void*)(ul.y * cheight * stride + ul.x * cwidth);
-        src_far = dest_far + nblines * cheight * stride;
-    } else {
-        // Scroll down
-        nblines = -nblines;
-        dest_far = (void*)(lr.y * cheight * stride + ul.x * cwidth);
-        src_far = dest_far - nblines * cheight * stride;
-        stride = -stride;
-    }
-    if (attr < 0)
-        attr = 0;
+    void *dest_far = (void*)(dest.y * cheight * stride + dest.x * cwidth);
+    void *src_far = (void*)(src.y * cheight * stride + src.x * cwidth);
+    memmove_stride(SEG_CTEXT, dest_far, src_far
+                   , movesize.x * cwidth, stride, movesize.y * cheight);
+    memmove_stride(SEG_CTEXT, dest_far + 0x2000, src_far + 0x2000
+                   , movesize.x * cwidth, stride, movesize.y * cheight);
+}
+
+static void
+cga_clear_chars(struct vgamode_s *vmode_g, struct cursorpos dest
+                , struct carattr ca, struct cursorpos clearsize)
+{
+    int cheight = GET_BDA(char_height) / 2;
+    int cwidth = GET_GLOBAL(vmode_g->depth);
+    int stride = GET_BDA(video_cols) * cwidth;
+    void *dest_far = (void*)(dest.y * cheight * stride + dest.x * cwidth);
+    u8 attr = ca.attr;
     if (cwidth == 1)
         attr = (attr&1) | ((attr&1)<<1);
     attr &= 3;
     attr |= (attr<<2) | (attr<<4) | (attr<<6);
-    int cols = lr.x - ul.x + 1;
-    int rows = lr.y - ul.y + 1;
-    if (nblines < rows) {
-        memcpy_stride(SEG_CTEXT, dest_far+0x2000, src_far+0x2000, cols * cwidth
-                      , stride, (rows - nblines) * cheight);
-        dest_far = memcpy_stride(SEG_CTEXT, dest_far, src_far, cols * cwidth
-                                 , stride, (rows - nblines) * cheight);
-    }
-    memset_stride(SEG_CTEXT, dest_far + 0x2000, attr, cols * cwidth
-                  , stride, nblines * cheight);
-    memset_stride(SEG_CTEXT, dest_far, attr, cols * cwidth
-                  , stride, nblines * cheight);
+    memset_stride(SEG_CTEXT, dest_far, attr
+                  , clearsize.x * cwidth, stride, clearsize.y * cheight);
+    memset_stride(SEG_CTEXT, dest_far + 0x2000, attr
+                  , clearsize.x * cwidth, stride, clearsize.y * cheight);
 }

static void
-scroll_lin(struct vgamode_s *vmode_g, int nblines, int attr
-           , struct cursorpos ul, struct cursorpos lr)
+packed_move_chars(struct vgamode_s *vmode_g, struct cursorpos dest
+                  , struct cursorpos src, struct cursorpos movesize)
 {
     int cheight = GET_BDA(char_height);
     int cwidth = 8;
     int stride = GET_BDA(video_cols) * cwidth;
-    void *src_far, *dest_far;
-    if (nblines >= 0) {
-        dest_far = (void*)(ul.y * cheight * stride + ul.x * cwidth);
-        src_far = dest_far + nblines * cheight * stride;
-    } else {
-        // Scroll down
-        nblines = -nblines;
-        dest_far = (void*)(lr.y * cheight * stride + ul.x * cwidth);
-        src_far = dest_far - nblines * cheight * stride;
-        stride = -stride;
-    }
-    if (attr < 0)
-        attr = 0;
-    int cols = lr.x - ul.x + 1;
-    int rows = lr.y - ul.y + 1;
-    if (nblines < rows)
-        dest_far = memcpy_stride(SEG_GRAPH, dest_far, src_far, cols * cwidth
-                                 , stride, (rows - nblines) * cheight);
-    memset_stride(SEG_GRAPH, dest_far, attr, cols * cwidth
-                  , stride, nblines * cheight);
+    void *dest_far = (void*)(dest.y * cheight * stride + dest.x * cwidth);
+    void *src_far = (void*)(src.y * cheight * stride + src.x * cwidth);
+    memmove_stride(SEG_GRAPH, dest_far, src_far
+                   , movesize.x * cwidth, stride, movesize.y * cheight);
 }

static void
-scroll_text(struct vgamode_s *vmode_g, int nblines, int attr
-            , struct cursorpos ul, struct cursorpos lr)
+packed_clear_chars(struct vgamode_s *vmode_g, struct cursorpos dest
+                   , struct carattr ca, struct cursorpos clearsize)
+{
+    int cheight = GET_BDA(char_height);
+    int cwidth = 8;
+    int stride = GET_BDA(video_cols) * cwidth;
+    void *dest_far = (void*)(dest.y * cheight * stride + dest.x * cwidth);
+    memset_stride(SEG_GRAPH, dest_far, ca.attr
+                  , clearsize.x * cwidth, stride, clearsize.y * cheight);
+}
+
+static void
+text_move_chars(struct vgamode_s *vmode_g, struct cursorpos dest
+                , struct cursorpos src, struct cursorpos movesize)
 {
     int cheight = 1;
     int cwidth = 2;
     int stride = GET_BDA(video_cols) * cwidth;
-    void *src_far, *dest_far = (void*)(GET_BDA(video_pagesize) * ul.page);
-    if (nblines >= 0) {
-        dest_far += ul.y * cheight * stride + ul.x * cwidth;
-        src_far = dest_far + nblines * cheight * stride;
-    } else {
-        // Scroll down
-        nblines = -nblines;
-        dest_far += lr.y * cheight * stride + ul.x * cwidth;
-        src_far = dest_far - nblines * cheight * stride;
-        stride = -stride;
-    }
-    if (attr < 0)
-        attr = 0x07;
-    attr = (attr << 8) | ' ';
-    int cols = lr.x - ul.x + 1;
-    int rows = lr.y - ul.y + 1;
+    void *dest_far = (void*)(dest.y * cheight * stride + dest.x * cwidth);
+    void *src_far = (void*)(src.y * cheight * stride + src.x * cwidth);
+    u32 pageoffset = GET_BDA(video_pagesize) * dest.page;
+    u16 seg = GET_GLOBAL(vmode_g->sstart);
+    memmove_stride(seg, dest_far + pageoffset, src_far + pageoffset
+                   , movesize.x * cwidth, stride, movesize.y * cheight);
+}
+
+static void
+text_clear_chars(struct vgamode_s *vmode_g, struct cursorpos dest
+                , struct carattr ca, struct cursorpos clearsize)
+{
+    int cheight = 1;
+    int cwidth = 2;
+    int stride = GET_BDA(video_cols) * cwidth;
+    void *dest_far = (void*)(dest.y * cheight * stride + dest.x * cwidth);
+    u16 attr = ((ca.use_attr ? ca.attr : 0x07) << 8) | ca.car;
+    u32 pageoffset = GET_BDA(video_pagesize) * dest.page;
     u16 seg = GET_GLOBAL(vmode_g->sstart);
-    if (nblines < rows)
-        dest_far = memcpy_stride(seg, dest_far, src_far, cols * cwidth
-                                 , stride, (rows - nblines) * cheight);
-    memset16_stride(seg, dest_far, attr, cols * cwidth
-                    , stride, nblines * cheight);
+    memset16_stride(seg, dest_far + pageoffset, attr
+                    , clearsize.x * cwidth, stride, clearsize.y * cheight);
 }

void
-vgafb_scroll(int nblines, int attr, struct cursorpos ul, struct cursorpos lr)
+vgafb_move_chars(struct vgamode_s *vmode_g, struct cursorpos dest
+                 , struct cursorpos src, struct cursorpos movesize)
 {
-    // Get the mode
-    struct vgamode_s *vmode_g = get_current_mode();
-    if (!vmode_g)
-        return;
+    switch (GET_GLOBAL(vmode_g->memmodel)) {
+    case MM_TEXT:
+        text_move_chars(vmode_g, dest, src, movesize);
+        break;
+    case MM_PLANAR:
+        planar_move_chars(vmode_g, dest, src, movesize);
+        break;
+    case MM_CGA:
+        cga_move_chars(vmode_g, dest, src, movesize);
+        break;
+    case MM_PACKED:
+        packed_move_chars(vmode_g, dest, src, movesize);
+        break;
+    default:
+        break;
+    }
+}

-    // FIXME gfx mode not complete
+void
+vgafb_clear_chars(struct vgamode_s *vmode_g, struct cursorpos dest
+                 , struct carattr ca, struct cursorpos movesize)
+{
     switch (GET_GLOBAL(vmode_g->memmodel)) {
     case MM_TEXT:
-        scroll_text(vmode_g, nblines, attr, ul, lr);
+        text_clear_chars(vmode_g, dest, ca, movesize);
         break;
     case MM_PLANAR:
-        scroll_pl4(vmode_g, nblines, attr, ul, lr);
+        planar_clear_chars(vmode_g, dest, ca, movesize);
         break;
     case MM_CGA:
-        scroll_cga(vmode_g, nblines, attr, ul, lr);
+        cga_clear_chars(vmode_g, dest, ca, movesize);
         break;
-    case MM_DIRECT:
     case MM_PACKED:
-        scroll_lin(vmode_g, nblines, attr, ul, lr);
+        packed_clear_chars(vmode_g, dest, ca, movesize);
         break;
     default:
         break;

[PATCH 3/7] vgabios: Rewrite vgafb.c graphics operations to set of 4 standard operators.

[Kevin O'Connor]

The vgabios graphics manipulations can all be implemented on top of 4
basic primitives: read 8 pixels, write 8 pixels, move pixels, and
clear pixels.  Implement these four operators for all the graphics
modes and rewrite the graphics functions in vgafb.c to use them.  This
simplifies the graphics code as the high level logic no longer needs
to be implemented for each graphical framebuffer type.

Signed-off-by: Kevin O'Connor 
---
 vgasrc/vgabios.h |  26 ++-
 vgasrc/vgafb.c   | 647 ++++++++++++++++++++++++-------------------------------
 2 files changed, 306 insertions(+), 367 deletions(-)

diff --git a/vgasrc/vgabios.h b/vgasrc/vgabios.h
index de6ae59..645e0e2 100644
--- a/vgasrc/vgabios.h
+++ b/vgasrc/vgabios.h
@@ -56,6 +56,28 @@ struct vgamode_s {
     u16 sstart;
 };

+// Graphics pixel operations.
+struct gfx_op {
+    struct vgamode_s *vmode_g;
+    u32 linelength;
+
+    u8 op;
+    u16 x, y;
+
+    u8 pixels[8];
+    u16 xlen, ylen;
+    u16 srcy;
+};
+
+#define GO_READ8   1
+#define GO_WRITE8  2
+#define GO_MEMSET  3
+#define GO_MEMMOVE 4
+
+// Debug settings
+#define DEBUG_VGA_POST 1
+#define DEBUG_VGA_10 3
+
 // vgafonts.c
 extern u8 vgafont8[];
 extern u8 vgafont14[];
@@ -63,10 +85,6 @@ extern u8 vgafont16[];
 extern u8 vgafont14alt[];
 extern u8 vgafont16alt[];

-// Debug settings
-#define DEBUG_VGA_POST 1
-#define DEBUG_VGA_10 3
-
 // vgainit.c
 extern struct VideoSavePointer_s video_save_pointer_table;

diff --git a/vgasrc/vgafb.c b/vgasrc/vgafb.c
index 5a2c4c8..3c6065c 100644
--- a/vgasrc/vgafb.c
+++ b/vgasrc/vgafb.c
@@ -1,6 +1,6 @@
 // Code for manipulating VGA framebuffers.
 //
-// Copyright (C) 2009  Kevin O'Connor 
+// Copyright (C) 2009-2014  Kevin O'Connor 
 // Copyright (C) 2001-2008 the LGPL VGABios developers Team
 //
 // This file may be distributed under the terms of the GNU LGPLv3 license.
@@ -11,11 +11,7 @@
 #include "stdvga.h" // stdvga_planar4_plane
 #include "string.h" // memset_far
 #include "vgabios.h" // vgafb_scroll
-
-
-/****************************************************************
- * Screen scrolling
- ****************************************************************/
+#include "vgahw.h" // vgahw_get_linelength

static inline void
 memmove_stride(u16 seg, void *dst, void *src, int copylen, int stride, int lines)
@@ -43,185 +39,215 @@ memset16_stride(u16 seg, void *dst, u16 val, int setlen, int stride, int lines)
         memset16_far(seg, dst, val, setlen);
 }

-static void
-planar_move_chars(struct vgamode_s *vmode_g, struct cursorpos dest
-                  , struct cursorpos src, struct cursorpos movesize)
-{
-    if (!CONFIG_VGA_STDVGA_PORTS)
-        return;
-    int cheight = GET_BDA(char_height);
-    int cwidth = 1;
-    int stride = GET_BDA(video_cols) * cwidth;
-    void *dest_far = (void*)(dest.y * cheight * stride + dest.x * cwidth);
-    void *src_far = (void*)(src.y * cheight * stride + src.x * cwidth);
-    int i;
-    for (i=0; i<4; i++) {
-        stdvga_planar4_plane(i);
-        memmove_stride(SEG_GRAPH, dest_far, src_far
-                       , movesize.x * cwidth, stride, movesize.y * cheight);
-    }
-    stdvga_planar4_plane(-1);
-}
+
+/****************************************************************
+ * Basic stdvga graphic manipulation
+ ****************************************************************/

static void
-planar_clear_chars(struct vgamode_s *vmode_g, struct cursorpos dest
-                   , struct carattr ca, struct cursorpos clearsize)
+gfx_planar(struct gfx_op *op)
 {
     if (!CONFIG_VGA_STDVGA_PORTS)
         return;
-    int cheight = GET_BDA(char_height);
-    int cwidth = 1;
-    int stride = GET_BDA(video_cols) * cwidth;
-    void *dest_far = (void*)(dest.y * cheight * stride + dest.x * cwidth);
-    int i;
-    for (i=0; i<4; i++) {
-        stdvga_planar4_plane(i);
-        u8 attr = (ca.attr & (1<<i)) ? 0xff : 0x00;
-        memset_stride(SEG_GRAPH, dest_far, attr
-                      , clearsize.x * cwidth, stride, clearsize.y * cheight);
+    void *dest_far = (void*)(op->y * op->linelength + op->x / 8);
+    int plane;
+    switch (op->op) {
+    default:
+    case GO_READ8:
+        memset(op->pixels, 0, sizeof(op->pixels));
+        for (plane = 0; plane < 4; plane++) {
+            stdvga_planar4_plane(plane);
+            u8 data = GET_FARVAR(SEG_GRAPH, *(u8*)dest_far);
+            int pixel;
+            for (pixel=0; pixel<8; pixel++)
+                op->pixels[pixel] |= ((data>>(7-pixel)) & 1) << plane;
+        }
+        break;
+    case GO_WRITE8:
+        for (plane = 0; plane<4; plane++) {
+            stdvga_planar4_plane(plane);
+            u8 data = 0;
+            int pixel;
+            for (pixel=0; pixel<8; pixel++)
+                data |= ((op->pixels[pixel]>>plane) & 1) << (7-pixel);
+            SET_FARVAR(SEG_GRAPH, *(u8*)dest_far, data);
+        }
+        break;
+    case GO_MEMSET:
+        for (plane = 0; plane < 4; plane++) {
+            stdvga_planar4_plane(plane);
+            u8 data = (op->pixels[0] & (1<<plane)) ? 0xff : 0x00;
+            memset_stride(SEG_GRAPH, dest_far, data
+                          , op->xlen / 8, op->linelength, op->ylen);
+        }
+        break;
+    case GO_MEMMOVE: ;
+        void *src_far = (void*)(op->srcy * op->linelength + op->x / 8);
+        for (plane = 0; plane < 4; plane++) {
+            stdvga_planar4_plane(plane);
+            memmove_stride(SEG_GRAPH, dest_far, src_far
+                           , op->xlen / 8, op->linelength, op->ylen);
+        }
+        break;
     }
     stdvga_planar4_plane(-1);
 }

static void
-cga_move_chars(struct vgamode_s *vmode_g, struct cursorpos dest
-               , struct cursorpos src, struct cursorpos movesize)
+gfx_cga(struct gfx_op *op)
 {
-    int cheight = GET_BDA(char_height) / 2;
-    int cwidth = GET_GLOBAL(vmode_g->depth);
-    int stride = GET_BDA(video_cols) * cwidth;
-    void *dest_far = (void*)(dest.y * cheight * stride + dest.x * cwidth);
-    void *src_far = (void*)(src.y * cheight * stride + src.x * cwidth);
-    memmove_stride(SEG_CTEXT, dest_far, src_far
-                   , movesize.x * cwidth, stride, movesize.y * cheight);
-    memmove_stride(SEG_CTEXT, dest_far + 0x2000, src_far + 0x2000
-                   , movesize.x * cwidth, stride, movesize.y * cheight);
+    int bpp = GET_GLOBAL(op->vmode_g->depth);
+    void *dest_far = (void*)(op->y / 2 * op->linelength + op->x / 8 * bpp);
+    switch (op->op) {
+    default:
+    case GO_READ8:
+        if (op->y & 1)
+            dest_far += 0x2000;
+        if (bpp == 1) {
+            u8 data = GET_FARVAR(SEG_CTEXT, *(u8*)dest_far);
+            int pixel;
+            for (pixel=0; pixel<8; pixel++)
+                op->pixels[pixel] = (data >> (7-pixel)) & 1;
+        } else {
+            u16 data = GET_FARVAR(SEG_CTEXT, *(u16*)dest_far);
+            data = be16_to_cpu(data);
+            int pixel;
+            for (pixel=0; pixel<8; pixel++)
+                op->pixels[pixel] = (data >> ((7-pixel)*2)) & 3;
+        }
+        break;
+    case GO_WRITE8:
+        if (op->y & 1)
+            dest_far += 0x2000;
+        if (bpp == 1) {
+            u8 data = 0;
+            int pixel;
+            for (pixel=0; pixel<8; pixel++)
+                data |= (op->pixels[pixel] & 1) << (7-pixel);
+            SET_FARVAR(SEG_CTEXT, *(u8*)dest_far, data);
+        } else {
+            u16 data = 0;
+            int pixel;
+            for (pixel=0; pixel<8; pixel++)
+                data |= (op->pixels[pixel] & 3) << ((7-pixel) * 2);
+            data = cpu_to_be16(data);
+            SET_FARVAR(SEG_CTEXT, *(u16*)dest_far, data);
+        }
+        break;
+    case GO_MEMSET: ;
+        u8 data = op->pixels[0];
+        if (bpp == 1)
+            data = (data&1) | ((data&1)<<1);
+        data &= 3;
+        data |= (data<<2) | (data<<4) | (data<<6);
+        memset_stride(SEG_CTEXT, dest_far, data
+                      , op->xlen / 8 * bpp, op->linelength, op->ylen / 2);
+        memset_stride(SEG_CTEXT, dest_far + 0x2000, data
+                      , op->xlen / 8 * bpp, op->linelength, op->ylen / 2);
+        break;
+    case GO_MEMMOVE: ;
+        void *src_far = (void*)(op->srcy / 2 * op->linelength + op->x / 8 * bpp);
+        memmove_stride(SEG_CTEXT, dest_far, src_far
+                       , op->xlen / 8 * bpp, op->linelength, op->ylen / 2);
+        memmove_stride(SEG_CTEXT, dest_far + 0x2000, src_far + 0x2000
+                       , op->xlen / 8 * bpp, op->linelength, op->ylen / 2);
+        break;
+    }
 }

static void
-cga_clear_chars(struct vgamode_s *vmode_g, struct cursorpos dest
-                , struct carattr ca, struct cursorpos clearsize)
+gfx_packed(struct gfx_op *op)
 {
-    int cheight = GET_BDA(char_height) / 2;
-    int cwidth = GET_GLOBAL(vmode_g->depth);
-    int stride = GET_BDA(video_cols) * cwidth;
-    void *dest_far = (void*)(dest.y * cheight * stride + dest.x * cwidth);
-    u8 attr = ca.attr;
-    if (cwidth == 1)
-        attr = (attr&1) | ((attr&1)<<1);
-    attr &= 3;
-    attr |= (attr<<2) | (attr<<4) | (attr<<6);
-    memset_stride(SEG_CTEXT, dest_far, attr
-                  , clearsize.x * cwidth, stride, clearsize.y * cheight);
-    memset_stride(SEG_CTEXT, dest_far + 0x2000, attr
-                  , clearsize.x * cwidth, stride, clearsize.y * cheight);
+    void *dest_far = (void*)(op->y * op->linelength + op->x);
+    switch (op->op) {
+    default:
+    case GO_READ8:
+        memcpy_far(GET_SEG(SS), op->pixels, SEG_GRAPH, dest_far, 8);
+        break;
+    case GO_WRITE8:
+        memcpy_far(SEG_GRAPH, dest_far, GET_SEG(SS), op->pixels, 8);
+        break;
+    case GO_MEMSET:
+        memset_stride(SEG_GRAPH, dest_far, op->pixels[0]
+                      , op->xlen, op->linelength, op->ylen);
+        break;
+    case GO_MEMMOVE: ;
+        void *src_far = (void*)(op->srcy * op->linelength + op->x);
+        memmove_stride(SEG_GRAPH, dest_far, src_far
+                       , op->xlen, op->linelength, op->ylen);
+        break;
+    }
 }

-static void
-packed_move_chars(struct vgamode_s *vmode_g, struct cursorpos dest
-                  , struct cursorpos src, struct cursorpos movesize)
-{
-    int cheight = GET_BDA(char_height);
-    int cwidth = 8;
-    int stride = GET_BDA(video_cols) * cwidth;
-    void *dest_far = (void*)(dest.y * cheight * stride + dest.x * cwidth);
-    void *src_far = (void*)(src.y * cheight * stride + src.x * cwidth);
-    memmove_stride(SEG_GRAPH, dest_far, src_far
-                   , movesize.x * cwidth, stride, movesize.y * cheight);
-}

-static void
-packed_clear_chars(struct vgamode_s *vmode_g, struct cursorpos dest
-                   , struct carattr ca, struct cursorpos clearsize)
-{
-    int cheight = GET_BDA(char_height);
-    int cwidth = 8;
-    int stride = GET_BDA(video_cols) * cwidth;
-    void *dest_far = (void*)(dest.y * cheight * stride + dest.x * cwidth);
-    memset_stride(SEG_GRAPH, dest_far, ca.attr
-                  , clearsize.x * cwidth, stride, clearsize.y * cheight);
-}
+/****************************************************************
+ * Gfx interface
+ ****************************************************************/

+// Prepare a struct gfx_op for use.
 static void
-text_move_chars(struct vgamode_s *vmode_g, struct cursorpos dest
-                , struct cursorpos src, struct cursorpos movesize)
+init_gfx_op(struct gfx_op *op, struct vgamode_s *vmode_g)
 {
-    int cheight = 1;
-    int cwidth = 2;
-    int stride = GET_BDA(video_cols) * cwidth;
-    void *dest_far = (void*)(dest.y * cheight * stride + dest.x * cwidth);
-    void *src_far = (void*)(src.y * cheight * stride + src.x * cwidth);
-    u32 pageoffset = GET_BDA(video_pagesize) * dest.page;
-    u16 seg = GET_GLOBAL(vmode_g->sstart);
-    memmove_stride(seg, dest_far + pageoffset, src_far + pageoffset
-                   , movesize.x * cwidth, stride, movesize.y * cheight);
+    memset(op, 0, sizeof(*op));
+    op->vmode_g = vmode_g;
+    op->linelength = vgahw_get_linelength(vmode_g);
 }

+// Issue a graphics operation.
 static void
-text_clear_chars(struct vgamode_s *vmode_g, struct cursorpos dest
-                , struct carattr ca, struct cursorpos clearsize)
+handle_gfx_op(struct gfx_op *op)
 {
-    int cheight = 1;
-    int cwidth = 2;
-    int stride = GET_BDA(video_cols) * cwidth;
-    void *dest_far = (void*)(dest.y * cheight * stride + dest.x * cwidth);
-    u16 attr = ((ca.use_attr ? ca.attr : 0x07) << 8) | ca.car;
-    u32 pageoffset = GET_BDA(video_pagesize) * dest.page;
-    u16 seg = GET_GLOBAL(vmode_g->sstart);
-    memset16_stride(seg, dest_far + pageoffset, attr
-                    , clearsize.x * cwidth, stride, clearsize.y * cheight);
-}
-
-void
-vgafb_move_chars(struct vgamode_s *vmode_g, struct cursorpos dest
-                 , struct cursorpos src, struct cursorpos movesize)
-{
-    switch (GET_GLOBAL(vmode_g->memmodel)) {
-    case MM_TEXT:
-        text_move_chars(vmode_g, dest, src, movesize);
-        break;
+    switch (GET_GLOBAL(op->vmode_g->memmodel)) {
     case MM_PLANAR:
-        planar_move_chars(vmode_g, dest, src, movesize);
+        gfx_planar(op);
         break;
     case MM_CGA:
-        cga_move_chars(vmode_g, dest, src, movesize);
+        gfx_cga(op);
         break;
     case MM_PACKED:
-        packed_move_chars(vmode_g, dest, src, movesize);
+        gfx_packed(op);
         break;
     default:
         break;
     }
 }

-void
-vgafb_clear_chars(struct vgamode_s *vmode_g, struct cursorpos dest
-                 , struct carattr ca, struct cursorpos movesize)
+// Move characters when in graphics mode.
+static void
+gfx_move_chars(struct vgamode_s *vmode_g, struct cursorpos dest
+               , struct cursorpos src, struct cursorpos movesize)
 {
-    switch (GET_GLOBAL(vmode_g->memmodel)) {
-    case MM_TEXT:
-        text_clear_chars(vmode_g, dest, ca, movesize);
-        break;
-    case MM_PLANAR:
-        planar_clear_chars(vmode_g, dest, ca, movesize);
-        break;
-    case MM_CGA:
-        cga_clear_chars(vmode_g, dest, ca, movesize);
-        break;
-    case MM_PACKED:
-        packed_clear_chars(vmode_g, dest, ca, movesize);
-        break;
-    default:
-        break;
-    }
+    struct gfx_op op;
+    init_gfx_op(&op, vmode_g);
+    op.x = dest.x * 8;
+    op.xlen = movesize.x * 8;
+    int cheight = GET_BDA(char_height);
+    op.y = dest.y * cheight;
+    op.ylen = movesize.y * cheight;
+    op.srcy = src.y * cheight;
+    op.op = GO_MEMMOVE;
+    handle_gfx_op(&op);
 }

+// Clear are of screen in graphics mode.
+static void
+gfx_clear_chars(struct vgamode_s *vmode_g, struct cursorpos dest
+                , struct carattr ca, struct cursorpos clearsize)
+{
+    struct gfx_op op;
+    init_gfx_op(&op, vmode_g);
+    op.x = dest.x * 8;
+    op.xlen = clearsize.x * 8;
+    int cheight = GET_BDA(char_height);
+    op.y = dest.y * cheight;
+    op.ylen = clearsize.y * cheight;
+    op.pixels[0] = ca.attr;
+    op.op = GO_MEMSET;
+    handle_gfx_op(&op);
+}

-/****************************************************************
- * Read/write characters to screen
- ****************************************************************/
-
-static struct segoff_s
+// Return the font for a given character
+struct segoff_s
 get_font_data(u8 c)
 {
     int char_height = GET_BDA(char_height);
@@ -236,153 +262,159 @@ get_font_data(u8 c)
     return font;
 }

+// Write a character to the screen in graphics mode.
 static void
-write_gfx_char_pl4(struct vgamode_s *vmode_g
-                   , struct cursorpos cp, struct carattr ca)
+gfx_write_char(struct vgamode_s *vmode_g
+                , struct cursorpos cp, struct carattr ca)
 {
-    if (!CONFIG_VGA_STDVGA_PORTS)
-        return;
-    u16 nbcols = GET_BDA(video_cols);
-    if (cp.x >= nbcols)
+    if (cp.x >= GET_BDA(video_cols))
         return;

struct segoff_s font = get_font_data(ca.car);
+    struct gfx_op op;
+    init_gfx_op(&op, vmode_g);
+    op.x = cp.x * 8;
     int cheight = GET_BDA(char_height);
-    int cwidth = 1;
-    int stride = nbcols * cwidth;
-    int addr = cp.y * cheight * stride + cp.x * cwidth;
+    op.y = cp.y * cheight;
+    int usexor = ca.attr & 0x80 && GET_GLOBAL(vmode_g->depth) < 8;
     int i;
-    for (i=0; i<4; i++) {
-        stdvga_planar4_plane(i);
-        u8 colors = ((ca.attr & (1<<i)) ? 0xff : 0x00);
-        int j;
-        for (j = 0; j < cheight; j++) {
-            u8 *dest_far = (void*)(addr + j * stride);
-            u8 fontline = GET_FARVAR(font.seg, *(u8*)(font.offset+j));
-            u8 pixels = colors & fontline;
-            if (ca.attr & 0x80)
-                pixels ^= GET_FARVAR(SEG_GRAPH, *dest_far);
-            SET_FARVAR(SEG_GRAPH, *dest_far, pixels);
+    for (i = 0; i < cheight; i++, op.y++) {
+        u8 fontline = GET_FARVAR(font.seg, *(u8*)(font.offset+i));
+        if (usexor) {
+            op.op = GO_READ8;
+            handle_gfx_op(&op);
+            int j;
+            for (j = 0; j < 8; j++)
+                op.pixels[j] ^= (fontline & (0x80>>j)) ? (ca.attr & 0x7f) : 0x00;
+        } else {
+            int j;
+            for (j = 0; j < 8; j++)
+                op.pixels[j] = (fontline & (0x80>>j)) ? ca.attr : 0x00;
         }
+        op.op = GO_WRITE8;
+        handle_gfx_op(&op);
     }
-    stdvga_planar4_plane(-1);
 }

-static void
-write_gfx_char_cga(struct vgamode_s *vmode_g
-                   , struct cursorpos cp, struct carattr ca)
+// Set the pixel at the given position.
+void
+vgafb_write_pixel(u8 color, u16 x, u16 y)
 {
-    u16 nbcols = GET_BDA(video_cols);
-    if (cp.x >= nbcols)
+    struct vgamode_s *vmode_g = get_current_mode();
+    if (!vmode_g)
         return;

-    struct segoff_s font = get_font_data(ca.car);
-    int cheight = GET_BDA(char_height) / 2;
-    int cwidth = GET_GLOBAL(vmode_g->depth);
-    int stride = nbcols * cwidth;
-    int addr = cp.y * cheight * stride + cp.x * cwidth;
-    int i;
-    for (i = 0; i < cheight*2; i++) {
-        u8 *dest_far = (void*)(addr + (i >> 1) * stride);
-        if (i & 1)
-            dest_far += 0x2000;
-        u8 fontline = GET_FARVAR(font.seg, *(u8*)(font.offset+i));
-        if (cwidth == 1) {
-            u8 colors = (ca.attr & 0x01) ? 0xff : 0x00;
-            u8 pixels = colors & fontline;
-            if (ca.attr & 0x80)
-                pixels ^= GET_FARVAR(SEG_GRAPH, *dest_far);
-            SET_FARVAR(SEG_CTEXT, *dest_far, pixels);
-        } else {
-            u16 fontline16 = ((fontline & 0xf0) << 4) | (fontline & 0x0f);
-            fontline16 = ((fontline16 & 0x0c0c) << 2) | (fontline16 & 0x0303);
-            fontline16 = ((fontline16 & 0x2222) << 1) | (fontline16 & 0x1111);
-            fontline16 |= fontline16<<1;
-            u16 colors = (((ca.attr & 0x01) ? 0x5555 : 0x0000)
-                          | ((ca.attr & 0x02) ? 0xaaaa : 0x0000));
-            u16 pixels = cpu_to_be16(colors & fontline16);
-            if (ca.attr & 0x80)
-                pixels ^= GET_FARVAR(SEG_GRAPH, *(u16*)dest_far);
-            SET_FARVAR(SEG_CTEXT, *(u16*)dest_far, pixels);
-        }
-    }
+    struct gfx_op op;
+    init_gfx_op(&op, vmode_g);
+    op.x = ALIGN_DOWN(x, 8);
+    op.y = y;
+    op.op = GO_READ8;
+    handle_gfx_op(&op);
+
+    int usexor = color & 0x80 && GET_GLOBAL(vmode_g->depth) < 8;
+    if (usexor)
+        op.pixels[x & 0x07] ^= color & 0x7f;
+    else
+        op.pixels[x & 0x07] = color;
+    op.op = GO_WRITE8;
+    handle_gfx_op(&op);
 }

-static void
-write_gfx_char_lin(struct vgamode_s *vmode_g
-                   , struct cursorpos cp, struct carattr ca)
+// Return the pixel at the given position.
+u8
+vgafb_read_pixel(u16 x, u16 y)
 {
-    // Get the dimensions
-    u16 nbcols = GET_BDA(video_cols);
-    if (cp.x >= nbcols)
-        return;
+    struct vgamode_s *vmode_g = get_current_mode();
+    if (!vmode_g)
+        return 0;

-    struct segoff_s font = get_font_data(ca.car);
-    int cheight = GET_BDA(char_height);
-    int cwidth = 8;
-    int stride = nbcols * cwidth;
-    int addr = cp.y * cheight * stride + cp.x * cwidth;
-    int i;
-    for (i = 0; i < cheight; i++) {
-        u8 *dest_far = (void*)(addr + i * stride);
-        u8 fontline = GET_FARVAR(font.seg, *(u8*)(font.offset+i));
-        int j;
-        for (j = 0; j < 8; j++) {
-            u8 pixel = (fontline & (0x80>>j)) ? ca.attr : 0x00;
-            SET_FARVAR(SEG_GRAPH, dest_far[j], pixel);
-        }
-    }
+    struct gfx_op op;
+    init_gfx_op(&op, vmode_g);
+    op.x = ALIGN_DOWN(x, 8);
+    op.y = y;
+    op.op = GO_READ8;
+    handle_gfx_op(&op);
+
+    return op.pixels[x & 0x07];
 }

-static void
-write_text_char(struct vgamode_s *vmode_g
-                , struct cursorpos cp, struct carattr ca)
+
+/****************************************************************
+ * Text ops
+ ****************************************************************/
+
+// Move characters on screen.
+void
+vgafb_move_chars(struct vgamode_s *vmode_g, struct cursorpos dest
+                 , struct cursorpos src, struct cursorpos movesize)
 {
+    if (GET_GLOBAL(vmode_g->memmodel) != MM_TEXT) {
+        gfx_move_chars(vmode_g, dest, src, movesize);
+        return;
+    }
+
     int cheight = 1;
     int cwidth = 2;
     int stride = GET_BDA(video_cols) * cwidth;
-    int addr = cp.y * cheight * stride + cp.x * cwidth;
-    void *dest_far = (void*)(GET_BDA(video_pagesize) * cp.page + addr);
-    if (ca.use_attr) {
-        u16 dummy = (ca.attr << 8) | ca.car;
-        SET_FARVAR(GET_GLOBAL(vmode_g->sstart), *(u16*)dest_far, dummy);
-    } else {
-        SET_FARVAR(GET_GLOBAL(vmode_g->sstart), *(u8*)dest_far, ca.car);
+    void *dest_far = (void*)(dest.y * cheight * stride + dest.x * cwidth);
+    void *src_far = (void*)(src.y * cheight * stride + src.x * cwidth);
+    u32 pageoffset = GET_BDA(video_pagesize) * dest.page;
+    u16 seg = GET_GLOBAL(vmode_g->sstart);
+    memmove_stride(seg, dest_far + pageoffset, src_far + pageoffset
+                   , movesize.x * cwidth, stride, movesize.y * cheight);
+}
+
+// Clear are of screen.
+void
+vgafb_clear_chars(struct vgamode_s *vmode_g, struct cursorpos dest
+                  , struct carattr ca, struct cursorpos clearsize)
+{
+    if (GET_GLOBAL(vmode_g->memmodel) != MM_TEXT) {
+        gfx_clear_chars(vmode_g, dest, ca, clearsize);
+        return;
     }
+
+    int cheight = 1;
+    int cwidth = 2;
+    int stride = GET_BDA(video_cols) * cwidth;
+    void *dest_far = (void*)(dest.y * cheight * stride + dest.x * cwidth);
+    u16 attr = ((ca.use_attr ? ca.attr : 0x07) << 8) | ca.car;
+    u32 pageoffset = GET_BDA(video_pagesize) * dest.page;
+    u16 seg = GET_GLOBAL(vmode_g->sstart);
+    memset16_stride(seg, dest_far + pageoffset, attr
+                    , clearsize.x * cwidth, stride, clearsize.y * cheight);
 }

+// Write a character to the screen.
 void
 vgafb_write_char(struct cursorpos cp, struct carattr ca)
 {
-    // Get the mode
     struct vgamode_s *vmode_g = get_current_mode();
     if (!vmode_g)
         return;

-    // FIXME gfx mode not complete
-    switch (GET_GLOBAL(vmode_g->memmodel)) {
-    case MM_TEXT:
-        write_text_char(vmode_g, cp, ca);
-        break;
-    case MM_PLANAR:
-        write_gfx_char_pl4(vmode_g, cp, ca);
-        break;
-    case MM_CGA:
-        write_gfx_char_cga(vmode_g, cp, ca);
-        break;
-    case MM_DIRECT:
-    case MM_PACKED:
-        write_gfx_char_lin(vmode_g, cp, ca);
-        break;
-    default:
-        break;
+    if (GET_GLOBAL(vmode_g->memmodel) != MM_TEXT) {
+        gfx_write_char(vmode_g, cp, ca);
+        return;
+    }
+
+    int cheight = 1;
+    int cwidth = 2;
+    int stride = GET_BDA(video_cols) * cwidth;
+    int addr = cp.y * cheight * stride + cp.x * cwidth;
+    void *dest_far = (void*)(GET_BDA(video_pagesize) * cp.page + addr);
+    if (ca.use_attr) {
+        u16 dummy = (ca.attr << 8) | ca.car;
+        SET_FARVAR(GET_GLOBAL(vmode_g->sstart), *(u16*)dest_far, dummy);
+    } else {
+        SET_FARVAR(GET_GLOBAL(vmode_g->sstart), *(u8*)dest_far, ca.car);
     }
 }

+// Return the character at the given position on the screen.
 struct carattr
 vgafb_read_char(struct cursorpos cp)
 {
-    // Get the mode
     struct vgamode_s *vmode_g = get_current_mode();
     if (!vmode_g)
         goto fail;
@@ -393,7 +425,6 @@ vgafb_read_char(struct cursorpos cp)
         goto fail;
     }

-    // Compute the address
     int cheight = 1;
     int cwidth = 2;
     int stride = GET_BDA(video_cols) * cwidth;
@@ -407,113 +438,3 @@ fail: ;
     struct carattr ca2 = {0, 0, 0};
     return ca2;
 }
-
-
-/****************************************************************
- * Read/write pixels
- ****************************************************************/
-
-void
-vgafb_write_pixel(u8 color, u16 x, u16 y)
-{
-    // Get the mode
-    struct vgamode_s *vmode_g = get_current_mode();
-    if (!vmode_g)
-        return;
-
-    u8 *addr_far, mask, attr, data, i;
-    switch (GET_GLOBAL(vmode_g->memmodel)) {
-    case MM_PLANAR:
-        if (!CONFIG_VGA_STDVGA_PORTS)
-            return;
-        addr_far = (void*)(x / 8 + y * GET_BDA(video_cols));
-        mask = 0x80 >> (x & 0x07);
-        for (i=0; i<4; i++) {
-            stdvga_planar4_plane(i);
-            u8 colors = (color & (1<<i)) ? 0xff : 0x00;
-            u8 orig = GET_FARVAR(SEG_GRAPH, *addr_far);
-            if (color & 0x80)
-                colors ^= orig;
-            SET_FARVAR(SEG_GRAPH, *addr_far, (colors & mask) | (orig & ~mask));
-        }
-        stdvga_planar4_plane(-1);
-        break;
-    case MM_CGA:
-        if (GET_GLOBAL(vmode_g->depth) == 2)
-            addr_far = (void*)((x >> 2) + (y >> 1) * 80);
-        else
-            addr_far = (void*)((x >> 3) + (y >> 1) * 80);
-        if (y & 1)
-            addr_far += 0x2000;
-        data = GET_FARVAR(SEG_CTEXT, *addr_far);
-        if (GET_GLOBAL(vmode_g->depth) == 2) {
-            attr = (color & 0x03) << ((3 - (x & 0x03)) * 2);
-            mask = 0x03 << ((3 - (x & 0x03)) * 2);
-        } else {
-            attr = (color & 0x01) << (7 - (x & 0x07));
-            mask = 0x01 << (7 - (x & 0x07));
-        }
-        if (color & 0x80) {
-            data ^= attr;
-        } else {
-            data &= ~mask;
-            data |= attr;
-        }
-        SET_FARVAR(SEG_CTEXT, *addr_far, data);
-        break;
-    case MM_DIRECT:
-    case MM_PACKED:
-        addr_far = (void*)(x + y * (GET_BDA(video_cols) * 8));
-        SET_FARVAR(SEG_GRAPH, *addr_far, color);
-        break;
-    default:
-    case MM_TEXT:
-        return;
-    }
-}
-
-u8
-vgafb_read_pixel(u16 x, u16 y)
-{
-    // Get the mode
-    struct vgamode_s *vmode_g = get_current_mode();
-    if (!vmode_g)
-        return 0;
-
-    u8 *addr_far, mask, attr=0, data, i;
-    switch (GET_GLOBAL(vmode_g->memmodel)) {
-    case MM_PLANAR:
-        if (!CONFIG_VGA_STDVGA_PORTS)
-            return 0;
-        addr_far = (void*)(x / 8 + y * GET_BDA(video_cols));
-        mask = 0x80 >> (x & 0x07);
-        attr = 0x00;
-        for (i = 0; i < 4; i++) {
-            stdvga_planar4_plane(i);
-            data = GET_FARVAR(SEG_GRAPH, *addr_far) & mask;
-            if (data > 0)
-                attr |= (0x01 << i);
-        }
-        stdvga_planar4_plane(-1);
-        break;
-    case MM_CGA:
-        addr_far = (void*)((x >> 2) + (y >> 1) * 80);
-        if (y & 1)
-            addr_far += 0x2000;
-        data = GET_FARVAR(SEG_CTEXT, *addr_far);
-        if (GET_GLOBAL(vmode_g->depth) == 2)
-            attr = (data >> ((3 - (x & 0x03)) * 2)) & 0x03;
-        else
-            attr = (data >> (7 - (x & 0x07))) & 0x01;
-        break;
-    case MM_DIRECT:
-    case MM_PACKED:
-        addr_far = (void*)(x + y * (GET_BDA(video_cols) * 8));
-        attr = GET_FARVAR(SEG_GRAPH, *addr_far);
-        break;
-    default:
-    case MM_TEXT:
-        return 0;
-    }
-    return attr;
-}

[PATCH 4/7] vgabios: Introduce text_address().

[Kevin O'Connor]

Factor out code that calculates the text mode address of a given
character.

Signed-off-by: Kevin O'Connor 
---
 vgasrc/vgabios.c | 12 ++++--------
 vgasrc/vgabios.h |  1 +
 vgasrc/vgafb.c   | 47 +++++++++++++++++++----------------------------
 3 files changed, 24 insertions(+), 36 deletions(-)

diff --git a/vgasrc/vgabios.c b/vgasrc/vgabios.c
index d0efc11..400e296 100644
--- a/vgasrc/vgabios.c
+++ b/vgasrc/vgabios.c
@@ -104,9 +104,7 @@ set_cursor_pos(struct cursorpos cp)
         return;

// Calculate the memory address
-    int address = (GET_BDA(video_pagesize) * page
-                   + (x + y * GET_BDA(video_cols)) * 2);
-    stdvga_set_cursor_pos(address);
+    stdvga_set_cursor_pos((int)text_address(cp));
 }

static struct cursorpos
@@ -136,11 +134,9 @@ set_active_page(u8 page)
     if (!vmode_g)
         return;

-    // Get cursor pos for the given page
-    struct cursorpos cp = get_cursor_pos(page);
-
     // Calculate memory address of start of page
-    int address = GET_BDA(video_pagesize) * page;
+    struct cursorpos cp = {0, 0, page};
+    int address = (int)text_address(cp);
     vgahw_set_displaystart(vmode_g, address);

// And change the BIOS page
@@ -150,7 +146,7 @@ set_active_page(u8 page)
     dprintf(1, "Set active page %02x address %04x\n", page, address);

// Display the cursor, now the page is active
-    set_cursor_pos(cp);
+    set_cursor_pos(get_cursor_pos(page));
 }

static void
diff --git a/vgasrc/vgabios.h b/vgasrc/vgabios.h
index 645e0e2..73c0697 100644
--- a/vgasrc/vgabios.h
+++ b/vgasrc/vgabios.h
@@ -105,6 +105,7 @@ struct vgamode_s *get_current_mode(void);
 int vga_set_mode(int mode, int flags);

// vgafb.c
+void *text_address(struct cursorpos cp);
 void vgafb_move_chars(struct vgamode_s *vmode_g, struct cursorpos dest
                       , struct cursorpos src, struct cursorpos movesize);
 void vgafb_clear_chars(struct vgamode_s *vmode_g, struct cursorpos dest
diff --git a/vgasrc/vgafb.c b/vgasrc/vgafb.c
index 3c6065c..31bf5de 100644
--- a/vgasrc/vgafb.c
+++ b/vgasrc/vgafb.c
@@ -343,6 +343,15 @@ vgafb_read_pixel(u16 x, u16 y)
  * Text ops
  ****************************************************************/

+// Return the fb offset for the given character address when in text mode.
+void *
+text_address(struct cursorpos cp)
+{
+    int stride = GET_BDA(video_cols) * 2;
+    u32 pageoffset = GET_BDA(video_pagesize) * cp.page;
+    return (void*)pageoffset + cp.y * stride + cp.x * 2;
+}
+
 // Move characters on screen.
 void
 vgafb_move_chars(struct vgamode_s *vmode_g, struct cursorpos dest
@@ -353,15 +362,10 @@ vgafb_move_chars(struct vgamode_s *vmode_g, struct cursorpos dest
         return;
     }

-    int cheight = 1;
-    int cwidth = 2;
-    int stride = GET_BDA(video_cols) * cwidth;
-    void *dest_far = (void*)(dest.y * cheight * stride + dest.x * cwidth);
-    void *src_far = (void*)(src.y * cheight * stride + src.x * cwidth);
-    u32 pageoffset = GET_BDA(video_pagesize) * dest.page;
-    u16 seg = GET_GLOBAL(vmode_g->sstart);
-    memmove_stride(seg, dest_far + pageoffset, src_far + pageoffset
-                   , movesize.x * cwidth, stride, movesize.y * cheight);
+    int stride = GET_BDA(video_cols) * 2;
+    memmove_stride(GET_GLOBAL(vmode_g->sstart)
+                   , text_address(dest), text_address(src)
+                   , movesize.x * 2, stride, movesize.y);
 }

// Clear are of screen.
@@ -374,15 +378,10 @@ vgafb_clear_chars(struct vgamode_s *vmode_g, struct cursorpos dest
         return;
     }

-    int cheight = 1;
-    int cwidth = 2;
-    int stride = GET_BDA(video_cols) * cwidth;
-    void *dest_far = (void*)(dest.y * cheight * stride + dest.x * cwidth);
+    int stride = GET_BDA(video_cols) * 2;
     u16 attr = ((ca.use_attr ? ca.attr : 0x07) << 8) | ca.car;
-    u32 pageoffset = GET_BDA(video_pagesize) * dest.page;
-    u16 seg = GET_GLOBAL(vmode_g->sstart);
-    memset16_stride(seg, dest_far + pageoffset, attr
-                    , clearsize.x * cwidth, stride, clearsize.y * cheight);
+    memset16_stride(GET_GLOBAL(vmode_g->sstart), text_address(dest), attr
+                    , clearsize.x * 2, stride, clearsize.y);
 }

// Write a character to the screen.
@@ -398,11 +397,7 @@ vgafb_write_char(struct cursorpos cp, struct carattr ca)
         return;
     }

-    int cheight = 1;
-    int cwidth = 2;
-    int stride = GET_BDA(video_cols) * cwidth;
-    int addr = cp.y * cheight * stride + cp.x * cwidth;
-    void *dest_far = (void*)(GET_BDA(video_pagesize) * cp.page + addr);
+    void *dest_far = text_address(cp);
     if (ca.use_attr) {
         u16 dummy = (ca.attr << 8) | ca.car;
         SET_FARVAR(GET_GLOBAL(vmode_g->sstart), *(u16*)dest_far, dummy);
@@ -425,12 +420,8 @@ vgafb_read_char(struct cursorpos cp)
         goto fail;
     }

-    int cheight = 1;
-    int cwidth = 2;
-    int stride = GET_BDA(video_cols) * cwidth;
-    int addr = cp.y * cheight * stride + cp.x * cwidth;
-    u16 *src_far = (void*)(GET_BDA(video_pagesize) * cp.page + addr);
-    u16 v = GET_FARVAR(GET_GLOBAL(vmode_g->sstart), *src_far);
+    u16 *dest_far = text_address(cp);
+    u16 v = GET_FARVAR(GET_GLOBAL(vmode_g->sstart), *dest_far);
     struct carattr ca = {v, v>>8, 0};
     return ca;

[PATCH 5/7] vgabios: Add support for manipulating framebuffers in high memory.

[Kevin O'Connor]

Add code for manipulating "direct" style linear RGB framebuffers that
may be in high memory.

Signed-off-by: Kevin O'Connor 
---
 vgasrc/vgabios.h |   1 +
 vgasrc/vgafb.c   | 111 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 112 insertions(+)

diff --git a/vgasrc/vgabios.h b/vgasrc/vgabios.h
index 73c0697..8fe65d4 100644
--- a/vgasrc/vgabios.h
+++ b/vgasrc/vgabios.h
@@ -60,6 +60,7 @@ struct vgamode_s {
 struct gfx_op {
     struct vgamode_s *vmode_g;
     u32 linelength;
+    u32 displaystart;

u8 op;
     u16 x, y;
diff --git a/vgasrc/vgafb.c b/vgasrc/vgafb.c
index 31bf5de..b90a956 100644
--- a/vgasrc/vgafb.c
+++ b/vgasrc/vgafb.c
@@ -181,6 +181,113 @@ gfx_packed(struct gfx_op *op)

/****************************************************************
+ * Direct framebuffers in high mem
+ ****************************************************************/
+
+// Use int 1587 call to copy memory to/from the framebuffer.
+static void
+memcpy_high(void *dest, void *src, u32 len)
+{
+    u64 gdt[6];
+    gdt[2] = GDT_DATA | GDT_LIMIT(0xfffff) | GDT_BASE((u32)src);
+    gdt[3] = GDT_DATA | GDT_LIMIT(0xfffff) | GDT_BASE((u32)dest);
+
+    // Call int 1587 to copy data.
+    len/=2;
+    u32 flags;
+    u32 eax = 0x8700;
+    u32 si = (u32)&gdt;
+    SET_SEG(ES, GET_SEG(SS));
+    asm volatile(
+        "stc\n"
+        "int $0x15\n"
+        "cli\n"
+        "cld\n"
+        "pushfl\n"
+        "popl %0\n"
+        : "=r" (flags), "+a" (eax), "+S" (si), "+c" (len)
+        : : "cc", "memory");
+}
+
+static void
+memmove_stride_high(void *dst, void *src, int copylen, int stride, int lines)
+{
+    if (src < dst) {
+        dst += stride * (lines - 1);
+        src += stride * (lines - 1);
+        stride = -stride;
+    }
+    for (; lines; lines--, dst+=stride, src+=stride)
+        memcpy_high(dst, src, copylen);
+}
+
+// Map a CGA color to a "direct" mode rgb value.
+static u32
+get_color(int depth, u8 attr)
+{
+    int rbits, gbits, bbits;
+    switch (depth) {
+    case 15: rbits=5; gbits=5; bbits=5; break;
+    case 16: rbits=5; gbits=6; bbits=5; break;
+    default:
+    case 24: rbits=8; gbits=8; bbits=8; break;
+    }
+    int h = (attr&8) ? 1 : 0;
+    int r = (attr&4) ? 2 : 0, g = (attr&2) ? 2 : 0, b = (attr&1) ? 2 : 0;
+    if ((attr & 0xf) == 6)
+        g = 1;
+    return ((((((1<<rbits) - 1) * (r + h) + 1) / 3) << (gbits+bbits))
+            + (((((1<<gbits) - 1) * (g + h) + 1) / 3) << bbits)
+            + ((((1<<bbits) - 1) * (b + h) + 1) / 3));
+}
+
+static void
+gfx_direct(struct gfx_op *op)
+{
+    void *fb = (void*)GET_GLOBAL(VBE_framebuffer);
+    if (!fb)
+        return;
+    int depth = GET_GLOBAL(op->vmode_g->depth);
+    int bypp = DIV_ROUND_UP(depth, 8);
+    void *dest_far = (fb + op->displaystart + op->y * op->linelength
+                      + op->x * bypp);
+    switch (op->op) {
+    default:
+    case GO_READ8:
+        // XXX - not implemented.
+        break;
+    case GO_WRITE8: {
+        u8 data[64];
+        int i;
+        for (i=0; i<8; i++)
+            *(u32*)&data[i*bypp] = get_color(depth, op->pixels[i]);
+        memcpy_high(dest_far, MAKE_FLATPTR(GET_SEG(SS), data), bypp * 8);
+        break;
+    }
+    case GO_MEMSET: {
+        u32 color = get_color(depth, op->pixels[0]);
+        u8 data[64];
+        int i;
+        for (i=0; i<8; i++)
+            *(u32*)&data[i*bypp] = color;
+        memcpy_high(dest_far, MAKE_FLATPTR(GET_SEG(SS), data), bypp * 8);
+        memcpy_high(dest_far + bypp * 8, dest_far, op->xlen * bypp - bypp * 8);
+        for (i=1; i < op->ylen; i++)
+            memcpy_high(dest_far + op->linelength * i
+                        , dest_far, op->xlen * bypp);
+        break;
+    }
+    case GO_MEMMOVE: ;
+        void *src_far = (fb + op->displaystart + op->srcy * op->linelength
+                         + op->x * bypp);
+        memmove_stride_high(dest_far, src_far
+                            , op->xlen * bypp, op->linelength, op->ylen);
+        break;
+    }
+}
+
+
+/****************************************************************
  * Gfx interface
  ****************************************************************/

@@ -191,6 +298,7 @@ init_gfx_op(struct gfx_op *op, struct vgamode_s *vmode_g)
     memset(op, 0, sizeof(*op));
     op->vmode_g = vmode_g;
     op->linelength = vgahw_get_linelength(vmode_g);
+    op->displaystart = vgahw_get_displaystart(vmode_g);
 }

// Issue a graphics operation.
@@ -207,6 +315,9 @@ handle_gfx_op(struct gfx_op *op)
     case MM_PACKED:
         gfx_packed(op);
         break;
+    case MM_DIRECT:
+        gfx_direct(op);
+        break;
     default:
         break;
     }

[PATCH 6/7] Set the color attribute when calling vgabios print character.

[Kevin O'Connor]

Set the color attribute in case the SeaBIOS console code is used while
the vgabios is in a graphics mode.

Signed-off-by: Kevin O'Connor 
---
 src/output.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/output.c b/src/output.c
index 994a1d6..45397b3 100644
--- a/src/output.c
+++ b/src/output.c
@@ -76,6 +76,7 @@ screenc(char c)
     br.flags = F_IF;
     br.ah = 0x0e;
     br.al = c;
+    br.bl = 0x07;
     call16_int(0x10, &br);
 }

[PATCH 7/7] vgabios: PMM scan was incorrectly depending on a zero %ds segment.

[Kevin O'Connor]

Make sure the PMM scanning code uses the GET_FARVAR macro.  (The
existing code only worked because SeaBIOS happens to call the vgabios
in bigreal mode with %ds == %ss = 0.)  Also, the scan doesn't require
bigreal mode - use accesses relative to the SEG_BIOS segment so that
the scan can work in regular real mode.

Signed-off-by: Kevin O'Connor 
---
 vgasrc/vgainit.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/vgasrc/vgainit.c b/vgasrc/vgainit.c
index 33b8eb9..6d9a224 100644
--- a/vgasrc/vgainit.c
+++ b/vgasrc/vgainit.c
@@ -60,14 +60,14 @@ allocate_extra_stack(void)
 {
     if (!CONFIG_VGA_ALLOCATE_EXTRA_STACK)
         return;
-    void *pmmscan = (void*)BUILD_BIOS_ADDR;
-    for (; pmmscan < (void*)BUILD_BIOS_ADDR+BUILD_BIOS_SIZE; pmmscan+=16) {
-        struct pmmheader *pmm = pmmscan;
-        if (pmm->signature != PMM_SIGNATURE)
+    u32 pmmscan;
+    for (pmmscan=0; pmmscan < BUILD_BIOS_SIZE; pmmscan+=16) {
+        struct pmmheader *pmm = (void*)pmmscan;
+        if (GET_FARVAR(SEG_BIOS, pmm->signature) != PMM_SIGNATURE)
             continue;
-        if (checksum_far(0, pmm, pmm->length))
+        if (checksum_far(SEG_BIOS, pmm, GET_FARVAR(SEG_BIOS, pmm->length)))
             continue;
-        struct segoff_s entry = pmm->entry;
+        struct segoff_s entry = GET_FARVAR(SEG_BIOS, pmm->entry);
         dprintf(1, "Attempting to allocate VGA stack via pmm call to %04x:%04x\n"
                 , entry.seg, entry.offset);
         u16 res1, res2;

[Kevin O'Connor]

On Sun, Apr 06, 2014 at 06:59:53PM -0400, Kevin O'Connor wrote:
> This patch series refactors the vgabios graphics mode framebuffer
> manipulation code.  It then adds in support for manipulating "direct"
> framebuffers that modern vga modes use.
>
> This series (along with the followup patches for coreboot native vga
> vgabios) is also available at:
>
> https://github.com/KevinOConnor/seabios/tree/testing

FYI, I have pushed this patch series (as well as the follow up
coreboot native vgabios series) to seabios master.

-Kevin

[Gerd Hoffmann]

On Fr, 2014-04-11 at 11:31 -0400, Kevin O'Connor wrote:
> On Sun, Apr 06, 2014 at 06:59:53PM -0400, Kevin O'Connor wrote:
> > This patch series refactors the vgabios graphics mode framebuffer
> > manipulation code.  It then adds in support for manipulating "direct"
> > framebuffers that modern vga modes use.
> >
> > This series (along with the followup patches for coreboot native vga
> > vgabios) is also available at:
> >
> > https://github.com/KevinOConnor/seabios/tree/testing
>
> FYI, I have pushed this patch series (as well as the follow up
> coreboot native vgabios series) to seabios master.

Some configs fail to build, seems to be the logging:

CONFIG_COREBOOT=y
CONFIG_QEMU_HARDWARE=y
CONFIG_VGA_COREBOOT=y
CONFIG_DEBUG_IO=y

out/vgaccode16.o: In function `runningOnQEMU':
/home/kraxel/projects/seabios/src/fw/paravirt.h:18: undefined reference
to `PlatformRunningOn'

Flipping CONFIG_DEBUG_IO to 'n' makes it build fine.

cheers,
  Gerd

[Kevin O'Connor]

On Mon, Apr 14, 2014 at 11:59:13AM +0200, Gerd Hoffmann wrote:
> On Fr, 2014-04-11 at 11:31 -0400, Kevin O'Connor wrote:
> > On Sun, Apr 06, 2014 at 06:59:53PM -0400, Kevin O'Connor wrote:
> > > This patch series refactors the vgabios graphics mode framebuffer
> > > manipulation code.  It then adds in support for manipulating "direct"
> > > framebuffers that modern vga modes use.
> > >
> > > This series (along with the followup patches for coreboot native vga
> > > vgabios) is also available at:
> > >
> > > https://github.com/KevinOConnor/seabios/tree/testing
> >
> > FYI, I have pushed this patch series (as well as the follow up
> > coreboot native vgabios series) to seabios master.
>
> Some configs fail to build, seems to be the logging:
>
> CONFIG_COREBOOT=y
> CONFIG_QEMU_HARDWARE=y
> CONFIG_VGA_COREBOOT=y
> CONFIG_DEBUG_IO=y
>
> out/vgaccode16.o: In function `runningOnQEMU':
> /home/kraxel/projects/seabios/src/fw/paravirt.h:18: undefined reference
> to `PlatformRunningOn'
>
> Flipping CONFIG_DEBUG_IO to 'n' makes it build fine.

Thanks.  Looks like that set of options would have broken the build
even prior to the vga patch series.  I can fix the compilation with
the patch below, but it wont actually permit the vgabios to use the
debug_io support.  (Getting the debug_io support to work with those
options would be a pain - it would require linking in paravirt.c and
making the qemu detection code work in 16bit mode or making the
vgabios init run in 32bit mode.)

-Kevin
> From fe2cbf6d606e138055b00b0aac6162503a5fb38d Mon Sep 17 00:00:00 2001

Message-Id: 
From: Kevin O'Connor 
Date: Mon, 14 Apr 2014 10:46:34 -0400
Subject: [PATCH] vgabios: Define PlatformRunningOn to make compile happy.
To: [e-mail]

With CONFIG_COREBOOT=y, CONFIG_QEMU_HARDWARE=y, CONFIG_VGA_COREBOOT=y,
CONFIG_DEBUG_IO=y the compile would break because this variable was
missing.

Signed-off-by: Kevin O'Connor 
---
 vgasrc/vgainit.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/vgasrc/vgainit.c b/vgasrc/vgainit.c
index 6d9a224..9d6dd1e 100644
--- a/vgasrc/vgainit.c
+++ b/vgasrc/vgainit.c
@@ -32,6 +32,9 @@ struct VideoSavePointer_s video_save_pointer_table VAR16;

struct VideoParam_s video_param_table[29] VAR16;

+// Type of emulator platform - for dprintf with certain compile options.
+int PlatformRunningOn VAR16;
+

/****************************************************************
  * PCI Data

[Gerd Hoffmann]

Hi,
> Thanks.  Looks like that set of options would have broken the build
> even prior to the vga patch series.

There was no reason to try though ;)
> I can fix the compilation with
> the patch below, but it wont actually permit the vgabios to use the
> debug_io support.

Reasonable.

cheers,
  Gerd

[Gerd Hoffmann]

On Fr, 2014-04-11 at 11:31 -0400, Kevin O'Connor wrote:
> On Sun, Apr 06, 2014 at 06:59:53PM -0400, Kevin O'Connor wrote:
> > This patch series refactors the vgabios graphics mode framebuffer
> > manipulation code.  It then adds in support for manipulating "direct"
> > framebuffers that modern vga modes use.
> >
> > This series (along with the followup patches for coreboot native vga
> > vgabios) is also available at:
> >
> > https://github.com/KevinOConnor/seabios/tree/testing
>
> FYI, I have pushed this patch series (as well as the follow up
> coreboot native vgabios series) to seabios master.

Updated the firmware builder to include this.  If you wanna try things
out in qemu install coreboot.git package from the firmware builder repo
(http://www.kraxel.org/repos/).   It's enabled for the stdvga:

coreboot-i440fx-seabios.rom: 256 kB, bootblocksize 944, romsize 262144,
offset 0x0
alignment: 64 bytes

Name                           Offset     Type         Size
cmos_layout.bin                0x0        cmos_layout  1160
pci1234,1111.rom               0x4c0      optionrom    26112
fallback/romstage              0x6b00     stage        17654
fallback/coreboot_ram          0xb040     stage        57426
fallback/payload               0x19100    payload      58083
config                         0x27440    raw          3391
(empty)                        0x281c0    null         96792

So you can try this:

qemu -vga std -bios /usr/share/coreboot.git/coreboot-i440fx-seabios.rom

to see it live in action.

Two problems spotted so far:

(1) ipxe hangs at rom load time.
      can be worked around by adding '-net none' to the qemu cmd line.

(2) color attributes don't work correctly.  Result is that you can't
      figure which line in your boot menu is highlighted.  Visible in
      Fedora live isos (which use isolinux as boot loader) I think.

cheers,
  Gerd

[Kevin O'Connor]

On Mon, Apr 14, 2014 at 02:22:51PM +0200, Gerd Hoffmann wrote:
> So you can try this:
>
> qemu -vga std -bios /usr/share/coreboot.git/coreboot-i440fx-seabios.rom
>
> to see it live in action.
>
> Two problems spotted so far:
>
> (1) ipxe hangs at rom load time.
>       can be worked around by adding '-net none' to the qemu cmd line.

I didn't try your firmware, but I did try building coreboot and
seabios from source.  I can't reproduce this ipxe failure.  (Though, I
realize now that the seavgabios call to int 1587 may transition the
cpu from bigreal mode to regular real mode and confuse ipxe.)

Oddly, I can't boot from a hard drive (-hda) when I use the
coreboot/seabios I built though.  It keeps failing at "Booting from
Hard Disk...".

I'll have to look at this further.  If you have any tips on the above
let me know.  (Otherwise, I'll see if I can work through the options
you have in your repos.)
> (2) color attributes don't work correctly.  Result is that you can't
>       figure which line in your boot menu is highlighted.  Visible in
>       Fedora live isos (which use isolinux as boot loader) I think.

This isn't surprising as the vgabios handling of attributes in text
mode is different from the handling in graphics mode.  It may be
possible to tweak seavgabios so it tries to emulate the text mode
attr handling.

-Kevin

[Gerd Hoffmann]

Hi,
> Oddly, I can't boot from a hard drive (-hda) when I use the
> coreboot/seabios I built though.  It keeps failing at "Booting from
> Hard Disk...".

I see that with my builds too, but only with -vga std (where coreboot
vgabios is used).  Same bios image with -vga cirrus works fine.  So it
seems this is triggered by the coreboot vgabios too.
> > (2) color attributes don't work correctly.  Result is that you can't
> >       figure which line in your boot menu is highlighted.  Visible in
> >       Fedora live isos (which use isolinux as boot loader) I think.
>
> This isn't surprising as the vgabios handling of attributes in text
> mode is different from the handling in graphics mode.  It may be
> possible to tweak seavgabios so it tries to emulate the text mode
> attr handling.

Ok, so simply not implemented.

cheers,
  Gerd

[Kevin O'Connor]

On Tue, Apr 15, 2014 at 07:52:47AM +0200, Gerd Hoffmann wrote:
> Hi,
> > Oddly, I can't boot from a hard drive (-hda) when I use the
> > coreboot/seabios I built though.  It keeps failing at "Booting from
> > Hard Disk...".
>
> I see that with my builds too, but only with -vga std (where coreboot
> vgabios is used).  Same bios image with -vga cirrus works fine.  So it
> seems this is triggered by the coreboot vgabios too.

I tracked down the failures I was seeing - my coreboot builds had
CONFIG_ATA_DMA enabled and that doesn't work with coreboot on qemu.
For some reason, coreboot doesn't enable the memory or io bits in the
pci config space.  The seabios patch below fixes it for me, but I'm
not sure if seabios is the right place to fix it.  In any case, I
think I'll just stop using CONFIG_ATA_DMA for my coreboot builds.

-Kevin

--- a/src/hw/ata.c
+++ b/src/hw/ata.c
@@ -950,7 +950,7 @@ init_pciata(struct pci_device *pci, u8 prog_if)
         u32 bar = pci_config_readl(bdf, PCI_BASE_ADDRESS_4);
         if (bar & PCI_BASE_ADDRESS_SPACE_IO) {
             master = bar & PCI_BASE_ADDRESS_IO_MASK;
-            pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER);
+            pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER | PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
         }
     }

[Gerd Hoffmann]

Hi,
> --- a/src/hw/ata.c
> +++ b/src/hw/ata.c
>        pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER);
>        pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER | PCI_COMMAND_IO | PCI_COMMAND_MEMORY);

Ah.  I think that should go in.  If we need it, we better make sure it
is enabled instead of expecting someone else to do it.  It's more
robust, even if it should not be needed in theory.

That patch also makes CONFIG_ATA_DMA=y on CONFIG_QEMU=y builds work.

cheers,
  Gerd

[Kevin O'Connor]

On Wed, Apr 16, 2014 at 06:17:45PM +0200, Gerd Hoffmann wrote:
> Hi,
> > --- a/src/hw/ata.c
> > +++ b/src/hw/ata.c
> >        pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER);
> >        pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER | PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
>
> Ah.  I think that should go in.  If we need it, we better make sure it
> is enabled instead of expecting someone else to do it.  It's more
> robust, even if it should not be needed in theory.
>
> That patch also makes CONFIG_ATA_DMA=y on CONFIG_QEMU=y builds work.

On CONFIG_QEMU=y, pciinit.c turns those bits on, so it isn't needed in
ata.c.  On real hardware coreboot used to enable those bits, as I used
to use CONFIG_ATA_DMA=y on my boards.  (Recently, all my boards have
AHCI, so I use that instead and I'm not sure I can retest on real
hardware.)

In any case, I guess there is no harm in doing the above.  It's just
odd that coreboot doesn't turn it on.  That said, CONFIG_ATA_DMA is
very fragile and it may be worth just removing at some point.

-Kevin

[Gerd Hoffmann]

Hi,
> That said, CONFIG_ATA_DMA is
> very fragile and it may be worth just removing at some point.

Why is it fragile?

cheers,
  Gerd

[Kevin O'Connor]

On Wed, Apr 16, 2014 at 06:48:43PM +0200, Gerd Hoffmann wrote:
> Hi,
> > That said, CONFIG_ATA_DMA is
> > very fragile and it may be worth just removing at some point.
>
> Why is it fragile?

On PATA you're supposed to do all these weird controller specific
stuff to make sure DMA works, to set what level of DMA to use, and to
tell the drive that DMA will be used.  The SeaBIOS code does none of
that.  This is why CONFIG_ATA_DMA is disabled by default.

Now that I think about it though, the code is likely okay for SATA
drives attached to SATA controllers that use ATA emulation.

-Kevin

[Peter Stuge]

Gerd Hoffmann wrote:
> >        pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER);
> >        pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER | PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
>
> I think that should go in.  If we need it, we better make sure it
> is enabled instead of expecting someone else to do it.

I think this reasoning is completely broken; what you are saying is
that SeaBIOS should in principle do the complete hardware init.

It is IMO broken enough that SeaBIOS does so for QEMU - please don't
push that into SeaBIOS built for coreboot, but fix the actual
problem where it exists (in coreboot), instead of making a workaround
in SeaBIOS under the guise of "we need it so we must do it ourselves"

It sounds like NIH syndrome. :\

//Peter

[H. Peter Anvin]

On 04/16/2014 03:59 PM, Peter Stuge wrote:
> Gerd Hoffmann wrote:
> > >        pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER);
> > >        pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER | PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
> >
> > I think that should go in.  If we need it, we better make sure it
> > is enabled instead of expecting someone else to do it.
>
> I think this reasoning is completely broken; what you are saying is
> that SeaBIOS should in principle do the complete hardware init.
>
> It is IMO broken enough that SeaBIOS does so for QEMU - please don't
> push that into SeaBIOS built for coreboot, but fix the actual
> problem where it exists (in coreboot), instead of making a workaround
> in SeaBIOS under the guise of "we need it so we must do it ourselves"
>
> It sounds like NIH syndrome. :\

Uh... seriously, robustness is a good thing, and it is not entirely
clear that this responsibility necessarily belongs in Coreboot.

But especially if you are touching the command register anyway, the cost
is literally zero to just do it.

-hpa

[Peter Stuge]

H. Peter Anvin wrote:
> Uh... seriously, robustness is a good thing, and it is not entirely
> clear that this responsibility necessarily belongs in Coreboot.

It's clear to me. The coreboot (all lowercase please) code for QEMU
isn't neccessarily complete, and needs these kinds of improvements.
> But especially if you are touching the command register anyway,

SeaBIOS should perhaps not. That's not so clear to me however. :)

//Peter

[Kevin O'Connor]

On Mon, Apr 14, 2014 at 02:22:51PM +0200, Gerd Hoffmann wrote:
> So you can try this:
>
> qemu -vga std -bios /usr/share/coreboot.git/coreboot-i440fx-seabios.rom
>
> to see it live in action.
>
> Two problems spotted so far:
>
> (1) ipxe hangs at rom load time.
>       can be worked around by adding '-net none' to the qemu cmd line.

I've reproduced this.  It only fails for me with -enable-kvm.  It was
(as I feared) the result of segment limits getting trashed from the
int 1587 call.  (Presumably, qemu tcg doesn't fail because it doesn't
implement segment limits?)  The (incorrect) patch below enables the
boot to proceed past the ipxe prompt.

This is unfortunate.  I can put a hack into seabios (not seavgabios)
to use bigreal mode for int 1587 calls during option rom execution.
But, it does raise the question of how many other callers expect the
bios to not mess with the segment limits.  (Though, to be honest, the
only goal I have with coreboot native seavgabios is support for grub,
lilo, syslinux, and maybe ntldr.)

-Kevin

--- a/src/system.c
+++ b/src/system.c
@@ -115,11 +115,11 @@ handle_1587(struct bregs *regs)
     SET_FARVAR(gdt_seg, gdt_far[1], GDT_DATA | GDT_LIMIT((6*sizeof(u64))-1)
                | GDT_BASE(loc));
     // Initialize CS descriptor
-    SET_FARVAR(gdt_seg, gdt_far[4], GDT_CODE | GDT_LIMIT(BUILD_BIOS_SIZE-1)
+    SET_FARVAR(gdt_seg, gdt_far[4], GDT_CODE | GDT_GRANLIMIT(0xffffffff)
                | GDT_BASE(BUILD_BIOS_ADDR));
     // Initialize SS descriptor
     loc = (u32)MAKE_FLATPTR(GET_SEG(SS), 0);
-    SET_FARVAR(gdt_seg, gdt_far[5], GDT_DATA | GDT_LIMIT(0x0ffff)
+    SET_FARVAR(gdt_seg, gdt_far[5], GDT_DATA | GDT_GRANLIMIT(0xffffffff)
                | GDT_BASE(loc));

u16 count = regs->cx;

[Gerd Hoffmann]

Hi,
> But, it does raise the question of how many other callers expect the
> bios to not mess with the segment limits.  (Though, to be honest, the
> only goal I have with coreboot native seavgabios is support for grub,
> lilo, syslinux, and maybe ntldr.)

Yes, it should be good enough for bootloaders.

Emulating attributes would be great so you can actually navigate
bootloader menus.

And we don't have a cursor I've noticed meanwhile.  That would be very
helpful for editing your linux cmd line in the boot loader.  A blinking
cursor is probably a bit unrealistic (need timer interrupt etc), but
emulating a block cursor should be possible I think.

cheers,
  Gerd

[Kevin O'Connor]

On Wed, Apr 16, 2014 at 06:56:57PM +0200, Gerd Hoffmann wrote:
> And we don't have a cursor I've noticed meanwhile.  That would be very
> helpful for editing your linux cmd line in the boot loader.  A blinking
> cursor is probably a bit unrealistic (need timer interrupt etc), but
> emulating a block cursor should be possible I think.

I was thinking something similar - just xor the character cell
underneath the cursor.  The only tricky part is to make sure to undo
the cursor manipulation during any scrolls or screen writes.

I was also thinking about hooking the timer interrupt to support
blinking, but like you, I concluded it wasn't worth the effort.

-Kevin

[Kevin O'Connor]

On Wed, Apr 16, 2014 at 06:56:57PM +0200, Gerd Hoffmann wrote:
> Emulating attributes would be great so you can actually navigate
> bootloader menus.

I took a look a closer look at this.  Some things I found:

- it looks like the syslinux menu on recent Fedora versions doesn't
  work correctly at all in text mode.  This can be most easily seen by
  disabling CONFIG_VGA_VBE on one of the standard qemu seavgabios
  builds - without vbe syslinux wont go into graphics mode and the
  same broken text menu is shown.  So, this isn't a seavgabios issue
  but a fedora and/or syslinux issue.

- I took a quick look at what happens with the ati vgabios on my
  e350m1 if it is forced into a graphics mode (eg, mode 0x12).  I see
  similar results to what I see with coreboot native seavgabios.  So,
  it looks like most of our troubles are with the standard vgabios
  interface in graphics vs text mode and not something specific to
  seavgabios.

- The patch below seems to help a little with older Fedora versions
  using syslinux.  The idea is to try and treat the attr in graphics
  mode more like a text mode attr.  (The patch is broken for non
  native coreboot builds, but it demonstrates the idea.)
  Unfortunately, this patch doesn't help for the freedos menu - as
  that uses a different mechanism for setting the attributes (it fills
  areas of the screen with an attr and then separately writes text to
  the screen with the assumption that the existing text attr will
  remain in place).

-Kevin

--- a/vgasrc/vgafb.c
+++ b/vgasrc/vgafb.c
@@ -400,7 +400,7 @@ gfx_write_char(struct vgamode_s *vmode_g
         } else {
             int j;
             for (j = 0; j < 8; j++)
-                op.pixels[j] = (fontline & (0x80>>j)) ? ca.attr : 0x00;
+                op.pixels[j] = (fontline & (0x80>>j)) ? (ca.attr & 0x0f) : (ca.attr >> 4);
         }
         op.op = GO_WRITE8;
         handle_gfx_op(&op);

[H. Peter Anvin]

On 04/16/2014 03:47 PM, Kevin O'Connor wrote:
> On Wed, Apr 16, 2014 at 06:56:57PM +0200, Gerd Hoffmann wrote:
> > Emulating attributes would be great so you can actually navigate
> > bootloader menus.
>
> I took a look a closer look at this.  Some things I found:
> it looks like the syslinux menu on recent Fedora versions doesn't
> work correctly at all in text mode.  This can be most easily seen by
> disabling CONFIG_VGA_VBE on one of the standard qemu seavgabios
> builds - without vbe syslinux wont go into graphics mode and the
> same broken text menu is shown.  So, this isn't a seavgabios issue
> but a fedora and/or syslinux issue.

?!  I would love more details...
> I took a quick look at what happens with the ati vgabios on my
> e350m1 if it is forced into a graphics mode (eg, mode 0x12).  I see
> similar results to what I see with coreboot native seavgabios.  So,
> it looks like most of our troubles are with the standard vgabios
> interface in graphics vs text mode and not something specific to
> seavgabios.

The non-VESA graphics mode text interface is a bit "special"... and
quite frankly broken on a lot of systems.  I thought SeaBIOS was doing
okay, so there might be a bug here...

-hpa

[Kevin O'Connor]

On Wed, Apr 16, 2014 at 04:00:07PM -0700, H. Peter Anvin wrote:
> On 04/16/2014 03:47 PM, Kevin O'Connor wrote:
> > On Wed, Apr 16, 2014 at 06:56:57PM +0200, Gerd Hoffmann wrote:
> > > Emulating attributes would be great so you can actually navigate
> > > bootloader menus.
> >
> > I took a look a closer look at this.  Some things I found:
> > it looks like the syslinux menu on recent Fedora versions doesn't
> > work correctly at all in text mode.  This can be most easily seen by
> > disabling CONFIG_VGA_VBE on one of the standard qemu seavgabios
> > builds - without vbe syslinux wont go into graphics mode and the
> > same broken text menu is shown.  So, this isn't a seavgabios issue
> > but a fedora and/or syslinux issue.
>
> ?!  I would love more details...

If I build SeaVGABIOS without vesa and run the Fedora 16 (or later)
installer in qemu I get a menu that is not centered and the bottom
line of the menu box isn't drawn.  Also, when I hit up arrow or down
arrow I can't see which menu item is highlighted.  I'm really not sure
if this is syslinux related though.  (This wasn't an issue in Fedora
15 or earlier - though they always show up as black-and-white.)  See
the attached screenshot as an example.

Steps to reproduce - grab latest seabios from
git://git.seabios.org/seabios.git , run "make menuconfig" and select
CONFIG_VGA_CIRRUS=y and CONFIG_VGA_VBE=n.  Then run "make", then
"mkdir qemutest; cp out/bios.bin qemutest/; cp out/vgabios.bin
qemutest/vgabios-cirrus.bin".  Then run qemu with: qemu-system-i386 -L
qemutest/ -m 512 -cdrom Fedora-19-x86_64-DVD.iso
> > I took a quick look at what happens with the ati vgabios on my
> > e350m1 if it is forced into a graphics mode (eg, mode 0x12).  I see
> > similar results to what I see with coreboot native seavgabios.  So,
> > it looks like most of our troubles are with the standard vgabios
> > interface in graphics vs text mode and not something specific to
> > seavgabios.

That section of my email was not related to syslinux.  As background,
we're looking into ways to run standard boot loaders with only a
graphical framebuffer by emulating text mode on top of a graphical
framebuffer.  The section above was discussing how the vgabios
interprets certain calls differently depending on whether it is text
or graphics mode.

-Kevin

[H. Peter Anvin]

On 04/16/2014 04:21 PM, Kevin O'Connor wrote:
> On Wed, Apr 16, 2014 at 04:00:07PM -0700, H. Peter Anvin wrote:
> > On 04/16/2014 03:47 PM, Kevin O'Connor wrote:
> > > On Wed, Apr 16, 2014 at 06:56:57PM +0200, Gerd Hoffmann wrote:
> > > > Emulating attributes would be great so you can actually navigate
> > > > bootloader menus.
> > >
> > > I took a look a closer look at this.  Some things I found:
> > > it looks like the syslinux menu on recent Fedora versions doesn't
> > > work correctly at all in text mode.  This can be most easily seen by
> > > disabling CONFIG_VGA_VBE on one of the standard qemu seavgabios
> > > builds - without vbe syslinux wont go into graphics mode and the
> > > same broken text menu is shown.  So, this isn't a seavgabios issue
> > > but a fedora and/or syslinux issue.
> >
> > ?!  I would love more details...
>
> If I build SeaVGABIOS without vesa and run the Fedora 16 (or later)
> installer in qemu I get a menu that is not centered and the bottom
> line of the menu box isn't drawn.  Also, when I hit up arrow or down
> arrow I can't see which menu item is highlighted.  I'm really not sure
> if this is syslinux related though.  (This wasn't an issue in Fedora
> 15 or earlier - though they always show up as black-and-white.)  See
> the attached screenshot as an example.

I wonder if they simply specify a geometry that is too large for the
text mode screen.  I'll have to take a look at their config file.
> Steps to reproduce - grab latest seabios from
> git://git.seabios.org/seabios.git , run "make menuconfig" and select
> CONFIG_VGA_CIRRUS=y and CONFIG_VGA_VBE=n.  Then run "make", then
> "mkdir qemutest; cp out/bios.bin qemutest/; cp out/vgabios.bin
> qemutest/vgabios-cirrus.bin".  Then run qemu with: qemu-system-i386 -L
> qemutest/ -m 512 -cdrom Fedora-19-x86_64-DVD.iso
> > > I took a quick look at what happens with the ati vgabios on my
> > > e350m1 if it is forced into a graphics mode (eg, mode 0x12).  I see
> > > similar results to what I see with coreboot native seavgabios.  So,
> > > it looks like most of our troubles are with the standard vgabios
> > > interface in graphics vs text mode and not something specific to
> > > seavgabios.
>
> That section of my email was not related to syslinux.  As background,
> we're looking into ways to run standard boot loaders with only a
> graphical framebuffer by emulating text mode on top of a graphical
> framebuffer.  The section above was discussing how the vgabios
> interprets certain calls differently depending on whether it is text
> or graphics mode.

That it does, indeed.

For the record, Syslinux in text mode will use only BIOS calls (no
direct write to the screen) to allow things like serial port capture and
text-to-graphics conversion to work.

Let me know if I can be of assistance.

-hpa

[Kevin O'Connor]

On Wed, Apr 16, 2014 at 04:25:48PM -0700, H. Peter Anvin wrote:
> On 04/16/2014 04:21 PM, Kevin O'Connor wrote:
> > If I build SeaVGABIOS without vesa and run the Fedora 16 (or later)
> > installer in qemu I get a menu that is not centered and the bottom
> > line of the menu box isn't drawn.  Also, when I hit up arrow or down
> > arrow I can't see which menu item is highlighted.  I'm really not sure
> > if this is syslinux related though.  (This wasn't an issue in Fedora
> > 15 or earlier - though they always show up as black-and-white.)  See
> > the attached screenshot as an example.
>
> I wonder if they simply specify a geometry that is too large for the
> text mode screen.  I'll have to take a look at their config file.

Thanks.  The menu definitely looks weird even with a larger screen.
If I disable vesa and switch into a graphics mode (eg, 0x12) prior to
running syslinux I get a screenshot like the attached.  (Same steps to
reproduce as in my previous email with the additional patch below to
force SeaBIOS to switch to mode 0x12.)

To be clear, the biggest issue is not being able to see which item is
highlighted.

[...]
> For the record, Syslinux in text mode will use only BIOS calls (no
> direct write to the screen) to allow things like serial port capture and
> text-to-graphics conversion to work.

Yes - modern bootloaders seem quite good about using the bios
interface.
> Let me know if I can be of assistance.

While you're here - what conditions does syslinux require in order to
use graphics mode?  When we use the coreboot native vga SeaVGABIOS, it
does include vesa support and it will advertise exactly one vesa mode.
(For example, in our qemu tests it's mode 0x0140 that is 800x600 with
32bpp.)  However, syslinux (at least on my fedora tests) doesn't seem
to want to go into graphics mode and instead uses its text mode.  Are
we doing something wrong here, or is it just that a different
resolution is required?

Thanks,
-Kevin

--- a/src/bootsplash.c
+++ b/src/bootsplash.c
@@ -45,6 +45,7 @@ enable_vga_console(void)
     /* Enable VGA text mode */
     memset(&br, 0, sizeof(br));
     br.ax = 0x0003;
+    br.ax = 0x0012;
     call16_int10(&br);

// Write to screen.

[H. Peter Anvin]

Syslinux currently looks for a 640x480 mode unless MENU RESOLUTION is given.

On April 16, 2014 5:12:21 PM PDT, Kevin O'Connor  wrote:
> On Wed, Apr 16, 2014 at 04:25:48PM -0700, H. Peter Anvin wrote:
> > On 04/16/2014 04:21 PM, Kevin O'Connor wrote:
> > > If I build SeaVGABIOS without vesa and run the Fedora 16 (or later)
> > > installer in qemu I get a menu that is not centered and the bottom
> > > line of the menu box isn't drawn.  Also, when I hit up arrow or
>
> down
> > > arrow I can't see which menu item is highlighted.  I'm really not
>
> sure
> > > if this is syslinux related though.  (This wasn't an issue in
>
> Fedora
> > > 15 or earlier - though they always show up as black-and-white.)
>
> See
> > > the attached screenshot as an example.
> >
> > I wonder if they simply specify a geometry that is too large for the
> > text mode screen.  I'll have to take a look at their config file.
>
> Thanks.  The menu definitely looks weird even with a larger screen.
> If I disable vesa and switch into a graphics mode (eg, 0x12) prior to
> running syslinux I get a screenshot like the attached.  (Same steps to
> reproduce as in my previous email with the additional patch below to
> force SeaBIOS to switch to mode 0x12.)
>
> To be clear, the biggest issue is not being able to see which item is
> highlighted.
>
> [...]
> > For the record, Syslinux in text mode will use only BIOS calls (no
> > direct write to the screen) to allow things like serial port capture
>
> and
> > text-to-graphics conversion to work.
>
> Yes - modern bootloaders seem quite good about using the bios
> interface.
> > Let me know if I can be of assistance.
>
> While you're here - what conditions does syslinux require in order to
> use graphics mode?  When we use the coreboot native vga SeaVGABIOS, it
> does include vesa support and it will advertise exactly one vesa mode.
> (For example, in our qemu tests it's mode 0x0140 that is 800x600 with
> 32bpp.)  However, syslinux (at least on my fedora tests) doesn't seem
> to want to go into graphics mode and instead uses its text mode.  Are
> we doing something wrong here, or is it just that a different
> resolution is required?
>
> Thanks,
> -Kevin
>
> --- a/src/bootsplash.c
> +++ b/src/bootsplash.c
> @@ -45,6 +45,7 @@ enable_vga_console(void)
>     /* Enable VGA text mode */
>     memset(&br, 0, sizeof(br));
>     br.ax = 0x0003;
>
> br.ax = 0x0012;
> call16_int10(&br);
>
> // Write to screen.

[Gerd Hoffmann]

On Mi, 2014-04-16 at 17:30 -0700, H. Peter Anvin wrote:
> Syslinux currently looks for a 640x480 mode unless MENU RESOLUTION is given.

Do the vgabios interfaces allow for bytes_per_line being different from
x_res * bytes_per_pixel?  If so seavgabios could allow setting 640x480
on 800x600 ...

cheers,
  Gerd

[H. Peter Anvin]

It does.  On the other hand, Syslinux really need to handle various resolutions better.  Unto recently, though, 640x480 was universally available.

On April 17, 2014 1:41:58 AM PDT, Gerd Hoffmann  wrote:
> On Mi, 2014-04-16 at 17:30 -0700, H. Peter Anvin wrote:
> > Syslinux currently looks for a 640x480 mode unless MENU RESOLUTION is
>
> given.
>
> Do the vgabios interfaces allow for bytes_per_line being different from
> x_res * bytes_per_pixel?  If so seavgabios could allow setting 640x480
> on 800x600 ...
>
> cheers,
>  Gerd

[H. Peter Anvin]

On 04/17/2014 01:41 AM, Gerd Hoffmann wrote:
> On Mi, 2014-04-16 at 17:30 -0700, H. Peter Anvin wrote:
> > Syslinux currently looks for a 640x480 mode unless MENU RESOLUTION is given.
>
> Do the vgabios interfaces allow for bytes_per_line being different from
> x_res * bytes_per_pixel?  If so seavgabios could allow setting 640x480
> on 800x600 ...

They do.  However, it is also on my list to fix Syslinux to at least
deal better with multiple resolutions.

-hpa

[Gerd Hoffmann]

Hi,
> But, it does raise the question of how many other callers expect the
> bios to not mess with the segment limits.  (Though, to be honest, the
> only goal I have with coreboot native seavgabios is support for grub,
> lilo, syslinux, and maybe ntldr.)

Yes, it should be good enough for bootloaders.

Emulating attributes would be great so you can actually navigate
bootloader menus.

And we don't have a cursor I've noticed meanwhile.  That would be very
helpful for editing your linux cmd line in the boot loader.  A blinking
cursor is probably a bit unrealistic (need timer interrupt etc), but
emulating a block cursor should be possible I think.

cheers,
  Gerd

[David Woodhouse]

On Wed, 2014-04-16 at 12:37 -0400, Kevin O'Connor wrote:
> On Mon, Apr 14, 2014 at 02:22:51PM +0200, Gerd Hoffmann wrote:
> > So you can try this:
> >
> > qemu -vga std
>
> -bios /usr/share/coreboot.git/coreboot-i440fx-seabios.rom
> > to see it live in action.
> >
> > Two problems spotted so far:
> >
> > (1) ipxe hangs at rom load time.
> >       can be worked around by adding '-net none' to the qemu cmd
>
> line.
>
> I've reproduced this.  It only fails for me with -enable-kvm.  It was
> (as I feared) the result of segment limits getting trashed from the
> int 1587 call.  (Presumably, qemu tcg doesn't fail because it doesn't
> implement segment limits?)  The (incorrect) patch below enables the
> boot to proceed past the ipxe prompt.
>
> This is unfortunate.  I can put a hack into seabios (not seavgabios)
> to use bigreal mode for int 1587 calls during option rom execution.
> But, it does raise the question of how many other callers expect the
> bios to not mess with the segment limits.  (Though, to be honest, the
> only goal I have with coreboot native seavgabios is support for grub,
> lilo, syslinux, and maybe ntldr.)

Was there a conclusion to this? Did you eventually convince yourself
that it could be *OK* for the video BIOS to trash the segment limits,
for a limited set of boot targets?

[Kevin O'Connor]

On Mon, May 19, 2014 at 08:23:25PM +0100, David Woodhouse wrote:
> On Wed, 2014-04-16 at 12:37 -0400, Kevin O'Connor wrote:
> > This is unfortunate.  I can put a hack into seabios (not seavgabios)
> > to use bigreal mode for int 1587 calls during option rom execution.
> > But, it does raise the question of how many other callers expect the
> > bios to not mess with the segment limits.  (Though, to be honest, the
> > only goal I have with coreboot native seavgabios is support for grub,
> > lilo, syslinux, and maybe ntldr.)
>
> Was there a conclusion to this? Did you eventually convince yourself
> that it could be *OK* for the video BIOS to trash the segment limits,
> for a limited set of boot targets?

Commit 6c68e7ad changed SeaBIOS to stay in bigreal mode for an int
1587 call that comes during option rom processing.

The coreboot native vga init stuff (vgasrc/cbvga.c) is basically a
demonstration of SeaVGABIOS for cases where a "real" vgabios is not
available or not desirable.  As above, my goal for cbvga was only
modern OSes.

-Kevin