Skip to content

fix(vk): retry sparse blocks whose bind failed - #2743

Open
juliotec wants to merge 1 commit into
NotRexed:mainfrom
juliotec:fix/vk-sparse-bind-failure
Open

juliotec wants to merge 1 commit into
NotRexed:mainfrom
juliotec:fix/vk-sparse-bind-failure

Conversation

@juliotec

Copy link
Copy Markdown

sparse_queue marks a tile (or mip tail) bound when it queues its bind. If
vkQueueBindSparse then failed, sparse_flush only logged it: the blocks stayed
marked bound, so no later write queued them again, and the copies went into
unbacked memory. The fence it waits on was also created unchecked.

What changed (gpu/vk/vk_texture.cpp and .h, sparse_flush, sparse_take): a failed bind, or a fence
that could not be created, marks the queued tiles and mip tails unbound again
(new sparse_unqueue) and returns false, so the next write retries them. The
memory those binds had taken goes to a per-texture free list (Texture::sparse_free)
that sparse_take gives out first, so a bind that keeps failing reuses it instead
of taking new memory on every retry. The success path is unchanged.

How tested: release build; Zelda OoT on Radeon RX 9070 XT (Windows,
Adrenalin).

Assisted-by: GitHub Copilot

sparse_queue marks a tile (or mip tail) bound when it queues its bind. If
vkQueueBindSparse then failed, sparse_flush only logged it: the blocks stayed
marked bound, so no later write queued them again, and the copies went into
unbacked memory. The fence it waits on was also created unchecked.

What changed (gpu/vk/vk_texture.cpp and .h, sparse_flush, sparse_take): a failed bind, or a fence
that could not be created, marks the queued tiles and mip tails unbound again
(new sparse_unqueue) and returns false, so the next write retries them. The
memory those binds had taken goes to a per-texture free list (Texture::sparse_free)
that sparse_take gives out first, so a bind that keeps failing reuses it instead
of taking new memory on every retry. The success path is unchanged.

How tested: release build; Zelda OoT on Radeon RX 9070 XT (Windows,
Adrenalin).

Assisted-by: GitHub Copilot

@Joey-Magnus Joey-Magnus 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.

CI never ran here. I merged this with #2718, #2719, #2741 and #2742 onto current main (vk_texture.cpp merges cleanly with #2741), compiled vk_texture.cpp with clang++, and ran tests/run_all.py with the Vulkan headers. None of the failures come from this PR.

The logic is right. sparse_unqueue marks the queued tiles and mip tails unbound again by the same math sparse_queue uses. The memory goes to a free list that only references the texture's own chunks, which are freed with it. The accounting matches the success path.

Two notes:

  • A texture is only sparse when it is a 2D array over RX_VK_TEX_MAX_MB (1.5 GB) with sparse support (gpu/vk/vk_texture.cpp:1957), which is MKW's course-map case. OoT likely never makes one, so the RX 9070 XT test can't have reached this code. The description should say so.
  • With retries, a bind that keeps failing now logs "binding failed" at :3721 on every write to that texture, with no cap, and each attempt also flushes the queue under present_mu. Please cap the log like the other sparse messages (logged++ < 8).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants