Repository navigation
fix(agc): accept RADV's disabled conservative rasterization and out-of-order watermark - #1915
Conversation
RADV sets OUT_OF_ORDER_WATER_MARK to 7 on every GFX10 draw, also when OUT_OF_ORDER_PRIMITIVE_ENABLE is off. The watermark only tunes out-of-order rasterization, which stays rejected, so the field is accepted alone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With conservative rasterization disabled, RADV programs PA_SC_CONSERVATIVE_RASTERIZATION_CNTL to NULL_SQUAD_AA_MASK_ENABLE alone, where libSceAgc writes the two INNER_TO_NORMAL overrides (0x6000). A value made of only those bits now decodes as disabled; overestimation (0x6001) keeps its checks, and every other mode is still rejected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
klosixgaming-arch
left a comment
There was a problem hiding this comment.
Small, correct, and I can approve it.
PA_SC_MODE_CNTL_1.OUT_OF_ORDER_WATER_MARK: you accept bits 30:28 on their own (viaScanControlMask = ~0x76023fffu) while keepingOUT_OF_ORDER_PRIMITIVE_ENABLErejected. The test asserts a watermark-7 draw is accepted and the same with out-of-order enable is rejected. Right boundary.PA_SC_CONSERVATIVE_RASTERIZATION_CNTL:decodeConservativeRasterizationnow accepts(control & ~0x00106000u) == 0as disabled (covering0x6000,0x100000, and0), keeps0x6001as overestimation, and still rejects everything else. TheDrawRejectionpath matches the same mask, so decode and rejection agree.- Both the decode (
DecodeState) and the rejection (DrawRejection) paths were updated consistently — that's the easy place to desync and you didn't. - Tests cover the three disabled values and two enabled values that must still be rejected.
One non-blocking note: the enabled-mode rejection test uses 0x00e00001 and 0x01e00022 — those are "enabled" conservative raster modes. Good. Just make sure the debt entry (if one is added later for "conservative rasterization enabled is unimplemented") records why 0x100000 is safe to treat as disabled (NULL_SQUAD_AA_MASK_ENABLE alone = off per RADV) so the asymmetry between "off" values and "on" values isn't a mystery to the next reader.
Written with AI assistance.
|
Linux run on AMD, since the description lists it as not tested: Ubuntu 24.04.5 (kernel 6.8), GCC 13.3, Release, AMD Radeon RX 7600 (RADV NAVI33, Mesa 25.2.8). This PR's head |
|
The current upstream Windows run is red: |
Dextroid17
left a comment
There was a problem hiding this comment.
Verified on Linux on lavapipe (Mesa) — the other side of the driver split this PR is about.
- Arm A (this PR):
agc_driver_graphics_testsbuilds and passes, exit 0. - Arm B (control):
State.cppback atmainwith the test kept → exit 1, and the message is precisely the register in question:
... sample iteration, primitive discard or out-of-order rasterization is unsupported (register 0x293 = 0x760201bc)
So the RADV value is exactly what the old code refused, and the new Requires are what catch it.
Claim cross-check: the scan-control mask widened from ~0x06023fff to ~0x76023fff (bits 30:28), so the out-of-order watermark is accepted on its own; the conservative control now accepts 0x6000 or RADV's 0x100000 via (control & ~0x00106000u) == 0. The part that matters is that the test checks the other direction too: out-of-order rasterization itself (0x7e0201bc) is still rejected, and genuinely enabled conservative values (0x00e00001, 0x01e00022) are still refused through expectFailure. So this accepts a value that means "off" without accepting the feature.
Citable source is in the body (mihawk-99/PS5_Mesa, the RADV-on-PS5 port), which is what a quirk acceptance needs.
Not verified: the RADV path itself — there is no AMD GPU or PS5 Mesa here, so lavapipe only shows that the change does not regress the Mesa path — and Windows.
Written with AI assistance.
…draws With depth bias enabled, RADV sets POLY_OFFSET_PARA_ENABLE in PA_SU_SC_MODE_CNTL next to the front and back enables, whatever the topology; vkQuake's draws carry 0x3a46. The bit applies the offset to points and lines, so it changes nothing for triangles, and Vulkan applies depth bias only to polygons. It is now accepted with depth bias on triangle topologies; point and line draws with it are still rejected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Another disabled value from a retail title, which this PR would still reject: Ratchet & Clank: Rift Apart (PPSA01474) writes Decoded with the field shifts in the kernel's
So both rasterization enables are off, like If you think it fits this PR, would you consider treating any value with Measured on Linux, RX 9070 XT. Tested and written by Claude (AI assistant) on a contributor's machine. |
|
Your restraint is the right instinct and I would not overrule it. With no source for what the uncertainty-region fields do when both enables are clear, accepting If it does get accepted, the shape you propose is the narrowest version: treat a value as disabled only when What would make it easy for them to say yes is the test you offer. If the candidate renders correctly on Rift Apart in the cases where overestimation actually matters, that is evidence rather than an argument — and if you run it, please say what the candidate was and what changed in the frame, not only that it worked. Written with AI assistance. |
|
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.
|
…defaults fix(agc): accept RADV's disabled conservative rasterization and out-of-order watermark
What
RADV on the PS5 (mihawk-99/PS5_Mesa) programs two raster registers with values that mean "off" but that AnyPS5 rejected. ccleste's (PPSA59322) draws were rejected because of them.
Three commits:
PA_SC_MODE_CNTL_1OUT_OF_ORDER_WATER_MARK. RADV sets the watermark to 7 on every GFX10 draw, also whenOUT_OF_ORDER_PRIMITIVE_ENABLEis off. The watermark only tunes out-of-order rasterization, which stays rejected, so the field (bits 30:28) is accepted on its own.PA_SC_CONSERVATIVE_RASTERIZATION_CNTLwhen disabled. RADV writesNULL_SQUAD_AA_MASK_ENABLEalone (0x100000), where libSceAgc writes the twoINNER_TO_NORMALoverrides (0x6000). A value made only of those bits, including 0, now decodes as disabled. Overestimation (0x6001) keeps its checks, and every other mode is still rejected.Point and line depth bias enable on triangle draws (added later, found running vkQuake, PPSA99010). With depth bias on, RADV sets
POLY_OFFSET_PARA_ENABLEinPA_SU_SC_MODE_CNTLnext to the front and back enables whatever the topology (vkQuake's draws carry0x3a46). That bit applies the offset to points and lines, so it changes nothing for triangles, and Vulkan applies depth bias only to polygons. It is now accepted with depth bias on triangle topologies; point and line draws with it are still rejected.Tests (
agc_driver_graphics):OUT_OF_ORDER_PRIMITIVE_ENABLEset the draw is still rejected.0x6000,0x100000and 0 decode as disabled and are not rejected. Two enabled modes (0x00e00001,0x01e00022) are still rejected.0x3a46on a triangle list keeps its depth bias and back-face culling; the same value on a point or line topology is still rejected. It fails with the change reverted.Tested
Windows 11 Pro (26200), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release, NVIDIA GeForce RTX 5070 Ti.
mainwith this PR and my FORCE_DST_ALPHA_1 PR merged together. It was run from the generatedCTestTestfile.cmakecommands because antivirus blocksctest.exeon this machine.tools/check_conventions.py --base origin/mainpasses on this branch alone.agc_driver_graphics,agc_driver_pm4andagc_driverpass on the pushed head; I didn't rerun the full suite for it.Not tested on AMD, Intel, Linux or macOS.
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