Repository navigation
fix(agc): attach a smaller native multisample depth extent with the same Z_X layout to its owner - #15690
fix(agc): attach a smaller native multisample depth extent with the same Z_X layout to its owner#15690hugodearaujo wants to merge 7 commits into
Conversation
…ILE to its depth and stencil planes
…ame Z_X layout to its owner
…ication # Conflicts: # docs/dev/debt/libSceAgcDriver.md
✅ NVIDIA GPU test: passedPR head
Unstable on main (4, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Validation IDs already present on main (13 IDs in 547 tests)Skipped (49)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
# Conflicts: # core/libs/prx/libSceAgcDriver/tests/cmake/agc_driver_native_multisample_depth_tests.cmake
|
Merged #15688's updated head ( |
✅ AMD GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (7, not counted against this PR)Validation IDs already present on main (8 IDs in 544 tests)Skipped (60)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
✅ NVIDIA GPU test: passedPR head
Unstable on main (2, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Validation IDs already present on main (13 IDs in 544 tests)Skipped (49)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
|
Hi hugodearaujo, thanks for the focused smaller-extent regression and the detailed paired results. Could you remove the assertions in CheckSmallerExtent that require the other-identity attachment refusal and the smaller sampled descriptor refusal? These are valid paths whose support is currently unimplemented, so the assertions would fail when support is added. Please keep the production guards and the positive owner-view reuse and per-sample readback checks, including preservation outside the smaller extent. In the stacked publication test, the assertion requiring guest writes over native multisample depth to remain unimplemented has the same issue. The race and data-preservation checks can remain. This does not ask for the unsupported paths to be implemented in this PR. |
… stay unimplemented
…lemented, not refused by design
|
@oneandonlydean thanks, done. The test-only changes keep the production guards:
On the top of the stack, with |
✅ NVIDIA GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Validation IDs already present on main (13 IDs in 544 tests)Skipped (49)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
✅ AMD GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (7, not counted against this PR)Validation IDs already present on main (8 IDs in 544 tests)Skipped (60)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
|
Windows and NVIDIA Blackwell run, for head
The failure, This head contains #15688's head PPSA13960 was not run. Edited after posting: added the validation row and AI-assisted: yes. Claude Code ran the builds and tests and drafted this comment; the numbers are from the runs on this machine. |
Superseabas
left a comment
There was a problem hiding this comment.
Head 6c1c4ef23f merged onto current main b11c7bf84f (clean merge, local only): code read, red/green check and a run on AMD's Windows driver, which no run of this stack has covered yet, with an NVIDIA Blackwell run alongside.
Windows 11 Home 10.0.26200, WinLibs GCC 16.2.0 (x86_64-ucrt-posix-seh, r3), Release, BUILD_TESTING=ON. ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation, ctest -R '^agc_driver_native_(depth|multisample_depth).*$', ANYPS5_GPU selecting the device (confirmed by Selected GPU). In each tree one test, agc_driver_native_depth_driver_queued_fill, skips on Windows as on main:
| GPU | Driver | main |
#15688 merged | this PR merged |
|---|---|---|---|---|
NVIDIA GeForce RTX 5070 Ti (10de:2c05) |
621.01 | 16 + 1 skip | 18 + 1 skip | 19 + 1 skip |
AMD Radeon Graphics, Ryzen 7 7700X iGPU (1002:164e) |
AMD 26.10.2 | 16 + 1 skip | 18 + 1 skip | 19 + 1 skip |
- There are no
VUID-lines and no missing-layer line in any run. - Red check: the merged tree with only this PR's own
DepthSurface.cppchange reversed (git diffof #15688's headfa590e435aagainst this head, applied in reverse) fails the newagc_driver_native_depth_smaller_extenton both GPUs. It fails witha native multisample depth attachment over the planes of a host-written owner of another identity is not implemented, the stop the description names. check_conventions.py --base mainpasses on the merge.
Read through the change:
sameLayoutWithinaccepts only a multisampled owner without HTILE and withdepthSwizzle24 that is at least as wide and tall as the target. The target, resized to the owner's extent, must besameSurface. For the depth plane and for the stencil (or the depth width again when there is no separate stencil), the Z_64KB_X layout must give the same pitch, padded height and byte size at both extents, so the smaller target addresses the same bytes as the owner.holdsTargetreplacessameSurfacein the write notice, the sample-order and sample-location lookups and the feedback check. The view lookup only falls back to asameLayoutWithinowner when no surface matched.DepthSurfaceOrderTargetnow returns the owner's extent, which matches the image it hands out.- The debt entry states what stays unimplemented. Per the requested change, the test keeps the positive reuse and per-sample readback, including bytes outside the smaller extent, and no longer asserts the refusals.
I found nothing to change.
AI-assisted: yes (Claude Code)
What
Assassin's Creed Mirage (PPSA13960) uses dynamic resolution on a 4-sample D32S8 depth target without HTILE. It draws the planes at 1920x1080, a guest access publishes them (#15688), and the next frame binds the same depth and stencil addresses with every field equal except the extent, 1872x1056. A new owner over the planes of a host-written owner is refused, so the title stops with
a native multisample depth attachment over the planes of a host-written owner of another identity is not implemented.On the console both extents address the same memory. For 4-sample Z_X, a 64 KiB block is 64x64 pixels for 32-bit depth and 128x128 for the 8-bit stencil plane. 1872x1056 and 1920x1080 pad to the same pitch and padded height on both planes (30x17 and 15x9 blocks), so every pixel of the smaller extent has the address it has in the larger one.
A multisampled depth target without HTILE is now drawn into a resident owner when:
MultisampleTextureLayout, are the owner's.The owner keeps its image and samples. The draw's framebuffer is the render extent, which may be smaller than the attachment. The same rule applies where a draw notes its depth write and where the sample-order and sample-location paths look up the owner. Those paths take the owner's extent for the reorder, since the whole image is in host order.
Still refused, as before:
native multisample depth sampled identity or format mismatch).Tested
Linux 7.2.11, AMD Radeon RX 9070 XT, RADV (Mesa 26.2.6), Release with tests.
agc_driver_native_depth_smaller_extent(--smaller-extent), at 2, 4 and 8 samples:a native multisample depth attachment over the planes of a host-written owner of another identity is not implemented.VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation,ANYPS5_NO_SHADER_CACHE=1,ANYPS5_REQUIRE_VULKAN=1), the 16 tests ofagc_driver_native_multisample_depth_tests. On fix(agc): publish a written native multisample D32S8 owner without HTILE to its depth and stencil planes #15688's head288fc4e584(onmainacf7ac2): withDepthSurface.cppfrom fix(agc): publish a written native multisample D32S8 owner without HTILE to its depth and stencil planes #15688 and the new test, 15 of 16, the new test failing as above; with this change, 16 of 16. Neither log has aVUID-line or "was not found but was requested".tools/check_conventions.py: passes.main1c8c0ef + fix(agc): store storage results pending under a multisampled color target before it takes the memory #13963 (fc1dfc0, needed for an earlier stop) + fix(agc): publish a written native multisample D32S8 owner without HTILE to its depth and stencil planes #15688:depth 0x1249300000 stencil 0x124b2e0000 htile 0 1872x1056over the published 1920x1080 owner of the same addresses, every other field equal.native multisample depth publication over pending storage image writes is not implemented. That is a separate stop.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)