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

DSPHLE: Add variables in CMailHandler to savestate. #10784

Merged
merged 1 commit into from Jun 25, 2022

Conversation

AdmiralCurtiss
Copy link
Contributor

Should fix #10761 (comment) but I haven't actually verified it. They should definitely be stored in the state either way though.

Copy link
Contributor

@Pokechu22 Pokechu22 left a comment

Choose a reason for hiding this comment

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

Not tested yet, but this seems correct.

I think I originally had these, but then when I converted m_pending_mails into a std::deque and switched to p.Do(m_pending_mails);, I lost them from a merge conflict. Or maybe I just never had them; I have been forgetting about savestates lately (#10768 originally was missing them, but it has them now).

The hang was probably related to #10732, as there's an initialization step that lasts for 130ish timebase units (I'm not sure how long that is in seconds, but it's pretty short), during which the DSP is halted, and actually it's halted for longer than that (10230 timebase units) while it does some DMAs. If a savestate is loaded during that period, m_halted would be true when it should be false, and things would be hung.

@JMC47
Copy link
Contributor

JMC47 commented Jun 25, 2022

Seems to work.

@JMC47 JMC47 merged commit b30e1c5 into dolphin-emu:master Jun 25, 2022
11 checks passed
@AdmiralCurtiss AdmiralCurtiss deleted the mail-handler-state branch June 25, 2022 18:57
@Pokechu22
Copy link
Contributor

Can confirm.

To reproduce the issue, first boot Super Mario Sunshine normally and create a save state on the title screen.

Then, check Options → Boot to Pause (you may need the debugging UI enabled), and launch Super Mario Sunshine again. Advance one frame (for me, this is bound to N, but you may need to bind it in Options → Hotkey Settings), and then load the state. The game will be hung.

With this PR, that no longer happens.

Also, I was wrong about the time period. If you look at my test case, you can see that when __dsp_bootstrap returns, the DSP is halted. If you look at Super Mario Sunshine's log, after the first frame advance the following is logged:

37:49:841 Core\HW\DSPHLE\UCodes\UCodes.cpp:229 I[DSPHLE]: Switching to ROM ucode
37:49:841 Core\HW\DSPHLE\UCodes\ROM.cpp:28 I[DSPHLE]: UCode_Rom - initialized
37:49:846 Core\HW\DSPHLE\UCodes\UCodes.cpp:233 I[DSPHLE]: Switching to INIT ucode
37:49:846 Core\HW\DSPHLE\UCodes\INIT.cpp:16 I[DSPHLE]: INITUCode - initialized
37:49:847 Core\HW\DSPHLE\DSPHLE.cpp:198 I[DSPHLE]: DSP_CONTROL halt bit changed: 0004 -> 0000
37:49:849 Core\HW\DSPHLE\DSPHLE.cpp:198 I[DSPHLE]: DSP_CONTROL halt bit changed: 0000 -> 0004
37:49:849 Core\HW\DSPHLE\UCodes\UCodes.cpp:229 I[DSPHLE]: Switching to ROM ucode
37:49:850 Core\HW\DSPHLE\UCodes\ROM.cpp:28 I[DSPHLE]: UCode_Rom - initialized

Then, after 6 more frame advances (1/10th of a second, which is definitely possible for a human to accidentally run into when loading a state), this is logged:

38:09:188 Core\HW\DSPHLE\DSPHLE.cpp:198 I[DSPHLE]: DSP_CONTROL halt bit changed: 0954 -> 0950
38:09:197 Core\HW\DSPHLE\UCodes\ROM.cpp:107 I[DSPHLE]: CurrentUCode SOURCE Addr: 0x003e47a0
38:09:197 Core\HW\DSPHLE\UCodes\ROM.cpp:108 I[DSPHLE]: CurrentUCode Length:      0x00001ce0
38:09:197 Core\HW\DSPHLE\UCodes\ROM.cpp:109 I[DSPHLE]: CurrentUCode DEST Addr:   0x00000000
38:09:197 Core\HW\DSPHLE\UCodes\ROM.cpp:110 I[DSPHLE]: CurrentUCode DMEM Length: 0x00000000
38:09:197 Core\HW\DSPHLE\UCodes\ROM.cpp:111 I[DSPHLE]: CurrentUCode init_vector: 0x00000000
38:09:197 Core\HW\DSPHLE\UCodes\ROM.cpp:112 I[DSPHLE]: CurrentUCode CRC:         0x56d36052
38:09:197 Core\HW\DSPHLE\UCodes\ROM.cpp:113 I[DSPHLE]: BootTask - done
38:09:197 Core\HW\DSPHLE\UCodes\Zelda.cpp:128 I[DSPHLE]: Zelda UCode loaded, crc=56d36052, flags=000000c0
38:09:206 Core\HW\DSP.cpp:392 I[AI]: Audio DMA configured: 70 blocks from 0x005f4d80

__OSInitAudioSystem is responsible for the DSP initialization process (and matches __dsp_bootstrap), and that's called by OSInit which is one of the first things that happens. But that leaves the DSP halted (and thus m_halted set to true) until the game calls DSPInit, which in Super Mario Sunshine's case happens several frames later further into the game's startup process (and on a separate thread, I think?).

@Adamillo
Copy link

Adamillo commented Jun 25, 2022

Huh, I couldn't seem to reproduve this on Zelda Ucode games. But nice to know it happens on Zelda Ucode as well. EDIT: Woops I didn't realize that while testing Mario Kart Double Dash was a Zelda Ucode title

dvessel pushed a commit to dvessel/dolphin that referenced this pull request Jun 28, 2022
…-state

DSPHLE: Add variables in CMailHandler to savestate.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants