Skip to content

agc: #2699 undoes #2984 where host imports stay write-watched, so agc_driver_dcc_keys_first_write fails on main (NVIDIA); driver-independent test branch ready #3179

Closed

Description

@milescoviello

agc_driver_dcc_keys_first_write fails on main again on Linux with an NVIDIA GPU (an unchanged pending image was uploaded again), although #2984 fixed it (#2767). The fix holds on #2984's own head (c061095), but not on its merge commit 3db065a or on current main f1ae427. Details first posted on #2984: #2984 (comment)

Cause: a first-parent bisect of the 88 merges between #2984's base (4a747c4) and its merge, applying c061095 at each step, points to #2699 (926ad9e, "read a fast-cleared surface's guest bytes only when the import is compared"). #2699 makes the DCC fast-clear upload leave originalValid false when the range is write-watched. #2984's comparison needs that snapshot: compareUntracked returns false without one, so a unit of a fast-cleared surface that an import window stamped BlockMaybeWritten counts as changed again. Each change is right on its own; they conflict where host imports stay watched.

Setup: Gentoo, Linux 7.2.8, GCC 15.3.0, Release with CI's configure flags. RTX 5090 Laptop GPU (610.57.04); Intel Arrow Lake ANV (Mesa 26.1.8) for comparison.

Tree (with the test commit below applied) agc_driver_dcc_keys_first_write, NVIDIA
main f1ae427 fails 3/3: an unchanged pending image was uploaded again
main f1ae427, only the new import-window part (the existing watched run skipped) fails 3/3: import window: a pending image was uploaded again after an import that stored nothing new
main 59a8cba with #2699's three-line clear-path change and its two test lines reverted passes 3/3 (whole test); Intel ANV passes too
#2984's head c061095 passes 3/3

Test branch ready: milescoviello/AnyPS5:test/dcc-import-window, one commit, 63649253, which merges cleanly into main f1ae427. RunImportWindow opens an import window itself over a pending storage image with GuestMemory::ImportWatched and stores the byte already there. That leaves the same BlockMaybeWritten stamp as the NVIDIA import, without depending on the driver, so it should catch this on any device whose imports stay watched. The test's surface is fast-cleared (its keys start at 0000), so it hits exactly the #2699 path. It fails on current main and passes with #2699's change reverted. It also adds one TechnicalDebt entry for #2984's comparison (an identical-byte CPU store inside a window can't be told apart from the import; images tracked by layer still upload again).

I haven't written a fix for the conflict itself: it means choosing between #2699's saving and the snapshot #2984 needs (for example, keeping the snapshot on the clear path when the range's import can leave import-window stamps).

Not opened as a PR because intake is paused for new contributors; a maintainer is welcome to carry the commit.

AI-assisted: yes

Activity

  1. oneandonlydean commented on Oct 11, 2026

    @oneandonlydean
    Collaborator

    Confirming this on another NVIDIA host: RTX 4090, 610.57.04, Linux, GCC 16, Release, main 77bd928.

    Tree agc_driver_dcc_keys_first_write
    main fails 3/3: an unchanged pending image was uploaded again
    main with #2699 reverted (git revert -m1 926ad9e3) passes 3/3
    main on lavapipe (Mesa 25.2.8) passes; the probe reports 0 after the import there, so no import window stamps the surface

    The code path is as described above. Since #2699, StorageTexture::upload leaves originalValid false on the fast-clear path when the range is watched. Since #2984, Refresh turns a BlockMaybeWritten unit into BlockUnchanged only through compareUntracked, which returns false without that snapshot. GuestMemory::ImportWatched stamps StampKind::ImportWindow exactly when the import itself marks pages as written: 4 after the import on this NVIDIA host, 0 on lavapipe.

    One way to keep both: skip the snapshot on the clear path only when the host-import probe saw no pages written at import (writtenAtImport == 0). That is the case where no import window can stamp the surface. It needs the probe result kept in HostImports. @alfalfa-star, @MarcZX100, which of you wants to take it?

  2. milescoviello commented on Oct 11, 2026

    @milescoviello
    ContributorAuthor

    @oneandonlydean thanks for confirming it and for the proposal. In case it saves @alfalfa-star or @MarcZX100 the time, I put it on a branch. I'm happy to drop it if one of you has already started.

    milescoviello/AnyPS5:fix/fast-clear-snapshot-import-window, two commits on main 961b25f:

    • 7cc5adfb fix(agc): keep a fast-clear copy where an import window can stamp watched memory
    • 53130615 test(agc): the driver-independent import-window test from this issue.

    Tested (Linux; RTX 5090 Laptop GPU 610.57.04; Intel Arrow Lake ANV, Mesa 26.1.8):

    agc_driver_dcc_keys_first_write
    main f1ae427, NVIDIA fails 3/3: an unchanged pending image was uploaded again
    fix only, NVIDIA passes 3/3
    fix + test, NVIDIA passes 3/3
    fix + test, Intel (imports compared) passes 3/3; prints that the import window is not tested

    Full ctest with both commits: NVIDIA 761/761. Intel ANV 750/760, and all ten failures fail the same way on plain main 961b25f (f64_fma_subnormals, flat_store, gated_dispatch_capture, masked_global_store, vop1_float_unary, vopc_compare and the four prepared_shaders variants), none in this path. tools/check_conventions.py passes.

    Not changed: the direct host-import upload path in StorageTexture::upload (uncompressed keys) also skips the copy when the range is watched. I haven't checked whether an import window can reach an image uploaded there, so I left it out of this change.

    Not opened as a PR because intake is paused for new contributors; a maintainer is welcome to carry the commits.

    AI-assisted: yes

  3. milescoviello commented on Oct 11, 2026

    @milescoviello
    ContributorAuthor

    Rebased the branch onto main 4f60248, since 60f12c8 (#3226) changed the same lines in decideImportWatch. Thanks @fma-rc for flagging the conflict and for running #1532. The two flags now sit next to #3226's writtenAfterFill decisions. Nothing else in the change moved, and the commit message no longer lists lavapipe among the devices that skip the copy, since #3226 makes lavapipe compare its imports. Same branch, new commits: 4a085303 (fix) and 7ec9747b (test). It also merges cleanly into today's main b64cc2c.

    agc_driver_dcc_keys_first_write Result
    main 4f60248, NVIDIA fails 3/3: an unchanged pending image was uploaded again
    rebased fix + test, NVIDIA passes 3/3
    rebased fix + test, Intel ANV (imports compared) passes 3/3

    Full ctest on NVIDIA with both commits: 763/763. tools/check_conventions.py passes. As fma-rc's run shows, #1532's (s) stop has a different cause.

    AI-assisted: yes

  4. alfalfa-star commented on Oct 11, 2026

    @alfalfa-star
    Contributor

    @milescoviello @oneandonlydean I've carried milescoviello's two commits (4a085303, 7ec9747b, unchanged and kept under their name) as #3372, since the regression comes from my #2699. On my RX 9070 XT (RADV) the full ctest passes 813/813, but RADV compares its imports, so this host doesn't exercise the fix; the NVIDIA results in the PR are milescoviello's. Thanks to both of you for the bisect and the fix.

    AI-assisted: yes (Claude Code)

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions