fix(mmce): quiesce the art worker BEFORE the VCD launch handoff (FifthFox's bombed VCD launch) - #242
Merged
Merged
Conversation
…hFox's bombed VCD launch) FifthFox (HW): "MMCE VCD launching is either poisoned or not working. It was working not that long ago. I bombed one launch of a VCD on the MMCE." ROOT CAUSE (ordering, probabilistic -- which is exactly why it "worked until it didn't"): the MMCE art quiesce (cacheAbortMmceImageLoadsTimed, March) drains in-flight art reads before the launch path touches the card. The VCD by-name handoff (ccd1d7a, 2026-06-24) was added ABOVE it as an early return -- so a VCD launch skipped the quiesce entirely and mmceLaunchVcd's POPSTARTER resolution + BDMA equip performed card IO on the SAME shared mmceman SIO2 channel while the art worker could be mid-read. The disc path has always quiesced first; the VCD path never did. FIX: move the single quiesce guard ABOVE the VCD handoff so both launch paths are covered before any card IO. The wedged-worker branch keeps the full #120 abandon-and-retry rationale (no TerminateThread, toast + bail, never a hard freeze) and now protects the VCD path too -- POPSTARTER's own reads would hit the same wedged channel and dead-launch to OSDSYS. Idea source: PR #236 commit 66eeaa7 (the unified agent PR), vetted against this tree -- the reorder is correct; re-landed here on current master with the explanatory comment kept instead of trimmed. Builds clean (make opl.elf, exit 0). HW-pending: FifthFox's MMCE VCD launches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (20)
🔇 Additional comments (1)
📝 WalkthroughWalkthroughThe MMCE art-read quiesce/abort check in ChangesMMCE launch flow
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request reorders the launch sequence in mmceLaunchGame within src/mmcesupport.c to ensure that in-flight MMCE art reads are quiesced before the VCD launch path is executed. This prevents potential SIO2 channel collisions when launching VCDs via POPSTARTER. There are no review comments, so we have no additional feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
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.
The report
Root cause — an ordering bug, probabilistic by nature
The MMCE art quiesce (
cacheAbortMmceImageLoadsTimed, March) drains in-flight art reads before the launch path touches the card. The VCD by-name handoff (ccd1d7a4, June 24) was added above it as an early return — so a VCD launch skipped the quiesce entirely, andmmceLaunchVcd's POPSTARTER resolution + BDMA equip performed card IO on the same shared mmceman SIO2 channel while the art worker could be mid-read. The disc path has always quiesced first; the VCD path never did. Collision odds depend on what art is in flight at the moment of launch — which is exactly why it "worked until it didn't."Fix
Move the single quiesce guard above the VCD handoff so both launch paths are covered before any card IO. The wedged-worker branch keeps the full #120 abandon-and-retry rationale (no TerminateThread, toast + bail, never a hard freeze) and now protects the VCD path too — POPSTARTER's own reads would hit the same wedged channel and dead-launch to OSDSYS.
Provenance
Idea source: PR #236 commit
66eeaa7d(the unified agent PR) — vetted against this tree: the reorder is correct and minimal; re-landed on current master with the explanatory comment kept instead of trimmed.Builds clean (
make opl.elf, exit 0). HW-pending: FifthFox's MMCE VCD launches.🤖 Generated with Claude Code
Summary by CodeRabbit