[SeaBIOS] Re: [PATCH] vbe: implement function 09h (get/set palette data)

[Kevin O'Connor]

On Sun, Mar 10, 2024 at 10:32:39PM -0700, Daniel Verkamp wrote:
> Since the VBE mode attributes indicate that all modes are not VGA
> compatible, applications must use VBE function 09h to manipulate the
> palette rather than directly accessing the VGA registers.
>
> This implementation uses the standard VGA registers for all hardware,
> which may not be appropriate; I only verified qemu -device VGA.
>
> Without this patch, the get/set palette function returns an error code,
> so programs that use 8-bit indexed color modes fail. For example, Quake
> (DOS) printed "Error: Unable to load VESA palette" and exited when
> trying to set a SVGA mode like 640x480, but with the patch it succeeds.
> This fixes qemu issue #251 and #1862.
>
> https://gitlab.com/qemu-project/qemu/-/issues/251
> https://gitlab.com/qemu-project/qemu/-/issues/1862

Interesting, thanks.

If I understand the vbe_104f09() requirements, it does basically the
same thing as handle_101012() and handle_101017(), but with a slightly
different memory layout.

Have you considered implementing vbe_104f09() using the existing
stdvga_dac_read() and stdvga_dac_write() calls?  That is, check for
CONFIG_VGA_STDVGA_PORTS at the start of vbe_104f09() (if that
definition is not set then it is not valid to access the dac
registers).  If it is set, then loop through each pallete request,
translate the pallette request to/from the existing dac format, and
call stdvga_dac_read/write().  The advantage of this approach is that
we maintain only one low-level mechanisms for accessing the dac
pallette registers.

Cheers,
-Kevin
> Signed-off-by: Daniel Verkamp
>
> src/std/vbe.h     |  7 +++++++
>  vgasrc/stdvga.h   |  3 +++
>  vgasrc/stdvgaio.c | 30 ++++++++++++++++++++++++++++++
>  vgasrc/vbe.c      | 42 ++++++++++++++++++++++++++++++++++++++++++
>  vgasrc/vgahw.h    |  8 ++++++++
>  5 files changed, 90 insertions(+)
>
> diff --git a/src/std/vbe.h b/src/std/vbe.h
> index fe96f5ec..fddaa4fd 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.h b/vgasrc/stdvga.h
> index 4184c600..af10fd78 100644
> --- a/vgasrc/stdvga.h
> +++ b/vgasrc/stdvga.h
> @@ -74,6 +74,9 @@ int stdvga_get_displaystart(struct vgamode_s *vmode_g);
>  int stdvga_set_displaystart(struct vgamode_s *vmode_g, int val);
>  int stdvga_get_dacformat(struct vgamode_s *vmode_g);
>  int stdvga_set_dacformat(struct vgamode_s *vmode_g, int val);
> +struct vbe_palette_entry;
> +int stdvga_set_palette_colors(u16 seg, struct vbe_palette_entry *pal_far, u8 start, int count);
> +int stdvga_get_palette_colors(u16 seg, struct vbe_palette_entry *pal_far, u8 start, int count);
>  int stdvga_save_restore(int cmd, u16 seg, void *data);
>  void stdvga_enable_video_addressing(u8 disable);
>  int stdvga_setup(void);
> diff --git a/vgasrc/stdvgaio.c b/vgasrc/stdvgaio.c
> index 77fcecdf..ec22c4a1 100644
> --- a/vgasrc/stdvgaio.c
> +++ b/vgasrc/stdvgaio.c
> @@ -6,6 +6,7 @@
>
> #include "farptr.h" // GET_FARVAR
>  #include "stdvga.h" // VGAREG_PEL_MASK
> +#include "std/vbe.h" // vbe_palette_entry
>  #include "vgautil.h" // stdvga_pelmask_read
>  #include "x86.h" // inb
>
> @@ -184,3 +185,32 @@ stdvga_dac_write(u16 seg, u8 *data_far, u8 start, int count)
>          count--;
>      }
>  }
>
> +int
> +stdvga_set_palette_colors(u16 seg, struct vbe_palette_entry *pal_far, u8 start, int count)
> +{
> outb(start, VGAREG_DAC_WRITE_ADDRESS);
> while (count) {
>    outb(GET_FARVAR(seg, pal_far->red), VGAREG_DAC_DATA);
>    outb(GET_FARVAR(seg, pal_far->green), VGAREG_DAC_DATA);
>    outb(GET_FARVAR(seg, pal_far->blue), VGAREG_DAC_DATA);
>    pal_far++;
>    count--;
> }
> return 0;
>
> +}
>
> +int
> +stdvga_get_palette_colors(u16 seg, struct vbe_palette_entry *pal_far, u8 start, int count)
> +{
> outb(start, VGAREG_DAC_READ_ADDRESS);
> while (count) {
>    SET_FARVAR(seg, pal_far->red, inb(VGAREG_DAC_DATA));
>    SET_FARVAR(seg, pal_far->green, inb(VGAREG_DAC_DATA));
>    SET_FARVAR(seg, pal_far->blue, inb(VGAREG_DAC_DATA));
>    SET_FARVAR(seg, pal_far->align, 0);
>    pal_far++;
>    count--;
> }
> return 0;
>
> +}
> diff --git a/vgasrc/vbe.c b/vgasrc/vbe.c
> index 91abc9ab..0ac88805 100644
> --- a/vgasrc/vbe.c
> +++ b/vgasrc/vbe.c
> @@ -376,6 +376,47 @@ fail:
>      regs->ax = 0x014f;
>  }
>
> +static void
> +vbe_104f09(struct bregs *regs)
> +{
> struct vgamode_s *vmode_g = get_current_mode();
> if (! vmode_g)
>    goto fail;
> u8 memmodel = GET_GLOBAL(vmode_g->memmodel);
> u8 depth = GET_GLOBAL(vmode_g->depth);
> if (memmodel == MM_DIRECT || memmodel == MM_YUV || depth > 8) {
>    regs->ax = 0x034f;
>    return;
> }
> if (regs->dh)
>    goto fail;
> u8 start = regs->dl;
> int count = regs->cx;
> int max_colors = 1 << depth;
> if (start + count > max_colors)
>    goto fail;
> u16 seg = regs->es;
> struct vbe_palette_entry *pal = (void*)(regs->di+0);
> int ret;
> switch (regs->bl) {
> case 0x80:
> case 0x00:
>    ret = vgahw_set_palette_colors(seg, pal, start, count);
>    break;
> case 0x01:
>    ret = vgahw_get_palette_colors(seg, pal, start, count);
>    break;
> default:
>    goto fail;
> }
> if (ret < 0)
>    goto fail;
> regs->ax = 0x004f;
> return;
>
> +fail:
> regs->ax = 0x014f;
>
> +}
>
> static void
>  vbe_104f0a(struct bregs *regs)
>  {
> @@ -456,6 +497,7 @@ handle_104f(struct bregs *regs)
>      case 0x06: vbe_104f06(regs); break;
>      case 0x07: vbe_104f07(regs); break;
>      case 0x08: vbe_104f08(regs); break;
> case 0x09: vbe_104f09(regs); break;
> case 0x0a: vbe_104f0a(regs); break;
> case 0x10: vbe_104f10(regs); break;
> case 0x15: vbe_104f15(regs); break;
>
> diff --git a/vgasrc/vgahw.h b/vgasrc/vgahw.h
> index 8b64660e..684530d6 100644
> --- a/vgasrc/vgahw.h
> +++ b/vgasrc/vgahw.h
> @@ -141,6 +141,14 @@ static inline int vgahw_set_dacformat(struct vgamode_s *vmode_g, int val) {
>      return stdvga_set_dacformat(vmode_g, val);
>  }
>
> +static inline int vgahw_set_palette_colors(u16 seg, struct vbe_palette_entry *pal_far, u8 start, int count) {
> return stdvga_set_palette_colors(seg, pal_far, start, count);
>
> +}
>
> +static inline int vgahw_get_palette_colors(u16 seg, struct vbe_palette_entry *pal_far, u8 start, int count) {
> return stdvga_get_palette_colors(seg, pal_far, start, count);
>
> +}
>
> static inline int vgahw_save_restore(int cmd, u16 seg, void *data) {
>      if (CONFIG_VGA_CIRRUS)
>          return clext_save_restore(cmd, seg, data);
> -- 
> 2.43.0
>
> SeaBIOS mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]