jotego / jtcores

FPGA cores compatible with multiple arcade game machines and KiCAD schematics of arcade games. Working on MiSTer FPGA/Analogue Pocket
https://patreon.com/jotego
GNU General Public License v3.0
228 stars 40 forks source link

s18: games not booting up #685

Closed jotego closed 3 months ago

jotego commented 3 months ago

The following games do not boot at all on JTS18 core a8cad670

gyurco commented 3 months ago

Seems generic 5987 problem, as all such games (which start after a while in service mode) has bad A5/A7 ROMs.

jotego commented 3 months ago

Maybe the ROM files are not loaded at the right position. They are loaded with MAME's offset but MAME seems to do some sort of remapping for the 5987:

case ROM_BOARD_171_5987:    
    if (romsize <= 0x100000)
        mapper.map_as_rom(0x00000, 0x80000, 0xf80000, "rom1base", 
            "decrypted_rom1base", 0x80000, write16_delegate(*this, FUNC(segas18_state::rom_5987_bank_w)));
    else
        mapper.map_as_rom(0x00000,0x100000, 0xf00000, "rom1base", 
            "decrypted_rom1base",0x100000, write16_delegate(*this, FUNC(segas18_state::rom_5987_bank_w)));
    break;

The core must be activating rom_cs correctly:

rom_cs <= (active[0] || (active[1] && !game_id[PCB_7248])) && RnW;

but the rom_addr must be needing an adjustment:

// System 18 memory map
always @* begin
    rom_addr = A[20:1];
    if(active[0]) begin
        if(game_id[PCB_5874]|game_id[PCB_7248]) rom_addr[20:19]=0;
        if(game_id[PCB_7525]|game_id[PCB_5987]) rom_addr[20]=0; // may need extra masking for smaller ROM sizes
    end
    // assuming that if active[1] is set, then A is already pointing after 512kB
end

The assumption in the comment must be wrong.

gyurco commented 3 months ago

What's the best way to determine the ROM size? e.g. Desert Breaker has 2MB of ROM, the others have 1MB, and they need a slightly different handling.

gyurco commented 3 months ago

...or use a separate PCB type for Desert Breaker, like PCB_5987_2M?

jotego commented 3 months ago

...or use a separate PCB type for Desert Breaker, like PCB_5987_2M?

You can add a bit to identify desertbr in the header. Like:

[header]
data = [
# ... other entries
    { machines=["desertbr"],  offset=0x14, data="02" },
]
gyurco commented 3 months ago

I was able to start all games and make the ROM tests pass, except wwallyj. Something is strange with that game. Doesn't seem the controller, the CPU goes to 'unchartered waters'.

gyurco commented 3 months ago

Am I doing it wrong with jtsim? First preload the rom:

jtsim -setname wwallyj

then try to do some dumps:

jtsim -w -video 100

But it doesn't seem to use the correct ROM data.

jotego commented 3 months ago

Splitting it in two steps, so ROM load does not need to be done each time, is not working well for this core. I debugged it a bit but could not get it working yet. Do it in a single step and it will work:

jtsim -setname wwallyj -w -video 100

gyurco commented 3 months ago

Ok, no problem. (just it takes a more time and disk space to dump during ROM downloading).

gyurco commented 3 months ago

...but I only wants to see only once where the CPU is derailed.

jotego commented 3 months ago

I know. Skipping the ROM load for faster sims is a JTFRAME standard so I'll fix it eventually.

gyurco commented 3 months ago

Something with the decryption? At $542, it fetches $4123 for the operand of STOP image

However in mame, the instruction at $540 is STOP #$2000

image

Even at $53E, the data input is $2240 instead of $2300.

jotego commented 3 months ago

Ok, no problem. (just it takes a more time and disk space to dump during ROM downloading).

Now that I'm checking it, I had actually implemented this. It has to be enabled manually with a macro in this case though. That's because the offset of each region is stored in the header and that makes things a bit more convoluted.

Use jtsim -d SIM_LOAD_PROM to manually force the full PROM download.

Overall, the fastest way to setup the sim in s18 is:

jtutil sdram
jtsim -d -d SIM_LOAD_PROM

The first command generates the SDRAM files bypassing simulation. I have not tested it on all cores. Alternatively, you can do jtsim -setname xxx which will always work.

gyurco commented 3 months ago

Seems there's a bug handling the special instructions (cmp.l in this case). The cmp.l is detected at the prefetch cycle, but after that there can be data access of the previous instruction (instead of immediate fetch of the cmp.l's operand) , and if that happens, the core 'forget' to act on the cmp.l to change decryption state.

gyurco commented 3 months ago

Here's the sim: image $536 is cmp.l's opcode's prefetch then two writes from the previous instruction $538 and $53a are the operands of cmp.l, but stchange already cleared.

gyurco commented 3 months ago

Deleting the code which resets stchange after two non-instruction fetch cycle fixes Wally startup and other games also works (tried with running them for a bit in attract mode). Do you have an idea why this code was needed?

diff --git a/cores/s16/hdl/jts16_fd1094_ctrl.v b/cores/s16/hdl/jts16_fd1094_ctrl.v
index 6ea15f33..e7cdce46 100644
--- a/cores/s16/hdl/jts16_fd1094_ctrl.v
+++ b/cores/s16/hdl/jts16_fd1094_ctrl.v
@@ -78,10 +78,6 @@ always @(posedge clk, posedge rst) begin
             // rte
             if( dec == 16'h4e73 ) irqmode <= 0;
         end
-        if( !dtackn && dtacknl ) begin
-            other <= op_n;
-            if(other && op_n) stchange<=0;
-        end
     end
 end
jotego commented 3 months ago

That is very interesting. The original commit for that change is in the old jts16 repository (here). The commit only adds the lines you have deleted (remember to delete the declaration for other too). So this must have fixed something in a System 16A/B game. I have looked in the issues for JTS16 (also in the third repository we used for it) but could not find the reference for this change.

Please push the commit to your branch and @jtmiki will test the S16A/S16B games that use FD1094. The crashes due to this may happen later in the game, not as soon as in wwallyj so this is a bit tricky.

gyurco commented 3 months ago

Pushed the change, I wonder if it'll break something on s16.

jotego commented 3 months ago

After much testing, it looks like S16 is stable so we can go ahead with this change.

jotego commented 3 months ago

Please make a PR for this issue so we close it.

jotego commented 3 months ago

Thank you. All games boot up correctly. @jtmiki this issue is closed.