Skip to content

fix(agc): accept the REWIND offload-enable payload bit - #7867

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
xXPBSXx:fix/agc-rewind-offload-bit
Oct 23, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
xXPBSXx:fix/agc-rewind-offload-bit

Conversation

@xXPBSXx

@xXPBSXx xXPBSXx commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

What

Pm4::Validate rejects a REWIND packet whose payload has any bit other than bit 31 set (unsupported REWIND payload bits, Pm4.cpp). PPSA15246 writes 0x81000000: bit 31 (valid) and bit 24. shadPS4's PM4CmdRewind (src/video_core/amdgpu/pm4_cmds.h) describes the payload as two fields, bit 31 valid ("subsequent packets are valid") and bit 24 offload_enable (lets the CP offload polling of the valid bit to the IQ), and its Valid() tests bit 31 only. KytyPS5's CpOpRewind (src/graphics/guest_gpu/command_processor/pm4Handlers.cpp) accepts exactly bits 31 and 24 of the payload, stops with a not-implemented error on any other bit, and waits on bit 31 only. The driver here also waits on bit 31 only (executeRewindTail), so bit 24 needs no handling beyond being accepted.

This lets the validator accept bit 24 together with bit 31 and keeps rejecting every other bit. New testRewindPayloadBits cases in agc_driver_pm4_tests: 0x00000000, 0x80000000, 0x01000000 and 0x81000000 validate; 0x82000000, 0x80000001 and 0x00800000 are still rejected.

Title evidence: PPSA15246 (Teardown), Windows 10, main 3ddbc4acc plus an unmerged local change that lets a zero header word through (run_062434.err, line 149): driver failure: AGC driver: REWIND at DWORD 190: unsupported REWIND payload bits, dump dwords c0005900 81000000. On unmodified main this title stops earlier, at unsupported PM4 packet type: header 0x00000000 at DWORD 15, in front of a 169-dword prefix of zeros; that stop is not changed here, so this PR alone does not get Teardown further. It removes the next one.

Tested

Windows 10 (10.0.19045.6466), MinGW-w64 GCC 15.2.0 (WinLibs), Release, on main 7a36de66b9 + this commit (rebased afterwards onto 77b3edf32a as c0b68e443; no commit in between touches the changed files):

  • ctest -N -R '^agc_driver_pm4$' lists exactly one test; ctest -R '^agc_driver_pm4$' --no-tests=error: passes 3 of 3 (testRewindPayloadBits included).
  • With Execution/src/Pm4.cpp restored to main (git checkout upstream/main -- <file>, git status shows only that file staged, 1 insertion / 1 deletion; the new test kept): the same ctest entry fails 3 of 3 (ctest exit code 8).
  • python tools/check_conventions.py --base upstream/main with this title and body passes.
  • No GPU or Vulkan device is involved (command-stream validation only), and no Khronos validation layer is installed on this machine.
  • Not tested: Linux, a title run past the zero-header stop in front of the REWIND (see What).

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 under Silent stubs in their library's technical debt file
  • 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
  • Fixes: none
  • Unblocks: none yet (PPSA15246 stops earlier at the zero-filled command prefix; no Game bring-up discussion exists for it)
  • GPUs tested: n/a (command validation only)
  • AI-assisted: yes

AI-assisted (Claude Code).

🤖 Generated with Claude Code

A REWIND packet's payload carries a valid bit (31) and an offload-enable bit (24) that lets the CP offload polling of the valid bit. The validator rejected every bit but 31, so a title that writes both (0x81000000, PPSA15246) stopped with 'unsupported REWIND payload bits'. The driver waits on bit 31 only, so bit 24 needs no other handling.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the unblocks title Names a title ID on the Unblocks line label Oct 19, 2026
@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

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.

Pull request Shared files not compared
PR 6186 core/libs/prx/libSceAgcDriver/tests/Pm4.cpp
PR 6188 core/libs/prx/libSceAgcDriver/Execution/include/Pm4.hpp
core/libs/prx/libSceAgcDriver/Execution/src/Pm4.cpp
PR 6549 core/libs/prx/libSceAgcDriver/Execution/include/Pm4.hpp
core/libs/prx/libSceAgcDriver/Execution/src/Pm4.cpp
core/libs/prx/libSceAgcDriver/tests/Pm4.cpp
PR 9893 core/libs/prx/libSceAgcDriver/Execution/src/Pm4.cpp
core/libs/prx/libSceAgcDriver/tests/Pm4.cpp

@fogydes fogydes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified on Windows 11 with MinGW-w64 GCC 15.2.0 (Release):

  • Architecture & Diff: In core/libs/prx/libSceAgcDriver/Execution/src/Pm4.cpp, allowing RewindOffloadEnable (0x01000000u, bit 24) alongside RewindValid (0x80000000u, bit 31) in Pm4::Validate aligns packet validation with CP offload polling semantics seen in PPSA15246 without altering driver wait behavior in executeRewindTail.
  • Unit Testing: Verified testRewindPayloadBits in core/libs/prx/libSceAgcDriver/tests/Pm4.cpp. Built and ran ctest --test-dir build-libs -R "^agc_driver_pm4$" -V passing in 0.24s:
    1/1 Test #422: agc_driver_pm4 ...................   Passed    0.24 sec
    
    All catalog, register, state, memory, and conditional execution fixtures passed.
  • Conventions: No code comments added; tools/check_conventions.py passes cleanly.

@github-actions github-actions Bot added the agc libSceAgc* label Oct 19, 2026
@savvaniss

Copy link
Copy Markdown
Contributor

AMD and Intel Windows data: with this PR's test changes, agc_driver_pm4 fails on main and passes with the PR, on an AMD RX 9070 XT and an Intel UHD 770.

Windows 11 Pro 26300, WinLibs GCC 15.2.0 r7, Release, every execution test built, ANYPS5_NO_SHADER_CACHE=1, serial CTest. Intel UHD Graphics 770 (8086:a780), driver 32.0.101.7088. AMD Radeon RX 9070 XT (1002:7550), driver 32.0.31041.1004, with VkCache emptied first. This head c0b68e443 merged into main 85029a00f. "Without" means only this PR's diff to tests/Pm4.cpp, applied to main 85029a00f.

Tests, ×3 UHD 770 RX 9070 XT
This head: the 13 tests built from tests/Pm4.cpp 13 of 13 pass 13 of 13 pass
Without agc_driver_pm4 fails: unsupported REWIND payload bits; the other 12 pass same

AI-assisted: yes (Claude Code)

@adriadam10 adriadam10 self-assigned this Oct 19, 2026
@github-actions github-actions Bot removed the unblocks title Names a title ID on the Unblocks line label Oct 20, 2026
@adriadam10
adriadam10 merged commit 777bdbb into boykopovar:main Oct 23, 2026
11 checks passed
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the validator change against shadPS4 PM4CmdRewind and KytyPS5 CpOpRewind (bits 31 and 24 only), the driver wait on bit 31 only, and the new testRewindPayloadBits rows (valid and still-rejected payloads). Re-checked on current main after #6794: the patch applies cleanly and the REWIND line is unchanged. Third-party runs on Windows (fogydes, savvaniss) confirm the test fails on main and passes with the change. Checked on the merge queue full Linux and Windows build; merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc*

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants