Re: [hatari-devel] Suggested patch to fix problem handling STOP when DSP is active

[Eero Tamminen <oak@xxxxxxxxxxxxxx>]

Hi Roger,

I've updated your patch to latest Hatari version
(attached) and tested it.

Results...

All the bad DSP audio issues I found are fixed,
i.e. following work now as well as with TOS4:
* FlaySID
* GemMOD
* GemGT2
* GemPlay
* MP2AUDIO

In all of them Hatari takes now same or smaller
amount of CPU with EmuTOS than with TOS4.

Time shown in MP2AUDIO window titlebar advances
now as it should also with EmuTOS.


Following issues with EmuTOS were not fixed by it:
* Sound missing from GemPlay AON module
* Issues with Escape's "Hmmm..." demo:
  - Sound missing
  - Hatari taking 2x more CPU with EmuTOS
  - Demo being 2x slower with EmuTOS


Nicolas, are there any changes that should be
done to how cycles are counted in the patch,
or is there any reason why it shouldn't be
commited before Hatari release?


	- Eero

On 10/29/20 4:03 AM, Roger Burrows wrote:
> As you probably know, I'm currently investigating differences in behaviour of
> some programs between EmuTOS and Atari TOS.  The programs in question use the
> DSP to process audio, and the audio output is worse on EmuTOS than Atari TOS.
>
> Because of the actual symptoms, it occurred to me that perhaps the biggest
> difference in basic processing between TOS4 and EmuTOS is the latter's use of
> the STOP instruction to lighten processor load on emulators.  I built a copy of
> EmuTOS that doesn't use the STOP instruction (it's just a simple config option
> to disable it), but alas my poor underpowered Linux box could not cope with the
> additional load.
>
> So I thought about the Hatari side of things and wondered how the DSP was run
> in parallel with the 680x0.  If I understand things correctly, it's the job of
> DSP_Run() to make sure that it gets the right number of cycles, on a regular
> basis.  However, when the 680x0 is stopped, Hatari runs through a while() loop
> in do_specialties(), waiting for a matching interrupt, and during this period,
> DSP_Run() is NOT called.
>
> I added some debugging code and discovered that the STOP typically lasts around
> 60000 680x0 cycles (@ 16MHz), long enough for up to 60000 DSP instructions.
> The way the code is written, the DSP will eventually get those cycles, but they
> will be delayed, and the DSP processing will not be overlapped with the STOP
> state (unlike under TOS4, when it *will* be overlapped with the busy-waiting).
> Apart from affecting the DSP, this also affects the 680x0, since the host will
> not run 680x0 code until the DSP has 'caught up'.
>
> So I made the attached small patch to Hatari 2.2.1, to call DSP_Run() on a
> regular basis within the 'STOP is active' loop.  With this patch, the sound
> quality from EmuTOS is now better than that from Atari TOS (identical
> configurations), although it's still not perfect - that underpowered Linux box
> again.
>
> Please consider this patch (or something equivalent) for incorporation in
> Hatari.
>
> Thanks,
> Roger Burrows
> Cross-posted to emutos-devel since it's relevant there too.
>
>
> The following section of this message contains a file attachment
> prepared for transmission using the Internet MIME message format.
> If you are using Pegasus Mail, or any other MIME-compliant system,
> you should be able to save it or view it from within your mailer.
> If you cannot, please ask your system administrator for assistance.

commit ffcfe0af35c419b396a976c6374f773e72941212
Author: Eero Tamminen <oak@xxxxxxxxxxxxxx>
Date:   Thu Oct 29 23:22:57 2020 +0200

    WIP: fix DSP handling with STOP instruction
    
    STOP instruction is used by EmuTOS to save emulator cycles when TOS is
    idling.
    
    STOP take an arbitrarily large number of cycles, so DSP needs to be
    run while CPU core waits it to end. Otherwise DSP will run a huge
    number of instructions after STOP, and it won't be synching to CPU
    side as it should.
    
    This fixes broken sound under EmuTOS in several music players that use
    DSP to handle music playback (MP2AUDIO, GemMOD, GemGT2, GemPlay).
    
    Reported and original patch by Roger Burrows.

diff --git a/src/cpu/newcpu.c b/src/cpu/newcpu.c
index 0afddfe1..d84b7f7e 100644
--- a/src/cpu/newcpu.c
+++ b/src/cpu/newcpu.c
@@ -5047,6 +5047,17 @@ static int do_specialties (int cycles)
 		}
 #endif
 
+#ifdef WINUAE_FOR_HATARI
+		if (bDspEnabled) {
+			/* need to process DSP instructions while stopped,
+			 * otherwise there can be massive amount of them
+			 * to process, which causes huge pause on CPU side
+			 */
+			Uint64 tmpcycles = CyclesGlobalClockCounter - DSP_CyclesGlobalClockCounter;
+			if (tmpcycles > 100u)
+				DSP_Run(tmpcycles);
+		}
+#endif
 	}
 
 	if (regs.spcflags & SPCFLAG_TRACE)
diff --git a/src/uae-cpu/newcpu.c b/src/uae-cpu/newcpu.c
index 865af5b2..faf96273 100644
--- a/src/uae-cpu/newcpu.c
+++ b/src/uae-cpu/newcpu.c
@@ -1708,6 +1708,9 @@ static int do_specialties (void)
 		regs.stopped = 0;
 		unset_special (SPCFLAG_STOP);
 	    }
+	    /* TODO: call DSP_Run() to avoid accumulation of
+	     * huge amount of DSP cycles during STOP instruction
+	     */
 	}
     }