bfirsh / bfirsh/jsnes

Implement cycle-accurate DMA bus interleaving

Open
#535 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
6.4k
Forks
857
PR merge metrics
No merged PRs in 30d

Description

# Cycle-Accurate DMC DMA Bus Interleaving

## Context

The emulator currently handles DMA atomically: OAM DMA copies 256 bytes in a loop
then calls `haltCycles(513)`, and DMC DMA fetches a byte then calls `haltCycles(4)`.
The frame loop processes halt cycles in bulk chunks of 8, with no actual bus operations.

On real hardware, DMA performs real bus reads/writes each cycle. During OAM DMA's 513
halt cycles, the CPU repeats its last read (with side effects on I/O registers), DMC
DMA can steal cycles from OAM DMA, and the data bus is updated on each read. 10
AccuracyCoin tests fail because of this.

The goal is to make DMA cycle-accurate: OAM DMA reads source bytes through the mapper
(triggering I/O side effects), DMC DMA interleaves with OAM DMA, data bus tracks
correctly, and get/put alignment determines cycle counts.

## Approach: DMA State Machine in Frame Loop

Replace the atomic DMA + `haltCycles()` pattern with a cycle-by-cycle DMA state
machine. Instructions remain atomic (no rewrite of `emulate()`). DMA processing
happens in the frame loop's halt-cycle branch, one cycle at a time.

**Key insight**: OAM DMA already runs between instructions (triggered by `STA $4014`
completing). The failing tests check what happens *during* those halt cycles, not
mid-instruction. We just need to make halt-cycle processing cycle-accurate.

## Files to Modify

- `src/cpu.js` - Add DMA state fields, get/put tracking, `lastReadAddress`
- `src/nes.js` - Rewrite halt-cycle processing as DMA state machine
- `src/ppu.js` - Change `sramDMA()` from atomic copy to state machine setup
- `src/papu.js` - Change `nextSample()` from inline fetch to pending flag
- `test/accuracycoin.spec.js` - Remove fixed tests from KNOWN_FAILURES

## Implementation Steps

### Step 1: Add DMA state fields to CPU (`cpu.js`)

Add to constructor and `reset()`:

```
dmaActive: 0 // 0=idle, 1=OAM active, 2=DMC active (can overlap)
oamDmaPage: 0 // Source page for OAM DMA ($XX)
oamDmaByte: 0 // Current byte index 0-255
oamDmaPhase: 0 // 0=halt, 1=align, 2=get, 3=put
oamDmaValue: 0 // Value read during get phase
dmcDmaPending: false // DMC wants a DMA fetch
dmcDmaPhase: 0 // 0=halt, 1=dummy, 2=align, 3=read
cpuCycleOdd: false // Get/put alignment (tracks APU half-cycle)
lastReadAddress: 0 // Address of last CPU read (repeated during halt)
```

Add all to `JSON_PROPERTIES`. Update `lastReadAddress` in `load()` (not `write()`).

### Step 2: Change OAM DMA from atomic to state machine (`ppu.js`)

Replace `sramDMA()`:
- Remove the 256-byte copy loop and `haltCycles(513)`
- Instead: set `cpu.oamDmaPage = value`, `cpu.oamDmaByte = 0`,
`cpu.oamDmaPhase = 0`, `cpu.dmaActive |= 1`
- The actual byte transfers happen in the frame loop

### Step 3: Change DMC DMA from inline fetch to pending (`papu.js`)

In `nextSample()`:
- Remove `mmap.load(playAddress)` and `haltCycles(4)`
- Instead: set `cpu.dmcDmaPending = true`, save `this.dmcDmaAddress = playAddress`
- Still update `playLengthCounter--`, `playAddress++` (address state advances immediately)
- `hasSample` stays false until the DMA read cycle completes in the frame loop

In `clockDmc()`: skip bit-shifting when `!hasSample && dmcDmaPending` (buffer
empty, DMA in flight).

In `writeReg($4015)` initial fetch path: also use pending flag instead of
immediate fetch.

### Step 4: Rewrite frame loop halt processing (`nes.js`)

Replace the `cyclesToHalt > 0` branch with cycle-by-cycle DMA processing:

```
if (cpu.dmaActive > 0 || cpu.dmcDmaPending) {
cycles = 3; // 1 CPU cycle = 3 PPU dots
papu.clockFrameCounter(1);
cpu.cpuCycleOdd = !cpu.cpuCycleOdd;
processDmaCycle(cpu, ppu, papu);
}
```

The `processDmaCycle()` method:

1. **DMC DMA has priority** over OAM DMA. If `dmcDmaPending` and OAM is in
get/put, DMC steals the cycle and OAM retries.

2. **OAM DMA state machine**:
- Phase 0 (halt): Re-read `lastReadAddress` through mapper (side effects!),
advance to align or get based on `cpuCycleOdd`
- Phase 1 (align): Wait for get cycle
- Phase 2 (get): Read `mmap.load((oamDmaPage << 8) | oamDmaByte)`, update
`dataBus`, go to put
- Phase 3 (put): Write to `spriteMem[(sramAddress + oamDmaByte) & 0xff]`,
increment byte, loop or finish

3. **DMC DMA state machine**:
- Phase 0 (halt): Re-read `lastReadAddress`, advance
- Phase 1 (dummy): No-op cycle
- Phase 2 (align): Wait for get cycle if needed
- Phase 3 (read): `mmap.load(dmcDmaAddress)`, set `hasSample = true`,
clear `dmcDmaPending`

4. **Repeated reads**: During halt/dummy/align, the CPU repeats its last read.
Read from `lastReadAddress` through the mapper. This triggers side effects
on $2002 (clears VBlank), $2007 (advances VRAM address), $4015 (clears IRQ
flags), $4016/$4017 (shifts controller). This is what the DMA+register tests
check.

### Step 5: Get/put cycle alignment

Track `cpuCycleOdd` on CPU state. Toggle on every CPU cycle:
- After each instruction: `cpuCycleOdd ^= (cycleCount & 1)`
- During DMA: toggle once per cycle in `processDmaCycle()`

Get cycles = even (`!cpuCycleOdd`), put cycles = odd. OAM DMA's get phase
requires a get cycle. If the first cycle after halt is a put cycle, an extra
alignment cycle is inserted (514 cycles instead of 513).

### Step 6: DMA abort handling

**Explicit abort** (0x0479): Writing `$4015` with DMC bit clear while DMC DMA
is in progress. Check: if `dmcDmaPending` when $4015 disables DMC, clear
`dmcDmaPending` and `dmcDmaPhase`.

**Implicit abort** (0x0478): When the last sample byte has already been fetched
and the DMC tries to reload but there's nothing left. The DMA fires but aborts
after the halt cycle because `playLengthCounter == 0`.

### Step 7: Remove old haltCycles usage

After steps 2-4 are complete:
- `haltCycles()` is no longer called by anyone (only OAM DMA and DMC DMA
called it, and they now use the state machine)
- Keep `haltCycles()` and `cyclesToHalt` as dead code initially for safety,
then remove once all tests pass

### Step 8: Update save state serialization

Add new fields to `JSON_PROPERTIES` in `cpu.js`. Add `dmcDmaAddress` to DMC
channel's `JSON_PROPERTIES` in `papu.js`.

## Tests This Should Fix

| Test | Addr | What it checks |
|------|------|----------------|
| DMA + Open Bus | 0x046c | Data bus values during DMA cycles |
| DMA + $2002 Read | 0x0488 | Repeated reads clear VBlank flag |
| DMA + $2007 Read | 0x044c | Repeated reads advance VRAM address |
| DMA + $2007 Write | 0x044f | DMA interacting with VRAM writes |
| DMA + $4015 Read | 0x045d | Repeated reads affect APU status |
| DMA + $4016 Read | 0x045e | Repeated reads shift controller |
| DMC DMA bus conflicts | 0x046b | DMC DMA during instructions |
| DMC DMA + OAM DMA | 0x0477 | DMC steals cycles from OAM |
| Explicit DMA abort | 0x0479 | Disabling DMC cancels pending DMA |
| Implicit DMA abort | 0x0478 | DMA aborts when no sample left |

May also fix:
- 0x046a (DMC not accurate) - subtests that depend on DMA timing
- 0x045c (APU register activation) - may depend on DMA cycle accuracy

## Not Fixed By This Plan

- CPU interrupt tests (0x0461/0x0462/0x0463) - need interrupt polling timing
- Controller strobing subtest 4 (0x045f) - needs per-cycle CPU get/put tracking
- APU frame counter IRQ subtest 7 (0x0467) - needs APU get/put alignment

## Verification

1. Run `npm test` after each step to catch regressions
2. Focus on AccuracyCoin DMA tests one at a time (start with 0x046c, then
0x0488, 0x044c, etc.)
3. Run full test suite including nestest and existing AccuracyCoin passes
4. Test with real games (especially those using DMC audio) to verify no
audio regression

## Risk Assessment

**Performance**: Minimal. DMA cycles are ~520 per frame during OAM DMA (1.7% of
~29780 cycles/frame). Processing them individually instead of in chunks of 8 is
negligible.

**Regression risk**: Medium. The frame loop restructuring must be careful.
Mitigation: keep `cyclesToHalt` as fallback initially.

**DMC timing**: Separating "buffer empty" from "DMA fetch" requires careful
`hasSample`/`dmcDmaPending` coordination. `clockDmc()` must handle the window
between buffer-empty and DMA-complete.

## Open Questions

1. **0x046b (DMC DMA bus conflicts)**: The plan handles this through the state
machine, but if the test specifically checks mid-instruction DMA hijacking
(not just during halt cycles), we may need a targeted `load()` hook as a
Phase 2 extension. Start with the state machine approach and see what
subtest fails.

2. **Repeated reads during halt**: The nesdev wiki says "the CPU repeats the
last read cycle." We need to confirm whether the test ROMs arrange for the
last read to be from an I/O register, or whether the halt itself causes
reads from a specific address. Track `lastReadAddress` and verify against
test behavior.

3. **OAM DMA source through mapper**: Current code reads `cpu.mem[addr]`.
Real hardware reads through the bus. If source page is $20-$3F, reads hit
PPU registers. If source page is $40, reads hit APU/I/O. This is likely
what the DMA+register tests rely on.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.