Skip to content

fix(agc): give each slice of a depth array its own HTILE - #5111

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
XairRrR:fix/agc-depth-array-slice-htile
Oct 14, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
XairRrR:fix/agc-depth-array-slice-htile

Conversation

@XairRrR

@XairRrR XairRrR commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

What

A slice of a depth array target (DB_DEPTH_VIEW SLICE_START) offsets its depth and stencil addresses, but it kept the array's DB_HTILE_DATA_BASE as its HTILE. Every slice then shared slice 0's metadata:

  • a fast clear of slice 0's HTILE cleared every slice;
  • a clear of any other slice's HTILE matched no surface.

Changes:

  • HTILE address per slice: a slice's HTILE now starts at the base plus the slice index times one slice's pipe-aligned metadata. This follows addrlib, which adds sliceSize * slice in Gfx10Lib::HwlComputeHtileAddrFromCoord; HtileMetadataBytes is that slice size.
  • Clear kernel range: the recognized HTILE clear kernel now reports the bytes it clears: 16 per thread, since the kernel does global_store_dwordx4 at the thread index << 4. A surface takes the clear when that range covers its HTILE, as with a DMA_DATA fill. A clear over a whole array still clears every slice.
  • Clear noted early: a clear noted before its surface exists is kept with its range and taken by the first surface whose HTILE it covers.

Tests:

  • tests/Graphics.cpp: slice 2 decodes the base plus two slices of HTILE. This fails on main.
  • tests/DepthSurfaceReuse.cpp: two slices with adjacent HTILE ranges. A clear of the first range clears only the first slice, and a clear of both ranges clears both. The second check fails with exact-address matching.

Uncharted: Legacy of Thieves Collection (PPSA05684) renders its sun shadow cascades into the four slices of a 2048x2048 D16 array. Each slice's HTILE lies 256 KiB after the previous one, and decoding a slice's words at that offset gives that cascade's footprint.

Tested

  • Arch Linux (kernel 7.2.9), GCC 16.2.1, NVIDIA RTX 4070 Ti SUPER (driver 615.78.08), Release with BUILD_TESTING=ON.
  • Branch built on main ff8fe4d. Full ctest: 1041/1041. One run hit an unrelated failure in guest_kernel_fcntl, which passed when rerun. Conventions check passed.
  • Title: Uncharted: Legacy of Thieves Collection (PPSA05684), on its own bring-up branch, because plain main does not run it yet.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: PPSA05684 Uncharted: Legacy of Thieves Collection (PPSA05684) on Linux / NVIDIA #4001
  • AI-assisted: yes

@kr4y4

kr4y4 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

First Windows and Intel run.

Setup: Windows 11 (26200), WinLibs GCC 15.2.0 r7, Release, CI configure flags, an RTX 4070 Laptop GPU (NVIDIA 595.97) and an Intel Raptor Lake-S iGPU (32.0.101.6790). The head 5ea6a25 merged into main 0f2683c, which passes 1069 of 1069 on both GPUs.

  • I ran the tests built from the changed files with --repeat until-fail:3: agc_driver_graphics (which includes DepthSurfaceReuse.cpp), agc_sparse_buffer_holes, agc_descriptor_pool_reset_failure and agc_compute_descriptor_layout_flags. All pass on both GPUs.
  • Full suite: 1069 of 1069 on both GPUs.
  • There's no negative control: tests/DepthSurfaceReuse.cpp calls the new two-argument NoteHtileDepthClear, so it doesn't build against main.
  • It merges cleanly with fix(agc): store a written storage image over a depth plane into the surface #5113.

Written with AI assistance.

@XairRrR

XairRrR commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

@kr4y4 thanks for the Windows and Intel run. Here is a negative control that builds. I merged 5ea6a25 into main 0214069 and reverted each half of the fix on its own, keeping the two-argument NoteHtileDepthClear:

  • Slice offset only: State.cpp keeps depth.htileAddress = base(0x005, 0x01e). agc_driver_graphics fails with slice 2 of a depth array must use the third slice of its HTILE.
  • Clear range only: NoteHtileDepthClear and takeClearedHtile match the exact HTILE address again instead of HtileFillCovers over the reported range. agc_driver_graphics fails in DepthSurfaceReuse with an HTILE clear over both slices must clear both.
  • Whole PR: agc_driver_graphics passes, and so do all 487 agc tests on the merge (Linux, RTX GPU, NVIDIA driver, Release; agc_driver_stencil_ref_export is skipped).

AI-assisted: yes

@Amoenus

Amoenus commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

@XairRrR Our focused native-depth clear follow-up #5554 declares #5111 as a prerequisite, alongside #5524. Your range/slice routing goes first; our addition uses a separate strict native extent and requires complete tracked stores, settled GPU work, no CPU/depth race, and exact zero/one HTILE words before clearing a held image.

The actual-head validation composition includes your exact 5ea6a253 head. Full build/libs and seven groups ×5 on both RTX5080 and AMD integrated graphics pass without skips, including a real BDA mask-kernel producer and partial-store refusal. Your slice/range implementation remains in the validation composition; the focused follow-up carries our integration with that API. I will resolve the shared interface context after #5111 merges. Synthetic results; no new title replay. AI-assisted: Codex.

A slice of a depth array target (DB_DEPTH_VIEW SLICE_START) offset its
depth and stencil addresses but kept the array's DB_HTILE_DATA_BASE as
its HTILE. Every slice then shared slice 0's metadata: a fast clear of
slice 0's HTILE cleared every slice, and a clear of any other slice's
HTILE matched none.

A slice's HTILE now starts at the base plus the slice times one slice's
pipe-aligned metadata, as addrlib addresses it (sliceSize * slice in
Gfx10Lib::HwlComputeHtileAddrFromCoord; HtileMetadataBytes is that slice
size). The recognized HTILE clear kernel now reports the bytes it
clears, 16 per thread (global_store_dwordx4 at the thread index << 4),
and a surface takes the clear when that range covers its HTILE, as a
DMA_DATA fill does; a clear over a whole array still clears every slice.

Uncharted: Legacy of Thieves Collection (PPSA05684) renders its sun
shadow cascades into the four slices of a 2048x2048 D16 array; each
slice's HTILE lies 256 KiB after the previous one.
@adriadam10

Copy link
Copy Markdown
Collaborator

Rebased onto current main (conflicts in DepthSurface.cpp and DepthSurfaceReuse.cpp with the plane-owner and image-copy changes; resolved additively, takeClearedHtile now feeds the new DepthSurfaceView). Checked on the rebased tree: the HTILE slice offset follows addrlib's sliceSize * slice, the clear kernel range (16 bytes per thread, global_store_dwordx4 at thread index << 4) matches the kernel, and both new tests fail to build/pass without the change. Scoped build + ctest pass except agc_driver_tessellation, which segfaults on a local RADV without this change too; Windows/Intel/NVIDIA runs by @kr4y4 and @XairRrR cover the rest. Merging once the full Build is green.

@adriadam10
adriadam10 force-pushed the fix/agc-depth-array-slice-htile branch from 5ea6a25 to 8fec47a Compare October 14, 2026 17:57
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 14, 2026
@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

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.

Pull request Conflicting files (lines on main) Same exports Same new files
#5781 core/libs/prx/libSceAgcDriver/tests/DepthSurfaceReuse.cpp L41

@adriadam10
adriadam10 merged commit 0c9e980 into boykopovar:main Oct 14, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc*

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants