dirkwhoffmann / vAmiga

vAmiga is a user-friendly Amiga 500, 1000, 2000 emulator for macOS
https://dirkwhoffmann.github.io/vAmiga
Other
297 stars 25 forks source link

RAM init pattern #586

Closed dirkwhoffmann closed 2 years ago

dirkwhoffmann commented 2 years ago

@chris70c has just shown that the "Random" RAM init pattern causes trouble with a variety of demos.

Shall we simply remove this RAM pattern? Or should we remove the RAM pattern option altogether and initialise RAM with zeroes, no matter what?

chris70c commented 2 years ago

I remember Toni having some similar issues in WinUAE because it is not clearly understood how exactly the RAM is initialized at power on; I guess we can leave it there but set All zeroes by default, in the future it might become useful if/when the RAM initialization mechanism is found.

mras0 commented 2 years ago

Wouldn't most DRAM be zero on startup (at least after being powered down for "long")? Of course it's formally undefined and it shouldn't be depended upon, but it seems like many things do. If either of you have a quick way to test disks, what does the following show that just lets BPL1PT run wild in 1 BPL screen from the bootblock (to disturb things minimally)? ramvis.zip

If that doesn't show anything except the very occasional flash of data from interrupt vectors DF0/DF1 buffers (test could be modified to clear those explicitly), I'd probably only leave that options as an advanced setting only useful for developers. Perhaps it could be useful for the odd game/demo that only happens to work if non-zero values are present, but then you could debug it by loading something else first.

Just my 2 cents.

dirkwhoffmann commented 2 years ago

I just run ramvis on my A500 8A. As expected, the screen is black most of the time with occasional flashes:

https://user-images.githubusercontent.com/12561945/140610985-f14904d3-188a-4132-9965-d7c4b11cacd8.mov

I also think that we should keep the option (at least internally), because the code is already there and it could be useful for debugging. I might remove it from the GUI eventually in case the Hardware configuration panel becomes too cluttered.

mras0 commented 2 years ago

Actually, maybe the issue is something else. I looked at the disassembly of the startup of the KS1.2 ROM and it should clear both chip and slow ram using the routine at $FC0602 (calls are at $FC01F8 and $FC0222). And KS1.3 does the same thing (and addresses match for this part).

I checked how https://github.com/dirkwhoffmann/vAmiga/issues/582 detects NTSC, and it simply does:

    CMP.B   #$32, VBlankFrequency(A0)   ; A0 = SysBase (struct ExecBase)
    BEQ.B   lab_000400ac

Try to see what the byte at $c00488 (execbase + $0212 (VBlankFrequency)) is when the random fill pattern is enabled (in 512K chip+512K slow config). It should be either $32 (PAL) or $3C (NTSC). In KS1.3 the value is written by the code at either $fcae56 or $fcae5e (part of the init routine for graphics.library starting at $fcaba2).

dirkwhoffmann commented 2 years ago

Try to see what the byte at $c00488

I've done some experiments with the latest build:

Pattern Kick 1.2 Kick 1.3
All zeroes 32 32
Random 32 3C
All ones 32 3C

Bottom line: When using an init pattern other than "All zeroes", Kickstart 1.3 will detect a NTSC machine.

dirkwhoffmann commented 2 years ago

Since it has become clear that ALL_ZEROES must be the default pattern, I am going to rearrange

enum_long(RAM_INIT_PATTERN)
{
    RAM_INIT_RANDOMIZED,
    RAM_INIT_ALL_ZEROES,
    RAM_INIT_ALL_ONES
};

to

enum_long(RAM_INIT_PATTERN)
{
    RAM_INIT_ALL_ZEROES,
    RAM_INIT_ALL_ONES,
    RAM_INIT_RANDOMIZED
};

Other than that, is there anything else to do? Shall I close the issue?

mras0 commented 2 years ago

Bottom line: When using an init pattern other than "All zeroes", Kickstart 1.3 will detect a NTSC machine.

Seems weird, I don't see this if I modify memory_clear in latest WinUAE to clear the memory to e.g. 0xA5, 0xFF or random values (Note: I'm lazy and the routine is only called on first boot, not after reset).

Other than that, is there anything else to do? Shall I close the issue?

It's your bug tracker, so you can do what you want :) , but I suspect you still have a bug. Like I wrote above, KS should clear the memory. I don't have any ideas as to what might cause it, but maybe you could compare traces of the two different situations and see where they diverge.

mras0 commented 2 years ago

Had some time to do a few quick test (will try to get "my" vAmigaSDL up to date at some point). I suspect it has something to do with the custom mirroring/slow ram stuff. Tested w/ KS 1.3 and 1.1b9. (I added a hack in Memory::setConfigItem to allow > 512K slow ram size). It's only slow RAM initialization that seems to affect the result.

Slow RAM configured Results
0K OK
256K OK
512K Not OK
768K Not OK
1024K OK
1280K OK
1536 OK

Test code:

VBlankFrequency=$0212
dmacon=$096
intena=$09a
color=$180

bootblock_start:
        dc.b 'DOS', 0   ; Disk type + flags
        dc.l 0          ; Checksum
        dc.l 880        ; Root block

        ; a1 = IO Request, a6 = SysBase
start:
        move.w  #$7fff, d0
        move.l  #$dff000, a5
        move.w  d0, intena(a5)
        move.w  d0, dmacon(a5)

        move.b  VBlankFrequency(a6), d1
        move.w  #$f00, d0
        cmp.b   #$3c, d1
        beq.s   .halt
        move.w  #$0f0, d0
        cmp.b   #$32, d1
        beq.s   .halt
        move.w  #$00f, d0
.halt:
        move.w  d0, color(a5)
        jmp .halt

ADF: ntsccheck.zip

dirkwhoffmann commented 2 years ago

I'm not 100% sure, but maybe we're simply hitting a bug in Kickstart here: https://retrocomputing.stackexchange.com/questions/1387/why-would-a-pal-amiga-sometimes-start-up-in-ntsc-display-mode

As the article explains, Kickstart tries to reach a scanline that is unreachable on NTSC machines. If it succeeds, it assumes a PAL machine, otherwise a NTSC machine. With POSPREG_DEBUG set to 1, I see the following with startup patternALL_ZEROES:

[105] (271, 34) FC5ECE  0 --B-D- 202C 0040 Agnus:266 peekVPOSR() = 2001
[105] (271, 36) FC5ECE  0 --B-D- 202C 0040 Agnus:207 peekVHPOSR() = 0f28
[105] (273, 21) FCAF72  0 --B-D- 602C 0040 Agnus:207 peekVHPOSR() = 1119
[105] (273, 49) FCAF86  0 --B-D- 602C 0040 Agnus:266 peekVPOSR() = 2001
[105] (273, 63) FCAF90  0 --B-D- 602C 0040 Agnus:273 pokeVPOS(a001)
[105] (274, 11) FCAF72  0 BCBSD- 602C 0040 Agnus:207 peekVHPOSR() = 120f
[105] (274, 38) FCAF86  0 BCBSD- 602C 0040 Agnus:266 peekVPOSR() = a001
[105] (274, 52) FCAF90  0 BCBSD- 602C 0040 Agnus:273 pokeVPOS(a001)
[106] (  1, 69) FC6D86  3 BCBSD- 602C 0060 Agnus:207 peekVHPOSR() = 0149
[108] (  7,110) FC6D86  3 BCBSD- 602C 0060 Agnus:207 peekVHPOSR() = 0772

The routine reaches scanline 274 and concludes correctly to run on a PAL machine. With init pattern ALL_ONES, cycle distribution is a bit different which results in the following run:

[105] (255,151) FC5ECE  0 --B-D- 202C 0040 Agnus:266 peekVPOSR() = 2000
[105] (255,153) FC5ECE  0 --B-D- 202C 0040 Agnus:207 peekVHPOSR() = ff9d
[105] (255,223) FC5ECE  0 --B-D- 202C 0040 Agnus:266 peekVPOSR() = 2000
[105] (255,225) FC5ECE  0 --B-D- 202C 0040 Agnus:207 peekVHPOSR() = 0002
[105] (257,206) FCAF72  0 --B-D- 602C 0040 Agnus:207 peekVHPOSR() = 01d2
[105] (258,  7) FCAF86  0 --B-D- 602C 0040 Agnus:266 peekVPOSR() = 2001
[105] (258, 21) FCAF90  0 --B-D- 602C 0040 Agnus:273 pokeVPOS(a001)
[105] (258,196) FCAF72  0 BCBSD- 602C 0040 Agnus:207 peekVHPOSR() = 02c8
[105] (258,223) FCAF86  0 BCBSD- 602C 0040 Agnus:266 peekVPOSR() = a001
[105] (259, 10) FCAF90  0 BCBSD- 602C 0040 Agnus:273 pokeVPOS(a001)
[106] (  1, 64) FC6D86  3 BCBSD- 602C 0060 Agnus:207 peekVHPOSR() = 0144
[108] (  7,125) FC6D86  3 BCBSD- 602C 0060 Agnus:207 peekVHPOSR() = 0781

In scanline 255, Kickstart happens to scan VPOSR in cycle 223 and VHPOSR in cycle 225. In these cycles, strange values are given back because the horizontal and the vertical counter wrap over differently. I guess the function gets confused by the returned values and bails out early in this scanline. This is just a hypothesis though. It is well possible that it's still a bug, caused by something similar to #649 or #643. I.e., it could be possible that the CPU would't get cycle 223 or 225 on the real machine in the same scenario.

Yesterday, I found a bug in peekVPOSR() which I assumed to be related to this issue at first. Unfortunately, it didn't fix it. With an init pattern other than ALL_ZEROES, vAmiga still detects an NTSC machine.

mras0 commented 2 years ago

OK, yeah, this seems to be a known bug in 1.3. The off hand comment by Toni here "there is always 1/227 chance of NTSC misdetection" highly suggest that unlucky alignment of V(H)POSR reads are the cause. Another comment on EAB by Ross seems to indicate that it's in all 1.x releases (and that seems likely, 1.2 detection code is identical except for not saving/restoing D2. I think 1.1 and earlier had different ROMs for PAL/NTSC).

For reference here's an annotated disassembly (interrupts and important DMA are disabled at this point):

DetectPAL                           ; $00fcb00c
    MOVE.L  D2, -(A7)
    JSR VBeamPos
    CMP.L   #$0000010e, D0
    BLE.B   .WaitLine100
.IsPAL
    MOVEQ.L #$04, D0 ; PAL returned  (for GfxBase->DisplayFlags)
    BRA.B   .Out
.WaitLine100
    JSR VBeamPos
    MOVE.L  D0, D2
    MOVEQ.L #$64, D0
    CMP.L   D2, D0
    BGT.B   .WaitLine100
.LineGT50
    JSR VBeamPos
    MOVE.L  D0, D1
    CMP.L   #$0000010e, D1
    BLE.B   .NotDecided
    BRA.B   .IsPAL
.NotDecided
    MOVEQ.L #$32, D0
    CMP.L   D1, D0
    BLT.B   .LineGT50
    MOVEQ.L #$01, D0 ; NTSC
.Out
    MOVE.L  (A7)+, D2
    RTS

; ...

VBeamPos                            ; $00fc5ece
    MOVE.L  vposr, D0
    ASR.L   #$08, D0
    AND.L   #$000001ff, D0
    RTS

Just before calling this routine it's checked if GENLOCK is enabled (which puts a bound on what VPOS can be, so I don't think the first check actually matters). It then waits for line 100 and then checks if line 270 or 50 is seen first. If the unlucky rollover happens (I didn't know about that, interesting stuff) it'll terminate the loop too early and decide the machine is NTSC. Looks like was fixed in later KS versions by first waiting until VPOS >= 256 (instead of 100).

I've changed my mind, and think you should probably close this issue (again at your discretion). There might still be some timing bugs, but they're unlikely to be helped by having this one open :)

Note though that you may want to re-check some of the vamigats/cputester/cycles test cases. They're super picky about V(H)POSR timing changes and I just checked lastest dev branch code and e.g. MOVEA.L_cycles.adf fails.

dirkwhoffmann commented 2 years ago

@mras0: Thanks a lot for the explanations and the annotated disassembly. Yesterday, I stepped through the code in the debugger and I was surprised how complex it is.

MOVEA.L_cycles.adf fails.

This test (as all other cycle related tests) requires to run in Fast RAM. With 512KB Chip + 0KB Slow + 8MB Fast, it still works:

Bildschirmfoto 2022-02-09 um 22 39 47

Note though that you may want to re-check some of the vamigats/cputester/cycles test cases

What I definitely need to do is to improve test automisation. The test suite already contains Makefiles capable of running many tests automatically and comparing the result with reference images. However, a lot of test cases are missing test scripts yet. Because writing these scripts (and verifying the created reference image) is kind of a boring task, I haven't done it at an earlier stage.

mras0 commented 2 years ago

With 512KB Chip + 0KB Slow + 8MB Fast

D'oh, I'd forgotten they were picky about that part as well. Sorry for the false alarm. No need to re-check then, it was only because you mentioned a VPOSR change that I ran one of them.

For running the cputester I found basic harddrive support really sped things up (both loading and not needing to reboot between tests), but probably not worth looking in to that either at this point since they probably won't catch anything.

dirkwhoffmann commented 2 years ago

Sorry for the false alarm

No problem. I run into the same thing a while ago. It should be documented better.

For running the cputester I found basic harddrive support really sped things up

Rudimentary hard drive support is really something I'd like to integrate eventually. I tried this before, but I gave up at some point. I remember that you mentioned a couple a month ago that adding basic hard drive support is not that complicated. Could you point me to some docs/specs/code/whatever where I can see how this can be achieved? Running cputester with ADFs is really cumbersome, because the test cases need to be split up in very small chunks, and even then, some of the small chunks only fit on HD disks.

mras0 commented 2 years ago

High level: Create an autoconfig enabled expansion board with a small boot ROM that contains a small driver and adds a device to the OS. I used the example device in the RKM manual as a basis. The driver then hands off IORequests to the emulator and the read/writes can then be processed in C++ code.

That's also how it works in WinUAE with uaehf.device (filesys.asm contains the boot ROM code and it talks to filesys.cpp and a few other files). That code is pretty complicated because it supports multiple HDs (you implement these as extra units), mounting native directories, adding >2MB chip RAM, mouse stuff, RDB (for partitions) and extra complicated stuff to allow autobooting under KS versions earlier than 1.3.

My expansion ROM is much simpler since it doesn't support all that: https://github.com/mras0/AmiEmu/blob/master/exprom.asm and corresponding device code: https://github.com/mras0/AmiEmu/blob/master/harddisk.cpp

The device driver talks to the C++ code by writing just beyond the end of the ROM code (still in expansion memory). First longword beyond RomCodeEnd is a pointer and just after that a word that tells the C++ code what to do.

Done here during initialization to make the device node match the HDF size and here with the IORequest. In my simplified design I always complete the request instantly.

The corresponding C++ code is here - it starts processing the request when the "magic word" write is seen in its address space.

Of course many things could be improved and/or features added, but this is fine for simple testing. If the emulator stalls for too long during reads a simple "fix" could be to read in smaller chunks from the expansion rom code (would require processing the IOrequests in asm code, more like the example device).

dirkwhoffmann commented 2 years ago

Very cool, thanks a lot! I'll open up another issue for hard drive support...