Skip to content

fix(agc): present EOP flips when draws are pipelined - #6616

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
hashaam1217:fix/agc-eop-flip-pipelined-draws
Oct 19, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
hashaam1217:fix/agc-eop-flip-pipelined-draws

Conversation

@hashaam1217

Copy link
Copy Markdown
Contributor

What

EOP flips (sceVideoOutSubmitEopFlip, #5539) never present on current main. #1202 turned the queue-0 draw pipeline on by default. With the pipeline on, Driver::execute hands every label-writing packet to the pipeline (enqueueLabelPacket) and continues. Flip packets and rendering waits are excluded, but a RELEASE_MEM that completes a reserved EOP flip is not. So its label lands while the submission.flips.contains(cursor) branch never runs: the reservation is consumed (flipPendingNum 0), the label reads 1, and count stays 0 with currentBuffer -1.

This PR keeps packets that carry a reserved flip out of the pipeline. They take the normal path, which drains the pipeline first, because RELEASE_MEM is not a pipeline-keeping opcode, and then present the flip, as flip packets already do. It is one condition in PacketExecution.cpp.

CI does not catch this. All 23 VideoOut flip tests skip on both Linux and Windows CI, because SDL2's offscreen driver has no Vulkan. I ran them against a local Xvfb with -DANYPS5_VIDEO_OUT_TEST_DRIVER=x11.

Tested

Ubuntu 24.04, GCC 13, Release, full build. VideoOut tests use Xvfb (DISPLAY=:99) and -DANYPS5_VIDEO_OUT_TEST_DRIVER=x11 on lavapipe (Mesa 25.2.8).

Bisecting with git bisect --first-parent on video_out_flip_eop from #5539's merge (937cd6cab, passes) to main gives 8856f180c (#1202) as the first failing merge; its parent e49ec591d passes.

video_out_flip_eop _eop_main _eop_aftergpu
main, APS5_PIPELINED_DRAWS=0 pass pass pass
main, pipeline on (default) fail, 5.1 s fail, 5.1 s timeout, 30 s
this PR, pipeline on pass pass pass
this PR, APS5_PIPELINED_DRAWS=0 pass pass pass

With this PR, all 21 VideoOut tests pass on Xvfb/lavapipe. ctest -R '^agc_' passes 606/606 on the Intel HD 530. On lavapipe it passes 605/606: the one failure is the existing agc_frame_dump_io pipe-capacity flake that #6573 fixes. python3 tools/check_conventions.py --base origin/main passes.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: none
  • GPUs tested: lavapipe (Mesa 25.2.8, LLVM 20.1.2), Ubuntu 24.04; Intel HD Graphics 530 (Skylake GT2, Mesa 25.2.8 ANV), Ubuntu 24.04, agc_ tests only (Xvfb has no DRI3 for presentation)
  • AI-assisted: yes

🤖 Generated with Claude Code

With the queue-0 draw pipeline on, the default since boykopovar#1202, the packet
loop hands every label-writing packet to the pipeline and skips the rest
of the loop. A RELEASE_MEM that completes a reserved EOP flip
(sceVideoOutSubmitEopFlip, boykopovar#5539) is a label write, so its label landed
but the flip branch never ran: the reservation was consumed and the flip
never presented. video_out_flip_eop and video_out_flip_eop_main failed
and video_out_flip_eop_aftergpu timed out.

Keep packets that carry a reserved flip out of the pipeline. They take
the normal path, which drains the pipeline before the flip as it does
for flip packets.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MarcZX10

Copy link
Copy Markdown
Contributor

I checked the one-line guard against #6462. That issue reports an EOP delivery on flips that crashes Death Stranding 2, while this PR restores the reserved EOP RELEASE_MEM path when draw pipelining is enabled. I do not have evidence that this change alters the event count, but it would be useful to confirm the #6462 reproduction still behaves as expected (or document the interaction) before merging, since the title reports different expectations for flip-generated events.

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

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.

Pull request Shared files not compared
PR 4448 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp
PR 5115 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp
PR 5925 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp
PR 6188 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp
PR 6292 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp
PR 6501 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp
PR 6615 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp
PR 6946 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp
PR 6947 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp
PR 7008 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp
PR 7340 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp
PR 7642 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp

@fma-rc

fma-rc commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

I ran the Death Stranding 2 reproduction from #6462 with this change. The base is main 0029e62, which already has the pipelined default, plus the three local fixes DS2 needs to reach that point. Without this PR the title dies at 56.6 s on a null call in its event thread (app.elf+0x1bd01b1). With this PR cherry-picked onto the same stack it dies at 56.5 s at the same address, in 2 of 2 runs. So the guard leaves that reproduction as it was. This fits what the issue shows: DS2's first flip is a flip packet alone, with no RELEASE_MEM after it, and the existing FlipPacketHeader check already keeps that packet out of the pipeline.

@hashaam1217

Copy link
Copy Markdown
Contributor Author

On the overlap with #6501: both PRs change the same if at L176, and they compose. #6501 adds enqueueSampleDumpPacket to the enqueue list, and this PR adds !submission.flips.contains(cursor) before the list, so a packet carrying a reserved flip skips all three enqueues. Whichever merges second needs only a one-line rebase. No ordering is needed (Depends on: none stands).

Thanks @fma-rc for running the #6462 Death Stranding 2 reproduction. Its first flip is a flip packet alone, which FlipPacketHeader already kept out of the pipeline, so this guard does not change that path.

@MarcZX10

Copy link
Copy Markdown
Contributor

One additional related data point from #6350: the reporter measured two AgcDriverDeliverEopInterrupt deliveries for one guest int_sel=1 release and a separate flip on GT7. Suppressing the synthetic flip EOP at both inline and deferred delivery sites removed the null dereference in an 80-second RADV run, though there is no console trace and the regression/lifetime rule is still being worked out. I checked this PR diff: excluding reserved flip packets from pipelining does not itself change either delivery site or the delivery count, so I see no direct conflict; sharing this as relevant EOP-flip interaction evidence alongside the #6462 check.

@MarcZX10

Copy link
Copy Markdown
Contributor

I opened #6963 stacked on this PR, with Depends on: #6616. It builds on the reserved flip EOP path here and adds delivery-time queue-0 guest-EOP tracking so one subsequent synthetic flip event is suppressed, while later bare flips still deliver. The focused packet-order regression passes locally; details and host limitations are in #6963.

@cadenasynyster

Copy link
Copy Markdown
Contributor

Confirmation on Windows with a real presenting GPU (the PR's tests were Xvfb/lavapipe):

Windows 11 10.0.26200.9711, RTX 5070 Ti (NVIDIA 617.42), Ryzen 7 9800X3D, WinLibs GCC 15.2.0 posix UCRT r7, Release, main f3263df, video_out_flip_tests run directly in a real window (ctest skips them on Windows under the offscreen driver).

  • main: eop and eop_main fail 5/5 ("EOP packet did not finish its label and reserved flip"), with pending=0 count=0 current=-1 arg=-1 label=1 and no stored failure; eop_aftergpu times out ("the flip did not complete") and aborts.
  • main + APS5_PIPELINED_DRAWS=0: all three pass.
  • main + this PR's one-line guard: all 16 cases I ran pass (eop, eop_main, eop_aftergpu, labels, labels_main, lifetime, reopen, controls, overlay, unopened, present, backtoback, pacing, aftergpu, deferred, unregistered). decode, dcc and onedevice also pass on main; unavailable fails as expected with a working display.

I did not run the full ctest with the change.

AI-assisted: yes

@marco-scheffler

Copy link
Copy Markdown

Linux data point on NVIDIA.

Ubuntu 24.04.4 (kernel 6.8.0-147), NVIDIA RTX 4000 SFF Ada Generation, driver 595.99.02 (open kernel module), GCC 13.3.0, Release, bundled dependencies, SPIRV-Tools ON. Base main bc3c0fc13, this PR at 75f5beffa applied as its diff. Xvfb with -DANYPS5_VIDEO_OUT_TEST_DRIVER=x11; ANYPS5_GPU=RTX and ANYPS5_GPU=llvmpipe on the same machine.

A first-parent binary search over 979ad7b6a..bc3c0fc13 on video_out_flip_eop (RTX) also gives 8856f180c (#1202) as the first failing merge, with e49ec591d passing.

Build video_out_flip_eop / _main / _aftergpu VideoOut tests (21)
main, pipeline on (default) fail / fail / timeout on RTX and lavapipe 18 pass, 3 fail
main, APS5_PIPELINED_DRAWS=0 pass / pass / pass –
this PR, pipeline on and off pass / pass / pass; 5/5 repeats on RTX 20 pass, 1 skip (high_frame_rate, no 120 Hz on Xvfb) on RTX and lavapipe

ctest -R '^agc_' with this PR on RTX: 626/628. agc_driver_store_over_untracked is the NVIDIA failure #6385 fixes; agc_driver_store_idle_results failed once under --parallel 24 and passes 20/20 serial on RTX and lavapipe both on main and with this PR.

@JJenkx

JJenkx commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

Linux integration validation on latest main c8e3fcb703 (RX 6650 XT, RADV Mesa 26.2.4, GCC Release):

  • Before this fix: video_out_flip_eop and video_out_flip_eop_main fail; the newer video_out_flip_eop_aftergpu times out.
  • After applying exactly 75f5beffa0: all 14 currently registered video_out_flip_* cases pass, including eop_aftergpu and both CPU-flip ordering modes. The three sparse-import cases run alongside them also pass: 17/17 total, zero skips.
  • Rebuilt libraries and flip tests for both revisions. No test timeout was increased. The first two failures report EOP packet did not finish its label and reserved flip.

These are local integration results: the base also carries the existing Crimson Spires rendering/import fixes, so this is not a clean-main-only test. Integration pins before/after: 4aee85f5402cc8675f158541e96756e4ea286b9f / 3939d3e59f2c9d7f661bf743a6ce737c393f7382. No performance claim; Windows not tested.

AI-assisted: yes.

@Scramblrrrr

Copy link
Copy Markdown
Contributor

Independent Windows/NVIDIA cross-test, current main ca13a56 plus this PR's 75f5bef, no other patches:
https://github.com/Scramblrrrr/AnyPS5/tree/codex/videoout-eop-windows-cross-test
Exact derived head 5a5e5aa; your authorship preserved. Windows11 26200, Ryzen7800X3D, RTX5070Ti, NVIDIA617.42, 32GiB, WinLibs GCC15.2.0 r7 UCRT/POSIX/SEH, Ninja Release. Configured ANYPS5_VIDEO_OUT_TEST_DRIVER=windows, actual Vulkan windows (not offscreen).

  • Unmodified main ca13: video_out_flip_eop and _eop_main both fail at the existing completion assertion, 0/2 in11.82s. No fixture changes.
  • Same source plus your one-line fix: all14 ^video_out_flip_ CTests pass, 13.02s, zero skips. This includes _eop, _eop_main, _eop_aftergpu and both CPU-flip modes. Conventions pass.
  • We independently hit the same three failures on our separate 007 combined stack before finding your PR: EOP/main fail and aftergpu times out; with PIPELINED_DRAWS=0 those3 pass2.61s. A temporary local diagnostic showed label1/count0/pending0/currentBuffer-1, matching your diagnosis; removed before the positive run. Your fix is now carried in that stack too, with its expanded native gate still running. This is not a 007 title/FPS benefit claim.

Also confirms #6899 on Windows: our initial offscreen run skipped the display cases, and switching the test driver exposed this regression. No replacement PR opened. AI-assisted: Codex; only public synthetic fixtures used for these tests.

@adriadam10 adriadam10 self-assigned this Oct 19, 2026
@adriadam10
adriadam10 merged commit d8d0222 into boykopovar:main Oct 19, 2026
9 checks passed
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the one-line guard against the packet loop: submission.flips.contains(cursor) is the same test the flip branch uses later (presentation and GpuReady), so a RELEASE_MEM carrying a reserved flip now takes the normal path, which drains the pipeline first. The regression bisect to #1202 and the bad-main/good-PR results are confirmed independently on Linux RADV, NVIDIA and Windows by the reports above. Merging through the queue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc*

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants