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(+)
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>
Signed-off-by: Daniel Verkamp <daniel@drv.nu>
---
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 -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.orgOn 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 <daniel@drv.nu>
> ---
> 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 -- seabios@seabios.org
> To unsubscribe send an email to seabios-leave@seabios.org
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
On Mon, Mar 11, 2024 at 9:23 AM Kevin O'Connor <kevin@koconnor.net> wrote: > > 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 Rewriting the buffer in place is a clever idea; I'll give that a try. I guess it should be okay to rewrite the buffer in place as long as we restore it to the original format after a "set" call so from the caller's perspective, the buffer is not modified by a "read-only" function. It can probably be arranged so that this happens naturally - convert to stdvga format before calling either read or write, then convert to VBE format after the call (which would be needed for "get" anyway). Thanks, -- Daniel _______________________________________________ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-leave@seabios.org
On Mon, Mar 11, 2024 at 06:08:22PM -0700, Daniel Verkamp wrote:
> On Mon, Mar 11, 2024 at 9:23 AM Kevin O'Connor <kevin@koconnor.net> wrote:
> >
> > 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
>
> Rewriting the buffer in place is a clever idea; I'll give that a try.
> I guess it should be okay to rewrite the buffer in place as long as we
> restore it to the original format after a "set" call so from the
> caller's perspective, the buffer is not modified by a "read-only"
> function. It can probably be arranged so that this happens naturally -
> convert to stdvga format before calling either read or write, then
> convert to VBE format after the call (which would be needed for "get"
> anyway).
Rewriting the buffer I guess is possible, but just to be clear I was
more thinking about splitting the request up into many smaller
requests.
Something like (totally untested):
int
vbe_set_palette_colors(u16 seg, struct vbe_palette_entry *pal_far, u8 start, int count)
{
while (count) {
u8 rgb[3] = { pal_far->red, pal_far->green, pal_far->blue };
stdvga_dac_write(GET_SEG(SS), rgb, start, 1);
pal_far++;
start++;
count--;
}
return 0;
}
int
vbe_get_palette_colors(u16 seg, struct vbe_palette_entry *pal_far, u8 start, int count)
{
while (count) {
u8 rgb[3];
stdvga_dac_read(GET_SEG(SS), rgb, start, 1);
pal_far->red = rgb[0];
pal_far->green = rgb[1];
pal_far->blue = rgb[2];
pal_far++;
start++;
count--;
}
return 0;
}
It's also totally fine to redefine stdvga_dac_read/write() and/or
introduce a stdhw_dac_read/write(). Whatever the interface is though,
I do think that vbe_104f09(), handle_101010(), handle_101012(),
handle_101015(), handle_101017() should use that same interface.
Cheers,
-Kevin
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
© 2016 - 2024 Red Hat, Inc.