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

Fix gamecube games not noticing disc changes #8571

Open
wants to merge 4 commits into
base: master
from

Conversation

@Pokechu22
Copy link
Contributor

Pokechu22 commented Jan 20, 2020

Fixes issue 9019.

The problems:

  • Changing a disc reset the drive state completely (which was something I added in 55a88ba). This included the DI registers, meaning interrupts would no longer be sent (so the drive errors that were supposed to be sent in that case just weren't making it anywhere). Now it only resets "internal" properties. I don't know if this is 100% accurate behavior (and I only changed it for disc changes; other places that reset it were not changed)

  • Although that allowed games to detect disc errors if they happened on e.g. request stream status, they still didn't care about errors on the read command (which are far more likely). This happened to be because games look at DILENGTH, and if it's 0 they assume the read was successful even if DEINT was fired instead of TCINT. I touched related code in 3110599 but didn't actually think about the error case. Note that YAGCD mentions that DILENGTH can indicate the amount of data left over for transfer, but it isn't explicit about that being used in errors.

    One possible change here is that it might be possible for commands (especially reads) to fail partway through, in which case DILENGTH would maybe be half-updated instead of the current all-or-nothing situation. But all-or-nothing seems sufficient for most games.

  • There were some other inaccuracies with drive state behavior, based on attempts at reverse-engineering the drive firmware. There are some old half-archived notes here, which was enough for me to get started (but it's very hard to understand). These changes required a savestate version bump.

    One thing this fixes (when combined with the other changes) is the AudioStream command, which gave the wrong error code in some states, which games wouldn't know what to do with and would then crash.

    A note: at least with Alien Hominid, if a game using DTK is ejected while it is streaming, upon reinsertion it'll start the stream from the beginning again. I tested and confirmed this behavior on console (specifically my Wii).

@Pokechu22 Pokechu22 changed the title Fix games not noticing disc changes Fix gamecube games not noticing disc changes Jan 20, 2020
Source/Core/Core/HW/DVD/DVDInterface.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/DVD/DVDInterface.cpp Outdated Show resolved Hide resolved
@Pokechu22 Pokechu22 force-pushed the Pokechu22:di-interrupts branch from 2b7ef4e to 275b3e8 Jan 20, 2020
Pokechu22 added 4 commits Jan 19, 2020
…acies

In particular:
- Trying to play audio in a non-ready state returns the state-specific error, not an audio buf error
- Audio status cannot be requested in non-ready states
- The audio buffer cannot be configured in states other than ReadyNoReadsMade
- Using the stop motor command while the motor is already stopped doesn't change states

Additionally, the internal state IDs are used (which distinguish ReadyNoReadsMade and Ready), instead of the state IDs exposed in request error.  This makes some of the weird behavior a bit more obvious.

State and error behavior of the seek command was not implemented in this commit.
Turns out, Gamecube games actually do check DILENGTH, and if DILENGTH is at 0, they'll think the transfer completed successfully even if DEINT is used, since after all, surely that means everything was sent.  That caused all sorts of issues, from audio looping when a disc is removed since it's re-using the same buffer to just flat-out crashing instead of showing the disc removed screen.
Resetting the DI registers disables interrupts, which means any errors reported (for instance) are just not sent though.
@Pokechu22 Pokechu22 force-pushed the Pokechu22:di-interrupts branch from 275b3e8 to f67c7b5 Jan 21, 2020
@leoetlino leoetlino requested a review from JosJuice Feb 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.