Repository navigation
fix(agc): accept a compared host import for an initial multisampled DCC import - #9940
EagleBearerYS wants to merge 4 commits into
Conversation
github-actions
Bot
added
the
unblocks title
|
I reviewed the Linux comparison-mode path and its control cases. The change admits an initial DCC range only when a live unwatched host import covers the whole range, snapshots bytes and the post-read write stamp, then validates unchanged bytes/stamps on reuse; explicit unwatch, partial/misaligned/overlapping planes remain refused. The test additions exercise compare-mode detection, changed data, partial imports, explicit-unwatch controls and the Rush 2 import shape. I ran This head is 3 commits ahead/67 behind current main; my changed-path comparison found no files modified by main since the merge base that this PR itself edits. Please refresh/recheck the integration before merge, especially the two PRs' combined behavior. #9941's description requests #9940 first; it also reports a combined synthetic tree passing 77/77, which is useful but not a substitute for the final rebased heads. I found no independent code defect in this pass. I could not build locally because the filesystem is full. |
|
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.
|
|
I rechecked exact head I reviewed the paired sequencing too: #9940 is the first integration step, followed by #9941’s Windows pending-settle/rewatch handling. #9941 describes a combined |
|
Linux title run in comparison mode, which the description lists as pending: lavapipe (Mesa 25.2.8, Both arms log the probe result this PR targets:
So both titles get past the #9724 stop. The next stop is the One note for anyone repeating this in Docker: the default seccomp profile blocks |
|
The new Linux title run is useful: both games pass the original DCC refusal on the merged |
Hardware & Environment
Test Methodology & Verification
LGTM! AI-assisted: yes. Written with AI assistance and reviewed by me. |
|
Thanks, @klosixgaming-arch. That's the Windows side this PR should leave as it is, now checked on an RTX 5090: your probe keeps imports watched, the multisampled DCC tests pass, and the compared-import tests skip as intended. The Tested section now lists your run. |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
The Linux compared-import approach is distinct from #9941’s pending-Windows-settle path, and the branch declares the intended order (this PR before #9941). Its reported lavapipe title runs and test comparison are useful for that older tree.
I cannot approve the current integration yet. The merge base is eb8dcfebaa; this head is 618 commits behind current main 777bdbb4f8 (3 commits ahead). Since that base, current main has changed the same AGC integration paths: 511 lines in GuestBufferMemory.cpp, 119 in the AGC CMake registrations, and 18 in GuestBufferMemory.hpp, along with the debt index. Please rebase and preserve the declared ordering before #9941, then rerun the focused comparison-mode/MSAA DCC tests on current main and the refreshed head and recheck the combined tree. The current PR’s product jobs are skipped by policy, so I am relying only on the test runs actually reported in its description for the old base.
One convention warning also remains: the multisampled DCC debt entry is 1,585 characters (the prior main entry was 1,232). Please trim it to the unresolved behavior and assumptions so the refreshed conventions check no longer reports the debt-length warning.
|
A real-GPU run in comparison mode: AMD iGPU on Linux (RADV). Head Setup
The three failures are Unlike lavapipe, the 860M renders 2 samples, so Where it stops. I added a temporary print of the mode at the top of the loop, and one in
So the compared-import path this PR adds works on RADV: the imports, the reuse, and the refusals of modes 1-3 and 9-10. What fails is mode 5, which forces Same machine, lavapipe ( Validation: the loader inserted |
|
Thanks, @Lander0606, that pins it down: the three tests failed only in mode 5, the control that forces the imports to stay watched, and on a device whose probe reports watched pages written at the import and after a GPU read, that control can't hold. 54e2b57 runs mode 5 only where the probe reports neither, the condition #10138 uses for the forced-watch shadow tests, so lavapipe still runs it and the 860M skips it. Your run is in the Tested section. If you have time, could you run |
|
@EagleBearerYS, here's the rerun on the 860M at the current head Same setup as before: Radeon 860M, RADV KRACKAN1, Mesa 26.2.5, Fedora 44, kernel 7.2.11.
|
|
Thanks, @Lander0606, that closes it: the 860M now runs the whole set with the control case skipped where it can't hold. It's in the Tested section. |
|
Windows run on NVIDIA and Intel. The runs above are all Linux, where the probe picks comparison mode. Head 4df0813 was merged into Setup: Windows 11 Pro 10.0.26300, WinLibs GCC 15.2.0 r7, Release. Environment:
AI-assisted: yes (Claude Code) |
|
Thanks, @BKlaasWerkman, a Windows comparison against |
adriadam10
left a comment
adriadam10
left a comment
There was a problem hiding this comment.
main moved and this PR now conflicts with it, only in docs/dev/debt/libSceAgcDriver.md. The code in MultisampleTarget.cpp, GuestBufferMemory.* and the CMake merges cleanly, but replaying the commits one by one stops in MultisampleTarget.cpp (the #7948 merge commit is now part of main), so please update the branch against main yourself, with a single linear history.
Resolution for the debt file:
- keep
main'sMultisampled CMASK stateentry as it is (it now lists the 0xcccccccc/0xdddddddd/0xeeeeeeee/0xffffffff patterns; this PR doesn't touch it); - keep
main's newMultisampled DCC CMASK without fast clearsentry; - keep this PR's
Compressed FMASK readsentry (watched or compared planes).
Then re-run python3 tools/check_conventions.py --base origin/main (the DCC entry must stay under 1,000 characters) and the focused tests on the rebased tree.
@MarcZX10 your three requests on 0c1f6b5571 look addressed at 4df08139c0 (debt entry 970 characters, RADV 860M 38/38, Windows RTX 3060 Ti / Arc against main). Could you re-review once the author pushes the rebased branch?
…CC import Rush 2 Recompiled (PPSA69423) and Wave Race 64 Recompiled (PPSA69424) stop with "AGC graphics: initial multisampled DCC contents require watched guest memory" (boykopovar#9724). On Linux with lavapipe, the write-watch probe finds that device accesses to a host import trip the write watch, so every imported range is compared rather than watched. A multisampled DCC target whose color, DCC or CMASK range lies in such an import was then refused at its first draw: importing the target takes a write generation for each range, and a compared range has none. A range that is not watched but lies in a live host import the probe compares is now copied when the target is imported, and its metadata again when a whole fill is taken as its fast clear. Each later use compares the range with that copy, at most once per collect epoch, and counts any driver or GPU store over its 64 KiB blocks since the copy as a write, so a guest store of new bytes or any store by the driver refuses the resident target as a watched range does. A range left unwatched outside such an import is refused as before. This covers the Linux comparison mode, not the Windows deferred settle (BF-150): compared imports on Windows still refuse.
…MASK range
be write-watched at the initial import ("native FMASK contents require
watched guest memory") and at each reuse ("native FMASK sampling requires
watched guest memory"). In Linux comparison mode a host import over the
FMASK unwatches it, so the target is refused one check after the color,
DCC and CMASK ranges that the previous commit accepts.
Treat an FMASK range that a compared host import covers
(ComparedHostImportCovers) as the color and metadata ranges are treated:
keep a copy and its NewestStamp at the initial import, skip it in the
import's UnchangedSinceAll, and at reuse require the stamp and the bytes
to be unchanged, comparing them at most once per collect epoch. An
explicitly unwatched FMASK range is still refused.
Four-sample targets take no FMASK range, so this changes nothing for them.
Refs boykopovar#9724
…pages written DccComparedImportTests' mode 5 forces the write watch on the planes' host imports as a control. On a device whose import probe reports watched pages written at the import or after a GPU read, as RADV on a Radeon 860M does, the watched planes see those writes and the reuse check refuses with the resident-DCC message, so the three compared-import tests failed there in mode 5 alone. Mode 5 now runs only where the probe reports neither, the condition boykopovar#10138 uses for the forced-watch shadow tests; lavapipe still runs it.
4df0813 to
e2449e2
Compare
|
@adriadam10 Thanks, done. The branch is now four commits on
@MarcZX10 the rebased branch is pushed. Could you re-review? |
What
Rush 2 Recompiled (PPSA69423) and Wave Race 64 Recompiled (PPSA69424) stop at their first multisampled DCC target with
AGC graphics: initial multisampled DCC contents require watched guest memory(#9724). On Linux, babyhuey saw it with lavapipe onmain8b3a6ba: Rush 2 at 6.4 s, Wave Race 64 at 1.5 s (comment).Why Linux refuses. Linux has no deferred settle. When the write-watch probe finds that host imports lose their write protection, it compares every import instead of watching it. The probe then logs
[write-watch] host imports resolve write protection: yes (…); imported ranges are compared.initialDccCleartakes a write generation of the color, DCC and CMASK ranges and requires each one to be watched. So a target in such an import is refused at its first use.MarcZX10 asked to keep the reproductions split by that probe result (comment). This PR is his case (2), Linux comparison mode. The Windows case (1), a watched import whose deferred settle is still pending, is #9941.
The change:
ComparedHostImportCovers(context, address, bytes): true when a live host import marked unwatched covers the whole range. It returns false on Windows, so nothing changes there.initialDccClear: a range that isn't watched but lies in such an import is read once and kept as a copy, withNewestStamptaken after the read. Watched ranges take their generation as before.checkDccCurrenton reuse: a compared range is current while itsNewestStampis unchanged and its bytes equal the copy (EqualsCommittedUnsynced).ForgetSerialof the last equal compare.writes to a resident multisampled DCC target or its metadata are not implemented, and the pending-write refusal.absorbDccFill: compared metadata takes the landed key bytes of a whole fill as its new copy.use.samples == 8u), so this commit doesn't change them.GuestMemory::Unwatch;What a compared range can't see: a guest CPU store of the bytes already there. That is comparison mode's general limit, and the storage-texture compare has it too. A driver or GPU store into the range's 64 KiB blocks counts as a change through
NewestStamp, same-value stores included. The debt entry says both.Cost. Each use of a resident target with compared ranges takes one
NewestStampper compared range and at most one memcmp per range per collect epoch. That replaces a refusal at the first draw. memcmp on a cloud Xeon at 2.80 GHz:The memory cost is one copy of each compared range. A cheaper current check, such as a hash per 64 KiB block, would be follow-up work.
Commits:
fix(agc): accept a compared host import for an initial multisampled DCC import. It is on amainthat has feat(agc): expose expanded FMASK for tracked native owners #7948, soRecreatekeeps both resets, and the end ofinitialDccClearkeeps feat(agc): expose expanded FMASK for tracked native owners #7948's FMASK check.fix(agc): accept a compared host import for a multisampled target's FMASK range.test(agc): skip the watched-import control case where imports report pages written(test only). Mode 5 forces the planes' imports to stay watched, as a control. On a device whose probe reports watched pages written at the import or after a GPU read, the watched planes see those writes and the reuse check refuses, so mode 5 now runs only where the probe reports neither, the condition test(agc): skip the forced-watch shadow tests where imports report pages written #10138 uses for the forced-watch shadow tests.docs(debt): shorten the native multisampled DCC fast clear entry(docs only): the entry is under the 1,000-character limit, with the same cases.Tested
agc_driver_msaa_dcc_compared_import_{2,4,8}(new,DccComparedImportTestsintests/execution/AlphaToCoverage.cpp).NewestStampUnwatched, nothing importedmainmain, passesUnwatchedDccRefillTestsmode 6, a metadata fill in forced comparison mode, used to expectrequires watched guest memory. It now expects the fill to clear the target.DccFastClearTestsmode 4 (an explicitUnwatchover the DCC metadata) is unchanged. It refuses on both trees.ANYPS5_REQUIRE_VULKAN=1 GALLIUM_OVERRIDE_CPU_CAPS=avx):main's product files with the new tests:compared_import_2,_4and_8fail with the title's text.msaa_dcc_clear_4fails withmultisampled DCC target reuse requires watched guest memory(the refill case)._4andclear_4pass, 20/20 each with--repeat until-fail:20. The 2- and 8-sample tests skip, because lavapipe renders color only at 1 and 4 samples._8fails withnative FMASK contents require watched guest memory.ComparedHostImportCoversin the FMASK check fails mode 11.agc_driver_native_fmask, which skips for the same reason.ctest -R 'msaa|dcc|fmask|multisample': 38/38, 6 skipped for sample counts.main's product files (compared_import_2,_4and_8fail), and green ×10 (13 tests; 5 skip on lavapipe for their sample counts). The libc and libkernel suites pass on Linux. On Windows it builds and passes ×10, and its Vulkan tests skip.guest_raise_in_system_callfailed on both arms, with and without this PR. It also timed out twice onmain's own Windows Build today (run, run). This PR changes nothing in libkernel.APS5_SKIP_TIMING_TESTS=1, which relaxesprecise_wait's median bounds on GitHub's hosted runners.main, green ×10, suites pass; Windows builds, its Vulkan tests skip, suites pass.agc_driver_msaa_dcc_compared_import_4and_8,agc_driver_msaa_dcc_clear_4,agc_driver_sample_multisample_targetandagc_driver_native_fmask, one run on each tree. With the change,agc_driver_msaa_dcc_compared_import_4,agc_driver_msaa_dcc_clear_4andagc_driver_sample_multisample_targetpass, andagc_driver_msaa_dcc_compared_import_8andagc_driver_native_fmaskskip on llvmpipe.main's tree has three of the five:agc_driver_msaa_dcc_clear_4andagc_driver_sample_multisample_targetpass andagc_driver_native_fmaskskips. None fail. The layer loaded 3 times onmainand 5 with the change. Both logs report onlyVUID-VkDeviceCreateInfo-pNext-pNext, so the change adds noVUID-message.VUID-VkDeviceCreateInfo-pNext-pNext, on both arms.main895f47f and on thatmainwith this head merged. Both arms logimported ranges are compared.initial multisampled DCC contents require watched guest memoryonmain.the device cannot render color with the target's sample count. That is lavapipe's limit: it renders color only at 1 and 4 samples.[write-watch] host imports of write-watched memory keep the write watch: yes.agc_driver_msaa_dcc_stateandagc_driver_msaa_dcc_clear_2,_4and_8pass.agc_driver_msaa_dcc_compared_import_2,_4and_8skip with their Linux-only message, as this PR intends on Windows.maineb8dcfe,ctest -R 'msaa|dcc|fmask|multisample', three runs without and three with the Khronos validation layer (1.4.341). The probe picks comparison mode there, with pages reported written at the import and after a GPU read.main: 35 of 35 pass in every run. 0c1f6b5: 35 of 38, withagc_driver_msaa_dcc_compared_import_2,_4and_8failing in every run, in mode 5 alone, withwrites to a resident multisampled DCC target or its metadata are not implemented. With mode 5 skipped in a local edit, all three pass at 2, 4 and 8 samples, so the compared-import path holds on RADV.VUID-line.msaa|dcc|fmask|multisampleset's 38 tests, 32 pass 10 runs in a row and 6 skip for sample counts (agc_driver_msaa_dcc_compared_import_4passes;_2and_8skip); none fail. On Windows, 5 pass 10 runs in a row and 33 skip without a Vulkan device. The libc and libkernel suites pass on both, run withAPS5_SKIP_TIMING_TESTS=1, which relaxesprecise_wait's median bounds on GitHub's hosted runners. GitHub's runners can't show the red: it needs a device whose probe reports those writes, as the 860M's does.agc_driver_msaa_dcc_compared_import_2,_4and_8print the control-case skip line and then pass, with all five FMASK cases at 8 samples. On lavapipe on the same machine (Mesa 26.2.5),_4and_8pass without that line, so mode 5 still runs there, and_2skips for the sample count. The layer loaded in every run, and no run logs aVUID-line.main15e71db (clean) and compared with thatmainon the same machine; Windows 11 Pro, WinLibs GCC 15.2.0, Release,ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1, with the Khronos validation layer (1.4.363) loaded; the 1,282 teststools/select_tests.pypicks for this diff.main, the only difference on each GPU is the 3 new tests, which skip.agc_driver_msaa_dcc_compared_import_2,_4and_8skip on both GPUs with their "Linux comparison mode only" text, as intended.agc_driver_alpha_to_coverageincluded, and the validation messages are the same asmain's on both GPUs. The failures fail onmaintoo: theagc_driver_sample_ordertrio on NVIDIA (test(agc): read the sample order tests' depth through a sampled copy #9056), andagc_storage_write_backandagc_driver_multisample_retirementon Intel.check_conventions.py: passes with no warning: the fourth commit shortens the multisampled DCC debt entry from 1,585 characters to 970, under the 1,000 limit. No comment lines are added.main672c38f, after adriadam10's review (review): the four commits were replayed onmainwith no merge: 27371e9 → ff167c3, 0c1f6b5 → 1fc53ab, 54e2b57 → 0045c93, 4df0813 → e2449e2.MultisampleTarget.cppas the earlier merge did. e2449e2's tree is identical to that merge of 4df0813 withmain672c38f, and it merges cleanly withmain00198bf.main'sMultisampled CMASK stateandMultisampled DCC CMASK without fast clearsentries as they are. This PR changes only theNative multisampled DCC fast clearsentry (970 characters) and theCompressed FMASK readsentry (watched or compared planes).python3 tools/check_conventions.py --base origin/mainpasses againstmain00198bf.^agc_driver_msaa_dcc_withagc_driver_native_fmask,agc_driver_sample_multisample_target,agc_driver_sample_order,agc_driver_multisample_retirementandagc_storage_write_back, ×10 on that tree (run): Linux: 12 run, 7 pass, 5 skip (agc_driver_native_fmaskand the 2- and 8-sample cases, for lavapipe's sample counts), 0 fail. Windows: 12 run, 1 pass (agc_driver_msaa_dcc_state), 11 skip (no Vulkan device), 0 fail. The libc and libkernel suites pass on both, run withAPS5_SKIP_TIMING_TESTS=1, which relaxesprecise_wait's median bounds on GitHub's hosted runners.Overlap:
initialDccClearand the FMASK checks. They compose with the settle first: a pending Windows settle returns the range to the watch, and anything still compared takes this path.test/9724-both-main(6a9d648) is that resolution, and itsimport|settle|dcc|msaa|alpha_to_coverage|fmask|multisampleset passes 77/77. Merge order: this PR first.tests/Recorder.cppconflict is withmain. This PR doesn't touch that file.maincaa089f: clean.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)initial multisampled DCC contents require watched guest memory(babyhuey's lavapipe runs; their next stop there is lavapipe's sample-count limit)