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

VideoBackends:Metal: Allocate bounding box uploads on a cpu buffer #11854

Merged
merged 1 commit into from Jun 2, 2023

Conversation

TellowKrinkle
Copy link
Contributor

AMD Metal drivers have a goofy bug where the bbox buffer stops being coherent with the cpu if you copy to it from a private (gpu) buffer and don't do anything else with it in that command buffer.

Test case available here if anyone's curious

The old way was actually missing a required fence wait on the upload fence (to prevent the GPU from attempting the copy before it uploaded the copy source), but that's not the actual cause of the breakage, as indicated by the above test case, which doesn't disable hazard tracking and therefore doesn't have any fence requirements.

Side note, if anyone has any AMD GPUs running not-Ventura, please run the linked test case and report back, I'd love to figure out if that bug is new or if it's been around for a while.

@OatmealDome
Copy link
Member

Tried the testing program on a MacBookPro16,1 running macOS 12.4, and it fails.

$ ./AMDCoherency 
Intel(R) UHD Graphics 630
1024 1024 0 0
256 256 511 511
256 256 511 511
AMD Radeon Pro 5500M
1024 1024 0 0
1024 1024 0 0
1024 1024 0 0

Also, to fix the build failure, rebase onto latest master.

AMD Metal drivers have a goofy bug where the bbox buffer stops being coherent with the cpu if you copy to it from a private (gpu) buffer and don't do anything else with it in that command buffer.
@TellowKrinkle
Copy link
Contributor Author

Tried the testing program on a MacBookPro16,1 running macOS 12.4, and it fails.

Good to know
For reference, I've tested it on a 13.3.1 (a) MacBookPro16,4 (UHD 630, Radeon Pro 5600M), and a 11.7.5 MacBookPro11,3 (Iris Pro 5200, GT 750M), and so far only AMD has been broken

The in-dolphin effect is bbox sometimes breaking, I had the ttyd boat flip scene break fairly consistently off one save state but not off another and not off running from the start with no save state load, which was fun

@iwubcode
Copy link
Contributor

iwubcode commented Jun 1, 2023

So is this still in draft since Oatmeal had some issues?

@TellowKrinkle
Copy link
Contributor Author

TellowKrinkle commented Jun 2, 2023

No, those issues are on a test program meant to reproduce the failure that this PR works around

(And I did rebase onto (then) latest master, which is why there are no more build failures)

@OatmealDome OatmealDome merged commit 7bbd530 into dolphin-emu:master Jun 2, 2023
14 checks passed
@TellowKrinkle TellowKrinkle deleted the AMDFix branch June 2, 2023 04:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants