hoglet67 / BeebFpga

48 stars 8 forks source link

Question: MISTer (and Revs bug) #12

Open sharpie7 opened 2 years ago

sharpie7 commented 2 years ago

Hi David,

This looks very impressive. I've been playing with the BBC Micro Core on the DE10-nano/MISTer platform which has limitations currently. Your BBC FPGA is considerably ahead. Have you by any chance looked at what it would take to port this package to MISTer? It seems to share common roots with the package there, so it should be transferable I would think.

Thanks

(PS I am lushprojects on the Stardot forum if you prefer to message me there).

hoglet67 commented 2 years ago

Hi Iain,

Yes, I noticed your MISTer thread on Stardot.

Sorgelig based the MISTer BBC core on a snapshot of BeebFPGA from a couple of years ago, and it doesn't look like he's had the time or motivation to keep updating it. I don't think it's one of the more popular cores.

I haven't really any experience with MISTer; I don't have access to the DE10 hardware and I haven't really studied the MISTer architecture, so it's hard for me to really offer any concrete advice here.

(also BeebFPGA is in a bit of a mess at the moment; there is a lot of work on the dev branch that needs merging back into master)

Dave

sharpie7 commented 2 years ago

Thanks. That makes sense.

I suspect it's not a huge job, but that's a slippery slope.

sharpie7 commented 2 years ago

Sorry for driving this issue off-topic, but I have a further question: I've built a prototype joining the BeebFpga core into the MISTer framework. It boots successfully to BBC Basic. However, if I have the swmmfs ROM from the official MISTer build installed it just produces the "Card ?" error even though there is a suitable card available.

I've tried to debug this by routing the internal SD Card signals to external test points. Oddly I don't see any activity on the sdclk_int signal. It seems to stay high. Any ideas what might cause this? Is Does swmmfs do any check that there is hardware attached before sending clocks? Might I have messed up the extended RAM mapping and caused this effect?

I am a bit stumped at the moment.

hoglet67 commented 2 years ago

Card? error is really quite generic, and just means that MMFS, for some reason, has failed to communicate with the SD Card.

The most common cause is just using the wrong build of MMFS for the hardware that you have. There are different builds, for example, depending on the type of interface, and in the case of the 6522 based interfaces, on the address of the 6522.

It looks like MISTer uses the "SWMMFS2" build, which expects the SD Card to be on a second 6522 at address &FE8x.

Depending on how you have configured the BeebFPGA core, that may or may not be the case.

Is your core checked into github somewhere?

Card? can also result from the "Sideways RAM" build of MMFS being loaded into a bank that is not writeable (it needs to be writeable minimally from &B600-&BFFF).

sharpie7 commented 2 years ago

Thanks. It's a mess right now so the repo is private, but I've sent you an invite. It does seem to be setup to use the SD Card on the user port as you describe.

I wonder if the "Sideways RAM" writeability is the problem. I didn't pay much attention to the memory treatment when I merged the projects.

hoglet67 commented 2 years ago

I think the problem is just a mis-match between the SWMMFS2 ROM (which uses a 6522 at &FE8x) and your configuration of bbc_micro_core (which seems to have IncludeAMXMouse=false). With that set to false, there is just a single 6522 at &FE6x.

You could try building with IncludeAMXMouse=true, or switch to using the SWMMFS ROM, which uses a 6522 at the standard address of &FE6x.

The latest version of the MMFS ROMs is here: https://github.com/hoglet67/MMFS/releases/tag/mmfs_1_50_0

You would need /MMFS/U/SWMMFS.rom

Dave

sharpie7 commented 2 years ago

Quite right thanks. Wish I had asked earlier!

hoglet67 commented 2 years ago

No problem, do ask if there is anything else.

sharpie7 commented 2 years ago

So I just tried Revs which was one of the problem cases on the official MiSTer. It still has a timing bug that causes the pallet used for the sky to roll down the screen. Do you know if Revs works in BeebFPGA on other platforms?

hoglet67 commented 2 years ago

What video output are you using? (i.e. RGB, VGA, HDMI, etc)

What 6502 core are you using?

It's certainly the case that the VGA and HDMI modes have slighly different timing to the original Beeb, because they effectively force non-interlaced operation of the CRTC. So any game that depends on perfect cycle accuracy wrt interfaced VSYNC may fail.

In 15HKz RGB mode, and with the T6502 core, the timing should be cycle accurate.

Can you post a link to the version of Revs you are testing with and I'll give it a try.

Also, are you using the Model B or the Master?

sharpie7 commented 2 years ago

It's the Revs version in RetroClinic BEEB.MMB File v1.02 for MicroSPI..

The code is as per repo you've seen. So currently built using UseAlanDCore : boolean := true;

To fit with the MiSTer template I've disabled all your nice video code and pulled the vidproc signals out to the BBCMicro.sv level where they are fed into the MiSTer implementation of scan doubler and out on HDMI. vga_mode is hardcoded to 0 currently.

I haven't looked at the Revs code, but the fact it rolls makes me think the timing isn't from vsync, but maybe it's using a 6522 timer (or maybe just cycle counting) to make very accurate 50Hz changes to pallets and the timing between frames is not 100% matching that assumed in the code.

hoglet67 commented 2 years ago

I can confirm this Revs bug does seem to be present in my upstream version of BeebFPGA.

I found a couple of possibly related posts:

Revs does reprogram the palette several times during the frame. It's unusual in that it doesn't base the timing of vsync every frame, as most other split palette games do, but instead just chains back-to-back VIA timer interrupts and relies on the total period adding to exactly the length of a frame. This probably means that VIA timing got broken at some point...

This suggests something is not quite cycle accurate, but it could be:

I've found the timer interrupt code in Revs that is doing the palette changes, and it does look like it's simply chaining timer interrupts:

4E5C : AD 6D FE : LDA $FE6D
4E5F : 29 40    : AND #$40
4E61 : F0 F6    : BEQ $4E59
4E63 : 8D 6D FE : STA $FE6D
4E66 : 8A       : TXA 
4E67 : 48       : PHA 
4E68 : D8       : CLD 
4E69 : AD 43 4F : LDA $4F43
4E6C : F0 0E    : BEQ $4E7C
4E6E : 30 22    : BMI $4E92
4E70 : C9 02    : CMP #$02
4E72 : 90 27    : BCC $4E9B
4E74 : F0 4D    : BEQ $4EC3
4E76 : C9 03    : CMP #$03
4E78 : F0 5C    : BEQ $4ED6
4E7A : B0 6B    : BCS $4EE7
4E7C : A9 88    : LDA #$88
4E7E : 8D 20 FE : STA $FE20
4E81 : A2 0F    : LDX #$0F
4E83 : BD 68 34 : LDA $3468,X
4E86 : 8D 21 FE : STA $FE21
4E89 : CA       : DEX 
4E8A : 10 F7    : BPL $4E83
4E8C : A9 C4    : LDA #$C4
4E8E : A2 0F    : LDX #$0F
4E90 : D0 6F    : BNE $4F01
4E92 : C9 FF    : CMP #$FF
4E94 : D0 74    : BNE $4F0A
4E96 : EE 43 4F : INC $4F43
4E99 : F0 F1    : BEQ $4E8C
4E9B : A9 C4    : LDA #$C4
4E9D : 8D 20 FE : STA $FE20
4EA0 : 18       : CLC 
4EA1 : A9 03    : LDA #$03
4EA3 : 8D 21 FE : STA $FE21
4EA6 : 69 10    : ADC #$10
4EA8 : 90 F9    : BCC $4EA3
4EAA : A9 3C    : LDA #$3C
4EAC : 38       : SEC 
4EAD : ED 1F 4F : SBC $4F1F
4EB0 : 8D 21 4F : STA $4F21
4EB3 : A9 15    : LDA #$15
4EB5 : ED 20 4F : SBC $4F20
4EB8 : 8D 22 4F : STA $4F22
4EBB : AD 1F 4F : LDA $4F1F
4EBE : AE 20 4F : LDX $4F20
4EC1 : B0 3E    : BCS $4F01
4EC3 : A2 0F    : LDX #$0F
4EC5 : BD 58 34 : LDA $3458,X
4EC8 : 8D 21 FE : STA $FE21
4ECB : CA       : DEX 
4ECC : 10 F7    : BPL $4EC5
4ECE : AD 21 4F : LDA $4F21
4ED1 : AE 22 4F : LDX $4F22
4ED4 : D0 2B    : BNE $4F01
4ED6 : A2 03    : LDX #$03
4ED8 : BD 78 34 : LDA $3478,X
4EDB : 8D 21 FE : STA $FE21
4EDE : CA       : DEX 
4EDF : 10 F7    : BPL $4ED8
4EE1 : A9 00    : LDA #$00
4EE3 : A2 1E    : LDX #$1E
4EE5 : D0 1A    : BNE $4F01
4EE7 : A2 03    : LDX #$03
4EE9 : BD 7C 34 : LDA $347C,X
4EEC : 8D 21 FE : STA $FE21
4EEF : CA       : DEX 
4EF0 : 10 F7    : BPL $4EE9
4EF2 : 8E 43 4F : STX $4F43
4EF5 : 20 A4 52 : JSR $52A4
4EF8 : A9 FF    : LDA #$FF
4EFA : 8D 69 FE : STA $FE69
4EFD : A9 16    : LDA #$16
4EFF : A2 0B    : LDX #$0B
4F01 : 8E 67 FE : STX $FE67
4F04 : 8D 66 FE : STA $FE66
4F07 : EE 43 4F : INC $4F43
4F0A : 68       : PLA 
4F0B : AA       : TAX 
4F0C : A5 FC    : LDA $FC
4F0E : 40       : RTI 

Here's a quick trace of some timings as it runs (the first number is 2MHz ticks):

>> bl
0: 4E5C mask FFFF: Ex Watch (trigger: Always)
1: FE60 mask FFF0: Mem Wr Watch (trigger: Always)
>> f
Flushing Event FIFO
>> c
CPU free running...
08.660726 : Ex Watch hit at 4E5C
08.660734 : Mem Wr Watch hit at 4E63 writing FE6D:40  @
08.660975 : Mem Wr Watch hit at 4F01 writing FE67:0F  .
08.660979 : Mem Wr Watch hit at 4F04 writing FE66:C4  .
08.661030 : Ex Watch hit at 4E5C
08.661242 : Ex Watch hit at 4E5C
08.661524 : Ex Watch hit at 4E5C
08.662217 : Ex Watch hit at 4E5C
08.662225 : Mem Wr Watch hit at 4E63 writing FE6D:40  @
08.662436 : Mem Wr Watch hit at 4F01 writing FE67:04  .
08.662440 : Mem Wr Watch hit at 4F04 writing FE66:D8  .
08.670287 : Ex Watch hit at 4E5C
08.670295 : Mem Wr Watch hit at 4E63 writing FE6D:40  @
08.670542 : Mem Wr Watch hit at 4F01 writing FE67:10  .
08.670546 : Mem Wr Watch hit at 4F04 writing FE66:64  d
08.672763 : Ex Watch hit at 4E5C
08.672771 : Mem Wr Watch hit at 4E63 writing FE6D:40  @
08.672862 : Mem Wr Watch hit at 4F01 writing FE67:1E  .
08.672866 : Mem Wr Watch hit at 4F04 writing FE66:00  .
08.681154 : Ex Watch hit at 4E5C
08.681162 : Mem Wr Watch hit at 4E63 writing FE6D:40  @
08.681289 : Mem Wr Watch hit at 4EFA writing FE69:FF  .
08.681297 : Mem Wr Watch hit at 4F01 writing FE67:0B  .
08.681301 : Mem Wr Watch hit at 4F04 writing FE66:16  .
08.681727 : Ex Watch hit at 4E5C
08.689853 : Ex Watch hit at 4E5C
08.694817 : Ex Watch hit at 4E5C
08.696472 : Ex Watch hit at 4E5C
08.696480 : Mem Wr Watch hit at 4E63 writing FE6D:40  @
08.696721 : Mem Wr Watch hit at 4F01 writing FE67:0F  .
08.696725 : Mem Wr Watch hit at 4F04 writing FE66:C4  .
08.701932 : Ex Watch hit at 4E5C
08.702210 : Ex Watch hit at 4E5C
08.702218 : Mem Wr Watch hit at 4E63 writing FE6D:40  @
08.702429 : Mem Wr Watch hit at 4F01 writing FE67:04  .
08.702433 : Mem Wr Watch hit at 4F04 writing FE66:D8  .
08.710203 : Ex Watch hit at 4E5C
08.710211 : Mem Wr Watch hit at 4E63 writing FE6D:40  @
08.710458 : Mem Wr Watch hit at 4F01 writing FE67:10  .
08.710462 : Mem Wr Watch hit at 4F04 writing FE66:64  d
08.712679 : Ex Watch hit at 4E5C
08.712687 : Mem Wr Watch hit at 4E63 writing FE6D:40  @
08.712778 : Mem Wr Watch hit at 4F01 writing FE67:1E  .
08.712782 : Mem Wr Watch hit at 4F04 writing FE66:00  .
08.721070 : Ex Watch hit at 4E5C
08.721078 : Mem Wr Watch hit at 4E63 writing FE6D:40  @
08.721205 : Mem Wr Watch hit at 4EFA writing FE69:FF  .
08.721213 : Mem Wr Watch hit at 4F01 writing FE67:0B  .
08.721217 : Mem Wr Watch hit at 4F04 writing FE66:16  .
08.722130 : Ex Watch hit at 4E5C
08.729690 : Ex Watch hit at 4E5C
08.734573 : Ex Watch hit at 4E5C
08.736298 : Ex Watch hit at 4E5C
08.736306 : Mem Wr Watch hit at 4E63 writing FE6D:40  @
08.736547 : Mem Wr Watch hit at 4F01 writing FE67:0F  .
08.736551 : Mem Wr Watch hit at 4F04 writing FE66:C4  .
08.741973 : Ex Watch hit at 4E5C
08.741981 : Mem Wr Watch hit at 4E63 writing FE6D:40  @
08.742192 : Mem Wr Watch hit at 4F01 writing FE67:04  .
08.742196 : Mem Wr Watch hit at 4F04 writing FE66:D8  .
08.742260 : Ex Watch hit at 4E5C
08.750028 : Ex Watch hit at 4E5C

You can see a pattern of values being written to the User Port 6522 T1 timer:

Some of these are hard-coded (*), others seem to be calculated.

What's a bit strange is the there are still other interrupts happening. Normally code that has to be cycle accurate disables other interrupts. Also, the same code works on the Master, which has a 65C02. So I'm thinking there is some odd/even frame calibration code that is malfunctioning.

I'll look into this some more over the weekend.

sharpie7 commented 2 years ago

If you are not aware, you might find this helpful, though it is still a work in progress.

What tool did you use for your bebugger output BTW? I see there is reference to a debugger in BeebFpga, but I haven't tried to understand how to use it or whether it needs any external hardware.

hoglet67 commented 2 years ago

Thanks for the link to Mark Moxom's disassembly - I'd completely forgotton about that.

The debugger is my own ICE-T65 (aka AtomBusMon) which is a seperate project than can be used on a real Beeb, but is also included in BeebFPGA (includeICEDebugger). You connect to it over the USB serial port.

You can read more here: https://github.com/hoglet67/AtomBusMon/wiki

hoglet67 commented 2 years ago

Hi Iain,

I've think I've found the cause of the Revs bug - it was a recent issue introduced last year in commit 9cb55520 - and was only present on my dev branch. It was part of a package of work to get all of the BitShifter's demos working, discussed in this thread: 6845 Quirks (and FPGA implementation)

The bug caused the interlaced timing flag to be ignored in MODE 0..6, resuting in 624-line non-interlaced video all the time. The went unnoticed as most games either disable interlace, or are ambialent.

Acornsoft Revs is different because the palatte change is quite complex and uses the 6522 T1 latch to hold the next counter reload value. There are five palette changes (and two mode changes) during the field, and this technique allows them to always happen at exactly the right point in the 20,000 us interlaced field. For the purposes of this code, the fields are treated as equal length, and the game ensures that the +/- 32us error this introduces is not visible. This approach is very clever, because it isn't affected by 6502 cycle timings, interrupt latencies, or brief periods of IRQ being masked. This is why it works regardless of whether the CPU is a 6502 or a 65C02.

Anyway, using the debugger I was able to see that the interrupt chaining was working correctly, so the only place the error could be was in the mc6845 implementation. The palette drift rate was about 25 lines per second in a down direction, which was also a clue there was an error of one line per frame (with the frame being too short)

It's interesting that a bug with a similar effect is present in Sorgelig's Mister branch, as the mc6845 code there predates the introduction of this bug. What's the rate of palette shift (how long until it gets back to the right poing again) and in what direction? I'd like to understand the reason that is failing, as it's just possible I have got the wrong end of the stick here, and fixed this bug incorrecty. That's easy to do with the 6845 as it's rather a mess....

The fix is very small: 6289d8a2

Dave

sharpie7 commented 2 years ago

Thanks Dave

That's quite a journey. Revs is working fine on my port with that change too. Ironically I took your dev branch as I thought the 6845 updates might have fixed the Revs bug that's on the Sorgelig implementation!

I feel like I am giving you an endless list of "problem" games here, but I suppose I have an attachment to certain games from my youth. What's the status of Acornsoft Defender on your platforms? I have seen it work on my port, though with some visual bugs, but mostly it crashes just after the game starts. I willl try the other 6502 model in case it's an illegal opcode problem again.

On the original topic: I guess I feel I've demonstrated that it is possible to have a common core BBCMicro sim that supports MiSTer as well as the platforms you're working on. There are some ifs and buts about how exactly you would update the design to support that, but it's more in relation to how to structure the project and separate the common parts from the platform specific parts than it is about making deep changes to the existing core (though it would need some changes).

Happy to talk about that in more detail if it's something you think you might be interested in.

Iain

hoglet67 commented 2 years ago

That's quite a journey. Revs is working fine on my port with that change too. Ironically I took your dev branch as I thought the 6845 updates might have fixed the Revs bug that's on the Sorgelig implementation!

I would like to also understand what's wrong in Sorgelig's case (just so it doesn't come back and bite us). I also don't like leaving loose ends.

If you could try to observe (or make a video of) the palette drift rate and direction that would be very helpful.

What's the status of Acornsoft Defender on your platforms? I have seen it work on my port, though with some visual bugs, but mostly it crashes just after the game starts. I willl try the other 6502 model in case it's an illegal opcode problem again.

I didn't think there was a problem - it's one that I do run myself occasionally.

I'm more than happy to keep tracking down these kind of issues, as that's the only way the core gets improved. And I do rely on other people to report bugs.

So if you do find anything amiss, please do just start a new issue.

On the original topic: I guess I feel I've demonstrated that it is possible to have a common core BBCMicro sim that supports MiSTer as well as the platforms you're working on. There are some ifs and buts about how exactly you would update the design to support that, but it's more in relation to how to structure the project and separate the common parts from the platform specific parts than it is about making deep changes to the existing core (though it would need some changes).

Happy to talk about that in more detail if it's something you think you might be interested in.

I'm generally in favour of this, with some caveats...

I've held back until now, because it seemed like MISTer was very much Sorgelig's project. He hasn't tried to reach out in the same way you have, so I have just assumed he wanted to take the most expedient path which supports his goals of getting as many cores running as possible.

I've also avoided getting myself in the position where I feel obliged to support the core on platform I don't personally have easy access to myself.

I've been approached by several people to port the core to their platform, sometimes even with the offer of money. It's not the initial port that's the problem, it's the ongoing support and testing of N platforms. Especially if that involves me asking other people to do stuff.

Finally, I'm not clear what is the best way to support multiple video output modes (15KHz RGB, VGA and HDMI). What I have currently is a bit of a mess due to how it evolved during the Spectrum Next port. I'd like to think some more about that, possibly informed by what you have done in Mister.

Anyway, do keep the bugs coming, and do run Revs on Sorgelig's core and let me know the drift rate and direction.

Cheers,

Dave

sharpie7 commented 2 years ago

Hi Dave

On Sorgelig's version the pallet drift is downwards again, taking 12.5s to roll though the whole screen. Seems like a similar bug to me. Maybe around lines 334 of that 6845 code which seems to be doing a similar calculation to the one you fixed.

On the common core idea. I think you're dead right about the issues, and TBH I am wary about getting too deep in this because I don't really have time to maintain stuff.

I agree that Sorgelig will look after MiSTer and I would leave him to it unless he reaches out. But, the MiSTer platform does allow users to use alternative cores in addition-to or instead-of the ones that are officially recognized as part of the project. My goal is to make an alternative BBC Micro Core which incorporates your recent improvements. It's mostly for myself, but I am happy to share with others on an "it works for me" basis. I don't want my MiSTer to feel inferior to all those shiny Spectrum-nexts!

The path of least resistance is just to take a snap-shot of your code and for me to modify where necessary. Obviously I can pick-up your later bug fixes as and when I get around to it. I would be fine to go down that route, but pleased to have your help too.

I guess the next level would be to try and share the files for individual components. They are 98% common anyway, but I modified the 6845, the saa5050 and the vidProc_orig to support the MiSTer video API. They are not big mods. I think it's your call whether you feel that having those features in the common code (that you don't use) is something you want to do or not. You can compre the /rtl folder in my repo with your /src/common to see the differences.

Making bbc_micro_core.vhd common would require thinking about the structure and adding new conditional features which could be klunky. I think that's where maintaining a single common working source may become a bind without having access to all the platforms.

The BBCMicro.sv file and everything above that is platform specific.

hoglet67 commented 2 years ago

On Sorgelig's version the pallet drift is downwards again, taking 12.5s to roll though the whole screen. Seems like a similar bug to me. Maybe around lines 334 of that 6845 code which seems to be doing a similar calculation to the one you fixed.

It's the same symptoms but a different cause.

I think the problem is line 357: https://github.com/MiSTer-devel/BBCMicro_MiSTer/blob/master/rtl/mc6845.vhd#L357

if r08_interlace(0) = '1' then
    --odd_field <= not odd_field;
else
    odd_field <= '0';
end if;

Sorgelig has effetively disabled all interlaced output by commenting out this line.

That was done in this commit: https://github.com/MiSTer-devel/BBCMicro_MiSTer/commit/f6f1afb3

Dave