Repository navigation
feat(agc): draw PS4 multisampled color targets as host-resident targets - #9892
Merged
oneandonlydean merged 10 commits intoNov 4, 2026
Merged
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions
Bot
added
unblocks title
|
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.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
Merged
12 tasks done
12 tasks done
12 tasks done
This was referenced Oct 23, 2026
Collaborator
|
@savvaniss This PR now conflicts with main in |
12 tasks done
Brings in main 07da04f through the boykopovar#9431 merge offered as Get-fix#22. No file conflicts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
@oneandonlydean merged, no rebase. The head is now
On the RX 9070 XT with the validation layer:
AI-assisted: yes (Claude Code) |
12 tasks done
Collaborator
|
@savvaniss The metadata and layered-depth merge points are answered. Please remove the two GcnColorTarget.cpp assertions requiring valid EQAA and single-sample fast-clear configurations to throw because they are unimplemented. Keep the supported 2/4/8-sample cases, the missing-CMASK-address check, the partial-coverage validation and the production refusals with their debt. #9431 remains an open prerequisite for this branch. |
12 tasks done
12 tasks done
Contributor
|
This head These are AI-assisted: yes (Claude Code) |
Contributor
|
A Windows run of this head on two NVIDIA generations and an AMD iGPU: Windows 11 Home 10.0.26200, WinLibs GCC 15.2.0 r7 (MinGW-w64 UCRT), Release.
Update: in the run above, the AI-assisted: yes (Claude Code) |
This was referenced Oct 28, 2026
XairRrR
suggested changes
Oct 29, 2026
XairRrR
left a comment
XairRrR
left a comment
Contributor
There was a problem hiding this comment.
Reviewed — changes requested
This PR's own change (the GCN multisample decode, the CMASK fields and the padded-extent rule) builds and passes its tests when I apply it to main + #9431's current head. The head 545e2b5b3f itself conflicts with main and does not compile on Linux, because it carries an older #9431.
What I ran
- Head:
545e2b5b3f83fed05a1a34fb21b1fdfda0c0707a main:b433c40f2f46aa2e59e19806010a839ac8572d64- #9431 head used for the reconstruction:
eefa50841267cd0bd5352af17248139445fb0613 - Host: Arch Linux 7.2.11-zen, GCC 16.2.1, Release, Ninja, Intel Core i9-14900KF, NVIDIA GeForce RTX 4070 Ti SUPER (driver 615.78.08)
# 1. merge onto main
git checkout --detach origin/main
git merge --no-ff --no-edit 545e2b5b3f # conflicts, aborted
git diff --check origin/main...545e2b5b3f
python3 tools/check_conventions.py --base origin/main
# 2. head alone
git checkout --detach 545e2b5b3f
cmake --build build -j10 --target agc_gcn_color_target_tests # fails
# 3. this PR's own commit on main + #9431's current head
git checkout --detach origin/main
git merge --no-ff --no-edit eefa508412 # clean
git diff 4e8243ac48 545e2b5b3f | git apply --3way # clean
cmake --build build -j10 --target agc_gcn_color_target_tests agc_driver_register_key_tests \
agc_driver_recorder_tests agc_driver_pm4_tests agc_driver_graphics_tests agc_driver_gcn_detile_tests \
agc_driver_multisample_fill_host_only_tests agc_driver_multisample_readback_tests agc_driver_pattern_buffer_fill_tests
VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation ANYPS5_NO_SHADER_CACHE=1 ANYPS5_REQUIRE_VULKAN=1 \
VK_LOADER_DEBUG=error,warn,layer ctest --test-dir build -j4 -V \
-R '^(agc_gcn_color_target|agc_driver_graphics|agc_driver_pm4|agc_driver_register_key|agc_tiled_resolve|agc_driver_gcn_detile|agc_driver_multisample_fill_host_only|agc_driver_multisample_readback|agc_driver_pattern_buffer_fill)$'
# 4. the same 8 tests (all but agc_gcn_color_target) on main + #9431 without this PR's commit, same flagsResults
| Check | Result |
|---|---|
Merge onto main b433c40f2f |
Conflicts in Graphics/include/State.hpp (2 hunks) and Graphics/src/State.cpp (2 hunks) |
git diff --check |
clean |
tools/check_conventions.py --base origin/main |
All conventions checks passed. |
Build of the head 545e2b5b3f (Linux, GCC 16) |
fails: PipelineJournal.cpp:41-42 static asserts |
Own commit on main + #9431 eefa508412: build |
OK |
agc_gcn_color_target |
Passed (0.00 s) |
| Validation layer, PR (9 tests) | 9/9 pass, 0 VUID- lines, no was not found but was requested by env var VK_INSTANCE_LAYERS |
| Validation layer, base without the PR (8 tests) | 8/8 pass, 0 VUID- lines, no was not found but was requested |
| Layer actually loaded | yes: loader log shows VK_LAYER_KHRONOS_validation inserted; every Selected GPU line names the RTX 4070 Ti SUPER |
Merge conflicts with main
CONFLIT (contenu) : Conflit de fusion dans core/libs/prx/libSceAgcDriver/Graphics/include/State.hpp
CONFLIT (contenu) : Conflit de fusion dans core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp
State.hpp:ShaderStages(mainaddshullWaveSizeandtessellationGeometry, the branch addslegacyVertex) andDrawKeyRegisters(array size 50 onmain, 52 on the branch).State.cpp:decodeDepthPlane's extent (GCNDB_DEPTH_SIZEbranch) andDecodeShaderStages(main's tessellation-geometry path against the branch's GCN legacy-vertex block).
All four hunks are #9431 code. #9431's current head resolves them: it merges cleanly with this main.
Build of the head alone
FAILED: core/libs/prx/libSceAgcDriver/CMakeFiles/libSceAgcDriver.dir/Graphics/src/PipelineJournal.cpp.o
PipelineJournal.cpp:41:29: error: static assertion failed: State changed: update CanonicalPipelineState and the recipe codec
• the comparison reduces to '(848 == 840)'
PipelineJournal.cpp:42:36: error: static assertion failed: ShaderStages changed: update the recipe codec
• the comparison reduces to '(116 == 112)'
Validation-layer run on the PR
1/9 Test #717: agc_gcn_color_target .................... Passed 0.00 sec
2/9 Test #948: agc_driver_pm4 .......................... Passed 0.14 sec
3/9 Test #828: agc_tiled_resolve ....................... Passed 0.28 sec
4/9 Test #715: agc_driver_register_key ................. Passed 0.84 sec
5/9 Test #1275: agc_driver_multisample_fill_host_only ... Passed 0.60 sec
6/9 Test #1126: agc_driver_gcn_detile ................... Passed 0.95 sec
7/9 Test #1276: agc_driver_multisample_readback ......... Passed 0.47 sec
8/9 Test #1303: agc_driver_pattern_buffer_fill .......... Passed 0.78 sec
9/9 Test #949: agc_driver_graphics ..................... Passed 4.97 sec
100% tests passed out of 9
Negative control
On the reconstructed tree I restored only Graphics/src/State.cpp to the version on main + #9431, rebuilt agc_gcn_color_target_tests and ran it. It fails, as the description says:
1/1 Test #717: agc_gcn_color_target .............***Failed 0.00 sec
AGC graphics: multisampled GCN color targets are not implemented
I then restored the PR's State.cpp.
Findings
Blocking
- Conflicts with
main. See the table above. The conflicting hunks are #9431 code that this branch carries from the older merge4e8243ac48. #9431 has since moved on (eefa508412, rebased onto a newermain), and that head merges cleanly. Fix: rebuild this branch on #9431's current head, or rebase the single commitdb05653037onto it. This PR's own diff (CMakeLists.txt,State.cpp,GcnColorTarget.cpp, the debt entry) applies tomain+eefa508412without conflicts. The checklist line "Based on currentmain" does not hold for the current head. - The head does not compile on Linux x86-64 with libstdc++. This confirms babyhuey's report, now on the head alone and not only on a merge with a newer
main. The guards atPipelineJournal.cpp#L41-L42expectsizeof(State) == 840andsizeof(ShaderStages) == 112, but #9431'sShaderStages::legacyVertex(State.hpp#L35) makes them 848 and 116. The guards sit under#if defined(__linux__) && defined(__x86_64__) && defined(__GLIBCXX__), which is why the Windows/MinGW runs in the description and in ycbuzi's comment did not hit them. #9431 fixed this ineefa508412("update the pipeline journal layout guards for ShaderStages::legacyVertex"), so moving to that head (finding 1) fixes this too.
Non-blocking
- A resolve accepted by the padded-extent rule writes rows the pass does not cover. At
State.cpp#L2200-L2202,drawCoversswaps the target's padded extent for the covered extent, so the pass is accepted. The host resolve still uses the whole padded extent:MultisampleTarget.cpp#L1275setsrenderArea = {{0, 0}, source.extent}. For a 1920x1088 target resolved with a 1080-row viewport, the host writes destination rows 1080 to 1087, which the console leaves alone. Those rows are usually tile padding, but a texture with the padded height, or a copy of the whole surface, would see different bytes. Fix: either limit the resolve's render area to the covered extent, or state in the debt entry that the padding rows outside the viewport are resolved (and that an FMASK decompress also expands them). - Requested test change still open. oneandonlydean asked for the two assertions that a valid EQAA target and a single-sample fast clear throw to be removed. They are still in the head:
GcnColorTarget.cpp#L100-L101. The head has not changed since that comment. - Debt entry order. CONTRIBUTING.md asks that an entry state the gap first and describe the implemented behaviour only as far as the gap needs. The new entry opens with the implemented behaviour and puts "None of it is checked on a console" last. Suggest leading with the gaps (not checked on a console; FMASK contents not modeled; EQAA, more than 8 samples and single-sample fast clear throw; the padded-extent acceptance) and shortening the description of the implemented behaviour.
I checked the decode against what the AGC multisample path consumes and found nothing wrong. The GCN path fills only cmaskAddress, cmaskBytes and cmaskFastClear. With dccAddress == 0, MultisampledCmask only acts on whole-range uniform fills (NoteFill, SeedCode), so the pipe-interleaved GCN CMASK layout does not matter, and a single slice's (CMASK_SLICE.TILE_MAX + 1) * 128 bytes is covered by the guest's aligned clear. MultisampledColorBytes returns 0 for GCN tile modes, so a memory fill over the color surface is refused, not absorbed. That is a throw, not silent behaviour.
Not verified
- Behaviour in Boggle (CUSA03312) or any other title; I launched no title.
- The test on the head itself, which does not build here; all test results above are from
main+ #9431eefa508412+ this PR's own diff. - The tests on plain
mainwithout #9431:agc_gcn_color_target,agc_driver_gcn_detileandagc_driver_pattern_buffer_filldo not exist there. The baseline ismain+ #9431. - The
video_out_flip_*tests and theagc_driver_depth_*tests from the description; I ran only the tests closest to this PR's own change. - AMD or Intel GPUs; NVIDIA only.
To get this to a good state
- Rebuild the branch on #9431's current head (
eefa508412or later) so it merges cleanly withmainand builds on Linux (blocking findings 1 and 2). - Settle oneandonlydean's request about
GcnColorTarget.cpplines 100 and 101. - Optionally, limit the resolve's render area to the covered extent or document the extra rows, and reorder the debt entry.
12 tasks done
Contributor
✅ NVIDIA GPU test: passedPR head
Failing on main too (3, not counted against this PR)Validated but not compared (1, not in the main baseline's validation pass)Validation IDs already present on main (5 IDs in 480 tests)Skipped (44)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
Collaborator
|
Hi savvaniss, main moved today, and this PR no longer merges cleanly with main. These conflicts overlap your changes, so they need your call:
Could you merge main into this branch and resolve them, keeping what main added? A merge commit is fine; no need to rebase or force-push. Once it's pushed we'll re-review the merge. |
This branch and main both contain 26d651d, so a default merge uses a virtual base and reports conflicts that neither side made. Merged against main's last merge base cae3eda instead. One conflict: GuestTextureResource.hpp, where main added linearPitch and this branch added fmaskCmaskAddress at the same place; keep both, main's first. The result differs from main by exactly this branch's own changes.
Collaborator
|
Hi XairRrR, Could you take another look at #9892? Head
|
Contributor
✅ NVIDIA GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Passed on retry (1, failed in the parallel run, passed when re-run with a 360 s timeout; not counted against this PR)Validated but not compared (1, not in the main baseline's validation pass)Validation IDs already present on main (4 IDs in 510 tests)Skipped (45)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
Contributor
Windows re-run at
|
| Run | ctest -R '^agc_gcn_color_target$' |
|---|---|
| Merge of this PR | 1/1 pass |
Same tree with main's Graphics/src/State.cpp restored |
0/1; fails with "AGC graphics: multisampled GCN color targets are not implemented" |
XairRrR's Blocking 1 (duplicate fields)
Resolved in 5c8fb7de19: GuestTextureResource.hpp declares gcnPitch, gcnPow2Pad and gcnLinearGeneral once each.
Non-blocking: the header still differs from main. git diff upstream/main...fde9ff2d9a -- core/libs/prx/libSceAgcDriver/Graphics/include/GuestTextureResource.hpp shows only fmaskCmaskAddress moving from before gcnPitch to after linearPitch. git grep at the head finds no sizeof/offsetof assert or memcmp of GuestTextureResource; the field writes are by name (GuestTextureResource.cpp:309, MultisampleTarget.cpp:85, :103). I'd expect the move to be harmless. With main's order restored, the file would drop out of the PR's diff.
XairRrR's optional notes (the resolve's render area, the debt entry order) are unchanged at this head, as oneandonlydean's comment says.
Contributor
✅ AMD GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (12, not counted against this PR)Passed on retry (1, failed in the parallel run, passed when re-run with a 360 s timeout; not counted against this PR)Validated but not compared (1, not in the main baseline's validation pass)Validation IDs already present on main (4 IDs in 513 tests)Skipped (54)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
XairRrR
approved these changes
Nov 3, 2026
XairRrR
left a comment
XairRrR
left a comment
Contributor
There was a problem hiding this comment.
Re-checked — approved
5c8fb7de19 removes the duplicate GuestTextureResource fields that blocked the previous head, and fde9ff2d9a merges current main. The merged tree builds, agc_gcn_color_target passes and fails with main's production files, and both failures that look new also fail with main's production files.
What changed since my last review
5c8fb7de19deletes the second copies ofgcnPitch,gcnPow2PadandgcnLinearGeneral.9110e4e17a,83a0d8ea00andfde9ff2d9amergemain. The AGC test registration now lives intests/cmake/agc_gcn_color_target_tests.cmake, followingmain's split. The PR's ownState.cppchange, test and debt entry are unchanged.- The description now says
Depends on: noneand lists eachmainmerge with its test results. - In the discussion, oneandonlydean, BunnyFl (Windows, RX 9070 XT) and TechLevelZero's NVIDIA CI ran the PR's test at this head. These results are author- and contributor-reported.
What I ran
- Head
fde9ff2d9a1a81604be4962c7f717fb10d06e3c6,main5cb015caaaaf83f8ee431414ee6a457b5b307d49, merged treed7c08d33052c2d537b6490e4fa0f540615dbdc50. - Machine: Debian 13 VM, GCC 14.2.0, Release, lavapipe (Mesa 26.1.6, LLVM 19). There is no hardware GPU.
git diff --check 5cb015caaa...fde9ff2d9a
python3 tools/check_conventions.py --base 5cb015caaa
ctest --test-dir build --output-on-failure -R '^(agc_gcn_color_target|agc_driver_image_sample_lod_clamp_shader_lane|unwrap_self_UnwrapTest_test_new_output_inherits_the_destination_directory_group)$'
# negative control: State.cpp and GuestTextureResource.hpp from main
ninja -C build agc_gcn_color_target_tests agc_driver_image_sample_lod_clamp_tests
ctest ... (same three tests)
# restore the merged files, then rebuild
ctest --test-dir build -j8 --output-on-failure -R '^(agc_gcn_color_target|agc_driver_graphics|agc_tiled_resolve|agc_driver_gcn_detile)$'
git status --porcelain # empty| Check | Result |
|---|---|
Merge onto main |
clean |
git diff --check |
clean |
check_conventions.py |
All conventions checks passed. |
| Build (script) | OK |
| Selected tests (script) | 2115 of 2235 passed and 112 skipped as expected. 6 failures are known on main; I traced the 2 that look new to main (see below). |
| Validation layer (script, 759 tests) | 0 tests with a new VUID-; the layer loaded in 758 |
agc_gcn_color_target, agc_driver_graphics, agc_tiled_resolve, agc_driver_gcn_detile |
4/4 pass |
Diff against main |
only this PR's 5 files, +183/−5 |
The 2 failures that look new are not the PR's. The script's baseline (096c45570f) is older than main 5cb015caaa. With the PR's production files restored from main and the targets rebuilt, both still fail in the same way:
agc_driver_image_sample_lod_clamp_shader_lanereportsthread 0 component 1 is 0.000000, expected 0.062500.unwrap_self_UnwrapTest_test_new_output_inherits_the_destination_directory_groupreportsOSError: [Errno 22]fromos.chown, which is the sandbox's group handling.
Run with main's production files
OSError: [Errno 22] Invalid argument: ...
2/3 Test #1772: agc_driver_image_sample_lod_clamp_shader_lane ...***Failed 3.15 sec
AGC graphics: image_sample_lz ... thread 0 component 1 is 0.000000, expected 0.062500
3/3 Test #2035: agc_gcn_color_target ............................***Failed 0.01 sec
AGC graphics: multisampled GCN color targets are not implemented
Negative control
I restored Graphics/src/State.cpp and Graphics/include/GuestTextureResource.hpp from main and rebuilt. agc_gcn_color_target then fails with AGC graphics: multisampled GCN color targets are not implemented. With the merged files restored and rebuilt, it passes, and git status --porcelain is empty.
Previous findings
- New blocking 1, duplicate fields: Resolved. At the head,
GuestTextureResource.hpp#L108-L112declares each field once, and the merged tree builds. - New non-blocking 1, stale description: Mostly resolved.
Depends onis now none and each merge is listed. The Base paragraph still says "This branch is now545e2b5b3f", and its numbers are for older heads. The latest results the description itself gives are for83a0d8ea00. - Non-blocking, a resolve accepted by the padded-extent rule renders the full extent: Still open, and optional as before. The rule in
State.cpp#L2376-L2379is unchanged. The resolve'srenderAreais stillsource.extent(MultisampleTarget.cpp#L1468). oneandonlydean says this was left on purpose. - Non-blocking, order of the debt entry: Still open, and optional. The entry still describes the implemented behaviour before the gaps.
New findings
Blocking
None.
Non-blocking
fmaskCmaskAddressmoves inGuestTextureResource.hppfor no reason. This confirms BunnyFl's note. The PR's only change to the header (L112) moves the field belowlinearPitch. I found no positional initialization,sizeof,offsetoformemcmpof the struct; every use assigns by name (GuestTextureResource.cpp:309,MultisampleTarget.cpp:85,:103), so the move is harmless. Restoringmain's order would remove the file from the diff and avoid future merge conflicts there.
Not verified
- Any title, including Boggle (CUSA03312). I launched no title.
- Hardware GPUs. Lavapipe only here; the AMD and NVIDIA results in the discussion are contributor-reported.
- Windows.
github-actions
Bot
added
review queue
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
XairRrR
approved these changes
Nov 3, 2026
XairRrR
left a comment
XairRrR
left a comment
Contributor
There was a problem hiding this comment.
Re-checked — approved
The only new commit is 3c0ee17922, a merge of main 1e658521a0. The PR's own three commits are byte-identical in range-diff, and the diff against main is still the same five files (+183/−5). The merged tree builds, agc_gcn_color_target passes and fails again with main's production files, and the one test flagged with new VUID- lines reports the same lines with main's production files.
What changed since my last review
3c0ee17922mergesmain1e658521a0intofde9ff2d9a. The conflict indocs/dev/debt/libSceAgcDriver.mdis resolved by keeping both sides: the PS4 multisampled color target entry (now L119) sits directly abovemain's render-target entry (L120), andmain's new PS4 depth-target sentence is kept.range-diffshowsdb05653037,6a7d99f3beand5c8fb7de19unchanged.- The description adds a "Merge of
main1e658521a0" paragraph with RX 9070 XT results for this head. These results are author-reported.
What I ran
- Head
3c0ee179221d343ec624148fe66a3b7c3f90f436,main086c0686c8adff3ee648fdbe34e0b0b647e1d511, merged tree1c4c5e95106cae318149af2b382b3cd106062d4b. - Machine: Debian 13 VM, GCC 14.2.0, Release, lavapipe (Mesa 26.1.6, LLVM 19). No hardware GPU.
git diff --check 086c0686c8...3c0ee17922
python3 tools/check_conventions.py --base 086c0686c8
git diff --stat 086c0686c8 1c4c5e9510
env VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation ANYPS5_NO_SHADER_CACHE=1 ANYPS5_REQUIRE_VULKAN=1 \
ctest --test-dir build -V -R '^(agc_driver_image_gather_volume|agc_gcn_color_target)$'
# negative control: State.cpp and GuestTextureResource.hpp from main
ninja -C build -j32 -l32 agc_gcn_color_target_tests agc_driver_image_gather_volume_tests
env VK_INSTANCE_LAYERS=... ctest --test-dir build -V -R '^(agc_driver_image_gather_volume|agc_gcn_color_target)$'
# restore the merged files, rebuild
ctest --test-dir build -j8 --test-load 32 --output-on-failure -R '^(agc_gcn_color_target|agc_driver_graphics|agc_driver_gcn_detile)$'
git status --porcelain # empty| Check | Result |
|---|---|
Merge onto main |
clean |
git diff --check |
clean |
check_conventions.py |
All conventions checks passed. |
Build and libs (script) |
OK |
| Selected tests (script) | 2117 of 2239 passed, 115 skipped as expected. 0 new failures and 0 timeouts. The 7 failures also fail on main. |
| Validation layer (script, 783 tests) | The layer loaded in 782 tests and was missing in none. One test, agc_driver_image_gather_volume, was flagged as "not run on main". It comes from main (see below). |
agc_gcn_color_target, agc_driver_graphics, agc_driver_gcn_detile |
3/3 pass |
The agc_driver_image_gather_volume VUIDs (VUID-VkSamplerCreateInfo-addressModeU-01079 and the addressModeU/V/W-parameter ones) are not the PR's. I restored main's State.cpp and GuestTextureResource.hpp and rebuilt. The test then reports the same four validation errors with the same message IDs, and it still passes. The script's baseline did not run this test. The PR changes only the GCN color-target decode and the metadata-pass coverage check, so it does not reach sampler creation.
With main's production files
1236: Validation Error: [ VUID-VkSamplerCreateInfo-addressModeU-01079 ] | MessageID = 0x1aa95c2b
1236: Validation Error: [ VUID-VkSamplerCreateInfo-addressModeU-parameter ] | MessageID = 0x3fcec07
1236: Validation Error: [ VUID-VkSamplerCreateInfo-addressModeV-parameter ] | MessageID = 0xf8361085
1236: Validation Error: [ VUID-VkSamplerCreateInfo-addressModeW-parameter ] | MessageID = 0xd3b90dae
1/2 Test #1236: agc_driver_image_gather_volume ... Passed 10.64 sec
2/2 Test #2104: agc_gcn_color_target .............***Failed 0.01 sec
2104: AGC graphics: multisampled GCN color targets are not implemented
Negative control
With Graphics/src/State.cpp and Graphics/include/GuestTextureResource.hpp restored from main and rebuilt, agc_gcn_color_target fails with AGC graphics: multisampled GCN color targets are not implemented. With the merged files restored and rebuilt, it passes, and git status --porcelain is empty.
Previous findings
- Duplicate fields (blocking in an earlier review): Resolved. These were already resolved at the last review and are unchanged here.
- Stale description: Still open, non-blocking. The new paragraph covers this head. The Base paragraph still says "This branch is now
545e2b5b3f", though, and the long validation list below it is for older heads. - A resolve accepted by the padded-extent rule renders the full extent: Still open, optional.
State.cpp#L2388is unchanged. - Order of the debt entry: Still open, optional. The entry still describes the implemented behaviour before the gaps.
fmaskCmaskAddressmoved inGuestTextureResource.hpp: Still open, non-blocking. The field is still belowlinearPitch(L112-L113). The move is harmless, but restoringmain's order would drop the file from the diff.
New findings
Blocking
None.
Non-blocking
None.
Not verified
- Any title, including Boggle (CUSA03312). I launched no title.
- Hardware GPUs and Windows. I ran on lavapipe only; the RX 9070 XT results in the description are author-reported.
Contributor
✅ NVIDIA GPU test: passedPR head
Unstable on main (4, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Validated but not compared (1, not in the main baseline's validation pass)Validation IDs already present on main (4 IDs in 517 tests)Skipped (48)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
github-actions
Bot
added
the
review queue
github-actions
Bot
added
needs rebase
Contributor
✅ AMD GPU test: passedPR head
Failing on main too (6, not counted against this PR)Passed on retry (2, failed in the parallel run, passed when re-run with a 360 s timeout; not counted against this PR)Validated but not compared (55, not in the main baseline's validation pass)Validation IDs already present on main (4 IDs in 506 tests)Skipped (111)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
github-actions
Bot
added
review queue
Contributor
✅ NVIDIA GPU test: passedPR head
Unstable on main (3, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Validated but not compared (1, not in the main baseline's validation pass)Validation IDs already present on main (8 IDs in 531 tests)Skipped (50)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
Contributor
✅ AMD GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (7, not counted against this PR)Validated but not compared (1, not in the main baseline's validation pass)Validation IDs already present on main (8 IDs in 531 tests)Skipped (61)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
github-actions
Bot
removed
the
review queue
12 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Get-fix#6, moved upstream. #9431 has merged, so the diff against
mainis now only this pull request's own files. oneandonlydean mergedmaininto the branch and fixed it on 29 Oct (head5c8fb7de19), and removed the two refusal assertions (EQAA and single-sample fast clear) fromtests/GcnColorTarget.cppthat their review asked to drop.On #9431 a PS4 color target with more than one sample threw "multisampled GCN color targets are not implemented". Boggle (CUSA03312) renders its 3D scenes into a 4x target, then resolves it with a CB resolve, after an FMASK decompress.
DecodeGcnColorBuffernow decodes 2x, 4x and 8x targets (CB_COLOR_ATTRIB NUM_SAMPLES) when NUM_FRAGMENTS matches. These go to the host-resident multisampled targets that the AGC path already draws, fast-clears and resolves.The debt file gains an entry for PS4 multisampled color targets.
main'sdrawCoversnow holds the CB metadata pass's viewport and scissor test. The padded-extent rule above is an optional GCN target argument of that helper.Tested
Windows 11 Pro 26300, WinLibs GCC 15.2.0, Release, AMD Radeon RX 9070 XT (driver 32.0.31041.1004),
ANYPS5_GPU=AMD.Merge of
main828ac0b2ae, 31 Oct. Head9110e4e17a. The only conflict was indocs/dev/debt/libSceAgcDriver.md: both sides kept.agc_gcn_color_target,agc_driver_graphicsandagc_driver_gcn_detilepass, 3 of 3 on the RX 9070 XT, noVUID-line. Not rerun: the other tests, Linux, games.Merge of
maincae3edaf11, 1 Nov. Head83a0d8ea00. The only conflict waslibSceAgcDriver/CMakeLists.txt:mainmoved the AGC test registrations intotests/cmake, andtools/agc_test_split.py resolvemovedagc_gcn_color_target_teststo its own fragment.agc_gcn_color_target,agc_driver_graphicsandagc_driver_gcn_detilepass, 3 of 3 on the RX 9070 XT under the validation layer, noVUID-line. Not rerun: the other tests, Linux, games.Merge of
main1e658521a0, 3 Nov. Head3c0ee17922(onfde9ff2d9a). The only conflict was indocs/dev/debt/libSceAgcDriver.md: the merge keeps this branch's PS4 multisampled color target entry andmain's render-target entry with its new PS4 depth-target sentence.FormatVersionstaysmain's 52 (this branch does not change it) and no size guard changes. On the RX 9070 XT (Windows, Release) withANYPS5_GPU=AMD ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation:agc_gcn_color_target,agc_driver,agc_driver_graphicsandagc_shader_memorypass, 4 of 4, everySelected GPUline names the RX 9070 XT and there is noVUID-line.tools/check_conventions.py --base origin/mainpasses. Not rerun: the rest of the suite, Linux, and the game.Base. This branch is now
545e2b5b3f, built in two merges:main07da04f947merged into feat(libSceAgcDriver): draw PS4 (GCN) command buffers #9431's head26d651d3e0, asmerge/9431-main(4e8243ac48). That merge is offered to Get-fix as chore: merge main into ps4/agc-gcn Get-fix/AnyPS5#22.merge/9431-mainmerged into this branch, with no file conflicts.agc_gcn_color_target(new) covers these cases:With
State.cppas onmerge/9431-main, on545e2b5b3f, it fails with "multisampled GCN color targets are not implemented".Validation layer: run with
ANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1andVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation,ctest -V, onmain07da04f947and on this branch. The tests:agc_gcn_color_target,agc_driver,agc_driver_pm4,agc_driver_graphics,agc_driver_register_key,agc_driver_gcn_detileandagc_driver_pattern_buffer_fillagc_driver_flipand the 16video_out_flip_*modesagc_gcn_decode,texture_format_lookup,agc_driver_resource_cache_replaced_textureandagc_depth_copy_kept_bytesagc_driver_depth_*testsResults:
main: 48 of 49 pass (agc_gcn_color_targetandagc_driver_gcn_detileare not onmain).agc_driver_depth_plane_partial_store, "repeated lookups of the first layer of an unchanged array proved no refresh", on every run on this machine.VUID-line orwas not found but was requested by env var VK_INSTANCE_LAYERS. EverySelected GPUline names the RX 9070 XT.Boggle (CUSA03312), on
ps4/limbo-gpuat93eeee0141(feat: run PS4 games on the AGC driver — Limbo (CUSA01664) in game (integration branch) #6977's head then) with fix(agc): PS4 display, textures and GCN instructions for CUSA03312 Get-fix/AnyPS5#1, fix(build): register relinker tests with CTest #2 and Boot Demon's Souls to its first level: AGC recorder driver, runtime, audio, Bink fixes #5, fix(agc): retire a multisampled color target the title wrote over #8223, fix(agc): bind a strided buffer's last record only up to the render target it runs into #8224 and local stand-ins forsceGnmDrawInitDefaultHardwareState200andsceGnmUpdate*Shader: its title, menus and a round render through the 4x target, the FMASK decompress and the resolve, at 16 to 60 fps. Not rerun on this base: the title also needs feat(libSceGnmDriver): build PS4 command buffers and submit them #9432.tools/check_conventions.py --base origin/mainpasses.tools/pr_overlap.py --branch feat/ps4-gcn-msaa-targets, againstmain88f99678bf. This PR's own lines conflict only indocs/dev/debt/libSceAgcDriver.md, on neighbouring entries, with three pull requests:The other rows come from feat(libSceAgcDriver): draw PS4 (GCN) command buffers #9431. feat: run PS4 games on the AGC driver — Limbo (CUSA01664) in game (integration branch) #6977, feat(libSceGnmDriver): build PS4 command buffers and submit them #9432, fix(libSceAgcDriver): sample a PS4 depth buffer through a T# smaller than its padded extent #9672 to fix(libSceGnmDriver): report type-0 packets and optionally wait at each submission #9676, fix(libSceAgcDriver): pad level 0 rows of a POW2_PAD PS4 mip chain #9842 and fix(libSceAgcDriver): size a PS4 depth target's HTILE in 512x512 blocks #10435 share its new files and exports. feat(agc): draw tessellation followed by an NGG geometry stage #6612, perf(agc): store whole words of 1- and 2-byte texels in the detile shader #6905, feat(agc): back extra guest mip levels with storage images #7085, fix(agc): raise no end-of-pipe event from flips and carry the release's interrupt context #7978, perf(agc): GPU descriptor heap on VK_EXT_descriptor_heap (APS5_GPU_HEAP) #9508, perf(agc): reuse user-data callee links and check a cached raw shader's range once #9899, fix(agc): follow RDNA 2's range checks for strided vertex fetches with OOB_SELECT 2 and 3 #9915, fix(agc): build a partially committed sampled 3D view from its committed slices #10064, fix(agc): clamp an exported pixel depth to the viewport depth range #10348, fix(shader): decode a T# with an unset format field as null #10556, perf(agc): take the texture detiler's Vulkan entry points from the device table #10764 and fix(agc): select the first level of a single-level depth view #11098 conflict in files feat(libSceAgcDriver): draw PS4 (GCN) command buffers #9431 changes, and conflict the same way with
merge/9431-mainalone.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)main)🤖 Generated with Claude Code