[Qemu-devel] [PULL 6/8] ati-vga: Attempt to handle CRTC offset not exact multiple of stride

From: Gerd Hoffmann <kraxel@redhat.com>
To: qemu-devel@nongnu.org
Cc: Gerd Hoffmann <kraxel@redhat.com>
Subject: [Qemu-devel] [PULL 6/8] ati-vga: Attempt to handle CRTC offset not exact multiple of stride
Date: Thu, 22 Aug 2019 10:05:00 +0200	[thread overview]
Message-ID: <20190822080503.12063-7-kraxel@redhat.com> (raw)
In-Reply-To: <20190822080503.12063-1-kraxel@redhat.com>

From: BALATON Zoltan <balaton@eik.bme.hu>

MacOS uses non-0 offset so it needs this and the resulting
vbe_start_addr seems correct but picture is still broken with OpenBIOS
after FCode runs but that maybe due to firmware problems now. After
boot, picture is now correct.

It also occured to me that these CRTC regs are also present in VGA so
I wonder if they should be shared in case some drivers try to poke
them via VGA regs or these are a separate set of regs for extended
mode. Added a comment noting this but drivers I've tried so far
program the card accessing ati regs so I did not attempt to change it.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Message-id: 1c6fce457ef7e6f889e38dc0423791be92310a62.1565558093.git.balaton@eik.bme.hu
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/ati.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/hw/display/ati.c b/hw/display/ati.c
index ff04f7eb4a2d..f24c23fa89c8 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -50,6 +50,7 @@ static void ati_vga_switch_mode(ATIVGAState *s)
         s->mode = EXT_MODE;
         if (s->regs.crtc_gen_cntl & CRTC2_EN) {
             /* CRT controller enabled, use CRTC values */
+            /* FIXME Should these be the same as VGA CRTC regs? */
             uint32_t offs = s->regs.crtc_offset & 0x07ffffff;
             int stride = (s->regs.crtc_pitch & 0x7ff) * 8;
             int bpp = 0;
@@ -101,16 +102,23 @@ static void ati_vga_switch_mode(ATIVGAState *s)
                 (s->regs.dac_cntl & DAC_8BIT_EN ? VBE_DISPI_8BIT_DAC : 0));
             /* now set offset and stride after enable as that resets these */
             if (stride) {
+                int bypp = DIV_ROUND_UP(bpp, BITS_PER_BYTE);
+
                 vbe_ioport_write_index(&s->vga, 0, VBE_DISPI_INDEX_VIRT_WIDTH);
                 vbe_ioport_write_data(&s->vga, 0, stride);
-                if (offs % stride == 0) {
-                    vbe_ioport_write_index(&s->vga, 0, VBE_DISPI_INDEX_Y_OFFSET);
-                    vbe_ioport_write_data(&s->vga, 0, offs / stride);
-                } else {
-                    /* FIXME what to do with this? */
-                    error_report("VGA offset is not multiple of pitch, "
-                                 "expect bad picture");
+                stride *= bypp;
+                if (offs % stride) {
+                    DPRINTF("CRTC offset is not multiple of pitch\n");
+                    vbe_ioport_write_index(&s->vga, 0,
+                                           VBE_DISPI_INDEX_X_OFFSET);
+                    vbe_ioport_write_data(&s->vga, 0, offs % stride / bypp);
                 }
+                vbe_ioport_write_index(&s->vga, 0, VBE_DISPI_INDEX_Y_OFFSET);
+                vbe_ioport_write_data(&s->vga, 0, offs / stride);
+                DPRINTF("VBE offset (%d,%d), vbe_start_addr=%x\n",
+                        s->vga.vbe_regs[VBE_DISPI_INDEX_X_OFFSET],
+                        s->vga.vbe_regs[VBE_DISPI_INDEX_Y_OFFSET],
+                        s->vga.vbe_start_addr);
             }
         }
     } else {
-- 
2.18.1


