Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overlapping text in Windows 95 with CL-GD5430 #398

Closed
Vort opened this issue Nov 28, 2024 · 8 comments
Closed

Overlapping text in Windows 95 with CL-GD5430 #398

Vort opened this issue Nov 28, 2024 · 8 comments

Comments

@Vort
Copy link
Contributor

Vort commented Nov 28, 2024

With default driver, Cirrus Logic 5429/30/34/36, text is rendered incorrectly sometimes in Windows 95.
This problem appears with 8 or 16 bpp video modes. 24 bpp modes are fine.

Looks like text rendering should be done with some sort of masking/clipping, but it is not active.
Here is how problem looks like, with 8 bpp mode:
bochs_cir5430

Zoomed in:
image

Correct rendering, with 24 bpp mode:
image

Version: 63ff26c

@vruppert
Copy link
Contributor

I tried to reproduce this issue with both Win95 original and Win95 B using default drivers, but I don't see overlapping text here.

@Vort
Copy link
Contributor Author

Vort commented Nov 30, 2024

I found two things.

First, driver behaves weird in this case. It renders text with BitBLT first (BLT, cpu-to-video, transparent) and then writes the same text over it with bx_svga_cirrus_c::mem_write. Similar things (strange combination of memory writes and BLTs) happen in #387 as well. Probably these problems are related.

Second, similarily to #364, bx_svga_cirrus_c::mem_write is missing support for masking with SR2 Plane Mask Register. Here are changes which allow to get rid of text overlapping for me. Not sure if they create unwanted side effects however.

diff --git a/bochs/iodev/display/svga_cirrus.cc b/bochs/iodev/display/svga_cirrus.cc
index 893d70747..d33d3c0c0 100644
--- a/bochs/iodev/display/svga_cirrus.cc
+++ b/bochs/iodev/display/svga_cirrus.cc
@@ -468,36 +468,44 @@ void bx_svga_cirrus_c::redraw_area(unsigned x0, unsigned y0, unsigned width,
 
 void bx_svga_cirrus_c::mem_write_mode4and5_8bpp(Bit8u mode, Bit32u offset, Bit8u value)
 {
+  Bit8u sequ_map_mask = BX_CIRRUS_THIS sequencer.reg[0x02];
   Bit8u val = value;
   Bit8u *dst;
 
   dst = BX_CIRRUS_THIS s.memory + offset;
   for (int x = 0; x < 8; x++) {
-    if (val & 0x80) {
-      *dst = BX_CIRRUS_THIS control.shadow_reg1;
-    } else if (mode == 5) {
-      *dst = BX_CIRRUS_THIS control.shadow_reg0;
+    if (sequ_map_mask & 0x80) {
+      if (val & 0x80) {
+        *dst = BX_CIRRUS_THIS control.shadow_reg1;
+      } else if (mode == 5) {
+        *dst = BX_CIRRUS_THIS control.shadow_reg0;
+      }
     }
     val <<= 1;
+    sequ_map_mask <<= 1;
     dst++;
   }
 }
 
 void bx_svga_cirrus_c::mem_write_mode4and5_16bpp(Bit8u mode, Bit32u offset, Bit8u value)
 {
+  Bit8u sequ_map_mask = BX_CIRRUS_THIS sequencer.reg[0x02];
   Bit8u val = value;
   Bit8u *dst;
 
   dst = BX_CIRRUS_THIS s.memory + offset;
   for (int x = 0; x < 8; x++) {
-    if (val & 0x80) {
-      *dst = BX_CIRRUS_THIS control.shadow_reg1;
-      *(dst + 1) = BX_CIRRUS_THIS control.reg[0x11];
-    } else if (mode == 5) {
-      *dst = BX_CIRRUS_THIS control.shadow_reg0;
-      *(dst + 1) = BX_CIRRUS_THIS control.reg[0x10];
+    if (sequ_map_mask & 0x80) {
+      if (val & 0x80) {
+        *dst = BX_CIRRUS_THIS control.shadow_reg1;
+        *(dst + 1) = BX_CIRRUS_THIS control.reg[0x11];
+      } else if (mode == 5) {
+        *dst = BX_CIRRUS_THIS control.shadow_reg0;
+        *(dst + 1) = BX_CIRRUS_THIS control.reg[0x10];
+      }
     }
     val <<= 1;
+    sequ_map_mask <<= 1;
     dst += 2;
   }
 } 

@vruppert
Copy link
Contributor

vruppert commented Dec 1, 2024

After having a look at the Qemu and PCem sources, I decided to accept the suggested change. Qemu does not implement the extended SR2 usage at all, but PCem using similar code in the CL-GD5429 code. Having a look at it's memory write code may help us to verify the correctness of other suggested changes. I guess the currently missing features are also available in the PCI version, but the drivers use the bitblt engine instead of the legacy extended memory write features.

BTW: I tried to find some specs for the 5430, but I have only found a data sheet for the 543X family. It says that the 5430 is VLB or PCI and the 5434 is ISA only. Since the drivers are almost working, it seems that it doesn't matter.

@Vort
Copy link
Contributor Author

Vort commented Dec 1, 2024

It says that the 5430 is VLB or PCI and the 5434 is ISA only

I noticed that something is not right when I tried to search for 5430 ISA BIOS.
I only managed to find CL-GD5429 ISA BIOS (cl-gd5429.zip).

This is not good because it makes impossible to compare Bochs behaviour with behaviour of real hardware.
May it be better to switch Bochs from 5430 to different controller (5429 for example)?
Are there any scenarios when 5430 fits better than 5429?

Sadly, I don't have examples when 5429 may be better (except for theoretical tests on real hardware).
I know case when software supports 5428, but not 5429+ (3D Studio Release 4).
But 5428 is out of range for Windows 95 "Cirrus Logic 5429/30/34/36" driver.
So leaving 5430 support as is may be good option as well.

What do you think, @vruppert?

@vruppert
Copy link
Contributor

vruppert commented Dec 1, 2024

When we started implementing the Cirrus cards in Bochs I have only found a file called VGALOCAL.BIN for the ISA version. Now I had a look at this binary and found out that it is designed for the 542x family. Since the author of the Cirrus code for Bochs and Qemu also wrote the Cirrus extension for the LGPL VGABIOS, we did not use the original VGABIOS images for a long time.
Since it's hard to find docs for this legacy hardware, I don't know much about the differences of the 542x / 543x models. The PDF I mentioned has a lot of technical descriptions, but no programming notes. So I think it's okay to leave the Cirrus models as they are and fix bugs if we find some.

@Nable80
Copy link

Nable80 commented Dec 1, 2024

I noticed that something is not right when I tried to search for 5430 ISA BIOS.

I have a damaged i486 motherboard (a lot of things happened before I obtained it) with a CL-GD5428-80QC-A onboard.
I don't think it can be used for tests (although it still looks possible to restore it with significant efforts) but at least I can try to dump and upload its BIOS. Is it useful?

@vruppert
Copy link
Contributor

vruppert commented Dec 1, 2024

I don't know if it is useful to have such a BIOS. For emulating the 5428 it is required to compare the current code with the specs and add some extra code for the different models. Now I tried to find some stuff about the 5430 and found it at vgamuseum.info. As expected the BIOS file contains versions for VLB and ISA, but not for ISA. Booting to DOS works with both images. Since this mistake happend 20 years ago and most people using the Cirrus PCI, I think it's not necessary to change the Cirrus ISA model. But if I change would be required, I would prefer to use the 5434, since it's ISA only.

@Vort
Copy link
Contributor Author

Vort commented Dec 1, 2024

Since it's hard to find docs for this legacy hardware, I don't know much about the differences of the 542x / 543x models. The PDF I mentioned has a lot of technical descriptions, but no programming notes.

I don't know what you mean by "programming notes", but here are manuals and they are huge:

CL-GD542X, 621 pages, 24 MB:
http://www.bitsavers.org/components/cirrusLogic/graphics/CL-GD542X_Technical_Reference_Manual_Jan1994.pdf

CL-GD543X/'4X, 700 pages, 27 MB
http://vgamuseum.info/images/doc/cirrus/cl-gd543-4x_technical.pdf

Since this mistake happend 20 years ago and most people using the Cirrus PCI, I think it's not necessary to change the Cirrus ISA model. But if I change would be required, I would prefer to use the 5434, since it's ISA only.

5434 will allow to verify emulation correctness with real hardware, right.
However, another important property is software support:
The more software Bochs can run without hacks, the better.

So I think it's okay to leave the Cirrus models as they are and fix bugs if we find some.

Ok, let's leave it as is for now, until more information is collected.
I'm closing this issue, thanks.

@Vort Vort closed this as completed Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants