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

Ignore PI_RESET_CODE resetting DI in Wii mode #9060

Merged
merged 1 commit into from Sep 8, 2020

Conversation

Pokechu22
Copy link
Contributor

Fixes bug 12241. I did a quick hardware test, and it doesn't seem like writes to it actually do anything in Wii mode; IOS reads still return normally and the drive didn't make any noises that sound like it was reset (unlike when resetting it through IOS). I'm guessing Project M's write to it is just a leftover.

I did notice a different oddity, though: even though I always wrote with bit 1 set, reading back never had bit 1 set. Also, bit 2 was set by the time HBC loaded my code. That should be investigated further (along with behavior in GameCube compatibility mode, and behavior on actual GameCubes). So, there's probably further behavior that could be emulated, but this still seems to be accurate.

I've tested and confirmed that this does not interfere with actually changing discs for GameCube games; they still properly detect the disc change and spin it up again. This includes when launched from the Wii menu (since bWii is set to false by the current MIOS implementation).

@JosJuice
Copy link
Member

JosJuice commented Sep 5, 2020

Is the real console's behavior affected by whether AHBPROT is enabled?

@Ebola16
Copy link
Member

Ebola16 commented Sep 5, 2020

This does appear to fix the problem. I don't know much about this part of Dolphin's code but I appreciate the explanations for what's going on here. Also, if any of the "oddities" remain when this PR is ready to merge, can they be added as TODOs so we don't forget about them?

@Pokechu22
Copy link
Contributor Author

Pokechu22 commented Sep 5, 2020

I'm pretty sure I had AHBPROT enabled when I tested this (the same code I used to test it was used to manually write to DI registers (though I didn't do that in this test), which to my understanding requires AHBPROT); I ran it using wiiload which to my understanding also enables AHBPROT (since it doesn't have an XML file to specify whether or not to enable it).

@JosJuice
Copy link
Member

JosJuice commented Sep 5, 2020

AHBPROT enabled means that the protections are enabled (i.e. you can't write to the DI registers). Not providing an XML file when launching from an SD card means that AHBPROT will be enabled, but using Wiiload means that AHBPROT will be disabled. So I suppose you actually had AHBPROT disabled when testing? If so, this change seems fine to me.

@Pokechu22
Copy link
Contributor Author

Right, sorry; I had AHBPROT in the state where I could write to stuff. I'll do a tests with AHBPROT explicitly enabled and disabled, though; Project M doesn't have AHBPROT specified in its XML. (Though it seems unlikely to me that it'll only do things in the more restrictive mode...)

@Pokechu22
Copy link
Contributor Author

Results are the same with <ahb_access/> set in the XML file (HW_AHBPROT reads back as 0xffffffff) and without it set (HW_AHBPROT reads back as 0, but probably there are bits set in it, just not the one needed to access HW_AHBPROT itself).

In GameCube mode, the drive does reset. Trying to do a read while bit 2 is unset resulted in the read never completing (and the eject button not working, oddly). I didn't check whether the disc remains spinning in that state (since I launched it with SD Media Launcher, and that stops the motor beforehand; to check that I'd need to properly reset it once and then leave bit 2 unset the second time). I did confirm that clearing bit 2 and then setting it spun the drive up again, exiting the motor stopped state.

@JMC47
Copy link
Contributor

JMC47 commented Sep 8, 2020

Is this good to go?

@Pokechu22
Copy link
Contributor Author

There isn't any additional testing or changes I plan on doing, so I'd say yes.

@JMC47 JMC47 merged commit 3d33b1c into dolphin-emu:master Sep 8, 2020
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