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

karnov/cps1/cps15: blank screen #324

Closed
jotego opened this issue Aug 10, 2023 · 48 comments
Closed

karnov/cps1/cps15: blank screen #324

jotego opened this issue Aug 10, 2023 · 48 comments
Assignees
Labels
core bug Something isn't working

Comments

@jotego
Copy link
Owner

jotego commented Aug 10, 2023

it looks like after the recent change to jtframe_68kdtack, jtkarnov does not boot correctly (both Karnov and Chelnov). Seen in c874ac4 and probably started with the jtframe_68kdtack change.

It affects

  • Karnov
  • CPS1
  • CPS15
  • SF
@jotego jotego added the core bug Something isn't working label Aug 10, 2023
@jotego jotego added this to the karnov milestone Aug 10, 2023
@jotego jotego changed the title karnov: blank screen karnov/cps1: blank screen Sep 3, 2023
@jotego jotego self-assigned this Sep 3, 2023
@jotego jotego modified the milestones: karnov, cps Sep 3, 2023
@jotego
Copy link
Owner Author

jotego commented Sep 3, 2023

public working version is 46ba76a

@jotego jotego changed the title karnov/cps1: blank screen karnov/cps1/cps15: blank screen Sep 7, 2023
jotego added a commit that referenced this issue Sep 8, 2023
@jotego
Copy link
Owner Author

jotego commented Sep 8, 2023

  • CPS1
  • CPS15
  • SF

Fixed in 1e5a491. The problem was that the SDRAM was doing a write access with the write mask at 'b11 (disabled). That happened as the ram_cs was active when AS was asserted and the access was registered there but UDS/LDS are asserted later.
It looks like RW is asserted at the same time than AS, though.
The logic was changed to wait until UDS/LDS are asserted before enabling the ram_cs and other *cs signals.

@asturur
Copy link

asturur commented Sep 8, 2023

is there a way to try this commit yet?

@jotego
Copy link
Owner Author

jotego commented Nov 3, 2023

Karnov public version is 46ba76a. At that time the repository used JTFRAME as a submodule, so it is hard to switch between the current commit (JTFRAME as a folder) and the old one.
Trying the old commit renders a working core on MiST, but the verilator simulation does not boot up.

@jotego
Copy link
Owner Author

jotego commented Nov 3, 2023

The last one with JTFRAME as a submodule still worked: 6725662
The first one with JTFRAME as a folder that works is: 9438bdd

@jotego
Copy link
Owner Author

jotego commented Nov 4, 2023

cf58e37 is the commit that broken karnov

@jotego
Copy link
Owner Author

jotego commented Nov 4, 2023

  • just taking karnov_ok's dtack module to master's HEAD (7d15969) does not fix it
  • simulation does not work on karnov_ok (but the FPGA does)

@gyurco
Copy link
Collaborator

gyurco commented Jan 16, 2024

From a quick debugging session, it looks like RAM writes don't go through. E.g. at 5BE, the JSR should write the next instruction address to the stack (5c2), however at the place of the RTS, only 0000 read back.
image

@jotego jotego assigned gyurco and unassigned jotego and florgana Jan 16, 2024
@jotego
Copy link
Owner Author

jotego commented Jan 16, 2024

Sometimes comparing with the working version is an easy way to find the problem. You can save the waveforms in the two versions and compare. If you convert a few critical waveforms to VCD, you can use jtutil vcd compare to find the time at which the signals split up.

These M68000 games are very sensitive to how the DTACK signal is generated. Even though the generation should be straight forward, I've found all sort of odd sensitivites over the years. I think we broke this game when we fixed Cotton in System 16B, by modifying the DTACK generation. Dynamite Dux also had a weird DTACK sensitivity #279 and so did Shinobi. So it is always scary to touch that module. Yet, the same module should work for all games. There is no technical justification for requiring two different versions of it.

@gyurco
Copy link
Collaborator

gyurco commented Jan 16, 2024

I'm a bit lost in the SDRAM controller, there are so many components...

I followed the data to jtframe_ram_rq:u_slot0:
req_rnw=0, sdram_addr=101fff, wrdata=05c2h and req=1 when the address is pushed into the stack,
req_rnw=1, sdram_addr=101fff and req=1 when the RTS should pops it out, but dout is constantly 0000, DTACK doesn't matter, it could wait forever for the correct data, it doesn't arrive.

Also there's a case when dout reflects wrdata during write, and another case it's not. So I guess I have to understand the logic first.

@gyurco
Copy link
Collaborator

gyurco commented Jan 16, 2024

Also there's 'we' and 'wrin' inputs, but what's the difference? I see we arrives later then wrin, then the module acts on wrin.

@gyurco
Copy link
Collaborator

gyurco commented Jan 16, 2024

It's the write:
image

And the read:
image

The read arrives in 1 cycle, so it must be cached from the previous read, I guess. But it's not identical to the one is written.

@jotego
Copy link
Owner Author

jotego commented Jan 17, 2024

This core is a bit unusual in the sense that it writes to SDRAM bank 3, rather than 0. There is a macro to enable that

JTFRAME_BA3_WEN

But it might be failing. You can easily move it to bank 0 by dragging those lines in mem.yaml up to the bank 0 part, where the main CPU ROM is.

I think I tried that the last time I looked at this. But I don't remember well. Maybe it's written in this issue log.

That's the only funny thing I can think of related to the SDRAM as writes to banks other than the zero have not been used elsewhere as far as I remember.

@gyurco
Copy link
Collaborator

gyurco commented Jan 17, 2024

I noticed that macro. As now I went up to the top with the jtframe_ramx_xslot signals, the next step is to move into the real sdram controller.

@gyurco
Copy link
Collaborator

gyurco commented Jan 17, 2024

BTW, CPU ROM and RAM logically should be put to the same bank, as there can be no overlapped access between the two.

@gyurco
Copy link
Collaborator

gyurco commented Jan 17, 2024

Looks like objram_cs for writes goes to BRAM? Then why obj_cs goes to jtframe_ram1_2slots?

@gyurco
Copy link
Collaborator

gyurco commented Jan 17, 2024

BTW, the game has music, and it seems to run...just without any graphics.

@gyurco
Copy link
Collaborator

gyurco commented Jan 17, 2024

I just died three times, I got endgame tune :D Now it's in demo mode...with a full black screen.

@gyurco
Copy link
Collaborator

gyurco commented Jan 17, 2024

Ah, obj_cs is for tilemap data...then probably it's not written into bank3 during programming.

@gyurco
Copy link
Collaborator

gyurco commented Jan 17, 2024

Here's the diff which makes the CPU RAM OK (also makes Verilator/FPGA behavior match):

diff --git a/modules/jtframe/hdl/sdram/jtframe_sdram64.v b/modules/jtframe/hdl/sdram/jtframe_sdram64.v
index 325bffe8..34817fbc 100644
--- a/modules/jtframe/hdl/sdram/jtframe_sdram64.v
+++ b/modules/jtframe/hdl/sdram/jtframe_sdram64.v
@@ -97,7 +97,7 @@ module jtframe_sdram64 #(
     // of the SDRAM, as done in the MiSTer 128MB module
     inout       [15:0]  sdram_dq,       // SDRAM Data bus 16 Bits
     `ifdef VERILATOR // sdram_dq is used as input-only in Verilator sims
-    output      [15:0]  sdram_din,      // Data to be stored in SDRAM
+    output reg  [15:0]  sdram_din,      // Data to be stored in SDRAM
     `endif
     output reg  [12:0]  sdram_a,        // SDRAM Address bus 13 Bits
     output              sdram_dqml,     // SDRAM Low-byte Data Mask
@@ -136,7 +136,7 @@ reg  [14:0] prio_lfsr;
 wire [12:0] bx0_a, bx1_a, bx2_a, bx3_a, init_a, next_a, rfsh_a,
             ba0_row, ba1_row, ba2_row, ba3_row;
 wire [ 1:0] next_ba, prio;
-reg  [15:0] din;
+wire [15:0] din;
 
 wire [AW-1:0] ba0_addr_l, ba1_addr_l, ba2_addr_l, ba3_addr_l;
 wire    [3:0] rd_l, wr_aux;
@@ -188,8 +188,6 @@ assign mask_mux = prog_en ? prog_dsn :
 `ifndef VERILATOR
 reg  [15:0] dq_pad;
 assign sdram_dq = dq_pad;
-`else
-assign sdram_din = prog_en ? prog_din : din;
 `endif
 
 always @(negedge clk) begin
@@ -198,6 +196,10 @@ always @(negedge clk) begin
     other_rst <= prog_en | init | rst;
 end
 
+assign din = (bg[3] && BA3_WEN) ? ba3_din :
+             (bg[2] && BA2_WEN) ? ba2_din :
+             (bg[1] && BA1_WEN) ? ba1_din : ba0_din;
+
 always @(posedge clk) begin
     dst      <= ba_dst;
     rdy      <= ba_rdy;
@@ -217,12 +219,11 @@ always @(posedge clk) begin
     sdram_a[10:0] <= next_a[10:0];
 
     wr_l <= wr_aux;
-    din <= (bg[3] && BA3_WEN) ? ba3_din :
-           (bg[2] && BA2_WEN) ? ba2_din :
-           (bg[1] && BA1_WEN) ? ba1_din : ba0_din;
 
 `ifndef VERILATOR
     dq_pad <= wr_cycle ? (prog_en ? prog_din : din) : 16'hzzzz;
+`else
+    sdram_din <= prog_en ? prog_din : din;
 `endif
     if( MISTER ) begin
         if( next_cmd==CMD_ACTIVE )
_en ? prog_din : din) : 16'hzzzz;

Meanwhile obj_cs is never asserted during the running game, obj_addr is always 0. Maybe VRAM/Sprite RAM writes are bad (but they're in BRAM).

@jotego
Copy link
Owner Author

jotego commented Jan 17, 2024

Very good progress! If look at the log:

cf58e37 is the commit that broken karnov

commit cf58e37 (tag: karnov_bad)
commit f6995e3 (tag: karnov_ok)

And if you compare those two commits, the sdram controller was not touched. Indeed the only common file was the DTACK generation. Maybe the new DTACK unveiled a weakness in the controller?

Did you check that Verilator sims still work after the change? Do you get video in the sims?

@gyurco
Copy link
Collaborator

gyurco commented Jan 17, 2024

I got this with f6995e3:
panic: open /home/gyuri/git/jtcores/modules/jtframe/hdl/inc/game_sdram.v: no such file or directory

I didn't check anything with simulator yet.

@jotego
Copy link
Owner Author

jotego commented Jan 17, 2024

Recompile jtframe each time you change to an old (or new) commit. Just type jtframe and it should recompile itself.

@gyurco
Copy link
Collaborator

gyurco commented Jan 17, 2024

It's surely something with the address decoder, as CPU write to 80000 doesn't trigger objram_we at all.

@gyurco
Copy link
Collaborator

gyurco commented Jan 17, 2024

Well, if dsn is low active, then this:

assign objram_we  = {2{objram_cs}} & main_dsn;

won't work.

@gyurco
Copy link
Collaborator

gyurco commented Jan 17, 2024

This one does the job well:

diff --git a/cores/karnov/hdl/jtkarnov_game.v b/cores/karnov/hdl/jtkarnov_game.v
index ef8b4f73..af60e211 100644
--- a/cores/karnov/hdl/jtkarnov_game.v
+++ b/cores/karnov/hdl/jtkarnov_game.v
@@ -38,9 +38,9 @@ assign debug_view = debug_bus[7] ? st_main : mcu_st;
 assign ram_we     = ram_cs & ~main_wrn;
 assign dma_we     = {2{pre_dma_we}};
 
-assign vram_we    = {2{vram_cs  }} & main_dsn;
-assign scrram_we  = {2{scrram_cs}} & main_dsn;
-assign objram_we  = {2{objram_cs}} & main_dsn;
+assign vram_we    = {2{vram_cs   & ~main_wrn}} & ~main_dsn;
+assign scrram_we  = {2{scrram_cs & ~main_wrn}} & ~main_dsn;
+assign objram_we  = {2{objram_cs & ~main_wrn}} & ~main_dsn;
 
 // Remove this when bus contention is done
 assign sdtkn = 0;

Now Karnov kills!

@gyurco
Copy link
Collaborator

gyurco commented Jan 17, 2024

Looks like the sim also produces something:
frame_00003

@gyurco
Copy link
Collaborator

gyurco commented Jan 17, 2024

What I noticed in the game that sprites are 'dancing' during horizontal scroll. I saw similar thing in Sega System1, where the sprite X position had to be rounded:
Gehstock/Mist_FPGA@983ede0#diff-537cb8e07f29e5147253d24fb2ed61daf1e312a4194312082bf05692ba4f4309L150
(line 150)

@gyurco gyurco mentioned this issue Jan 17, 2024
@jotego
Copy link
Owner Author

jotego commented Jan 18, 2024

Excellent news!

The *_we assignments were wrong indeed. Looking at the git log, I think the write signals had been merged in the mem template before and when I consolidated all repos into one, this mismatch between the mem template and the core occured and this regression error appeared. That's why I've been trying to have regression tests for a year! Thanks a lot for finding this. DTACK had nothing to do in the end.

The sprite dancing should be related to when the object table buffer is updated. I think that rounded the X position is the wrong solution. Games normally write the sprite data to a table that needs to be copied into the rendering stage at a specific time. What controls that is system dependent. Also, some games in some systems do it wrong due to bad software, not bad hardware. Once we close this issue, I think it's worth it opening a different one to review the sprite DMA.

@jotego
Copy link
Owner Author

jotego commented Jan 18, 2024

@gyurco, could you check if #409 still occurs? I think it must have been related to the missing signals in the write.

@gyurco
Copy link
Collaborator

gyurco commented Jan 18, 2024

@gyurco, could you check if #409 still occurs? I think it must have been related to the missing signals in the write.

It looks okay now.

jotego pushed a commit that referenced this issue Jan 18, 2024
* JTFrame: fix sdram64 writes by making din combinatorial (less delay)
* Karnov: fix gfx RAM write enables
@jotego jotego closed this as completed Jan 18, 2024
@jotego
Copy link
Owner Author

jotego commented Jan 19, 2024

Simulation is not working for me for karnov/chelnov. Can you double check on your end please?

@gyurco
Copy link
Collaborator

gyurco commented Jan 19, 2024

It still works with current master:

Verilator sim starts
Multibank SDRAM enabled
WARNING: (test.cpp)sdram_bank0.bin not found
WARNING: (test.cpp)sdram_bank1.bin not found
WARNING: (test.cpp)sdram_bank2.bin not found
WARNING: (test.cpp)sdram_bank3.bin not found
Read 1284096 bytes from rom.bin
Simulation clock period set to 20832 ps (48.003072 MHz)
Verilator will dump to test.vcd
DIP sw set to FFBF
WARNING: TOP.game_test.u_game.u_game.u_mcu.u_prom cannot open file: ../../../../rom/chelnov/ee-e.k14
WARNING: TOP.game_test.u_game.u_bram_vram.u_lo.u_ram cannot open file: vram_lo.bin
WARNING: TOP.game_test.u_game.u_bram_vram.u_hi.u_ram cannot open file: vram_hi.bin
WARNING: TOP.game_test.u_game.u_bram_scrram.u_lo.u_ram cannot open file: scrram_lo.bin
WARNING: TOP.game_test.u_game.u_bram_scrram.u_hi.u_ram cannot open file: scrram_hi.bin
WARNING: TOP.game_test.u_game.u_bram_objram.u_lo.u_ram cannot open file: objram_lo.bin
WARNING: TOP.game_test.u_game.u_bram_objram.u_hi.u_ram cannot open file: objram_hi.bin
jtframe_pxlcen: using 4 as clock divider
SDRAM burst mode changed to 2 mask 0xFFFFFFFE -> burst per bank = { 2, 2, 2, 2 }

@jotego
Copy link
Owner Author

jotego commented Jan 19, 2024

Did you check the output in the frames folder? I only get a blank first frame.

@gyurco
Copy link
Collaborator

gyurco commented Jan 19, 2024

I got two files, frame_00001 and frame_00027, the first is blank, the second is good (high scores on it).
I don't know why just two files...the one I posted above was frame_00009 I think. Looks the sim chooses two random frames?

@jotego
Copy link
Owner Author

jotego commented Jan 19, 2024

By default, you only get a new image file when it is different from the previous one in order not to flood the folder and your eyes with irrelevant information.

I am stuck at frame_00001. Can you please try going through the load process: jtsim -setname chelnov -video 100?

@gyurco
Copy link
Collaborator

gyurco commented Jan 19, 2024

BTW, I copied the .rom file by hand to jtcores/rom, as it wrote 'cannot produce chelnov.rom'
Running with bash -x, it just revealed that it just looking for the .rom, but it doesn't want to 'produce' it.

@jotego
Copy link
Owner Author

jotego commented Jan 19, 2024

BTW, I copied the .rom file by hand to jtcores/rom, as it wrote 'cannot produce chelnov.rom' Running with bash -x, it just revealed that it just looking for the .rom, but it doesn't want to 'produce' it.

Things to check:

  • always source setprj.sh in your current terminal (or source it in .bashrc)
  • jtframe mra looks for MAME zip files in ~/.mame/roms

jtframe mra corename will create the .rom file. No need for the mra tool. When calling jtsim -setname xxx it will run jtframe mra for you, generate the file in $ROM and link it.

@gyurco
Copy link
Collaborator

gyurco commented Jan 19, 2024

The issue was I had ~/mame/roms, now I have ~/.mame/roms, too. ROM files are generated.
I get this for frame_00036:
frame_00036

@jotego
Copy link
Owner Author

jotego commented Jan 19, 2024

Thank you. I'll check it further on my end.

@gyurco
Copy link
Collaborator

gyurco commented Jan 19, 2024

Just a question: what macro adds core_mod[2] to the MRA? I think it's missing in 1943, even the OSD is rotated in the wrong direction with current master (and old MRA). Comparing with 1942 (which has 5) I don't see any obvious macro to affect this.

..but probably rotation things could belong a new issue.

@jotego
Copy link
Owner Author

jotego commented Jan 19, 2024

core_mod

Bit Use Set by
0 High for vertical games mame.xml
1 4-way joysticks JTFRAME_SUPPORT_4WAY
2 XOR with dip_flip mame.xml
3 dial input enable mame.xml
4 reverse the dial mame.xml

Most core_mod bits are derived from MAME. Some games produce a flip bit that means the opposite as in MAME. In that case dip_flip must be assigned to ~flip. Making sure everything is consistent is a bit of a pain that pops up from time to time. When things are right, you get the right orientation in MiST's OSD and the right orientation in MiSTer's rotation and simulation. All should agree if the core produces the right dip_flip value.

If there's something to fix, let's move it to a different issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core bug Something isn't working
Projects
Status: ✅ Done
Development

No branches or pull requests

4 participants