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

Vulkan: Raise number of command buffers #11090

Merged
merged 5 commits into from Oct 1, 2022
Merged

Conversation

K0bin
Copy link
Contributor

@K0bin K0bin commented Sep 24, 2022

In order to keep the GPU busy when Dolphin does EFB readbacks, there's the option to automatically flush the command buffer at specific points instead of just before a readback or at the end of the frame.

The problem with this is that Dolphin currently only has 2 Vulkan command buffers and when both of those are in use, it will just wait for one to become available. So flushing more often could make CPU-GPU parallelism worse if the GPU didn't finish the command buffers faster than Dolphin was flushing.

My Snapdragon 865 phone sits at <30% GPU utilization with Mario Galaxy (1x res) while the CPU spends most of its time waiting for the CPU to finish. So flushing more often is definitely beneficial because it allows the GPU to start working sooner.

This PR raises the number of command buffers to 8. We could easily use a higher number here but 8 seems fine in my very limited testing. Mario Galaxy for example uses ~22 command buffers but 8 is enough here because it stalls before exceeding 8, so we can just reuse the earlier ones.
Dolphins VkDescriptorPools are huge, so I've separated those from the command buffer. The number of VkDescriptorPools is still 2 and they are associated with a frame rather than a command buffer.

I've also tweaked the synchronization logic with the submission thread. Before, it would sync on every submission, now it only syncs on submits which need to finish immediately.

@Pokechu22
Copy link
Contributor

Can you split the m_submit_semaphorem_submit_worker_condvar change (and maybe also the m_frame_resourcesm_descriptor_pools/m_command_buffers change) into separate commits from the one that actually raises the number of buffers, if that wouldn't be too difficult?

@iwubcode
Copy link
Contributor

I looked at this the other night and thought this looked good overall. From glancing it over, it seems like there shouldn't be any issues but I plan on doing some testing tonight since we can't count on fifoci for these changes.

@K0bin
Copy link
Contributor Author

K0bin commented Sep 30, 2022

I'll try to split them up.

…cessary

We only need to synchronize with the submission thread
when submitting on the GPU thread or when waiting for a command buffer.
Avoid waiting for earlier submissions when we flush more often.
The vertex manager will flush more often if the game accesses the EFB
on the CPU, to give the GPU a head start.
Copy link
Contributor

@Pokechu22 Pokechu22 left a comment

Choose a reason for hiding this comment

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

Looks good to me, though I haven't done any testing.

@JMC47
Copy link
Contributor

JMC47 commented Oct 1, 2022

Seems to work, though I haven't combined it with other PRs to see the full potential.

@JMC47 JMC47 merged commit c196c47 into dolphin-emu:master Oct 1, 2022
11 checks passed
@K0bin K0bin deleted the submit-rework branch October 1, 2022 00:18
@TellowKrinkle
Copy link
Contributor

I didn't look especially hard, but what's preventing a third frame from starting when two frames are queued, making a descriptor pool get reset before the GPU is done with it?

@K0bin
Copy link
Contributor Author

K0bin commented Oct 1, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants