Repository navigation
Conversation
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
One correctness concern in the current diff: eopInterruptDelivered is initialized once before the packet loop and set to true after every FlipPacketHeader, but it is never cleared. Submission.cpp records each flip packet in submission.flips, and execute processes each one in order, so a second ordinary flip in the same DCB will skip its synthetic AgcDriverDeliverEopInterrupt even if no RELEASE_MEM EOP was associated with that second flip. The pre-change path delivered one graphics EOP for each flip.
Could the suppression state be consumed/reset for the matching flip, with a regression that submits two bare flips in one DCB and checks for two EOP notifications? I have not run this PR; this is based on the packet loop and submission parser.
|
Thanks for catching this @MarcZX100. I've updated the PR (commit 9c9d801):
|
|
I ran Death Stranding 2 (PPSA02015), which the description names for #6462, with and without both commits of this PR, on the local stack I used in #6462 ( Logging the flip callers shows why: the game submits its flips with its own The PR's tests pass on |
|
Thank you for the thorough testing and trace details @fma-rc! I've updated the PR description to remove Death Stranding 2 (#6462) from \Unblocks\ and kept only Gran Turismo 7 (#6350). Since Death Stranding 2 submits flips via its own \sceAgcDcbSetFlip\ rather than \sceVideoOutSubmitFlip, resolving #6462 is a separate investigation regarding how the console distinguishes per-flip vs release interrupts for AGC DCB flips. |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Thanks for the update. I checked head 9c9d801af8d4eb42b66aad566b62b474e35c8072: the suppression state is now reset after the flip handling, and testEndOfPipeInterrupts includes two bare flips in one DCB and expects two interrupts. That resolves the concern in my earlier comment.
|
Thanks for correcting the |
|
Real-window VideoOut flip results, plus an overlap with #6616 worth noting. Windows 11 10.0.26200.9711, RTX 5070 Ti (NVIDIA 617.42), WinLibs GCC 15.2.0 r7, Release. Head 9c9d801 merged onto main f3263df (clean merge, no new build errors). All 20
The EOP cases pass because this PR's line 176 change in Not checked: GT7 or any title, and the per-flip AI-assisted: yes |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed the current head and my earlier multi-flip concern. The suppression state is reset after each flip, and the added regression submits two bare flips in one DCB and expects two notifications. The VideoOut-specific packet header is also handled in the parser, validation, reservation, and execution paths I inspected. Hosted eligibility, conventions, and report checks pass; platform matrix jobs were skipped by path filters. I reviewed the diff and reported test evidence but did not run the GPU/title tests locally.
9c9d801 to
c48a0f1
Compare
|
Rebased onto latest origin/main (415cd49):
|
|
Closing as superseded by merged work; you're welcome to reopen with the points below addressed.
If you still want the VideoOut split, please open it as its own PR on current |
What
This PR resets the flip end-of-pipe interrupt delivery state (
eopInterruptDelivered = false) after each flip packet in AGC packet execution. When multiple AGC flips (FlipPacketHeader) are submitted in a single packet stream or across consecutive submissions without an intervening release packet interrupt, each flip now properly delivers its corresponding queue end-of-pipe event (sceAgcDriverAddEqEvent) rather than suppressing all flips after the first one. Also updates debt documentation.Tested
SubmitTests:testEndOfPipeInterrupts: verified multi-flip sequence delivers distinct end-of-pipe events for each flip.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)r14+0x48) while the title's video-out/flip frame-pacing loop runs #6350)