Skip to content

fix(nr-8px): JSR reads its high target byte last; DMC timer phase, load-DMA delay and $4016 bit deletion match Mesen2 - #3341

Merged
rmstdope merged 3 commits into
mainfrom
nr-8px
Oct 4, 2026
Merged

rmstdope merged 3 commits into
mainfrom
nr-8px

Conversation

@rmstdope

@rmstdope rmstdope commented Oct 4, 2026

Copy link
Copy Markdown
Owner

Bead: nr-8px (NES attract demos diverge from Mesen2 in game state). Follow-ups: nr-xyiu (4 ROMs left, cause not established), nr-z3gl (the $4017 side of fix 4).

What was wrong

Four timing bugs moved DMC sample fetches, and controller reads under them, off Mesen2. Games that play DPCM then made different decisions.

  1. JSR bus order. JSR fetched both target bytes up front. The 6502 reads the high byte last, on cycle 6, after pushing the return address. A DMC halt due during the pushes therefore landed on the next opcode, with 3 stall cycles instead of 4. Fix in get_operand and execute (src/nes/cpu/cpu/execute.rs).
  2. DMC timer phase at power-on. Mesen2 clocks the timer through its 8-cycle reset sequence; NESER started the first period at the first instruction, so every fetch was 8 cycles late (the specification leaves this phase open). Fix in Dmc::reinit_timer_after_reset.
  3. Load DMA after $4015. The CPU saw it 1/2 cycles after the write (even/odd). NESdev "DMA" says it halts on the get cycle of the 2nd APU cycle after the write, which is 3/4 cycles, as in Mesen2. Fix in Dmc::set_enabled.
  4. $4016 read halted by a DMC fetch. The halt cycle was a dummy read, so the pad saw a single clock and never lost a bit. NESdev "DMA" (Register conflicts) and Mesen2 give it one extra clock. Fix in process_pending_dma; $4017 is deliberately unchanged (nr-z3gl).

Tests

  • New test_jsr_reads_high_target_byte_after_pushing_return_address: JSR at $01FD, whose high operand byte is overwritten by the PCH push. Was $1234, now $0134.
  • New test_timer_after_reset_sequence_ticks_420_cycles_into_the_program and test_load_dma_after_enable_is_seen_by_the_cpu_three_or_four_cycles_later. Both fail on main.
  • test_read_joy3_count_errors(_fast) re-pinned from NESER's own "0/1000" to Mesen2's "CONFLICTS: 67/1000" / "ERRORS: 15/1000". Mesen2's frame 900 matches NESER's at 0 px for both ROMs. They fail without fix 1, 2 or 4.
  • test_dmc_tests_latency (an audio-alternation heuristic, recalibrated once before when the phase changed) re-pinned from 70 to 71.

Validation

./scripts/gate-full.sh: passed. nr-tu3 checkpoint recipe (zero RAM, no input, frames 300..3600, both ports pinned to joypads) on the bead's 16 ROMs and on the 9 ROMs nr-xgb flagged:

  • Now match at all 12 checkpoints: contra, contra-t-porta1, 22-in-1, double-dribble (bead list); snakes-revenge, tiny-toon-adventures-2 (nr-xgb list).
  • The recorded nr-046 difference (NESER delays an NMI inside a taken branch per NESdev; Mesen2 does not, and the navigator chose to keep that). These match at all 12 only in a throwaway build that takes NMI the Mesen2 way: alex-demeos-race-america, donkey-kong-classics, donkey-kong-jr-ju, mappy-land, solomons-key, urban-champion, gunsmoke, ultimate-basketball, and also ninja-gaiden. In that build, nothing in either list got worse than on main, and 20 of the 25 match.
  • Shipping (spec-mode NMI) changes that come from nr-046 alone: ninja-gaiden now differs at 900, and joe-mac at 2700 as well as 2400. Both match Mesen2 when NMI is taken the Mesen2 way, so the CPU/DMC timing is right; the nr-046 delay moves the game somewhere else.
  • Left, with other causes (nr-xyiu): advanced-dungeons-dragons-dragonstrike, blades-of-steel, mortal-kombat-trilogy, three-stooges-the (its CPU timing matches Mesen2 at every NMI; 28 px at frame 1200 is a rendering difference).

No deviation from the bead; a bug bead has no plan, so the reproduction tests above are the plan.

🤖 Generated with Claude Code

https://claude.ai/code/session_016JbwWdcsgsf3sY8e5bgMDo

rmstdope and others added 2 commits October 4, 2026 15:21
…ad-DMA delay and $4016 bit deletion match Mesen2

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016JbwWdcsgsf3sY8e5bgMDo
…read; fix a stale delay comment

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016JbwWdcsgsf3sY8e5bgMDo
@rmstdope

rmstdope commented Oct 4, 2026

Copy link
Copy Markdown
Owner Author

Review (cold read, reviewed_head: a5d2092ad81964f01eb79ea0704546f50f7798af) — reviewed before merge under the producer's review practice, by an agent given the diff and the bead, and not the producer's reasoning. A bug bead has no plan, so the reviewer read the diff against the bead's bug claim and the reproduction tests.

All four fixes check out against the bead's claim and against Mesen2 (NesCpu.cpp, DeltaModulationChannel.cpp). JSR now leaves PC on the high byte, which is the old return address, and reads that byte after the pushes through the DMA-haltable read. The timer at period - 9 equals Mesen2's period−1 clocked 8 times. The 3/4 load delay is Mesen2's 2/3 plus its one-cycle visibility lag. For $4016 the pad now gets two clocks, as NESdev's register-conflicts section describes. Nothing blocks the merge.

  1. No focused test pins fix 4 (src/nes/cpu/cpu/dma.rs). Only the statistical read_joy3 counts would catch a regression, and they depend on fixes 1 and 2 as well. Suggest a unit test that a DMC DMA on a multi-byte sample, halting a $4016 read, clocks the pad twice; ideally the aliasing (should_skip_first_input_clock) case too.
  2. Minor: the comment in test_dmc_dma_stalls_cpu_on_sample_fetch (src/nes/console/nes.rs) still says a "transfer_start_delay of 2-3 cycles"; it is now 3-4.
  3. Question: the load delay (3/4) now expires after the disable delay (2/3), so $4015 enable followed quickly by disable ends with bytes_remaining = 0 before the load DMA fires. That is Mesen2's order too, but no test pins it.

Producer's replies (head 47b124cf):

  1. Changed. New test_dmc_dma_halting_4016_read_clocks_the_pad_twice (src/nes/cpu/cpu/tests.rs) presses only B and reads $4016 under a DMC DMA on a 17-byte sample. The first read must return B (A deleted) and the next one Select. On main it returns A, then B. The aliasing case is not added: this PR does not change that branch (it still makes the halt a dummy read), and should_skip_first_input_clock has its own unit tests.
  2. Changed: the comment now says 3-4.
  3. Not changed. The order now matches Mesen2's ProcessClock (disable checked before the load delay, with the same cycle counts), and the full gate is green, including dmc_dma_during_read4, sprdma_and_dmc_dma, dma_sync_test_v2 and the blargg DMC tests. A dedicated test for the enable-then-disable race would pin a Mesen2 choice the bead did not examine, so it is left out.

…ut; nr-046 noise in Mesen2 comparisons

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016JbwWdcsgsf3sY8e5bgMDo
@rmstdope
rmstdope merged commit f42e184 into main Oct 4, 2026
12 checks passed
@rmstdope
rmstdope deleted the nr-8px branch October 4, 2026 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant