spu: reverb, noise, sweeps and SPU IRQ (closes #103); CAUSE.IP2 combinational (supersedes #102) - #104
Merged
Merged
Conversation
…inational 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 <source>.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 <Alexbeav@live.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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) <noreply@anthropic.com>
…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) <noreply@anthropic.com>
…rp 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) <noreply@anthropic.com>
This was referenced Aug 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #103. Absorbs and supersedes #102.
Implements the four SPU gaps Issue #103 names — reverb, noise generator, volume
sweeps and SPU IRQ — and lands the separable, independently verified parts of
PR #102. Full per-piece disposition, including everything I could not settle,
is in
docs/internal/SPU_FIDELITY_103.md.Why this supersedes #102 rather than merging alongside it
#102 covers 1 of #103's 4 items. I verified that by grepping its complete
diff for
reverb|noise|sweep|LFSR|vLOUT|vROUT|dAPF— zero matches. Itimplements SPU IRQ plus three unrelated fixes. Its SPU IRQ and this branch's
would collide textually in
spu.c, so there is one implementation, here.Everything of Alexbeav's that stands is carried, and he is credited as
Co-Authored-Byon the implementation commit:Checked against the in-tree Beetle oracle rather than accepted on reasoning:
beetle-psx/mednafen/psx/irq.cpprecomputes the line asAssertIRQ(0, Status & Mask)at power-on, at every assert, and at both thestatus-ack and mask-write halves of a register write, and
cpu.cpp'sAssertIRQclearsCAUSEbit 10 unconditionally before re-setting it fromthe level. That is a combinational mirror, and it matches R3000A, where the
Cause.IPfield is not storage.SPU RAM is readable memory; that was not a transfer at all.
busy-wait that never presents a frame must not freeze SPU time.
Two defects in #102 are fixed here:
runtime/src/psx_interpreter.chad its own
cop0[COP0_CAUSE] |= (1u << 10)at delivery — the identicaldefect in the interpreter path. An audit of every
cop0[13]writer confirmsno other site touches bit 10, and
psx_irq_refresh_cause_ip2()is now thesole owner.
to
sdl_audio_pump()unconditionally, butsdl_audio_update()is the soleauthority on whether to emit, discard, or not pump at all — during a
turbo-load hard mute it deliberately does not pump, so the queue drains and
voice positions freeze in place and music resumes where it left off.
Pumping unconditionally from the VBlank edge bypasses that and would replay
time-compressed audio on every load. The hook now goes through a wrapper
that mirrors the gate state.
#102's own validation (Medal of Honor Underground, Gran Turismo) could not
be reproduced — neither disc is in our suite — so those claims are not repeated
here as if verified. What is verified is the hardware behaviour behind each
change, plus non-regression on our own titles.
Clean-room, and why that mattered
psxrecompships under PolyForm Noncommercial 1.0.0; Beetle PSX/mednafen isGPL-2.0-or-later. Incompatible. Beetle's tree contains a complete, correct
reverb, so the obvious route is closed, and two contributors walked into it:
PR #16 was parked on license grounds after an audit found it tracked
beetle-psx/mednafen/psx/spu.cpplines 590–730, and PR #13's reverb wasseparately wrong — it gated on bit 15 of
dAPF1, an address offset whose topbit is set for any offset ≥ 0x8000, so reverb switched on and off according to
an address value. The real gate is SPUCNT bit 7.
This is written from the psx-spx / nocash register map and documented
algorithm. Beetle's source was not consulted for the implementation; it is used
only as a runtime output oracle, which is not a derivative work. That is the
path
docs/internal/upstream/martin-pr16-spu-reverb.mdprescribes.What landed
vLIN/vRINinput, same-side andcrossed different-side IIR reflections, 4-tap comb early echo, both all-pass
filters,
vLOUT/vROUToutput, work-area advance. SPUCNT bit 7 gateswrites only; reads, IRQ checks, address advance and output always run, as
on hardware. Addressing is provably confined to
[mBASE, 0x80000). Per-voicesends from EON, CD send from SPUCNT bits 0+2.
SPUCNT bits 8–13 rate; NON voices substitute it for their ADPCM sample.
same rate machinery as ADSR; a guest read of a sweeping register returns the
live level.
directions, ADPCM block fetch, capture writes, reverb work-area accesses),
gated on SPUCNT bit 6, mirrored in SPUSTAT bit 6, acked by clearing bit 6.
parks the IRQ address there still gets its interrupt.
The CD-only fast path in
spu_renderwas removed rather than extended: the SPUnow has per-frame work that must run in every path, and an FMV (XA audio, zero
active voices) is exactly the CD-with-reverb case.
Verification
test_spu_fidelity— 137 checks, 0 failures. New, from scratch. CoversIRQ on each access class plus ack/re-arm, reverb write-gating and work-area
wrap confinement, the CD-only path, noise non-repetition, sweep rise/fall and
live readback, direct-volume decode, and savestate round-trip.
test_cause_ip2_combinational— rise, fall on ack, fall on mask, partial ackwith another source pending, partial-width writes, preservation of
ExcCode/BD/IP0/IP1. Both registered in CTest.
bit 7 and allocates a 62 KB work area at
mBASE = 0x70940. Through the bootchime the work area holds 256/256 non-zero reflection halfwords peaking at
24681,
reverb_curlaps correctly inside the work area, and after the drysignal goes silent the work area keeps ringing down (628 → 18 → 2) for ~3 s —
a decaying tail outliving its input. Noise LFSR observed shifting with correct
parity feedback.
shard cache and the pre-SPU exe as control:
interp_share0.7813 vs 0.7775,exc_share0.2386 vs 0.2381, 39.1 vs 38.0–38.4 fps. Identical within noise —the added per-frame DSP work costs nothing measurable.
total. Note honestly that this stage streams its audio as CD-XA and drives
no SPU voices (
active_mask = 0,eon = 0,non = 0, SPUCNT bit 7clear), so it validates no regression on the CD-only path — which is exactly
where the fast path was removed, the CD bus made to drain continuously, and
capture writes added — and not the reverb.
Known deviation, stated plainly
The reverb engine runs at 22050 Hz, but the hardware's 22.05 → 44.1 kHz
reconstruction filter is not specified in the documentation available under
the licensing constraint, and Beetle's filter was deliberately not copied. This
branch uses linear reconstruction, isolated in a single swappable function and
marked in the source. All ten places the documentation ran out are enumerated
in §6 of the design doc as an oracle-comparison work list. An
output-vs-Beetle audio comparison at a fixed scene is the natural follow-up and
has not been done.
Merge prerequisites
recomp-uimust be bumped in lockstep, unrelated to this change butblocking: framework master's
feat(mods): expose linked package attributionneeds recomp-ui
99558ee, which addsauthor_links/author_link_count/source_name/source_urltoRecompLauncherCModPackage. A title pinned to854ae10fails to compile with'RecompLauncherCModPackage' has no member named 'author_link_count'.delivery for all of them. This is the highest-risk item in the branch. It is
the faithful behaviour, so per Rule -1 it lands and titles are revalidated
rather than the fix being narrowed.
Also recommend closing
feat/pr13-salvage-remainderretains nothing but the rejected reverb commit5fc8e15d. Its other three commits are already on master — verified four ways(
git cherry -vmarks them-; cherry-picking all three onto master producesempty commits; and
execute_ch5_pio, theCFC0+ Cause write-protection, andthe
autocompile.cchange are all present). So PR #13 can be closed and thatbranch abandoned;
origin/park/pr13-spu-reverb-rejectedstill retains thecommit.