bochs-emu / Bochs

Bochs - Cross Platform x86 Emulator Project
https://bochs.sourceforge.io/
GNU Lesser General Public License v2.1
875 stars 102 forks source link

Black dots in Windows 3.11 with CL-GD5446 driver #357

Closed Vort closed 2 weeks ago

Vort commented 2 weeks ago

After Windows 3.11 is loaded in 640x480x8 mode with CL-GD5446 driver v1.31, black dots can be seen in the top left corner of the screen: bochs_cirrus_dots

Zoomed in: image

Turned out, these dots are leftovers from cpu-to-video BLT operation. This place in code decides whether writes should go to the BitBLT engine or to the screen: https://github.com/bochs-emu/Bochs/blob/4a32e422309b087eb3a31b91eec3634fcc33174a/bochs/iodev/display/svga_cirrus.cc#L773-L780

The thing is driver sometimes makes dword write (4 bytes) while Bochs code expects to read 2 bytes (memsrc_needed). Which results in extra 2 bytes appearing on screen. I assume real hardware can't split dword write this way, that's why Bochs should discard extra data as well.

Version: 4a32e42.

vruppert commented 2 weeks ago

Please try whether or not these changes fix the issue.

--- /home/volker/Bochs/bochs/iodev/display/svga_cirrus.cc   2024-09-21 10:38:55.290641562 +0200
+++ ./iodev/display/svga_cirrus.cc  2024-10-03 17:47:22.270481668 +0200
@@ -735,14 +735,30 @@
 #else // BX_BIG_ENDIAN
   data_ptr = (Bit8u *) data + (len - 1);
 #endif
-  for (unsigned i = 0; i < len; i++) {
-    BX_CIRRUS_THIS mem_write(addr, *data_ptr);
-    addr++;
+  if (BX_CIRRUS_THIS bitblt.memsrc_needed > 0) {
+    // cpu-to-video BLT
+    for (unsigned i = 0; i < len; i++) {
+      *(BX_CIRRUS_THIS bitblt.memsrc_ptr)++ = *data_ptr;
+      if (BX_CIRRUS_THIS bitblt.memsrc_ptr >= BX_CIRRUS_THIS bitblt.memsrc_endptr) {
+        svga_asyncbitblt_next();
+        break;
+      }
+#ifdef BX_LITTLE_ENDIAN
+      data_ptr++;
+#else // BX_BIG_ENDIAN
+      data_ptr--;
+#endif
+    }
+  } else {
+    for (unsigned i = 0; i < len; i++) {
+      BX_CIRRUS_THIS mem_write(addr, *data_ptr);
+      addr++;
 #ifdef BX_LITTLE_ENDIAN
-    data_ptr++;
+      data_ptr++;
 #else // BX_BIG_ENDIAN
-    data_ptr--;
+      data_ptr--;
 #endif
+    }
   }
   return 1;
 }
@@ -770,15 +786,6 @@
         return;
       }

-      // cpu-to-video BLT
-      if (BX_CIRRUS_THIS bitblt.memsrc_needed > 0) {
-        *(BX_CIRRUS_THIS bitblt.memsrc_ptr)++ = (value);
-        if (BX_CIRRUS_THIS bitblt.memsrc_ptr >= BX_CIRRUS_THIS bitblt.memsrc_endptr) {
-          svga_asyncbitblt_next();
-        }
-        return;
-      }
-
       // BX_DEBUG(("write offset 0x%08x,value 0x%02x",offset,value));
       if ((BX_CIRRUS_THIS control.reg[0x0b] & 0x14) == 0x14) {
         offset <<= 4;
Vort commented 2 weeks ago

Please try whether or not these changes fix the issue.

Dots disappear, but text becomes corrupted.

Such variation however appears to work. But I'm not sure if other checks from mem_write like if (BX_CIRRUS_THIS pci_enabled) { and if ((addr >= BX_CIRRUS_THIS pci_bar[0].addr) && needs to be copied to cirrus_mem_write_handler as well.

diff --git a/bochs/iodev/display/svga_cirrus.cc b/bochs/iodev/display/svga_cirrus.cc
index 54432a04b..6ace045cd 100644
--- a/bochs/iodev/display/svga_cirrus.cc
+++ b/bochs/iodev/display/svga_cirrus.cc
@@ -735,14 +735,31 @@ bool bx_svga_cirrus_c::cirrus_mem_write_handler(bx_phy_address addr, unsigned le
 #else // BX_BIG_ENDIAN
   data_ptr = (Bit8u *) data + (len - 1);
 #endif
-  for (unsigned i = 0; i < len; i++) {
-    BX_CIRRUS_THIS mem_write(addr, *data_ptr);
-    addr++;
+  if (BX_CIRRUS_THIS bitblt.memsrc_needed > 0) {
+    // cpu-to-video BLT
+    for (unsigned i = 0; i < len; i++) {
+      if (BX_CIRRUS_THIS bitblt.memsrc_needed > 0) {
+        *(BX_CIRRUS_THIS bitblt.memsrc_ptr)++ = *data_ptr;
+        if (BX_CIRRUS_THIS bitblt.memsrc_ptr >= BX_CIRRUS_THIS bitblt.memsrc_endptr) {
+          svga_asyncbitblt_next();
+        }
+      }
 #ifdef BX_LITTLE_ENDIAN
-    data_ptr++;
+      data_ptr++;
 #else // BX_BIG_ENDIAN
-    data_ptr--;
+      data_ptr--;
+#endif
+    }
+  } else {
+    for (unsigned i = 0; i < len; i++) {
+      BX_CIRRUS_THIS mem_write(addr, *data_ptr);
+      addr++;
+#ifdef BX_LITTLE_ENDIAN
+      data_ptr++;
+#else // BX_BIG_ENDIAN
+      data_ptr--;
 #endif
+    }
   }
   return 1;
 }
@@ -770,15 +787,6 @@ void bx_svga_cirrus_c::mem_write(bx_phy_address addr, Bit8u value)
         return;
       }

-      // cpu-to-video BLT
-      if (BX_CIRRUS_THIS bitblt.memsrc_needed > 0) {
-        *(BX_CIRRUS_THIS bitblt.memsrc_ptr)++ = (value);
-        if (BX_CIRRUS_THIS bitblt.memsrc_ptr >= BX_CIRRUS_THIS bitblt.memsrc_endptr) {
-          svga_asyncbitblt_next();
-        }
-        return;
-      }
-
       // BX_DEBUG(("write offset 0x%08x,value 0x%02x",offset,value));
       if ((BX_CIRRUS_THIS control.reg[0x0b] & 0x14) == 0x14) {
         offset <<= 4; 
vruppert commented 2 weeks ago

Thanks. I had to ask you since I cannot reproduce the issue here. A check for 'pci_enabled' is not necessary since the cirrus_mem_write_handler() is only called in the PCI case. I'll apply the changes soon.

Vort commented 2 weeks ago

Thank you.

I had to ask you since I cannot reproduce the issue here.

Problem appeared with BitBLTs of 13x13 pixels.

00163985183d[CIRRUS] BLT: src:0x00000000,dst 0x00040949,block 13x13,mode 0x8c,ROP 0x0d
00163985183d[CIRRUS] BLT: srcpitch:0x00000000,dstpitch 0x00000280,modeext 0x00,writemask 0x00
00163985183d[CIRRUS] BLT redraw: x = 201, y = 413, w = 13, h = 13

Maybe my version of Windows have fonts of different sizes.