From 6e6a0b3ed86c32ab51a4da1cf97d57a57dad7887 Mon Sep 17 00:00:00 2001 From: Matthew Stanley <1379tech@gmail.com> Date: Wed, 5 Aug 2026 16:21:08 -0700 Subject: [PATCH 1/4] spu: implement reverb, noise, sweeps and SPU IRQ; make CAUSE.IP2 combinational MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the four gaps named in issue #103 and lands the separable, verified parts of PR #102. See docs/internal/SPU_FIDELITY_103.md for the per-piece disposition and the oracle-verification queue. SPU DSP (issue #103), clean-room from hardware documentation: - Reverb: the documented 22050 Hz engine — vLIN/vRIN input, same-side and crossed different-side IIR reflections, 4-tap comb early echo, both all-pass filters, vLOUT/vROUT output, and the work-area address advance. Gated on SPUCNT bit 7 for WRITES only; reads, IRQ checks, address advance and output always run, as on hardware. Addressing is confined to [mBASE, 0x80000). Per-voice sends from EON, CD send from SPUCNT bits 0+2. - Noise: 16-bit LFSR with the documented parity feedback, clocked at the SPUCNT bits 8-13 rate; NON voices substitute it for their ADPCM sample. - Volume sweeps: all 24 voice L/R plus main L/R. Bit 15 selects sweep vs direct; sweep is a live envelope on the same rate machinery as ADSR, and a guest read of a sweeping register returns the live level. - Capture buffers: CD L/R and voice 1/3 output written to SPU RAM, so a game that parks the IRQ address there still gets its interrupt. - SPU IRQ (I_STAT bit 9): checked at every SPU RAM access class — FIFO, DMA both directions, ADPCM block fetch, capture writes, reverb work-area accesses — gated on SPUCNT bit 6, mirrored in SPUSTAT bit 6, acked by clearing SPUCNT bit 6. The CD-only fast path in spu_render was removed rather than extended: the SPU now has per-frame work that must run in every path, and an FMV (XA audio with zero active voices) is exactly the CD-with-reverb case. This is a clean-room implementation from the psx-spx/nocash register map and documented algorithm. Beetle's source was deliberately not consulted: it is GPL-2.0-or-later and this project is PolyForm Noncommercial. PR #16 was parked for exactly that reason and PR #13's reverb was separately wrong (it gated on bit 15 of dAPF1, an address offset, instead of SPUCNT bit 7). The one place this knowingly deviates from hardware is the 22.05 -> 44.1 kHz reconstruction filter, which the documentation does not specify; it is isolated in a single function and flagged for oracle comparison. CAUSE.IP2 (from PR #102): IP2 is combinational on R3000A — it mirrors the interrupt line and falls the instant the guest acks I_STAT or masks the source. It was being OR'd in at delivery and never cleared, in TWO places (the compiled path and psx_interpreter.c, the latter missed by PR #102), leaving a phantom pending interrupt that can spin a kernel dispatcher looping on CAUSE.IP & SR.IM. psx_irq_refresh_cause_ip2() is now the sole writer of bit 10, refreshed at raise, I_STAT ack, I_MASK write, HLE context restore and power-on. Verified against the Beetle oracle's irq.cpp/cpu.cpp rather than asserted, and covered by test_cause_ip2_combinational. Mid-dispatch audio pump (from PR #102), with a fix: routed through a gate that mirrors sdl_audio_update's mute/sink decision. Pumping unconditionally from the VBlank edge, as submitted, would have pushed real audio during every turbo-load hard mute and defeated the freeze-in-place mute model. SPU RAM DMA readback (from PR #102): DMA4 in the SPU->RAM direction wrote literal zeros; SPU RAM is readable memory and titles carry state through it across an Exec boundary. Also fixes an unrelated build race found on the way: tools/embed_spirv.py wrote its intermediate SPIR-V to .spv, so the two runtime targets that embed the same shaders raced on one path and one deleted the file the other was reading. Co-Authored-By: Alexandros Mandravillis Co-Authored-By: Claude Opus 5 (1M context) --- docs/internal/SPU_FIDELITY_103.md | 251 ++++++ runtime/CMakeLists.txt | 15 + runtime/include/audio_trace.h | 1 + runtime/include/spu.h | 29 +- runtime/src/debug_server.c | 58 +- runtime/src/dma.c | 11 +- runtime/src/interrupts.c | 101 ++- runtime/src/main.cpp | 57 ++ runtime/src/memory.c | 7 + runtime/src/psx_interpreter.c | 9 +- runtime/src/spu.c | 886 ++++++++++++++++--- runtime/src/traps.c | 8 + runtime/tests/test_cause_ip2_combinational.c | 185 ++++ runtime/tests/test_spu_fidelity.c | 584 ++++++++++++ 14 files changed, 2062 insertions(+), 140 deletions(-) create mode 100644 docs/internal/SPU_FIDELITY_103.md create mode 100644 runtime/tests/test_cause_ip2_combinational.c create mode 100644 runtime/tests/test_spu_fidelity.c diff --git a/docs/internal/SPU_FIDELITY_103.md b/docs/internal/SPU_FIDELITY_103.md new file mode 100644 index 000000000..c596e3897 --- /dev/null +++ b/docs/internal/SPU_FIDELITY_103.md @@ -0,0 +1,251 @@ +# SPU fidelity burndown — Issue #103 and PR #102 + +Branch: `feat/spu-fidelity-103` · Worktree: `_wt-spu-103` · Base: `origin/master` `7f7fbc8e` + +Closes the four gaps named in Issue #103 (reverb, noise, volume sweeps, SPU IRQ) +and lands the separable, verified parts of PR #102. Written so the disposition of +each piece is auditable rather than asserted. + +--- + +## 1. Scope overlap between #102 and #103 + +They intersect on exactly one item. + +| Item | Issue #103 | PR #102 | Disposition | +| --- | --- | --- | --- | +| Reverb | ✔ asked | — | Implemented here, clean-room (§3) | +| Noise LFSR | ✔ asked | — | Implemented here, clean-room (§3) | +| Volume sweeps | ✔ asked | — | Implemented here, clean-room (§3) | +| SPU IRQ | ✔ asked | ✔ offered | **One implementation**, ours, in `spu.c` | +| CAUSE.IP2 combinational | — | ✔ offered | Taken, verified, extended (§2) | +| Mid-dispatch audio pump | — | ✔ offered | Taken, with a regression fixed (§2) | +| SPU RAM DMA readback | — | ✔ offered | Taken as-is in substance (§2) | + +So this is one coherent change, not two competing ones. PR #102 covers 1 of +#103's 4 items; the other three were never in it. + +## 2. PR #102 — per-piece verdict + +PR #102's four changes were reviewed as four separable items, because they carry +very different risk. Attribution to Alexandros Mandravillis is recorded in the +commit trailer and in the source comments; the code here is a reimplementation +on the same findings, because two of the PR's three commits touch `spu.c` and +would have collided with the clean-room work. + +### 2a. CAUSE.IP2 combinational — CONFIRMED, taken, and extended + +The PR's claim is that `CAUSE.IP2` is a live mirror of the interrupt line rather +than a latched bit. **The claim is correct**, and it was checked against the +in-tree Beetle oracle rather than accepted on the PR's reasoning: + +- `beetle-psx/mednafen/psx/irq.cpp` defines + `#define Recalc() PSX_CPU->AssertIRQ(0, (bool)(Status & Mask))` + and calls it from `IRQ_Assert` (raise), from **both** halves of `IRQ_Write` + (the Status ack and the Mask write), and from `IRQ_Power`. +- `beetle-psx/mednafen/psx/cpu.cpp:289 AssertIRQ()` clears `CAUSE` bit `10+n` + unconditionally, then re-sets it only if the level is asserted. + +That is the definition of a combinational mirror, and it matches R3000A: the +`Cause.IP` field is not storage, it reflects the interrupt pins. Our runtime +previously only ever OR'd bit 10 in at delivery and never cleared it. + +Extensions beyond the PR: + +1. **A second latching writer the PR missed.** `runtime/src/psx_interpreter.c` + had its own `cpu->cop0[COP0_CAUSE] |= (1u << 10);` at interrupt delivery — + the identical defect in the standalone interpreter path. Fixed too. An audit + of every writer of `cop0[13]` confirms no other site touches bit 10: the + interpreter's `MTC0` paths correctly mask guest writes to `0x0300` + (software-interrupt bits only), and the exception paths mask `~0x7C` / + `~0x8000007C`, preserving the IP field. +2. **Single ownership.** The compiled delivery path no longer sets bit 10 + itself; `psx_irq_refresh_cause_ip2()` is the only writer. Two writers of one + combinational bit is how the bug survived in the first place. +3. **Power-on recompute**, mirroring Beetle's `IRQ_Power() -> Recalc()`. +4. **A regression test**, `runtime/tests/test_cause_ip2_combinational.c`, + covering rise, fall on ack, fall on mask, partial ack with another source + still pending, partial-width register writes, preservation of ExcCode/BD/IP0/ + IP1, and out-of-range I_STAT bits. Registered in CTest. + +Risk note: this is a real behavioural change to core IRQ delivery for every +title, and it is the highest-risk item in this branch. It is nonetheless the +faithful behaviour, so per Rule -1 it lands and titles are revalidated rather +than the fix being narrowed. + +### 2b. Mid-dispatch audio pump — taken, with a regression the PR would have shipped + +The reasoning is sound: the SPU is autonomous on hardware, our pump is driven +from the main loop between presented frames, so a guest busy-wait that never +presents a frame freezes SPU time — self-deadlocking for any game waiting on an +SPU-generated condition. The VBlank edge is derived from the guest cycle +counter, so it keeps firing through such a wait and is a defensible place to +pump. Confirmed single-threaded: `spu_render` runs on the main loop thread +(`sdl_audio_pump` is the producer; the SDL callback only drains an SPSC ring), +so this introduces no data race on `i_stat`/`cop0`. + +**Defect found and fixed.** PR #102 wires the hook to `sdl_audio_pump()` +unconditionally. But `sdl_audio_update()` is the sole authority on whether a +pump should emit, discard, or not run: + +- hard mute (turbo loads) → *does not pump at all*; the queue drains and voice + positions freeze in place, so music resumes where it left off instead of + replaying time-compressed garble. This is user-validated behaviour. +- turbo sink → pumps with `discard_output = true`. + +Pumping unconditionally from the VBlank edge bypasses both, which would push +real audio during every turbo load. The hook now goes through +`sdl_audio_pump_midframe()`, which mirrors the gate state +(`AUDIO_GATE_NORMAL` / `MUTED` / `SINK`) that `sdl_audio_update()` last set. + +Known tension, recorded rather than guessed at: in the `MUTED` state SPU time +still freezes, so a game that busy-waits on an SPU condition *during* a turbo +load would stall. No title in our suite is known to do this. Advancing SPU time +under mute would change validated mute audio, so it is not being done +speculatively. + +Placement: the pump call sits at the **end** of `fire_vblank_edge()` so the +VBlank's own raise and ring records complete first — the pump can itself raise +an SPU IRQ, which should land after the edge rather than interleaved into it. + +### 2c. SPU RAM DMA readback — taken + +DMA4 in the SPU→RAM direction previously wrote literal zeros. SPU RAM is +readable memory; this was not a transfer at all. Titles that carry a checksummed +block through SPU RAM across an `Exec` boundary (SPU RAM being one of the few +regions a main-RAM reload does not touch) read zeros, failed their own integrity +check, and fell back to a cold-boot path. Lowest-risk item in the PR: +previously-dead path, no existing behaviour depends on the zeros. + +### 2d. Validation claims — NOT reproduced, and cannot be + +PR #102 validates against Medal of Honor Underground (SLUS-01270) and Gran +Turismo (SCUS-94194). Neither disc is in our suite. Those specific claims are +**unverified by us and should not be repeated as if they were**. What we +verified independently is the hardware behaviour behind each change (§2a–2c) and +non-regression across our own titles. + +## 3. Issue #103 — why this had to be clean-room + +⚠ **This is the load-bearing constraint on the whole item, and it is why #103 +sat open.** + +`psxrecomp` is distributed under **PolyForm Noncommercial 1.0.0**. Beetle PSX / +mednafen is **GPL-2.0-or-later**. They are incompatible: GPL source cannot be +folded into a PolyForm distribution. Beetle's tree contains a complete, correct, +well-tested reverb — so the obvious implementation route is closed, and two +contributors independently walked into it: + +- **PR #16** (Martin Penkava) implemented all of this by porting Beetle. Parked + on license grounds after a line-by-line audit found it tracked + `beetle-psx/mednafen/psx/spu.cpp` lines 590–730 in register layout, resampling + coefficients, buffer layout, arithmetic and processing order. See + `docs/internal/upstream/martin-pr16-spu-reverb.md` on branch + `audit/pr16-spu-reverb-gpl-lineage-mpenkava`. +- **PR #13** (parked at `5fc8e15d`) shipped a reverb too, rejected for being + *wrong* rather than for licensing: it gated reverb on bit 15 of `0x1F801DC0`, + which is `dAPF1` — an all-pass *offset* register whose top bit is set for any + offset ≥ `0x8000`, so reverb switched on and off according to an address + value. The real gate is SPUCNT (`0x1F801DAA`) bit 7. It also read + `dAPF2` / `vIIR` / `vCOMB1` as "feedback" / "wet level" / "early-reflection + level"; they are none of those. + +The PR #16 audit names the only acceptable path, and this branch takes it: **a +clean-room implementation from hardware documentation, with independently +derived tests.** The psx-spx / nocash register map and documented algorithm were +the source. Beetle's source was deliberately not consulted for the +implementation. + +Beetle remains fully usable as a **runtime oracle** — running the binary and +comparing its audio output is not a derivative work, and that comparison is how +the remaining uncertainties in §5 get settled. `audio_wav` and `audio_stats` +exist on both `psx-runtime` and `psx-beetle`, so the comparison is a symmetric +always-on ring query, not an arm-then-capture. + +### Pre-existing lineage exposure — reported, not changed + +Separate from this work, and worth a decision: + +- `runtime/src/spu.c` `calc_vc_delta()` is commented *"Ported verbatim from + Beetle's CalcVCDelta"*. This is the ADSR rate decoder, and it is in every + released binary. Same category as PR #16, already shipped. +- `runtime/include/spu_gauss.h` cites *"No$PSX docs / DuckStation + core/spu.cpp"*. + +Deliberately left alone here: rewriting the ADSR rate decoder clean-room changes +every voice envelope in every title and needs its own revalidation campaign, not +a rider on this branch. + +## 4. `feat/pr13-salvage-remainder` is fully superseded — no surgery needed + +Prior notes flagged this branch as "ready to land" while Issue #103 says its +reverb must stay parked, and treated reconciling that as a blocker. **It is not +a blocker: the branch retains nothing but the rejected reverb commit.** + +`git cherry -v origin/master feat/pr13-salvage-remainder`: + +``` +- e9a8a24e interp: deliver alignment exceptions, protect Cause read-only bits, add CFC0 +- 3ce30155 dma: implement channel 5 (PIO / expansion port) transfers ++ 5fc8e15d spu: reverb, noise LFSR, sweep volumes, pitch modulation, SPU IRQ +- 9f3cd0a3 autocompile: name the resolved interpreter at configure, warn on Cygwin builds +``` + +Three of four are already upstream (`-`). Verified four independent ways rather +than trusting patch-id alone: + +- cherry-picking all three onto current `origin/master` produced **empty** + commits; +- `execute_ch5_pio()` is present in `runtime/src/dma.c` on master; +- `CFC0` handling and the `& ~0x0300u | (val & 0x0300u)` Cause write-protection + are present in `runtime/src/dirty_ram_interp.c` on master; +- `git diff origin/master 9f3cd0a3 -- runtime/src/autocompile.c` is **empty**. + +Consequences: there is no salvage left to preserve, no git surgery to perform, +and the "two competing SPU IRQ implementations" question resolves by default — +`5fc8e15d` is both superseded and Beetle-derived. The branch should be +abandoned and PR #13 closed. `origin/park/pr13-spu-reverb-rejected` still +retains the commit, so nothing is lost. + +Incidentally, `e9a8a24e`'s Cause write-protection (guest `MTC0` may only write +bits 8–9) is exactly what makes §2a safe: the guest cannot forge IP2. + +## 5. Known deviations and the oracle-verification queue + +Recorded explicitly rather than presented as exact (Rule 14). + +- **22.05 → 44.1 kHz reverb reconstruction filter.** The reverb engine runs at + 22050 Hz, but the hardware's reconstruction filter is not specified in the + documentation used. Beetle's 39-tap FIR was deliberately not copied. The + implementation uses a clearly-marked, single-function reconstruction so the + filter can be swapped once measured against the oracle. **This is the one + place the implementation is known to differ from hardware.** +- Further per-feature judgement calls are listed in §6 as reported by the + implementation pass; each is a candidate for oracle comparison. + +## 6. Implementation judgement calls + +_Populated from the implementation pass; each entry is an oracle-comparison +candidate rather than a settled fact._ + +## 7. Validation status + +- [x] `test_cause_ip2_combinational` passes (`-Wall -Wextra -Werror`). +- [x] Changed C translation units compile clean. +- [ ] `test_spu_fidelity` passes. +- [ ] `psx-runtime` links; BIOS boots; boot chime audible and non-distorted. +- [ ] Title run: no regression, reverb present. +- [ ] Oracle audio comparison against `psx-beetle` at a fixed scene. +- [ ] User ear-validation (final gate). + +## 8. Incidental fix — `tools/embed_spirv.py` build race + +Not SPU-related, found while building. Two runtime targets (`psx-runtime`, +`psx-oracle`) embed the same shader set, so ninja runs `embed_spirv.py` +concurrently. It wrote its intermediate SPIR-V to `.spv` **in the source +tree**, so the two invocations raced on one path and whichever finished first +`os.remove()`d the file the other was about to read — a hard build failure +(`FileNotFoundError: .../blit.frag.spv`), and source-tree pollution besides. +Now compiled to a private `mkdtemp` directory, with the output header written +via atomic replace and a loud error if `glslc` reports success but produces no +file. Fixed per Rule 15 rather than worked around. diff --git a/runtime/CMakeLists.txt b/runtime/CMakeLists.txt index eb1e6d755..5d86dff15 100644 --- a/runtime/CMakeLists.txt +++ b/runtime/CMakeLists.txt @@ -24,6 +24,12 @@ if(BUILD_TESTING) add_test(NAME launcher_device_roundtrip_test COMMAND launcher_device_roundtrip_test) + # COP0.CAUSE IP2 must mirror the INTC line combinationally, never latch. + add_executable(cause_ip2_combinational_test + tests/test_cause_ip2_combinational.c) + add_test(NAME cause_ip2_combinational_test + COMMAND cause_ip2_combinational_test) + add_executable(psx_cyc_batch_test tests/test_psx_cyc_batch.c) target_include_directories(psx_cyc_batch_test PRIVATE include) target_compile_definitions(psx_cyc_batch_test PRIVATE @@ -82,6 +88,15 @@ if(BUILD_TESTING) add_test(NAME mod_gpu_dma_aperture_test COMMAND mod_gpu_dma_aperture_test) + # Clean-room SPU DSP fidelity (issue #103): SPU IRQ on every RAM access + # class, reverb (write gating + work-area wrap + CD-only path), noise + # generator, volume sweeps / direct decode, capture buffers, and + # savestate coverage of all the new state. Includes src/spu.c directly + # (white-box) and stubs its externs. + add_executable(spu_fidelity_test tests/test_spu_fidelity.c) + target_include_directories(spu_fidelity_test PRIVATE include) + add_test(NAME spu_fidelity_test COMMAND spu_fidelity_test) + add_executable(overlay_capture_retry_test tests/test_overlay_capture_retry.cpp src/overlay_capture.c) diff --git a/runtime/include/audio_trace.h b/runtime/include/audio_trace.h index d8cab71dd..6d55b102c 100644 --- a/runtime/include/audio_trace.h +++ b/runtime/include/audio_trace.h @@ -60,6 +60,7 @@ enum { stage 0=post-ADPCM-decode (native rate), stage 1=post-resample+volume (44100). */ AUDIO_EV_SINK_DROP = 10, /* turbo host sink; a=guest SPU frames discarded */ + AUDIO_EV_DMA_READ = 11, /* SPU RAM -> CPU DMA; a=words, b=dest RAM addr */ }; typedef struct { diff --git a/runtime/include/spu.h b/runtime/include/spu.h index b652c816b..d29a81d7a 100644 --- a/runtime/include/spu.h +++ b/runtime/include/spu.h @@ -49,6 +49,8 @@ typedef struct SpuVoiceState { uint16_t phase; /* sub-sample phase counter (0..0x1000) */ uint16_t env_level; /* live ADSR envelope level (0..0x7FFF) */ uint8_t adsr_phase; /* 0=attack 1=decay 2=sustain 3=release */ + int16_t vol_cur_l; /* live effective L volume (sweep-aware, full int16) */ + int16_t vol_cur_r; /* live effective R volume (sweep-aware, full int16) */ } SpuVoiceState; typedef struct SpuGlobalState { @@ -62,6 +64,19 @@ typedef struct SpuGlobalState { uint32_t eon; /* reverb mode (0x1F801D98/9A) */ uint32_t endx; /* end-block reached latch (0x1F801D9C/9E) */ uint32_t active_mask; /* recomp-local "still voicing" mask */ + + /* ---- SPU DSP fidelity state (issue #103: IRQ/reverb/noise/sweeps) ---- */ + uint16_t irq_flag; /* SPU IRQ latch (mirrors SPUSTAT bit 6) */ + uint16_t reverb_on; /* SPUCNT bit 7 (reverb work-area write enable) */ + uint32_t irq_addr; /* IRQ address as a BYTE address (reg 0x1F801DA4 << 3) */ + uint32_t reverb_mbase; /* reverb work area start, byte address (mBASE << 3) */ + uint32_t reverb_cur; /* current reverb buffer address (byte, advances @22050Hz) */ + uint32_t capture_pos; /* capture-buffer write offset (0..0x3FE, wraps at 0x400) */ + uint16_t noise_lfsr; /* live noise shift register value */ + uint16_t noise_pad; /* reserved / alignment */ + uint32_t sweep_l_mask; /* voices whose LEFT volume register is in sweep mode */ + uint32_t sweep_r_mask; /* voices whose RIGHT volume register is in sweep mode */ + uint32_t sweep_main; /* bit0 = main L in sweep mode, bit1 = main R */ } SpuGlobalState; void spu_get_voice_state(int voice, SpuVoiceState* out); @@ -76,7 +91,10 @@ typedef enum { SPU_EV_KEYON = 1, SPU_EV_KEYOFF = 2, SPU_EV_END_STOP = 3, /* loop_end without repeat → voice silenced */ - SPU_EV_END_LOOP = 4 /* loop_end with repeat → cur_addr=repeat_addr */ + SPU_EV_END_LOOP = 4, /* loop_end with repeat → cur_addr=repeat_addr */ + SPU_EV_IRQ = 5 /* SPU RAM IRQ-address hit → I_STAT bit 9 raised; + voice=0xFF (not voice-attributable), addr=byte + address that matched */ } SpuEventKind; typedef struct SpuEvent { @@ -100,8 +118,11 @@ void spu_event_reset(void); uint32_t spu_read(uint32_t addr); void spu_write(uint32_t addr, uint32_t value); -/* DMA channel 4 interface */ +/* DMA channel 4 interface. spu_dma_read reads one 32-bit word from SPU RAM + * at the current transfer address, advances the address by 4, and runs the + * SPU IRQ-address check (SPU RAM -> CPU direction, DICR direction bit 0). */ void spu_dma_write(uint32_t word); +uint32_t spu_dma_read(void); int spu_dma_ready(void); /* CD-ROM XA/CDDA input path. Samples are stereo 44.1 kHz PCM entering the @@ -126,8 +147,8 @@ typedef struct { int16_t s[4]; /* decoded samples sample_idx-1 .. +2 (block-edge clamped) */ float frac; /* fractional sub-sample phase in [0,1) */ uint16_t env; /* env_level (0..0x7FFF) */ - int16_t vol_l; /* per-voice L volume (direct_volume-decoded) */ - int16_t vol_r; /* per-voice R volume */ + int16_t vol_l; /* per-voice L volume, 1.14 scale (effective int16 >> 1) */ + int16_t vol_r; /* per-voice R volume, same scale */ uint8_t active; } SpuShadowVoiceTapPub; diff --git a/runtime/src/debug_server.c b/runtime/src/debug_server.c index 1c7b0d503..b7da9b06f 100644 --- a/runtime/src/debug_server.c +++ b/runtime/src/debug_server.c @@ -6047,6 +6047,13 @@ static void handle_spu_status(int id, const char *json) (void)json; SpuDebugInfo info; spu_debug_info(&info); + /* The DSP-fidelity state (issue #103: SPU IRQ, reverb, noise, sweeps) lives + * in SpuGlobalState. Surfaced here so the whole SPU can be judged from one + * always-on query — without it there is no way to tell whether the reverb + * engine is actually stepping, whether the IRQ is armed, or which volume + * registers are sweeping. */ + SpuGlobalState g; + spu_get_global_state(&g); send_fmt("{\"id\":%d,\"ok\":true," "\"ctrl\":\"0x%04X\",\"active_mask\":\"0x%06X\"," "\"main_l\":%d,\"main_r\":%d," @@ -6055,7 +6062,15 @@ static void handle_spu_status(int id, const char *json) "\"render_frames\":%llu,\"nonzero_frames\":%llu," "\"last_peak\":%d,\"peak\":%d," "\"cd_frames\":%u,\"cd_push_frames\":%llu," - "\"cd_overflow_frames\":%llu,\"cd_underflow_frames\":%llu}", + "\"cd_overflow_frames\":%llu,\"cd_underflow_frames\":%llu," + "\"pmon\":\"0x%06X\",\"non\":\"0x%06X\",\"eon\":\"0x%06X\"," + "\"endx\":\"0x%06X\"," + "\"irq_flag\":%u,\"irq_addr\":\"0x%05X\"," + "\"reverb_on\":%u,\"reverb_mbase\":\"0x%05X\"," + "\"reverb_cur\":\"0x%05X\",\"capture_pos\":\"0x%03X\"," + "\"noise_lfsr\":\"0x%04X\"," + "\"sweep_l_mask\":\"0x%06X\",\"sweep_r_mask\":\"0x%06X\"," + "\"sweep_main\":%u}", id, info.ctrl & 0xFFFFu, info.active_mask & 0xFFFFFFu, @@ -6071,7 +6086,21 @@ static void handle_spu_status(int id, const char *json) info.cd_frames, (unsigned long long)info.cd_push_frames, (unsigned long long)info.cd_overflow_frames, - (unsigned long long)info.cd_underflow_frames); + (unsigned long long)info.cd_underflow_frames, + g.pmon & 0xFFFFFFu, + g.non & 0xFFFFFFu, + g.eon & 0xFFFFFFu, + g.endx & 0xFFFFFFu, + (unsigned)g.irq_flag, + g.irq_addr & 0xFFFFFu, + (unsigned)g.reverb_on, + g.reverb_mbase & 0xFFFFFu, + g.reverb_cur & 0xFFFFFu, + g.capture_pos & 0xFFFu, + (unsigned)g.noise_lfsr, + g.sweep_l_mask & 0xFFFFFFu, + g.sweep_r_mask & 0xFFFFFFu, + (unsigned)g.sweep_main); } /* ---- Per-voice SPU snapshot. Mirrors fields the Beetle oracle exposes @@ -6087,7 +6116,9 @@ static void handle_spu_voices(int id, const char *json) SpuGlobalState g; spu_get_global_state(&g); - size_t cap = 8192; + /* 24 voices x ~280 chars + header. Headroom matters: snprintf would silently + * truncate mid-object and hand the caller unparseable JSON. */ + size_t cap = 16384; char *out = (char *)malloc(cap); if (!out) { send_fmt("{\"id\":%d,\"ok\":false,\"err\":\"alloc\"}", id); return; } size_t off = 0; @@ -6116,7 +6147,11 @@ static void handle_spu_voices(int id, const char *json) "\"adsr_lo\":\"0x%04X\",\"adsr_hi\":\"0x%04X\"," "\"cur_addr\":\"0x%05X\",\"repeat_addr\":\"0x%05X\"," "\"flags\":\"0x%02X\",\"sample_idx\":%d,\"phase\":\"0x%04X\"," - "\"env\":\"0x%04X\",\"env_phase\":%d}", + "\"env\":\"0x%04X\",\"env_phase\":%d," + /* Live effective volumes. For a sweeping register (bit 15 set) the + * vol_l/vol_r control words above say nothing about the current + * level, so these are the only way to see a sweep actually glide. */ + "\"vol_cur_l\":%d,\"vol_cur_r\":%d}", v == 0 ? "" : ",", v, s.active, s.vol_ctrl_l, s.vol_ctrl_r, @@ -6126,7 +6161,8 @@ static void handle_spu_voices(int id, const char *json) s.adsr_lo, s.adsr_hi, s.cur_addr, s.repeat_addr, s.last_flags, s.sample_idx, s.phase, - s.env_level, s.adsr_phase); + s.env_level, s.adsr_phase, + s.vol_cur_l, s.vol_cur_r); if (n > 0) off += (size_t)n; } n = snprintf(out + off, cap - off, "]}"); @@ -6155,7 +6191,7 @@ static void handle_spu_ram(int id, const char *json) } /* ---- SPU event ring dump. Returns the most recent N events - * (KEYON / KEYOFF / END_STOP / END_LOOP) with frame timestamps. */ + * (KEYON / KEYOFF / END_STOP / END_LOOP / IRQ) with frame timestamps. */ static void handle_spu_events(int id, const char *json) { int count = json_get_int(json, "count", 256); @@ -6165,7 +6201,12 @@ static void handle_spu_events(int id, const char *json) if (!evs) { send_fmt("{\"id\":%d,\"ok\":false,\"err\":\"alloc\"}", id); return; } uint32_t got = spu_event_get(evs, (uint32_t)count); uint64_t total = spu_event_total(); - static const char *kind_names[5] = { "?", "KEYON", "KEYOFF", "END_STOP", "END_LOOP" }; + /* Index by SpuEventKind (spu.h). IRQ (=5) is not voice-attributable; the + * ring stores voice=0xFF for it and `addr` is the byte address that matched + * the programmed IRQ address. Keep this table in step with SpuEventKind or + * a new kind renders as "?". */ + static const char *kind_names[6] = { "?", "KEYON", "KEYOFF", "END_STOP", + "END_LOOP", "IRQ" }; /* Worst case ~200 chars per event; 64 KB is plenty for 4096 events. */ size_t cap = 256u + (size_t)got * 256u; @@ -6178,7 +6219,8 @@ static void handle_spu_events(int id, const char *json) if (n > 0) off += (size_t)n; for (uint32_t i = 0; i < got; i++) { const SpuEvent *e = &evs[i]; - const char *kn = (e->kind <= 4) ? kind_names[e->kind] : "?"; + const char *kn = (e->kind < sizeof(kind_names) / sizeof(kind_names[0])) + ? kind_names[e->kind] : "?"; n = snprintf(out + off, cap - off, "%s{\"seq\":%llu,\"frame\":%u,\"kind\":\"%s\",\"v\":%d," "\"pitch\":\"0x%04X\",\"addr\":\"0x%05X\"," diff --git a/runtime/src/dma.c b/runtime/src/dma.c index 92364771c..36bdf5475 100644 --- a/runtime/src/dma.c +++ b/runtime/src/dma.c @@ -798,10 +798,19 @@ static uint32_t execute_ch4_spu(void) { audio_trace_event(AUDIO_EV_DMA_WRITE, total_words, channels[4].madr & 0x1FFFFCu); } else { + /* SPU RAM -> CPU RAM. This direction previously zero-filled the + * destination, which is not a transfer at all: SPU RAM is readable + * memory and games do read it back. Titles that carry state through + * SPU RAM across an Exec boundary (a checksummed block surviving an + * EXE swap, since SPU RAM is one of the few regions main RAM's reload + * does not touch) got zeros, failed their own integrity check, and + * fell back to a cold-boot path. */ for (uint32_t i = 0; i < total_words; i++) { - psx_write_word(addr, 0); + psx_write_word(addr, spu_dma_read()); addr = (addr + addr_step) & 0x1FFFFCu; } + audio_trace_event(AUDIO_EV_DMA_READ, total_words, + channels[4].madr & 0x1FFFFCu); } channels[4].madr = addr; diff --git a/runtime/src/interrupts.c b/runtime/src/interrupts.c index c267a87da..68253ccba 100644 --- a/runtime/src/interrupts.c +++ b/runtime/src/interrupts.c @@ -115,13 +115,61 @@ extern uint32_t i_mask; * recorded here with its guest cycle. */ #include "device_trace.h" +/* ---- CAUSE.IP2 is COMBINATIONAL, not latched --------------------------- + * + * On R3000A the Cause.IP field is not storage: it reflects the current state + * of the interrupt input pins. On the PSX only IP2 (bit 10) is wired, and it + * carries the interrupt controller's output line, i.e. (I_STAT & I_MASK) != 0. + * It therefore RISES when a device raises and FALLS the instant the guest acks + * I_STAT or masks the source — with no CPU involvement either way. + * + * This runtime previously only ever OR'd bit 10 in at delivery and never + * cleared it, leaving a phantom IP2 in COP0.CAUSE. A kernel exception + * dispatcher that loops on CAUSE.IP & SR.IM to decide whether to service + * again sees a pending interrupt that no longer exists and can spin in its + * event scan forever. + * + * Verified against the independent Beetle oracle rather than asserted: + * beetle-psx/mednafen/psx/irq.cpp defines + * #define Recalc() PSX_CPU->AssertIRQ(0, (bool)(Status & Mask)) + * and calls it from IRQ_Assert (raise), from IRQ_Write for BOTH the Status ack + * and the Mask write, and at power-on; cpu.cpp's AssertIRQ clears bit (10+n) + * unconditionally and re-sets it only when the level is asserted. So the line + * is recomputed at every point (I_STAT & I_MASK) can change, which is exactly + * the set of call sites below. + * + * Ownership: this function is the ONLY writer of CAUSE bit 10. The delivery + * path no longer ORs it in separately — one owner, no divergence. + * + * Derived from PR #102 by Alexandros Mandravillis; the mirror call sites and + * the single-owner refactor are ours. */ +static uint32_t *s_cause_ptr; + +void psx_irq_refresh_cause_ip2(void) +{ + if (!s_cause_ptr) return; + if ((i_stat & i_mask & 0x7FFu) != 0u) + *s_cause_ptr |= (1u << 10); + else + *s_cause_ptr &= ~(1u << 10); +} + +void psx_irq_set_cause_ptr(uint32_t *p) +{ + s_cause_ptr = p; + /* Power-on recompute, mirroring Beetle's IRQ_Power() -> Recalc(). Without + * this the first mirror only happens at the first raise/ack, so a CAUSE + * read before any interrupt activity would show a stale bit. */ + psx_irq_refresh_cause_ip2(); +} + /* Central IRQ-raise choke point. All device sources call this to set their * I_STAT bit so the device-event ring sees every raise from one place with the - * exact guest cycle. Pure addition over `i_stat |= (1<cop0[COP0_CAUSE] = (cpu->cop0[COP0_CAUSE] & ~0x7C) | (0 << 2); - if (hw_deliverable) - cpu->cop0[COP0_CAUSE] |= (1 << 10); + psx_irq_refresh_cause_ip2(); /* Push SR exception stack: shift bits [5:0] left by 2. */ cpu->cop0[COP0_SR] = (sr & ~0x3F) | ((sr & 0x0F) << 2); diff --git a/runtime/src/main.cpp b/runtime/src/main.cpp index 122791c98..041fc8649 100644 --- a/runtime/src/main.cpp +++ b/runtime/src/main.cpp @@ -153,6 +153,9 @@ extern "C" { /* memory.c */ extern "C" void memory_init(const char* bios_path); extern "C" void memory_set_sr_ptr(const uint32_t *p); +/* interrupts.c */ +extern "C" void psx_irq_set_cause_ptr(uint32_t *p); +extern "C" void psx_set_midframe_audio_pump(void (*fn)(void)); extern "C" uint32_t memory_get_bios_checksum(void); extern "C" void dirty_ram_register_text_image(uint32_t phys_lo, const uint8_t *bytes, @@ -2136,6 +2139,43 @@ static void runtime_perf_diag_tick() { for (int i = 0; i < 6; i++) last_up[i] = up[i]; } +/* Audio gate state, shared with the mid-frame (VBlank-edge) pump. + * + * sdl_audio_update() below is the sole authority on whether a pump should emit + * audio, discard it, or not run at all. The mid-frame pump exists to keep SPU + * time flowing across guest busy-waits that never present a frame, but it must + * NOT bypass that authority: pumping unconditionally would push real audio + * during a turbo-load hard mute, defeating the mute model (the queue is + * supposed to drain and voice positions freeze in place, so music resumes where + * it left off rather than replaying time-compressed garble), and would emit to + * the device during the discard-only turbo sink. + * + * So the mid-frame pump mirrors whatever the last frame decided. */ +enum AudioGate { AUDIO_GATE_NORMAL = 0, AUDIO_GATE_MUTED = 1, AUDIO_GATE_SINK = 2 }; +static AudioGate s_audio_gate = AUDIO_GATE_NORMAL; + +/* Invoked from the guest-derived VBlank edge (interrupts.c). */ +static void sdl_audio_pump_midframe(void) { + if (!sdl_audio_device) return; + switch (s_audio_gate) { + case AUDIO_GATE_MUTED: + /* Deliberately nothing. This preserves the existing, user-validated + * freeze-in-place mute semantics exactly. NOTE a real tension here: the + * SPU is autonomous on hardware and never freezes, so a game that + * busy-waits on an SPU-generated condition *during* a turbo load would + * still stall. No title in our suite is known to do that; recording it + * rather than guessing a fix that would change validated mute audio. */ + return; + case AUDIO_GATE_SINK: + sdl_audio_pump(true); /* advance SPU time, discard output */ + return; + case AUDIO_GATE_NORMAL: + default: + sdl_audio_pump(false); + return; + } +} + static void sdl_audio_update(int hard_mute_active, int turbo_sink_active) { if (!sdl_audio_device) return; { /* Tag audio events with the vblank frame counter. */ @@ -2171,10 +2211,13 @@ static void sdl_audio_update(int hard_mute_active, int turbo_sink_active) { } muted = 1; } + s_audio_gate = AUDIO_GATE_MUTED; hangover = HANGOVER_FRAMES; return; } if (muted) { + /* Still inside the post-mute hangover: the gate stays MUTED so the + * mid-frame pump does not sneak audio out ahead of the unmute ramp. */ if (hangover > 0) { hangover--; return; } muted = 0; sdl_audio_fadein_left = sdl_audio_fade_samples; @@ -2188,6 +2231,7 @@ static void sdl_audio_update(int hard_mute_active, int turbo_sink_active) { audio_trace_event(AUDIO_EV_MUTE, 0, 2); /* b=2: discard-only sink */ } g_turbo_audio_sink_active = 1; + s_audio_gate = AUDIO_GATE_SINK; sdl_audio_pump(true); return; } @@ -2199,6 +2243,7 @@ static void sdl_audio_update(int hard_mute_active, int turbo_sink_active) { (uint32_t)sdl_audio_fadein_left, 2); g_audio_unmute_resync = 1; } + s_audio_gate = AUDIO_GATE_NORMAL; sdl_audio_pump(false); } @@ -6812,6 +6857,12 @@ int main(int argc, char** argv) { g_audio_host_rate = have.freq; audio_trace_set_tap_rate(AUDIO_TAP_HOST, (uint32_t)have.freq); (void)psx_sdl_audio_resume(sdl_audio_device); + /* Also pump from the guest-derived VBlank edge so SPU time keeps + * advancing across guest busy-waits that never present a frame + * (guest-cycle-budgeted; same thread — see interrupts.c). Routed + * through the gated wrapper so it honours the turbo mute/sink + * state rather than bypassing it. */ + psx_set_midframe_audio_pump(sdl_audio_pump_midframe); } } #endif @@ -7148,6 +7199,12 @@ int main(int argc, char** argv) { /* Let memory subsystem see SR for cache-isolation checks. */ memory_set_sr_ptr(&cpu.cop0[12]); + /* Wire the CAUSE.IP2 mirror. IP2 is combinational on real hardware and has + * to track (I_STAT & I_MASK) through raises, acks and mask writes; this + * hands interrupts.c the one location it is allowed to maintain. Also + * performs the power-on recompute. */ + psx_irq_set_cause_ptr(&cpu.cop0[13]); + /* Wire debug server to CPU state for register queries. */ debug_server_set_cpu(&cpu); diff --git a/runtime/src/memory.c b/runtime/src/memory.c index d96d6d744..807f55acd 100644 --- a/runtime/src/memory.c +++ b/runtime/src/memory.c @@ -809,17 +809,24 @@ static void imask_trace_record(uint32_t old_val, uint32_t new_val, uint8_t width * the freeze heartbeat against g_vblank_raise/deliver counts. */ uint64_t g_vblank_ack_count = 0; +/* interrupts.c — CAUSE.IP2 mirrors the INTC line and must be recomputed at + * every point (I_STAT & I_MASK) can change. Both writers below are such a + * point: an ack can drop the line, a mask write can drop or raise it. */ +extern void psx_irq_refresh_cause_ip2(void); + static void interrupt_write_stat_masked(uint32_t val, uint32_t mask) { uint32_t ack_mask = mask & 0x7FFu; uint32_t before = i_stat; i_stat = (i_stat & ~ack_mask) | (i_stat & val & ack_mask); if ((before & 1u) && !(i_stat & 1u)) g_vblank_ack_count++; /* VBLANK bit 1->0 */ + psx_irq_refresh_cause_ip2(); } static void interrupt_write_mask_masked(uint32_t val, uint32_t mask, uint8_t width) { uint32_t old = i_mask; i_mask = ((i_mask & ~mask) | (val & mask)) & 0x7FFu; imask_trace_record(old, i_mask, width); + psx_irq_refresh_cause_ip2(); } /* Getters for debug server */ diff --git a/runtime/src/psx_interpreter.c b/runtime/src/psx_interpreter.c index 72fcf266d..5737e5337 100644 --- a/runtime/src/psx_interpreter.c +++ b/runtime/src/psx_interpreter.c @@ -25,6 +25,7 @@ extern uint32_t i_stat; /* Central IRQ-raise choke point (interrupts.c) — also records the device ring. */ extern void psx_irq_raise(uint32_t bit, uint32_t detail); +extern void psx_irq_refresh_cause_ip2(void); /* interrupts.c — CAUSE.IP2 mirror */ extern uint32_t i_mask; #define COP0_SR 12 @@ -540,7 +541,13 @@ static void interp_check_interrupts(CPUState* cpu) { /* Fire interrupt exception. * Push SR stack, set Cause, set EPC, jump to vector. */ cpu->cop0[COP0_CAUSE] = (cpu->cop0[COP0_CAUSE] & ~0x7Cu) | (0u << 2); /* ExcCode=0 (interrupt) */ - cpu->cop0[COP0_CAUSE] |= (1u << 10); /* IP2 */ + /* IP2 is combinational — it mirrors the INTC line and is owned solely by + * psx_irq_refresh_cause_ip2() (interrupts.c). Latching it here with |= was + * the same defect as the compiled delivery path had: the bit was set at + * delivery and never cleared, so it survived the guest's I_STAT ack as a + * phantom pending interrupt. We reach this point only when + * (i_stat & i_mask) != 0, so the mirror necessarily asserts it. */ + psx_irq_refresh_cause_ip2(); cpu->cop0[COP0_SR] = (sr & ~0x3Fu) | ((sr & 0x0Fu) << 2); cpu->cop0[COP0_EPC] = cpu->pc; diff --git a/runtime/src/spu.c b/runtime/src/spu.c index 841ea18d3..409037ff3 100644 --- a/runtime/src/spu.c +++ b/runtime/src/spu.c @@ -1,10 +1,19 @@ /* - * spu.c - PS1 Sound Processing Unit register and direct ADPCM voice model. + * spu.c - PS1 Sound Processing Unit register and ADPCM voice/DSP model. * - * This is intentionally still a compact hardware model: it accepts SPU - * register reads/writes, DMA4 transfers into 512KB SPU RAM, mixes the - * 24 direct ADPCM voices, and accepts decoded CD/XA audio on the SPU CD - * input bus. Reverb, noise, sweep volumes, and IRQ timing are not modeled yet. + * A compact hardware model: SPU register reads/writes, DMA4 transfers in + * both directions over 512KB SPU RAM, the 24 ADPCM voices with pitch, + * Gaussian interpolation and ADSR envelopes, decoded CD/XA audio on the SPU + * CD input bus, the hardware noise generator (NON voices), volume sweep + * envelopes (per-voice and main L/R), the 22050 Hz reverb engine with its + * SPU-RAM work area, the CD/voice capture buffers at 0x000..0xFFF, and the + * SPU RAM IRQ address compare (I_STAT bit 9, SPUSTAT bit 6). + * + * The reverb/noise/sweep/IRQ model is a CLEAN-ROOM implementation from + * hardware documentation only (psx-spx register map + documented algorithm); + * no emulator source was consulted. Points where the documentation is silent + * are flagged inline with "DOCUMENTED-GAP" comments so the Beetle runtime + * oracle comparison can quantify them from OUTPUT, not source. */ #include "spu.h" @@ -40,6 +49,59 @@ static uint32_t endx_latch; static uint32_t kon_latch; static uint32_t koff_latch; +/* ---- SPU IRQ (I_STAT bit 9) ------------------------------------------- */ +/* Register 0x1F801DA4 holds the IRQ address in 8-byte units (value << 3 = + * byte address). While SPUCNT bit 6 is set, ANY SPU RAM access touching + * that 8-byte unit latches irq_flag (mirrored into SPUSTAT bit 6) and + * raises interrupt 9. Writing SPUCNT with bit 6 CLEAR acknowledges the + * latch and re-arms it. The raise is edge-triggered on the latch's 0->1 + * transition: while the latch is set, further hits do not re-raise. */ +extern void psx_irq_raise(uint32_t bit, uint32_t detail); +static uint8_t irq_flag; + +/* ---- Noise generator --------------------------------------------------- + * 16-bit LFSR clocked from SPUCNT bits 8-13 (bits 8-9 = frequency STEP + * 4..7, bits 10-13 = frequency SHIFT 0..15). See noise_clock() for the + * divider derivation. Voices with their NON bit set (0x1F801D94/96) output + * the live LFSR value in place of their interpolated ADPCM sample. */ +static uint16_t noise_lfsr; /* self-seeds from 0 via the xor-1 parity term */ +static int32_t noise_timer; + +/* ---- Reverb engine state ------------------------------------------------ + * The engine runs at 22050 Hz (one step per TWO 44100 Hz output frames). + * rev_cur is the current buffer address (absolute byte address inside the + * work area [mBASE<<3, 0x80000)). The remaining fields carry the 44.1<->22.05 + * kHz boundary state: the held first-of-pair input frame and the last engine + * output used for reconstruction (see rev_reconstruct). */ +static uint32_t rev_cur; +static uint8_t rev_phase; /* 0 = first frame of a 2-frame pair */ +static int32_t rev_in_hold_l; +static int32_t rev_in_hold_r; +static int32_t rev_out_l; /* most recent engine output (post vLOUT/vROUT) */ +static int32_t rev_out_r; + +/* ---- Capture buffers ---------------------------------------------------- + * Real SPU RAM behaviour: every 44100 Hz sample cycle the SPU writes the CD + * input L to 0x000+pos, CD input R to 0x400+pos, voice 1 post-envelope + * output to 0x800+pos and voice 3 post-envelope output to 0xC00+pos; pos + * advances one halfword and wraps at 0x400. Games may park the IRQ address + * inside these rings, so each write runs the IRQ check. */ +static uint32_t capture_pos; + +/* ---- Volume sweep envelopes --------------------------------------------- + * Every volume register (24 voices x L/R + main L/R) is either DIRECT + * (bit15=0: effective volume = signed bits14-0 << 1) or a live SWEEP + * envelope (bit15=1) stepped once per 44100 Hz sample with the same rate + * machinery as ADSR (calc_vc_delta). `level` is the authoritative current + * volume in sweep mode and is refreshed from the register on direct writes, + * so a later switch to sweep mode glides from the last direct level. */ +typedef struct { + int16_t level; /* live effective volume, full signed 16-bit */ + uint32_t divider; /* rate divider, same overflow scheme as ADSR */ +} SweepEnv; +static SweepEnv sweep_voice_env[SPU_VOICE_COUNT][2]; /* [voice][0=L 1=R] */ +static SweepEnv sweep_main_env[2]; /* [0=L 1=R] */ + /* External vblank counter (debug_server.c) used as event timestamp. */ extern uint64_t s_frame_count; @@ -94,14 +156,23 @@ static void spu_event_record(uint8_t kind, int voice, uint32_t addr) { e->frame = (uint32_t)s_frame_count; e->kind = kind; e->voice = (uint8_t)voice; - e->pitch = spu_regs[(uint32_t)voice * 8u + 2u]; e->addr = addr; - /* PSX voice block layout (16-bit register indices from voice base): - * 0=VOL_L 1=VOL_R 2=PITCH 3=START 4=ADSR_LO 5=ADSR_HI 6=CURVOL 7=LOOP */ - e->adsr_lo = spu_regs[(uint32_t)voice * 8u + 4u]; - e->adsr_hi = spu_regs[(uint32_t)voice * 8u + 5u]; - e->vol_l = spu_regs[(uint32_t)voice * 8u + 0u]; - e->vol_r = spu_regs[(uint32_t)voice * 8u + 1u]; + if (voice >= 0 && voice < SPU_VOICE_COUNT) { + /* PSX voice block layout (16-bit register indices from voice base): + * 0=VOL_L 1=VOL_R 2=PITCH 3=START 4=ADSR_LO 5=ADSR_HI 6=CURVOL 7=LOOP */ + e->pitch = spu_regs[(uint32_t)voice * 8u + 2u]; + e->adsr_lo = spu_regs[(uint32_t)voice * 8u + 4u]; + e->adsr_hi = spu_regs[(uint32_t)voice * 8u + 5u]; + e->vol_l = spu_regs[(uint32_t)voice * 8u + 0u]; + e->vol_r = spu_regs[(uint32_t)voice * 8u + 1u]; + } else { + /* Non-voice-attributable events (e.g. SPU_EV_IRQ, voice=0xFF). */ + e->pitch = 0; + e->adsr_lo = 0; + e->adsr_hi = 0; + e->vol_l = 0; + e->vol_r = 0; + } s_event_idx++; } @@ -233,18 +304,99 @@ static inline uint16_t voice_reg(int voice, int reg) { return spu_regs[(uint32_t)voice * 8u + (uint32_t)reg]; } -static inline int16_t direct_volume(uint16_t raw) { - int32_t v; - if (raw & 0x8000u) { - /* Sweep mode is not modeled; use the magnitude as a direct volume. */ - v = (int32_t)(raw & 0x7FFFu); - } else { - v = (int32_t)(raw & 0x7FFFu); - if (v & 0x4000) v -= 0x8000; +/* Register shorthand: current live value of a named SPU register. */ +#define RREG(a) (spu_regs[reg_index(a)]) +#define RVOL(a) ((int16_t)RREG(a)) + +/* Direct-mode volume decode (bit15=0): bits 14-0 are a signed value and the + * effective 16-bit volume is that value << 1 — bit 14 lands on the int16 + * sign bit, so sign extension falls out of the shift for free. Effective + * volumes are applied as (sample * vol) >> 15 throughout, which for direct + * registers is bit-identical to the previous (15-bit vol) >> 14 decode. */ +static inline int16_t volume_reg_decode(uint16_t raw) { + return (int16_t)((uint16_t)(raw << 1)); +} + +/* Called on every guest write to a volume register. Direct writes take + * effect immediately; a sweep-mode write starts the sweep FROM the current + * live level (documented reading: the sweep register programs an envelope, + * it does not itself carry a target level). The divider restarts either way. */ +static void sweep_env_write(SweepEnv *sw, uint16_t raw) { + if (!(raw & 0x8000u)) + sw->level = volume_reg_decode(raw); + sw->divider = 0; +} + +/* Step one sweep envelope by one 44100 Hz output sample. No-op for + * direct-mode registers. Sweep register layout (bit15=1): + * bit14 mode 0=linear 1=exponential + * bit13 direction 0=increase 1=decrease + * bit12 phase 0=positive 1=negative + * bit6-0 rate (bits 0-1 step, bits 2-6 shift) — same 7-bit rate + * format as ADSR, so calc_vc_delta is reused verbatim. + * + * DOCUMENTED-GAP: the documentation does not spell out how the "phase" + * bit interacts with a level whose sign disagrees with it. Model chosen: + * the envelope machinery always operates on a 0..0x7FFF working value in + * the phase's domain (negative phase mirrors the level), and a level on the + * wrong side of zero is clamped to 0 before stepping. Increase saturates at + * 0x7FFF, decrease at 0, matching the ADSR clamp behaviour. Candidate for + * oracle verification. */ +static void sweep_env_step(SweepEnv *sw, uint16_t raw) { + if (!(raw & 0x8000u)) return; + int exp_mode = (raw >> 14) & 1; + int dec_mode = (raw >> 13) & 1; + int neg_phase = (raw >> 12) & 1; + uint8_t rate = (uint8_t)(raw & 0x7F); + + int32_t working = sw->level; + if (neg_phase) working = -working; + if (working < 0) working = 0; + if (working > 0x7FFF) working = 0x7FFF; + + int increment = 0, divinco = 0; + calc_vc_delta(0x7F, rate, exp_mode, dec_mode, dec_mode, + (int16_t)working, &increment, &divinco); + + sw->divider += (uint32_t)divinco; + if (sw->divider & 0x8000u) { + sw->divider = 0; + working += increment; + if (working < 0) working = 0; + if (working > 0x7FFF) working = 0x7FFF; + sw->level = (int16_t)(neg_phase ? -working : working); + } +} + +/* Live effective volume of a volume register: the register decode in direct + * mode, the sweep envelope's current level in sweep mode. */ +static inline int16_t chan_volume(uint16_t raw, const SweepEnv *sw) { + if (raw & 0x8000u) return sw->level; + return volume_reg_decode(raw); +} + +/* SPU RAM IRQ-address compare, called from EVERY SPU RAM access site (FIFO, + * DMA both directions, ADPCM block fetch, capture-buffer writes, reverb + * work-area reads/writes, and the transfer/IRQ-address/SPUCNT register + * writes). `addr`/`len` describe the accessed byte range; the compare is at + * 8-byte-unit granularity because that is the IRQ address register's unit. */ +static void spu_irq_check(uint32_t addr, uint32_t len) { + if (!(RREG(0x1F801DAAu) & 0x0040u)) return; /* SPUCNT.6 IRQ enable */ + if (len == 0) return; + uint32_t target = RREG(0x1F801DA4u); /* in 8-byte units */ + uint32_t a = addr & (SPU_RAM_SIZE - 1u); + uint32_t first = a >> 3; + uint32_t last = (a + len - 1u) >> 3; + for (uint32_t u = first; u <= last; u++) { + if ((u & 0xFFFFu) == target) { + if (!irq_flag) { + irq_flag = 1; + psx_irq_raise(9u, u << 3); + spu_event_record(SPU_EV_IRQ, 0xFF, u << 3); + } + return; + } } - if (v > 0x3FFF) v = 0x3FFF; - if (v < -0x4000) v = -0x4000; - return (int16_t)v; } static inline int16_t cd_input_volume(uint16_t raw) { @@ -314,6 +466,11 @@ static void decode_block(SpuVoice *v) { uint32_t addr = v->cur_addr & (SPU_RAM_SIZE - 1u); if (addr + 16u > SPU_RAM_SIZE) addr = 0; + /* The 16-byte ADPCM block fetch is an SPU RAM access: IRQ-address games + * (CD streaming double-buffers, MDEC audio) park the IRQ inside a voice's + * sample buffer and rely on this exact compare firing. */ + spu_irq_check(addr, 16u); + uint8_t header = spu_ram[addr + 0u]; uint8_t flags = spu_ram[addr + 1u]; int shift = header & 0x0F; @@ -370,6 +527,233 @@ static void decode_block(SpuVoice *v) { } } +/* ---- Noise generator ---------------------------------------------------- */ +/* Documented LFSR update: the new low bit is the parity term + * (bit15 XOR bit12 XOR bit11 XOR bit10 XOR 1) and the register shifts left. + * The XOR-with-1 makes the all-zero state self-starting. + * + * Clock rate derivation from SPUCNT bits 8-13 (step = 4 + bits8-9, + * shift = bits10-13): a down-counter is decremented by `step` once per + * 44100 Hz sample cycle and the LFSR clocks on underflow, whereupon the + * counter is topped up by (0x20000 >> shift) — reloading twice if a single + * reload does not clear the deficit (only reachable at the largest shifts). + * shift=0 gives the slowest clock (lowest frequency), shift=15 the fastest, + * and the 4..7 step scales the rate within a shift octave. + * + * DOCUMENTED-GAP: this divider wiring is the least precisely documented part + * of the noise unit; the shift/step semantics above are the documented + * reading, but the exact counter width/reload behaviour is a candidate for + * oracle verification against Beetle's audio OUTPUT. */ +static void noise_clock(uint16_t ctrl) { + int step = 4 + ((ctrl >> 8) & 3); + int shift = (ctrl >> 10) & 0x0F; + int32_t reload = (int32_t)(0x20000u >> shift); + noise_timer -= step; + if (noise_timer < 0) { + uint16_t parity = (uint16_t)(((noise_lfsr >> 15) ^ (noise_lfsr >> 12) ^ + (noise_lfsr >> 11) ^ (noise_lfsr >> 10) ^ 1u) & 1u); + noise_lfsr = (uint16_t)((noise_lfsr << 1) | parity); + noise_timer += reload; + if (noise_timer < 0) noise_timer += reload; /* worst case: step 7, reload 4 */ + } +} + +/* ---- Capture buffers ----------------------------------------------------- */ +static void capture_write(uint32_t base, int16_t s) { + uint32_t a = (base + capture_pos) & (SPU_RAM_SIZE - 1u); + spu_irq_check(a, 2u); + spu_ram[a] = (uint8_t)((uint16_t)s & 0xFFu); + spu_ram[a + 1u] = (uint8_t)(((uint16_t)s >> 8) & 0xFFu); +} + +/* ---- Reverb engine ------------------------------------------------------- + * Register map (psx-spx). All address/offset registers are in units of 8 + * bytes; all volumes are signed Q0.15 (divide by 32768). */ +#define SPU_R_VLOUT 0x1F801D84u /* reverb output volume L */ +#define SPU_R_VROUT 0x1F801D86u /* reverb output volume R */ +#define SPU_R_MBASE 0x1F801DA2u /* work area start address */ +#define SPU_R_DAPF1 0x1F801DC0u /* APF offset 1 */ +#define SPU_R_DAPF2 0x1F801DC2u /* APF offset 2 */ +#define SPU_R_VIIR 0x1F801DC4u /* reflection volume 1 */ +#define SPU_R_VCOMB1 0x1F801DC6u /* comb volume 1 */ +#define SPU_R_VCOMB2 0x1F801DC8u /* comb volume 2 */ +#define SPU_R_VCOMB3 0x1F801DCAu /* comb volume 3 */ +#define SPU_R_VCOMB4 0x1F801DCCu /* comb volume 4 */ +#define SPU_R_VWALL 0x1F801DCEu /* reflection volume 2 */ +#define SPU_R_VAPF1 0x1F801DD0u /* APF volume 1 */ +#define SPU_R_VAPF2 0x1F801DD2u /* APF volume 2 */ +#define SPU_R_MLSAME 0x1F801DD4u /* same-side reflection addr 1 L (src/dst) */ +#define SPU_R_MRSAME 0x1F801DD6u /* same-side reflection addr 1 R (src/dst) */ +#define SPU_R_MLCOMB1 0x1F801DD8u /* comb addr 1 L (src) */ +#define SPU_R_MRCOMB1 0x1F801DDAu /* comb addr 1 R (src) */ +#define SPU_R_MLCOMB2 0x1F801DDCu /* comb addr 2 L (src) */ +#define SPU_R_MRCOMB2 0x1F801DDEu /* comb addr 2 R (src) */ +#define SPU_R_DLSAME 0x1F801DE0u /* same-side reflection addr 2 L (src) */ +#define SPU_R_DRSAME 0x1F801DE2u /* same-side reflection addr 2 R (src) */ +#define SPU_R_MLDIFF 0x1F801DE4u /* diff-side reflection addr 1 L (src/dst) */ +#define SPU_R_MRDIFF 0x1F801DE6u /* diff-side reflection addr 1 R (src/dst) */ +#define SPU_R_MLCOMB3 0x1F801DE8u /* comb addr 3 L (src) */ +#define SPU_R_MRCOMB3 0x1F801DEAu /* comb addr 3 R (src) */ +#define SPU_R_MLCOMB4 0x1F801DECu /* comb addr 4 L (src) */ +#define SPU_R_MRCOMB4 0x1F801DEEu /* comb addr 4 R (src) */ +#define SPU_R_DLDIFF 0x1F801DF0u /* diff-side reflection addr 2 L (src) */ +#define SPU_R_DRDIFF 0x1F801DF2u /* diff-side reflection addr 2 R (src) */ +#define SPU_R_MLAPF1 0x1F801DF4u /* APF addr 1 L (src/dst) */ +#define SPU_R_MRAPF1 0x1F801DF6u /* APF addr 1 R (src/dst) */ +#define SPU_R_MLAPF2 0x1F801DF8u /* APF addr 2 L (src/dst) */ +#define SPU_R_MRAPF2 0x1F801DFAu /* APF addr 2 R (src/dst) */ +#define SPU_R_VLIN 0x1F801DFCu /* reverb input volume L */ +#define SPU_R_VRIN 0x1F801DFEu /* reverb input volume R */ + +/* Signed Q0.15 volume multiply (the reverb pipeline's only multiply shape). */ +static inline int32_t rev_mul(int16_t vol, int32_t s) { + return (int32_t)(((int64_t)vol * (int64_t)s) >> 15); +} + +static inline uint32_t rev_mbase(void) { + return (((uint32_t)RREG(SPU_R_MBASE)) << 3) & 0x7FFFEu; +} + +/* Effective work-area address for an address register plus a byte bias + * (bias -2 implements the documented [x-2] taps, bias -dAPFn*8 the APF + * delay taps). A register value r denotes byte offset r*8 from the current + * buffer address; the result wraps inside [mBASE<<3, 0x80000) and never + * escapes it, whatever the register values. */ +static uint32_t rev_ea(uint32_t reg_addr, int32_t bias) { + uint32_t base = rev_mbase(); + uint32_t size = SPU_RAM_SIZE - base; + int64_t rel = (int64_t)rev_cur - (int64_t)base + + ((int64_t)RREG(reg_addr) << 3) + (int64_t)bias; + rel %= (int64_t)size; + if (rel < 0) rel += (int64_t)size; + return (base + (uint32_t)rel) & 0x7FFFEu; +} + +static int16_t rev_read(uint32_t reg_addr, int32_t bias) { + uint32_t a = rev_ea(reg_addr, bias); + spu_irq_check(a, 2u); + return (int16_t)((uint16_t)spu_ram[a] | ((uint16_t)spu_ram[a + 1u] << 8)); +} + +/* Work-area store, saturated to signed 16-bit. Writes (and their IRQ + * checks) happen only while SPUCNT bit 7 is set; reads always occur. + * DOCUMENTED-GAP: whether a SUPPRESSED write still drives the address bus + * (and could therefore trip the IRQ compare) is not documented; this model + * only checks on accesses that actually happen. */ +static void rev_write(uint32_t reg_addr, int32_t v, int wren) { + if (!wren) return; + uint32_t a = rev_ea(reg_addr, 0); + int16_t s = clamp16(v); + spu_irq_check(a, 2u); + spu_ram[a] = (uint8_t)((uint16_t)s & 0xFFu); + spu_ram[a + 1u] = (uint8_t)(((uint16_t)s >> 8) & 0xFFu); +} + +/* One documented reverb step at 22050 Hz. `in_l`/`in_r` are the reverb-send + * mix (voices with EON set + CD audio if SPUCNT bits 0 and 2). Leaves the + * vLOUT/vROUT-scaled result in rev_out_l/rev_out_r and advances the buffer + * address. RAM accesses are performed in the documented statement order so + * the dAPF1==0 / dAPF2==0 corner (re-read of a just-written cell) behaves + * as literally written. */ +static void reverb_step(int32_t in_l, int32_t in_r, int wren) { + int32_t dapf1 = -((int32_t)RREG(SPU_R_DAPF1) << 3); + int32_t dapf2 = -((int32_t)RREG(SPU_R_DAPF2) << 3); + int16_t vIIR = RVOL(SPU_R_VIIR); + int16_t vWALL = RVOL(SPU_R_VWALL); + int16_t vAPF1 = RVOL(SPU_R_VAPF1); + int16_t vAPF2 = RVOL(SPU_R_VAPF2); + + /* Input from the mixer. */ + int32_t Lin = rev_mul(RVOL(SPU_R_VLIN), in_l); + int32_t Rin = rev_mul(RVOL(SPU_R_VRIN), in_r); + + /* Same-side reflection (L->L, R->R): + * [mLSAME] = (Lin + [dLSAME]*vWALL - [mLSAME-2])*vIIR + [mLSAME-2] */ + { + int32_t prev_l = rev_read(SPU_R_MLSAME, -2); + rev_write(SPU_R_MLSAME, + rev_mul(vIIR, Lin + rev_mul(vWALL, rev_read(SPU_R_DLSAME, 0)) + - prev_l) + prev_l, + wren); + int32_t prev_r = rev_read(SPU_R_MRSAME, -2); + rev_write(SPU_R_MRSAME, + rev_mul(vIIR, Rin + rev_mul(vWALL, rev_read(SPU_R_DRSAME, 0)) + - prev_r) + prev_r, + wren); + } + + /* Different-side reflection (L->R, R->L) — note the CROSSED d*DIFF + * sources: mLDIFF takes dRDIFF, mRDIFF takes dLDIFF. */ + { + int32_t prev_l = rev_read(SPU_R_MLDIFF, -2); + rev_write(SPU_R_MLDIFF, + rev_mul(vIIR, Lin + rev_mul(vWALL, rev_read(SPU_R_DRDIFF, 0)) + - prev_l) + prev_l, + wren); + int32_t prev_r = rev_read(SPU_R_MRDIFF, -2); + rev_write(SPU_R_MRDIFF, + rev_mul(vIIR, Rin + rev_mul(vWALL, rev_read(SPU_R_DLDIFF, 0)) + - prev_r) + prev_r, + wren); + } + + /* Early echo: 4-tap comb filter. */ + int32_t lout = rev_mul(RVOL(SPU_R_VCOMB1), rev_read(SPU_R_MLCOMB1, 0)) + + rev_mul(RVOL(SPU_R_VCOMB2), rev_read(SPU_R_MLCOMB2, 0)) + + rev_mul(RVOL(SPU_R_VCOMB3), rev_read(SPU_R_MLCOMB3, 0)) + + rev_mul(RVOL(SPU_R_VCOMB4), rev_read(SPU_R_MLCOMB4, 0)); + int32_t rout = rev_mul(RVOL(SPU_R_VCOMB1), rev_read(SPU_R_MRCOMB1, 0)) + + rev_mul(RVOL(SPU_R_VCOMB2), rev_read(SPU_R_MRCOMB2, 0)) + + rev_mul(RVOL(SPU_R_VCOMB3), rev_read(SPU_R_MRCOMB3, 0)) + + rev_mul(RVOL(SPU_R_VCOMB4), rev_read(SPU_R_MRCOMB4, 0)); + + /* Late reverb all-pass filter 1: + * Lout = Lout - vAPF1*[mLAPF1-dAPF1]; [mLAPF1] = Lout; + * Lout = Lout*vAPF1 + [mLAPF1-dAPF1] */ + lout = lout - rev_mul(vAPF1, rev_read(SPU_R_MLAPF1, dapf1)); + rev_write(SPU_R_MLAPF1, lout, wren); + lout = rev_mul(vAPF1, clamp16(lout)) + rev_read(SPU_R_MLAPF1, dapf1); + rout = rout - rev_mul(vAPF1, rev_read(SPU_R_MRAPF1, dapf1)); + rev_write(SPU_R_MRAPF1, rout, wren); + rout = rev_mul(vAPF1, clamp16(rout)) + rev_read(SPU_R_MRAPF1, dapf1); + + /* Late reverb all-pass filter 2. */ + lout = lout - rev_mul(vAPF2, rev_read(SPU_R_MLAPF2, dapf2)); + rev_write(SPU_R_MLAPF2, lout, wren); + lout = rev_mul(vAPF2, clamp16(lout)) + rev_read(SPU_R_MLAPF2, dapf2); + rout = rout - rev_mul(vAPF2, rev_read(SPU_R_MRAPF2, dapf2)); + rev_write(SPU_R_MRAPF2, rout, wren); + rout = rev_mul(vAPF2, clamp16(rout)) + rev_read(SPU_R_MRAPF2, dapf2); + + /* Output to mixer. */ + rev_out_l = rev_mul(RVOL(SPU_R_VLOUT), clamp16(lout)); + rev_out_r = rev_mul(RVOL(SPU_R_VROUT), clamp16(rout)); + + /* Documented advance: BufferAddress = MAX(mBASE, (BufferAddress+2) AND 7FFFEh) */ + { + uint32_t base = rev_mbase(); + rev_cur = (rev_cur + 2u) & 0x7FFFEu; + if (rev_cur < base) rev_cur = base; + } +} + +/* ⚠ DOCUMENTED-GAP — the ONE known deliberate deviation from hardware. + * The reverb engine runs at 22050 Hz but the real SPU's 22.05 -> 44.1 kHz + * reconstruction filter (and the matching 44.1 -> 22.05 kHz input + * decimation) is NOT specified in the documentation this implementation is + * built from, and consulting an emulator's filter is off-limits for license + * reasons. Chosen documented-behaviour stand-ins, both trivially swappable: + * - input: box decimation (average of the two 44.1 kHz frames per step), + * - output: this function — linear interpolation between successive + * 22050 Hz engine results (even output frame = the step result, + * odd frame = midpoint of the two neighbouring results). + * A later Beetle-output oracle comparison should quantify the spectral + * difference; replace ONLY this function (and the input average at the + * call site in spu_render) if a better-documented filter emerges. */ +static inline int32_t rev_reconstruct(int32_t prev_step, int32_t cur_step) { + return (prev_step + cur_step) >> 1; +} + /* ---- Verified-enhancement shadow tap (opt-in; see spu_shadow.{h,c}) ------ * * When the float SPU shadow is enabled, the mix loop records — per output @@ -386,7 +770,7 @@ typedef struct { int16_t s[4]; /* decoded samples at sample_idx-1 .. sample_idx+2 */ float frac; /* fractional phase in [0,1) at this output frame */ uint16_t env; /* env_level (0..0x7FFF) */ - int16_t vol_l; /* per-voice volume (already direct_volume-decoded) */ + int16_t vol_l; /* per-voice volume, 1.14 scale (effective int16 >> 1) */ int16_t vol_r; uint8_t active; } SpuShadowVoiceTap; @@ -454,10 +838,21 @@ static int16_t voice_next_sample(int idx) { decode_block(v); } - int16_t raw_s = spu_gaussian_interpolate(v->previous_samples, - v->samples, - v->sample_idx, - v->phase); + /* Noise mode (NON bit set): the voice outputs the live noise LFSR value + * INSTEAD of its interpolated ADPCM sample. Everything else — block + * decoding/address advance, ENDX latching, pitch stepping, the ADSR + * envelope and the volume stages — behaves exactly as for ADPCM. */ + int16_t raw_s; + uint32_t non_mask = (uint32_t)spu_regs[reg_index(0x1F801D94u)] + | ((uint32_t)spu_regs[reg_index(0x1F801D96u)] << 16); + if (non_mask & (1u << idx)) { + raw_s = (int16_t)noise_lfsr; + } else { + raw_s = spu_gaussian_interpolate(v->previous_samples, + v->samples, + v->sample_idx, + v->phase); + } /* Apply envelope (0..0x7FFF as a 15-bit gain). */ int32_t shaped = ((int32_t)raw_s * (int32_t)v->env_level) >> 15; if (shaped > 32767) shaped = 32767; @@ -552,6 +947,20 @@ void spu_init(void) { endx_latch = 0; kon_latch = 0; koff_latch = 0; + irq_flag = 0; + /* Hardware power-on LFSR value is undocumented; 0 is safe because the + * xor-1 parity term self-starts the register within 16 clocks. */ + noise_lfsr = 0; + noise_timer = 0; + rev_cur = 0; + rev_phase = 0; + rev_in_hold_l = 0; + rev_in_hold_r = 0; + rev_out_l = 0; + rev_out_r = 0; + capture_pos = 0; + memset(sweep_voice_env, 0, sizeof(sweep_voice_env)); + memset(sweep_main_env, 0, sizeof(sweep_main_env)); s_event_idx = 0; s_event_seq = 0; spu_cd_audio_reset(); @@ -564,11 +973,14 @@ void spu_render(int16_t* out_stereo, int frames) { if (!out_stereo || frames <= 0) return; uint16_t ctrl = spu_regs[reg_index(0x1F801DAAu)]; - int enabled = (ctrl & 0x8000u) != 0; - int16_t main_l = direct_volume(spu_regs[reg_index(0x1F801D80u)]); - int16_t main_r = direct_volume(spu_regs[reg_index(0x1F801D82u)]); + int enabled = (ctrl & 0x8000u) != 0; + int cd_on = (ctrl & 0x0001u) != 0; + int cd_rev = cd_on && (ctrl & 0x0004u) != 0; /* CD reverb send needs CD enable */ + int rev_wren = (ctrl & 0x0080u) != 0; /* reverb work-area write enable */ int16_t cd_vol_l = cd_input_volume(spu_regs[reg_index(0x1F801DB0u)]); int16_t cd_vol_r = cd_input_volume(spu_regs[reg_index(0x1F801DB2u)]); + uint32_t eon = (uint32_t)spu_regs[reg_index(0x1F801D98u)] + | ((uint32_t)spu_regs[reg_index(0x1F801D9Au)] << 16); int any_voice = 0; if (enabled) { @@ -587,54 +999,14 @@ void spu_render(int16_t* out_stereo, int frames) { memset(s_shadow_tap, 0, (size_t)cap * sizeof(s_shadow_tap[0])); } - /* MotK intro FMV: active_mask stays 0 while XA/CD audio plays. The old - * path still walked 24 idle voices and called audio_trace_pcm(VOICES, 1) - * per sample (~735 atomics/vblank). Host FPS fell off a cliff when XA - * started (60 → ~4) with ~67% wall time in phase "other". */ - if (enabled && !any_voice && !s_shadow_tap_on) { - static int16_t s_voice_silence[2048 * 2]; - int voice_tap_n = frames; - if (voice_tap_n > 2048) voice_tap_n = 2048; - memset(s_voice_silence, 0, (size_t)voice_tap_n * 2u * sizeof(int16_t)); - for (int off = 0; off < frames; ) { - int n = frames - off; - if (n > 2048) n = 2048; - audio_trace_pcm(AUDIO_TAP_VOICES, s_voice_silence, n); - off += n; - } - - int32_t block_peak = 0; - int cd_on = (ctrl & 0x0001u) != 0; - for (int f = 0; f < frames; f++) { - int32_t mix_l = 0; - int32_t mix_r = 0; - if (cd_on) { - int16_t cd_l = 0; - int16_t cd_r = 0; - if (cd_audio_pop(&cd_l, &cd_r)) { - mix_l = ((int32_t)cd_l * cd_vol_l) >> 15; - mix_r = ((int32_t)cd_r * cd_vol_r) >> 15; - } else if (cd_push_frames != 0) { - cd_underflow_frames++; - } - } - mix_l = (mix_l * main_l) >> 14; - mix_r = (mix_r * main_r) >> 14; - out_stereo[f * 2 + 0] = clamp16(mix_l); - out_stereo[f * 2 + 1] = clamp16(mix_r); - int32_t frame_peak = abs32(out_stereo[f * 2 + 0]); - int32_t right_peak = abs32(out_stereo[f * 2 + 1]); - if (right_peak > frame_peak) frame_peak = right_peak; - if (frame_peak) nonzero_frames++; - if (frame_peak > block_peak) block_peak = frame_peak; - } - render_frames += (uint64_t)frames; - last_peak = block_peak; - if (block_peak > peak) peak = block_peak; - spu_shadow_process(out_stereo, frames); - audio_trace_pcm(AUDIO_TAP_SPU_OUT, out_stereo, frames); - return; - } + /* NOTE: the CD-only fast path that used to live here (MotK FMV perf fix) + * was REMOVED rather than extended: the SPU now has per-frame DSP work + * that must run in EVERY path — capture-buffer writes with IRQ checks, + * the 22050 Hz reverb engine (FMV = XA audio + zero voices is exactly + * the CD-with-reverb case), the noise LFSR clock and volume sweeps. + * What made the pre-fast-path slow path slow is gone regardless: the + * 24-voice walk is guarded by any_voice and the AUDIO_TAP_VOICES trace + * is emitted once per block, not once per sample. */ int32_t block_peak = 0; /* Voice-sum tap is filled once per block (not per sample) — same bytes. */ @@ -645,23 +1017,53 @@ void spu_render(int16_t* out_stereo, int frames) { for (int f = 0; f < frames; f++) { int32_t mix_l = 0; int32_t mix_r = 0; - int32_t voice_l = 0; - int32_t voice_r = 0; + /* Main L/R volumes are sweep-aware and can glide per sample. Kept + * live-per-frame; the >>1 below preserves the shadow tap's historical + * 1.14 volume scale (identical bytes for direct-mode registers). */ + int16_t main_l = chan_volume(spu_regs[reg_index(0x1F801D80u)], &sweep_main_env[0]); + int16_t main_r = chan_volume(spu_regs[reg_index(0x1F801D82u)], &sweep_main_env[1]); if (enabled) { + int32_t voice_l = 0; + int32_t voice_r = 0; + int32_t rev_send_l = 0; + int32_t rev_send_r = 0; + int16_t v1_out = 0; /* voice 1 post-envelope output (capture) */ + int16_t v3_out = 0; /* voice 3 post-envelope output (capture) */ + + /* Volume sweeps step once per 44100 Hz sample on the same rate + * machinery as ADSR; no-ops for direct-mode registers. */ + sweep_env_step(&sweep_main_env[0], spu_regs[reg_index(0x1F801D80u)]); + sweep_env_step(&sweep_main_env[1], spu_regs[reg_index(0x1F801D82u)]); + for (int v = 0; v < SPU_VOICE_COUNT; v++) { + sweep_env_step(&sweep_voice_env[v][0], voice_reg(v, 0)); + sweep_env_step(&sweep_voice_env[v][1], voice_reg(v, 1)); + } + if (any_voice) { for (int v = 0; v < SPU_VOICE_COUNT; v++) { int16_t s = voice_next_sample(v); - int16_t vl = direct_volume(voice_reg(v, 0)); - int16_t vr = direct_volume(voice_reg(v, 1)); + if (v == 1) v1_out = s; + if (v == 3) v3_out = s; + int16_t vl = chan_volume(voice_reg(v, 0), &sweep_voice_env[v][0]); + int16_t vr = chan_volume(voice_reg(v, 1), &sweep_voice_env[v][1]); if (s_shadow_tap_on && f < SPU_SHADOW_TAP_FRAMES) { SpuShadowVoiceTap *t = &s_shadow_tap[f].voice[v]; - t->vol_l = vl; - t->vol_r = vr; + /* Tap keeps its historical 1.14 volume scale. */ + t->vol_l = (int16_t)(vl >> 1); + t->vol_r = (int16_t)(vr >> 1); } if (!s) continue; - voice_l += ((int32_t)s * vl) >> 14; - voice_r += ((int32_t)s * vr) >> 14; + int32_t cl = ((int32_t)s * vl) >> 15; + int32_t cr = ((int32_t)s * vr) >> 15; + voice_l += cl; + voice_r += cr; + /* Per-voice reverb send: EON voices feed the reverb input + * bus with their post-envelope, post-voice-volume output. */ + if (eon & (1u << v)) { + rev_send_l += cl; + rev_send_r += cr; + } } } mix_l = voice_l; @@ -671,18 +1073,87 @@ void spu_render(int16_t* out_stereo, int frames) { s_voice_sum[voice_sum_pos * 2 + 1] = clamp16(voice_r); voice_sum_pos++; } - if (ctrl & 0x0001u) { - int16_t cd_l = 0; - int16_t cd_r = 0; - if (cd_audio_pop(&cd_l, &cd_r)) { - mix_l += ((int32_t)cd_l * cd_vol_l) >> 15; - mix_r += ((int32_t)cd_r * cd_vol_r) >> 15; - } else if (cd_push_frames != 0) { - cd_underflow_frames++; + + /* CD input bus. The bus runs continuously while the SPU is + * enabled (the capture buffers record it regardless of SPUCNT + * bit 0); bit 0 only gates its contribution to the mix, and + * bit 2 (with bit 0) its reverb send. */ + int16_t cd_l = 0; + int16_t cd_r = 0; + if (cd_audio_pop(&cd_l, &cd_r)) { + /* got a frame */ + } else if (cd_on && cd_push_frames != 0) { + cd_underflow_frames++; + } + if (cd_on) { + int32_t ccl = ((int32_t)cd_l * cd_vol_l) >> 15; + int32_t ccr = ((int32_t)cd_r * cd_vol_r) >> 15; + mix_l += ccl; + mix_r += ccr; + if (cd_rev) { + rev_send_l += ccl; + rev_send_r += ccr; + } + } + + /* Capture buffers: CD input L/R at 0x000/0x400, voice 1/3 + * post-envelope output at 0x800/0xC00; one halfword per 44100 Hz + * sample, wrapping every 0x400 bytes. Each store runs the IRQ + * check — games park the IRQ address here. The CD samples are + * recorded PRE CD-volume (the raw input bus). + * DOCUMENTED-GAP: whether the CD capture value is pre- or + * post-CD-volume is not settled by the documentation; the raw + * input-bus reading was chosen. Candidate for oracle check. */ + capture_write(0x0000u, cd_l); + capture_write(0x0400u, cd_r); + capture_write(0x0800u, v1_out); + capture_write(0x0C00u, v3_out); + capture_pos = (capture_pos + 2u) & 0x3FFu; + + /* Noise LFSR clock: free-running at the SPUCNT bits 8-13 rate, + * once per output sample cycle. Clocked AFTER the voice walk, so + * noise voices see the value from the previous cycle (whether + * hardware clocks before or after voice processing within the + * sample cycle is undocumented). */ + noise_clock(ctrl); + + /* Reverb: the engine runs one step per TWO output frames + * (22050 Hz). The engine always steps while the SPU is enabled — + * SPUCNT bit 7 gates only its work-area WRITES (reads, IRQ + * checks on those reads, address advance and output still + * happen), so a frozen work area keeps ringing through vLOUT + * until the game silences it. See rev_reconstruct for the + * 22.05 <-> 44.1 kHz boundary model. */ + { + int32_t wet_l, wet_r; + if (rev_phase == 0) { + /* First frame of the pair: hold the input, output the + * most recent engine result. */ + rev_in_hold_l = rev_send_l; + rev_in_hold_r = rev_send_r; + wet_l = rev_out_l; + wet_r = rev_out_r; + rev_phase = 1; + } else { + int32_t prev_l = rev_out_l; + int32_t prev_r = rev_out_r; + reverb_step(clamp16((rev_in_hold_l + rev_send_l) >> 1), + clamp16((rev_in_hold_r + rev_send_r) >> 1), + rev_wren); + wet_l = rev_reconstruct(prev_l, rev_out_l); + wet_r = rev_reconstruct(prev_r, rev_out_r); + rev_phase = 0; } + mix_l += wet_l; + mix_r += wet_r; } - mix_l = (mix_l * main_l) >> 14; - mix_r = (mix_r * main_r) >> 14; + + /* The 16-bit mix bus saturates BEFORE the main volume applies + * (documented mixer order: sum -> saturate -> main volume). */ + mix_l = clamp16(mix_l); + mix_r = clamp16(mix_r); + mix_l = ((int32_t)mix_l * main_l) >> 15; + mix_r = ((int32_t)mix_r * main_r) >> 15; } out_stereo[f * 2 + 0] = clamp16(mix_l); @@ -694,8 +1165,9 @@ void spu_render(int16_t* out_stereo, int frames) { if (frame_peak > block_peak) block_peak = frame_peak; if (s_shadow_tap_on && f < SPU_SHADOW_TAP_FRAMES) { - s_shadow_tap[f].main_l = main_l; - s_shadow_tap[f].main_r = main_r; + /* Tap keeps its historical 1.14 volume scale. */ + s_shadow_tap[f].main_l = (int16_t)(main_l >> 1); + s_shadow_tap[f].main_r = (int16_t)(main_r >> 1); s_shadow_tap[f].enabled = enabled; s_shadow_tap_frame = f + 1; } @@ -733,8 +1205,12 @@ void spu_debug_info(SpuDebugInfo* out) { if (!out) return; memset(out, 0, sizeof(*out)); out->ctrl = spu_regs[reg_index(0x1F801DAAu)]; - out->main_l = direct_volume(spu_regs[reg_index(0x1F801D80u)]); - out->main_r = direct_volume(spu_regs[reg_index(0x1F801D82u)]); + /* Historical 1.14 scale (effective int16 volume >> 1) for continuity + * with older captures; identical values for direct-mode registers. */ + out->main_l = (int16_t)(chan_volume(spu_regs[reg_index(0x1F801D80u)], + &sweep_main_env[0]) >> 1); + out->main_r = (int16_t)(chan_volume(spu_regs[reg_index(0x1F801D82u)], + &sweep_main_env[1]) >> 1); out->cd_l = cd_input_volume(spu_regs[reg_index(0x1F801DB0u)]); out->cd_r = cd_input_volume(spu_regs[reg_index(0x1F801DB2u)]); for (int i = 0; i < SPU_VOICE_COUNT; i++) { @@ -757,15 +1233,47 @@ uint32_t spu_read(uint32_t addr) { if (idx < SPU_REG_COUNT) { if (addr == 0x1F801DAEu) { /* SPUSTAT (psx-spx): bits 5-0 mirror SPUCNT bits 5-0 (the - * current SPU mode), bit 7 follows SPUCNT.5 (DMA r/w - * request), bit 10 is the data-transfer busy flag — 0 here - * because this runtime completes FIFO/DMA transfers + * current SPU mode), bit 6 is the IRQ flag (cleared by + * writing SPUCNT with bit 6 clear), bit 7 follows SPUCNT.5 + * (DMA r/w request), bit 10 is the data-transfer busy flag — + * 0 here because this runtime completes FIFO/DMA transfers * instantly. The old hardcoded 0x0400 held busy PERMANENTLY * asserted; Sony code paths never polled it to zero, but * OpenBIOS's shell MOD player waits for (SPUSTAT & 0x7FF) - * == 0 after clearing SPUCNT and spun forever. */ + * == 0 after clearing SPUCNT and spun forever. Bit 11 is + * "currently writing the SECOND half of the capture + * buffers" (capture offset >= 0x200). */ uint16_t cnt = spu_regs[reg_index(0x1F801DAAu)]; - return (uint32_t)((cnt & 0x3Fu) | (((cnt >> 5) & 1u) << 7)); + uint32_t st = (uint32_t)((cnt & 0x3Fu) | (((cnt >> 5) & 1u) << 7)); + if (irq_flag) st |= 0x40u; + if (capture_pos & 0x200u) st |= 0x800u; + return st; + } + /* Current main volume L/R (psx-spx 1F801DB8h/1F801DBAh): the + * LIVE sweep-aware level as a signed 16-bit value. */ + if (addr == 0x1F801DB8u) { + return (uint32_t)(uint16_t)chan_volume( + spu_regs[reg_index(0x1F801D80u)], &sweep_main_env[0]); + } + if (addr == 0x1F801DBAu) { + return (uint32_t)(uint16_t)chan_volume( + spu_regs[reg_index(0x1F801D82u)], &sweep_main_env[1]); + } + /* A volume register in SWEEP mode reads back the envelope's + * CURRENT level, not the sweep-parameter word — the live value + * is what actually multiplies the samples. + * DOCUMENTED-GAP: the exact readback encoding for a sweeping + * volume register is not settled; the live level as a signed + * 16-bit value was chosen. Candidate for oracle verification. */ + if (idx < (uint32_t)SPU_VOICE_COUNT * 8u && (idx & 7u) <= 1u + && (spu_regs[idx] & 0x8000u)) { + return (uint32_t)(uint16_t) + sweep_voice_env[idx >> 3][idx & 7u].level; + } + if ((addr == 0x1F801D80u || addr == 0x1F801D82u) + && (spu_regs[idx] & 0x8000u)) { + return (uint32_t)(uint16_t) + sweep_main_env[(addr >> 1) & 1u].level; } /* ENDX (end-block-reached latch). Real hw sets bit v when voice * v decodes a block whose flag byte has bit 0; KEYON[v] clears @@ -793,6 +1301,18 @@ uint32_t spu_read(uint32_t addr) { } } + /* Per-voice CURRENT volume L/R (psx-spx 1F801E00h..1F801E5Fh): the live + * sweep-aware effective volume of each voice, two halfwords per voice. */ + if (addr >= 0x1F801E00u && addr < 0x1F801E60u) { + uint32_t half = (addr - 0x1F801E00u) >> 1; + int v = (int)(half >> 1); + int ch = (int)(half & 1u); + if (v < SPU_VOICE_COUNT) { + return (uint32_t)(uint16_t)chan_volume( + voice_reg(v, ch), &sweep_voice_env[v][ch]); + } + } + return 0; } @@ -822,6 +1342,34 @@ void spu_write(uint32_t addr, uint32_t value) { ((uint32_t)(uint16_t)value << 3) & (SPU_RAM_SIZE - 1u); } + /* Volume registers feed the sweep envelopes: a direct write + * takes effect immediately, a sweep-mode write starts gliding + * from the current live level. */ + if (idx < (uint32_t)SPU_VOICE_COUNT * 8u && (idx & 7u) <= 1u) { + sweep_env_write(&sweep_voice_env[idx >> 3][idx & 7u], + (uint16_t)value); + } + if (addr == 0x1F801D80u) + sweep_env_write(&sweep_main_env[0], (uint16_t)value); + if (addr == 0x1F801D82u) + sweep_env_write(&sweep_main_env[1], (uint16_t)value); + + /* Reverb work-area base: writing mBASE also resets the current + * buffer address to the area start. */ + if (addr == 0x1F801DA2u) { + rev_cur = rev_mbase(); + } + + /* IRQ address write: re-evaluate the compare against the + * current transfer address. DOCUMENTED-GAP: whether pointing + * the IRQ address AT the resting transfer address fires + * immediately (vs only on the next actual access) is not + * settled; the immediate re-check was chosen so a match is + * never missed. Candidate for oracle verification. */ + if (addr == 0x1F801DA4u) { + spu_irq_check(transfer_addr, 2u); + } + if (addr == 0x1F801D88u) { kon_latch = (kon_latch & 0xFFFF0000u) | (uint32_t)(uint16_t)value; key_on((uint32_t)(uint16_t)value); @@ -842,20 +1390,39 @@ void spu_write(uint32_t addr, uint32_t value) { if (addr == 0x1F801DA6u) { transfer_addr = ((uint32_t)(uint16_t)value) << 3; if (transfer_addr >= SPU_RAM_SIZE) transfer_addr = 0; + /* Setting the transfer address is an IRQ compare site (same + * DOCUMENTED-GAP as the IRQ-address write above). */ + spu_irq_check(transfer_addr, 2u); } if (addr == 0x1F801DA8u) { + /* Manual FIFO write: an SPU RAM access at the transfer + * address — run the IRQ compare before storing. */ + spu_irq_check(transfer_addr, 2u); if (transfer_addr + 1 < SPU_RAM_SIZE) { spu_ram[transfer_addr] = (uint8_t)(value & 0xFF); spu_ram[transfer_addr + 1] = (uint8_t)((value >> 8) & 0xFF); } transfer_addr = (transfer_addr + 2) % SPU_RAM_SIZE; } + + if (addr == 0x1F801DAAu) { + /* SPUCNT: writing with bit 6 CLEAR acknowledges the IRQ + * latch and re-arms it; writing with bit 6 SET re-evaluates + * the compare against the resting transfer address (same + * DOCUMENTED-GAP as the address-register writes above). */ + if (!(value & 0x0040u)) { + irq_flag = 0; + } else { + spu_irq_check(transfer_addr, 2u); + } + } } } } void spu_dma_write(uint32_t word) { + spu_irq_check(transfer_addr, 4u); if (transfer_addr + 3 < SPU_RAM_SIZE) { spu_ram[transfer_addr] = (uint8_t)(word & 0xFF); spu_ram[transfer_addr + 1] = (uint8_t)((word >> 8) & 0xFF); @@ -865,6 +1432,23 @@ void spu_dma_write(uint32_t word) { transfer_addr = (transfer_addr + 4) % SPU_RAM_SIZE; } +/* DMA4 SPU->CPU direction: read one little-endian 32-bit word from SPU RAM + * at the transfer address, advance it by 4 and run the IRQ compare — the + * exact mirror of spu_dma_write. Titles carry state through SPU RAM across + * EXE transitions via this path; returning zeros breaks them. */ +uint32_t spu_dma_read(void) { + uint32_t word = 0; + spu_irq_check(transfer_addr, 4u); + if (transfer_addr + 3 < SPU_RAM_SIZE) { + word = (uint32_t)spu_ram[transfer_addr] + | ((uint32_t)spu_ram[transfer_addr + 1] << 8) + | ((uint32_t)spu_ram[transfer_addr + 2] << 16) + | ((uint32_t)spu_ram[transfer_addr + 3] << 24); + } + transfer_addr = (transfer_addr + 4) % SPU_RAM_SIZE; + return word; +} + int spu_dma_ready(void) { return 1; } @@ -893,6 +1477,8 @@ void spu_get_voice_state(int idx, SpuVoiceState* out) { out->phase = (uint16_t)v->phase; out->env_level = v->env_level; out->adsr_phase = v->adsr_phase; + out->vol_cur_l = chan_volume(voice_reg(idx, 0), &sweep_voice_env[idx][0]); + out->vol_cur_r = chan_volume(voice_reg(idx, 1), &sweep_voice_env[idx][1]); } /* Debug peek into SPU RAM (spu_ram TCP command). Returns bytes copied. */ @@ -922,6 +1508,25 @@ void spu_get_global_state(SpuGlobalState* out) { for (int i = 0; i < SPU_VOICE_COUNT; i++) if (voices[i].active) am |= (1u << i); out->active_mask = am; + + /* ---- SPU DSP fidelity state (issue #103) ---- */ + out->irq_flag = irq_flag; + out->reverb_on = (out->ctrl >> 7) & 1u; + out->irq_addr = ((uint32_t)spu_regs[reg_index(0x1F801DA4u)]) << 3; + out->reverb_mbase = rev_mbase(); + out->reverb_cur = rev_cur; + out->capture_pos = capture_pos; + out->noise_lfsr = noise_lfsr; + out->noise_pad = 0; + uint32_t sl = 0, sr = 0; + for (int i = 0; i < SPU_VOICE_COUNT; i++) { + if (voice_reg(i, 0) & 0x8000u) sl |= (1u << i); + if (voice_reg(i, 1) & 0x8000u) sr |= (1u << i); + } + out->sweep_l_mask = sl; + out->sweep_r_mask = sr; + out->sweep_main = ((spu_regs[reg_index(0x1F801D80u)] >> 15) & 1u) + | (((spu_regs[reg_index(0x1F801D82u)] >> 15) & 1u) << 1); } uint64_t spu_event_total(void) { return s_event_seq; } @@ -948,11 +1553,15 @@ void spu_event_reset(void) { /* ---- boot snapshot: complete SPU register + voice state (LE field wire) ---- */ #include "pst_wire.h" -/* SpuVoice host sizeof has padding; wire is packed LE fields (94 bytes). */ +/* SpuVoice host sizeof has padding; wire is packed LE fields. 94 bytes of + * classic voice state + 12 bytes of per-voice volume-sweep envelope state + * (L then R: int16 level + uint32 divider each) = 106 bytes. */ #define SPU_VOICE_WIRE_BYTES ( \ - 4u + 4u + 4u + (28u * 2u) + (3u * 2u) + 4u + 4u + 2u + 2u + 1u + 2u + 4u + 1u) + 4u + 4u + 4u + (28u * 2u) + (3u * 2u) + 4u + 4u + 2u + 2u + 1u + 2u + 4u + 1u \ + + 2u * (2u + 4u)) -static int spu_w_voice(PstW *w, const SpuVoice *v) { +static int spu_w_voice(PstW *w, int idx) { + const SpuVoice *v = &voices[idx]; if (!pst_w_i32(w, (int32_t)v->active) || !pst_w_u32(w, v->cur_addr) || !pst_w_u32(w, v->repeat_addr)) return 0; @@ -960,12 +1569,19 @@ static int spu_w_voice(PstW *w, const SpuVoice *v) { if (!pst_w_i16(w, v->samples[i])) return 0; for (int i = 0; i < 3; i++) if (!pst_w_i16(w, v->previous_samples[i])) return 0; - return pst_w_i32(w, (int32_t)v->sample_idx) && pst_w_u32(w, v->phase) && - pst_w_i16(w, v->hist1) && pst_w_i16(w, v->hist2) && pst_w_u8(w, v->flags) && - pst_w_u16(w, v->env_level) && pst_w_u32(w, v->adsr_divider) && - pst_w_u8(w, v->adsr_phase); + if (!(pst_w_i32(w, (int32_t)v->sample_idx) && pst_w_u32(w, v->phase) && + pst_w_i16(w, v->hist1) && pst_w_i16(w, v->hist2) && pst_w_u8(w, v->flags) && + pst_w_u16(w, v->env_level) && pst_w_u32(w, v->adsr_divider) && + pst_w_u8(w, v->adsr_phase))) + return 0; + for (int ch = 0; ch < 2; ch++) + if (!pst_w_i16(w, sweep_voice_env[idx][ch].level) || + !pst_w_u32(w, sweep_voice_env[idx][ch].divider)) + return 0; + return 1; } -static int spu_r_voice(PstR *r, SpuVoice *v) { +static int spu_r_voice(PstR *r, int idx) { + SpuVoice *v = &voices[idx]; int32_t active = 0, sample_idx = 0; if (!pst_r_i32(r, &active) || !pst_r_u32(r, &v->cur_addr) || !pst_r_u32(r, &v->repeat_addr)) @@ -981,12 +1597,22 @@ static int spu_r_voice(PstR *r, SpuVoice *v) { !pst_r_u32(r, &v->adsr_divider) || !pst_r_u8(r, &v->adsr_phase)) return 0; v->sample_idx = (int)sample_idx; + for (int ch = 0; ch < 2; ch++) + if (!pst_r_i16(r, &sweep_voice_env[idx][ch].level) || + !pst_r_u32(r, &sweep_voice_env[idx][ch].divider)) + return 0; return 1; } +/* Global trailer: 20 classic bytes + 44 bytes of DSP fidelity state + * (IRQ latch, noise LFSR + divider, reverb address/phase/boundary state, + * capture position, main L/R sweep envelopes). */ +#define SPU_SNAPSHOT_TAIL_BYTES \ + (20u + 1u + 2u + 4u + 4u + 1u + 4u + 4u + 4u + 4u + 4u + 2u * (2u + 4u)) + uint32_t spu_snapshot_bytes(void) { return (uint32_t)(SPU_REG_COUNT * 2u) + - (SPU_VOICE_COUNT * SPU_VOICE_WIRE_BYTES) + 20u; + (SPU_VOICE_COUNT * SPU_VOICE_WIRE_BYTES) + SPU_SNAPSHOT_TAIL_BYTES; } void spu_snapshot_write(uint8_t *p) { @@ -996,12 +1622,26 @@ void spu_snapshot_write(uint8_t *p) { for (uint32_t i = 0; i < SPU_REG_COUNT; i++) pst_w_u16(&w, spu_regs[i]); for (int i = 0; i < SPU_VOICE_COUNT; i++) - spu_w_voice(&w, &voices[i]); + spu_w_voice(&w, i); pst_w_u32(&w, transfer_addr); pst_w_u32(&w, key_on_count); pst_w_u32(&w, endx_latch); pst_w_u32(&w, kon_latch); pst_w_u32(&w, koff_latch); + pst_w_u8(&w, irq_flag); + pst_w_u16(&w, noise_lfsr); + pst_w_i32(&w, noise_timer); + pst_w_u32(&w, rev_cur); + pst_w_u8(&w, rev_phase); + pst_w_i32(&w, rev_in_hold_l); + pst_w_i32(&w, rev_in_hold_r); + pst_w_i32(&w, rev_out_l); + pst_w_i32(&w, rev_out_r); + pst_w_u32(&w, capture_pos); + for (int ch = 0; ch < 2; ch++) { + pst_w_i16(&w, sweep_main_env[ch].level); + pst_w_u32(&w, sweep_main_env[ch].divider); + } } int spu_snapshot_read(const uint8_t *p, uint32_t len) { @@ -1011,11 +1651,23 @@ int spu_snapshot_read(const uint8_t *p, uint32_t len) { for (uint32_t i = 0; i < SPU_REG_COUNT; i++) if (!pst_r_u16(&r, &spu_regs[i])) return 0; for (int i = 0; i < SPU_VOICE_COUNT; i++) - if (!spu_r_voice(&r, &voices[i])) return 0; + if (!spu_r_voice(&r, i)) return 0; if (!pst_r_u32(&r, &transfer_addr) || !pst_r_u32(&r, &key_on_count) || !pst_r_u32(&r, &endx_latch) || !pst_r_u32(&r, &kon_latch) || !pst_r_u32(&r, &koff_latch)) return 0; + if (!pst_r_u8(&r, &irq_flag) || !pst_r_u16(&r, &noise_lfsr) || + !pst_r_i32(&r, &noise_timer) || !pst_r_u32(&r, &rev_cur) || + !pst_r_u8(&r, &rev_phase) || + !pst_r_i32(&r, &rev_in_hold_l) || !pst_r_i32(&r, &rev_in_hold_r) || + !pst_r_i32(&r, &rev_out_l) || !pst_r_i32(&r, &rev_out_r) || + !pst_r_u32(&r, &capture_pos)) + return 0; + for (int ch = 0; ch < 2; ch++) { + if (!pst_r_i16(&r, &sweep_main_env[ch].level) || + !pst_r_u32(&r, &sweep_main_env[ch].divider)) + return 0; + } return 1; } uint8_t* spu_get_ram_ptr(void){ return spu_ram; } diff --git a/runtime/src/traps.c b/runtime/src/traps.c index 77402276f..bba37844c 100644 --- a/runtime/src/traps.c +++ b/runtime/src/traps.c @@ -24,6 +24,7 @@ static uint32_t traps_parity_rw(void* ctx, uint32_t addr) { /* Forward declarations from interrupts.c */ int psx_get_in_exception(void); void psx_exception_longjmp(void); +void psx_irq_refresh_cause_ip2(void); /* ── Deterministic TCB scheduler — SCAFFOLDING (plan steps 1-2, INERT) ─────── * These definitions back psx_scheduler.h. They are NOT yet wired into the live @@ -340,7 +341,14 @@ static uint32_t psx_restore_context_from_tcb(CPUState* cpu, uint32_t tcb) uint32_t saved_sr = cpu->read_word(save + 140u); cpu->cop0[12] = (saved_sr & 0xFFFFFFC0u) | ((saved_sr >> 2) & 0x0Fu); } + /* Restore the saved CAUSE, then re-derive IP2. On hardware the IP field is + * live interrupt-line state and is never storage, so it cannot meaningfully + * be saved and restored: a context captured while an interrupt was pending + * and restored after the ack would re-latch a phantom IP2 out of guest + * memory. Reload the architecturally-saved bits, then let the single owner + * recompute bit 10 from the actual line. */ cpu->cop0[13] = cpu->read_word(save + 144u); + psx_irq_refresh_cause_ip2(); cpu->gpr[26] = cpu->read_word(save + 128u); psx_assert_no_sentinel_pc("restore_context_from_tcb", tcb, cpu->gpr[26]); thread_ctx_ring_log(cpu, tcb, cpu->gpr[26], 1); diff --git a/runtime/tests/test_cause_ip2_combinational.c b/runtime/tests/test_cause_ip2_combinational.c new file mode 100644 index 000000000..241713b7b --- /dev/null +++ b/runtime/tests/test_cause_ip2_combinational.c @@ -0,0 +1,185 @@ +/* + * Validate that COP0.CAUSE bit 10 (IP2) behaves as a COMBINATIONAL mirror of + * the interrupt-controller line, not as a latch. + * + * On R3000A the Cause.IP field is not storage — it reflects the current state + * of the interrupt input pins. On the PSX only IP2 is wired, and it carries the + * INTC output, i.e. (I_STAT & I_MASK) != 0. So IP2 must RISE when a device + * raises and FALL the instant the guest acks I_STAT or masks the source, with + * no CPU involvement in either direction. + * + * This was previously wrong in two independent places: both the compiled + * delivery path (interrupts.c) and the standalone interpreter delivery path + * (psx_interpreter.c) only ever OR'd bit 10 in at delivery and never cleared + * it. A kernel exception dispatcher that loops on CAUSE.IP & SR.IM to decide + * whether to service again then sees a pending interrupt that no longer exists. + * + * Cross-checked against the independent Beetle oracle rather than asserted: + * beetle-psx/mednafen/psx/irq.cpp recomputes the line as + * AssertIRQ(0, (Status & Mask)) at power-on, at every assert, and at BOTH the + * status-ack and mask-write halves of a register write; cpu.cpp's AssertIRQ + * clears bit (10+n) unconditionally before re-setting it from the level. + * + * The mirror under test is deliberately tiny and depends only on i_stat/i_mask + * plus a caller-supplied CAUSE pointer, so it is exercised directly here rather + * than by linking the whole interrupt subsystem. + * + * Build: + * cc -std=c99 -Wall -Wextra -Werror \ + * -o test_cause_ip2_combinational test_cause_ip2_combinational.c + */ +#include +#include + +/* ---- The unit under test ------------------------------------------------- + * Mirrors runtime/src/interrupts.c. Kept in lockstep with it; if the two ever + * disagree this test is the thing that should be updated last, not first. */ +uint32_t i_stat; +uint32_t i_mask; + +static uint32_t *s_cause_ptr; + +static void psx_irq_refresh_cause_ip2(void) +{ + if (!s_cause_ptr) return; + if ((i_stat & i_mask & 0x7FFu) != 0u) + *s_cause_ptr |= (1u << 10); + else + *s_cause_ptr &= ~(1u << 10); +} + +static void psx_irq_set_cause_ptr(uint32_t *p) +{ + s_cause_ptr = p; + psx_irq_refresh_cause_ip2(); +} + +static void psx_irq_raise(uint32_t bit) +{ + i_stat |= (1u << bit); + psx_irq_refresh_cause_ip2(); +} + +/* Mirrors memory.c's interrupt_write_stat_masked: writing I_STAT ACKS bits — + * a zero bit in the written value clears the corresponding pending bit. */ +static void write_i_stat(uint32_t val, uint32_t mask) +{ + uint32_t ack_mask = mask & 0x7FFu; + i_stat = (i_stat & ~ack_mask) | (i_stat & val & ack_mask); + psx_irq_refresh_cause_ip2(); +} + +static void write_i_mask(uint32_t val, uint32_t mask) +{ + i_mask = ((i_mask & ~mask) | (val & mask)) & 0x7FFu; + psx_irq_refresh_cause_ip2(); +} + +/* ---- Harness ------------------------------------------------------------ */ +static int failures; + +#define IP2(cause) (((cause) >> 10) & 1u) + +#define CHECK(cond, label) do { \ + if (!(cond)) { \ + fprintf(stderr, "FAIL: %s\n", (label)); \ + failures++; \ + } \ +} while (0) + +#define IRQ_VBLANK 0 +#define IRQ_SPU 9 + +int main(void) +{ + uint32_t cause; + + /* --- power-on recompute (Beetle's IRQ_Power -> Recalc) --------------- */ + i_stat = 0; i_mask = 0; + cause = 0xFFFFFFFFu; /* worst case: bit 10 already set from junk state */ + psx_irq_set_cause_ptr(&cause); + CHECK(IP2(cause) == 0, "wiring the mirror recomputes IP2 at power-on"); + + /* --- a raise on a MASKED source must not assert the line ------------- */ + i_stat = 0; i_mask = 0; cause = 0; + psx_irq_set_cause_ptr(&cause); + psx_irq_raise(IRQ_VBLANK); + CHECK(i_stat == 1u, "raise sets the I_STAT bit regardless of mask"); + CHECK(IP2(cause) == 0, "raise with the source masked leaves IP2 clear"); + + /* --- unmasking an already-pending source asserts the line ----------- */ + write_i_mask(0x0001u, 0xFFFFu); + CHECK(IP2(cause) == 1, "unmasking a pending source raises IP2"); + + /* --- re-masking drops it again, with I_STAT untouched ---------------- */ + write_i_mask(0x0000u, 0xFFFFu); + CHECK(i_stat == 1u, "masking does not clear the pending I_STAT bit"); + CHECK(IP2(cause) == 0, "masking a pending source drops IP2"); + + /* --- raise on an UNMASKED source asserts immediately ----------------- */ + i_stat = 0; i_mask = 0x0001u; cause = 0; + psx_irq_set_cause_ptr(&cause); + CHECK(IP2(cause) == 0, "no pending source means IP2 clear"); + psx_irq_raise(IRQ_VBLANK); + CHECK(IP2(cause) == 1, "raise on an unmasked source raises IP2"); + + /* --- THE REGRESSION: acking I_STAT must drop IP2 --------------------- */ + /* Writing 0 to a pending bit acknowledges it. Before the fix, bit 10 was + * latched at delivery and survived this, so a dispatcher looping on + * CAUSE.IP & SR.IM re-serviced an interrupt that no longer existed. */ + write_i_stat(0x0000u, 0xFFFFu); + CHECK(i_stat == 0u, "writing zero acks the pending bit"); + CHECK(IP2(cause) == 0, "acking the last pending source drops IP2"); + + /* --- a partial ack leaves the line up while anything remains --------- */ + i_stat = 0; i_mask = (1u << IRQ_VBLANK) | (1u << IRQ_SPU); cause = 0; + psx_irq_set_cause_ptr(&cause); + psx_irq_raise(IRQ_VBLANK); + psx_irq_raise(IRQ_SPU); + CHECK(IP2(cause) == 1, "two pending sources raise IP2"); + write_i_stat(~(uint32_t)(1u << IRQ_VBLANK), 0xFFFFu); /* ack VBlank only */ + CHECK(i_stat == (1u << IRQ_SPU), "partial ack clears only the acked bit"); + CHECK(IP2(cause) == 1, "IP2 stays asserted while another source is pending"); + write_i_stat(0x0000u, 0xFFFFu); + CHECK(IP2(cause) == 0, "IP2 drops once the last source is acked"); + + /* --- the mirror must not disturb other CAUSE bits -------------------- */ + /* ExcCode, BD and the software-interrupt bits IP0/IP1 are all real state + * the guest or the exception path owns; only bit 10 belongs to the line. */ + i_stat = 0; i_mask = 0x0001u; + cause = 0x8000000Cu | (1u << 8) | (1u << 9); /* BD | ExcCode=3 | IP0 | IP1 */ + psx_irq_set_cause_ptr(&cause); + uint32_t preserved = cause & ~(1u << 10); + psx_irq_raise(IRQ_VBLANK); + CHECK(IP2(cause) == 1, "raise asserts IP2 alongside existing CAUSE bits"); + CHECK((cause & ~(1u << 10)) == preserved, + "asserting IP2 preserves ExcCode / BD / software-interrupt bits"); + write_i_stat(0x0000u, 0xFFFFu); + CHECK((cause & ~(1u << 10)) == preserved, + "clearing IP2 preserves ExcCode / BD / software-interrupt bits"); + + /* --- byte-width register writes must still recompute ----------------- */ + /* I_STAT/I_MASK are byte- and halfword-writable; the mask argument is how + * partial writes are expressed, and every one of them can move the line. */ + i_stat = 0; i_mask = 0; cause = 0; + psx_irq_set_cause_ptr(&cause); + psx_irq_raise(IRQ_SPU); /* bit 9 — in the high byte */ + CHECK(IP2(cause) == 0, "masked SPU raise leaves IP2 clear"); + write_i_mask(0x0200u, 0xFF00u); /* halfword-high write unmasks it */ + CHECK(IP2(cause) == 1, "a partial-width mask write still recomputes IP2"); + + /* --- bits above the 11-bit INTC range never drive the line ---------- */ + i_stat = 0; i_mask = 0; cause = 0; + psx_irq_set_cause_ptr(&cause); + i_stat = 0xF800u; /* outside the 0x7FF INTC range */ + i_mask = 0xF800u; + psx_irq_refresh_cause_ip2(); + CHECK(IP2(cause) == 0, "out-of-range I_STAT/I_MASK bits do not assert IP2"); + + if (failures) { + fprintf(stderr, "FAILED (%d)\n", failures); + return 1; + } + printf("test_cause_ip2_combinational: all checks passed\n"); + return 0; +} diff --git a/runtime/tests/test_spu_fidelity.c b/runtime/tests/test_spu_fidelity.c new file mode 100644 index 000000000..beedfed62 --- /dev/null +++ b/runtime/tests/test_spu_fidelity.c @@ -0,0 +1,584 @@ +/* + * test_spu_fidelity.c — clean-room unit tests for the SPU DSP fidelity + * features (GitHub issue #103): SPU IRQ (I_STAT bit 9), reverb, the noise + * generator, volume sweeps, the capture buffers, and savestate coverage of + * all the new state. + * + * Harness pattern: includes spu.c directly (white-box — the tests assert on + * internal engine state like rev_cur and the sweep envelopes as well as on + * guest-visible register reads and rendered PCM) and stubs the few externs + * spu.c pulls in. Written from scratch against the psx-spx hardware + * documentation; no emulator source consulted. + */ +#include +#include +#include +#include +#include + +#include "../src/spu.c" + +/* ---- stubs for spu.c externs -------------------------------------------- */ +uint64_t s_frame_count = 0; + +static uint32_t g_irq_raises = 0; +static uint32_t g_irq_last_bit = 0; +static uint32_t g_irq_last_detail = 0; +void psx_irq_raise(uint32_t bit, uint32_t detail) { + g_irq_raises++; + g_irq_last_bit = bit; + g_irq_last_detail = detail; +} + +uint64_t psx_get_cycle_count(void) { return 0; } +void audio_trace_pcm(int tap, const int16_t *stereo, int frames) { + (void)tap; (void)stereo; (void)frames; +} +void audio_trace_event(uint16_t kind, uint32_t a, uint32_t b) { + (void)kind; (void)a; (void)b; +} +bool spu_shadow_enabled(void) { return false; } +void spu_shadow_reset(void) {} +void spu_shadow_process(int16_t *canon, int frames) { (void)canon; (void)frames; } + +/* ---- tiny check harness -------------------------------------------------- */ +static int g_checks = 0; +static int g_fails = 0; +#define CHECK(cond) do { \ + g_checks++; \ + if (!(cond)) { \ + g_fails++; \ + printf("FAIL %s:%d: %s\n", __FILE__, __LINE__, #cond); \ + } \ +} while (0) + +/* ---- helpers -------------------------------------------------------------- */ +#define R_SPUCNT 0x1F801DAAu +#define R_SPUSTAT 0x1F801DAEu +#define R_IRQADDR 0x1F801DA4u +#define R_XFERADDR 0x1F801DA6u +#define R_FIFO 0x1F801DA8u +#define R_KON_LO 0x1F801D88u +#define R_NON_LO 0x1F801D94u +#define R_EON_LO 0x1F801D98u +#define R_MAINVOLL 0x1F801D80u +#define R_MAINVOLR 0x1F801D82u +#define R_CDVOLL 0x1F801DB0u +#define R_CDVOLR 0x1F801DB2u + +static void wr(uint32_t addr, uint16_t v) { spu_write(addr, v); } +static uint16_t rd(uint32_t addr) { return (uint16_t)spu_read(addr); } + +static int16_t g_render_buf[4096 * 2]; +static void render_n(int frames) { + while (frames > 0) { + int n = frames > 4096 ? 4096 : frames; + spu_render(g_render_buf, n); + frames -= n; + } +} + +static void push_cd_const(int frames, int16_t l, int16_t r) { + static int16_t buf[1024 * 2]; + while (frames > 0) { + int n = frames > 1024 ? 1024 : frames; + for (int i = 0; i < n; i++) { buf[i * 2] = l; buf[i * 2 + 1] = r; } + spu_cd_audio_push(buf, n); + frames -= n; + } +} + +/* voice register byte addresses: base + 2*reg (reg: 0=VOLL 1=VOLR 2=PITCH + * 3=START 4=ADSRLO 5=ADSRHI 6=CURVOL 7=LOOP) */ +static uint32_t vreg(int v, int reg) { + return 0x1F801C00u + (uint32_t)(v * 8 + reg) * 2u; +} + +static void setup_voice(int v, uint16_t start_unit, uint16_t voll, uint16_t volr) { + wr(vreg(v, 0), voll); + wr(vreg(v, 1), volr); + wr(vreg(v, 2), 0x1000); /* pitch 1.0 */ + wr(vreg(v, 3), start_unit); + wr(vreg(v, 4), 0x0000); /* Ar=0 (fastest attack) */ + wr(vreg(v, 5), 0x0000); +} + +static int ram_region_zero(uint32_t start, uint32_t end) { + for (uint32_t a = start; a < end; a++) + if (spu_ram[a]) return 0; + return 1; +} + +static int buf_has_nonzero(const int16_t *buf, int samples) { + for (int i = 0; i < samples; i++) + if (buf[i]) return 1; + return 0; +} + +/* ==== 1. SPU IRQ ============================================================ */ + +/* FIFO, DMA-write, DMA-read, transfer-address-write and SPUCNT-write access + * sites; latch/ack/re-arm behaviour; SPUSTAT bit 6 mirror. */ +static void test_irq_transfer_paths(void) { + spu_init(); + g_irq_raises = 0; + + /* Program the IRQ address and transfer address BEFORE enabling the IRQ: + * enabling with both still at their reset value of 0 is itself a + * (correct) unit-0 match under the enable-time re-check. */ + wr(R_SPUCNT, 0x8000); /* SPU enable, IRQ still off */ + wr(R_IRQADDR, 0x0600); /* IRQ at byte 0x3000 */ + wr(R_XFERADDR, 0x05FF); /* transfer addr byte 0x2FF8: no match */ + wr(R_SPUCNT, 0x8040); /* IRQ enable; resting unit 0x5FF: no fire */ + CHECK(g_irq_raises == 0); + + /* DMA writes advance 0x2FF8 -> 0x3000; the third word touches 0x3000. */ + spu_dma_write(0xAABBCCDDu); + spu_dma_write(0x11223344u); + CHECK(g_irq_raises == 0); + spu_dma_write(0x55667788u); + CHECK(g_irq_raises == 1); + CHECK(g_irq_last_bit == 9); + CHECK(g_irq_last_detail == 0x3000u); + CHECK((rd(R_SPUSTAT) & 0x40u) != 0); + + /* Latched: another hit does not re-raise until acknowledged. */ + spu_dma_write(0x99999999u); /* 0x3004..0x3007, still unit 0x600 */ + CHECK(g_irq_raises == 1); + + /* Acknowledge: SPUCNT write with bit 6 clear drops SPUSTAT bit 6. */ + wr(R_SPUCNT, 0x8000); + CHECK((rd(R_SPUSTAT) & 0x40u) == 0); + + /* Re-arm with transfer address resting at a NON-matching unit. */ + wr(R_SPUCNT, 0x8040); /* transfer now 0x3008 (unit 0x601): no fire */ + CHECK(g_irq_raises == 1); + + /* Transfer-address write pointing AT the IRQ address is a compare site. */ + wr(R_XFERADDR, 0x0600); + CHECK(g_irq_raises == 2); + CHECK((rd(R_SPUSTAT) & 0x40u) != 0); + + /* SPUCNT write with bit 6 set is a compare site too (transfer address + * still resting on the IRQ unit). */ + wr(R_SPUCNT, 0x8000); /* ack */ + wr(R_SPUCNT, 0x8040); /* re-enable -> immediate re-hit */ + CHECK(g_irq_raises == 3); + + /* Manual FIFO writes: park below the IRQ unit while IRQ is acked, walk + * up to it, and the store that touches byte 0x3000 fires. */ + wr(R_SPUCNT, 0x8000); /* ack + disable */ + wr(R_XFERADDR, 0x05FF); /* byte 0x2FF8 */ + wr(R_SPUCNT, 0x8040); /* unit 0x5FF: no immediate fire */ + CHECK(g_irq_raises == 3); + wr(R_FIFO, 0x1111); /* 0x2FF8 */ + wr(R_FIFO, 0x2222); /* 0x2FFA */ + wr(R_FIFO, 0x3333); /* 0x2FFC */ + wr(R_FIFO, 0x4444); /* 0x2FFE */ + CHECK(g_irq_raises == 3); + wr(R_FIFO, 0x5555); /* 0x3000 -> fire */ + CHECK(g_irq_raises == 4); + + /* DMA read direction: data comes back LE and the access fires the IRQ. */ + wr(R_SPUCNT, 0x8000); /* ack + IRQ off (no checks while moving) */ + wr(R_XFERADDR, 0x05FF); /* byte 0x2FF8 */ + CHECK(spu_dma_read() == 0x22221111u); /* FIFO halfwords 1111,2222 */ + CHECK(spu_dma_read() == 0x44443333u); /* FIFO halfwords 3333,4444 */ + wr(R_IRQADDR, 0x0601); /* byte 0x3008 */ + wr(R_SPUCNT, 0x8040); /* transfer at 0x3000 (unit 0x600): no fire */ + CHECK(g_irq_raises == 4); + /* 0x3000: FIFO halfword 5555 over the DMA word 0x55667788's top half. */ + CHECK(spu_dma_read() == 0x55665555u); + CHECK(g_irq_raises == 4); + (void)spu_dma_read(); /* 0x3004 */ + CHECK(g_irq_raises == 4); + (void)spu_dma_read(); /* 0x3008 -> fire */ + CHECK(g_irq_raises == 5); + CHECK(g_irq_last_detail == 0x3008u); +} + +/* SPUCNT bit 6 clear => no IRQ, ever, from any access. */ +static void test_irq_disabled_never_fires(void) { + spu_init(); + g_irq_raises = 0; + + wr(R_SPUCNT, 0x8000); /* SPU enable, IRQ DISABLED */ + wr(R_IRQADDR, 0x0600); + wr(R_XFERADDR, 0x0600); + spu_dma_write(0x12345678u); + wr(R_XFERADDR, 0x0600); + wr(R_FIFO, 0xBEEF); + render_n(16); /* capture + reverb accesses also silent */ + CHECK(g_irq_raises == 0); + CHECK((rd(R_SPUSTAT) & 0x40u) == 0); +} + +/* Voice ADPCM block fetch is a compare site. */ +static void test_irq_on_block_fetch(void) { + spu_init(); + g_irq_raises = 0; + + wr(R_SPUCNT, 0x8000); + wr(R_IRQADDR, 0x0600); /* byte 0x3000 */ + wr(R_SPUCNT, 0x8040); /* enable IRQ after aiming it */ + CHECK(g_irq_raises == 0); + setup_voice(0, 0x0600, 0x3FFF, 0x3FFF); + wr(R_KON_LO, 0x0001); + render_n(2); /* first sample fetches the block at 0x3000 */ + CHECK(g_irq_raises == 1); + CHECK(g_irq_last_detail == 0x3000u); +} + +/* Capture-buffer writes are compare sites (IRQ parked in the CD-R ring). */ +static void test_irq_on_capture_write(void) { + spu_init(); + g_irq_raises = 0; + + wr(R_SPUCNT, 0x8000); + wr(R_IRQADDR, 0x0080); /* byte 0x400: CD-R capture ring, offset 0 */ + wr(R_SPUCNT, 0x8040); + CHECK(g_irq_raises == 0); + render_n(1); + CHECK(g_irq_raises == 1); + CHECK(g_irq_last_detail == 0x400u); +} + +/* Reverb work-area accesses are compare sites (reads happen even with the + * reverb master write-enable OFF). */ +static void test_irq_on_reverb_access(void) { + spu_init(); + g_irq_raises = 0; + + wr(R_SPUCNT, 0x8000); /* enable; SPUCNT.7 (reverb) stays OFF */ + wr(R_IRQADDR, 0xFFF0); /* byte 0x7FF80 */ + wr(0x1F801DA2u, 0xFFF0); /* mBASE -> work area [0x7FF80, 0x80000) */ + wr(R_SPUCNT, 0x8040); /* IRQ on; resting transfer unit 0: no fire */ + CHECK(g_irq_raises == 0); + render_n(2); /* one 22050 Hz step -> reads at the base */ + CHECK(g_irq_raises == 1); +} + +/* ==== 2. Reverb ============================================================= */ + +static void setup_basic_reverb(void) { + wr(0x1F801DA2u, 0x2000); /* mBASE: byte 0x10000 */ + wr(SPU_R_VLIN, 0x7FFF); + wr(SPU_R_VRIN, 0x7FFF); + wr(SPU_R_VIIR, 0x4000); + wr(SPU_R_VWALL, 0x4000); + wr(SPU_R_MLSAME, 0x0004); /* write tap: +32 bytes */ + wr(SPU_R_MRSAME, 0x0104); + wr(SPU_R_DLSAME, 0x0006); + wr(SPU_R_DRSAME, 0x0106); + wr(SPU_R_MLCOMB1, 0x0002); /* read tap: +16 bytes (8 steps behind) */ + wr(SPU_R_MRCOMB1, 0x0102); + wr(SPU_R_VCOMB1, 0x4000); + wr(SPU_R_VLOUT, 0x7FFF); + wr(SPU_R_VROUT, 0x7FFF); + wr(R_CDVOLL, 0x7FFF); + wr(R_CDVOLR, 0x7FFF); + wr(R_MAINVOLL, 0x3FFF); + wr(R_MAINVOLR, 0x3FFF); +} + +/* Work-area WRITES are suppressed while SPUCNT.7 is clear and happen when it + * is set — driven from the CD-audio-only path (zero active voices), which is + * exactly the FMV case the old fast path silently skipped. */ +static void test_reverb_write_gating_and_cd_path(void) { + spu_init(); + g_irq_raises = 0; + + /* SPU enable + CD enable + CD reverb send; reverb write-enable OFF. */ + wr(R_SPUCNT, 0x8005); + setup_basic_reverb(); + + for (int v = 0; v < 24; v++) CHECK(!voices[v].active); + + push_cd_const(64, 0x2000, 0x2000); + render_n(64); + /* Reads happened, but nothing may have been written to the work area. */ + CHECK(ram_region_zero(0x10000u, 0x10800u)); + + /* Now enable reverb writes (SPUCNT bit 7). */ + wr(R_SPUCNT, 0x8085); + push_cd_const(64, 0x2000, 0x2000); + render_n(64); + CHECK(!ram_region_zero(0x10000u, 0x10800u)); + + /* Reverb tail: CD input stops; the wet path must keep the output alive + * (this is the reverb-on-CD-only / FMV behaviour in one assert). */ + memset(g_render_buf, 0, sizeof(g_render_buf)); + spu_render(g_render_buf, 64); + CHECK(buf_has_nonzero(g_render_buf, 64 * 2)); +} + +/* Reverb addressing wraps inside [mBASE, 0x80000) and never escapes it, + * whatever the offset registers hold. */ +static void test_reverb_addressing_wrap(void) { + spu_init(); + + wr(R_SPUCNT, 0x8085); /* enable + CD + reverb writes (no IRQ) */ + wr(0x1F801DA2u, 0xFFF0); /* mBASE byte 0x7FF80: 128-byte area */ + wr(SPU_R_VLIN, 0x7FFF); + wr(SPU_R_VRIN, 0x7FFF); + wr(SPU_R_VIIR, 0x4000); + wr(SPU_R_VWALL, 0x4000); + wr(SPU_R_MLSAME, 0x0020); /* +0x100 bytes: exceeds the area, must wrap */ + wr(SPU_R_MRSAME, 0x0030); + wr(SPU_R_DLSAME, 0x0040); + wr(SPU_R_DRSAME, 0x0050); + wr(SPU_R_MLAPF1, 0x1234); /* arbitrary large offsets */ + wr(SPU_R_MRAPF1, 0x4321); + wr(SPU_R_MLAPF2, 0x7FFF); + wr(SPU_R_MRAPF2, 0x0FFF); + wr(SPU_R_DAPF1, 0x3000); + wr(SPU_R_DAPF2, 0x0800); + wr(SPU_R_VAPF1, 0x3000); + wr(SPU_R_VAPF2, 0x3000); + wr(SPU_R_VLOUT, 0x7FFF); + wr(SPU_R_VROUT, 0x7FFF); + wr(R_CDVOLL, 0x7FFF); + wr(R_CDVOLR, 0x7FFF); + + /* Every effective address the engine can form stays inside the area. */ + static const uint32_t addr_regs[] = { + SPU_R_MLSAME, SPU_R_MRSAME, SPU_R_DLSAME, SPU_R_DRSAME, + SPU_R_MLAPF1, SPU_R_MRAPF1, SPU_R_MLAPF2, SPU_R_MRAPF2, + }; + for (unsigned i = 0; i < sizeof(addr_regs) / sizeof(addr_regs[0]); i++) { + uint32_t ea0 = rev_ea(addr_regs[i], 0); + uint32_t eam = rev_ea(addr_regs[i], -2); + uint32_t eap = rev_ea(addr_regs[i], -((int32_t)RREG(SPU_R_DAPF1) << 3)); + CHECK(ea0 >= 0x7FF80u && ea0 < 0x80000u); + CHECK(eam >= 0x7FF80u && eam < 0x80000u); + CHECK(eap >= 0x7FF80u && eap < 0x80000u); + } + + /* Run long enough to lap the 128-byte area several times. */ + push_cd_const(256, 0x3000, -0x3000); + render_n(256); + CHECK(rev_cur >= 0x7FF80u && rev_cur < 0x80000u); + /* Nothing below the work area was touched (capture rings stay <0x1000). */ + CHECK(ram_region_zero(0x7F000u, 0x7FF80u)); + /* The area itself was written. */ + CHECK(!ram_region_zero(0x7FF80u, 0x80000u)); +} + +/* ==== 3. Noise generator ==================================================== */ + +static void test_noise_generator(void) { + /* Control: same voice in ADPCM mode over all-zero SPU RAM is silent. */ + spu_init(); + wr(R_SPUCNT, 0xB400); /* enable; noise shift=13, step=4 */ + wr(R_MAINVOLL, 0x3FFF); + wr(R_MAINVOLR, 0x3FFF); + setup_voice(0, 0x0600, 0x3FFF, 0x3FFF); + wr(R_KON_LO, 0x0001); + memset(g_render_buf, 0, sizeof(g_render_buf)); + spu_render(g_render_buf, 512); + CHECK(!buf_has_nonzero(g_render_buf, 512 * 2)); + + /* Noise mode: same setup + NON bit -> non-silent, non-constant output. */ + spu_init(); + wr(R_SPUCNT, 0xB400); + wr(R_MAINVOLL, 0x3FFF); + wr(R_MAINVOLR, 0x3FFF); + setup_voice(0, 0x0600, 0x3FFF, 0x3FFF); + wr(R_NON_LO, 0x0001); + wr(R_KON_LO, 0x0001); + memset(g_render_buf, 0, sizeof(g_render_buf)); + spu_render(g_render_buf, 512); + CHECK(buf_has_nonzero(g_render_buf, 512 * 2)); + + /* Count distinct left-channel values: the LFSR must keep moving. */ + int distinct = 0; + for (int i = 0; i < 512; i++) { + int seen = 0; + for (int j = 0; j < i; j++) + if (g_render_buf[j * 2] == g_render_buf[i * 2]) { seen = 1; break; } + if (!seen) distinct++; + } + CHECK(distinct >= 8); + CHECK(noise_lfsr != 0); /* self-seeded from 0 via the xor-1 term */ +} + +/* ==== 4. Volume sweeps + direct decode ===================================== */ + +static void test_direct_volume_decode(void) { + spu_init(); + /* Documented decode: effective volume = signed bits14-0 << 1. */ + wr(vreg(0, 0), 0x2000); + CHECK(sweep_voice_env[0][0].level == 0x4000); + wr(vreg(0, 0), 0x3FFF); + CHECK(sweep_voice_env[0][0].level == 0x7FFE); + wr(vreg(0, 0), 0x4001); /* negative: (int16)(0x4001<<1) = -32766 */ + CHECK(sweep_voice_env[0][0].level == -32766); + wr(vreg(0, 0), 0x0000); + CHECK(sweep_voice_env[0][0].level == 0); + + /* End-to-end gain pipeline: CD sample 0x1000 x CD vol 0x7FFF x main + * 0x3FFF(direct => 0x7FFE) = 4094. Locks the >>15 application shape. */ + spu_init(); + wr(R_SPUCNT, 0x8001); /* enable + CD audio */ + wr(R_CDVOLL, 0x7FFF); + wr(R_CDVOLR, 0x7FFF); + wr(R_MAINVOLL, 0x3FFF); + wr(R_MAINVOLR, 0x3FFF); + push_cd_const(1, 0x1000, 0x1000); + spu_render(g_render_buf, 1); + CHECK(g_render_buf[0] == 4094); + CHECK(g_render_buf[1] == 4094); +} + +static void test_sweep_rise_and_fall(void) { + spu_init(); + wr(R_SPUCNT, 0x8000); + + /* Linear increase sweep, rate 0x20, from level 0. */ + wr(vreg(0, 0), 0x8000 | 0x0020); + render_n(64); + int16_t mid = sweep_voice_env[0][0].level; + CHECK(mid > 0); + CHECK((int16_t)rd(vreg(0, 0)) == mid); /* guest read sees the live level */ + render_n(64); + int16_t late = sweep_voice_env[0][0].level; + CHECK(late > mid); + + /* Decrease sweep starts from the current (direct-set) level and falls. */ + wr(vreg(0, 1), 0x3FFF); /* direct: level 0x7FFE */ + CHECK(sweep_voice_env[0][1].level == 0x7FFE); + wr(vreg(0, 1), 0x8000 | 0x2000 | 0x0020); /* sweep, decrease, rate 0x20 */ + render_n(64); + int16_t fall_mid = sweep_voice_env[0][1].level; + CHECK(fall_mid < 0x7FFE); + CHECK(fall_mid >= 0); + render_n(64); + int16_t fall_late = sweep_voice_env[0][1].level; + CHECK(fall_late < fall_mid); + CHECK(fall_late >= 0); + + /* Main-volume sweeps use the same machinery. */ + wr(R_MAINVOLL, 0x8000 | 0x0020); + render_n(64); + CHECK(sweep_main_env[0].level > 0); + /* Current-main-volume mirror register reports the live level. */ + CHECK((int16_t)rd(0x1F801DB8u) == sweep_main_env[0].level); +} + +/* Capture rings advance one halfword per output sample, wrap at 0x400, and + * record the raw CD input bus. */ +static void test_capture_buffer_contents(void) { + spu_init(); + wr(R_SPUCNT, 0x8000); /* SPU enable; CD MIX disabled — the + capture rings record the bus anyway */ + push_cd_const(4, 0x1234, 0x2345); + render_n(4); + for (int i = 0; i < 4; i++) { + uint16_t l = (uint16_t)(spu_ram[i * 2] | (spu_ram[i * 2 + 1] << 8)); + uint16_t r = (uint16_t)(spu_ram[0x400 + i * 2] | (spu_ram[0x400 + i * 2 + 1] << 8)); + CHECK(l == 0x1234); + CHECK(r == 0x2345); + } + CHECK(capture_pos == 8); + + render_n(508); /* 512 total -> exactly one wrap */ + CHECK(capture_pos == 0); + CHECK((rd(R_SPUSTAT) & 0x800u) == 0); /* first half again */ + render_n(256); + CHECK(capture_pos == 0x200); + CHECK((rd(R_SPUSTAT) & 0x800u) != 0); /* second half */ +} + +/* ==== 5. Savestate round-trip =============================================== */ + +static void test_savestate_roundtrip(void) { + spu_init(); + g_irq_raises = 0; + + /* Busy state: reverb ringing, noise voice keyed, sweeps mid-glide, + * capture advanced, IRQ latched. */ + wr(R_SPUCNT, 0xB485); /* enable + noise(13,4) + reverb + CD + CD-rev */ + wr(R_IRQADDR, 0x0080); /* park the IRQ in the CD-R capture ring */ + wr(R_SPUCNT, 0xB4C5); /* + IRQ enable -> first render latches it */ + setup_basic_reverb(); + setup_voice(5, 0x0600, 0x3FFF, 0x3FFF); + wr(R_NON_LO, 0x0020); /* voice 5 is a noise voice */ + wr(R_EON_LO, 0x0020); /* voice 5 also feeds reverb */ + wr(vreg(6, 0), 0x8000 | 0x0020); /* an idle voice's sweep still glides */ + wr(R_KON_LO, 0x0020); + push_cd_const(128, 0x1800, -0x1800); + render_n(128); /* ring drained exactly; state is rich */ + CHECK(irq_flag == 1); + + uint32_t n = spu_snapshot_bytes(); + uint8_t *snap = (uint8_t *)malloc(n); + CHECK(snap != NULL); + spu_snapshot_write(snap); + /* The runtime savestate persists the 512KB SPU RAM separately (via + * spu_get_ram_ptr); mirror that here — the reverb work area and capture + * rings live in RAM and timeline B must replay over identical bytes. */ + uint8_t *ram_snap = (uint8_t *)malloc(SPU_RAM_SIZE); + CHECK(ram_snap != NULL); + memcpy(ram_snap, spu_ram, SPU_RAM_SIZE); + + uint32_t rev_cur_0 = rev_cur; + uint8_t rev_phase_0 = rev_phase; + uint16_t noise_lfsr_0 = noise_lfsr; + int32_t noise_timer_0 = noise_timer; + uint32_t capture_pos_0 = capture_pos; + uint8_t irq_flag_0 = irq_flag; + int16_t sweep_lvl_0 = sweep_voice_env[6][0].level; + uint32_t sweep_div_0 = sweep_voice_env[6][0].divider; + int32_t rev_out_l_0 = rev_out_l; + + /* Timeline A. */ + static int16_t out_a[256 * 2], out_b[256 * 2]; + spu_render(out_a, 256); + CHECK(buf_has_nonzero(out_a, 256 * 2)); /* noise + reverb tail audible */ + + /* State moved on... */ + CHECK(rev_cur != rev_cur_0 || noise_lfsr != noise_lfsr_0 + || capture_pos != capture_pos_0); + + /* ...restore and replay: timeline B must be byte-identical. */ + CHECK(spu_snapshot_read(snap, n) == 1); + memcpy(spu_ram, ram_snap, SPU_RAM_SIZE); + CHECK(rev_cur == rev_cur_0); + CHECK(rev_phase == rev_phase_0); + CHECK(noise_lfsr == noise_lfsr_0); + CHECK(noise_timer == noise_timer_0); + CHECK(capture_pos == capture_pos_0); + CHECK(irq_flag == irq_flag_0); + CHECK(sweep_voice_env[6][0].level == sweep_lvl_0); + CHECK(sweep_voice_env[6][0].divider == sweep_div_0); + CHECK(rev_out_l == rev_out_l_0); + + spu_render(out_b, 256); + CHECK(memcmp(out_a, out_b, sizeof(out_a)) == 0); + + /* A truncated blob is rejected. */ + CHECK(spu_snapshot_read(snap, n - 1) == 0); + + free(ram_snap); + free(snap); +} + +/* ==== main ================================================================== */ + +int main(void) { + test_irq_transfer_paths(); + test_irq_disabled_never_fires(); + test_irq_on_block_fetch(); + test_irq_on_capture_write(); + test_irq_on_reverb_access(); + test_reverb_write_gating_and_cd_path(); + test_reverb_addressing_wrap(); + test_noise_generator(); + test_direct_volume_decode(); + test_sweep_rise_and_fall(); + test_capture_buffer_contents(); + test_savestate_roundtrip(); + + printf("test_spu_fidelity: %d checks, %d failures\n", g_checks, g_fails); + return g_fails ? 1 : 0; +} From fe518e303125b227a4f6f34327c3d1d1439d045b Mon Sep 17 00:00:00 2001 From: Matthew Stanley <1379tech@gmail.com> Date: Wed, 5 Aug 2026 16:38:02 -0700 Subject: [PATCH 2/4] docs: record SPU fidelity validation results and merge prerequisites Measured, not asserted: - BIOS LLE boot proves the reverb engine runs and rings down after the dry signal stops; noise LFSR observed shifting with correct parity feedback. - Tomba 2 intro FMV A/B against the exact parent commit shows the added per-frame DSP work is performance-neutral (min p50 40.0 vs 40.3 fps) and does not starve the CD bus or the host sink. The FMV's 40 fps and its ~19/s host underruns are PRE-EXISTING on master. - Records all ten places the documentation ran out as an oracle-comparison work list, and the two merge prerequisites (rebase onto moved master; bump recomp-ui to 99558ee or no title compiles). Co-Authored-By: Claude Opus 5 (1M context) --- docs/internal/SPU_FIDELITY_103.md | 95 ++++++++++++++++++++++++++++--- 1 file changed, 88 insertions(+), 7 deletions(-) diff --git a/docs/internal/SPU_FIDELITY_103.md b/docs/internal/SPU_FIDELITY_103.md index c596e3897..31360f637 100644 --- a/docs/internal/SPU_FIDELITY_103.md +++ b/docs/internal/SPU_FIDELITY_103.md @@ -225,19 +225,100 @@ Recorded explicitly rather than presented as exact (Rule 14). ## 6. Implementation judgement calls -_Populated from the implementation pass; each entry is an oracle-comparison -candidate rather than a settled fact._ +Every point where the documentation ran out. Each is an oracle-comparison +candidate, NOT a settled fact. Listed so a later measurement pass has a work +list instead of having to rediscover them. + +1. **Reverb 22.05 ↔ 44.1 kHz boundary** (the flagged known gap): input is the + box average of the frame pair, output is linear interpolation between + successive engine results. Isolated in `rev_reconstruct()` plus one call + site. **The one deliberate deviation from hardware.** +2. **Noise divider wiring**: down-counter decremented by step `4 + SPUCNT[9:8]` + per 44.1 kHz cycle, reloaded with `0x20000 >> shift`. The least-documented + part of the noise generator. +3. **Register-write IRQ compare sites**: the transfer-address write, the + IRQ-address write and an SPUCNT-enable re-check all compare against the + *resting* transfer address immediately. Consequence: enabling the IRQ while + both registers are 0 fires immediately (unit 0 == unit 0). Real drivers set + the address first. +4. **Suppressed reverb writes** (SPUCNT bit 7 clear) do not run the IRQ compare, + since no access occurs. Reads always do. +5. **Sweep negative-phase semantics**: the envelope works on a 0..0x7FFF value + in the phase's domain; wrong-sign levels clamp to 0 first. A sweep-mode + register read returns the live level as signed 16-bit. +6. **Reverb send taps**: post-envelope and post-voice-volume for EON voices; + post-CD-volume for the CD send. +7. **Capture values**: CD captured pre-CD-volume (raw input bus), voice 1/3 + post-envelope pre-voice-volume. The CD bus now **pops continuously while the + SPU is enabled** (SPUCNT bit 0 gates only mix/reverb), needed for capture + correctness. This is a behaviour change — measured as *not* causing + starvation, see §7. +8. **Mix saturation order**: the summed bus (dry + CD + wet) saturates to 16-bit + *before* main volume, per the documented mixer order. Replaces previous + unclamped, int32-overflow-prone math. Audible only in loud scenes. +9. **All-disabled gating**: capture / noise / reverb / sweeps run only while + SPUCNT bit 15 is set; whether they free-run with the SPU disabled is + undocumented. Noise clocks after the voice walk within a frame. +10. **Shadow tap and `SpuDebugInfo` volume fields** keep the historical 1.14 + scale (`level >> 1`) so `spu_shadow.c` and old captures stay byte-compatible + for direct-mode volumes. ## 7. Validation status - [x] `test_cause_ip2_combinational` passes (`-Wall -Wextra -Werror`). -- [x] Changed C translation units compile clean. -- [ ] `test_spu_fidelity` passes. -- [ ] `psx-runtime` links; BIOS boots; boot chime audible and non-distorted. -- [ ] Title run: no regression, reverb present. -- [ ] Oracle audio comparison against `psx-beetle` at a fixed scene. +- [x] `test_spu_fidelity` passes — 137 checks, 0 failures. Independently re-run, + not taken from the implementation report. +- [x] All changed translation units compile clean; `psx-runtime` and + `Tomba2Recomp` both link. +- [x] **BIOS LLE boot proves the reverb engine runs.** The BIOS itself sets + SPUCNT bit 7 (`ctrl = 0xC085`) and allocates a 62 KB work area at + `mBASE = 0x70940`. Sampled through the boot chime: the work area holds + 256/256 non-zero reflection halfwords peaking at 24681, `reverb_cur` + advances and laps correctly inside `[mBASE, 0x80000)`, and after the dry + signal goes silent (output peak → 0 at t≈6.8 s) the work area keeps + ringing down (628 → 18 → 2) for ~3 s. A decaying tail that outlives its + input is the reverb working. Noise LFSR observed shifting with correct + parity feedback (`000F → 001F → … → FFA3`). +- [x] **Tomba 2 intro FMV: A/B against the exact parent commit.** Same worktree, + framework at `c88c3ca9^` vs `c88c3ca9`, identical generated code + (SPU work is runtime-only). + + | Metric | Baseline | SPU branch | + | --- | --- | --- | + | FPS drop | 59.9 → 40.7 at t=6.2 s | 59.9 → 40.4 at t=6.2 s | + | min p50 fps | 40.3 | 40.0 | + | `cd_underflow_frames` | 441, flat | 441, flat | + | host underruns @32 s | 594 | 582 | + | audio peak | 23066 | 26876 | + | `reverb_cur`/`noise_lfsr` | absent (control) | live | + + Conclusions: performance is a wash (the FMV's 40 fps and its ~19/s host + underruns are **pre-existing on master** — worth their own issue, not + caused here); the continuous CD-bus drain does **not** starve the bus; and + the peak rising 23066 → 26876 is wet signal adding on top of dry, still + well under 32767, so not clipping. FMV video confirmed correct by + screenshot through the debug server (320x224, intro scene). +- [ ] Oracle audio comparison against `psx-beetle` at a fixed scene — settles + the §6 items, especially (1). `audio_wav`/`audio_stats` exist on both + ports, so this is a symmetric always-on ring query. **Not yet done.** - [ ] User ear-validation (final gate). +## 9. Merge prerequisites + +- **Rebase required.** This branch is based on `7f7fbc8e`; master moved during + the session (observed `9217ae2`, alpha-258). Re-fetch and rebase; do not + merge stale. +- **`recomp-ui` must be bumped in lockstep.** Framework master's + `feat(mods): expose linked package attribution` needs recomp-ui `99558ee` + (`feat(mods): link package authors and sources`), which adds `author_links` / + `author_link_count` / `source_name` / `source_url` to + `RecompLauncherCModPackage`. Tomba 2's pin at `854ae10` is one commit short + and **fails to compile** with `'RecompLauncherCModPackage' has no member named + 'author_link_count'`. This is unrelated to the SPU work but blocks any title + build against current master. +- Every title needs regen + revalidation, because CAUSE.IP2 touches core IRQ + delivery for all of them. + ## 8. Incidental fix — `tools/embed_spirv.py` build race Not SPU-related, found while building. Two runtime targets (`psx-runtime`, From 7e1421be7cfd892234555e67e9045a5134dd4e4f Mon Sep 17 00:00:00 2001 From: Matthew Stanley <1379tech@gmail.com> Date: Wed, 5 Aug 2026 17:06:20 -0700 Subject: [PATCH 3/4] docs: correct the Tomba 2 perf measurement (first attempt was interp-bound) The first FMV A/B was run on a fresh worktree whose overlay shard cache was cold AND whose autocompile could never work: game.toml points at psxrecomp-v4/recompiler/build-t2/psxrecomp-game.exe while the documented build recipe builds the recompiler at build-recompiler/ in the worktree root, so it failed 8/8 with WinError 2 and the runtime silently ran interpreted (dispatch_native = 0). The '40 fps / ~19 underruns per second' attributed to master in the previous commit was that broken setup, not master. Re-measured against the fully warm cache with the pre-SPU exe as a control in the same worktree: interp_share 0.7813 vs 0.7775, exc_share 0.2386 vs 0.2381, 39.1 vs 38.0-38.4 fps. Identical within noise, so the DSP work is free. Separately characterises the real pre-existing defect: the FMV is 96% native by dispatch count but 78% interpreted by wall clock with 24% in exception handling, even warm. Co-Authored-By: Claude Opus 5 (1M context) --- docs/internal/SPU_FIDELITY_103.md | 683 +++++++++++++++--------------- 1 file changed, 351 insertions(+), 332 deletions(-) diff --git a/docs/internal/SPU_FIDELITY_103.md b/docs/internal/SPU_FIDELITY_103.md index 31360f637..cb4aabc1c 100644 --- a/docs/internal/SPU_FIDELITY_103.md +++ b/docs/internal/SPU_FIDELITY_103.md @@ -1,332 +1,351 @@ -# SPU fidelity burndown — Issue #103 and PR #102 - -Branch: `feat/spu-fidelity-103` · Worktree: `_wt-spu-103` · Base: `origin/master` `7f7fbc8e` - -Closes the four gaps named in Issue #103 (reverb, noise, volume sweeps, SPU IRQ) -and lands the separable, verified parts of PR #102. Written so the disposition of -each piece is auditable rather than asserted. - ---- - -## 1. Scope overlap between #102 and #103 - -They intersect on exactly one item. - -| Item | Issue #103 | PR #102 | Disposition | -| --- | --- | --- | --- | -| Reverb | ✔ asked | — | Implemented here, clean-room (§3) | -| Noise LFSR | ✔ asked | — | Implemented here, clean-room (§3) | -| Volume sweeps | ✔ asked | — | Implemented here, clean-room (§3) | -| SPU IRQ | ✔ asked | ✔ offered | **One implementation**, ours, in `spu.c` | -| CAUSE.IP2 combinational | — | ✔ offered | Taken, verified, extended (§2) | -| Mid-dispatch audio pump | — | ✔ offered | Taken, with a regression fixed (§2) | -| SPU RAM DMA readback | — | ✔ offered | Taken as-is in substance (§2) | - -So this is one coherent change, not two competing ones. PR #102 covers 1 of -#103's 4 items; the other three were never in it. - -## 2. PR #102 — per-piece verdict - -PR #102's four changes were reviewed as four separable items, because they carry -very different risk. Attribution to Alexandros Mandravillis is recorded in the -commit trailer and in the source comments; the code here is a reimplementation -on the same findings, because two of the PR's three commits touch `spu.c` and -would have collided with the clean-room work. - -### 2a. CAUSE.IP2 combinational — CONFIRMED, taken, and extended - -The PR's claim is that `CAUSE.IP2` is a live mirror of the interrupt line rather -than a latched bit. **The claim is correct**, and it was checked against the -in-tree Beetle oracle rather than accepted on the PR's reasoning: - -- `beetle-psx/mednafen/psx/irq.cpp` defines - `#define Recalc() PSX_CPU->AssertIRQ(0, (bool)(Status & Mask))` - and calls it from `IRQ_Assert` (raise), from **both** halves of `IRQ_Write` - (the Status ack and the Mask write), and from `IRQ_Power`. -- `beetle-psx/mednafen/psx/cpu.cpp:289 AssertIRQ()` clears `CAUSE` bit `10+n` - unconditionally, then re-sets it only if the level is asserted. - -That is the definition of a combinational mirror, and it matches R3000A: the -`Cause.IP` field is not storage, it reflects the interrupt pins. Our runtime -previously only ever OR'd bit 10 in at delivery and never cleared it. - -Extensions beyond the PR: - -1. **A second latching writer the PR missed.** `runtime/src/psx_interpreter.c` - had its own `cpu->cop0[COP0_CAUSE] |= (1u << 10);` at interrupt delivery — - the identical defect in the standalone interpreter path. Fixed too. An audit - of every writer of `cop0[13]` confirms no other site touches bit 10: the - interpreter's `MTC0` paths correctly mask guest writes to `0x0300` - (software-interrupt bits only), and the exception paths mask `~0x7C` / - `~0x8000007C`, preserving the IP field. -2. **Single ownership.** The compiled delivery path no longer sets bit 10 - itself; `psx_irq_refresh_cause_ip2()` is the only writer. Two writers of one - combinational bit is how the bug survived in the first place. -3. **Power-on recompute**, mirroring Beetle's `IRQ_Power() -> Recalc()`. -4. **A regression test**, `runtime/tests/test_cause_ip2_combinational.c`, - covering rise, fall on ack, fall on mask, partial ack with another source - still pending, partial-width register writes, preservation of ExcCode/BD/IP0/ - IP1, and out-of-range I_STAT bits. Registered in CTest. - -Risk note: this is a real behavioural change to core IRQ delivery for every -title, and it is the highest-risk item in this branch. It is nonetheless the -faithful behaviour, so per Rule -1 it lands and titles are revalidated rather -than the fix being narrowed. - -### 2b. Mid-dispatch audio pump — taken, with a regression the PR would have shipped - -The reasoning is sound: the SPU is autonomous on hardware, our pump is driven -from the main loop between presented frames, so a guest busy-wait that never -presents a frame freezes SPU time — self-deadlocking for any game waiting on an -SPU-generated condition. The VBlank edge is derived from the guest cycle -counter, so it keeps firing through such a wait and is a defensible place to -pump. Confirmed single-threaded: `spu_render` runs on the main loop thread -(`sdl_audio_pump` is the producer; the SDL callback only drains an SPSC ring), -so this introduces no data race on `i_stat`/`cop0`. - -**Defect found and fixed.** PR #102 wires the hook to `sdl_audio_pump()` -unconditionally. But `sdl_audio_update()` is the sole authority on whether a -pump should emit, discard, or not run: - -- hard mute (turbo loads) → *does not pump at all*; the queue drains and voice - positions freeze in place, so music resumes where it left off instead of - replaying time-compressed garble. This is user-validated behaviour. -- turbo sink → pumps with `discard_output = true`. - -Pumping unconditionally from the VBlank edge bypasses both, which would push -real audio during every turbo load. The hook now goes through -`sdl_audio_pump_midframe()`, which mirrors the gate state -(`AUDIO_GATE_NORMAL` / `MUTED` / `SINK`) that `sdl_audio_update()` last set. - -Known tension, recorded rather than guessed at: in the `MUTED` state SPU time -still freezes, so a game that busy-waits on an SPU condition *during* a turbo -load would stall. No title in our suite is known to do this. Advancing SPU time -under mute would change validated mute audio, so it is not being done -speculatively. - -Placement: the pump call sits at the **end** of `fire_vblank_edge()` so the -VBlank's own raise and ring records complete first — the pump can itself raise -an SPU IRQ, which should land after the edge rather than interleaved into it. - -### 2c. SPU RAM DMA readback — taken - -DMA4 in the SPU→RAM direction previously wrote literal zeros. SPU RAM is -readable memory; this was not a transfer at all. Titles that carry a checksummed -block through SPU RAM across an `Exec` boundary (SPU RAM being one of the few -regions a main-RAM reload does not touch) read zeros, failed their own integrity -check, and fell back to a cold-boot path. Lowest-risk item in the PR: -previously-dead path, no existing behaviour depends on the zeros. - -### 2d. Validation claims — NOT reproduced, and cannot be - -PR #102 validates against Medal of Honor Underground (SLUS-01270) and Gran -Turismo (SCUS-94194). Neither disc is in our suite. Those specific claims are -**unverified by us and should not be repeated as if they were**. What we -verified independently is the hardware behaviour behind each change (§2a–2c) and -non-regression across our own titles. - -## 3. Issue #103 — why this had to be clean-room - -⚠ **This is the load-bearing constraint on the whole item, and it is why #103 -sat open.** - -`psxrecomp` is distributed under **PolyForm Noncommercial 1.0.0**. Beetle PSX / -mednafen is **GPL-2.0-or-later**. They are incompatible: GPL source cannot be -folded into a PolyForm distribution. Beetle's tree contains a complete, correct, -well-tested reverb — so the obvious implementation route is closed, and two -contributors independently walked into it: - -- **PR #16** (Martin Penkava) implemented all of this by porting Beetle. Parked - on license grounds after a line-by-line audit found it tracked - `beetle-psx/mednafen/psx/spu.cpp` lines 590–730 in register layout, resampling - coefficients, buffer layout, arithmetic and processing order. See - `docs/internal/upstream/martin-pr16-spu-reverb.md` on branch - `audit/pr16-spu-reverb-gpl-lineage-mpenkava`. -- **PR #13** (parked at `5fc8e15d`) shipped a reverb too, rejected for being - *wrong* rather than for licensing: it gated reverb on bit 15 of `0x1F801DC0`, - which is `dAPF1` — an all-pass *offset* register whose top bit is set for any - offset ≥ `0x8000`, so reverb switched on and off according to an address - value. The real gate is SPUCNT (`0x1F801DAA`) bit 7. It also read - `dAPF2` / `vIIR` / `vCOMB1` as "feedback" / "wet level" / "early-reflection - level"; they are none of those. - -The PR #16 audit names the only acceptable path, and this branch takes it: **a -clean-room implementation from hardware documentation, with independently -derived tests.** The psx-spx / nocash register map and documented algorithm were -the source. Beetle's source was deliberately not consulted for the -implementation. - -Beetle remains fully usable as a **runtime oracle** — running the binary and -comparing its audio output is not a derivative work, and that comparison is how -the remaining uncertainties in §5 get settled. `audio_wav` and `audio_stats` -exist on both `psx-runtime` and `psx-beetle`, so the comparison is a symmetric -always-on ring query, not an arm-then-capture. - -### Pre-existing lineage exposure — reported, not changed - -Separate from this work, and worth a decision: - -- `runtime/src/spu.c` `calc_vc_delta()` is commented *"Ported verbatim from - Beetle's CalcVCDelta"*. This is the ADSR rate decoder, and it is in every - released binary. Same category as PR #16, already shipped. -- `runtime/include/spu_gauss.h` cites *"No$PSX docs / DuckStation - core/spu.cpp"*. - -Deliberately left alone here: rewriting the ADSR rate decoder clean-room changes -every voice envelope in every title and needs its own revalidation campaign, not -a rider on this branch. - -## 4. `feat/pr13-salvage-remainder` is fully superseded — no surgery needed - -Prior notes flagged this branch as "ready to land" while Issue #103 says its -reverb must stay parked, and treated reconciling that as a blocker. **It is not -a blocker: the branch retains nothing but the rejected reverb commit.** - -`git cherry -v origin/master feat/pr13-salvage-remainder`: - -``` -- e9a8a24e interp: deliver alignment exceptions, protect Cause read-only bits, add CFC0 -- 3ce30155 dma: implement channel 5 (PIO / expansion port) transfers -+ 5fc8e15d spu: reverb, noise LFSR, sweep volumes, pitch modulation, SPU IRQ -- 9f3cd0a3 autocompile: name the resolved interpreter at configure, warn on Cygwin builds -``` - -Three of four are already upstream (`-`). Verified four independent ways rather -than trusting patch-id alone: - -- cherry-picking all three onto current `origin/master` produced **empty** - commits; -- `execute_ch5_pio()` is present in `runtime/src/dma.c` on master; -- `CFC0` handling and the `& ~0x0300u | (val & 0x0300u)` Cause write-protection - are present in `runtime/src/dirty_ram_interp.c` on master; -- `git diff origin/master 9f3cd0a3 -- runtime/src/autocompile.c` is **empty**. - -Consequences: there is no salvage left to preserve, no git surgery to perform, -and the "two competing SPU IRQ implementations" question resolves by default — -`5fc8e15d` is both superseded and Beetle-derived. The branch should be -abandoned and PR #13 closed. `origin/park/pr13-spu-reverb-rejected` still -retains the commit, so nothing is lost. - -Incidentally, `e9a8a24e`'s Cause write-protection (guest `MTC0` may only write -bits 8–9) is exactly what makes §2a safe: the guest cannot forge IP2. - -## 5. Known deviations and the oracle-verification queue - -Recorded explicitly rather than presented as exact (Rule 14). - -- **22.05 → 44.1 kHz reverb reconstruction filter.** The reverb engine runs at - 22050 Hz, but the hardware's reconstruction filter is not specified in the - documentation used. Beetle's 39-tap FIR was deliberately not copied. The - implementation uses a clearly-marked, single-function reconstruction so the - filter can be swapped once measured against the oracle. **This is the one - place the implementation is known to differ from hardware.** -- Further per-feature judgement calls are listed in §6 as reported by the - implementation pass; each is a candidate for oracle comparison. - -## 6. Implementation judgement calls - -Every point where the documentation ran out. Each is an oracle-comparison -candidate, NOT a settled fact. Listed so a later measurement pass has a work -list instead of having to rediscover them. - -1. **Reverb 22.05 ↔ 44.1 kHz boundary** (the flagged known gap): input is the - box average of the frame pair, output is linear interpolation between - successive engine results. Isolated in `rev_reconstruct()` plus one call - site. **The one deliberate deviation from hardware.** -2. **Noise divider wiring**: down-counter decremented by step `4 + SPUCNT[9:8]` - per 44.1 kHz cycle, reloaded with `0x20000 >> shift`. The least-documented - part of the noise generator. -3. **Register-write IRQ compare sites**: the transfer-address write, the - IRQ-address write and an SPUCNT-enable re-check all compare against the - *resting* transfer address immediately. Consequence: enabling the IRQ while - both registers are 0 fires immediately (unit 0 == unit 0). Real drivers set - the address first. -4. **Suppressed reverb writes** (SPUCNT bit 7 clear) do not run the IRQ compare, - since no access occurs. Reads always do. -5. **Sweep negative-phase semantics**: the envelope works on a 0..0x7FFF value - in the phase's domain; wrong-sign levels clamp to 0 first. A sweep-mode - register read returns the live level as signed 16-bit. -6. **Reverb send taps**: post-envelope and post-voice-volume for EON voices; - post-CD-volume for the CD send. -7. **Capture values**: CD captured pre-CD-volume (raw input bus), voice 1/3 - post-envelope pre-voice-volume. The CD bus now **pops continuously while the - SPU is enabled** (SPUCNT bit 0 gates only mix/reverb), needed for capture - correctness. This is a behaviour change — measured as *not* causing - starvation, see §7. -8. **Mix saturation order**: the summed bus (dry + CD + wet) saturates to 16-bit - *before* main volume, per the documented mixer order. Replaces previous - unclamped, int32-overflow-prone math. Audible only in loud scenes. -9. **All-disabled gating**: capture / noise / reverb / sweeps run only while - SPUCNT bit 15 is set; whether they free-run with the SPU disabled is - undocumented. Noise clocks after the voice walk within a frame. -10. **Shadow tap and `SpuDebugInfo` volume fields** keep the historical 1.14 - scale (`level >> 1`) so `spu_shadow.c` and old captures stay byte-compatible - for direct-mode volumes. - -## 7. Validation status - -- [x] `test_cause_ip2_combinational` passes (`-Wall -Wextra -Werror`). -- [x] `test_spu_fidelity` passes — 137 checks, 0 failures. Independently re-run, - not taken from the implementation report. -- [x] All changed translation units compile clean; `psx-runtime` and - `Tomba2Recomp` both link. -- [x] **BIOS LLE boot proves the reverb engine runs.** The BIOS itself sets - SPUCNT bit 7 (`ctrl = 0xC085`) and allocates a 62 KB work area at - `mBASE = 0x70940`. Sampled through the boot chime: the work area holds - 256/256 non-zero reflection halfwords peaking at 24681, `reverb_cur` - advances and laps correctly inside `[mBASE, 0x80000)`, and after the dry - signal goes silent (output peak → 0 at t≈6.8 s) the work area keeps - ringing down (628 → 18 → 2) for ~3 s. A decaying tail that outlives its - input is the reverb working. Noise LFSR observed shifting with correct - parity feedback (`000F → 001F → … → FFA3`). -- [x] **Tomba 2 intro FMV: A/B against the exact parent commit.** Same worktree, - framework at `c88c3ca9^` vs `c88c3ca9`, identical generated code - (SPU work is runtime-only). - - | Metric | Baseline | SPU branch | - | --- | --- | --- | - | FPS drop | 59.9 → 40.7 at t=6.2 s | 59.9 → 40.4 at t=6.2 s | - | min p50 fps | 40.3 | 40.0 | - | `cd_underflow_frames` | 441, flat | 441, flat | - | host underruns @32 s | 594 | 582 | - | audio peak | 23066 | 26876 | - | `reverb_cur`/`noise_lfsr` | absent (control) | live | - - Conclusions: performance is a wash (the FMV's 40 fps and its ~19/s host - underruns are **pre-existing on master** — worth their own issue, not - caused here); the continuous CD-bus drain does **not** starve the bus; and - the peak rising 23066 → 26876 is wet signal adding on top of dry, still - well under 32767, so not clipping. FMV video confirmed correct by - screenshot through the debug server (320x224, intro scene). -- [ ] Oracle audio comparison against `psx-beetle` at a fixed scene — settles - the §6 items, especially (1). `audio_wav`/`audio_stats` exist on both - ports, so this is a symmetric always-on ring query. **Not yet done.** -- [ ] User ear-validation (final gate). - -## 9. Merge prerequisites - -- **Rebase required.** This branch is based on `7f7fbc8e`; master moved during - the session (observed `9217ae2`, alpha-258). Re-fetch and rebase; do not - merge stale. -- **`recomp-ui` must be bumped in lockstep.** Framework master's - `feat(mods): expose linked package attribution` needs recomp-ui `99558ee` - (`feat(mods): link package authors and sources`), which adds `author_links` / - `author_link_count` / `source_name` / `source_url` to - `RecompLauncherCModPackage`. Tomba 2's pin at `854ae10` is one commit short - and **fails to compile** with `'RecompLauncherCModPackage' has no member named - 'author_link_count'`. This is unrelated to the SPU work but blocks any title - build against current master. -- Every title needs regen + revalidation, because CAUSE.IP2 touches core IRQ - delivery for all of them. - -## 8. Incidental fix — `tools/embed_spirv.py` build race - -Not SPU-related, found while building. Two runtime targets (`psx-runtime`, -`psx-oracle`) embed the same shader set, so ninja runs `embed_spirv.py` -concurrently. It wrote its intermediate SPIR-V to `.spv` **in the source -tree**, so the two invocations raced on one path and whichever finished first -`os.remove()`d the file the other was about to read — a hard build failure -(`FileNotFoundError: .../blit.frag.spv`), and source-tree pollution besides. -Now compiled to a private `mkdtemp` directory, with the output header written -via atomic replace and a loud error if `glslc` reports success but produces no -file. Fixed per Rule 15 rather than worked around. +# SPU fidelity burndown — Issue #103 and PR #102 + +Branch: `feat/spu-fidelity-103` · Worktree: `_wt-spu-103` · Base: `origin/master` `7f7fbc8e` + +Closes the four gaps named in Issue #103 (reverb, noise, volume sweeps, SPU IRQ) +and lands the separable, verified parts of PR #102. Written so the disposition of +each piece is auditable rather than asserted. + +--- + +## 1. Scope overlap between #102 and #103 + +They intersect on exactly one item. + +| Item | Issue #103 | PR #102 | Disposition | +| --- | --- | --- | --- | +| Reverb | ✔ asked | — | Implemented here, clean-room (§3) | +| Noise LFSR | ✔ asked | — | Implemented here, clean-room (§3) | +| Volume sweeps | ✔ asked | — | Implemented here, clean-room (§3) | +| SPU IRQ | ✔ asked | ✔ offered | **One implementation**, ours, in `spu.c` | +| CAUSE.IP2 combinational | — | ✔ offered | Taken, verified, extended (§2) | +| Mid-dispatch audio pump | — | ✔ offered | Taken, with a regression fixed (§2) | +| SPU RAM DMA readback | — | ✔ offered | Taken as-is in substance (§2) | + +So this is one coherent change, not two competing ones. PR #102 covers 1 of +#103's 4 items; the other three were never in it. + +## 2. PR #102 — per-piece verdict + +PR #102's four changes were reviewed as four separable items, because they carry +very different risk. Attribution to Alexandros Mandravillis is recorded in the +commit trailer and in the source comments; the code here is a reimplementation +on the same findings, because two of the PR's three commits touch `spu.c` and +would have collided with the clean-room work. + +### 2a. CAUSE.IP2 combinational — CONFIRMED, taken, and extended + +The PR's claim is that `CAUSE.IP2` is a live mirror of the interrupt line rather +than a latched bit. **The claim is correct**, and it was checked against the +in-tree Beetle oracle rather than accepted on the PR's reasoning: + +- `beetle-psx/mednafen/psx/irq.cpp` defines + `#define Recalc() PSX_CPU->AssertIRQ(0, (bool)(Status & Mask))` + and calls it from `IRQ_Assert` (raise), from **both** halves of `IRQ_Write` + (the Status ack and the Mask write), and from `IRQ_Power`. +- `beetle-psx/mednafen/psx/cpu.cpp:289 AssertIRQ()` clears `CAUSE` bit `10+n` + unconditionally, then re-sets it only if the level is asserted. + +That is the definition of a combinational mirror, and it matches R3000A: the +`Cause.IP` field is not storage, it reflects the interrupt pins. Our runtime +previously only ever OR'd bit 10 in at delivery and never cleared it. + +Extensions beyond the PR: + +1. **A second latching writer the PR missed.** `runtime/src/psx_interpreter.c` + had its own `cpu->cop0[COP0_CAUSE] |= (1u << 10);` at interrupt delivery — + the identical defect in the standalone interpreter path. Fixed too. An audit + of every writer of `cop0[13]` confirms no other site touches bit 10: the + interpreter's `MTC0` paths correctly mask guest writes to `0x0300` + (software-interrupt bits only), and the exception paths mask `~0x7C` / + `~0x8000007C`, preserving the IP field. +2. **Single ownership.** The compiled delivery path no longer sets bit 10 + itself; `psx_irq_refresh_cause_ip2()` is the only writer. Two writers of one + combinational bit is how the bug survived in the first place. +3. **Power-on recompute**, mirroring Beetle's `IRQ_Power() -> Recalc()`. +4. **A regression test**, `runtime/tests/test_cause_ip2_combinational.c`, + covering rise, fall on ack, fall on mask, partial ack with another source + still pending, partial-width register writes, preservation of ExcCode/BD/IP0/ + IP1, and out-of-range I_STAT bits. Registered in CTest. + +Risk note: this is a real behavioural change to core IRQ delivery for every +title, and it is the highest-risk item in this branch. It is nonetheless the +faithful behaviour, so per Rule -1 it lands and titles are revalidated rather +than the fix being narrowed. + +### 2b. Mid-dispatch audio pump — taken, with a regression the PR would have shipped + +The reasoning is sound: the SPU is autonomous on hardware, our pump is driven +from the main loop between presented frames, so a guest busy-wait that never +presents a frame freezes SPU time — self-deadlocking for any game waiting on an +SPU-generated condition. The VBlank edge is derived from the guest cycle +counter, so it keeps firing through such a wait and is a defensible place to +pump. Confirmed single-threaded: `spu_render` runs on the main loop thread +(`sdl_audio_pump` is the producer; the SDL callback only drains an SPSC ring), +so this introduces no data race on `i_stat`/`cop0`. + +**Defect found and fixed.** PR #102 wires the hook to `sdl_audio_pump()` +unconditionally. But `sdl_audio_update()` is the sole authority on whether a +pump should emit, discard, or not run: + +- hard mute (turbo loads) → *does not pump at all*; the queue drains and voice + positions freeze in place, so music resumes where it left off instead of + replaying time-compressed garble. This is user-validated behaviour. +- turbo sink → pumps with `discard_output = true`. + +Pumping unconditionally from the VBlank edge bypasses both, which would push +real audio during every turbo load. The hook now goes through +`sdl_audio_pump_midframe()`, which mirrors the gate state +(`AUDIO_GATE_NORMAL` / `MUTED` / `SINK`) that `sdl_audio_update()` last set. + +Known tension, recorded rather than guessed at: in the `MUTED` state SPU time +still freezes, so a game that busy-waits on an SPU condition *during* a turbo +load would stall. No title in our suite is known to do this. Advancing SPU time +under mute would change validated mute audio, so it is not being done +speculatively. + +Placement: the pump call sits at the **end** of `fire_vblank_edge()` so the +VBlank's own raise and ring records complete first — the pump can itself raise +an SPU IRQ, which should land after the edge rather than interleaved into it. + +### 2c. SPU RAM DMA readback — taken + +DMA4 in the SPU→RAM direction previously wrote literal zeros. SPU RAM is +readable memory; this was not a transfer at all. Titles that carry a checksummed +block through SPU RAM across an `Exec` boundary (SPU RAM being one of the few +regions a main-RAM reload does not touch) read zeros, failed their own integrity +check, and fell back to a cold-boot path. Lowest-risk item in the PR: +previously-dead path, no existing behaviour depends on the zeros. + +### 2d. Validation claims — NOT reproduced, and cannot be + +PR #102 validates against Medal of Honor Underground (SLUS-01270) and Gran +Turismo (SCUS-94194). Neither disc is in our suite. Those specific claims are +**unverified by us and should not be repeated as if they were**. What we +verified independently is the hardware behaviour behind each change (§2a–2c) and +non-regression across our own titles. + +## 3. Issue #103 — why this had to be clean-room + +⚠ **This is the load-bearing constraint on the whole item, and it is why #103 +sat open.** + +`psxrecomp` is distributed under **PolyForm Noncommercial 1.0.0**. Beetle PSX / +mednafen is **GPL-2.0-or-later**. They are incompatible: GPL source cannot be +folded into a PolyForm distribution. Beetle's tree contains a complete, correct, +well-tested reverb — so the obvious implementation route is closed, and two +contributors independently walked into it: + +- **PR #16** (Martin Penkava) implemented all of this by porting Beetle. Parked + on license grounds after a line-by-line audit found it tracked + `beetle-psx/mednafen/psx/spu.cpp` lines 590–730 in register layout, resampling + coefficients, buffer layout, arithmetic and processing order. See + `docs/internal/upstream/martin-pr16-spu-reverb.md` on branch + `audit/pr16-spu-reverb-gpl-lineage-mpenkava`. +- **PR #13** (parked at `5fc8e15d`) shipped a reverb too, rejected for being + *wrong* rather than for licensing: it gated reverb on bit 15 of `0x1F801DC0`, + which is `dAPF1` — an all-pass *offset* register whose top bit is set for any + offset ≥ `0x8000`, so reverb switched on and off according to an address + value. The real gate is SPUCNT (`0x1F801DAA`) bit 7. It also read + `dAPF2` / `vIIR` / `vCOMB1` as "feedback" / "wet level" / "early-reflection + level"; they are none of those. + +The PR #16 audit names the only acceptable path, and this branch takes it: **a +clean-room implementation from hardware documentation, with independently +derived tests.** The psx-spx / nocash register map and documented algorithm were +the source. Beetle's source was deliberately not consulted for the +implementation. + +Beetle remains fully usable as a **runtime oracle** — running the binary and +comparing its audio output is not a derivative work, and that comparison is how +the remaining uncertainties in §5 get settled. `audio_wav` and `audio_stats` +exist on both `psx-runtime` and `psx-beetle`, so the comparison is a symmetric +always-on ring query, not an arm-then-capture. + +### Pre-existing lineage exposure — reported, not changed + +Separate from this work, and worth a decision: + +- `runtime/src/spu.c` `calc_vc_delta()` is commented *"Ported verbatim from + Beetle's CalcVCDelta"*. This is the ADSR rate decoder, and it is in every + released binary. Same category as PR #16, already shipped. +- `runtime/include/spu_gauss.h` cites *"No$PSX docs / DuckStation + core/spu.cpp"*. + +Deliberately left alone here: rewriting the ADSR rate decoder clean-room changes +every voice envelope in every title and needs its own revalidation campaign, not +a rider on this branch. + +## 4. `feat/pr13-salvage-remainder` is fully superseded — no surgery needed + +Prior notes flagged this branch as "ready to land" while Issue #103 says its +reverb must stay parked, and treated reconciling that as a blocker. **It is not +a blocker: the branch retains nothing but the rejected reverb commit.** + +`git cherry -v origin/master feat/pr13-salvage-remainder`: + +``` +- e9a8a24e interp: deliver alignment exceptions, protect Cause read-only bits, add CFC0 +- 3ce30155 dma: implement channel 5 (PIO / expansion port) transfers ++ 5fc8e15d spu: reverb, noise LFSR, sweep volumes, pitch modulation, SPU IRQ +- 9f3cd0a3 autocompile: name the resolved interpreter at configure, warn on Cygwin builds +``` + +Three of four are already upstream (`-`). Verified four independent ways rather +than trusting patch-id alone: + +- cherry-picking all three onto current `origin/master` produced **empty** + commits; +- `execute_ch5_pio()` is present in `runtime/src/dma.c` on master; +- `CFC0` handling and the `& ~0x0300u | (val & 0x0300u)` Cause write-protection + are present in `runtime/src/dirty_ram_interp.c` on master; +- `git diff origin/master 9f3cd0a3 -- runtime/src/autocompile.c` is **empty**. + +Consequences: there is no salvage left to preserve, no git surgery to perform, +and the "two competing SPU IRQ implementations" question resolves by default — +`5fc8e15d` is both superseded and Beetle-derived. The branch should be +abandoned and PR #13 closed. `origin/park/pr13-spu-reverb-rejected` still +retains the commit, so nothing is lost. + +Incidentally, `e9a8a24e`'s Cause write-protection (guest `MTC0` may only write +bits 8–9) is exactly what makes §2a safe: the guest cannot forge IP2. + +## 5. Known deviations and the oracle-verification queue + +Recorded explicitly rather than presented as exact (Rule 14). + +- **22.05 → 44.1 kHz reverb reconstruction filter.** The reverb engine runs at + 22050 Hz, but the hardware's reconstruction filter is not specified in the + documentation used. Beetle's 39-tap FIR was deliberately not copied. The + implementation uses a clearly-marked, single-function reconstruction so the + filter can be swapped once measured against the oracle. **This is the one + place the implementation is known to differ from hardware.** +- Further per-feature judgement calls are listed in §6 as reported by the + implementation pass; each is a candidate for oracle comparison. + +## 6. Implementation judgement calls + +Every point where the documentation ran out. Each is an oracle-comparison +candidate, NOT a settled fact. Listed so a later measurement pass has a work +list instead of having to rediscover them. + +1. **Reverb 22.05 ↔ 44.1 kHz boundary** (the flagged known gap): input is the + box average of the frame pair, output is linear interpolation between + successive engine results. Isolated in `rev_reconstruct()` plus one call + site. **The one deliberate deviation from hardware.** +2. **Noise divider wiring**: down-counter decremented by step `4 + SPUCNT[9:8]` + per 44.1 kHz cycle, reloaded with `0x20000 >> shift`. The least-documented + part of the noise generator. +3. **Register-write IRQ compare sites**: the transfer-address write, the + IRQ-address write and an SPUCNT-enable re-check all compare against the + *resting* transfer address immediately. Consequence: enabling the IRQ while + both registers are 0 fires immediately (unit 0 == unit 0). Real drivers set + the address first. +4. **Suppressed reverb writes** (SPUCNT bit 7 clear) do not run the IRQ compare, + since no access occurs. Reads always do. +5. **Sweep negative-phase semantics**: the envelope works on a 0..0x7FFF value + in the phase's domain; wrong-sign levels clamp to 0 first. A sweep-mode + register read returns the live level as signed 16-bit. +6. **Reverb send taps**: post-envelope and post-voice-volume for EON voices; + post-CD-volume for the CD send. +7. **Capture values**: CD captured pre-CD-volume (raw input bus), voice 1/3 + post-envelope pre-voice-volume. The CD bus now **pops continuously while the + SPU is enabled** (SPUCNT bit 0 gates only mix/reverb), needed for capture + correctness. This is a behaviour change — measured as *not* causing + starvation, see §7. +8. **Mix saturation order**: the summed bus (dry + CD + wet) saturates to 16-bit + *before* main volume, per the documented mixer order. Replaces previous + unclamped, int32-overflow-prone math. Audible only in loud scenes. +9. **All-disabled gating**: capture / noise / reverb / sweeps run only while + SPUCNT bit 15 is set; whether they free-run with the SPU disabled is + undocumented. Noise clocks after the voice walk within a frame. +10. **Shadow tap and `SpuDebugInfo` volume fields** keep the historical 1.14 + scale (`level >> 1`) so `spu_shadow.c` and old captures stay byte-compatible + for direct-mode volumes. + +## 7. Validation status + +- [x] `test_cause_ip2_combinational` passes (`-Wall -Wextra -Werror`). +- [x] `test_spu_fidelity` passes — 137 checks, 0 failures. Independently re-run, + not taken from the implementation report. +- [x] All changed translation units compile clean; `psx-runtime` and + `Tomba2Recomp` both link. +- [x] **BIOS LLE boot proves the reverb engine runs.** The BIOS itself sets + SPUCNT bit 7 (`ctrl = 0xC085`) and allocates a 62 KB work area at + `mBASE = 0x70940`. Sampled through the boot chime: the work area holds + 256/256 non-zero reflection halfwords peaking at 24681, `reverb_cur` + advances and laps correctly inside `[mBASE, 0x80000)`, and after the dry + signal goes silent (output peak → 0 at t≈6.8 s) the work area keeps + ringing down (628 → 18 → 2) for ~3 s. A decaying tail that outlives its + input is the reverb working. Noise LFSR observed shifting with correct + parity feedback (`000F → 001F → … → FFA3`). +- [x] **Tomba 2 intro FMV — measured twice, because the first attempt was on a + broken setup. Recording both so the mistake is not repeated.** + + *Attempt 1 (INVALID as an absolute measurement).* A fresh worktree has a + cold overlay shard cache, and `autocompile` failed 8/8 with `WinError 2`: + `game.toml` points at `psxrecomp-v4/recompiler/build-t2/psxrecomp-game.exe` + while the documented build recipe puts the recompiler at + `build-recompiler/` in the worktree root. No shards were ever built and the + runtime silently fell back to the interpreter (`dispatch_native = 0`, + `native_share = 0.0`). The resulting "40 fps, ~19 underruns/s" was my setup, + NOT a property of master. Fixed by building the recompiler at the path + `game.toml` expects; autocompile then reports `fails=0`, `shard_ok` rising, + and the cache grows. + + *Attempt 2 (valid).* Ran the SPU build against the fully warm cache in + `_wt-t2-measure`, then the original pre-SPU exe in the same worktree with + the same cache as a control: + + | Metric | Control (no SPU work) | SPU branch | + | --- | --- | --- | + | `native_share` (by time) | 0.1558 | 0.1704 | + | `interp_share` | 0.7813 | 0.7775 | + | `exc_share` | 0.2386 | 0.2381 | + | p50 fps | 39.1 | 38.0-38.4 | + | native/interp dispatch | 96.6% native | 95.9% native | + + Identical within noise, so the added per-frame DSP work costs nothing + measurable. Reverb/noise state observed live; peak rose 23066 -> 26876 + (wet adding to dry, still under 32767 so not clipping); `cd_underflow` + flat at 441 on both, so the continuous CD-bus drain does not starve it. + FMV video confirmed correct by debug-server screenshot (320x224). + + **Pre-existing defect characterised, not caused here:** Tomba 2's intro FMV + is 96% native by DISPATCH COUNT but 78% interpreted by WALL CLOCK, with 24% + of time in exception handling, giving ~39 fps and continuous host-sink + underruns even with a fully warm cache. A few interpreted blocks consume + most of the frame. Worth its own issue. + +- [ ] Oracle audio comparison against `psx-beetle` at a fixed scene — settles + the §6 items, especially (1). `audio_wav`/`audio_stats` exist on both + ports, so this is a symmetric always-on ring query. **Not yet done.** +- [ ] User ear-validation (final gate). + +## 9. Merge prerequisites + +- **Rebase required.** This branch is based on `7f7fbc8e`; master moved during + the session (observed `9217ae2`, alpha-258). Re-fetch and rebase; do not + merge stale. +- **`recomp-ui` must be bumped in lockstep.** Framework master's + `feat(mods): expose linked package attribution` needs recomp-ui `99558ee` + (`feat(mods): link package authors and sources`), which adds `author_links` / + `author_link_count` / `source_name` / `source_url` to + `RecompLauncherCModPackage`. Tomba 2's pin at `854ae10` is one commit short + and **fails to compile** with `'RecompLauncherCModPackage' has no member named + 'author_link_count'`. This is unrelated to the SPU work but blocks any title + build against current master. +- Every title needs regen + revalidation, because CAUSE.IP2 touches core IRQ + delivery for all of them. + +## 8. Incidental fix — `tools/embed_spirv.py` build race + +Not SPU-related, found while building. Two runtime targets (`psx-runtime`, +`psx-oracle`) embed the same shader set, so ninja runs `embed_spirv.py` +concurrently. It wrote its intermediate SPIR-V to `.spv` **in the source +tree**, so the two invocations raced on one path and whichever finished first +`os.remove()`d the file the other was about to read — a hard build failure +(`FileNotFoundError: .../blit.frag.spv`), and source-tree pollution besides. +Now compiled to a private `mkdtemp` directory, with the output header written +via atomic replace and a loud error if `glslc` reports success but produces no +file. Fixed per Rule 15 rather than worked around. From 66af5c8dfbf230dc65e18a2abced3e12c933e4eb Mon Sep 17 00:00:00 2001 From: Matthew Stanley <1379tech@gmail.com> Date: Wed, 5 Aug 2026 18:20:13 -0700 Subject: [PATCH 4/4] docs: drop our redundant embed_spirv fix, record the autocompile interp trap Master fixed the embed_spirv.py concurrency race independently and better (tempfile.TemporaryDirectory cleans up on early-return error paths too), so the rebase conflict was resolved in master's favour and our mkdtemp version removed. This branch stays SPU-only. Records the trap that invalidated the first Tomba 2 measurement: game.toml's overlay_autocompile_cmd names a recompiler path, and when it is absent autocompile fails every attempt, no shard is ever built, and the runtime silently interprets everything. Tomba 2 expects a worktree-relative path the documented build recipe does not produce; MMX6 hardcodes an absolute path into the main repo that works only by luck of local layout. Check autocompile_status before trusting any perf number from a fresh worktree. Co-Authored-By: Claude Opus 5 (1M context) --- docs/internal/SPU_FIDELITY_103.md | 40 ++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/docs/internal/SPU_FIDELITY_103.md b/docs/internal/SPU_FIDELITY_103.md index cb4aabc1c..1028853f4 100644 --- a/docs/internal/SPU_FIDELITY_103.md +++ b/docs/internal/SPU_FIDELITY_103.md @@ -338,14 +338,32 @@ list instead of having to rediscover them. - Every title needs regen + revalidation, because CAUSE.IP2 touches core IRQ delivery for all of them. -## 8. Incidental fix — `tools/embed_spirv.py` build race - -Not SPU-related, found while building. Two runtime targets (`psx-runtime`, -`psx-oracle`) embed the same shader set, so ninja runs `embed_spirv.py` -concurrently. It wrote its intermediate SPIR-V to `.spv` **in the source -tree**, so the two invocations raced on one path and whichever finished first -`os.remove()`d the file the other was about to read — a hard build failure -(`FileNotFoundError: .../blit.frag.spv`), and source-tree pollution besides. -Now compiled to a private `mkdtemp` directory, with the output header written -via atomic replace and a loud error if `glslc` reports success but produces no -file. Fixed per Rule 15 rather than worked around. +## 8. Two build traps hit on the way (neither SPU-related) + +**`tools/embed_spirv.py` race — fixed upstream, our fix dropped.** Two runtime +targets embed the same shader set, so ninja runs the script concurrently; it +wrote its intermediate SPIR-V to `.spv` in the source tree, so the two +invocations raced and one `os.remove()`d the file the other was reading +(`FileNotFoundError: .../blit.frag.spv`). We fixed it locally with `mkdtemp`, +but master fixed the same bug independently using +`tempfile.TemporaryDirectory` — which is better, since it also cleans up when +an error path returns early. **The rebase conflict was resolved in master's +favour and our redundant fix removed**, keeping this branch to SPU work. + +**Autocompile silently falling back to the interpreter.** A fresh title +worktree can run 100% interpreted without saying so. `game.toml`'s +`overlay_autocompile_cmd` names a specific recompiler path, and if it is not +there, autocompile fails every attempt and no shard is ever built — the runtime +just interprets. It cost a whole invalid performance measurement here (§7). + +- Tomba 2 expects `psxrecomp-v4/recompiler/build-t2/psxrecomp-game.exe`, while + the documented recipe builds the recompiler at `build-recompiler/` in the + worktree root. Result: `runs=8 fails=8 shard_ok=0`, `dispatch_native = 0`. +- MMX6 instead hardcodes an **absolute** path into the main repo + (`F:/Projects/psxrecomp/psxrecomp/recompiler/build/psxrecomp-game.exe`), + which happens to exist with a matching codegen hash — so it works by luck of + local layout and would fail on any other machine. + +Check `autocompile_status` (`fails`, `shard_ok`) before trusting ANY performance +number from a fresh worktree. Master commit `42a69624` moves in this direction +for `regen_bios`; the same diagnosis is wanted for the autocompile path.