KIRBY2: INVALID OPCODE at 6128

[YouMakeTech]

Kirby's Dream Land 2 no longer works and return an error: ./peanut-sdl Audio driver: Built-in Audio Digital Stereo (IEC958) Unable to assign joystick mappings: Invalid RWops ROM: KIRBY2 MBC: 1 Error 1 occurred: INVALID OPCODE at 6128 . Cart RAM saved to recovery.sav Exiting.

It used to work before the addition of PGB_UNREACHABLE statements in peanut_gb.h. The error was simply ignored

[deltabeard]

I've been comparing the program counter between SameBoy and Peanut-GB when playing Kirby's Dream Land 2 (Kirby 2) to see how Peanut-GB deviates from a very accurate emulator. Well it seems that Peanut-GB deviates significantly throughout, and I think that this is because of poor LCD timing at least. I don't think this make much difference in terms of game play until the program counter reaches 0x02A3 on line 826766 where in Peanut-GB, register A is cleared and then the program counter (PC) is set to 0x0038, even though the RST $38 isn't called.

I think this issue first occurs on line 756839 where PC is 0x02A3, and is then 0x0000 on the next instruction. From 0x029F, the instructions are POP DE; POP BC; POP HL; POP AF;. So I think this game relies on accurate emulation to stop this stack corruption from happening.

The program counter dumps for SameBoy and Peanut-GB: dump-kirby2.zip

I plan on doing another dump with more registers to hopefully narrow down why this issue occurs.

This problem previously did not occur because ignoring the invalid opcode instruction eventually calls a return which makes the game continue to the main menu.

[YouMakeTech]

Thank you so much for investigating this issue. This is the favorite game of my daughter! She has played a lot with this game using Peanut-GB on the Pico-GB! I need to confirm this but as far as I remember, when the error was simply ignored, she could play the game with no issue at all. She reached level 4 or 5 of the game!

[deltabeard]

The issue was due to incorrect LCD timing. I'm surprised I actually managed to find the problem! Please let me know if this fix doesn't work for you.

[YouMakeTech]

It works again. My daughter is testing now! Amazing how you managed to find the problem. Thank you!

[YouMakeTech]

It's strange, I tested it and it worked but now I get a new error: ./peanut-sdl INFO: No dmg_boot.bin file found; disabling boot ROM INFO: Audio driver: HDA NVidia Digital Stereo (HDMI) INFO: Unable to assign joystick mappings: Invalid RWops INFO: Peanut-SDL: KIRBY2 CRITICAL: Error: INVALID OPCODE at 0x131E with instruction DB. Cart RAM saved to recovery.sav Exiting.

I'm using the latest version of Peanut-GB

[deltabeard]

Is this at the main menu or another part of the game?

[YouMakeTech]

It's shortly after the main menu, when pressing start during the intro, just before the 1st level

[deltabeard]

The issue was due to incorrect LCD timing. I'm surprised I actually managed to find the problem! Please let me know if this fix doesn't work for you.

The LCD timing was correct before, so this change should be reverted. I think Peanut-GB should improve it's accuracy (pass all blargg tests) in order to fix this issue. Specifically, the mem_timing and interrupt_time tests should pass and I think that will fix this issue. This will reduce the performance of Peanut-GB though, so I'll have to give this a lot of thought as to how the implementation should be done.

[deltabeard]

The https://github.com/deltabeard/Peanut-GB/tree/mem-accuracy branch now correctly emulates memory access timing. However, this issue with Kirby2 still occurs. This issue is because of poor LCD emulation. According to https://www.reddit.com/r/EmuDev/comments/9c2q3o/difficult_gb_games_to_emulate/e57oza5/ Kirby2 requires accurate STAT emulation. I would like Peanut-GB to correctly pass the DMG-Acid2 test at https://github.com/mattcurrie/dmg-acid2 also, so I think this is a good opportunity to fix both issues.

[deltabeard]

With commit d3850957ff71785016f36731ca8ac57e130fdf94 Kirby 2 will now be broken again. I fixed the issue with DMG-Acid2 test and now I get a perfect result with the latest commit bbf094a2c75178e5b2e3962d3d0c1d783e8848c0:

I think that Kirby 2 is failing because it requires strict LCD STAT behaviour. I'll continue to investigate this further.

[deltabeard]

This issue seems to be fixed with commit fcb42682129243e34e6446eadb5e654778b35933 on the ppu-fix branch. It needs testing to make sure I didn't break support for other games.

I think the problem is where the game is writing to LCDC. In __gb_write(), we have:

        /* LCD Registers */
        case 0x40:
            if(((gb->hram_io[IO_LCDC] & LCDC_ENABLE) == 0) &&
                (val & LCDC_ENABLE))
            {
                gb->counter.lcd_count = 0;
                gb->lcd_blank = 1;
            }

            gb->hram_io[IO_LCDC] = val;

            /* LY fixed to 0 when LCD turned off. */
            if((gb->hram_io[IO_LCDC] & LCDC_ENABLE) == 0)
            {
                /* Do not turn off LCD outside of VBLANK. This may
                 * happen due to poor timing in this emulator. */
                if((gb->hram_io[IO_STAT] & STAT_MODE) != IO_STAT_MODE_VBLANK)
                {
                    gb->hram_io[IO_LCDC] |= LCDC_ENABLE;
                    return;
                }

                /* Set LCD to Mode 0. */
                gb->hram_io[IO_STAT] = (gb->hram_io[IO_STAT] & ~0x03);
                /* Set to line 0. */
                gb->hram_io[IO_LY] = 0;
                /* Reset LCD timer. */
                gb->counter.lcd_count = 0;
            }

            return;

If the LCD is switched off, the LCD is in mode 0 which is HBlank. If the game then writes to the LCDC register whilst the LCD is switched off, Peanut-GB would forcibly switch on the LCD because the LCD is in HBlank and not in VBlank.

The code has now been changed to only reset the LCD state when the LCD is being switched on and off, but not if the LCD is already off or already on.

case 0x40:
        {
            uint8_t lcd_enabled;

            /* Check if LCD is already enabled. */
            lcd_enabled = (gb->hram_io[IO_LCDC] & LCDC_ENABLE);

            gb->hram_io[IO_LCDC] = val;

            /* Check if LCD is going to be switched on. */
            if (!lcd_enabled && (val & LCDC_ENABLE))
            {
                gb->lcd_blank = 1;
            }
            /* Check if LCD is being switched off. */
            else if (lcd_enabled && !(val & LCDC_ENABLE))
            {
                /* Peanut-GB will happily turn off LCD outside
                 * of VBLANK even though this damages real
                 * hardware. */

                /* Set LCD to Mode 0. */
                gb->hram_io[IO_STAT] =
                    (gb->hram_io[IO_STAT] & ~STAT_MODE) |
                    IO_STAT_MODE_HBLANK;
                /* LY fixed to 0 when LCD turned off. */
                gb->hram_io[IO_LY] = 0;
                /* Reset LCD timer. */
                gb->counter.lcd_count = 0;
            }
            return;
        }

Let me know if there are still any issues. 😃

[YouMakeTech]

Thanks Mahyar for fixing this. I will test this on Kirby 2 and other games to see if there is any regression due to the change. Keep up the good work!

[YouMakeTech]

Thanks Mahyar, this change definitely fixes Kirby 2! I also tried other games and didn't notice any regression.This change also seems to fix issue "Prince of Persia - some flickering garbage rendered in the upper few lines of the screen #73"