Repository navigation
fix(agc): ignore unchanged imported texture blocks - #2984
Conversation
|
Tested this head (c061095, merged cleanly onto Arch Linux (kernel 7.2.9), GCC 16.2.1, Release,
ctest reports 4 The diff is the patch from the issue unchanged, so this is the change the three NVIDIA reports there were made with. Not tested: Windows. |
|
The description notes that the import-watch part of
Tested (Linux, RTX 5090 Laptop GPU 610.57.04 and Intel Arrow Lake ANV, Mesa 26.1.8):
AI-assisted: yes |
|
Checked the diff: the block is only reused when |
|
@adriadam10 @MarcZX100 thanks for merging this. Two things, the second one more important: 1. The driver-independent test, rebased onto 2. On current #2699 makes the DCC fast-clear upload leave Linux, RTX 5090 Laptop GPU (610.57.04),
So the new test catches the regression by itself. It doesn't rely on the import behaving like NVIDIA's, so it should fail on any device whose imports stay watched. I haven't written a fix: it means choosing between #2699's saving and the snapshot this comparison needs (for example, keeping the snapshot on the clear path when the range's import can leave import-window stamps). AI-assisted: yes |
…ched memory range is write-watched. Since boykopovar#2984, Refresh turns a BlockMaybeWritten unit into BlockUnchanged only through compareUntracked, which needs that copy. Where a host import leaves its range watched and making it stamps the imported pages (the probe's writtenAtImport, 4 of 4 on NVIDIA's Linux driver), the import window stamps the surface, so a pending fast-cleared image counted as changed again and agc_driver_dcc_keys_first_write failed on main (boykopovar#3179). HostImports now keeps whether making each kind of import stamped the probe's pages, and WatchedImportsStampPages reports whether an import that leaves its range watched can stamp it. The clear path skips the copy only when no such import can, as oneandonlydean proposed in boykopovar#3179. Devices whose probe saw no pages written at the import, devices without host imports and Windows (whose alias imports don't disarm the guest views) keep boykopovar#2699's saving. An unprobed watch (APS5_WRITE_WATCH_IMPORTS=watch) keeps the copy. The fast-clear check in agc_driver_dcc_keys_first_write now expects the copy exactly when the range is unwatched or such a stamp is possible. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
agc_driver_dcc_keys_first_write only exercises the import window when the device's own host import resolves the write protection of the imported pages, as the NVIDIA driver does on Linux. CI's devices do not, so the comparison that boykopovar#2984 adds for BlockMaybeWritten units is not run there. RunImportWindow opens an import window itself over a pending storage image with GuestMemory::ImportWatched and stores the byte already there, which leaves the same stamp without depending on the driver: the image must stay pending and unchanged. A store of a new byte inside a window must still be seen. Devices whose imports are compared print that the window is not tested, as the recorder's (w) test does. Linux only: the Windows path compares imports. TechnicalDebt records that an identical-byte CPU store inside a window cannot be told apart from the import, and that images tracked by layer still upload again. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ched memory range is write-watched. Since #2984, Refresh turns a BlockMaybeWritten unit into BlockUnchanged only through compareUntracked, which needs that copy. Where a host import leaves its range watched and making it stamps the imported pages (the probe's writtenAtImport, 4 of 4 on NVIDIA's Linux driver), the import window stamps the surface, so a pending fast-cleared image counted as changed again and agc_driver_dcc_keys_first_write failed on main (#3179). HostImports now keeps whether making each kind of import stamped the probe's pages, and WatchedImportsStampPages reports whether an import that leaves its range watched can stamp it. The clear path skips the copy only when no such import can, as oneandonlydean proposed in #3179. Devices whose probe saw no pages written at the import, devices without host imports and Windows (whose alias imports don't disarm the guest views) keep #2699's saving. An unprobed watch (APS5_WRITE_WATCH_IMPORTS=watch) keeps the copy. The fast-clear check in agc_driver_dcc_keys_first_write now expects the copy exactly when the range is unwatched or such a stamp is possible. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
agc_driver_dcc_keys_first_write only exercises the import window when the device's own host import resolves the write protection of the imported pages, as the NVIDIA driver does on Linux. CI's devices do not, so the comparison that #2984 adds for BlockMaybeWritten units is not run there. RunImportWindow opens an import window itself over a pending storage image with GuestMemory::ImportWatched and stores the byte already there, which leaves the same stamp without depending on the driver: the image must stay pending and unchanged. A store of a new byte inside a window must still be seen. Devices whose imports are compared print that the window is not tested, as the recorder's (w) test does. Linux only: the Windows path compares imports. TechnicalDebt records that an identical-byte CPU store inside a window cannot be told apart from the import, and that images tracked by layer still upload again. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What
Fixes #2767. When write tracking reports a 64 KiB texture block as possibly written, compare the block with the image snapshot before treating it as changed. If the bytes are identical, keep the pending GPU image and clear the false CPU-write stamp. This avoids redundant uploads when a host import remains write-watched but did not change guest bytes.
The issue reports the current failure on three NVIDIA setups and confirms the patch on those systems; an Intel report also reproduces the failure with import watching forced. No open PR overlaps this change.
Tested
agc_driver_dcc_keys_first_write_testsagc_driver_dcc_keys_first_write, repeated 5 times: passed 5/5tools/check_conventions.py --base upstream/mainpassedgit diff --checkpassedThe local llvmpipe run reports that write-watch is unavailable, so the import-watch portion of this execution test was skipped locally. The hardware-specific reproduction and patched results are documented by the reporters in #2767.
Checklist
main; no other open PR implements the same functions.mdfiles or images added to the repository