Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Timer Obscure Behaviour Missing #101

Open
brNX opened this issue Aug 16, 2020 · 10 comments
Open

Timer Obscure Behaviour Missing #101

brNX opened this issue Aug 16, 2020 · 10 comments
Labels

Comments

@brNX
Copy link
Collaborator

brNX commented Aug 16, 2020

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

@brNX brNX added the bug label Aug 27, 2020
@maij
Copy link
Contributor

maij commented Mar 21, 2023

Working on this.

maij added a commit to maij/Gameboy_MiSTer that referenced this issue Mar 26, 2023
Sound clock is passed to the APU but is not currently being used. Work needed to match this up.
maij added a commit to maij/Gameboy_MiSTer that referenced this issue Mar 30, 2023
Sound clock is passed to the APU but is not currently being used. Work needed to match this up.
@maij
Copy link
Contributor

maij commented May 2, 2023

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
Copy link
Member

sorgelig commented May 3, 2023

You can use polynomial CE, instead of separate clock.

@maij
Copy link
Contributor

maij commented May 3, 2023

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
Copy link
Member

sorgelig commented May 3, 2023

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

@maij
Copy link
Contributor

maij commented May 3, 2023

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

maij added a commit to maij/Gameboy_MiSTer that referenced this issue May 10, 2023
Sound clock is passed to the APU but is not currently being used. Work needed to match this up.
maij added a commit to maij/Gameboy_MiSTer that referenced this issue May 27, 2023
Sound clock is passed to the APU but is not currently being used. Work needed to match this up.
@maij
Copy link
Contributor

maij commented Jun 30, 2023

@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
Copy link
Collaborator

paulb-nl commented Jul 1, 2023

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
Copy link
Contributor

maij commented Jul 1, 2023

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
Copy link
Collaborator

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.

maij added a commit to maij/Gameboy_MiSTer that referenced this issue May 4, 2024
Sound clock is passed to the APU but is not currently being used. Work needed to match this up.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

4 participants