Closed nyanpasu64 closed 1 year ago
ramapcsx2
commented
1 year ago Ehr wait, I don't remember what registers it affected, but if it's working on "ADC_GGCTRL", that should totally work in bypass modes, too. Anything on the ADC directly will affect the image, as the source is always sampled through the ADC. What may not work is some related thing though, I don't quite remember what was required.
nyanpasu64
commented
1 year ago Ehr wait, I don't remember what registers it affected, but if it's working on "ADC_GGCTRL", that should totally work in bypass modes, too. Anything on the ADC directly will affect the image, as the source is always sampled through the ADC.
I was under the impression it didn't work, since enabling auto gain in fixed/custom passthrough mode didn't set the brightness to "deliberately clipping", and usually (not always) did not function properly due to DEC_TEST_ENABLE=0. Adjusting manual gain is a slow tedious process, but does appear to change image brightness.
Auto gain in bypass mode is useless because writing to ADC_GGCTRL has no effect in bypass mode. Additionally it's bugged because runAutoGain() assumes DEC_TEST_ENABLE=1, but during bypass mode with auto gain enabled, it's sometimes but not always set to 0:
Switching from a scaling preset to passthrough leaves DEC_TEST_ENABLE set to 1. Cycling auto gain off/on afterwards sets it to 0.
Enabling auto gain in passthrough mode runs runAutoGain(), but doesn't set DEC_TEST_ENABLE=1 (instead preserving the previous stale value).
Disabling auto gain in passthrough mode sets DEC_TEST_ENABLE=0 as usual.
Switching to a scaling output or custom preset (both scaling and passthrough) properly sets DEC_TEST_ENABLE=1 if auto gain is enabled (even if it was enabled on passthrough, leaving DEC_TEST_ENABLE=0), or to 0 otherwise (unobservable because it's already 0).
I'm not sure what to write to DEC_TEST_ENABLE in bypass mode. As a minimum viable fix, in this commit we avoid observing the wrong value by not calling runAutoGain() in bypass mode.
Testing
Background
While practicing Super Monkey Ball 2 in Nintendont with 480p forced in Nintendont and enabled in-game, and deflicker disabled in Nintendont, I got around 1 second of black screen mid-gameplay. The GBS-C was loaded in a custom preset in passthrough mode. My web browser (running but my computer was locked) showed only unintended "auto gain during bypass mode" messages, but it's possible the GBS-C lost Wi-Fi connection due to passthrough mode and did not print messages to my browser. I did not have a serial console open at the time.
I may have been running #446, or #429, or another debug branch with local changes.
I do not know why the black screen appeared, whether it was my Wii's software/hardware, the GBS-C, an overheating TV5725, my monitor, or a static discharge. It may be related to auto gain in passthrough, or not. But this is a long-standing bug worth fixing.
Future improvements
In a well-designed program, the value of DEC_TEST_ENABLE (along with all other registers) should be consistent, determined by a memoryless function of current "source state" alone, rather than by modifying past state but allowing part of it to survive unchanged.
There are various strategies, like writing every register unconditionally, or writing all registers which could change given the diff from the previous state. I think GBS-C attempts the latter strategy, but misses many registers due to a lack of rigor in ensuring correctness, and no "correct by design" systematic structured change tracking (the StateTransaction pattern).