Repository navigation
fix(agc): take a later slice's HTILE clear when every word has ZMask 0 - #7194
Conversation
A bind of slice 1 or later of a depth array read the slice's HTILE but took its clear only when all its words were equal, because the HTILE word layout of those slices is not modelled. Neither step needs the layout: the clear is taken when every word of the range has ZMask 0 (a check over the set of words), and the store-back rewrites each word in place from its own bits. The value the surface is cleared to is DB_DEPTH_CLEAR, which the DB's depth test reads for every cleared tile whatever its Min/Max Z (HTILE_Z_CLEAR_REG in Mesa's src/amd/common/ac_descriptors.h), the same rule slice 0 already follows. PPSA05684 resets the four slices of its 2048x2048 D16 sun cascade array every frame by writing ZMask 0 words with Min/Max Z 0 or 1 into their HTILE. Slices 1 to 3 kept the previous frame's depth, and the main menu lost its volumetric sun shafts and sunlit foliage. agc_driver_depth_fast_clear_submit: slice 1 with words alternating 0x0 and 0xfffffff0 takes the clear, stores them back expanded and leaves slice 0's HTILE alone. The refusal case keeps a word left expanded and is renamed. AI-assisted: yes
|
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.
|
|
Second NVIDIA datapoint plus first Intel, head
|
|
@AlxcBTW, could you take a look at this one? It drops the equal-words condition you kept on #2053/#5627 (your 2026-10-13 comment: keep the per-tile decode to slice 0 and refuse slices >= 1). The argument is that |
|
AMD data point, adding to the NVIDIA and Intel runs: head
Not checked: other tests, titles. AI-assisted: yes (Claude Code). |
|
Checked this branch in the title. On the local integration build, I replaced the old local HTILE reset with this change: reverted the three local commits and applied ac840ec, which applied cleanly. The main menu's sun shaft and sunlit foliage are back and match the earlier reference frames. With AI-assisted: yes |
github-actions
Bot
added
the
unblocks title
|
Menu frames from PPSA05684, as promised in the description. Left: this change (ac840ec) on the local integration build. Middle: the earlier local HTILE reset (the reference). Right:
AI-assisted: yes |
4 participants
What
When a depth target is bound,
storeBackClearedHtilereads its HTILE if the HTILE was written since the last read. If every word has ZMask 0, the surface takes the fast clear toDB_DEPTH_CLEARand the words are stored back as expanded. For slice 0 this holds whatever the words' Min/Max Z are. For slice 1 or later of a depth array,maintakes the clear only when all words are equal (cc022df, from #5627). Otherwise it keeps the previous depth.Uncharted: Legacy of Thieves Collection (PPSA05684, #4001) has a sun cascade array: four 2048x2048 D16 slices at
0x1492ab0000+ 0x800000 per slice. Their HTILE starts at0x14929b0000, 0x40000 per slice (HtileMetadataBytes, as #5111 lays it out). The title resets the whole array every frame by rewriting the HTILE itself. It uses no fill or kernel the driver recognizes. It writes depth-only clear words with ZMask 0 and either Min/Max Z 0 (0x00000000) or Min/Max Z 1.0 (0xfffffff0), mixed within each slice. Undermain's rule:The main menu loses the diagonal sun shaft and the sunlit foliage (side-by-side menu frames, left with later slices taking the clear, right with
main's rule, will be added in a comment). The orphanage chapter is a night scene and looks the same either way.Why the slice layout does not matter here
@AlxcBTW kept the equal-words requirement after my comment on #2053. That comment showed that slices 1 to 3 need X8 and Y8 to also take Z0 and Z1, which no
GFX10_HTILE_SW_PATTERNentry has, so a per-tile decode of those slices would put tiles in the wrong place. That is still true, and per-tile decoding of slices 1 and up stays unmodelled. But the path this PR changes does no per-tile decoding:[DB_HTILE_DATA_BASE + slice * HtileMetadataBytes, + HtileMetadataBytes)has ZMask 0". It is a check over the set of words, so it is the same under any permutation of tiles to words.expandedHtileWord) rewrites each word in place from that word's own bits. A depth-only word becomes0xfffc000f; a depth+stencil word keeps its own SMem/SR bits. No word moves and no tile position is computed.DB_DEPTH_CLEAR, one register for the whole surface. It does not come from the words.The only slice-dependent input is the range's address. That comes from #5111's per-slice stride, which the equal-words case already trusts. So a slice whose words differ but are all cleared is in exactly the position of slice 0, which
mainalready clears. The equal-words test would only matter if the clear value were taken per tile, and this path does not do that.On hardware, the DB's depth test reads
DB_DEPTH_CLEARfor a ZMask-0 tile whatever its Min/Max Z. Mesasrc/amd/common/ac_descriptors.h(main @ 14a19dd6c167d0d5fd0126688318ed5461022588),HTILE_Z_CLEAR_REG: "This only clears HiZ and sets the Z/S state to "cleared". The DB register contain the full clear values." PALgfx9MaskRam.cppGfx9Htile::GetClearValueand RADV (radv_cmd_buffer.c, which writesDB_DEPTH_CLEARon every fast clear) encode the same way.What this does not model, and the debt entry now says so: the HiZ test runs before the DB test and rejects against each tile's own Min/Max Z range (HTILE definitions in the same header). A LESS draw over a tile cleared with Min/Max Z 0 is therefore HiZ-rejected on the console, but here it is tested against
DB_DEPTH_CLEAR. That gap already exists for slice 0 onmain("taken as a whole-surface clear"). This PR does not widen it beyond extending the slice-0 rule to later slices.Changes
storeBackClearedHtile: the starting aspect mask no longer depends onfirstSlice == 0 || uniform. A later slice takes the clear and the store-back when every word clears the aspect, like slice 0.docs/dev/debt/libSceAgcDriver.md, "HTILE written by other kernels":Tested
Arch Linux, kernel 7.2.9-zen, NVIDIA RTX 4070 Ti SUPER (driver 615.78.08), GCC 16.2.1, Release,
BUILD_TESTING=ON, onmain84b0d2f.agc_driver_depth_fast_clear_submit:MixedClearedSliceClears. Slice 1's HTILE is filled with words alternating0x00000000and0xfffffff0in runs of 7. The case checks that a LESS draw at 0.5 passes (the clear to 1.0 was taken), that the words are stored back as0xfffc000f, that slice 0's HTILE is untouched, and that the depth drawn afterwards is kept. Onmainit fails with "slice 1 did not take the clear from HTILE words that all have ZMask 0 but different Min/Max Z"; with the change it passes.NonUniformSliceIsRefusedis renamedExpandedTileKeepsSlice, unchanged otherwise. Its words (0xffffffffthen zeros) have one tile left expanded, so they are still refused and stored back untouched.UniformSliceDecodesis unchanged.agc_tests pass (ctest -R ^agc_ -j4).agc_driver_stencil_ref_exportskips on this device, as onmain.main+ open PRs + local patches) with a local HTILE reset that took these slices' clears, against the same branch with that reset removed, i.e.main's rule. The light shafts come back with the clear taken. This branch replaces that local reset with the change above; it has not yet been run in the title on its own.Overlap with open PRs
storeBackClearedHtilegets its words (pending-fill fast paths), not the clear rule. perf(agc): take the HTILE words pending fills leave instead of reading them #6505 keeps the line next to the one this PR removes, so whichever merges second has a trivial rebase.DepthSurfaceViewandNoteDepthMetadataFill, not this check.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)