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: Eliminate global state in GBA and AX uCode + accuracy improvements #10768

Merged
merged 4 commits into from Aug 4, 2022

Conversation

Pokechu22
Copy link
Contributor

The accuracy improvements are:

  • The request mail must be 0xabba0000 exactly; both the low and high parts are checked
  • The address is masked with 0x0fffffff
  • Before, the global state meant that after the GBA uCode had been used once, it would accept 0xcdd1 commands immediately. Now, it only accepts them after execution has finished.

This is somewhat based on my WIP HLE version of the Card uCode, which the GBA uCode has some similarities with. I briefly checked the accuracy changes above, but I haven't done a thorough read-through of the GBA uCode myself.

This PR also bundles #10753.

Copy link
Contributor

@iwubcode iwubcode left a comment

Choose a reason for hiding this comment

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

LGTM. Untested

@Pokechu22
Copy link
Contributor Author

Ah, I forgot to mention that I did test this using the GameCube Preview Disc.

@delroth
Copy link
Member

delroth commented Jun 22, 2022

LGTM, fine to merge once the merge conflict is resolved.

@AdmiralCurtiss
Copy link
Contributor

Note that #10753 (comment) should be resolved before this is merged.

@Pokechu22 Pokechu22 marked this pull request as draft June 22, 2022 16:58
@Pokechu22
Copy link
Contributor Author

Gah, I just noticed that the AX uCode has similar static state - I tried checking for this, but I must have missed it.

void AXUCode::HandleMail(u32 mail)
{
// Indicates if the next message is a command list address.
static bool next_is_cmdlist = false;
static u16 cmdlist_size = 0;
bool set_next_is_cmdlist = false;

I'm going to fix that in this PR as well to avoid bumping the savestate version twice.

@Pokechu22 Pokechu22 changed the title DSPHLE: Eliminate global state in GBA uCode + accuracy improvements DSPHLE: Eliminate global state in GBA and AX uCode + accuracy improvements Jun 23, 2022
@Pokechu22 Pokechu22 marked this pull request as ready for review June 23, 2022 17:57
@Pokechu22 Pokechu22 force-pushed the dsp-hle-gba-class branch 3 times, most recently from 9259ac8 to c7872b7 Compare June 26, 2022 02:55
@AdmiralCurtiss
Copy link
Contributor

What's the status here, I assume this is done? I kinda lost track of it after the stuff in #10753.

@Pokechu22
Copy link
Contributor Author

Yep, this is done and ready for review.

@Pokechu22 Pokechu22 force-pushed the dsp-hle-gba-class branch 3 times, most recently from 086531a to 91b95ae Compare July 26, 2022 20:16
@Pokechu22 Pokechu22 force-pushed the dsp-hle-gba-class branch 2 times, most recently from 7916d01 to 212970e Compare August 3, 2022 22:13
Copy link
Member

@lioncash lioncash left a comment

Choose a reason for hiding this comment

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

Very cool to have the global state done away with. Just one minor nit, but this looks completely fine otherwise

Source/Core/Core/HW/DSPHLE/UCodes/AX.h Outdated Show resolved Hide resolved
(Apart from AXUCode, which is inherited by AXWiiUCode.)
The accuracy improvements are:

* The request mail must be 0xabba0000 exactly; both the low and high parts are checked
* The address is masked with 0x0fffffff
* Before, the global state meant that after the GBA uCode had been used once, it would accept 0xcdd1 commands immediately. Now, it only accepts them after execution has finished.
This also increases accuracy as to when specific mail is allowed, and correctly handles masking of the 0xCDD1 mails.
CARDUCode, GBAUCode, and INITUCode previously didn't have an implementation of it. In practice it's unlikely that this caused an issue, since these uCodes are only active for a few frames at most, but now that GBAUCode doesn't have global state, we can implement it there. I also implemented it for CARDUCode, although our CARDUCode implementation does not have all states handled yet - this is simply future-proofing so that when the card uCode is properly implemented, the save state version does not need to be bumped. INITUCode does not have any state to save, though.
@Tilka Tilka merged commit 3ad6e3a into dolphin-emu:master Aug 4, 2022
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
7 participants