MiSTer-devel / Gameboy_MiSTer

Gameboy for MiSTer
99 stars 46 forks source link

Timer Obscure Behaviour Missing #101

Open brNX opened 3 years ago

brNX commented 3 years ago

The timer needs to be re-implemented using the following information : https://gbdev.io/pandocs/#timer-obscure-behaviour-2, it directly influences the APU in some cases (the APU clock should be derived from it)

image

maij commented 1 year ago

Working on this.

maij commented 1 year ago

I have been looking into this, and it's been full of nefarious issues. The timer module was easy enough to reimplement, but taking the sound clock from DIV has been like crossing a clock-domain. Everything becomes desynced because of the way the APU is currently written. Someone may want to pick up where I left off but I won't touch this for some time.

Although the current implementation isn't strictly correct -- the core still works.

sorgelig commented 1 year ago

You can use polynomial CE, instead of separate clock.

maij commented 1 year ago

Ah, I wasn't clear. I am using clock enables, it's just that the clock enables generated inside the APU are different from those generated in the timer, in terms of synchronisation.

sorgelig commented 1 year ago

if CEs are generated from the same clock, then there won't be clock-domain crossing.

maij commented 1 year ago

I was exaggerating, saying it's 'like' crossing clock domains. Sorry for the confusion

maij commented 1 year ago

@brNX I have re-implemented the timer and APU frame counter now, and everything appears to work. Do you know of any cases (real games or test roms) where this functionality can be verified?

Edit: I have found a test rom and will report back with results.

paulb-nl commented 1 year ago

The TAC behavior is not completely correct yet. You need to change this:

wire clk_tac =  (tac[1:0] == 2'b00) ? clk_div[9]:
                    (tac[1:0] == 2'b01) ? clk_div[3]:
                    (tac[1:0] == 2'b10) ? clk_div[5]:
                          clk_div[7];
if(tac[2] && clk_tac_r && !clk_tac) begin
    {tima_overflow_buffer[0], tima} <= tima + 1'b1;
end

to this:


wire clk_tac = tac[2] & ( (tac[1:0] == 2'b00) ? clk_div[9]:
                (tac[1:0] == 2'b01) ? clk_div[3]:
                (tac[1:0] == 2'b10) ? clk_div[5]:
                          clk_div[7] );

if(clk_tac_r && !clk_tac) begin
    {tima_overflow_buffer[0], tima} <= tima + 1'b1;
end

TAC timer enable is included in the falling edge detector so it is possible to increment TIMA by toggling the enable bit.

maij commented 1 year ago

I put this in an edit to my comment above but this needs its own comment really.

I have used the AGE test roms, where there is a single test changing the CPU speed and seeing its effect on the DIV clk and subsequently the APU frame counter clock. Unfortunately, all of the speed-switch tests fail regardless of the changes I've made (i.e. the current release is failing too). So the core still has some divergence from a real Gameboy.

SameBoy's SameSuite does have an explicit test for the APU clock, though I can imagine it could have similar issues. I will give it a test sometime soon.

Another thing that is definitely wrong (in all core implementations up to now) is that the timer module takes the 4 MHz clock and uses a 16-bit counter to divide down from there, but based on the DMG schematics it should use a 1 MHz clock and a 14-bit counter. The key difference is that writing to DIV should not reset the bottom two-bits of the 16-bit counter (i.e. the 1 MHz clock is a free-running clock). However, if we are going to go by the schematics, quite a few elements would have to be modified to also share around the 2 MHz and 1 MHz clocks etc, rather than re-generating them all in each block.

There could be value in creating a clock generation module (perhaps inside speedcontrol.vhd) that creates all of these different principal clocks. Trouble is, I have no idea what the phase relation between these clocks would be, and aside from passing these tests I suspect most changes will have no tangible change in performance of the core.

We could consider creating a single issue that details all of the currently failing tests from a number of these suites...

paulb-nl commented 1 year ago

You shouldn't worry about those test roms that switch the CPU speed because speed switching is not properly implemented at all. It should take a certain amount of cycles and various things should not be clocked when the CPU is in STOP mode.

Focus on tests that do not change the CPU speed.