Repository navigation
fix(agc): raise no end-of-pipe event from flips and carry the release's interrupt context - #7978
Conversation
MarcZX10
left a comment
There was a problem hiding this comment.
I reviewed the event encoding and flip-ordering paths. The event now carries the queue in the low bits and the release context above it, and the query helpers split those fields consistently. The regression cases cover a flip without an EOP event, release events with contexts, and a following submission held until the flip work completes. I did not independently run the reported GPU/Windows test matrix or title checks.
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
On the overlap table: none of those PRs changes the end-of-pipe event or the flip hold; the rows are neighbouring lines in |
c92de55 to
c4a87b4
Compare
c4a87b4 to
2c0ce89
Compare
|
A data point from the PS4 side, on #6977's branch (head The Unity player in Boggle and Trivial Pursuit Live! (CUSA03312) ends each frame with an
With #7137's commits on that branch there is one event per frame: 2684 events for 2683 submissions, against two per frame before. The worker stays within two submissions, and both titles run their menus at 60 fps. Trivial Pursuit now reaches its game. That PR is Get-fix#5. Removing the flip event entirely, as this PR does, covers the same case. shadPS4 also raises a separate flip interrupt for a PS4 flip, not the end-of-pipe one. |
|
Heads-up while you rewrite Since this PR removes the latch and the flip end-of-pipe event, that test pins behaviour you're deleting, so it should not be carried over. Flagging it only so you're not surprised by the branch tip differing from the merge commit |
2c0ce89 to
00ba7be
Compare
|
Ran it on Linux/RADV. Short answer: yes, far further than Setup. Arch Linux, kernel 7.2.9, GCC 16.2.1, Mesa 26.2.4, RADV on an AMD Radeon RX 9070 XT (1002:7550, GFX1201), Wayland/GNOME. Gran Turismo 7 PPSA01317 v01.680, base package. Same relinked
On With this PR, GT7 renders real frames and reaches the PlayStation Network prompt. The window title carries a running frame counter, so the captures read:
So 4,203 frames presented at up to ~55 FPS, well past anything On the OK click: I ran it once with the dialog dismissed and once left alone. Both end on the same unrelated gap, Two limits on what this shows. I could not get past the sampled-image format gap, and #7256's layered-depth gap is behind that, so nothing past those is tested. And neither run is a real play session; the only input was the single OK click. Capture from the RADV run: |
✅ AMD GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (9, not counted against this PR)Passed on retry (4, failed in the parallel run, passed when re-run with a 360 s timeout; not counted against this PR)Validated but not compared (58, not in the main baseline's validation pass)Validation IDs already present on main (4 IDs in 519 tests)Skipped (109)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
XairRrR
left a comment
There was a problem hiding this comment.
Re-checked — changes still requested
The new head a3a6f4a5 only merges main 4f41f4c (one mechanical conflict in PacketExecution.cpp). It merges cleanly, builds, and the focused AGC tests pass. However, the second flip hold that oneandonlydean reported as deadlocking video_out_flip_deferred is unchanged at this head and has had no reply.
What changed since my last review
I have no text from my previous review, so I reviewed the whole change again.
- New commit: only
a3a6f4a5("Merge main 4f41f4c into #7978"), on top of612ceb75. The merge drops theflipInterruptDeferredlocal and keepsmain'sGraphics::StorageTexture::NoteFlip()call. Comparing612ceb75..a3a6f4a5over the PR's driver sources shows onlymain's own changes inEq/src/Event.cppandQueueWorker.cpp. The PR's logic is unchanged. - No reply from the author since oneandonlydean's comment of 2026-11-02 12:08 about
video_out_flip_deferred. - Two GPU CI reports on
612ceb75: NVIDIA RTX 2000 Ada and AMD RX 6300. Both report 0 new failures. They are Linux runs, where thevideo_out_flip_*tests skip.
What I ran
Head a3a6f4a5cb4e079f6818639edc597663e021ae89, main 4f41f4c375f5abb487dbd184fbc150f31994d762, merged tree 38b2a2505799bb88222e1f7d85771f3acd40a7eb. The machine is Debian 13 (VM, 32 vCPU), GCC 14.2.0, and lavapipe (Mesa 26.1.6, LLVM 19), with no GPU.
git diff --check 4f41f4c375f5abb487dbd184fbc150f31994d762...a3a6f4a5cb4e079f6818639edc597663e021ae89
python3 tools/check_conventions.py --base 4f41f4c375f5abb487dbd184fbc150f31994d762
git diff --stat 612ceb75d74ebebe156b6eaf007c9fe112e1f252 a3a6f4a5cb4e079f6818639edc597663e021ae89 -- <PR driver sources>
ANYPS5_REQUIRE_VULKAN=1 ctest --test-dir build -j8 --test-load 32 --output-on-failure -R "^(agc_driver|agc_driver_eop_interrupts|agc_driver_flip|agc_driver_flip_compute|video_out_flip_deferred|video_out_flip_lifetime)$"
# negative control: the 8 production files restored from main
git show 4f41f4c375f5abb487dbd184fbc150f31994d762:<path> > <path>
ninja -C build -j32 -l32 agc_driver_tests agc_driver_flip_tests
ANYPS5_REQUIRE_VULKAN=1 ctest --test-dir build -j8 --test-load 32 --output-on-failure -R "^(agc_driver|agc_driver_eop_interrupts|agc_driver_flip)$"
build/tests/agc_driver_flip_tests
build/tests/agc_driver_tests --eop-interrupts
# restore
git show 38b2a2505799bb88222e1f7d85771f3acd40a7eb:<path> > <path>
ninja -C build -j32 -l32 agc_driver_tests agc_driver_flip_tests libs
ANYPS5_REQUIRE_VULKAN=1 ctest --test-dir build -j8 --test-load 32 -R "^(agc_driver|agc_driver_eop_interrupts|agc_driver_flip)$"
git status --porcelain| Check | Result |
|---|---|
git diff --check |
clean |
check_conventions.py |
"All conventions checks passed." |
Build and libs (script) |
ok |
| Selected tests (script) | 2169 run, 0 new failures, 0 timeouts; 7 known failures that also fail on main |
| Validation (script) | layer loaded in 769 of 770 tests. The only new VUID is VUID-vkCmdDispatch-groupCountX-00386 in agc_driver_native_depth_uniform_planes. That test is absent at baseline d1ae628b and was added on main before 4f41f4c3. The PR does not touch dispatch or depth, so this VUID is not counted against it |
agc_driver, agc_driver_eop_interrupts, agc_driver_flip, agc_driver_flip_compute |
pass |
video_out_flip_deferred, video_out_flip_lifetime |
skipped (no display or Vulkan window on this machine) |
| Worktree after restore | git status --porcelain empty |
Negative control
I restored the 8 production files (Eq/include/Event.hpp, Eq/src/Event.cpp, Eq/src/Query.cpp, Driver.hpp, PacketExecution.cpp, PacketMemory.cpp, QueueWorker.cpp, Submission.cpp) from main and kept the PR's tests. All three changed tests then fail:
agc_driver: "graphics event type is not the event data"agc_driver_eop_interrupts: "a flip raised the graphics end-of-pipe event"agc_driver_flip: "a flip was accepted before the previous flip submission completed"
With the merged files restored and rebuilt, all three pass again.
Previous findings
I have no text of my previous review, so I can't list its findings one by one. The open points from other reviewers:
- MarcZX10's "branch not updated to current
main": resolved. The head merges cleanly ontomain4f41f4c and builds. - oneandonlydean's
video_out_flip_deferreddeadlock: still open. The code atcore/libs/prx/libSceAgcDriver/Execution/src/Driver/Queues/Submission.cpp:284-291is unchanged at this head. It is listed below as a new blocking finding because this is my first written record of it.
New findings
Blocking
- A command-buffer flip now blocks the submitting thread until the previous flip's output callback returns, which deadlocks
video_out_flip_deferred. This is atcore/libs/prx/libSceAgcDriver/Execution/src/Driver/Queues/Submission.cpp:284-291. After theflipsAhead == 0wait, a non-CPU flip also waits forflipWorkOpen == 0and then for the recorded GPU work (waitFlipWork).flipWorkOpenonly drops infinishFlipWork, which runs afterexecutehas passedflipHold.Release()andGpuReady(PacketExecution.cpp:461-462).- In
core/libs/prx/libSceVideoOut/tests/Flip.cpp:205-215, the first flip goes to aGateoutput whoseGpuReadyblocks untilgate->Release(). The test only reaches that call after the secondsceAgcDriverSubmitDcb(line 208) returns, so the two threads wait on each other. - I confirm oneandonlydean's analysis by reading the code. I could not run the test here because it skips without a display; the Windows failure is maintainer-reported.
- This also contradicts the description's "the next flip is still released before presentation as before": that holds for
flipsAhead, but the new wait still covers the previous flip'sGpuReady. The realFlipRequest::GpuReadyonly returns early whenAPS5_SYNC_FLIPis unset. - Suggested fix: stop counting a flip submission as open work once the worker has handed its flip to the output. For example, take the work serial for the hold before
GpuReady, or end the second wait atflipHold.Release()with the serial recorded up to that point. Then the title waits only for GPU work, not for presenter callbacks. If the PR keeps the wait, changetestDeferredBufferso it still checks that a flip submitted before its buffer is registered is accepted and presents after registration. Either way, the question needs an answer.
Non-blocking
- The context is decoded twice. At
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp:597, the context is recomputed aspacket[7] & 0x0fffffffu.releaseInterruptalready validated it and passed it topreparePacketMemory(line 396), where the bounds and typed-bit checks live. Passing the returnedstd::optionalvalue through would keep one decode site. - EV_CLEAR now keeps only the newest interrupt's data. Two interrupts on one registration before a wait now give one event with the newer context (
testEndOfPipeLabelsWithoutWorkexpects0x12), where it used to give a count. A title that indexes a handler table by context would miss the first one. Whether the console's kernel coalesces these the same way isn't stated in the description or in the debt entry. Consider adding it to thesceAgcDriverGetEqContextIddebt entry as unmeasured.
Not verified
video_out_flip_deferredand the othervideo_out_flip_*tests: they skip here (no display or Vulkan window). The Windows failure is maintainer-reported; I confirmed the cause from the code only.- Title results (Death Stranding 2, Gran Turismo 7, Minecraft, Astro's Playroom) and the NVIDIA, RADV and Windows matrices are author- or tester-reported. I did not reproduce them.
- Performance: I did not measure the cost of the title thread now waiting on, and reaping, the previous flip submission's GPU work on each command-buffer flip.
To get this to a good state
- Address the second flip hold in
holdFlipBehindWorkerso a submit does not wait for the previous flip's output callback, or explain why it must, and adapttestDeferredBufferwhile keeping what it checks. - Reply to oneandonlydean's comment.
- Rerun
video_out_flip_deferredand the othervideo_out_flip_*tests on a host where they don't skip (Windows, or Xvfb with SDL's x11 driver), onmainand on the new head.
✅ NVIDIA GPU test: passedPR head
Unstable on main (3, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (2, not counted against this PR)Validation IDs already present on main (4 IDs in 517 tests)Skipped (48)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
…eted, and raise no event from flips A flip packet delivered the queue's graphics end-of-pipe event after the work recorded before it. On the console a flip raises no such event: its interrupt context is always a typed token that the kernel routes to the display path (PS5 13.60 kernel), and neither Minecraft (PPSA17221) nor Death Stranding 2 (PPSA02015) sends a release that requests one. Death Stranding 2 registers the event before it fills its handler table, so the first flip's event reached a null handler (issue 6462). Minecraft did need it, but only because of when the flip hold ends. It writes its frame label after the flip in the same buffer and, after its next submission, waits on the event until that label is 1. The hold released the title at the flip packet, so it read the label before the worker had written it and slept until the flip event came. The hold now ends when the previous flip submission ran to its end and its recorded GPU work completed, so the label is in memory when the title reads it. The per-queue flip event latch is removed with the event: a queue's own release interrupts deliver as before.
…n has not completed
…ad of an interrupt count
…mand-buffer flips for the previous flip submission's work The flip hold was released at the end of the submission, after the presenter's GpuReady. A presenter that blocks there held every later flip, CPU flips included, so video_out_flip_lifetime, _reopen and _cpuflip timed out. Keep flipsAhead released before GpuReady as before and count open flip submissions separately; only a command-buffer flip waits for them and for their recorded work.
…before the next flip returns waitFlipWork waited for the recorded work's serial but never reaped it, so labels written by completion actions (write-backs and labels stored when a batch is reaped) could still hold their old value when the title read them after its next flip. Reap the batches up to that serial under the GPU mutex, as the packet drain does.
…hat a typed interrupt context fails
…n output callback the title releases
a3a6f4a to
04f9490
Compare
|
Thanks @oneandonlydean for tracking it down, and @XairRrR for the re-check. The second wait is there for Minecraft (PPSA17221): it writes its frame label after the flip in the same command buffer and, after its next submission, waits on the end-of-pipe event until that label is 1. Flips raise no event, so a label not yet written at that point leaves it asleep until some later event. The wait covers the rest of the previous flip submission, and that includes the output callback. A real output's So I kept the wait and changed At 04f9490 (rebased onto |
XairRrR
left a comment
There was a problem hiding this comment.
Re-checked — approved
The new head 04f94904 rebases the branch onto main 4f41f4c and replaces the merge commit. It adds one test commit that holds video_out_flip_deferred's flip behind a GPU WAIT_REG_MEM instead of a Gate output callback. The author has explained why the second flip hold stays, and the code supports that explanation, so my blocking finding is resolved.
What changed since my last review
- History: the branch was rebased onto
main4f41f4c in place of merge commita3a6f4a5. The range-diff shows that commits 2, 4, 5, 6 and 7 are identical. Commits 1, 3 and 8 differ only in context frommain(theRingOrder/NoteFlipneighbours, and nearby debt entries thatmainadded). The PR's driver logic is unchanged. - New commit
04f94904(test(videoout): ...):testDeferredBuffernow submits one command buffer: aWAIT_REG_MEMon a host label, then the flip. The test writes the label aftersceVideoOutRegisterBuffers2. TheGateon output 7 is gone from this test.- The commit also removes
AgcDriverUnregisterVideoOutput_nid_postfix(7, gate)fromtestLifetime(see the new non-blocking finding).
- Author reply (2026-11-03 13:45) to oneandonlydean and to me:
- The second wait is kept on purpose for Minecraft's after-flip label.
- A real
GpuReadyonly queues the request, so it never waits on the title. video_out_flip_deferredaborts ata3a6f4a5and passes at04f94904, 3 of 3 each, on an RTX 5080 with SDL's Wayland driver (author-reported).
- Description: a new "Tested" paragraph covers this rebase.
What I ran
Head 04f94904874156111efea0000631c4839fdeb554, main 4f41f4c375f5abb487dbd184fbc150f31994d762, merged tree a178dbdc01b93a90c6d151eeedf7ddbba48e57fb. The machine is Debian 13 (VM, 32 vCPU), GCC 14.2.0 and lavapipe (Mesa 26.1.6), with no GPU and no display.
git show 04f94904874156111efea0000631c4839fdeb554
git diff --check 4f41f4c375f5abb487dbd184fbc150f31994d762...04f94904874156111efea0000631c4839fdeb554
python3 tools/check_conventions.py --base 4f41f4c375f5abb487dbd184fbc150f31994d762
ANYPS5_REQUIRE_VULKAN=1 ctest --test-dir build -j8 --test-load 32 --output-on-failure -R "^(agc_driver|agc_driver_eop_interrupts|agc_driver_flip|agc_driver_flip_compute|video_out_flip_deferred|video_out_flip_lifetime|video_out_flip_reopen|video_out_flip_cpuflip)$"
git status --porcelain| Check | Result |
|---|---|
git diff --check |
clean |
check_conventions.py |
"All conventions checks passed." |
Build and libs (script) |
ok |
| Selected tests (script) | 2153 run, 0 new failures, 0 timeouts. 7 known failures also fail on main (denormal and division tests, vector_constant_factor_tininess, one unwrap test) |
| Validation (script) | layer loaded in 769 of 770 tests, no layer missing, 0 tests with new VUIDs compared with main 4f41f4c |
agc_driver, agc_driver_eop_interrupts, agc_driver_flip, agc_driver_flip_compute |
pass (11.4 s at most) |
video_out_flip_deferred, _lifetime, _reopen, _cpuflip |
skipped: no display or Vulkan window on this machine, and no Xvfb |
| Worktree | git status --porcelain empty |
Negative control
None for the changed test. video_out_flip_deferred skips on this machine, and the change adapts that test to the PR rather than adding a regression test for it. The author reports that it aborts with test gate timed out at a3a6f4a5 and passes at 04f94904. I accept that as author-reported.
The tests that check the PR's own behaviour (agc_driver, agc_driver_eop_interrupts, agc_driver_flip) are byte-identical to the previous head. My earlier negative control showed all three fail with the production files restored from main. They pass here on the rebased tree.
Previous findings
- Blocking — the second flip hold deadlocks
video_out_flip_deferred(Submission.cpp:284-291): Resolved. I concede the author's point:- The wait at
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Queues/Submission.cpp:284-291still covers the previous flip'sGpuReady. A deadlock needs aGpuReadythat waits on the submitting thread, though. - In the real
FlipRequest::GpuReady(core/libs/prx/libSceVideoOut/src/VideoOutDriver.cpp:314-317), the request is queued andGpuReadyreturns. WithAPS5_SYNC_FLIPit waits only forgpuComplete, failure or close, which is presenter progress, not the title. - The hold also ends when the worker waits on the title (
queue0AwaitsTitle). - The rewritten test (
core/libs/prx/libSceVideoOut/tests/Flip.cpp:197-225) still checks what it was written for. The flip is submitted while the GPU is held on a label (WAIT_REG_MEMwith function 3 and memory space, the same encoding asGatedDispatchCapture.cpp). The buffer is registered next, and only then is the label released. The test then expects the flip to present buffer 0 with arg 3. I could not run it here.
- The wait at
- Non-blocking — context decoded twice (
PacketExecution.cpp:597): Still open.deliverGuestEopInterrupt(submission.queue, packet[7] & 0x0fffffffu)still recomputes whatreleaseInterrupt(line 134) already validated. This does not block the merge. - Non-blocking — EV_CLEAR keeps only the newest interrupt's context, not recorded as unmeasured: Still open. The debt file has no wording on coalescing. This does not block the merge.
New findings
Blocking
None.
Non-blocking
testLifetimeno longer unregisters its gate output. Incore/libs/prx/libSceVideoOut/tests/Flip.cpp:111-125, commit04f94904dropsAgcDriverUnregisterVideoOutput_nid_postfix(7, gate)fromtestLifetime. That function is used byvideo_out_flip_lifetimeand_reopen, which this PR does not otherwise touch, and the commit message only coverstestDeferredBuffer.- Output 7 now stays registered until
LibcRunShutdown_nid_postfix, andDriver::UnregisterVideoOutput's registration check is no longer run on this path. - Each mode runs in its own process, so this probably has no effect.
- Suggested fix: restore the line. If it was removed because it now fails or blocks, say why in the PR.
- Output 7 now stays registered until
Not verified
video_out_flip_*tests: none of them ran here (no display or Vulkan window). The deferred test's before/after result and "all 26 modes pass" are author-reported (RTX 5080, SDL Wayland).- Title results (Death Stranding 2, Gran Turismo 7, Minecraft, Astro's Playroom) and the NVIDIA, RADV and Windows runs are author- or tester-reported.
- Cost of the hold: I did not measure what it costs a title thread to wait on, and reap, the previous flip submission's GPU work on each command-buffer flip.
✅ NVIDIA GPU test: passedPR head
Unstable on main (3, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (2, not counted against this PR)Validation IDs already present on main (4 IDs in 517 tests)Skipped (48)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
✅ AMD GPU test: passedPR head
Unstable on main (2, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (7, not counted against this PR)Validation IDs already present on main (4 IDs in 517 tests)Skipped (58)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
|
Hi fma-rc, closing out our earlier question about the Windows |
XairRrR
left a comment
There was a problem hiding this comment.
Re-checked — approved
The only new commit is 7ceffb39, a merge of main 086c068 into 04f94904 by oneandonlydean. The PR's nine commits are identical to the ones I last reviewed, and both conflict resolutions in the merge keep both sides correctly.
What changed since my last review
- PR commits: none changed. The range-diff shows commits 1 to 9 identical (
=). - Merge commit
7ceffb39:git show --ccshows two hand-resolved hunks.PacketExecution.cpp, the pipelined label/DMA condition: keeps the PR'senqueueLabelPacket(..., releaseInterrupt(packet, opcode, false))and addsmain's new|| enqueueConstantRamDump(...). That is correct.docs/dev/debt/libSceAgcDriver.md: keeps the PR's removal of the "Flip packets raise the graphics end-of-pipe event" entry and addsmain's new "Host imports across devices" entry. That is correct.Driver.hpp,PacketMemory.cppandlibSceVideoOut/tests/Flip.cpptakemain's changes without conflict.mainaddedenqueueConstantRamDumpandcommitStore, and a reopen check intestLifetime.- Every interrupt delivery in the merged tree still goes through
deliverGuestEopInterrupt(queue, context)(PacketMemory.cpp:97,:537,:540,PacketExecution.cpp:597). Nothing frommainbrings back a flip-raised event or the oldboolsignature ofcommitLabel.
- Description and discussion: no author changes since my last review. oneandonlydean reports that all 28
video_out_flip_*tests pass on Windows 11 at04f94904merged intomain4f41f4c (tester-reported).
What I ran
Head 7ceffb3974b59b6515ef021e010a78298510767b, main 04a9eb4fa2877eb6e40bf2c83adf972e40d89b24, merged tree 4678edbe247f4ae0a7d2d3f64b0b9cd714d2b036. Debian 13 VM, GCC 14.2.0, lavapipe (Mesa 26.1.6), no GPU and no display.
git log --format="%H %P %s" -3 7ceffb3974b59b6515ef021e010a78298510767b
git show --cc 7ceffb3974b59b6515ef021e010a78298510767b
git diff --check 04a9eb4fa2877eb6e40bf2c83adf972e40d89b24...7ceffb3974b59b6515ef021e010a78298510767b
python3 tools/check_conventions.py --base 04a9eb4fa2877eb6e40bf2c83adf972e40d89b24
ANYPS5_REQUIRE_VULKAN=1 ctest --test-dir build -j8 --test-load 32 --output-on-failure -R "^(agc_driver|agc_driver_eop_interrupts|agc_driver_flip|agc_driver_flip_compute|agc_driver_submit|video_out_flip_deferred|video_out_flip_lifetime)$"
git status --porcelain| Check | Result |
|---|---|
git diff --check |
clean |
check_conventions.py |
"All conventions checks passed." |
Build and libs (script) |
ok |
| Selected tests (script) | 2180 run, 0 new failures, 0 timeouts. The 7 known failures also fail on main (denormal and division tests, vector_constant_factor_tininess, one unwrap test) |
agc_driver, agc_driver_eop_interrupts, agc_driver_flip, agc_driver_flip_compute |
pass (11.6 s at most) |
video_out_flip_deferred, video_out_flip_lifetime |
skipped (no display or Vulkan window) |
| Validation (script) | layer loaded in 782 of 783 tests, none missing. One test has new VUIDs, see below |
| Worktree | git status --porcelain empty |
The one validation difference is not from this PR. agc_driver_image_gather_volume reports VUID-VkSamplerCreateInfo-addressModeU-01079 and the addressMode{U,V,W}-parameter VUIDs. Every line is marked "test not run on main": the test does not exist in the older validation baseline (1e658521) and came in from main. This PR changes no sampler, image or shader code (its 12 files are the end-of-pipe event, the flip hold, their tests and the debt doc), so I do not count it against the PR.
Negative control
Not repeated. The PR's tests are byte-identical to the previously reviewed head. My earlier control showed agc_driver, agc_driver_eop_interrupts and agc_driver_flip fail with the production files restored from main. All three pass on this merged tree.
Previous findings
- Non-blocking — context decoded twice (
PacketExecution.cpp:597): Still open.deliverGuestEopInterrupt(submission.queue, packet[7] & 0x0fffffffu)still recomputes whatreleaseInterruptalready validated. - Non-blocking — EV_CLEAR keeps only the newest interrupt's context, not recorded as unmeasured: Still open. The debt file still says nothing about coalescing.
- Non-blocking —
testLifetimeno longer unregisters its gate output (libSceVideoOut/tests/Flip.cpp): Still open. The onlyAgcDriverUnregisterVideoOutput_nid_postfix(7, ...)left is in another test (Flip.cpp:303). The author did not reply on this.
New findings
Blocking
None.
Non-blocking
None.
Not verified
main's new reopen check with the PR's flip hold:mainadded a check totestLifetime(reopen)that submits a label-store DCB while the gate holds a flip. With this PR only command-buffer flips are held, so that store should not block.video_out_flip_lifetimeand_reopenskip on this machine, though, so I could not confirm it. oneandonlydean's Windows run (28 of 28) was onmain4f41f4c, before this merge.- Other tests and title results: the other
video_out_flip_*tests, all title results, and the NVIDIA, RADV and Windows runs are author- or tester-reported. - Validation baseline: the comparison used an older
main(1e658521), so tests added tomainsince then were validated but not compared.
✅ NVIDIA GPU test: passedPR head
Unstable on main (2, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Validation IDs already present on main (8 IDs in 531 tests)Skipped (50)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
What
Death Stranding 2 (PPSA02015) crashes about 27 s in on current
mainwith PR 5426: its graphics event thread calls a null handler. Stop onmain00d7e61 + #5426:The thread waits on graphics event 0 and indexes a handler table by
sceAgcDriverGetEqContextId. Two things go wrong.Flips raise the event. Since #4215 every flip delivers the end-of-pipe event (#7137 later latched it off per queue). From the PS5 13.60 kernel, a flip's interrupt context is always typed and goes to the display path, not to this event. Flips now raise no event. #4215 added the event so that Minecraft (PPSA17221) could keep the labels its frames write after a flip; instead, a submission with a flip in its command buffer now returns only after every earlier flip submission ran to its end and its GPU work completed. CPU flips (
sceVideoOutSubmitFlip) do not wait, and the next flip is still released before presentation as before. This removes the #7137 latch.The event carries the wrong context. The event data is
queue | context << 16, where context isRELEASE_MEMdword 7 masked to 28 bits (from the PS5 13.60 kernel).sceAgcDriverGetEqContextIdreturnsdata >> 16andsceAgcDriverGetEqEventTypethe low 32 bits (from the PS5 10.01 module). Before, both returned the queue, so the title's first own interrupt (queue 0x50, context 0) read context 0x50 and called the same null entry. A typed context (bit 27) throws, since it is not delivered as this event and that path is not implemented. Both functions no longer read the event'sident, so its two overflow checks are removed;agc_driverchecks both decodes, up to a full 28-bit context.Earlier attempts at the flip event: #6963 and #7008 (closed). Overlap: #6977 rewrites
Eq/src/Event.cppfor PS4 titles. Rebased ontomainafter #7137, #6616 and #6794 merged.Tested
Linux (Arch, GCC 16.2.1) and Windows Server 2025 (WinLibs GCC 15.2, no GPU), on
main340247a with this PR (2c0ce89), unless a line names an earlier head.main4f41f4c (04f9490) in place of the merge commits.video_out_flip_deferreddeadlocked behind the second flip hold, because its output callback blocked until the test released it; the test now holds the GPU with aWAIT_REG_MEMon a label it writes after registering the buffer. Run outside ctest on the RTX 5080 with SDL's Wayland driver, it aborts withtest gate timed outat a3a6f4a (3 of 3) and passes at 04f9490 (3 of 3); all 26video_out_flip_testsmodes pass there. The 47 tests matching^agc_driver$|^agc_driver_(eop_interrupts|flip|flip_compute|present_mode|worker_failure_log|submit)$|^agc_submit|^video_outpass on NVIDIA, RADV (610M) and lavapipe, apart from thevideo_out_*tests that skip under ctest, and the Windows build and ctest pass 3918 of 3918.main4569fe6 (5adc50d), keeping fix(agc): do not evict host imports used in the current or previous frame #11357's host import frame checks in the flip tests and theAddEventregistrationmainadded: the tests matching^agc_driver$|^agc_driver_(eop_interrupts|flip|flip_compute|present_mode|worker_failure_log|submit)$|^agc_submit|^video_outpass on NVIDIA (RTX 5080), RADV (Radeon 610M) and lavapipe (thevideo_out_flip_*ones skip as before); the Windows build and ctest show no failure the merge base lacks (3282 tests).agc_driver,agc_driver_eop_interruptsandagc_driver_flipfail onmainwith this PR's tests and pass with the change, 3 of 3 runs each on RADV (Radeon 680M), on c4a87b4.^agc_driver$|^agc_driver_(eop_interrupts|flip|flip_compute|present_mode|worker_failure_log)$|^video_outpass on NVIDIA, RADV and lavapipe; the Windows build passes all 1700 tests.video_out_flip_*tests skip under ctest (the offscreen SDL driver has no Vulkan). Under Xvfb with SDL's x11 driver and Mesa's software WSI on RADV, all 12 pass onmainand with this PR.agc_driver_f64_add_mul_denormal_modes(lavapipe flushes an f64 denormal) fails the same way onmain340247a; RADV (Radeon 680M) only the same 4 failures as onmain(agc_driver_image_table_vouched_lookups,agc_driver_bda_device,agc_driver_guest_memory_lifetime,agc_driver_tessellation).unsupported MIMG opcode 0xb8, a separate bug.maina700809.main147 and 143 frames in two rounds, this PR 145 (one round on b834c76, the last code change). Its captured frames show the same scene asmain.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)