superfury wrote on 2021-06-30, 16:40:

Edit: Oh! It reaches C000:03D5! So that means that it's counted the amount of enables in the specified time interval and found it didn't match?

I'm just investigating that code. C000:03D5 is EGA POST error 5, but it seems BL is ignored by the POST error handler at C000:0448.

Well, it's still weird that it literally counts 0 horizontal retraces (instead of 350 it's supposed to see)!

Oh, the good old days when a POST really did a test of the hardware (or your emulator)...

superfury wrote on 2021-06-30, 16:40:

Edit: Oh! It reaches C000:03D5! So that means that it's counted the amount of enables in the specified time interval and found it didn't match?
EBX is C0000 at that point. [BP+02] contains 15E.

It's not "time interval". That code is counting how many horizontal display periods (3D8 bit 0 is zero) occur during the vertical display period (3D8 bit 3 is zero). As you are in a 350-line text mode, [BP+02] contains 350. Horizontal displayed characters is intentionally only half the expected value at that point, so that 3D8, bit zero has approximately 50% duty cycle, so the code does not miss either a display or a blanking period.

If you get there with BX=0000, the code either counted 65536 lines (jz at 3C6 triggered), or a vertical blanking indication was seen at 3A0 (jump at 3A7) just after the vertical blanking indication was not seen at 392 (jump at 395). Either way, your emulator does not seem to present proper video timing in the blanking status bits.

EDIT: you might want to test whether C000:03C6 is reached.

OK. C000:03C3 is reached at least once (CX=FFDE)..

OK. At C000:03C6, BX eventually ends up wrapping to 0000h?

superfury wrote on 2021-06-30, 18:58:

OK. C000:03C3 is reached at least once (CX=FFDE)..

OK. At C000:03C6, BX eventually ends up wrapping to 0000h?

First things first: EGA has the "blanking enable" bit just like CGA and VGA. Bit 0 in the status register set means blanking (horizontal OR vertical), whereas Bit 0 clear means display in progress. Line 490 in vga_io.c (as of commit dda05a) is definitely wrong.

The timing verification code in the EGA BIOS checks bit 3 (vertical blanking) only when the "master blanking" bit (bit 0) is indicating blanking. Your inversion of bit 0 causes for vblank checks only during the display period.

mkarcher wrote on 2021-06-30, 19:09:

superfury wrote on 2021-06-30, 18:58:

OK. C000:03C3 is reached at least once (CX=FFDE)..

OK. At C000:03C6, BX eventually ends up wrapping to 0000h?

First things first: EGA has the "blanking enable" bit just like CGA and VGA. Bit 0 in the status register set means blanking (horizontal OR vertical), whereas Bit 0 clear means display in progress. Line 490 in vga_io.c (as of commit dda05a) is definitely wrong.

The timing verification code in the EGA BIOS checks bit 3 (vertical blanking) only when the "master blanking" bit (bit 0) is indicating blanking. Your inversion of bit 0 causes for vblank checks only during the display period.

Huh? It's the same as the VGA?
But the documentation says (page 20, 16 in the document):

1Display Enable-Logical 0 indicates the CRT
2raster is in a horizontal or vertical retrace
3interval. This bit is the real time status of the
4display enable signal. Some programs use this
5status bit to restrict screen updates to inactive
6display intervals. The Enhanced Graphics
7Adapter does not require the CPU to update the
8screen buffer during inactive display intervals to .~
9avoid glitches in the display image. 

So that would be the opposite of what the FreeVGA documentation says? It tells a Logical 0(binary 0?) is retrace and 1 is active display. So that's why the toggle in the code exists (the opposite of the VGA handling of said bit)?

hardware/vga/vga_renderer.c:2277 sets the DisplayEnabled status to 1 when active display and not retracing. 0 otherwise.
Then, for every block of pixels handled (depending on timing), it updates the bit 0 of the register to become the inverse of said signal for the VGA(so 0 for display enabled and 1 for disabled or retracing).
The VGA takes that like that:

DD -- Display Disabled
"When set to 1, this bit indicates a horizontal or vertical retrace interval. This bit is the real-time status of the inverted 'display enable' signal. Programs have used this status bit to restrict screen updates to the inactive display intervals in order to reduce screen flicker. The video subsystem is designed to eliminate this software requirement; screen updates may be made at any time without screen degradation."

Then, the EGA handling of the input status #1 register flips that bit again to become 1 for enabled and 0 for retracing (as the EGA documentation says).

Edit: OK. Now C000:03CC is reached. Then I see it reach C000:03F6.

OK. So now probably something's going wrong with the red/green/blue/intensify dots?

This is how it parses the DAC input (the DAC itself is reduced to the defined 64-color lookup table for EGA only). The DACOutput variable will contain the outputted DAC color from the attribute controller.

1			const static byte bittablelow[2][4] = {{0,4,1,6},{0,4,1,8}}; //Bit 6 is undefined on EGA!
2			const static byte bittablehigh[2][4] = {{2,5,3,7},{2,5,3,8}}; //Bit 7 is undefined on EGA!
3			byte DACOutput = getActiveVGA()->CRTC.DACOutput; //Current DAC output to give!
4			SETBITS(*result,4,1,GETBITS(DACOutput,bittablelow[(getActiveVGA()->enable_SVGA==3)?1:0][GETBITS(getActiveVGA()->registers->AttributeControllerRegisters.REGISTERS.COLORPLANEENABLEREGISTER,4,3)],1));
5			SETBITS(*result,5,1,GETBITS(DACOutput,bittablehigh[(getActiveVGA()->enable_SVGA==3)?1:0][GETBITS(getActiveVGA()->registers->AttributeControllerRegisters.REGISTERS.COLORPLANEENABLEREGISTER,4,3)],1));

For reference, the bit macros:

1#define GETBITS(x,shift,mask) ((x&(mask<<shift))>>shift)
2#define SETBITS(x,shift,mask,val) x=((x&(~(mask<<shift)))|(((val)&mask)<<shift))

Edit: Just switched the middle 2 values (4/1 and 5/3) of the video mux to match EGA documentation. Still need to check the VGA ones though.

Edit: It seems the VGA swaps the middle two bit numbers? And the VGA adds bits 6/7 to the mix.

superfury wrote on 2021-06-30, 19:31:

Huh? It's the same as the VGA? But the documentation says (page 20, 16 in the document): […]
Show full quote

mkarcher wrote on 2021-06-30, 19:09:

First things first: EGA has the "blanking enable" bit just like CGA and VGA. Bit 0 in the status register set means blanking (horizontal OR vertical), whereas Bit 0 clear means display in progress. Line 490 in vga_io.c (as of commit dda05a) is definitely wrong.

The timing verification code in the EGA BIOS checks bit 3 (vertical blanking) only when the "master blanking" bit (bit 0) is indicating blanking. Your inversion of bit 0 causes for vblank checks only during the display period.

Huh? It's the same as the VGA?
But the documentation says (page 20, 16 in the document):

Copy code to clipboard

1Display Enable-Logical 0 indicates the CRT
2raster is in a horizontal or vertical retrace
3interval. [...] 

So that would be the opposite of what the FreeVGA documentation says? It tells a Logical 0(binary 0?) is retrace and 1 is active display. So that's why the toggle in the code exists (the opposite of the VGA handling of said bit)?

+1 to you for actually reading the original IBM EGA documentation.

-1 to IBM for documenting this bit wrong. Take a look at the BIOS listing comments on 03A1/03A3: It should jump to POD14_5 "when enable going on", and then checks for "vertical went off". The jump to POD14_5 happens when the bit is zero.

OK. After some edits, this is what I've got on the video mux inputs it tries to use:

1			//VGA according to the document Second Sight VGA registers
2			const static byte bittablelow[2][4] = {{0,4,1,6},{0,1,4,8}}; //Bit 6 is undefined on EGA!
3			const static byte bittablehigh[2][4] = {{2,5,3,7},{2,3,5,8}}; //Bit 7 is undefined on EGA!
4			byte DACOutput = getActiveVGA()->CRTC.DACOutput; //Current DAC output to give!
5			SETBITS(*result,4,1,GETBITS(DACOutput,bittablelow[(getActiveVGA()->enable_SVGA==3)?1:0][GETBITS(getActiveVGA()->registers->AttributeControllerRegisters.REGISTERS.COLORPLANEENABLEREGISTER,4,3)],1));
6			SETBITS(*result,5,1,GETBITS(DACOutput,bittablehigh[(getActiveVGA()->enable_SVGA==3)?1:0][GETBITS(getActiveVGA()->registers->AttributeControllerRegisters.REGISTERS.COLORPLANEENABLEREGISTER,4,3)],1));

Would that be correct behaviour?

superfury wrote on 2021-06-30, 19:46:

OK. So now probably something's going wrong with the red/green/blue/intensify dots?

Edit: Just switched the middle 2 values (4/1 and 5/3) of the video mux to match EGA documentation. Still need to check the VGA ones though.

Edit: It seems the VGA swaps the middle two bit numbers? And the VGA adds bits 6/7 to the mix.

My reference (as well as VGADOC from WHATVGA) claims that these bits (4/5 in "Input status #1") do not exist on VGA at all. If 6/7 is added, it seems to be either a not widely known undocumented feature of the VGA, or some SVGA extension.

superfury wrote on 2021-06-30, 20:14:

OK. After some edits, this is what I've got on the video mux inputs it tries to use: […]
Show full quote

OK. After some edits, this is what I've got on the video mux inputs it tries to use:

Copy code to clipboard

1			//VGA according to the document Second Sight VGA registers
2			const static byte bittablelow[2][4] = {{0,4,1,6},{0,1,4,8}}; //Bit 6 is undefined on EGA!
3			const static byte bittablehigh[2][4] = {{2,5,3,7},{2,3,5,8}}; //Bit 7 is undefined on EGA!
4			byte DACOutput = getActiveVGA()->CRTC.DACOutput; //Current DAC output to give!
5			SETBITS(*result,4,1,GETBITS(DACOutput,bittablelow[(getActiveVGA()->enable_SVGA==3)?1:0][GETBITS(getActiveVGA()->registers->AttributeControllerRegisters.REGISTERS.COLORPLANEENABLEREGISTER,4,3)],1));
6			SETBITS(*result,5,1,GETBITS(DACOutput,bittablehigh[(getActiveVGA()->enable_SVGA==3)?1:0][GETBITS(getActiveVGA()->registers->AttributeControllerRegisters.REGISTERS.COLORPLANEENABLEREGISTER,4,3)],1));

Would that be correct behaviour?

Seems correct for EGA. I don't have any documentation for these bits on VGA at hand.

Well, somehow that test seems to be the one failing?

superfury wrote on 2021-06-30, 20:22:

Well, somehow that test seems to be the one failing?

Do you make sure the AC output is zero during blanking? The code writes white characters to the screen and tests for seeing both "bit on" (which should happen during display) and "bit off" (which should happen during blanking). If you keep the last color pixel color on the status bits during blanking, that's the cause of the failure.

If the EGA POST fails, C000:0448 is reached. The value in BL contains the POST code. Knowing the value helps to understand why the POST failed.

And let's call the POST POET for now ("Power On Emulator Test") 😉

mkarcher wrote on 2021-06-30, 20:28:

Do you make sure the AC output is zero during blanking? The code writes white characters to the screen and tests for seeing both […]
Show full quote

superfury wrote on 2021-06-30, 20:22:

Well, somehow that test seems to be the one failing?

Do you make sure the AC output is zero during blanking? The code writes white characters to the screen and tests for seeing both "bit on" (which should happen during display) and "bit off" (which should happen during blanking). If you keep the last color pixel color on the status bits during blanking, that's the cause of the failure.

If the EGA POST fails, C000:0448 is reached. The value in BL contains the POST code. Knowing the value helps to understand why the POST failed.

And let's call the POST POET for now ("Power On Emulator Test") 😉

Now that you're saying that... It indeed doesn't clear said status to all bits off when the retraces are occurring.
OK. I now see the attribute controller eventually sending color 0xF to the 64-color output...

superfury wrote on 2021-06-30, 20:34:

Now that you're saying that... It indeed doesn't clear said status to all bits off when the retraces are occurring.
OK. I now see the attribute controller eventually sending color 0xF to the output...

Then something is still wrong. The attribute register 0x0F is set to 0x3F in C000:0409, so the AC should output the 6-bit color 3Fh (all bits on) during the display period. On the other hand, the input to the AC is the 4-bit color 0Fh, indeed.

Once the color 0x3F is output, the code reads the mux bits 4-5 both set.

superfury wrote on 2021-06-30, 21:07:

Once the color 0x3F is output, the code reads the mux bits 4-5 both set.

Which is correct during the display period. During the blanking period, the code expects the mux bits 4-5 to be clear.

I've just changed that. Now I'm running through the code at one of the two loops there where the DAC output is 0x32...

superfury wrote on 2021-06-30, 21:20:

I've just changed that. Now I'm running through the code at one of the two loops there where the DAC output is 0x32...

Oops, in that case you seem to got the DAC index/value mixed up. 0x32 is the index value for the color plane select register (0x12) ORed together with the display enable bit (0x20). That value should never end up in a color register. Remember that writing a data value to the AC needs to put it back into index mode.

mkarcher wrote on 2021-06-30, 21:23:

superfury wrote on 2021-06-30, 21:20:

I've just changed that. Now I'm running through the code at one of the two loops there where the DAC output is 0x32...

Oops, in that case you seem to got the DAC index/value mixed up. 0x32 is the index value for the color plane select register (0x12) ORed together with the display enable bit (0x20). That value should never end up in a color register. Remember that writing a data value to the AC needs to put it back into index mode.

Well, that's what it's been doing all along:

1OPTINLINE void PORT_write_ATTR_3C0(byte value) //Attribute controller registers!
2{
3	if (!VGA_3C0_FLIPFLOPR) //Index mode?
4	{
5		value &= VGA_RegisterWriteMask_AttributeIndex[(getActiveVGA()->enable_SVGA==3)?1:0]; //Apply the write mask to the data written to the register!
6		//Mirror to state register!
7		VGA_3C0_PALW((value&0x20)>>5); //Palette Address Source!
8		VGA_3C0_INDEXW(value&0x1F); //Which index?
9		VGA_calcprecalcs(getActiveVGA(),WHEREUPDATED_INDEX|INDEX_ATTRIBUTECONTROLLER); //Updated index!
10	}
11	else //Data mode?
12	{
13		if ((VGA_3C0_INDEXR >= 0x10) || (VGA_3C0_PALR == 0)) //Palette writable or not palette?
14		{
15			if (VGA_3C0_INDEXR < VGA_RegisterWriteLimits_Attribute[(getActiveVGA()->enable_SVGA == 3) ? 1 : 0]) //Within range?
16			{
17				value &= VGA_RegisterWriteMasks_Attribute[(getActiveVGA()->enable_SVGA == 3) ? 1 : 0][VGA_3C0_INDEXR]; //Apply the write mask to the data written to the register!		
18				getActiveVGA()->registers->AttributeControllerRegisters.DATA[VGA_3C0_INDEXR] = value; //Set!
19				VGA_calcprecalcs(getActiveVGA(), WHEREUPDATED_ATTRIBUTECONTROLLER | VGA_3C0_INDEXR); //We have been updated!
20			}
21		}
22	}
23
24	VGA_3C0_FLIPFLOPW(!VGA_3C0_FLIPFLOPR); //Flipflop!
25	VGA_calcprecalcs(getActiveVGA(), WHEREUPDATED_CRTCONTROLLER | VGA_CRTC_ATTRIBUTECONTROLLERTOGGLEREGISTER); //Our actual location!
26}