Add s3freak's 4 MiB S3 patch #1244

kcgen · 2021-09-03T19:46:15Z

This PR adds s3freak's excellent S3 patch to provide a similar vmemsize = setting like ECE and DOSBox-X have.

Care has been taken to ensure it's running clean using SciTech's VBETEST 5.3a testing tool, attached: sdd53a.zip. Simply unzip and launch this branch inside the unzipped directory.

Logging

A small feature is that the video adapter and memory details are provided (given vmem is now configurable):

machine = svga_paradise
# a couple vmemsize values

VIDEO: Initialized Paradise VGA 1A with 256 KiB of DRAM
VIDEO: Initialized Paradise VGA 1A with 512 KiB of DRAM

machine = svga_et3000
machine = svga_et4000
# a couple vmemsize values

VIDEO: Initialized Tseng Labs ET3000 with 512 KiB of DRAM
VIDEO: Initialized Tseng Labs ET4000 with 1 MiB of DRAM

machine = svga_s3
machine = vesa_nolfb
machine = vesa_oldvbe
# and various vmemsize values

VIDEO: Initialized S3 Trio 64 (VESA 2.0) with 512 KiB of EDO DRAM
VIDEO: Initialized S3 Trio 64 (VESA 2.0) without linear framebuffer modes with 8 MiB of EDO DRAM
VIDEO: Initialized S3 Trio 64 (VESA 1.2) with 2 MiB of EDO DRAM

Tall-mode handling

Tall VESA modes are those that are taller than they are wide, ie: 320x400.

When aspect = true, this branch will aspect-correct the image so things appear "correct" in terms of proportions.
For example, notice both VBETEST's graphically-drawn font is correctly proportioned as are the 3D structures and lettering inside Quake:

When aspect = false, tall modes will be stretched to "fit the CRT", which may result in distorted proportions.

We always provide information to the user so they can adjust as needed.

Fixes: #366

Reviewers: suggest reviewing commit-by-commit.

lgtm-com · 2021-09-03T20:03:51Z

This pull request introduces 12 alerts when merging e639f57 into 40f37ed - view on LGTM.com

new alerts:

12 for Multiplication result converted to larger type

src/hardware/hardware.cpp

src/hardware/vga_draw.cpp

Fixes: - warning: format specifies type 'int' but the argument has type 'Bitu' (aka 'unsigned long') [-Wformat] - LOG(LOG_VGA,LOG_NORMAL)("Blinking %d",enabled);

We know these Bitu's are only 32-bit on Win32 (and these don't involve pointers or memory addresses), so we can safely down-grade these to fixed sizes. Cleans up 20 warnings flagged on macOS Clang: vga_draw.cpp:1042:51: warning: format specifies type 'int' but the argument has type 'double' [-Wformat] vga_draw.cpp:1310:11: warning: format specifies type 'int' but the argument has type 'Bitu' (aka 'unsigned long') [-Wformat] vga_draw.cpp:1310:18: warning: format specifies type 'int' but the argument has type 'Bitu' (aka 'unsigned long') [-Wformat] vga_draw.cpp:1310:27: warning: format specifies type 'int' but the argument has type 'Bitu' (aka 'unsigned long') [-Wformat] vga_draw.cpp:1310:34: warning: format specifies type 'int' but the argument has type 'Bitu' (aka 'unsigned long') [-Wformat] vga_draw.cpp:1310:3: warning: format specifies type 'int' but the argument has type 'Bitu' (aka 'unsigned long') [-Wformat] vga_draw.cpp:1310:43: warning: format specifies type 'int' but the argument has type 'Bitu' (aka 'unsigned long') [-Wformat] vga_draw.cpp:1312:11: warning: format specifies type 'int' but the argument has type 'Bitu' (aka 'unsigned long') [-Wformat] vga_draw.cpp:1312:18: warning: format specifies type 'int' but the argument has type 'Bitu' (aka 'unsigned long') [-Wformat] vga_draw.cpp:1312:27: warning: format specifies type 'int' but the argument has type 'Bitu' (aka 'unsigned long') [-Wformat] vga_draw.cpp:1312:34: warning: format specifies type 'int' but the argument has type 'Bitu' (aka 'unsigned long') [-Wformat] vga_draw.cpp:1312:3: warning: format specifies type 'int' but the argument has type 'Bitu' (aka 'unsigned long') [-Wformat] vga_draw.cpp:1312:43: warning: format specifies type 'int' but the argument has type 'Bitu' (aka 'unsigned long') [-Wformat] vga_draw.cpp:1352:56: warning: format specifies type 'int' but the argument has type 'double' [-Wformat] vga_draw.cpp:1356:68: warning: format specifies type 'int' but the argument has type 'double' [-Wformat] vga_draw.cpp:1360:65: warning: format specifies type 'int' but the argument has type 'double' [-Wformat] vga_draw.cpp:1360:95: warning: format specifies type 'int' but the argument has type 'double' [-Wformat] vga_draw.cpp:1786:48: warning: format specifies type 'int' but the argument has type 'Bitu' (aka 'unsigned long') [-Wformat] vga_draw.cpp:1786:55: warning: format specifies type 'int' but the argument has type 'Bitu' (aka 'unsigned long') [-Wformat] vga_draw.cpp:819:40: warning: format specifies type 'int' but the argument has type 'Bitu' (aka 'unsigned long') [-Wformat]

Fixes many warnings of this type: "warning C4244: '=': conversion from 'Bitu' to 'uint32_t', possible loss of data" We know these sizes at most only hold 32-bit as that's their limit when compiled on Win32.

These are at-most hundreds of millions. Likewise, we also know on 32-bit platforms these Bitu values are only uint32's.

Per the S3 specification (last sentence): If bit 7 of CR68 is set to 1, the PD bus size for the Trio64 is determined by the memory size speci- fied in bits 7-5 of CR36. For a 1-MByte configura- tion, the bus size is 32 bits. For 2-MByte or larger configurations, the bus size is 64 bits. EDO mem- ory can be used for 1- or 2-MByte configurations. Fast page mode memory is required for 4-MByte configurations.

The previous version has silent expectations that the VGA mode enums would have specific values. For example, M_LIN8 was assumed to be 5. If M_LIN8 isn't 5 then the various bit-wise operations will produce unexpected results. Obviously this is extremely fragile, as changing the order of the enums (or inserting new items) will alter their underlying values. This commit lifts out those hardcoded expected values into local constexpr's, and then uses a switch statement to perform the mapping. This way, the original enum list can be changed as needed without any risk coming to the XGA code.

This will ease the comparison of multiple modes.

Also improve vmemsize conf setting by adding 'auto' as the default.

kcgen · 2021-09-13T19:31:19Z

Thanks to:

@GranMinigun for extensively testing many iterations of this PR.
@Grounded0 for historic perspective and providing VESA and S3 technical documents and mode tables.
@bmunger, @Grounded0, @MasterO2, @nemo93, @rderooy, and @shermp for sanity-checking and testing.

kcgen added the bug Something isn't working label Sep 3, 2021

kcgen requested review from kklobe and bmunger September 3, 2021 19:46

kcgen self-assigned this Sep 3, 2021

kcgen added this to In progress in 0.78 release via automation Sep 3, 2021

kcgen force-pushed the kc/4mb-video-memory-1 branch 5 times, most recently from 9098f71 to f165daf Compare September 3, 2021 23:51

dosbox-staging deleted a comment from lgtm-com bot Sep 4, 2021

kcgen force-pushed the kc/4mb-video-memory-1 branch 6 times, most recently from 986a3f5 to 9ee1cab Compare September 5, 2021 00:14

dosbox-staging deleted a comment from lgtm-com bot Sep 5, 2021

kcgen force-pushed the kc/4mb-video-memory-1 branch 3 times, most recently from e52faf2 to a28839c Compare September 5, 2021 16:55

kcgen added enhancement New feature or enhancement of existing features and removed bug Something isn't working labels Sep 5, 2021

kcgen force-pushed the kc/4mb-video-memory-1 branch from a28839c to 7a26dc2 Compare September 5, 2021 17:39

dosbox-staging deleted a comment from lgtm-com bot Sep 5, 2021

kklobe reviewed Sep 6, 2021

View reviewed changes

src/hardware/hardware.cpp Outdated Show resolved Hide resolved

kklobe reviewed Sep 6, 2021

View reviewed changes

src/hardware/vga_draw.cpp Outdated Show resolved Hide resolved

kcgen added 12 commits September 13, 2021 09:50

De-Bitu the VGA blinking function as it only is given an 8-bit value

4b3a47e

Fixes: - warning: format specifies type 'int' but the argument has type 'Bitu' (aka 'unsigned long') [-Wformat] - LOG(LOG_VGA,LOG_NORMAL)("Blinking %d",enabled);

De-Bitu some VGA element sizes to fix some VS2019 warnings

003a246

Fixes many warnings of this type: "warning C4244: '=': conversion from 'Bitu' to 'uint32_t', possible loss of data" We know these sizes at most only hold 32-bit as that's their limit when compiled on Win32.

De-Bitu setting and getting VGA clock frequencies

93f6273

These are at-most hundreds of millions. Likewise, we also know on 32-bit platforms these Bitu values are only uint32's.

De-Bitu an 8-bit blink value to fix a VS2019 warning

8201cf6

Initialize VGA struct elements with defaults

5c1548d

Fix a warning initializing the VGA_Tandy struct member

ac78386

De-Bitu and refactor VGA_SetClock to fix LGTM warnings

ac0cb2b

Improve comment regarding 24-bit color and ZMBV

df49ee8

Make VGAModes enum elements boolean-compatible

80c4475

This will ease the comparison of multiple modes.

kcgen force-pushed the kc/4mb-video-memory-1 branch from 790e4df to 7959308 Compare September 13, 2021 17:26

kcgen added 2 commits September 13, 2021 11:08

Add VESA mode compatilibity settings and prefer 70+ VESA refresh

e31fd4c

Also improve vmemsize conf setting by adding 'auto' as the default.

Update formatting of VGA mode tables

16257c1

kcgen force-pushed the kc/4mb-video-memory-1 branch from 7959308 to 3d9679f Compare September 13, 2021 18:12

Snip duplicate VESA mode 0x154

d7ac0b2

kcgen force-pushed the kc/4mb-video-memory-1 branch 2 times, most recently from 29addb0 to 420ebcb Compare September 13, 2021 19:03

ykhwong and others added 3 commits September 13, 2021 12:08

Add Sierra SC1148x Hi-Color DAC modes for Tseng

ff3fb7d

Use original VGA SetClock algorithm with doubler flags

7ccf8ae

Lower allowed warnings limits

93a63d9

kcgen force-pushed the kc/4mb-video-memory-1 branch from 420ebcb to 93a63d9 Compare September 13, 2021 19:20

kcgen merged commit 215a1ec into master Sep 13, 2021

0.78 release automation moved this from In progress to Done Sep 13, 2021

kcgen deleted the kc/4mb-video-memory-1 branch September 18, 2021 19:35

kcgen added this to the 0.78 release milestone Dec 21, 2021

dosbox-staging deleted a comment from lgtm-com bot Dec 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add s3freak's 4 MiB S3 patch #1244

Add s3freak's 4 MiB S3 patch #1244

kcgen commented Sep 3, 2021 •

edited

Loading

lgtm-com bot commented Sep 3, 2021

kcgen commented Sep 13, 2021

Add s3freak's 4 MiB S3 patch #1244

Add s3freak's 4 MiB S3 patch #1244

Conversation

kcgen commented Sep 3, 2021 • edited Loading

Logging

Tall-mode handling

lgtm-com bot commented Sep 3, 2021

kcgen commented Sep 13, 2021

kcgen commented Sep 3, 2021 •

edited

Loading