From 54eb51d8db6fc79e7e4258f84b06bb2b0800a64f Mon Sep 17 00:00:00 2001 From: Lior Halphon Date: Sat, 2 Sep 2017 22:08:20 +0300 Subject: [PATCH 1/6] Refined timer interrupt timing --- Core/gb.h | 1 + Core/timing.c | 4 +++- Core/z80_cpu.c | 7 +++---- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Core/gb.h b/Core/gb.h index 5965d51..a929762 100644 --- a/Core/gb.h +++ b/Core/gb.h @@ -332,6 +332,7 @@ struct GB_gameboy_internal_s { GB_PADDING(uint16_t, serial_cycles); uint16_t serial_cycles; /* This field changed its meaning in v0.10 */ uint16_t serial_length; + uint8_t delayed_interrupts; /* When an interrupt occurs while not aligned to a T-cycle, it must be "delayed" */ ); /* APU */ diff --git a/Core/timing.c b/Core/timing.c index 77348fe..72097c2 100644 --- a/Core/timing.c +++ b/Core/timing.c @@ -82,10 +82,13 @@ static void GB_ir_run(GB_gameboy_t *gb) static void advance_tima_state_machine(GB_gameboy_t *gb) { + gb->delayed_interrupts &= ~4; if (gb->tima_reload_state == GB_TIMA_RELOADED) { gb->tima_reload_state = GB_TIMA_RUNNING; } else if (gb->tima_reload_state == GB_TIMA_RELOADING) { + gb->io_registers[GB_IO_IF] |= 4; + gb->delayed_interrupts |= 4; // Timer interrupt is not aligned to a T-cycle and therefore is effective only the next one. gb->tima_reload_state = GB_TIMA_RELOADED; } } @@ -154,7 +157,6 @@ static void increase_tima(GB_gameboy_t *gb) gb->io_registers[GB_IO_TIMA]++; if (gb->io_registers[GB_IO_TIMA] == 0) { gb->io_registers[GB_IO_TIMA] = gb->io_registers[GB_IO_TMA]; - gb->io_registers[GB_IO_IF] |= 4; gb->tima_reload_state = GB_TIMA_RELOADING; } } diff --git a/Core/z80_cpu.c b/Core/z80_cpu.c index 6ea6a38..b6dde8a 100644 --- a/Core/z80_cpu.c +++ b/Core/z80_cpu.c @@ -1337,9 +1337,9 @@ static GB_opcode_t *opcodes[256] = { void GB_cpu_run(GB_gameboy_t *gb) { gb->vblank_just_occured = false; - bool interrupt = gb->interrupt_enable & gb->io_registers[GB_IO_IF] & 0x1F; + uint8_t interrupt_queue = gb->interrupt_enable & gb->io_registers[GB_IO_IF] & 0x1F & ~gb->delayed_interrupts; - if (interrupt) { + if (interrupt_queue) { gb->halted = false; } @@ -1354,9 +1354,8 @@ void GB_cpu_run(GB_gameboy_t *gb) gb->ime_toggle = false; } - if (effecitve_ime && interrupt) { + if (effecitve_ime && interrupt_queue) { uint8_t interrupt_bit = 0; - uint8_t interrupt_queue = gb->interrupt_enable & gb->io_registers[GB_IO_IF] & 0x1F; while (!(interrupt_queue & 1)) { interrupt_queue >>= 1; interrupt_bit++; From 9b490396bb79191051e2b8f38068ff139f22c119 Mon Sep 17 00:00:00 2001 From: Lior Halphon Date: Sat, 2 Sep 2017 22:11:38 +0300 Subject: [PATCH 2/6] Fixed timing when turning the LCD display on during double speed mode --- Core/display.c | 8 ++++---- Core/memory.c | 2 -- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/Core/display.c b/Core/display.c index aa6e681..30f5f63 100755 --- a/Core/display.c +++ b/Core/display.c @@ -299,11 +299,11 @@ static void update_display_state(GB_gameboy_t *gb, uint8_t cycles) for (; cycles; cycles -= atomic_increase) { gb->display_cycles += atomic_increase; - /* The very first line is 2 (4 from the CPU's perseptive) clocks shorter when the LCD turns on. - Todo: Verify on the 3 CGB modes, especially double speed mode. */ - if (gb->first_scanline && gb->display_cycles >= LINE_LENGTH - atomic_increase) { + /* The very first line is 4 clocks shorter when the LCD turns on. Verified on SGB2, CGB in CGB mode and + CGB in double speed mode. */ + if (gb->first_scanline && gb->display_cycles >= LINE_LENGTH - 4) { gb->first_scanline = false; - gb->display_cycles += atomic_increase; + gb->display_cycles += 4; } bool should_compare_ly = true; uint8_t ly_for_comparison = gb->io_registers[GB_IO_LY] = gb->display_cycles / LINE_LENGTH; diff --git a/Core/memory.c b/Core/memory.c index 8d34917..d8f7df7 100644 --- a/Core/memory.c +++ b/Core/memory.c @@ -454,8 +454,6 @@ static void write_high_memory(GB_gameboy_t *gb, uint16_t addr, uint8_t value) case GB_IO_LCDC: if ((value & 0x80) && !(gb->io_registers[GB_IO_LCDC] & 0x80)) { - /* It appears that there's a slight delay after enabling the screen? */ - /* Todo: verify this. */ gb->display_cycles = 0; gb->first_scanline = true; if (gb->frame_skip_state == GB_FRAMESKIP_SECOND_FRAME_RENDERED) { From 0532d2a159e6aea00efead7c17e66748c6401fbe Mon Sep 17 00:00:00 2001 From: Lior Halphon Date: Sat, 2 Sep 2017 23:51:02 +0300 Subject: [PATCH 3/6] =?UTF-8?q?A=20test=20ROM=20I=20wrote=20seems=20to=20c?= =?UTF-8?q?ontradicts=20some=20of=20AntonioND=E2=80=99s=20findings=20regra?= =?UTF-8?q?ding=20PPU=20timing=20in=20CGB=20mode.=20CGB=20mode=20now=20beh?= =?UTF-8?q?aves=20like=20DMG=20mode=20until=20I=20figure=20out=20what=20ca?= =?UTF-8?q?used=20the=20difference.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Core/display.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Core/display.c b/Core/display.c index 30f5f63..fd08ed7 100755 --- a/Core/display.c +++ b/Core/display.c @@ -289,7 +289,10 @@ static void update_display_state(GB_gameboy_t *gb, uint8_t cycles) } uint8_t atomic_increase = gb->cgb_double_speed? 2 : 4; - uint8_t stat_delay = gb->cgb_double_speed? 2 : (gb->cgb_mode? 0 : 4); + /* According to AntonioND's docs this value should be 0 in CGB mode, but tests I ran on my CGB seem to contradict + these findings. + Todo: Investigate what causes the difference between our findings */ + uint8_t stat_delay = gb->cgb_double_speed? 2 : 4; //(gb->cgb_mode? 0 : 4); /* Todo: This is correct for DMG. Is it correct for the 3 CGB modes (DMG/single/double)?*/ uint8_t scx_delay = ((gb->effective_scx & 7) + atomic_increase - 1) & ~(atomic_increase - 1); /* Todo: These are correct for DMG, DMG-mode CGB, and single speed CGB. Is is correct for double speed CGB? */ @@ -558,8 +561,10 @@ static void update_display_state(GB_gameboy_t *gb, uint8_t cycles) gb->io_registers[GB_IO_IF] |= 2; } +#if 0 /* The value of LY is glitched in the last cycle of every line in CGB mode CGB in single speed - This is based on GiiBiiAdvance's docs */ + This is based on AntonioND's docs, however I could not reproduce these findings on my CGB. + Todo: Find out why my tests contradict these docs */ if (gb->cgb_mode && !gb->cgb_double_speed && gb->display_cycles % LINE_LENGTH == LINE_LENGTH - 4) { uint8_t glitch_pattern[] = {0, 0, 2, 0, 4, 4, 6, 0, 8}; @@ -570,6 +575,7 @@ static void update_display_state(GB_gameboy_t *gb, uint8_t cycles) gb->io_registers[GB_IO_LY] = glitch_pattern[gb->io_registers[GB_IO_LY] & 7] | (gb->io_registers[GB_IO_LY] & 0xF8); } } +#endif } void GB_display_run(GB_gameboy_t *gb, uint8_t cycles) From a1a13c61bf6d90b9179b2387f8f8e67ec07d709a Mon Sep 17 00:00:00 2001 From: Lior Halphon Date: Sun, 3 Sep 2017 00:41:52 +0300 Subject: [PATCH 4/6] =?UTF-8?q?On=20CGB,=20the=20VBlank=20and=20STAT=20int?= =?UTF-8?q?errupts=20are=20=E2=80=9Cdelayed=E2=80=9D=20by=20one=20T-cycle?= =?UTF-8?q?=20(relative=20to=20IF)=20since=20they=E2=80=99re=20not=20align?= =?UTF-8?q?ed=20to=20a=20T-Cycle?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Core/display.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Core/display.c b/Core/display.c index fd08ed7..15d8d26 100755 --- a/Core/display.c +++ b/Core/display.c @@ -300,7 +300,7 @@ static void update_display_state(GB_gameboy_t *gb, uint8_t cycles) uint8_t vram_blocking_rush = gb->is_cgb? 0 : 4; for (; cycles; cycles -= atomic_increase) { - + gb->delayed_interrupts &= ~3; gb->display_cycles += atomic_increase; /* The very first line is 4 clocks shorter when the LCD turns on. Verified on SGB2, CGB in CGB mode and CGB in double speed mode. */ @@ -344,6 +344,10 @@ static void update_display_state(GB_gameboy_t *gb, uint8_t cycles) gb->io_registers[GB_IO_STAT] &= ~3; gb->io_registers[GB_IO_STAT] |= 1; gb->io_registers[GB_IO_IF] |= 1; + if (gb->is_cgb) { + /* See comment on STAT interrupt at the end of the loop */ + gb->delayed_interrupts |= 1; + } /* Entering VBlank state triggers the OAM interrupt. In CGB, it happens 4 cycles earlier */ if (gb->io_registers[GB_IO_STAT] & 0x20 && !gb->is_cgb) { @@ -559,6 +563,12 @@ static void update_display_state(GB_gameboy_t *gb, uint8_t cycles) if (gb->stat_interrupt_line && !previous_stat_interrupt_line) { gb->io_registers[GB_IO_IF] |= 2; + if (gb->is_cgb) { + /* On CGB, the STAT interrupt is not aligned to a T-Cycle, therefore it is only effective the next T-Cycle + Todo: verify on DMG mode CGB. This was only tested on LYC STAT interrupts, should be tested on others + as well. */ + gb->delayed_interrupts |= 2; + } } #if 0 From 9bde98dede40e5062670353b66d8c64baeb94e34 Mon Sep 17 00:00:00 2001 From: Lior Halphon Date: Mon, 4 Sep 2017 00:16:12 +0300 Subject: [PATCH 5/6] SCY latching is now correctly emulated, rendering mode timing refined. --- Core/display.c | 10 +++++++--- Core/gb.h | 1 + 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/Core/display.c b/Core/display.c index 15d8d26..4def6c8 100755 --- a/Core/display.c +++ b/Core/display.c @@ -133,7 +133,7 @@ static uint32_t get_pixel(GB_gameboy_t *gb, uint8_t x, uint8_t y) } else { x += gb->effective_scx; - y += gb->io_registers[GB_IO_SCY]; + y += gb->effective_scy; } if (gb->io_registers[GB_IO_LCDC] & 0x08 && !in_window) { map = 0x1C00; @@ -619,13 +619,17 @@ void GB_display_run(GB_gameboy_t *gb, uint8_t cycles) /* Render */ - /* Todo: it appears that the actual rendering starts 4 cycles after mode 3 starts. Is this correct? */ - int16_t current_lcdc_x = gb->display_cycles % LINE_LENGTH - MODE2_LENGTH - (gb->effective_scx & 0x7) - 4; + int16_t current_lcdc_x = gb->display_cycles % LINE_LENGTH - MODE2_LENGTH - (gb->effective_scx & 0x7) - 7; for (;gb->previous_lcdc_x < current_lcdc_x; gb->previous_lcdc_x++) { if (gb->previous_lcdc_x >= WIDTH) { continue; } + + if (((gb->previous_lcdc_x + gb->effective_scx) & 7) == 0) { + gb->effective_scy = gb->io_registers[GB_IO_SCY]; + } + if (gb->previous_lcdc_x < 0) { continue; } diff --git a/Core/gb.h b/Core/gb.h index a929762..3756256 100644 --- a/Core/gb.h +++ b/Core/gb.h @@ -385,6 +385,7 @@ struct GB_gameboy_internal_s { bool oam_write_blocked; bool vram_write_blocked; bool window_disabled_while_active; + uint8_t effective_scy; // SCY is latched when starting to draw a tile ); /* Unsaved data. This includes all pointers, as well as everything that shouldn't be on a save state */ From 72d26c7046b4c322b0157377e85b068108ad3776 Mon Sep 17 00:00:00 2001 From: Lior Halphon Date: Mon, 4 Sep 2017 18:40:43 +0300 Subject: [PATCH 6/6] Fixed obscure timer behavior, fixed regression in rapid_toggle.gb. --- Core/gb.h | 3 +++ Core/timing.c | 6 +++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/Core/gb.h b/Core/gb.h index 3756256..773dccc 100644 --- a/Core/gb.h +++ b/Core/gb.h @@ -333,6 +333,9 @@ struct GB_gameboy_internal_s { uint16_t serial_cycles; /* This field changed its meaning in v0.10 */ uint16_t serial_length; uint8_t delayed_interrupts; /* When an interrupt occurs while not aligned to a T-cycle, it must be "delayed" */ + bool dont_delay_timer_interrupt; /* If the timer glitch causes a TIMA overflow, it causes the timer to overflow + with different timing, so the triggered interrupt is not delayed. + Todo: needs test ROM. */ ); /* APU */ diff --git a/Core/timing.c b/Core/timing.c index 72097c2..626cb45 100644 --- a/Core/timing.c +++ b/Core/timing.c @@ -88,7 +88,9 @@ static void advance_tima_state_machine(GB_gameboy_t *gb) } else if (gb->tima_reload_state == GB_TIMA_RELOADING) { gb->io_registers[GB_IO_IF] |= 4; - gb->delayed_interrupts |= 4; // Timer interrupt is not aligned to a T-cycle and therefore is effective only the next one. + if (!gb->dont_delay_timer_interrupt) { + gb->delayed_interrupts |= 4; // Timer interrupt is not aligned to a T-cycle and therefore is effective only the next one. + } gb->tima_reload_state = GB_TIMA_RELOADED; } } @@ -156,6 +158,7 @@ static void increase_tima(GB_gameboy_t *gb) { gb->io_registers[GB_IO_TIMA]++; if (gb->io_registers[GB_IO_TIMA] == 0) { + gb->dont_delay_timer_interrupt = false; gb->io_registers[GB_IO_TIMA] = gb->io_registers[GB_IO_TMA]; gb->tima_reload_state = GB_TIMA_RELOADING; } @@ -195,6 +198,7 @@ void GB_emulate_timer_glitch(GB_gameboy_t *gb, uint8_t old_tac, uint8_t new_tac) /* And now either the timer must be disabled, or the new bit used for overflow testing be 0. */ if (!(new_tac & 4) || gb->div_cycles & (new_clocks >> 1)) { increase_tima(gb); + gb->dont_delay_timer_interrupt = true; } } }