paula: defer AUDxEN disable to the word boundary (fixes #74 regression)#151
Merged
Merged
Conversation
The HRM audio-FSM rework (PR #134) reverted the issue #74 fix: its apply_audio_dmacon_edges idled an outputting channel immediately on the AUDxEN falling edge, then ran a fresh 000->001 start-up on re-enable. That turns a clear/set pulse shorter than the current DMA word into a clean restart from AUDxLC, which is audibly wrong on 2c1.adf. Real Paula never samples the clear at the DMACON write; it only re-evaluates AUDxON at the word-start boundary. The 011 period event in advance_audio_channels already models that (idle on AUDxON low AND AUDxIP set), so the edge handler must leave the output states 010/011 alone and idle only the DMA start-up states 001/101 (which have not begun output; a posted request still survives to free-run them). vAmiga's synchronous disableDMA() idles immediately here and gets this case wrong too. No serialized field changed, so STATE_VERSION is untouched. Restore the missed-clear and held-clear regression tests, and document the behavior in docs/internals/chipset.md.
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
1 participant
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.
Summary
The Paula HRM audio-FSM rework (#134) reverted the original issue #74 fix (#77 /
a4277ed). Itsapply_audio_dmacon_edgesidled an outputting channel immediately on theAUDxENfalling edge, then ran a fresh000 -> 001start-up on re-enable. That turns a clear/set pulse shorter than the current DMA word into a clean restart fromAUDxLC, which is audibly wrong on2c1.adf.Real Paula never samples the clear at the DMACON write; it only re-evaluates
AUDxONat the word-start boundary. The011period event inadvance_audio_channelsalready models that ((AUDxON || !AUDxIP) ? move_011_010 : idle), so the edge handler must leave the output states010/011alone and idle only the DMA start-up states001/101(which have not begun output; a posted DMA request still survives to free-run them). vAmiga's synchronousdisableDMA()idles immediately here and gets this case wrong too, which is why it plays2c1.adfless accurately than Copperline.No serialized
AudChannelfield changed, soSTATE_VERSIONis untouched (unlike the original fix, which added adma_disable_pendingflag).Changes
src/chipset/paula.rs: on theAUDxENfalling edge, idle only001/101; leave010/011to the natural word-boundary handling.audio_dma_disable_reenabled_before_word_boundary_is_missed(the issue [Bug]: Paula state machine incorrect #74 case) andaudio_dma_disable_left_off_idles_at_word_boundary_then_reenable_restarts; fixed two tests that had encoded the regressed immediate-idle behavior.docs/internals/chipset.md: document the deferred/missed disable.Verification
cargo test --lib: 1347 pass;cargo clippyandcargo fmt --checkclean.2c1.adf(this fix vs. a temporary rebuild with the old immediate-idle line): byte-identical up to t=6.00s, then diverges exactly at t=6.10s = frame 308, the precise frame where the DMACON trace showswrite=0x0001 -> 0x07C0(clear AUD0EN) immediately followed bywrite=0x8201 -> 0x07C1(re-enable) within one frame. Nothing before that scene changed.Fixes #74.