Skip to content

video_out: SubmitEopFlip adding mutex and lockguard also fixing reject of full flips by adding a wait #2992

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

diegolix29
Copy link
Contributor

This stops the flip bug we have on video out on my end while testing dark souls 3 fire fades
updating buffer labels, and adding buffer id for logging on error
let me know if its ok.

Also fixed a discrepancy on driver.cpp

@GHU7924
Copy link

GHU7924 commented May 26, 2025

[CUSA03745] The Last Guardian

Errors appeared:

[Render.Vulkan] vk_pipeline_cache.cpp:493 CompileModule: Compiling vs shader 0xc8e8ce26
[Render.Vulkan] vk_pipeline_cache.cpp:493 CompileModule: Compiling fs shader 0xf694dd0e
[Render.Vulkan] vk_pipeline_cache.cpp:493 CompileModule: Compiling vs shader 0xd41f6d2b
[Tty] logger.cpp:62 log_flush: [stdout] Garlic AllocatedMemorySize: frame 00000003, 671.8M, 23.9% used
[Tty] logger.cpp:62 log_flush: [stdout] Peak volatile Garlic: 60 KBytes
[Tty] logger.cpp:62 log_flush: [stdout] Garlic AllocatedMemorySize: frame 00000004, 710.3M, 25.2% used
[Tty] logger.cpp:62 log_flush: [stdout] Peak DCB: 26.53 KBytes
[Tty] logger.cpp:62 log_flush: [stdout] Peak CCB: 3.62 KBytes
[Lib.VideoOut] <Error> driver.cpp:224 SubmitFlip: Flip queue is full
[Lib.VideoOut] <Error> video_out.cpp:354 operator(): EOP flip submission failed for buffer 2
[Lib.VideoOut] <Error> driver.cpp:224 SubmitFlip: Flip queue is full
[Lib.VideoOut] <Error> video_out.cpp:354 operator(): EOP flip submission failed for buffer 0
[Lib.VideoOut] <Error> driver.cpp:224 SubmitFlip: Flip queue is full
[Lib.VideoOut] <Error> video_out.cpp:354 operator(): EOP flip submission failed for buffer 1
[Tty] logger.cpp:62 log_flush: [stdout] Garlic AllocatedMemorySize: frame 00000011, 726.3M, 25.8% used
[Lib.VideoOut] <Error> driver.cpp:224 SubmitFlip: Flip queue is full
[Lib.VideoOut] <Error> video_out.cpp:354 operator(): EOP flip submission failed for buffer 2
[Lib.VideoOut] <Error> driver.cpp:224 SubmitFlip: Flip queue is full
[Lib.VideoOut] <Error> video_out.cpp:354 operator(): EOP flip submission failed for buffer 0
[Lib.VideoOut] <Error> driver.cpp:224 SubmitFlip: Flip queue is full
[Lib.VideoOut] <Error> video_out.cpp:354 operator(): EOP flip submission failed for buffer 1
[Tty] logger.cpp:62 log_flush: [stdout] exec at score engine
[Lib.VideoOut] <Error> driver.cpp:224 SubmitFlip: Flip queue is full
[Lib.VideoOut] <Error> video_out.cpp:354 operator(): EOP flip submission failed for buffer 2
[Lib.VideoOut] <Error> driver.cpp:224 SubmitFlip: Flip queue is full
[Lib.VideoOut] <Error> video_out.cpp:354 operator(): EOP flip submission failed for buffer 0
[Lib.VideoOut] <Error> driver.cpp:224 SubmitFlip: Flip queue is full
[Lib.VideoOut] <Error> video_out.cpp:354 operator(): EOP flip submission failed for buffer 1

These lines didn't exist before. My game stopped working properly. It doesn't show any critical errors.

@diegolix29
Copy link
Contributor Author

does the game works on main but this pr break it?

@GHU7924
Copy link

GHU7924 commented May 26, 2025

does the game works on main but this pr break it?

I have YES, but let someone else write who has this game.

@kalaposfos13
Copy link
Member

P.T. crashes, TLG hangs, and Journey and Minecraft spam the same lines as in the comment above, but otherwise work

CUSA03745.log
CUSA00265.log
CUSA02172.log
CUSA01127.log

@diegolix29
Copy link
Contributor Author

@kalaposfos13 @GHU7924 can u try TLG with the new commit?

@GHU7924
Copy link

GHU7924 commented May 26, 2025

@diegolix29 I have no changes, also hangs. I also think that the log will be useless here and it will be better to check through Visual Studio. But I won't be able to check this.

@kalaposfos13
Copy link
Member

You just changed two s32-s to int-s, this will have exactly zero effect and if I'm not mistaken will even compile to the exact same executable as s32 is basically just a typedef for an int

@diegolix29
Copy link
Contributor Author

You just changed two s32-s to int-s, this will have exactly zero effect and if I'm not mistaken will even compile to the exact same executable as s32 is basically just a typedef for an int

yes that was the discrepancy i find on driver.cpp, i revert it as s32 as it should be, on both header and constructor, the logging i place it to fetch info from testing as i only have darksouls 3 that triggers that crash. Rn im doing some changes to both logging and the registeronce hoping it will help, after local testing ds3 i will push a commit.

@diegolix29
Copy link
Contributor Author

can u try again with the new commit ds3 still is safe with this change.

@GHU7924
Copy link

GHU7924 commented May 27, 2025

shadPS4 v0 9 1 WIP pr-2992-Flip-bug-T 0cea540 _ CUSA03745 - The Last Guardian™ 01 03 Вт, 27 мая 2025 г  18_56_55

CUSA03745.log

P.S. I'm not sure if there are any other problems associated with this PR, but at least now it is possible to get into the game.

@diegolix29
Copy link
Contributor Author

Just need to see if this dosent regress any other game, but it should be ready after no regressions are confirmed

@diegolix29
Copy link
Contributor Author

diegolix29 commented May 27, 2025

Forgot to add this PR fixes out of order flip IRQ
image(130)

@kalaposfos13
Copy link
Member

Now none of those four games spam the error anymore, and P.T. went back to only occasionally crashing with EOP flip submission failed, so whilst this PR did not improve that, it now doesn't regress it either.
CUSA01127.log

@diegolix29
Copy link
Contributor Author

Now none of those four games spam the error anymore, and P.T. went back to only occasionally crashing with EOP flip submission failed, so whilst this PR did not improve that, it now doesn't regress it either. CUSA01127.log

It should be fixed now, i changed the log error too cause now we wait for space for flip instead of rejecting the flip when queue is full, let me know if u still randomly hit that crash

@diegolix29 diegolix29 changed the title video_out: SubmitEopFlip using mutex and lockguard to prevent race conditions video_out: SubmitEopFlip adding mutex and lockguard also fixing reject of full flips by adding a wait May 27, 2025
@diegolix29
Copy link
Contributor Author

So if PT and TLG are ok and were the only regressed games this should be ok now 💪

@GHU7924
Copy link

GHU7924 commented May 28, 2025

[CUSA30992] Teenage Mutant Ninja Turtles: Shredder's Revenge version 1.00 works for me in Main.
With this PR she doesn't even get to the menu.

At the end of the log there is a spam error:
[Core] <Error> stubs.cpp:42 CommonStub: Stub: select (nid: T8fER+tIGgk) called, returning zero to 0x814159deb

In general, here are the logs, but I don’t know if they will help.

Main: CUSA30992.log (I skipped the opening cutscenes and ended up in the game's main menu)

PR: CUSA30992.log (Always black screen)

UPD: PR Old Build ddefcaf the game worked.

@diegolix29
Copy link
Contributor Author

@GHU7924 could u try it again? i added something that should help if what i think its happening is correct.

@GHU7924
Copy link

GHU7924 commented May 29, 2025

Now it works.

shadPS4 v0 9 1 WIP pr-2992-Flip-bug-T 8451563 _ CUSA30992 - Teenage Mutant Ninja Turtles_ Shredder's Revenge 01 00 Чт, 29 мая 2025 г  21_52_26

@diegolix29
Copy link
Contributor Author

Niceee thanks hoping i have no more games regressing with this changes. gonna do a rebase

@GHU7924
Copy link

GHU7924 commented May 29, 2025

[CUSA03745] The Last Guardian

2025-05-29.22-34-04.mp4

CUSA03745.log


[CUSA00076] The Order: 1886 (this game doesn't work yet but it has a line)

[Lib.VideoOut] <Warning> video_out.cpp:159 sceVideoOutSubmitFlip: flipmode = 3

I don't know if this information is necessary for this PR, but just in case I'm attaching the game log for study, if the information is useful, then good, if not, then I apologize.

CUSA00076.log

@diegolix29
Copy link
Contributor Author

this is not related to the pr but i can take a look at it no problem but will be for the weekend

@GHU7924
Copy link

GHU7924 commented May 29, 2025

There are no changes. But I think @kalaposfos13 should also recheck his games, maybe there will be some differences.

this is not related to the pr but i can take a look at it no problem but will be for the weekend

There is no need for this, I only reported it because I thought it might be related, but I was wrong.
You still have other open pull requests, so you shouldn't waste your time on this.

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.

3 participants