On Fri, May 15, 2020 at 10:02:22PM -0700, [email protected] wrote:
> Ok, new theory:
>
> VirtualBox ignores the pas bit, so writes to the attribute data register
> after vga_enable() is called go ahead and modify the attribute palette
> index 0, but on a real vga the write is ignored.
>
> This implies something is writing to VGA_ATC_DATAW without resetting the
> flip flop.
>
> If I change vga_enable() to write 0x3f instead of 0x20, which is an
> actual illegal register + pas instead of text palette 0 + pas, the text
> background doesn't change.
>
> If I explicitly write a 0 to VGA_ATC_DATAW after the vga_enable(), the
> problem also doesn't appear; the background stays black.
>
> If I explicitly write a 3 to VGA_ATC_DATAW after the vga_enable(), the
> text background becomes cyan after the restore, which confirms that
> VirtualBox is allowing the palette registers to be modified even when
> pas is set.
>
> Oddly, if I reset the flip-flop again after vga_enable(), which should
> make the next write just an address change instead of doing anything,
> the problem still happens.
>
> So, I would propose:
>
> Removing the flip flop reset and vga_reset() from vga_restore_palette()
> since setting the 256 color VGA palette doesn't touch the attribute
> registers at all. This fixes VirtualBox.
>
> Leave the vga_enable() code in vga_var.h for the vga_attr_read/write()
> calls, where it probably is still needed, at least for old cards.
>
> It looks like the only place those are ever called from is
> vga_save_state() / vga_restore_state(), which only happens on ACPI
> events. That might still cause a problem on VirtualBox, but I don't
> see a way to simulate a hibernate event, so it probably doesn't matter.
>
> Alternately, vga_enable() could explicitly write a 0 after setting 0x20
> which would work around the VirtualBox issue and not harm anything
> else.
It turns out the flip-flop reset in the palette functions is using the
wrong address. _vga_attr_read()/_vga_attr_write() get it right.
ioh_vga 0x3c0
vh_ioh_6845 0x3d0 (unless mono)
vga_raw_read() uses ioh_vga so incorrectly reads 0x3ca to reset
instead of 0x3da.
This was a mistake made in importing code from FreeBSD
----------------------------
revision 1.48
date: 2009/02/01 14:37:22; author: miod; state: Exp; lines: +78 -3;
Save the text mode color palette upon startup, and restore it when
switching consoles or when X11 exits. Almost all other operating systems
do this, and thus do not suffer from palette bugs in some X11 drivers.
>From FreeBSD.
----------------------------
In FreeBSD they have
inb(adp->va_crtc_addr + 6)
where va_crtc_addr is set to COLOR_CRTC for non-mono
#define COLOR_CRTC (IO_CGA + 0x04)
#define IO_CGA 0x3D0
I agree the attempted flip-flop reset and pas setting can go:
Index: vga.c
===================================================================
RCS file: /cvs/src/sys/dev/ic/vga.c,v
retrieving revision 1.69
diff -u -p -r1.69 vga.c
--- vga.c 17 Jun 2017 19:20:30 -0000 1.69
+++ vga.c 16 May 2020 07:21:19 -0000
@@ -1213,8 +1213,6 @@ vga_save_palette(struct vga_config *vc)
vga_raw_write(vh, VGA_DAC_READ, 0x00);
for (i = 0; i < 3 * 256; i++)
*palette++ = vga_raw_read(vh, VGA_DAC_DATA);
-
- vga_raw_read(vh, 0x0a); /* reset flip/flop */
}
void
@@ -1231,10 +1229,6 @@ vga_restore_palette(struct vga_config *v
vga_raw_write(vh, VGA_DAC_WRITE, 0x00);
for (i = 0; i < 3 * 256; i++)
vga_raw_write(vh, VGA_DAC_DATA, *palette++);
-
- vga_raw_read(vh, 0x0a); /* reset flip/flop */
-
- vga_enable(vh);
}
void
