perf(agc): sync only the source for DMA_DATA copies into host-only memory

[alfalfa-star]

What

A DMA_DATA copy larger than the store limit (64 KiB) runs as a transfer between host imports. When its destination is not registered guest memory, there's no import for it (the GPU can neither read nor write that memory), so the transfer was refused. The packet then drained the device: every batch recorded so far was submitted and waited for, and then the CPU copied.

Such a copy only has to see the source's results:

• The packet now flushes storage results pending over the source and waits only for the batches that write it (Recorder::SyncThrough). Then it copies on the CPU without a drain.
• No GPU work reads or writes the destination, so there's nothing on the GPU to order against the CPU store.
• The [sync] line counts these copies (copies into host memory synced on their source).

Tested

Linux 7.2.9 (Arch), Mesa 26.2.4 RADV, RX 9070 XT.

• PPSA14632 (LEGO Horizon Adventures) copies 256 KiB of GPU results into host-only memory about 50 times per 10 s, and each one was a device drain. With this change the run has 2 drains in total (one RELEASE_MEM, one other DMA_DATA) instead of ~50 per 10 s, and 526 copies synced on their source. The title is currently limited by GPU time, so this frees the queue 0 worker rather than raising the present rate on its own.
• ctest: 396 of 396 pass.

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 in TechnicalDebt
• 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
• AI-assisted: yes

🤖 Generated with Claude Code

[popcoder565]

Native Windows run, since no Windows results were posted. Setup: Windows 10 (19045), RTX 3060 Laptop (NVIDIA 616.92), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release. Head 9653aa91, merged onto main 8b393b6 in one build with 31 other PRs that had no Windows run.

• This PR has no test of its own. It builds, and the suite passes apart from the failure below.
• Full ctest in that build: 520 of 521. The one failure is guest_offline_imports, broken by feat(libSceRudp): implement sceRudpGetStatus #2359 (reported there).

No title was run.

🤖 Addressed by Claude Code

[adriadam10]

Thanks. The idea is sound, but one case changes behaviour compared with the drain it replaces:

• PacketMemory.cpp:148-153: the new branch runs for every CopyBuffer outcome other than path 1/3 and never looks at outcome.reason. For a host-only destination the GPU can't read or write it, but CopyBuffer also refuses with Label (PendingLabelIn covers labels, wide labels and queuedLabels) and DestinationUnsettled (VulkanDevice.cpp:1670-1677): CPU stores that are still pending over the destination. The drain let them land before the CPU copy; now they can land after it and overwrite the copied bytes. Please either keep the drain when the reason is Label/DestinationUnsettled/DestinationPending/Reader/Shadow, or show that none of them can happen with a non-registered destination.
• The branch is also taken for path 0, where the copy was already done on the CPU; the flush + SyncThrough there is wasted work. Restricting it to path 2/8 would keep it to the case the PR describes.
• There is no test. A guest/driver test that writes a deferred label (or a completion store) over the destination range before a >64 KiB DMA_DATA into host-only memory, and checks the final bytes, would pin the ordering. It should fail on the unguarded version.

[frankea]

Windows run of this PR alone on current main. Head 9653aa91 merged onto c72d5ce1 (clean, no conflict with the files the overlap bot lists); Windows 11 10.0.26200, RTX 4090 (NVIDIA 616.92, subgroup 32), MinGW-w64 GCC 15.2.0 r7, Release, BUILD_TESTING=ON.

• Full ctest: 573 of 573 passed; 3 environment skips (video_out_flip_lifetime, video_out_flip_controls, guest_video_out_wqhd_detection_no_vulkan), identical to main on this machine.
• SDL_DIRECTINPUT_ENABLED=0 was set for the run because of Windows: SDL DirectInput enumeration in sceVideoOutOpen blocks forever on a HID device, so video out never opens #1042.

No title was run. Tested with AI assistance (Claude Code).

[alfalfa-star]

Thanks. Rebased onto current main and reworked in 481c27b:

• Reason check. The packet calls CopyBuffer with cpuMax = 0, so refusal() stops at its first test and outcome.reason is always Size here; checking the reason would not have guarded anything. The branch now asks the device directly: new VulkanDevice::StoresPendingOver(destination, bytes) is true for a recorded write, a label (recorded or queued, PendingLabelIn), a copied write-back (CopiedWriterOverlaps), a storage image's pending write-back or a unit shadow over the destination. Any of those keeps the drain.
• Path 0/8. The branch is limited to outcome.path == 2. Path 8 can't occur from this call (gpuMax is SIZE_MAX), and path 0 can't either with cpuMax = 0.
• Test. testCopyIntoHostMemory in agc_driver_tests (Submit.cpp): a 128 KiB DMA_DATA writes a registered buffer on the GPU (path 1), then a 128 KiB DMA_DATA copies it into unregistered host memory (path 2). It runs twice, the second time with a WRITE_DATA label into the destination between the two copies. It checks the final bytes and that copiesToHostMemory counted the copy (no drain).

One caveat on the test: I couldn't make it fail on the unguarded version, because no reachable state leaves a store pending over a destination without an import when CopyBuffer returns path 2:

• A label into such memory never defers. WriteLabelOnGpu returns 3 (not imported) while anything is recorded, so the label packet drains, and the label case above lands before the copy that way.
• A completion store from a writer over the destination (a copied write-back) is noted as a pending write (MarkGpuWrites → NotePendingWrites). CopyBuffer waits for it (waitDestination → SyncThrough) before it reaches the import lookup that returns path 2.
• CopyBuffer also flushes storage images over the destination (FlushPending(destination, …)) before that lookup.

So the guard is defensive. It costs a few range lookups, and it keeps the branch correct if either of those paths changes. The test pins the ordering for both cases you named. It also turned out the CPU copy's own read of the source would see the GPU result even without the explicit SyncThrough (the guest-memory read path syncs pending writes), so the test checks the outcome, not that particular wait.

ctest: 638/638 on RADV (RX 9070 XT), and agc_driver_tests (not registered in ctest) passes. Not re-run on the title.

[adriadam10]

Thanks for the rework; the StoresPendingOver guard and the path 2 restriction address the earlier points. Two things left:

• core/libs/prx/libSceAgcDriver/tests/Submit.cpp:445-471: testCopyIntoHostMemory lives in agc_driver_tests, which is EXCLUDE_FROM_ALL and not registered in ctest (core/libs/prx/libSceAgcDriver/CMakeLists.txt:267), so it never runs in CI. New tests must be registered in ctest. Please put it in a target that is (e.g. one of the agc_driver_*_tests execution targets that use VulkanTestDevice), with a skip (exit 77) when there is no Vulkan device.
• The branch conflicts with main: VulkanDevice.hpp/VulkanDevice.cpp next to the new RecordedWritesSettled. I rebased it locally by keeping both functions; the scoped libSceAgcDriver ctest (319 tests) and agc_driver_tests pass on the result.

Once the test is registered and the branch is rebased this is good to merge.

[alfalfa-star]

@adriadam10 Thanks. I rebased onto main db343a9 in 14c2eca, keeping both RecordedWritesSettled and StoresPendingOver as you did.

On the test registration: agc_driver_tests is already in ctest. Its definition at CMakeLists.txt:282 is EXCLUDE_FROM_ALL, but under if(BUILD_TESTING) the loop at CMakeLists.txt:911 names it (foreach(agcTest IN ITEMS agc_shared_write_tests agc_driver_tests ...)), turns EXCLUDE_FROM_ALL off and calls add_test(NAME agc_driver ...) with SKIP_RETURN_CODE 77. Submit.cpp's main already returns VulkanTestSkipped (77) when there is no Vulkan device. In my build tree:

$ ctest -N | grep ' agc_driver$'
  Test #114: agc_driver
$ ctest -R '^agc_driver$'
1/1 Test #114: agc_driver .......................   Passed    3.15 sec

testCopyIntoHostMemory runs inside that test. If you'd still like it in its own tests/execution/ target, I can move it.

Full ctest on the rebased head: 698/698 (4 video-out flip tests skipped, no display), RX 9070 XT, RADV.

[github-actions]

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.

Pull request	Conflicting files (lines on main)	Same exports	Same new files
#374	core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketMemory.cpp L7
#425	core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketMemory.cpp L7

[orishamir]

On the two remaining points, checked on 14c2eca3 merged onto current main (57f96fd2; clean merge, RecordedWritesSettled and StoresPendingOver both present):

• Registration: agc_driver_tests is in the foreach(agcTest ...) list at core/libs/prx/libSceAgcDriver/CMakeLists.txt:911, which clears EXCLUDE_FROM_ALL and registers it as agc_driver with SKIP_RETURN_CODE 77; ctest -N lists it (Test #114: agc_driver) and it runs here on llvmpipe (passes, 3.2 s), so testCopyIntoHostMemory does run under ctest. Whether it runs in CI is a separate matter: the linux/windows jobs are gated to pushes, so no library test runs on any PR.
• The test pins the branch: with PacketMemory.cpp alone reverted to main (the VulkanDevice and statistics changes kept), agc_driver fails with a copy into host memory drained instead of syncing its source. Restored, it passes again, and the agc driver set is green.

AI-assisted: yes (Claude Code Fable 5.1 xhigh)

[adriadam10]

Checked on the PR merged onto current main: scoped build + ctest of libSceAgcDriver 713/713 (agc_driver runs the new test, also repeated 10x), and the test fails to build without the change (copiesToHostMemory), so it pins the new path, including the pending-label-over-destination case. The StoresPendingOver guard and the path 2 restriction cover the earlier review points; agc_driver_tests is registered in ctest through the loop in CMakeLists.txt:911. Merging.