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: latch dma parameters #692

Merged
merged 1 commit into from Aug 2, 2014
Merged

Conversation

booto
Copy link
Contributor

@booto booto commented Jul 29, 2014

This needs to be tested a fair bit before it could be considered a candidate for merging.
DSP wasn't latching DMA parameters appropriately and it could break the audio output.

I don't have an extensive title collection, but if people could test this PR (windows binary: https://dl.dolphin-emu.org/prs/pr-692-dolphin-latest-x64.7z) and report any regressions, that would be awesome.

@@ -402,7 +420,7 @@ void RegisterMMIO(MMIO::Mapping* mmio, u32 base)
// the read side.
mmio->Register(base | AUDIO_DMA_BLOCKS_LEFT,
MMIO::ComplexRead<u16>([](u32) {
return (g_audioDMA.BlocksLeft > 0 ? g_audioDMA.BlocksLeft - 1 : 0);
return (g_audioDMA.progress_Numblocks > 0 ? g_audioDMA.progress_Numblocks - 1 : 0);

This comment was marked as off-topic.

This comment was marked as off-topic.

@JMC47
Copy link
Contributor

JMC47 commented Aug 1, 2014

I tested this in quite a few games, and while the old patch that I was given caused some issues, this one hasn't.

The main thing this fixes is audio problems in Harvest Moon: Magical Melody and the Project M loader music.

This should be considered a high priority to get into master, as without frame limit by audio, there is no work around for those titles with issues. But maybe should get another tester or two to go through their games.(I didn't notice any issues in other games, but my track record with audio commits isn't that great.)

@@ -472,31 +486,31 @@ void UpdateDSPSlice(int cycles)
// This happens at 4 khz, since 32 bytes at 4khz = 4 bytes at 32 khz (16bit stereo pcm)
void UpdateAudioDMA()
{
if (g_audioDMA.AudioDMAControl.Enable && g_audioDMA.BlocksLeft)
static short zero_samples[8*2] = { 0 };
if (g_audioDMA.dma_in_progress)
{
// Read audio at g_audioDMA.ReadAddress in RAM and push onto an
// external audio fifo in the emulator, to be mixed with the disc
// streaming output. If that audio queue fills up, we delay the

This comment was marked as off-topic.

@degasus
Copy link
Member

degasus commented Aug 1, 2014

LGTM

@booto
Copy link
Contributor Author

booto commented Aug 1, 2014

Most recent change includes a fix for https://code.google.com/p/dolphin-emu/issues/detail?id=7121

This could also affect other N64 VC titles

AudioDMA()
AudioDMA():
progress_SourceAddress(0),
progress_Numblocks(0),

This comment was marked as off-topic.

@booto
Copy link
Contributor Author

booto commented Aug 1, 2014

A brief summary of my understanding of the behaviour the audio DMA process:

There are a few registers which control the audio DMA.

AUDIO_DMA_START_HI: the upper 16bits of the address to DMA samples from
AUDIO_DMA_START_LO: the lower 16bits of the address to DMA samples from

AUDIO_DMA_CONTROL_LEN: the lower 15 bits are a count of blocks to transfer (each block is 32 bytes). The highest bit (0x8000) is set to enable the transfer process.

(There are some others e.g. DSP_CONTROL which can influence the process, but they're irrelevant for this discussion.)

When streaming begins (by setting the upper bit of AUDIO_DMA_CONTROL_LEN), the DMA controller loads a new source address from (from AUDIO_DMA_START_HI and AUDIO_DMA_START_LO) and length (from AUDIO_DMA_CONTROL_LEN), triggers an AID interrupt and begins the transfer.

Once those registers have been loaded into the DMA controller, software is free to change them. Once the DMA transfer ends, the controller will automatically resample the source address and length, and reissue the interrupt. i.e. The interrupt is a signal to software that it's safe to load new values.

If the upper bit of AUDIO_DMA_CONTROL_LEN is cleared, I believe that stops the current DMA transfer (probably makes for terrible audio). In order to get the streaming to stop after the current DMA transfer has finished, I believe it writes the enable bit with a length of zero. This wasn't handled properly and is what caused the N64 VC title issue (bad overflow issue). I don't know if the interrupt is meant to be issued when a load with length=0 occurs - seems to work either way.

lioncash added a commit that referenced this pull request Aug 2, 2014
@lioncash lioncash merged commit 44f751f into dolphin-emu:master Aug 2, 2014
@lioncash
Copy link
Member

lioncash commented Aug 2, 2014

[10:10] @skid_au Lioncash, would you please merge #692. looks good to me and phire

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants