Skip to content

fix(agc): store small pending storage results at each flip - #4841

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
gjonn:fix/agc-store-pending-at-flip
Oct 14, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
gjonn:fix/agc-store-pending-at-flip

Conversation

@gjonn

@gjonn gjonn commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

What

A storage image result stays on the GPU until something stores it to guest memory: a driver-observed use of its range (FlushPending through the flush hook, a packet that reads the range), the queue-idle store (#3777), or APS5_STORE_AT_FLIP=1 at PresentDisplayBuffer. A guest CPU that reads the result with plain loads gets none of the first, the second only runs when queue 0 drains, and the third is off by default. While a title keeps queue 0 busy, such a result never reaches guest memory.

PPSA01317 needs a guarantee here. Its CPU auto-exposure reads 16x16 R32F and RGBA16F images that the GPU writes in host-imported memory, a frame or more after they are written. On an older local tree without #3777, those images were marked written 50 to 80 times and stored 0 times, and a guard-page probe on them caught guest CPU loads. The exposure multiplier computed from them collapsed to 2^-32 and the garage car view went black within seconds of each camera cut.

On current main, #3777 covers this whenever queue 0 drains between frames. This change makes the store not depend on the queue going idle. That matters if the idle store is delayed, as #4410 proposes (50 ms): this title's gap between a flip and its next submit is about 5 to 13 ms. I have not run the title on current main; the test below shows the failure on main when the queue stays busy.

Change: when queue 0 reaches a flip packet, before the flip's batch is submitted, StorageTexture::StoreSmallPending stores every pending image of at most SmallStoreBytes (64 KiB) whose memory is host-imported. The stores are recorded on the GPU behind the frame's work and submitted in the flip's batch, so neither the queue worker nor the title waits for the GPU. A flip ends the title's frame, so a result it reads a frame later has been stored by then.

The bounds:

  • 64 KiB: one 64 KiB swizzle block (the largest tile block and the write-stamp granularity). It covers surfaces a CPU reads back as reductions (luminance, histogram and visibility results, up to 128x128 R32F). Full-size targets stay pending as before: storing those at every flip is what APS5_STORE_AT_FLIP=1 does, and its cost is why that switch is off by default (feat(agc): store pending storage-image results at each flip with APS5_STORE_AT_FLIP #1995).
  • Host imports only: a store into memory that is not host-imported goes through the CPU after a GPU wait, which would serialize the queue worker with the GPU at every flip. Such images keep today's behaviour.

APS5_STORE_AT_FLIP=1 is unchanged.

Related: #4794 (open) stores every image result at draw time instead; this PR stores only small imported results, once per frame.

Tested

  • Windows 11 Pro, MinGW GCC 15.2, Release, Ninja, NVIDIA GPU; rebased on main at 90c6839.
  • New agc_driver_flip_stores_small_results: on queue 0, a dispatch writes a 16x16 R32 storage image in imported guest memory, then a flip, then a WAIT_REG_MEM on a dword only the test writes, which keeps queue 0 busy so the queue-idle store cannot run. The test polls the image with plain CPU loads for 2 s. Without the change it fails ("a small image written before a flip did not reach guest memory while the queue kept working"); with it, it passes.
  • ctest -R agc_driver: 347 tests, all passed (1 skipped as on main: agc_driver_stencil_ref_export).
  • PPSA01317, on the older local tree (without fix(agc): store pending storage-image results when queues idle #3777) with the same store done at the flip: the garage car view stayed lit in 3 of 3 runs (before: black within seconds of each camera cut). The stores cost about 0.37 ms per flip.
  • python tools/check_conventions.py --base origin/main passes.

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: PPSA01317 Gran Turismo 7 (PPSA01317) #4697
  • AI-assisted: yes

@kr4y4

kr4y4 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

An Intel run, and the full suite.

Setup: Windows 11 (26200), WinLibs GCC 15.2.0 r7, Release, CI configure flags, an RTX 4070 Laptop GPU and an Intel Raptor Lake-S iGPU. The head fb388af merged into main 495bf91, which passes 1023 of 1023 on both GPUs.

  • agc_driver_flip_stores_small_results passes on both GPUs with --repeat until-fail:3, and it ran on both without returning 77.
  • Full suite (ctest --timeout 120 --parallel 4): 1024 of 1024 on both GPUs.

No negative control here: with the production files as on main the test doesn't build, because StorageTexture::SmallStoreBytes is new.

Written with AI assistance.

@xXPBSXx

xXPBSXx commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Windows test (MinGW GCC 15.2, Release, Windows 10, RTX 4080 SUPER). Built on current main 3779597 together with four other open GPU PRs (their own commits cherry-picked, no merge commits), so only the tests that belong to this PR say something about it:

  • tests/execution/FlipStoresSmallResults.cpp (agc_driver_flip_stores_small_results): 3 of 3 runs passed.

Not tested: no game run. The batch was built once, not per PR; nothing failed, so no bisect was needed.

AI-assisted (Claude Code).

@gjonn
gjonn force-pushed the fix/agc-store-pending-at-flip branch from fb388af to 0b5a4aa Compare October 14, 2026 13:13
@gjonn

gjonn commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main 63fb0cd (force-push, one commit, 0b5a4aa). The only conflict was in Texture.hpp, where main added SaveBeforeUnwatch next to SmallStoreBytes and StoreSmallPending; both are kept. agc_driver_flip_stores_small_results passes on Windows (MinGW GCC, Release, RTX 5090 Laptop), and tools/check_conventions.py passes.

Thanks @kr4y4 and @xXPBSXx for the runs.

AI-assisted: yes

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

Copy link
Copy Markdown
Contributor

A game run: Kena: Bridge of Spirits (PPSA01802) shows this on its title screen, and this PR fixes it.

Setup: Linux (Arch, kernel 7.2.9), GCC, Release, RX 9070 XT (RADV, Mesa 26.2.4). Kena does not reach its title on plain main (it stops at buffer descriptor uses an unsupported type, which #5049 covers), so the runs are on a local stack: main f74ec63 + #5049 + #5156 + #4722 + #1201 (with APS5_PIPELINED_DRAWS=1) + local stand-ins, APS5_SYNC_FLIP=1. This PR's commit 0b5a4aa was cherry-picked on top. Each run starts from the same save and sits on the title; every presented frame from the same point on was dumped (APS5_DUMP_FRAMES) and its mean brightness measured.

  • Without this PR: the whole screen flickers in a 3-frame cycle, white, white, then a normal frame. 26 of 40 title frames were white (mean brightness above 0.9). [storage] write-backs by reason: memory access 284, no flip stores.
  • With this PR: 0 of 34 title frames white, the title is stable and correctly exposed. [storage] write-backs: flip 560/30.0 MiB over the run. No GPU reset.
  • agc_driver_flip_stores_small_results passes here (Linux, RADV).

The 3-frame cycle and the fix fit the exposure readback described above (small GPU-written results read by the CPU a frame later, here apparently rotating through three images). With queue 0 kept busy by the pipelined committer, the queue-idle store does not cover it.

Before finding this PR I had a narrower local version (images of at most 4x4 texels, stored through FlushPending at the flip); I am dropping it in favour of this one.

AI-assisted (Claude Code).

@adriadam10 adriadam10 self-assigned this Oct 14, 2026
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked on Linux (merged onto current main): agc_driver_flip_stores_small_results passes, and it fails with the flip hook in PacketMemory.cpp removed. The rest of the agc scope is green except agc_driver_tessellation, which segfaults here with and without this change. Also confirmed by @kr4y4 (Windows, RTX 4070 + Intel iGPU), @xXPBSXx (Windows, RTX 4080 SUPER) and @alfalfa-star (Linux RADV, Kena title screen flicker gone). Merging once the full Build is green.

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.

5 participants