Introduce stdvga_dac_read_many() and stdvga_dac_write_many() for
writing multiple dac palette entries.  Convert the stdvga_dac_read()
and stdvga_dac_write() low-level IO access functions in stdvgaio.c to
access just one color palette entry.

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
--

There should not be any functional change resulting from this patch.
However, during mode switches (and a few other rare calls) there will
likely be a few more outb() calls due to additional writes to the dac
index register.  In theory this could adversely impact performance,
but in practice I think it is unlikely it will be noticeable.  The
original code was also not optimized (it already makes a large number
of redundant writes to the dac index register), and performance
sensitve code already avoids using the vgabios.

---
 src/std/vbe.h        |  7 +++++++
 vgasrc/stdvga.c      | 50 +++++++++++++++++++++++++++++++++++++-------
 vgasrc/stdvga.h      | 26 +++++++++++++++++++++++
 vgasrc/stdvgaio.c    | 37 ++++++++++++--------------------
 vgasrc/stdvgamodes.c |  8 +++----
 vgasrc/vbe.c         | 16 +++++---------
 vgasrc/vgabios.c     | 22 +++++++++----------
 vgasrc/vgautil.h     | 23 --------------------
 8 files changed, 109 insertions(+), 80 deletions(-)

diff --git a/src/std/vbe.h b/src/std/vbe.h
index fe96f5e..fddaa4f 100644
--- a/src/std/vbe.h
+++ b/src/std/vbe.h
@@ -88,6 +88,13 @@ struct vbe_crtc_info {
     u8 reserved[40];
 } PACKED;
 
+struct vbe_palette_entry {
+    u8 blue;
+    u8 green;
+    u8 red;
+    u8 align;
+} PACKED;
+
 /* VBE Return Status Info */
 /* AL */
 #define VBE_RETURN_STATUS_SUPPORTED                      0x4F
diff --git a/vgasrc/stdvga.c b/vgasrc/stdvga.c
index afe26db..aeab9d4 100644
--- a/vgasrc/stdvga.c
+++ b/vgasrc/stdvga.c
@@ -9,6 +9,7 @@
 #include "farptr.h" // SET_FARVAR
 #include "stdvga.h" // stdvga_setup
 #include "string.h" // memset_far
+#include "std/vbe.h" // vbe_palette_entry
 #include "vgabios.h" // struct vgamode_s
 #include "vgautil.h" // stdvga_attr_write
 #include "x86.h" // outb
@@ -128,6 +129,41 @@ stdvga_get_palette_page(u8 *pal_pagesize, u8 *pal_page)
  * DAC control
  ****************************************************************/
 
+// Store dac colors into memory in 3-byte rgb format
+void
+stdvga_dac_read_many(u16 seg, u8 *data_far, u8 start, int count)
+{
+    while (count) {
+        struct vbe_palette_entry rgb = stdvga_dac_read(start);
+        SET_FARVAR(seg, *data_far, rgb.red);
+        data_far++;
+        SET_FARVAR(seg, *data_far, rgb.green);
+        data_far++;
+        SET_FARVAR(seg, *data_far, rgb.blue);
+        data_far++;
+        start++;
+        count--;
+    }
+}
+
+// Load dac colors from memory in 3-byte rgb format
+void
+stdvga_dac_write_many(u16 seg, u8 *data_far, u8 start, int count)
+{
+    while (count) {
+        u8 r = GET_FARVAR(seg, *data_far);
+        data_far++;
+        u8 g = GET_FARVAR(seg, *data_far);
+        data_far++;
+        u8 b = GET_FARVAR(seg, *data_far);
+        data_far++;
+        struct vbe_palette_entry rgb = { .red=r, .green=g, .blue=b };
+        stdvga_dac_write(start, rgb);
+        start++;
+        count--;
+    }
+}
+
 // Convert all loaded colors to shades of gray
 void
 stdvga_perform_gray_scale_summing(u16 start, u16 count)
@@ -135,16 +171,16 @@ stdvga_perform_gray_scale_summing(u16 start, u16 count)
     stdvga_attrindex_write(0x00);
     int i;
     for (i = start; i < start+count; i++) {
-        u8 rgb[3];
-        stdvga_dac_read(GET_SEG(SS), rgb, i, 1);
+        struct vbe_palette_entry rgb = stdvga_dac_read(i);
 
         // intensity = ( 0.3 * Red ) + ( 0.59 * Green ) + ( 0.11 * Blue )
-        u16 intensity = ((77 * rgb[0] + 151 * rgb[1] + 28 * rgb[2]) + 0x80) >> 8;
+        u16 intensity = ((77 * rgb.red + 151 * rgb.green
+                          + 28 * rgb.blue) + 0x80) >> 8;
         if (intensity > 0x3f)
             intensity = 0x3f;
-        rgb[0] = rgb[1] = rgb[2] = intensity;
+        rgb.red = rgb.green = rgb.blue = intensity;
 
-        stdvga_dac_write(GET_SEG(SS), rgb, i, 1);
+        stdvga_dac_write(i, rgb);
     }
     stdvga_attrindex_write(0x20);
 }
@@ -474,7 +510,7 @@ stdvga_save_dac_state(u16 seg, struct saveDACcolors *info)
     SET_FARVAR(seg, info->rwmode, inb(VGAREG_DAC_STATE));
     SET_FARVAR(seg, info->peladdr, inb(VGAREG_DAC_WRITE_ADDRESS));
     SET_FARVAR(seg, info->pelmask, stdvga_pelmask_read());
-    stdvga_dac_read(seg, info->dac, 0, 256);
+    stdvga_dac_read_many(seg, info->dac, 0, 256);
     SET_FARVAR(seg, info->color_select, 0);
 }
 
@@ -482,7 +518,7 @@ static void
 stdvga_restore_dac_state(u16 seg, struct saveDACcolors *info)
 {
     stdvga_pelmask_write(GET_FARVAR(seg, info->pelmask));
-    stdvga_dac_write(seg, info->dac, 0, 256);
+    stdvga_dac_write_many(seg, info->dac, 0, 256);
     outb(GET_FARVAR(seg, info->peladdr), VGAREG_DAC_WRITE_ADDRESS);
 }
 
diff --git a/vgasrc/stdvga.h b/vgasrc/stdvga.h
index ce5a80a..55ad76e 100644
--- a/vgasrc/stdvga.h
+++ b/vgasrc/stdvga.h
@@ -2,6 +2,7 @@
 #define __STDVGA_H
 
 #include "types.h" // u8
+#include "std/vbe.h" // struct vbe_palette_entry
 
 // VGA registers
 #define VGAREG_ACTL_ADDRESS            0x3c0
@@ -55,6 +56,8 @@ void stdvga_set_palette_blinking(u8 enable_blink);
 void stdvga_set_palette_pagesize(u8 pal_pagesize);
 void stdvga_set_palette_page(u8 pal_page);
 void stdvga_get_palette_page(u8 *pal_pagesize, u8 *pal_page);
+void stdvga_dac_read_many(u16 seg, u8 *data_far, u8 start, int count);
+void stdvga_dac_write_many(u16 seg, u8 *data_far, u8 start, int count);
 void stdvga_perform_gray_scale_summing(u16 start, u16 count);
 void stdvga_planar4_plane(int plane);
 void stdvga_set_font_location(u8 spec);
@@ -81,4 +84,27 @@ int stdvga_save_restore(int cmd, u16 seg, void *data);
 void stdvga_enable_video_addressing(u8 disable);
 int stdvga_setup(void);
 
+// stdvgaio.c
+u8 stdvga_pelmask_read(void);
+void stdvga_pelmask_write(u8 val);
+u8 stdvga_misc_read(void);
+void stdvga_misc_write(u8 value);
+void stdvga_misc_mask(u8 off, u8 on);
+u8 stdvga_sequ_read(u8 index);
+void stdvga_sequ_write(u8 index, u8 value);
+void stdvga_sequ_mask(u8 index, u8 off, u8 on);
+u8 stdvga_grdc_read(u8 index);
+void stdvga_grdc_write(u8 index, u8 value);
+void stdvga_grdc_mask(u8 index, u8 off, u8 on);
+u8 stdvga_crtc_read(u16 crtc_addr, u8 index);
+void stdvga_crtc_write(u16 crtc_addr, u8 index, u8 value);
+void stdvga_crtc_mask(u16 crtc_addr, u8 index, u8 off, u8 on);
+u8 stdvga_attr_read(u8 index);
+void stdvga_attr_write(u8 index, u8 value);
+void stdvga_attr_mask(u8 index, u8 off, u8 on);
+u8 stdvga_attrindex_read(void);
+void stdvga_attrindex_write(u8 value);
+struct vbe_palette_entry stdvga_dac_read(u8 color);
+void stdvga_dac_write(u8 color, struct vbe_palette_entry rgb);
+
 #endif // stdvga.h
diff --git a/vgasrc/stdvgaio.c b/vgasrc/stdvgaio.c
index 77fcecd..0fbff1a 100644
--- a/vgasrc/stdvgaio.c
+++ b/vgasrc/stdvgaio.c
@@ -155,32 +155,21 @@ stdvga_attrindex_write(u8 value)
 }
 
 
-void
-stdvga_dac_read(u16 seg, u8 *data_far, u8 start, int count)
+struct vbe_palette_entry
+stdvga_dac_read(u8 color)
 {
-    outb(start, VGAREG_DAC_READ_ADDRESS);
-    while (count) {
-        SET_FARVAR(seg, *data_far, inb(VGAREG_DAC_DATA));
-        data_far++;
-        SET_FARVAR(seg, *data_far, inb(VGAREG_DAC_DATA));
-        data_far++;
-        SET_FARVAR(seg, *data_far, inb(VGAREG_DAC_DATA));
-        data_far++;
-        count--;
-    }
+    outb(color, VGAREG_DAC_READ_ADDRESS);
+    u8 r = inb(VGAREG_DAC_DATA);
+    u8 g = inb(VGAREG_DAC_DATA);
+    u8 b = inb(VGAREG_DAC_DATA);
+    return (struct vbe_palette_entry){ .red=r, .green=g, .blue=b };
 }
 
 void
-stdvga_dac_write(u16 seg, u8 *data_far, u8 start, int count)
-{
-    outb(start, VGAREG_DAC_WRITE_ADDRESS);
-    while (count) {
-        outb(GET_FARVAR(seg, *data_far), VGAREG_DAC_DATA);
-        data_far++;
-        outb(GET_FARVAR(seg, *data_far), VGAREG_DAC_DATA);
-        data_far++;
-        outb(GET_FARVAR(seg, *data_far), VGAREG_DAC_DATA);
-        data_far++;
-        count--;
-    }
+stdvga_dac_write(u8 color, struct vbe_palette_entry rgb)
+{
+    outb(color, VGAREG_DAC_WRITE_ADDRESS);
+    outb(rgb.red, VGAREG_DAC_DATA);
+    outb(rgb.green, VGAREG_DAC_DATA);
+    outb(rgb.blue, VGAREG_DAC_DATA);
 }
diff --git a/vgasrc/stdvgamodes.c b/vgasrc/stdvgamodes.c
index c1d9722..b1d0ef6 100644
--- a/vgasrc/stdvgamodes.c
+++ b/vgasrc/stdvgamodes.c
@@ -471,11 +471,11 @@ stdvga_set_mode(struct vgamode_s *vmode_g, int flags)
         u16 palsize = GET_GLOBAL(stdmode_g->dacsize) / 3;
 
         // Always 256*3 values
-        stdvga_dac_write(get_global_seg(), palette_g, 0, palsize);
+        stdvga_dac_write_many(get_global_seg(), palette_g, 0, palsize);
         int i;
         for (i = palsize; i < 0x0100; i++) {
-            static u8 rgb[3] VAR16;
-            stdvga_dac_write(get_global_seg(), rgb, i, 1);
+            struct vbe_palette_entry rgb = { };
+            stdvga_dac_write(i, rgb);
         }
 
         if (flags & MF_GRAYSUM)
@@ -535,5 +535,5 @@ stdvga_set_mode(struct vgamode_s *vmode_g, int flags)
 void
 stdvga_set_packed_palette(void)
 {
-    stdvga_dac_write(get_global_seg(), pal_vga, 0, sizeof(pal_vga) / 3);
+    stdvga_dac_write_many(get_global_seg(), pal_vga, 0, sizeof(pal_vga) / 3);
 }
diff --git a/vgasrc/vbe.c b/vgasrc/vbe.c
index d2aaace..2ae0a98 100644
--- a/vgasrc/vbe.c
+++ b/vgasrc/vbe.c
@@ -403,26 +403,20 @@ vbe_104f09(struct bregs *regs)
     if (start + count > max_colors)
         goto fail;
     u16 seg = regs->es;
-    u8 *data_far = (void*)(regs->di+0);
-    u8 rgb[3];
+    struct vbe_palette_entry *data_far = (void*)(regs->di+0);
     int i;
     switch (regs->bl) {
     case 0x80:
     case 0x00:
         for (i = 0; i < count; i++) {
-            rgb[0] = GET_FARVAR(seg, data_far[i*4 + 2]);
-            rgb[1] = GET_FARVAR(seg, data_far[i*4 + 1]);
-            rgb[2] = GET_FARVAR(seg, data_far[i*4 + 0]);
-            stdvga_dac_write(GET_SEG(SS), rgb, start + i, 1);
+            struct vbe_palette_entry rgb = GET_FARVAR(seg, data_far[i]);
+            stdvga_dac_write(start + i, rgb);
         }
         break;
     case 0x01:
         for (i = 0; i < count; i++) {
-            stdvga_dac_read(GET_SEG(SS), rgb, start + i, 1);
-            SET_FARVAR(seg, data_far[i*4 + 0], rgb[2]);
-            SET_FARVAR(seg, data_far[i*4 + 1], rgb[1]);
-            SET_FARVAR(seg, data_far[i*4 + 2], rgb[0]);
-            SET_FARVAR(seg, data_far[i*4 + 3], 0);
+            struct vbe_palette_entry rgb = stdvga_dac_read(start + i);
+            SET_FARVAR(seg, data_far[i], rgb);
         }
         break;
     default:
diff --git a/vgasrc/vgabios.c b/vgasrc/vgabios.c
index 3659f01..44fb26c 100644
--- a/vgasrc/vgabios.c
+++ b/vgasrc/vgabios.c
@@ -567,17 +567,18 @@ handle_101009(struct bregs *regs)
     stdvga_get_all_palette_reg(regs->es, (u8*)(regs->dx + 0));
 }
 
-static void noinline
+static void
 handle_101010(struct bregs *regs)
 {
-    u8 rgb[3] = {regs->dh, regs->ch, regs->cl};
-    stdvga_dac_write(GET_SEG(SS), rgb, regs->bx, 1);
+    struct vbe_palette_entry rgb = {
+        .red=regs->dh, .green=regs->ch, .blue=regs->cl };
+    stdvga_dac_write(regs->bx, rgb);
 }
 
 static void
 handle_101012(struct bregs *regs)
 {
-    stdvga_dac_write(regs->es, (u8*)(regs->dx + 0), regs->bx, regs->cx);
+    stdvga_dac_write_many(regs->es, (u8*)(regs->dx + 0), regs->bx, regs->cx);
 }
 
 static void
@@ -589,20 +590,19 @@ handle_101013(struct bregs *regs)
         stdvga_set_palette_page(regs->bh);
 }
 
-static void noinline
+static void
 handle_101015(struct bregs *regs)
 {
-    u8 rgb[3];
-    stdvga_dac_read(GET_SEG(SS), rgb, regs->bx, 1);
-    regs->dh = rgb[0];
-    regs->ch = rgb[1];
-    regs->cl = rgb[2];
+    struct vbe_palette_entry rgb = stdvga_dac_read(regs->bx);
+    regs->dh = rgb.red;
+    regs->ch = rgb.green;
+    regs->cl = rgb.blue;
 }
 
 static void
 handle_101017(struct bregs *regs)
 {
-    stdvga_dac_read(regs->es, (u8*)(regs->dx + 0), regs->bx, regs->cx);
+    stdvga_dac_read_many(regs->es, (u8*)(regs->dx + 0), regs->bx, regs->cx);
 }
 
 static void
diff --git a/vgasrc/vgautil.h b/vgasrc/vgautil.h
index ce307c9..ea1ac77 100644
--- a/vgasrc/vgautil.h
+++ b/vgasrc/vgautil.h
@@ -48,29 +48,6 @@ void ati_list_modes(u16 seg, u16 *dest, u16 *last);
 int ati_set_mode(struct vgamode_s *vmode_g, int flags);
 int ati_setup(void);
 
-// stdvgaio.c
-u8 stdvga_pelmask_read(void);
-void stdvga_pelmask_write(u8 val);
-u8 stdvga_misc_read(void);
-void stdvga_misc_write(u8 value);
-void stdvga_misc_mask(u8 off, u8 on);
-u8 stdvga_sequ_read(u8 index);
-void stdvga_sequ_write(u8 index, u8 value);
-void stdvga_sequ_mask(u8 index, u8 off, u8 on);
-u8 stdvga_grdc_read(u8 index);
-void stdvga_grdc_write(u8 index, u8 value);
-void stdvga_grdc_mask(u8 index, u8 off, u8 on);
-u8 stdvga_crtc_read(u16 crtc_addr, u8 index);
-void stdvga_crtc_write(u16 crtc_addr, u8 index, u8 value);
-void stdvga_crtc_mask(u16 crtc_addr, u8 index, u8 off, u8 on);
-u8 stdvga_attr_read(u8 index);
-void stdvga_attr_write(u8 index, u8 value);
-void stdvga_attr_mask(u8 index, u8 off, u8 on);
-u8 stdvga_attrindex_read(void);
-void stdvga_attrindex_write(u8 value);
-void stdvga_dac_read(u16 seg, u8 *data_far, u8 start, int count);
-void stdvga_dac_write(u16 seg, u8 *data_far, u8 start, int count);
-
 // stdvgamodes.c
 struct vgamode_s *stdvga_find_mode(int mode);
 void stdvga_list_modes(u16 seg, u16 *dest, u16 *last);
-- 
2.44.0

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org

On Tue, Apr 09, 2024 at 11:20:14AM -0400, Kevin O'Connor wrote:
> Introduce stdvga_dac_read_many() and stdvga_dac_write_many() for
> writing multiple dac palette entries.  Convert the stdvga_dac_read()
> and stdvga_dac_write() low-level IO access functions in stdvgaio.c to
> access just one color palette entry.
> 
> Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
> --
> 
> There should not be any functional change resulting from this patch.
> However, during mode switches (and a few other rare calls) there will
> likely be a few more outb() calls due to additional writes to the dac
> index register.  In theory this could adversely impact performance,
> but in practice I think it is unlikely it will be noticeable.  The
> original code was also not optimized (it already makes a large number
> of redundant writes to the dac index register), and performance
> sensitve code already avoids using the vgabios.

I committed this change.

-Kevin
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org

On Tue, Apr 9, 2024 at 8:20 AM Kevin O'Connor <kevin@koconnor.net> wrote:
>
> Introduce stdvga_dac_read_many() and stdvga_dac_write_many() for
> writing multiple dac palette entries.  Convert the stdvga_dac_read()
> and stdvga_dac_write() low-level IO access functions in stdvgaio.c to
> access just one color palette entry.
>
> Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
> --
>
> There should not be any functional change resulting from this patch.
> However, during mode switches (and a few other rare calls) there will
> likely be a few more outb() calls due to additional writes to the dac
> index register.  In theory this could adversely impact performance,
> but in practice I think it is unlikely it will be noticeable.  The
> original code was also not optimized (it already makes a large number
> of redundant writes to the dac index register), and performance
> sensitve code already avoids using the vgabios.

Looks reasonable to me, and .red/.green/.blue is definitely more
readable than the array indexing. I don't think the performance
difference should practically matter either.

Thanks,
-- Daniel
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org