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

DSP: Add address mask for physical pointers to audio data buffers #4483

Merged
merged 2 commits into from Dec 4, 2018

Conversation

Projects
None yet
7 participants
@RoadrunnerWMC
Copy link
Contributor

RoadrunnerWMC commented Dec 3, 2018

Fixes #3313 and #3393, and probably also #3378 and #3392.

The physical audio buffer addresses that Luigi's Mansion Dark Moon gives to the DSP have the least-significant bit set for no obvious reason. Citra took it at its word, resulting in garbled audio. Masking the addresses with 0xFFFFFFFE is enough to fix the issue, but hardware testing (thanks to @Dragios and @jroweboy) indicated that 0xFFFFFFFC is actually the correct mask for all audio formats (mono and stereo PCM8, mono and stereo PCM16, and ADPCM). This fixes the audio on Dark Moon, The Amazing Spiderman, Metroid Prime Federation Force, and probably also some other games that haven't been re-tested yet (see the issues I linked above).

A few extra notes:

  • Luigi's Mansion Dark Moon, Metroid Prime Federation Force, The Amazing Spiderman and Disney Princess - My Fairytale Adventure all use the Wwise audio engine, so it's a good guess that this fix also affects any other games using that engine. Monster 4x4 3D was too obscure for me to determine just by Internet sleuthing if it uses Wwise or not.
  • Setting the bottom bit of the address didn't seem to cause any noticeable difference during the hardware tests.
  • I also hardware-tested whether the most significant address bit is masked out or not, and it seems to not be (unlike on several other Nintendo consoles). I didn't check any other high address bits, but it's a pretty safe assumption that those are also not masked out.

This change is Reviewable

DSP: Add address mask for physical pointers to audio data buffers
Hardware testing indicated that FFFFFFFC is the correct mask for all audio formats (mono and stereo PCM8, mono and stereo PCM16, and ADPCM). This fixes broken audio in Luigi's Mansion: Dark Moon and a few other games.
@B3n30

This comment has been minimized.

Copy link
Contributor

B3n30 commented Dec 3, 2018

#3378 also mentions som GFX issues. They are most likely not fixed by that PR, right?

@B3n30

B3n30 approved these changes Dec 3, 2018

Copy link
Contributor

B3n30 left a comment

Not much to review, so LGTM,

but maybe add a comment about address spaces and why that mask is necessary

@RoadrunnerWMC

This comment has been minimized.

Copy link
Contributor Author

RoadrunnerWMC commented Dec 3, 2018

I can't imagine any way this PR could affect graphics at all.

I'm not really sure what such a comment would say, other than just directly stating what the code does and what games it affects (e.g. The two least significant bits of this physical address are ignored; this fixes Luigi's Mansion: Dark Moon and some other games using the Wwise audio engine. This has been hardware-tested.). Doesn't seem like a very useful comment to me, but maybe you had a different idea about what it would say.

@wwylele wwylele added the canary-merge label Dec 3, 2018

@zhaowenlan1779
Copy link
Member

zhaowenlan1779 left a comment

LGTM
I still think it would be better if you leave a comment though, as people likely won't know about this magic number when they read the code

@adityaruplaha
Copy link
Contributor

adityaruplaha left a comment

LGTM. Nothing to say apart from what others have already said.

@wwylele

This comment has been minimized.

Copy link
Member

wwylele commented Dec 3, 2018

Here is some insights of how this works:

The DSP firmware doesn't do this masking in code; instead it directly hands this address over the DMA engine. However, the DSP firmware preconfigured the DMA engine to 32-bit mode + x8 bust size, which will effectively mask the address in the same way. I just happened to hwtest this part when implementing it in DSP LLE.

As for comment in the code. I think mentioning that this is DSP DMA hardware behaviour is enough.

DSP: Add a comment about physical address masking
See @wwylele's comment on PR 4483 for more details on what causes this behavior.

@wwylele wwylele merged commit 1d597db into citra-emu:master Dec 4, 2018

3 checks passed

ci/bitrise/4ccd8e5720f0d13b/pr Passed - citra
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Luffykun007

This comment has been minimized.

Copy link

Luffykun007 commented Dec 4, 2018

This PR also fixes audio on Batman Arkham Origins Blackgate that had the same problem as Luigi's mansion Dark Moon,game is perfect now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.