Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only allocate device memory if explicitly requested #2269

Merged

Conversation

doitsujin
Copy link
Collaborator

@doitsujin doitsujin commented Jan 4, 2025

Matches DXVK behaviour and should fix #2258, but needs testing.

The idea here is that if we want to allocate system memory (as a fallback or otherwise, e.g. UPLOAD heaps in an app that also uses GPU upload heaps), we should never try to allocate from device-local memory unless all supported memory types for a given allocation are device-local.

Also fixes the possible case where a fallback allocation with no property flags would attempt to allocate from DEVICE_LOCAL again. This only hasn't been an issue so far because most drivers allow oversubscribing VRAM, but this is not guarateed.

@doitsujin doitsujin force-pushed the memory-fallback-thing branch from 3227a46 to 5f8b1fb Compare January 4, 2025 12:33
NVK currently only exposes VRAM, HVV and cached sysmem in that
order, which essentially breaks no_upload_hvv because the first
memory type with HOST_VISIBLE | HOST_COHERENT set is HVV.

Signed-off-by: Philip Rebohle <[email protected]>
Signed-off-by: Philip Rebohle <[email protected]>
@doitsujin doitsujin force-pushed the memory-fallback-thing branch from 5f8b1fb to ff2badc Compare January 6, 2025 07:47
@doitsujin doitsujin marked this pull request as ready for review January 6, 2025 11:18
@Sid127
Copy link

Sid127 commented Jan 6, 2025

Confirmed, games no longer run into "out of video memory" errors on NVK (current git as of writing this comment)
Tested with Amid Evil DX12 and Marvel Rivals

Copy link
Owner

@HansKristian-Work HansKristian-Work left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@HansKristian-Work HansKristian-Work merged commit 639c69f into HansKristian-Work:master Jan 7, 2025
3 checks passed
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.

HVV -> Sysmem fallback logic is flawed
3 participants