Repository navigation
fix(agc): keep resident linear color target results pending on the GPU - #9276
KarthikeyaAnna wants to merge 1 commit into
Conversation
|
This pull request is labeled |
PR boykopovar#6892 added a resident image for linear color targets over imported memory, but keepRecordedDraw unconditionally called StorageTexture::FlushPending after every single draw. In titles such as METAL GEAR SOLID Master Collection Vol. 1 (PPSA14363) whose PS1 emulator draws into three 24 MiB linear targets with up to 80 draws per frame, this forced thousands of unnecessary whole-target write-backs over PCIe (72-76 GB per 10 s) and repeated pending-write synchronizations, dropping framerates from 60 to 10 FPS. Furthermore, eager per-draw write-backs could overwrite CPU stores made outside the scissor after Draw returned. Like tiled resident targets, resident linear targets now keep their drawn results pending on the GPU rather than flushing eagerly on every draw. They are stored to guest memory as needed by existing access paths, waits, flips, and idle stores.
f9cf0a8 to
a8475a6
Compare
CnRJay
left a comment
There was a problem hiding this comment.
The diagnosis in #9217 is a real cost (72-76 GB per 10 s of write-backs from a flush after every draw), but the change as submitted is one deleted loop in keepRecordedDraw plus tests, and it moves the correctness burden to a place that can't carry it on every host.
- The eager
FlushPendingafter each draw was the guarantee that a CPU reader of a resident linear target sees the results. The description says the results are now stored "as needed by existing access paths, waits, flips and idle stores". Those paths are triggered by the write watch and the read hook. On Windows, host imports sit outside the write watch (that is the premise of #6960 and thedecideImportWatchrule), so a CPU read of the target's memory cannot be intercepted and nothing causes the store. For a PS1 emulator that draws into linear targets, the question is whether anything reads those targets back on the CPU (readback for effects, screenshots, savestates, its own VRAM view). Please say what a CPU read of an unwatched linear target does after this change, with a test for it on a host-imported (unwatched) block; the new cases build their memory withBlock, which followsWriteWatched(), and the description doesn't say that any of them runs over an unwatched import. - The performance claim is not measured. The motivation is MGS Master Collection (60 to 10 FPS, 72-76 GB per 10 s), and the PR's evidence is ctest on an RTX 2050 running a Linux 570 driver. There are no MGS summary lines, no write-back volume after the change and no frame rate. Please post the
aps5-summary.jsonllines of main and this PR from the same scene in PPSA14363, and the bytes written back per 10 s, per the Performance report form. - "Idle stores" run when the queue goes idle. A title that draws all frame and is never idle then holds three 24 MiB targets pending for the whole run, plus whatever each later pending-write check costs. Show a pending count and size over time in the MGS scene so the held memory and the sync work are visible.
- #8246 ("preserve CPU writes after resident linear target draws", 1,686 lines) addresses #8152 as well, and this PR says it fixes #8152 too. The two should be reconciled by their authors before either lands; as it stands one of them is wrong about who owns that guarantee.
- The test file grows by about 15 cases, 40+ lines of programs and blend helpers, for a one-line production change. Several (blend modes, write masks, scissor partitioning) test the draw path rather than this change. The description should say which cases fail with the deleted loop restored, since those are the ones that show the behavior.
|
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.
|
What
Keep resident linear color target results pending on the GPU until explicitly accessed, presented, or idle, matching the behavior of tiled resident targets.
PR #6892 added a resident image for linear color targets over imported memory, but
keepRecordedDrawunconditionally calledStorageTexture::FlushPendingafter every single draw. In titles such as METAL GEAR SOLID Master Collection Vol. 1 (PPSA14363) whose PS1 emulator draws into three 24 MiB linear targets with up to 80 draws per frame, this forced thousands of unnecessary whole-target write-backs over PCIe (72-76 GB per 10 s) and repeated pending-write synchronizations, dropping framerates from 60 to 10 FPS (#9217). Furthermore, eager per-draw whole-target write-backs could overwrite CPU stores made afterDrawreturned (#8152).Resident linear targets now keep their drawn results pending on the GPU rather than flushing eagerly on every draw. They are stored to guest memory as needed by existing access paths, waits, flips, and idle stores.
Expands
tests/execution/LinearTargetResident.cppwith 15 comprehensive edge-case tests:ExpectSingleDraw).ExpectConsecutiveDraws).ExpectDrawAcrossBatches).ExpectMultipleSequentialDraws).StoreIdleResults) flushing pending storage textures to guest memory (ExpectStoreIdleResults).GuestMemory::Writepreserved alongside preceding and subsequent draws (ExpectCpuWritesWithGuestMemory).ExpectDirectCpuWriteAfterDraw).VK_ATTACHMENT_LOAD_OP_LOADand GPU blend math (ExpectAdditiveBlending).colorWriteMask) preserving unmasked channels without corruption (ExpectColorWriteMask).ExpectOverlappingScissors).ExpectMultipleLinearTargets).ExpectPartialGuestRead).ExpectEmptyScissorDraw).ExpectTargetReallocationWithoutRead).ExpectDirectCpuInterleavedDraw).Tested
python3 tools/check_conventions.py --base origin/mainpasses with zero violations.ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation ctest --test-dir build -R agc_driver_linear_target_resident -Vpasses with 0 validation errors on NVIDIA GeForce RTX 2050 (Vulkan 1.3, driver 570.86.16).linear_display_bufferand related resident tests pass.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)