Skip to content

perf(agc): pipeline queue-0 draw commits behind the packet worker - #6203

Closed
ivica00 wants to merge 3 commits into
boykopovar:mainfrom
ivica00:perf/agc-pipelined-draws
Closed

ivica00 wants to merge 3 commits into
boykopovar:mainfrom
ivica00:perf/agc-pipelined-draws

Conversation

@ivica00

@ivica00 ivica00 commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

What

Queue 0 can decode and capture a draw on the packet worker while a committer thread records it. Work the committer has not reached is tracked by the guest ranges it will write; a later packet that reads or writes them, or one the pipeline cannot order, drains it.

Off unless APS5_PIPELINED_DRAWS is set. Labels, the end of a queue-0 submission, and buffer fills stay ordered as commit items (APS5_PIPELINE_DRAIN_LABELS, APS5_PIPELINE_DRAIN_SUBMISSIONS, APS5_PIPELINE_DRAIN_FILLS drain them instead). A null storage image in a draw's written set writes nothing, and a pipelined release with INT_SEL 3 raises no interrupt.

Rebased onto current main. Upstream ShaderMemory already moves, so this does not add a separate move constructor. Descriptor-set recycling forwards the update-after-bind and variable-count arguments Allocate takes here.

Tested

The pipeline behavior is what the fork runs. Hello Neighbor 2 title and gameplay were exercised on this machine with pipelined draws enabled. This rebase onto current main was not run as its own title session.

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
  • Unblocks: none
  • GPUs tested: AMD Radeon RX 9070 XT, Adrenalin 32.0.32015.2008, Windows 10 (fork build with this pipeline; this rebase was not run separately)
  • AI-assisted: yes

MichonGoddijn231849 and others added 3 commits October 15, 2026 04:49
…S5_PIPELINED_DRAWS)

With APS5_PIPELINED_DRAWS=1, queue 0's worker decodes, captures and recompiles a draw and hands it to
a committer thread that records it (Graphics::Draw under the GPU lock), so the two run in parallel
instead of one after the other. Work the committer has not reached yet is tracked by the guest ranges
it will write; a later packet that reads or writes them, or one the pipeline cannot order, drains it.

Ordered as commit items instead of drains:
- labels (WRITE_DATA, RELEASE_MEM) and same-queue waits on them; APS5_PIPELINE_DRAIN_LABELS=1 drains;
- the end of a queue-0 submission and a released REWIND tail; APS5_PIPELINE_DRAIN_SUBMISSIONS=1 drains;
- the title's buffer fill kernels; APS5_PIPELINE_DRAIN_FILLS=1 drains;
- DMA_DATA immediates and copies (APS5_PIPELINE_DMA=1).
A draw's write ranges include the storage images it writes and its exact color and depth target
ranges (APS5_EXACT_DRAW_WRITES=1), not estimates.

Draw-path costs the pipeline exposed:
- ShaderMemory moves, so a commit owns its capture's pages;
- a submission made before the worker's current collect epoch began keeps the epoch, so a worker
  behind a queue of submissions walks a surface once per catch-up (APS5_SUBMISSION_EPOCH_EACH=1 bumps
  at every submission); a thread remembers the ranges it walked for the whole epoch;
- the pipeline signals only threads that wait; the pending-image and unit-shadow registries take a
  spinning slim lock instead of std::mutex (winpthreads contention was kernel events); ScanPending skips
  the registry walk when no pending image can overlap;
- fewer heap allocations per recorded draw (reused descriptor sets and scratch vectors, buffer pool
  size classes kept).

GTA V Performance mode, prologue bank interior, RTX 5080: 11.7 -> about 20 fps together with the cheaper
captures below it.
Off by default here; the follow-up turns it on.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Gjon Gojcevic <7807870+gjonn@users.noreply.github.com>
DrawWriteRanges decoded every written storage image element, and DecodeTextureResource refuses a zero
base address, so a draw with a null storage image bound failed (PPSA01317) instead of skipping it.
The element is now skipped by the binder's own null rule (base address words zero) before it is
decoded. agc_driver_draw_write_ranges binds a null image next to a written one.

Co-Authored-By: Gjon Gojcevic <7807870+gjonn@users.noreply.github.com>
Co-Authored-By: SleglJan <90033145+SleglJan@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The pipelined label path treated every nonzero RELEASE_MEM INT_SEL as an end-of-pipe interrupt;
main's PacketMemory excludes 3 (send data after write confirm). agc_driver failed on it with
APS5_PIPELINED_DRAWS=1 ("release with INT_SEL send data after write confirm raised an interrupt").

Co-Authored-By: TheAggie <67454+TheAggie@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 15, 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
#1201 core/libs/prx/libSceAgcDriver/Execution/include/Driver/Draw/DrawPipeline.hpp
core/libs/prx/libSceAgcDriver/Execution/include/Driver/Memory/DrawWriteRanges.hpp
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
core/libs/prx/libSceAgcDriver/Graphics/include/SlimMutex.hpp
core/libs/prx/libSceAgcDriver/tests/execution/DrawWriteRanges.cpp
core/libs/prx/libSceAgcDriver/tests/execution/PipelinedDma.cpp
#1202 core/libs/prx/libSceAgcDriver/Execution/include/Driver/Draw/DrawPipeline.hpp
core/libs/prx/libSceAgcDriver/Execution/include/Driver/Memory/DrawWriteRanges.hpp
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
core/libs/prx/libSceAgcDriver/Graphics/include/SlimMutex.hpp
core/libs/prx/libSceAgcDriver/tests/execution/DrawWriteRanges.cpp
core/libs/prx/libSceAgcDriver/tests/execution/PipelinedDma.cpp
#1747 core/libs/prx/libSceAgcDriver/Execution/include/Driver/Draw/DrawPipeline.hpp
core/libs/prx/libSceAgcDriver/Execution/include/Driver/Memory/DrawWriteRanges.hpp
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
core/libs/prx/libSceAgcDriver/Graphics/include/SlimMutex.hpp
core/libs/prx/libSceAgcDriver/tests/execution/DrawWriteRanges.cpp
core/libs/prx/libSceAgcDriver/tests/execution/PipelinedDma.cpp
#1896 core/libs/prx/libSceAgcDriver/Execution/include/Driver/Draw/DrawPipeline.hpp
core/libs/prx/libSceAgcDriver/Execution/include/Driver/Memory/DrawWriteRanges.hpp
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
core/libs/prx/libSceAgcDriver/Graphics/include/SlimMutex.hpp
core/libs/prx/libSceAgcDriver/tests/execution/DrawWriteRanges.cpp
core/libs/prx/libSceAgcDriver/tests/execution/PipelinedDma.cpp
#5867 core/libs/prx/libSceAgcDriver/Execution/include/Driver/Draw/DrawPipeline.hpp
core/libs/prx/libSceAgcDriver/Execution/include/Driver/Memory/DrawWriteRanges.hpp
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawPipeline.cpp
core/libs/prx/libSceAgcDriver/Graphics/include/SlimMutex.hpp
core/libs/prx/libSceAgcDriver/tests/execution/DrawWriteRanges.cpp
core/libs/prx/libSceAgcDriver/tests/execution/PipelinedDma.cpp

@github-actions github-actions Bot added build agc libSceAgc* labels Oct 15, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 15, 2026
@adriadam10 adriadam10 self-assigned this Oct 15, 2026
@adriadam10

Copy link
Copy Markdown
Collaborator

Closing as a duplicate of #1201: DrawPipeline.cpp, DrawPipeline.hpp and DrawWriteRanges.hpp here are byte-identical to #1201's head, and the commits are its authors' (Michon, gjonn, TheAggie). #1201 is still open and has an active review thread (ordering fixes from @Raghav314 and @eliyarson, A/B data on several titles), so a second copy of the same change would only split that discussion and conflict with it.

Also: this PR conflicts with main, declares Depends on: none while sitting on top of #1201, #1202, #1747, #1896 and #5867, and the body says the rebase was not run on any title.

If you have a fix or measurement for the pipelined path, please post it on #1201 (or as a branch there), and feel free to reopen with a distinct change.

@adriadam10 adriadam10 closed this Oct 15, 2026
@adriadam10

Copy link
Copy Markdown
Collaborator

The fixes from this review (BufferPool, stage lifetimes, ReportFailure, CMASK, fill drains, pipelined test variants) are now on #1201, rebased on main without #1191: #1201 (comment). #6208 can build on #1201.

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

Labels

agc libSceAgc* build needs rebase Conflicts with main, rebase needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants