Skip to content

fix(agc): take a CB resolve's sample count from its source target - #2121

Merged
oneandonlydean merged 1 commit into
boykopovar:mainfrom
Raghav314:fix/agc-cb-resolve-source-samples
Oct 13, 2026
Merged

oneandonlydean merged 1 commit into
boykopovar:mainfrom
Raghav314:fix/agc-cb-resolve-source-samples

Conversation

@Raghav314

@Raghav314 Raghav314 commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

What

Follow-up to #419, as discussed there. DecodeColorMetadataPass counted a draw with CB_COLOR_CONTROL in resolve mode as a CB resolve only when PA_SC_AA_CONFIG had a sample count, and required the source target to match it. A title that resolves with the rasterizer single-sample fell through to the normal draw path, which refuses it with "only normal color rendering with copy ROP is supported (CB_COLOR_CONTROL 0x00cc0030, mode resolve)".

A color target's own sample count is CB_COLOR_ATTRIB.NUM_SAMPLES, separate from the rasterizer's. radeonsi itself writes PA_SC_AA_CONFIG = 0 for a multisampled framebuffer when rasterizer multisampling and line smoothing are off (si_emit_msaa_config).

  • A resolve-mode draw is a CB resolve when either the rasterizer or CB0 is multisampled, and it takes its sample count from CB0.
  • When the rasterizer is multisampled, the source still has to match it, as before.
  • A resolve-mode draw where neither is multisampled is left to the draw path, as before (the existing test for that is unchanged).
  • TechnicalDebt gets an entry, since the rule is inferred from the title and radeonsi rather than from register documentation.

PPSA36082

Hades II resolves its 4x targets with no sample count in PA_SC_AA_CONFIG. On main f145139 with the fixes it needs to get that far plus #419, its resolve draws were skipped with the error above. With this change they go through as CB resolves. (It also needs a fix for its non-standard 4x sample positions to render correctly, which is a separate follow-up.)

Tested

Linux (CachyOS), x86-64, GCC, 7800X3D / RTX 4090.

  • agc_driver_graphics (new checks):
    • The existing 2x resolve with PA_SC_AA_CONFIG cleared still decodes as a resolve, with its 2 samples taken from CB0.
    • A 4x rasterizer with a 2x source is still refused ("differs from the rasterizer's").
  • Mutation: going back to "resolve only when the rasterizer is multisampled" fails the new check.
  • The linux job of current build.yml run locally on this head, rebased on main d1fd937 (SPIRV tools on, test registration check, build, libs, ctest): 1002/1004. guest_kernel_fcntl failed only because my script ran ctest with no terminal on stdin; it passes when run from a terminal. agc_driver_pixel_lds hung the GPU once (Xid 109). It is the intermittent hang I also see on other heads, doesn't touch CB resolves, and passed in the next run on the same machine.

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
  • AI-assisted: yes

@github-actions github-actions Bot added build docs agc libSceAgc* needs rebase Conflicts with main, rebase needed labels Oct 9, 2026
@Raghav314
Raghav314 marked this pull request as draft October 9, 2026 12:51
MichalSy added a commit to MichalSy/AnyPS5 that referenced this pull request Oct 9, 2026
Adapt the relevant AA_CONFIG case from upstream PR boykopovar#2121. A color resolve may disable rasterizer multisampling while its source still contains eight samples. Keep source, destination, geometry, EQAA, and unsupported-bit checks strict.

Verify the decoded resolve fields and draw key coverage, and reject a single-sample source and unsupported coverage bits.
@Mouhand-Kaddo

Copy link
Copy Markdown
Contributor

Additional platform validation on standalone PR head 3dc5175ec0000311e11ac00f2b7fd26156b87d73:

Windows: passed — Windows 11, supported WinLibs GCC 15.2.0 UCRT/POSIX/SEH r7; CMake 4.3.1 / Ninja.

Release / BUILD_TESTING=ON: built the selected test targets; 1 CTest cases, 0 failures, 0 CTest skips.

Tests: agc_driver_graphics.

Reproduce after building the test targets: ctest --test-dir <build> --verbose --no-tests=error --timeout 30 -R "^agc_driver_(graphics|cb_resolve)$".

Vulkan: native AMD RX 6900 XT, driver 32.0.21045.5002.

Integration limitation: a local merge against main 948a526c2 has source conflicts; these results validate the standalone PR head, not a resolved merge.

@Raghav314
Raghav314 force-pushed the fix/agc-cb-resolve-source-samples branch from 3dc5175 to 04747b2 Compare October 11, 2026 23:48
@KingDarkHN

Copy link
Copy Markdown

Confirmed needed: Fallen Legion (PPSA07645) issues a mode-3 CB resolve while CB_AA_CONFIG (0x2f8) reports a single sample, so DecodeColorMetadataPass does not treat it as a resolve and the draw is rejected with:

AGC graphics: only normal color rendering with copy ROP is supported (CB_COLOR_CONTROL 0x00cc0030, mode resolve)

Taking the sample count from the resolve source target (this PR) unblocks it. Two further MSAA-guard findings are on #1189. Repro: run app.elf from a standard PPSA07645 layout.

@Raghav314
Raghav314 force-pushed the fix/agc-cb-resolve-source-samples branch from 24b0c54 to b4dc347 Compare October 13, 2026 02:18
@oneandonlydean oneandonlydean self-assigned this Oct 13, 2026
DecodeColorMetadataPass treated a CB_COLOR_CONTROL resolve as one only
when PA_SC_AA_CONFIG has a sample count, and checked the source against
it. A title that resolves with the rasterizer single-sample fell through
to the draw path and threw "only normal color rendering with copy ROP is
supported (... mode resolve)".

A color target's sample count is CB_COLOR_ATTRIB.NUM_SAMPLES, separate
from the rasterizer's; radeonsi itself writes PA_SC_AA_CONFIG 0 for a
multisampled framebuffer when rasterizer multisampling and line
smoothing are off. So a resolve is decoded when either the rasterizer or
CB0 is multisampled, and its sample count is CB0's. When the rasterizer
is multisampled, the source still has to match it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@oneandonlydean
oneandonlydean force-pushed the fix/agc-cb-resolve-source-samples branch from b4dc347 to d57f9cf Compare October 13, 2026 03:14
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Rebased onto main myself (one conflict, the debt line next to #4199's entry; both kept). Build and ctest pass on d57f9cf.

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
@github-actions

github-actions Bot commented Oct 13, 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
#4767 core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp L1105-1107
#4861 core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp L1109-1110

@oneandonlydean
oneandonlydean merged commit 3fed8e9 into boykopovar:main Oct 13, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants