Repository navigation
fix(agc): depth fast clears through the HTILE metadata - #374
Conversation
24a114b to
acb176a
Compare
1c3f05a to
086e8b9
Compare
|
This looks right for depth-only surfaces. With a stencil plane, though, any fill whose ZMASK is 0 clears both aspects. In the depth plus stencil HTILE layout the stencil state is in its own SR0, SR1 and SMEM fields: radv's |
|
Thanks, both points are fixed in e384824 (rebased onto
|
086e8b9 to
e384824
Compare
|
Thanks, that matches radv's depth plus stencil HTILE layout (ZMask in bits 0 to 3, SMem in bits 8 and 9, and |
e384824 to
b504800
Compare
62040ff to
f7de926
Compare
|
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.
|
adriadam10
left a comment
There was a problem hiding this comment.
Built merged onto current main (e79c971): full ctest passes. The new checks cover the register decode and the two pure helpers. A few things before merging:
- No test reaches the clear itself. Nothing calls
NoteDepthMetadataFill, soApplyFastClearand both triggers, theDMA_DATAimmediate (PacketMemory.cpp:59-63) and the fill kernel (BufferFill.cpp:173), run in no test. Removing them leaves ctest green. Could you add an execution test (theVisual.cppharness can draw with a depth target)? It would cover: aDMA_DATAfill of 0 over the HTILE, then a depth-tested draw that seesDB_DEPTH_CLEAR; a depth-only fill on a depth plus stencil HTILE that keeps the stencil; a fill shorter than the HTILE that clears nothing; and sampling the surface after a fill. - TechnicalDebt (Unknown function info). Two choices here are inferred, not measured, and should each get a line:
HtileFillCoverstreatsceil(w/8) * ceil(h/8) * 4bytes from the HTILE base as the whole surface. The real gfx10 HTILE is pipe-aligned and larger (addrlib), so a fill of only its first part would clear every tile.- The sampled path clears to
DB_DEPTH_CLEAR. For a depth-only HTILE, the clear word also carries the value: radv writesminz = maxz = round(depth * 0x3fff)(HTILE_Z_CLEAR_REG,src/amd/common/ac_descriptors.h), and only allows TC-compatible fast clears to 0.0 or 1.0. Which of the two the texture unit returns for a cleared tile wasn't measured.
- Reuse:
PacketMemory.cpp:60-61repeatsdmaSource/dmaDestinationfromPm4.cpp:82-88. Please expose those (or decode throughPm4) instead of a second copy of the bit layout.
@oneandonlydean you planned a PPSA21564 run on your integration branch before calling this ready. Did it go through?
f7de926 to
0b03ec8
Compare
94d93e0 to
6d0fe18
Compare
|
Replica run of the new head
AI-assisted: yes (Claude Code Fable 5.1 xhigh) |
|
Independent validation found a concrete false positive in the documented Reproducer: Reference: pinned PAL HTILE info/address routines. This is an unchanged source reference, not a PS5 measurement. TechnicalDebt already acknowledges the approximation; this supplies a concrete counterexample. Local correction tested: require the complete physical extent for the supported layout (32 KiB per 1024x512 metadata block). A partial overlapping fill makes the resident surface throw on its next use; a complete known fill permits recovery. This is conservative, does not model partial tile updates or other layouts, and any general adoption still needs its layout assumptions enforced. Validation:
Windows 11 / RTX 5080, driver 32.0.16.1062, GCC 15.2.0. The aggregate includes previous local snapshot prerequisites, conflict repairs and the independently verified standard-8x table correction; this is not an unchanged full PR build or Linux/lavapipe validation. Controlled PPSA99902 runs still stop at raw Fork code and validation update (2026-10-11)Proposed correction (fork only): partial HTILE-fill handling and regressions. Parent |
…and boykopovar#374 - DepthSurfaceTexture keeps main's rule (9d65003): a view of another extent or an unrelated format over a depth surface's memory is reused memory (nullptr). The checks run once, before the plane copy paths. - A raw depth-bits view (DepthBitsTextureWidth) samples the depth plane directly, as on main; the integer plane copy is only for an R16_UINT view that is not one. - The surface's storage transfer, fast clear and plane copy record on their own device (boykopovar#374's RecorderFor). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@Amoenus thanks, confirmed and fixed in @oneandonlydean #425 is re-stacked on it ( |
Rebased onto main as one commit (the six commits of boykopovar#374). A uniform fill over a depth surface's whole HTILE (DMA_DATA, the fill kernels or a CPU store) clears the depth and stencil aspects its ZMask and SMem fields say, to DB_DEPTH_CLEAR / DB_STENCIL_CLEAR, when the surface is next bound or sampled. Main's RADV clear_htile_mask recognition (boykopovar#2229) now goes through the same pending clear: an HTILE clear marks the matching surfaces' depth aspect, and one for a surface not created yet is applied when it is first bound. Main's HTILE address decode and this one are merged into one (TILE_SURFACE_ENABLE, a nonzero DB_HTILE_DATA_BASE and a depth format). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`Recorder::Active()` is process-wide. With two devices (agc_driver_depth_fast_clear_submit draws on its own device; its compute-fill DCB makes the driver create a second one, whose recorder becomes the active one) a surface's fast clear was recorded into the other device's command buffer, unordered with the draw: the compute-fill case failed about once in 60-90 runs. A surface now records into the active recorder only when that recorder is its own device's, and submits on its own device otherwise. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… DepthSurface include Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…pe-aligned metadata A fill was taken to cover the surface when it spanned ceil(w/8)*ceil(h/8)*4 bytes, a linear HTILE layout. In the pipe-aligned SW_64KB_Z_X layout a surface's HTILE words lie across a 32 KiB metadata block per 1024x512 pixels (AMD PAL gfx10 addrlib: a 16x16 surface has a valid word at offset 260), so a fill of the linear size cleared tiles it never wrote. HtileMetadataBytes gives that extent; the fast clear tests fill it (the compute fixture now dispatches one group per KiB with TGID_X). Reported with a PAL reproducer by @Amoenus. Co-Authored-By: Amoenus <5391396+Amoenus@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Rebased onto current
Merging once the manual full Build (linux and windows) is green on this head. |
90e3c40 to
56ea14f
Compare
Rebased onto main as one commit (the six commits of boykopovar#374). A uniform fill over a depth surface's whole HTILE (DMA_DATA, the fill kernels or a CPU store) clears the depth and stencil aspects its ZMask and SMem fields say, to DB_DEPTH_CLEAR / DB_STENCIL_CLEAR, when the surface is next bound or sampled. Main's RADV clear_htile_mask recognition (boykopovar#2229) now goes through the same pending clear: an HTILE clear marks the matching surfaces' depth aspect, and one for a surface not created yet is applied when it is first bound. Main's HTILE address decode and this one are merged into one (TILE_SURFACE_ENABLE, a nonzero DB_HTILE_DATA_BASE and a depth format). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…and boykopovar#374 - DepthSurfaceTexture keeps main's rule (9d65003): a view of another extent or an unrelated format over a depth surface's memory is reused memory (nullptr). The checks run once, before the plane copy paths. - A raw depth-bits view (DepthBitsTextureWidth) samples the depth plane directly, as on main; the integer plane copy is only for an R16_UINT view that is not one. - The surface's storage transfer, fast clear and plane copy record on their own device (boykopovar#374's RecorderFor). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…and boykopovar#374 - DepthSurfaceTexture keeps main's rule (9d65003): a view of another extent or an unrelated format over a depth surface's memory is reused memory (nullptr). The checks run once, before the plane copy paths. - A raw depth-bits view (DepthBitsTextureWidth) samples the depth plane directly, as on main; the integer plane copy is only for an R16_UINT view that is not one. - The surface's storage transfer, fast clear and plane copy record on their own device (boykopovar#374's RecorderFor). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
With APS5_PIPELINE_DMA (on by default since the previous commit) a DMA_DATA was enqueued before the packet path's metadata notes ran, so a fill of 0 over a depth surface's HTILE (boykopovar#374) or a color target's CMASK (boykopovar#419) never reached the surface: agc_driver_depth_fast_clear_submit failed. The queued commit now notes the fill in order before it stores it, and a DMA copy is checked against multisampled targets at enqueue, as on the packet path. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
With APS5_PIPELINE_DMA (on by default since the previous commit) a DMA_DATA was enqueued before the packet path's metadata notes ran, so a fill of 0 over a depth surface's HTILE (boykopovar#374) or a color target's CMASK (boykopovar#419) never reached the surface: agc_driver_depth_fast_clear_submit failed. The queued commit now notes the fill in order before it stores it, and a DMA copy is checked against multisampled targets at enqueue, as on the packet path. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
With APS5_PIPELINE_DMA (on by default since the previous commit) a DMA_DATA was enqueued before the packet path's metadata notes ran, so a fill of 0 over a depth surface's HTILE (boykopovar#374) or a color target's CMASK (boykopovar#419) never reached the surface: agc_driver_depth_fast_clear_submit failed. The queued commit now notes the fill in order before it stores it, and a DMA copy is checked against multisampled targets at enqueue, as on the packet path. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
With APS5_PIPELINE_DMA=1 a DMA_DATA was enqueued before the packet path's metadata notes ran, so a fill of 0 over a depth surface's HTILE (boykopovar#374) or a color target's CMASK (boykopovar#419) never reached the surface: agc_driver_depth_fast_clear_submit failed. The queued commit now notes the fill in order before it stores it, and a DMA copy is checked against multisampled targets at enqueue, as on the packet path. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> (cherry picked from commit fda7dbf)
What
A title can fast-clear a depth surface by filling its HTILE metadata instead of drawing a clear. A ZMASK of 0 marks every tile as holding
DB_DEPTH_CLEAR/DB_STENCIL_CLEAR.DB_HTILE_DATA_BASE(0x005, high byte in0x01e). Both registers join the draw key.DMA_DATAimmediate fills and buffer-fill kernels whose ZMASK is 0 and that cover a depth surface's HTILE mark the surface cleared.GTA V clears its G-buffer depth this way every frame:
DMA_DATAfills of 0 over 0x48000 bytes of HTILE. Without this, only the first frame renders. From the second frame on, the G-buffer keeps the first frame's contents and the scene goes black.Tested
Windows 11 Pro, AMD Ryzen 5 7600, NVIDIA GeForce RTX 5080 (driver 32.0.16.1074), 32 GB DDR5-5600, MinGW-w64 GCC 15.2.0.
main.ctest: all tests pass.agc_driver_depth_fast_clear_submitsubmits aDMA_DATAimmediate fill and the fill kernel over the HTILE and checks the next depth-tested draw, and a stencil-tested draw after a depth-only fill. Sampling a surface after an HTILE fill is not covered by a test: the harness has no sampling shader with a depth T# yet.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository🤖 Generated with Claude Code