Repository navigation
perf(agc): handle the dword pattern-fill kernel as a buffer fill - #2081
Conversation
|
Could you measure main against main plus this PR alone on PPSA01325, frame time on the same scene over a few runs? Perf PRs need numbers for the change itself (#218); the 40-57 ms is the cost on main, and the after run had other changes in it. |
|
@oneandonlydean measured as an A/B with only this change between the two sides. PPSA01325 title screen, the same scene in every run; frames counted over 111-202 s of each run, three runs per side, A and B alternating. Current
So both sides run on my integration branch:
About 7 ms per frame, with B ahead in every pair. That is far below the 40-57 ms in the description. Those were the cost of the waits on |
A 64-thread compute kernel (69 dwords) stores pattern[min(i % s9, 3)], the pattern in s4..s7, to dword i of a 4-byte-stride buffer for every i below min(launched threads, s8, num_records), with the group id in s10. PPSA01325 runs it every frame to clear DCC keys, HTILE and other metadata. It ran as a shader; a direct dispatch with a period of 1, 2 or 4 dwords, whole 16-byte units and a 16-byte aligned destination now goes through fillBuffer like the existing fill kernel (clear path, FillBuffer, key memo). APS5_NO_FILL_HLE turns both off. tests/execution/DwordPatternFill.cpp checks the matcher and runs the kernel through Submit for matched and unmatched cases (periods 3 and 5, partial counts, record clamps, a partial dispatch); with APS5_NO_FILL_HLE=1 the same expectations hold for the translated kernel. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UTjpbMX7BUFMZZaxJE98zV
a2bd18b to
26035fa
Compare
|
Rebased onto current |
…tput format The kernel #2081 matches stores each pattern dword with buffer_store_format_x, so what lands in memory depends on the output descriptor's FORMAT (bits 12-18 of user[3]). Only 32_UINT, 32_SINT and 32_FLOAT (20-22) store the dword unchanged. Other formats convert it, and an 8- or 16-bit one writes 1 or 2 bytes of every 4-byte element, so a buffer fill would also overwrite the bytes in between. Such a dispatch now goes to the recompiler. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hunk) (#442) * feat(agc): write memory for WRITE_DATA with DST_SEL 3 Killzone: Liberation (PPSA11142) writes its frame fence with `sceAgcDcbWriteData(dst 6, cache policy 3, write confirm)`, which the SPRX-confirmed encoding turns into WRITE_DATA with DST_SEL 3 and a label address in the title's memory that RELEASE_MEM and WAIT_REG_MEM then use. The validator rejected DST_SEL 3 as the GDS; it now writes guest memory like DST_SEL 5, resolves as a label, and the title runs past its first frame. TechnicalDebt records where the semantics come from. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 9c53b7f) (cherry picked from commit a553e22) * fix(agc): wait for shutdown at shader ABI entrypoints (cherry picked from commit 5550ca6) * fix(agc): reject constant color factors and memory-writing stages in split SWAP_ALT blends Review follow-up to the 8_8 SWAP_ALT blending. The second channel of such a target holds the guest's alpha. Vulkan applies the green blend constant to it for CONSTANT_COLOR and ONE_MINUS_CONSTANT_COLOR, where the guest's alpha equation uses the alpha constant. Decoding now throws for these factors on a SWAP_ALT 8_8 target, unless the equation is MIN or MAX, which ignore factors. The second pass of a split draw runs every stage again, not only the pixel shader, so a vertex or geometry stage that writes memory would write twice. The memory-write check (now ShadersWriteMemory) covers all the draw's stages. (cherry picked from commit 79c8d36) * fix(agc): read a merged program's vertex tables past its hidden user words A merged (NGG geometry path) program's user data starts with eight hidden words, so the vertex buffer and attribute table offsets of its AGC header, which count from the first user SGPR, were read eight words too early and found zeros. Killzone: Liberation (PPSA11142) stopped on `AGC graphics: null vertex buffer table address` at the first frame of its PSP game. The decoder now adds the hidden word count to the table reads; the registers reported to the recompiler are unchanged, as it already maps them from the first user SGPR. `agc_driver_graphics` gains a merged and a plain layout case. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit b63996f) * fix(agc): render a slice of a 2D array color target into that layer of the array A color view of slice N of a 2D array (CB_COLOR_VIEW SLICE_START, with CB_COLOR_ATTRIB3 MIP0_DEPTH as the last slice) was rendered as a 2D surface at the slice's address. The swizzle equations of the XOR tile modes take the slice index (the detiler's slice bits), so the texels of slice N of an array are not where a 2D surface at that address puts them: a title that renders into array slices and then samples the array read them scrambled. The decoded target now keeps the array's address, its last slice and the view's slice. A resident target that is a slice of an array attaches that layer of the array's storage image, so the texels follow the array's layout; storage images of 2D arrays are color attachments for this, and an attachment view selects a layer. A slice whose array has DCC, CMASK or an attachment proxy has no resident array image, and a non-resident slice of an XOR-tiled array now throws instead of being written in the 2D layout. (cherry picked from commit eba67d4) * fix(agc): read zero for pixel inputs of parameters the vertex stage does not export A pixel input whose SPI_PS_INPUT_CNTL offset names a parameter the previous stage does not export failed validation ("graphics interfaces disagree"), so the draw threw. No export writes that parameter slot for the draw. radeonsi programs a pixel input without a matching vertex output as OFFSET 0x20 with DEFAULT_VAL 0, so that it reads (0, 0, 0, 0) (SI_PS_INPUT_CNTL_UNUSED in si_shader.h, applied in si_shader.c), and rect-list draws already pass zero for such inputs. Before the pixel stage of a draw is prepared, inputs that read a parameter the previous stage's program does not export now read zero: OFFSET bit 5 is set, DEFAULT_VAL and FLAT_SHADE are cleared, and an FP16 input also reads zero in its high half (USE_DEFAULT_ATTR1 set, DEFAULT_VAL_ATTR1 cleared). Inputs that already read a default value are unchanged. Fixes boykopovar#2975. (cherry picked from commit 2f140cb) * fix(agc): leave the depth surface out when no enabled test has its plane DecodeState bound the depth surface whenever DB_DEPTH_CONTROL enabled a depth, stencil or depth bounds test and either plane was valid. A stencil test beside a depth-only surface (DB_STENCIL_INFO format invalid, depth test off) tests nothing, but the surface was still attached and the render extent was clamped to it. A draw into a 1920x1080 target with a 960x540 depth surface left bound then covered only the top-left quarter. A test whose plane is present but invalid no longer binds the surface. Such a draw renders without a depth attachment, as it already does when both planes are invalid. DrawRejection follows the same rule. (cherry picked from commit c1287d7) * fix(agc): skip read-only depth resummarize draws (cherry picked from commit 82ac3bd) * feat(agc): render 1_5_5_5 UNORM color targets CB_COLOR_INFO format 17 with UNORM and the standard component swap renders into VK_FORMAT_A1R5G5B5_UNORM_PACK16, whose attachment and blend support Vulkan requires. AMD keeps X in the low bits and Vulkan keeps R in the high bits, so the exports are mapped with X and Z exchanged and the stored bits follow the guest layout. PPSA06699 renders into this format. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> (cherry picked from commit 61a9f5d) * fix(agc): keep write tracker alive through driver teardown Initialize the guest write tracker during Driver construction so its function-local static is destroyed after Driver on process exit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit 471d9ec) * fix(libSceAgc): write the KytyPS5 workload packets and return null for unregistered streams sceAgcDcbSetWorkloadsActive writes an 18-dword IT_NOP packet (stream id, 64-bit workload mask) and sceAgcDcbSetWorkloadComplete a 12-dword one (stream id, workload id, 64-bit clear mask), following KytyPS5. Both return null for a null buffer, an unregistered or out-of-range stream, a workload id above 63, and for active a duplicate id or a count of 0 or above 63. sceAgcDriverRegisterWorkloadStream and sceAgcDriverUnregisterWorkloadStream now record the stream ids the DCB calls check. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> (cherry picked from commit 2a7b38a) * style(agc): remove unnecessary shutdown explanation Keep the tracker initialization fix without an explanatory code comment or technical-debt entry, as requested in review. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit fc7a965) * fix(agc): accept a linear 2D texture whose DEPTH holds its default pitch On GFX10.3 the DEPTH field of a 1D or 2D non-array linear T# carries the row pitch minus one (Mesa's ac_descriptors.c sets it for custom pitches, which must be multiples of 256 bytes). A single-level, uncompressed 2D linear texture whose DEPTH equals its default 256-byte-aligned pitch now decodes as that 2D texture; any other pitch still throws. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 6b24328) * test(agc): preparation failures name the shader address and stage FailureCapture now requires the code address and stage in the diagnosed failure, with and without the request dump. Fails before the wrap, passes after. AI-assisted: yes. (cherry picked from commit 768929b) * fix(agc): name the shader address and stage in preparation failures PrepareShaderWithDiagnostics dumped the request but rethrew the raw error, so a failure never said which shader it came from. Wrap it with the shader address and stage, keeping the dump. AI-assisted: yes. (cherry picked from commit e71e3a4) * fix(agc): preserve dumps for non-standard shader failures Capture the request once before rethrowing from the shared handler. Standard exceptions keep the shader address, stage, optional dump filename and full message; non-standard exceptions remain unchanged after capture. AI-assisted: yes. (cherry picked from commit 56df893) * fix(agc): retry failed host imports after range replacement Retain weak identities and the full failed span so a replacement of either owner or a shorter remaining run retries the import. Keep unchanged failures cached across unrelated mapping changes without pinning their allocations. Existing upstream import lifetime, span windows and Windows fallback remain in place. AI-assisted implementation. (cherry picked from commit 2441a4b) * fix(agc): release parked host imports a pinned guest range waits for A device replaced at AttachWindow keeps its host imports in HostImports::parked until it is destroyed, and each parked import keeps its guest ranges pinned. A guest munmap, mprotect or free of such a range spun in the pin waiter until APS5_PIN_WAIT_MS and then failed, because the waiter only looked at the live imports. The replaced device was already waited idle by PrepareForReplacement, so the waiter now drops the parked imports over the range. agc_parked_import_unmap imports a range on one device, again on a second one, and unmaps it with a 3 s pin wait. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit cd067d1) * feat(libSceAgc): implement sceAgcAcbWaitUntilSafeForRendering (cherry picked from commit 661f8e1) * test(agc): draw into a 1_5_5_5 target and read the 16 bits back Draws 32_ABGR exports into a tiled and a linear format 17 target and compares every texel with the layout X in bits 0-4, Y in 5-9, Z in 10-14, W in bit 15, including the X/Z-swapped pairs, rounding edges and texels outside the scissor. Reverting the X/Z exchange fails it. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> (cherry picked from commit deaa2e3) * feat(agc): describe color mip targets with parent layout Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit c914bcd) * test(agc): cover packed color mip detiling Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit c58c948) * fix(agc): render nonresident packed mip targets Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit 89c1287) * style(agc): keep mip layout API comment-free Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit 485c0f3) * fix(agc): keep untailed color targets on their own detiler layout (cherry picked from commit 9394c74) * test(agc): render into packed mip tails through the guest-memory fallback (cherry picked from commit 6c94d31) * fix(agc): link the texture mip layout into the BDA device tests (cherry picked from commit 4dfe052) * fix(agc): synchronize recorded fast clears with attachments (cherry picked from commit 512ea3e) * test(agc): follow repository comment conventions (cherry picked from commit dd4504e) * fix(agc-driver): move indices far past every vertex fetch's records to the first zero vertex Goat Simulator 3 draws a triangle list whose indices are all 0xffffffff with primitive restart off. Each fetch past a descriptor's records reads zeros (out-of-bounds select 0 or 1), but the zero-padded vertex copies were sized to the highest index: 160 GiB, which vkAllocateMemory refused. When the highest index is at least 64Ki vertices past the largest per-vertex record count, the draw now uses a copy of its indices with every index past that count moved to it. Each attribute reads zeros there as at the original index; only the vertex index the shader sees changes, so indices closer to the records are left as they are. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit a3cba2d) * build(agc): avoid copied-write test registration overlap (cherry picked from commit a55472c) * feat(agc): place textures in host memory when device memory runs out (cherry picked from commit 6118098) * feat(agc): place storage images in host memory when device memory runs out (cherry picked from commit eb03e0e) * test(agc): cover the host memory fallback of device allocations (cherry picked from commit 8592938) * fix(agc): choose host memory from the heap budget instead of retrying Read the device-local heap's VK_EXT_memory_budget figures before a texture or storage image allocation and pick a host memory type when the allocation does not fit, rather than allocating again after the driver refused the first attempt. The allocation itself is made once and still throws when it fails. (cherry picked from commit b35117c) * perf(agc): memoize untracked texture comparisons by the stores that reach the surface The memo stamp held TrackerGeneration, which every collect and driver store advances, so it almost never matched. Driver stores into uncovered ranges record their generation per 64 KiB block, and NewestStamp of the surface replaces TrackerGeneration in the stamp. (cherry picked from commit ca99658) * fix(agc): take the dword pattern-fill kernel only through a 32-bit output format The kernel boykopovar#2081 matches stores each pattern dword with buffer_store_format_x, so what lands in memory depends on the output descriptor's FORMAT (bits 12-18 of user[3]). Only 32_UINT, 32_SINT and 32_FLOAT (20-22) store the dword unchanged. Other formats convert it, and an 8- or 16-bit one writes 1 or 2 bytes of every 4-byte element, so a buffer fill would also overwrite the bytes in between. Such a dispatch now goes to the recompiler. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 4309bff) * feat(libSceAgcDriver): export sceAgcDriverGetHsOffchipParam (cherry picked from commit bdb0be0) * feat(agc): support 64 KiB display XOR color targets Map SW_64KB_D_X color surfaces through the existing source-backed texture equations and mip geometry. Keep display and render-target equation caches separate. Reject unresolved raw column bits for the new mode instead of accepting them. Cover the captured PPSA99902 descriptor, independent source offsets, pipe/bank tags, transfers, array/mip placement and resident/non-resident GPU draws. Record unmeasured console behavior in TechnicalDebt. CPU validation on main e013f8c: captured-descriptor failure 5/5; changed passes 5/5; pinned AMD addrlib reference 536940 addresses per run, five runs. Full build and GPU validation are tracked externally. AI-assisted implementation and validation: Codex. (cherry picked from commit f27bdf4) * fix(agc): validate display XOR bits before resident lookup Apply the source-configured D_X XOR refusal during color decoding as well as direct layout construction. Regression tests reject each raw column-bit combination before storage or rendering is selected. Existing resident-array and packed-mip refusal paths remain in force. AI-assisted implementation and validation: Codex. (cherry picked from commit 4326c7c) * test(agc): verify display XOR array attachment transport Run the existing array-layer draw, recompiled shader load, untouched-layer and guest-publication checks with D_X as well as R_X. This exercises the resident array path that carries slice-dependent address XOR; a direct per-slice copy is not an equivalent model. AI-assisted implementation and validation: Codex. (cherry picked from commit b6f1a4a) * perf(agc): clear integer-format storage images with DCC code 0000 on the GPU ClearColorFor refused every integer format, so a fast-cleared R32_UINT/R16_UINT/R8_UINT surface was uploaded through the CPU path: a guest read that waits for the GPU writers, then FillDccClear's zero texels. Code 0000 is zero bits in any format, so it maps to a zero integer clear. PPSA28416: ~1.4 s per 10 s of CPU uploads gone, 4.9 -> 5.3 fps. (cherry picked from commit 92b46a2) * test(agc): pin the integer storage clear to DCC code 0000 StorageClearAvailable takes code 0000 on R8/R16/R32_UINT and refuses every other code; drop the header comment that said integer formats were refused. (cherry picked from commit 09aa34d) * test(agc): run the integer storage clear check from ctest (cherry picked from commit b3c8be5) * test(agc): remove non-discriminating recorded clear fixture (cherry picked from commit 0b54425) * fix(agc): accept a CB metadata pass beside depth state without depth or stencil planes (cherry picked from commit d9fbaf7) * fix(agc): accept MSAA_ENABLE on a single-sample target sampled at the pixel center (cherry picked from commit fdfbeba) Fork adaptation: applied to the fork MSAA model (rasterSamples from PA_SC_AA_CONFIG); a single-sample scan with MSAA_ENABLE that the attachments raise to a multisampled pass stays refused. Upstream sample-location tables are not taken here. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(agc): leave blending off for a color target with CB_COLOR_INFO.BLEND_BYPASS DecodeState threw "blend bypass conflicts with enabled blending" when CB_BLENDn_CONTROL enabled blending for a target whose CB_COLORn_INFO has BLEND_BYPASS (bit 16, Mesa src/amd/registers/gfx10.json) set. The bit makes the color block skip the blender for that target: Mesa sets it "according to docs" for UINT/SINT and 8_24/24_8/X24_8_32_FLOAT targets, which the blender cannot process (src/amd/common/ac_descriptors.c, shared by radeonsi and RADV). Marvel's Guardians of the Galaxy (PPSA01748) issues such draws right after its loading screen. Blending is now enabled only when it is requested and not bypassed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit baaad78) * fix(agc): accept a CB metadata pass whose depth test passes every fragment without writing DecodeColorMetadataPass refuses an eliminate-fast-clear or DCC-decompress pass when DB_DEPTH_CONTROL has any of its low four bits set and depth or stencil planes are bound. Marvel's Guardians of the Galaxy (PPSA01748) issues one with the Z test enabled, ZFUNC ALWAYS and no Z write (DB_DEPTH_CONTROL 0x72) while a Z32 and an S8 target are bound. That test passes every fragment and leaves depth unchanged, so the pass does no depth work. Draws already treat this state as a pass-through (depthPassThrough); the metadata pass now uses the same rule. Depth bounds, depth writes, stencil tests and DB_RENDER_CONTROL clears or copies are still refused, and the error names the four registers involved. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit b90ec1e) * build(metal): link the workload stream registry into the native AGC execution libSceAgc's workload packets (upstream 2a7b38a) ask libSceAgcDriver whether a stream is registered. The Rosetta libs link the two libraries; the Metal utilities build links libSceAgc's host objects against anyps5_metal_native_execution, which now carries Resource/src/Registration.cpp. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(metal): cover ported State decode fixes on the Metal path anyps5_metal_render_state now decodes a guest draw with CB_COLOR_INFO BLEND_BYPASS (upstream PR 4862) and with MSAA_ENABLE on a single-sample scan (upstream fdfbeba) and renders both through the Metal backend. MSAA_ENABLE is read only when PA_SC_MODE_CNTL_0 is written, as the fork's decode accepted before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Mouad <mouadscriptz@gmail.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: MarcZX100 <marczxrr@gmail.com> Co-authored-by: XairRrR <dev910382@gmail.com> Co-authored-by: Dean Galvin <dean.galvin@live.com> Co-authored-by: Mouhand Kaddo <73709175+Mouhand-Kaddo@users.noreply.github.com> Co-authored-by: JAB-dev <68183649+JAB-dev@users.noreply.github.com> Co-authored-by: Avijit Singh <assistantmark1.0@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: xXPBSXx <20614131+xXPBSXx@users.noreply.github.com> Co-authored-by: Achraf Reyani <achraf@reyani.dev> Co-authored-by: danzaio <danzaio@users.noreply.github.com> Co-authored-by: TibixDev <tibix@fhs.sh> Co-authored-by: Diversao <83223857+diversao@users.noreply.github.com> Co-authored-by: Utku Halis <personal@utkuhalis.com.tr> Co-authored-by: Felipe Arce <felipearce.2004@gmail.com> Co-authored-by: krishalivaibhav <krishalivaibhav@gmail.com> Co-authored-by: Tyler Westerberg <tylerwesterberg05@gmail.com> Co-authored-by: Gustavo Ariel Gamarra Rojas <111788638+GustavoGamarra95@users.noreply.github.com> Co-authored-by: ThatsLeo <leonardorocco.monaco@studio.unibo.it> Co-authored-by: AlxcBTW <187763517+AlxcBTW@users.noreply.github.com> Co-authored-by: Jordan Hans <jordan.hans@ymail.com> Co-authored-by: Romans Pokrovskis <motivated.it@gmail.com> Co-authored-by: Chad <chadmaster52@gmail.com> Co-authored-by: Adrià Franch <adriadam10@gmail.com> Co-authored-by: Nesyth <18372752+Nesyth@users.noreply.github.com> Co-authored-by: Cesar Rodriguez <11452991+Tamalero@users.noreply.github.com>
What
Astro's Playroom (PPSA01325) runs a 64-thread compute kernel (69 dwords) every frame to fill DCC keys, HTILE and other metadata. It ran as a shader. Before the fill, every key read of those surfaces waited for a drain (on the UI target
0x534830000, among others).What the kernel does, read from its code and confirmed by the new test against the translated kernel:
ibelowmin(launched threads, s8, num_records). The group id is ins10, so the kernel has exactly 10 user SGPRs.iof a 4-byte-stride buffer (s[0:3]) it storespattern[min(i % s9, 3)], where the pattern iss4..s7ands9is the period.s4,s5ands6; anything else storess7.MatchDwordPatternFill(new headerDriver/Memory/DwordPatternFill.hpp) recognizes the exact code and accepts only one shape:That shape is a 16-byte pattern fill, and
fillBuffertakes it the same way it takes the existing fill kernel: clear path,FillBuffer, key memo. Everything else still runs the kernel.APS5_NO_FILL_HLEturns both matchers off.#1335 recognizes a different, typed pattern-fill kernel (18 dwords, pattern and control read from buffers). Both PRs touch
fillBuffer, so whichever merges second needs a small rebase.Tested
Windows 11, WinLibs GCC 15.2, RTX 3060 Laptop:
agc_driver_dword_pattern_fill_tests(tests/execution/DwordPatternFill.cpp):Submit: matched ones and unmatched ones (periods 3 and 5, a count of 250, a partial dispatch).APS5_NO_FILL_HLE=1, the translated kernel meets the same expectations, which is where themin(i % s9, 3)reading comes from.agc_driver_testsandagc_driver_pm4_testspass, andtools/check_conventions.py --base upstream/mainpasses.maindoes not reach the title, so both sides ran on an integration branch; details and per-run numbers are in the comments.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 (attach them to this PR)🤖 Generated with Claude Code