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

Increase accuracy of DSP initialization process #10732

Merged
merged 4 commits into from Jun 22, 2022

Conversation

Pokechu22
Copy link
Contributor

@Pokechu22 Pokechu22 commented Jun 9, 2022

This PR makes the handling of the control register more accurate, so that clearing the init bit (0x800) causes the init code bit (0x400) to be set for ~130 timebase ticks, as well as copying the init uCode from main memory address 0x81000000 into IRAM when using DSP LLE (before, we just faked the result of this init uCode in a rather jank way). Other operations (in particular DMAs) complete instantly still, so we're still a bit fast, but the speed for this part of the initialization process is accurate now.

It also makes it so that the halt bit is properly respected by DSPHLE; no new mails are sent while the DSP is halted. This change fixes Datel titles when using DSP HLE (issue 12943). (This has been moved to #10761.) Datel's code actually wasn't broken due to insufficiently precise timings; rather, they had a typo: instead of clearing the init bit (0x800), they cleared the 0x80 bit (which is related to an interrupt, and clearing it does nothing). Here's a comparison between the DSP initialization logic in Super Mario Sunshine and Datel titles.

Their code only worked because it was getting the DSP ROM's initialization mail (0x8071feed) instead of the initialization code's mail (0x80544348). But both their initialization function and the official one read mail at the start before continuing the initialization process - this happens immediately after the DSP is reset and while it is halted, so this shouldn't pick up on new mail, only mail sent before it was reset (I think? I'm not sure what other purpose it serves). On DSP HLE, we ignored that the DSP was halted, so that first loop picked up the ROM's mail, and then the second loop never received anything. Respecting the halt status on HLE fixes this.

Here's the code I used to hardware test this. The test itself is a bit jank, but it shows that the new logic is a bit closer in regards to timing, and completely accurate with regards to mail received and the way the control register changes.

Output images

There are still a lot of mysteries as to how this initialization works (and why it exists at all). For instance, it's not clear whether the code used for initialization comes from the start of ARAM or from address 0x81000000 (the initialization function makes two DMAs from what I assume is physical address 0x01000000, but they seem like they're too small and it's not clear why it does it twice, and if it just needs to be at the start of ARAM why would they bother copying to address 0x81000000?). Datel also actually has distinct DSP initialization uCode from the snippet used by both Super Mario Sunshine and Libogc (Datel's doesn't zero out DRAM), but I'm not sure if this is just something that was changed with later titles that Datel didn't pick up on or what happened here. I don't think it matters too much since I don't think anything uses this initialization process for anything other than initialization, but it's still something to investigate.

@RSDuck
Copy link

RSDuck commented Jun 10, 2022

here are my findings regarding the dsp init: https://github.com/RSDuck/hocuspocube/blob/master/hocuspocube/dsp/dsp.nim#L13

The ARAM stuff is unrelated, I managed to copy any code to IRAM without it. Looking back at some chat logs, Extrems told me it might be related to some hw bug, though idk. I did all my tests on a Wii.

@Pokechu22
Copy link
Contributor Author

Thanks, those names are good for comparison (I don't like Dolphin's DSPInit and DSPInitCode or libogc's DSPCR_DSPRESET, and bootRom/busyCopying are a bit clearer at least...). Everything you've written there matches what I'm seeing apart from "Wait for any value to be received in it's mailbox" (instead, it's waiting for the top bit of DMBH to be clear, which means the CPU has read the previous mail from the DSP). You've also got information that's slightly more accurate than what I wrote in the manual (I wrote "The 11th bit being set appears to cause data from 0x01000000 to be DMAd to the start of IMEM" but it should be "being cleared" based on my new testing and what you wrote.) The 1kb payload portion is consistent with @xperia64's research in #10662 (which I haven't investigated myself).

I can confirm that the ARAM DMA does not matter on the Wii like you said. Removing it has no change in results, but I also tested changing the source address to 0 (which contains other stuff) and the init process works fine in that case too. (I checked this because libogc does the same initialization process during its startup, so ARAM will already have the same contents from the DMA done by libogc, so using a different source address would overwrite it with unrelated data.) If it matters anywhere, it only matters on GameCube, but I don't have easy access to a GameCube for testing.

I also modified my previous hardware test (new results) and can confirm that the HALT instruction (which is executed at the end of the normal init ucode) doesn't set the halt bit (0x4), and that the bootRom bit (0x800) doesn't get set when the current PC is in ROM (which is what Dolphin was doing before). This PR fixes the latter thing, but doesn't change the behavior of HALT so it's not quite right on LLE yet.

@Pokechu22
Copy link
Contributor Author

I'm going to split this into two PRs, one covering the change in behavior to the halt bit and one covering the initialization changes, since only the halt one matters for Datel. Hopefully that will make this easier to review.

Before, there were two distinct fields called cr and r.cr, which is needlessly confusing (see the comment in DSPCore.h).
The # option means that 0x is prepended already, so the old code resulted in 0x0xDEADBEEF instead of the intended 0xDEADBEEF. WriteMailboxLow was already correct.
This improves timing, but does not fix Datel titles.
@dolphin-emu-bot
Copy link
Contributor

FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:

  • nddemo-bumpmapping on sw-lin-mesa: diff

automated-fifoci-reporter

@delroth delroth merged commit c8e7162 into dolphin-emu:master Jun 22, 2022
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants