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

FifoPlayer: Wait after clearing the screen #10273

Merged
merged 2 commits into from Dec 27, 2021

Conversation

Pokechu22
Copy link
Contributor

Continues #10258. If we don't wait, then the copy will be performed at a later time, which may overwrite memory updates. This was causing problems for me when messing with memory updates, since the EFB copy would end up overwriting memory that had been written by the memory update. I think this may have been the cause of some of the differences in #10264, and will submit a second PR to check.

@Pokechu22
Copy link
Contributor Author

#10274 shows that this improves early memory updates (though early memory updates still isn't a good solution). So although this produces no visible fifoci differences, it is an improvement.

@Pokechu22 Pokechu22 force-pushed the fifoplayer-efb-clear-wait branch 3 times, most recently from 641dec2 to 379add9 Compare December 22, 2021 23:22
@iwubcode
Copy link
Contributor

I ran some fifo logs and all of them ran as a black screen. Master shows them correctly.

@Pokechu22
Copy link
Contributor Author

It seems like that happens with dual core (but not single core). However. fifoci uses dual core if I recall correctly, and it ran without issues so I'm not sure quite what's going on.

The actual values don't matter since we overwrite all of the relevant fields, but other bits were not initialized (e.g. the top 12 bits of X10Y10), so the warning was semi-valid.
If we don't wait, then the copy will be performed at a later time, which may overwrite memory updates.
@Pokechu22
Copy link
Contributor Author

Fixed. I needed to add this to the fifoplayer's CPUCore::Init:

// Without this call, we deadlock in initialization in dual core, as the FIFO is disabled and
// thus ClearEfb()'s call to WaitForGPUInactive() never returns
CPU::EnableStepping(false);

(Also, I was incorrect in my thought that FifoCI uses dual core; I misinterpreted what CPUThread = False meant in configs. CPUThread = False actually means "don't use a separate CPU thread from the GPU thread", i.e. "don't use dual core".)

Copy link
Contributor

@iwubcode iwubcode left a comment

Choose a reason for hiding this comment

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

Dual core and single core now work. Code changes make sense

@JMC47 JMC47 merged commit 1f1e78e into dolphin-emu:master Dec 27, 2021
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants