Repository navigation
feat(agc): accept the dispatch and indirect packet forms vkQuake's RADV emits - #2756
Conversation
|
Tested merged onto 1. The fill fast path ignores
2.
3. Question: The new tests in |
|
Points 1 and 2 from fma-rc need fixing first: the fill/copy fast path in |
|
Thanks @fma-rc and @oneandonlydean. Fixed in 7c72f04 (a new commit on top, so the change is easy to review):
Tests:
AI-assisted (Claude Code). |
|
Re-tested at 7c72f04 merged onto Fixed, by run through the driver (
1. The
A register number that is a multiple of 4 makes the assertion cover the guard. 2. A dispatch with a start offset is run from 0, and the partial arithmetic now applies to it. With
RADV emits this form when a dispatch has a base offset, and then the packet words are end values, not counts ( if (params->start[0] || params->start[1] || params->start[2]) {
/* The blocks in the packet are not counts but end values. */
for (unsigned i = 0; i < 3; ++i)
params->dim[i] += params->start[i];
} else {
params->dispatch_initiator |= S_00B800_FORCE_START_AT_000(1);
}Read that way, the first row is group 1 only and the third is groups 1 and 2 with a 16-thread last group; Not checked: vkQuake itself, the copy kernel with a partial last group, partial last groups on Y and Z through a shader, |
Independent local validation of 7c72f04Tested your current head I reproduced the two points in fma-rc's latest comment independently:
For local diagnostic use I added an explicit refusal of known nonzero COMPUTE_START_X/Y/Z when FORCE_START_AT_000 is clear, before fast paths/preparation. Separate driver cases cover X/Y/Z for direct, partial and indirect dispatches, verifying refusal and unchanged output. FORCE_START_AT_000 controls execute all three axes and verify the written/untouched range. The 12 new groups plus the four above passed five repetitions. Removing the start guard makes its direct-X case fail. Both mutation sources were restored byte-for-byte, full libraries rebuilt, and all 16 groups passed five repetitions again. The guard and stronger COPY_DATA assertion are separate unpublished local topic commits ( PPSA99902 (Sonic Unleashed Recompiled) also reaches your exact dispatch form on a conditional local startup stack: captured One source question remains outside these tests: the PR accepts ORDER_MODE=0 because the driver does not reorder waves. AMD's register reference describes ordered wave launch, while Vulkan shader execution leaves relative invocation execution order undefined. These are different guarantees; the tests above do not establish that the host honors ordered wave launch. Is there a source or measurement establishing that this bit can be ignored without changing observable behavior? This title's captured 0x2047 has ORDER_MODE=1. Logs, captured private title data, exact patches and hashes retained outside the repository. No branch push or new PR. AI-assisted validation/local changes: Codex. |
|
The |
RADV sets PARTIAL_TG_EN in the dispatch initiator for an unaligned direct dispatch, with each axis's last-group size in the upper half of COMPUTE_NUM_THREAD_X/Y/Z, and TUNNEL_ENABLE on every GFX10 dispatch. The validator rejected PARTIAL_TG_EN, and DecodeComputeStageInfo read the whole register, so a partial size would have widened the workgroup. A direct dispatch with PARTIAL_TG_EN now runs (groups - 1) * full + last threads on each axis through the dispatch-size limit USE_THREAD_DIMENSIONS already uses, or plain groups when every last group is complete; a zero or oversized last group throws. The workgroup size is NUM_THREAD_FULL. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Where the GPU has no LOAD_CONTEXT_REG_INDEX, RADV loads an image's clear values into DB_STENCIL_CLEAR/DB_DEPTH_CLEAR and CB_COLORn_CLEAR_WORD0/1 with COPY_DATA from memory to a register (DST_SEL 0), one or two dwords. The validator rejected every register destination. A COPY_DATA from memory to one or two context registers is now accepted. It is not a memory store, so the packet drains like other COPY_DATA the driver cannot record on the GPU, and executing it reads the source then and writes the registers. Register, GDS, immediate and clock sources, and destinations outside the context registers, are still rejected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RADV writes every COPY_DATA memory destination with DST_SEL 5 (COPY_DATA_DST_MEM in Mesa's sid.h), the current form of the deprecated GRBM-synchronized DST_SEL 1 that was already accepted. vkQuake's timestamp writes use it. It is now validated, resolved and executed as memory like 1 and 2; GDS and performance-counter destinations are still rejected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RADV clears ORDER_MODE in the dispatch initiator for a dispatch that needs its waves launched in order, and the validator required it set. In-order launch is the stricter guarantee and the driver does not reorder waves either way, so a direct or indirect dispatch now needs only COMPUTE_SHADER_EN; vkQuake's map load issues one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RADV emits 0 as the start-instance location of DRAW_INDIRECT and DRAW_INDEX_INDIRECT when the vertex shader does not read the base instance, and as the draw-index location of the multi forms when draw IDs are off. The validator accepted only 0x280 as "no register". Location 0 is now accepted and decoded as 0x280; vkQuake's draws use it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RADV sets the draw and dispatch indirect base with SET_BASE to the argument buffer address plus the command's offset, which Vulkan requires only to be a multiple of 4; vkQuake's base ends in 0x14. The validator required 8-byte alignment. The records are read as 32-bit words, so a 4-byte-aligned base is now accepted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…date every axis The buffer fill and copy fast paths returned before the partial last group was applied, so a PARTIAL_TG_EN dispatch of the fill kernel filled whole groups, and a zero last group did not throw. The partial sizes are now decoded before the fast paths, which are skipped when the last group is partial; the dispatch then runs the shader with the partial thread counts. PartialGroupThreads validates all three axes before an empty dispatch returns, and throws when (groups - 1) * full + last does not fit in 32 bits instead of wrapping. Validate rejects PARTIAL_TG_EN combined with USE_THREAD_DIMENSIONS (initiator 0x63). agc_driver_pattern_buffer_fill now runs the fill kernel with a 16-of-64 last group through the driver and checks that 80 dwords are written. AI-assisted (Claude Code). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
0xa00a is not 4-byte aligned, so ResolveStore rejected it before the register guard.
7c72f04 to
a585ec1
Compare
|
Checked at 7c72f04 merged onto main: build and the libSceAgcDriver ctest (354 tests) are green, and the new Added one commit on top for @oneandonlydean's last point: the Merging once CI is green. |
What
Split out of #1917 at review's request. These six commits came from running vkQuake (PPSA99010, the homebrew Vulkan port) through RADV on the PS5 (mihawk-99/PS5_Mesa). They change how many threads a dispatch runs and how indirect draws and dispatches are decoded, so they get their own review here. #1917, with the ccleste packet forms, is merged.
PARTIAL_TG_ENin the initiator and puts each axis's last-group size in the upper half ofCOMPUTE_NUM_THREAD_X/Y/Z. The dispatch now runs(groups - 1) * full + lastthreads per axis through the dispatch-size limitUSE_THREAD_DIMENSIONSalready uses, or plain groups when every last group is complete. A zero or oversized last group throws (Pm4::PartialGroupThreads).DecodeComputeStageInforeads onlyNUM_THREAD_FULL(bits 15:0); it read the whole register, so a partial size would have widened the workgroup.COPY_DATAfrom memory to context registers. Where it doesn't useLOAD_CONTEXT_REG_INDEX, RADV loads clear values intoDB_STENCIL_CLEAR/DB_DEPTH_CLEARandCB_COLORn_CLEAR_WORD0/1withCOPY_DATAto a register (DST_SEL0, one or two dwords). It isn't a memory store, so the packet drains like otherCOPY_DATAthe driver can't record on the GPU, and executing it reads the source then and writes the registers. Register, GDS, immediate and clock sources, and destinations outside the context registers, are still rejected.COPY_DATAto memory throughDST_SEL5 (COPY_DATA_DST_MEMin Mesa'ssid.h), the current form of the deprecated GRBM-synchronizedDST_SEL1 already accepted. RADV uses it for every memory destination; vkQuake's timestamp writes hit it.ORDER_MODEfor a dispatch that needs in-order wave launch. That is the stricter guarantee and the driver doesn't reorder waves, so a direct or indirect dispatch now needs onlyCOMPUTE_SHADER_EN.DRAW_INDIRECTandDRAW_INDEX_INDIRECTwhen the vertex shader doesn't read the base instance, and as the draw-index location of the multi forms when draw IDs are off. The validator accepted only0x280as "no register"; 0 is now accepted and decoded as0x280.SET_BASEto the argument buffer address plus the command's offset, which Vulkan only requires to be a multiple of 4 (vkQuake's ends in0x14). The validator required 8-byte alignment; the records are read as 32-bit words, so a 4-byte-aligned base is now accepted and a 2-byte-aligned one still rejected.Tests (
agc_driver_pm4,agc_driver_graphics):PartialGroupThreadsfor complete, partial, empty and invalid last groups;DecodeComputeStageInfoignoring the partial half;COPY_DATAwriting one and two context registers, not resolving as a store, and rejecting an immediate source, a non-context register, an overflowing range and a misaligned source;DST_SEL5 with an immediate and a 64-bit timestamp; initiators0x2047,0x2007and indirect0x2005accepted and0x40rejected. Each new check fails with its change reverted.Tests for these two (
agc_driver_pm4): location 0 inDRAW_INDIRECTand in the multi form resolves to0x280, and an out-of-range location (0x200) is still rejected; aSET_BASEending in0x14is accepted and one ending in0x12rejected. Each fails with its change reverted.Tested
Windows 11 Pro (26200), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release, NVIDIA GeForce RTX 5070 Ti.
maincf8b129. Full suite: 551 of 554 pass, 1 skipped. The two failures,agc_driver_prepared_shaders_debug_failureandguest_memory, fail the same way on cleanmaincf8b129 on this machine. It was run from the generatedCTestTestfile.cmakecommands because antivirus blocksctest.exeon this machine.tools/check_conventions.pypasses.mainalone yet: it stops atFAIL: unresolved ELF import 1nTKA7pN1jw from libSceLibcInternal.prx(utime, feat(libSceLibcInternal): implement guest utime #2024), and 13 more imports are missing after that. I have not rerun it since this rebase.Not tested on AMD, Intel, Linux or macOS on this head.
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