applyGsDispEnv programs display read circuit 2 only - #2
Closed
smmathews wants to merge 3 commits into
Closed
Conversation
Real libgraph sceGsPutDispEnv programs PMODE, SMODE2, DISPFB2, DISPLAY2 and BGCOLOR only; the single-circuit env struct describes read circuit 2. The HLE also wrote DISPFB1/DISPLAY1 from that env, forcing both read circuits onto the same surface and collapsing any two-buffer blend (e.g. movie underlay + UI overlay). Stop clobbering circuit 1 so a game's own GS privileged MMIO writes to DISPFB1/DISPLAY1 survive. Migrates the existing sceGsSwapDBuffDc display-page test from circuit 1 to circuit 2 (it was asserting the pre-fix behavior) and adds a byte-level regression test proving circuit 1 is left untouched.
The double-buffer helpers (sceGsSetDefDBuffDc, sceGsSetDefDBuff) seeded PMODE with both read circuits enabled (EN1+EN2), but their swap path programs read circuit 2 only via applyGsDispEnv (DISPFB2/DISPLAY2). Since the circuit-2-only fix, circuit 1 is never programmed on this path and stays frozen at reset defaults, so the compositor blended that stale surface over the correct circuit-2 page for any title using the stock double-buffer library without driving circuit 1 itself. Seed EN2 only; circuit 1 stays guest-owned. Add a regression test on the sceGsSwapDBuffDc path asserting PMODE does not enable EN1 and that circuit-1 DISPFB1/DISPLAY1 are left untouched.
Add a sibling regression test driving sceGsSetDefDBuff + sceGsSwapDBuff (the non-Dc double-buffer path) with the same assertions as the existing Dc test: seeded PMODE has EN1==0 and EN2==1, and pre-seeded circuit-1 DISPFB1/DISPLAY1 sentinels survive the swap. Previously only the Dc variant was exercised, leaving the byte-identical non-Dc seed uncovered.
Owner
Author
|
Superseded by the upstream draft PR against ran-j/PS2Recomp: ran-j#152 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
applyGsDispEnv— the shared helper behind thesceGsPutDispEnvHLE and thesceGsSwapDBuff/sceGsSwapDBuffDcdouble-buffer paths — translated a singlesceGsDispEnvstruct into GS privileged register writes and programmed bothdisplay read circuits:
dispfb1/display1anddispfb2/display2, fromthe one-circuit env. Real libgraph
sceGsPutDispEnvprograms onlyPMODE,SMODE2,DISPFB2,DISPLAY2andBGCOLOR; the env describes read circuit 2,and circuit 1 is left for the game to drive itself via GS privileged MMIO.
Titles that composite two different buffers (for example a movie/scene underlay
beneath a UI overlay,
PMODE=0x8007) programDISPFB1/DISPLAY1themselves.The HLE's circuit-1 clobber forced both read circuits onto the same surface,
collapsing the blend.
Fix
ps2xRuntime/src/lib/Kernel/Stubs/Helpers/Support.h—applyGsDispEnv: deletedthe two circuit-1 assignments (
regs.dispfb1 = env.dispfb; regs.display1 = env.display;)and replaced them with an explanatory comment.
PMODE,SMODE2,DISPFB2,DISPLAY2,BGCOLORare unchanged. The guest's own MMIO writes toDISPFB1/DISPLAY1now survive.Collateral: double-buffer PMODE seeds
Making
applyGsDispEnvcircuit-2-only exposed the double-buffer seed helpers.sceGsSetDefDBuffDcandsceGsSetDefDBuff(
ps2xRuntime/src/lib/Kernel/Stubs/GS.cpp) seededPMODE = makePmode(1,1,...) = 0x8007(both read circuits enabled), but the only thing that programs a displaycircuit on the swap path is
applyGsDispEnv(db.disp[which]), which post-fixwrites
DISPFB2/DISPLAY2only. So the double-buffer path drives circuit 2exclusively; the buffer flip only ever moved circuit 2.
Before the fix the shared helper wrote both circuits, so each swap dragged
circuit 1 along in lockstep (equal to circuit 2) — the only reason
EN1everlooked valid. Post-fix, circuit 1 would freeze at the runtime's reset defaults
(
dispfb1fbw=10,display1639×447) while circuit 2 flipped, so the compositor(
ps2_gs_gpu.cpp) would blend that stale circuit-1 surface (fbp 0) over thecorrect circuit-2 page for any title using the stock double-buffer library
without driving circuit 1 itself.
Remedy: seed the double-buffer
PMODEwith read circuit 2 only —makePmode(1u, 1u, ...)→makePmode(0u, 1u, ...)at both sites, i.e.0x8007→0x8006. A seed that enables a circuit the path never programs iswrong;
EN2-only matches real-hardware semantics, because plain libgraphdouble-buffering is single-circuit (one visible circuit alternating
framebuffers).
GsSetCrt/sceGsResetGraph, which enableEN1, also programthe circuit-1 registers consistently and are a different, correct code path —
left untouched.
Known residual
The runtime's
ps2_memory.cppreset defaults fordispfb1/display1arenon-zero (fbw=10 / 639×447) rather than the null display region a real GS reset
leaves. This is why circuit 1 could pass the compositor's
validCrt1/hasDisplaySetupcheck even though nothing programmed it. A game that manuallyenables
EN1without ever programming circuit 1 would therefore show garbagehere versus nothing on real hardware. The collateral remedy above fully closes
the issue for every stock-library title via the HLE swap paths; this residual is
latent and unreachable through the HLE paths, so zeroing the boot defaults — a
broader change with early-boot-presentation / full-suite blast radius — is left
as a known residual rather than folded into this change.
Tests (
ps2xTest/src/ps2_gs_tests.cpp)Three tests cover the behavior, each asserting
EN1==0/EN2==1andcircuit-1 sentinel preservation:
sceGsPutDispEnvcircuit-2-only: pre-seedsdispfb1/display1sentinels,invokes the HLE with a known env, asserts circuit-2 + shared registers carry
env values while the circuit-1 sentinels are untouched.
sceGsSwapDBuffDcswap path: aftersceGsSetDefDBuffDc+sceGsSwapDBuffDc,asserts the seeded
PMODEdoes not enable read circuit 1 (EN1==0) and doesenable read circuit 2 (
EN2==1), and that pre-seeded circuit-1DISPFB1/DISPLAY1sentinels survive the swap.sceGsSwapDBuffswap path: the non-Dc twin, drivingsceGsSetDefDBuff+sceGsSwapDBuffwith identical assertions so the non-Dc seed is directlyexercised rather than relying on correct-by-symmetry.
The migrated
sceGsSetDefDBuffDctest had its post-swap assertions correctedfrom
dispfb1/display1todispfb2/display2.Verification
ps2x_tests: 299 tests, 0 failed.makePmodeseeds back to(1,1)/0x8007atboth sites fails exactly the two swap-path
EN1==0assertions — one from theDc test, one from the non-Dc test (297/299); restoring returns to 299/299.
-DPS2X_BUILD_STUDIO=OFFand-march=native -include cstdintinCMAKE_CXX_FLAGS(host-local only, not committed).🤖 Generated with Claude Code