Repository navigation
fix(agc): publish a reused native multisample depth target with the guest's undecided bytes, then retire its owner - #15941
Conversation
… a native depth owner
|
A GFX8 data point: AMD Radeon RX 570 (RADV POLARIS10), Mesa 26.2.5, SteamOS 3.8.28 (Arch distrobox), GCC 16.2.1, Release. Head On GFX8, 11 of the 17
The 1,327 AGC, shader, VideoOut and Vulkan tests, this PR on
AI-assisted: yes (Claude Code) |
|
@JJenkx thank you for the three title runs, and @ryanc2112 for the RX 570 run. I've added both to the description.
The PR stays a draft until Blue Prince (PPSA25009) is in, since its intro is the case that needs the guest's byte. |
|
Windows check of head Setup: Windows 11 Pro (10.0.26200), WinLibs GCC 15.2.0 r7 UCRT, Release,
The skip is the pre-existing AI-assisted: yes. |
What
Since #9533, a multisampled D32S8 target whose stencil has its own plane and whose HTILE is depth-only is imported as a native target. #9533 was merged together with #9717, in
a4b76b2c0a.Four titles that ran before that merge stop on
main. JJenkx bisected each one toa4b76b2c0a: Curved Space (PPSA02817, #8691), NINJA GAIDEN: Ragebound (PPSA28002, #8671), Blue Prince (PPSA25009) and New Joe & Mac: Caveman Ninja (PPSA02801, #9091). Onmainthey stop with:What reaches the publication. JJenkx traced it in all four (#8671). Each title reuses the memory of a host-written native target for something new:
Why it stops. Building the new resource publishes the old target's planes to the guest (#13021).
a native multisample depth attachment over the planes of a host-written owner of another identity is not implemented.The change:
DepthReadCause::Access, the default, orDepthReadCause::Reuse.Reusemarks resources built over the memory:Draw.cpp's vertex and index buffer reads; the two storage-image accesses inShaderResources.cpp, which also cover the colour-target lookup fromrefreshResidentTarget; and a new depth surface's three planes.Access: CPU reads, DMA copies, texture reads, shader buffer accesses, dispatch-written ranges and the native sampling path.mainrefuses an undecided byte, aReusepublication takes the guest's byte. Every byte the publication can decide is decided as before:Reusepublication commits, the old owner is retired: its guards and its owner entries are removed. It is then inactive and no longer host-written, soreleaseRetiredfrees it at the next depth-surface creation (fix(agc): free native multisampled depth surfaces after their guest release #15300).Accesspublication leaves the owner resident, as before.Why the guest's byte. Before #9533 these targets were not native, so a resource built over their memory read the guest's bytes everywhere.
mainis, sincemainstops. It keeps the host's bytes everywhere the publication can decide them.What isn't proven. The untouched bytes on a page the title wrote are inferred, not proven. The write tracking can't tell a byte the title never touched from one it stored with the same value.
Tested
Linux, on a cloud machine: Ubuntu 24.04, GCC 13.3, CI's configure flags, lavapipe (Mesa 25.2.8, LLVM 20.1.2). The tests ran as an unprivileged user with
ANYPS5_REQUIRE_VULKAN=1 GALLIUM_OVERRIDE_CPU_CAPS=avx.New cases in
agc_driver_native_depth_publication(CheckReusePublication):The fixture: a 4-sample D32S8 target with a separate stencil plane and a depth-only HTILE. The host draws depth and stencil, then one CPU store leaves the rest of the stencil plane's first page undecided.
Red on
mainacf7ac208a, green here. Onmainall four fail with the refusal above.The checks: the guest's bytes on the undecided page, the host's bytes elsewhere in the plane, every depth sample, and the old owner retired. A rebind of its identity takes a fresh import. In (3) the new surface imports.
The same on both:
The existing tests, 49 in all: the native depth, write-tracking, guest memory and multisample tests (
ctest -R 'native_depth|native_multisample|depth_publication|write_track|guest_memory|multisample' --repeat until-fail:3).mainand here: 47 pass and 2 skip, ×3. The skips areagc_driver_multisample_centered_linesandagc_driver_native_depth_bda_clear, which skip themselves on lavapipe.Mutations:
Reusefails (4)-(8).Checks:
tools/check_test_registration.py buildandtools/check_conventions.py --base origin/mainpass.The red/green is from the cloud run (lavapipe, CI flags). The GitHub ×10 runs the branch only, because
mainlacksDepthReadCause.On today's
main: the four commits were rebased onto9ab6a281eawith the same lines. GitHub's runners, ×10 on that tree (run), the same tests: on Linux, 52 tests, 50 pass and 2 skip; on Windows, 53 tests, 8 pass and 45 skip. The libc and libkernel suites pass on both.With #15300, #15301 and #15855: the branch merged with
main9f35c937daand #15855 (since merged), the same tests, ×10 on GitHub's runners (run): on Linux, 52 tests, 50 pass and 2 skip; on Windows, 53 tests, 8 pass and 45 skip. The libc and libkernel suites pass on both.GitHub's Linux and Windows runners, ×10, on
acf7ac208a(run). The tests matchingnative_depth|write_track|guest_memory|multisample:agc_driver_native_depth_publicationpassed 10 of 10.agc_write_trackingtests and threeguest_memorytests run and pass.Validation layers (CONTRIBUTING's AGC rule; run).
VK_LAYER_KHRONOS_validationran on GitHub's Linux runner, withmain's tree and this branch each running the 35 tests matchingnative_depth|multisample.VUID-VkDeviceCreateInfo-pNext-pNext, onmainand here. This change adds none.The titles, run by JJenkx on this head merged onto
mainc4469b60da: Linux 7.2.11, AMD Radeon RX 6650 XT (RADV, Mesa 26.2.6), Release,APS5_PLAYGO_UNLISTED_CHUNKS=installed, 125 s with Cross every 15 s from 30 s. On thatmaineach title stops with the refusal above. With this PR each one gets past it and runs to the scheduled close, exit 0:main. Here it runs 122 s, and the story intro plays at 60 s.main. Here it runs 121 s, and the "Keep Going?" campaign screen is rendered at 60 s.main. Here it runs 123 s, and at 60 s the "Classic Modes" menu, opened with Cross, is shown.AMD Radeon RX 570 (RADV POLARIS10, GFX8), run by ryanc2112 (comment): this head merged onto
main7ad9749ed0, Mesa 26.2.5, Release, withVK_LAYER_KHRONOS_validation.agc_driver_native_depth_*tests, 3 runs each: on GFX8, 11 of them fail onmain,agc_driver_native_depth_publicationamong them, because the sample shadow lacks depth-attachment usage; fix(agc): give a multisampled depth-stencil sample shadow attachment usage #15750 fixes that. This PR alone gives the same 6 passes and 11 failures asmain. With fix(agc): give a multisampled depth-stencil sample shadow attachment usage #15750, all 17 pass onmain, and with this PR added all 17 still pass, the newCheckReusePublicationcases included. None of these runs reports aVUID-error.main, once each: the same results asmain, with 1,313 passing, the same 14 failing and 40 skipped, and the same 26pAttachments-00605validation errors on both sides (AGC: pipelines use differing per-attachment blend states while independentBlend is never enabled (VUID-VkPipelineColorBlendStateCreateInfo-pAttachments-00605) #11990). The 14 failures are known GFX8 or driver issues: the 11 above (fix(agc): give a multisampled depth-stencil sample shadow attachment usage #15750),agc_driver_f32_fma_denormal_mode2(agc_driver_f32_fma_denormal_mode2 fails on RADV before RDNA2: Mesa splits GLSL Fma, even with NoContraction #15316),agc_driver_tessellation(agc_driver_tessellation segfaults on RADV with Mesa 26.2+: nir_opt_varyings corrupts the TCS input offset's use list (Mesa bug) #5444) andagc_driver_sample_order_recipe_depth_locations(AGC: agc_driver_sample_order_recipe_depth_locations fails on RADV Polaris: the recorded 2x depth draw is mostly at the standard locations, its recipe replay at the custom ones #9759).Pending: JJenkx's run of Blue Prince (PPSA25009) on this branch: how far it gets, any new stop line, and whether its intro renders. Blue Prince is the title whose intro renders only with the guest's byte (above), so its run checks the byte this PR takes. This PR stays a draft until it's in.
Overlap
DepthSurface.hpp,DepthSurface.cpp(prepareDepthSampling) andNativeMultisampleDepth.cpp, and merges clean (git merge-tree, both on9ab6a281ea).cachedStorageTexturepublishes it. With this PR, that storage access is a reuse, so the publication has already retired the owner and removed its guards, and fix(agc): release a published native multisample depth owner whose plane a written storage view covers #15707's release skips it as retired. They differ where a storage view overlaps an owner without covering a plane: fix(agc): release a published native multisample depth owner whose plane a written storage view covers #15707 keeps that owner, and this PR retires it after the reuse publication.NativeMultisampleDepth.cpp, and keeping both sides resolves that file. The rest merges clean.PublishToGuest, and reclaims a D32 owner once its publication has completed and its sampled views are released.PublishToGuest's signature, where this PR adds the cause and fix(agc): reclaim published native depth owners safely #10469 adds the D32 check, and in the debt entry, which both rewrite. Keeping both sides resolves them.NativeMultisampleDepth.cpp, and keeping both sides resolves it.releaseRetiredfree a native multisample surface once it's inactive and no longer host-written. Before it, a retired multisample owner stayed listed. After this PR's reuse publication the old owner is both, so fix(agc): free native multisampled depth surfaces after their guest release #15300 frees it. fix(agc): let a sampled depth shadow reclaim memory or fall back to the host #15301 (Prophet069, merged) changes the sampled depth shadow's allocation inDepthSurface.cpp, away from this PR's hunks. The branch merges clean with both and with fix(agc): allocate the sampled depth shadow through the image allocator #15855, and withmainacac7c8541.DepthSurface.cppand the AGC debt file. Its stencil baseline runs only for surfaces whose stencil plane can be mirrored, which are single-sample, so this PR's multisample reuse path doesn't reach it. It leavesretireStencilunchanged. They merge 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)