Re: [hatari-devel] DSP "running" on non-falcon

[Eero Tamminen <oak@xxxxxxxxxxxxxx>]

Hi,

On 26.7.2022 20.07, Nicolas Pomarède wrote:
> Le 26/07/2022 à 18:37, Eero Tamminen a écrit :
> > And looking later to the trace, why the heck EmuTOS is trying to do 
> > DSP on emulated 030 MegaST?
>
> This is definitely wrong (since a long time in Hatari) : for example 
> m68k_run_mmu030 () does this :
>
>    if (bDspEnabled) {
>          DSP_Run(2 * cpu_cycles * 2 / CYCLE_UNIT);
>    }

Looking at the DSP_Run() code, this does not actually run DSP code on 
anything else than Falcon, only this:
----------------------------------------
        if ( nHostCycles == 0 )
                return;

        DSP_CyclesGlobalClockCounter = CyclesGlobalClockCounter;

        save_cycles += nHostCycles * 2;

        if (dsp_core.running == 0)
                return;
----------------------------------------

This is because dsp_core.running gets set only after first write to 
Falcon DSP CPU_HOST_TRXL IO register, which obviously can happen only on 
Falcon.


> so, as soon as DSP is enabled in the GUI or with some --options, this 
> calls DSP_Enable() in falcon/dsp.c, which sets bDspEnabled=true , even 
> if machine is not a falcon and has no DSP at all (even an STF will 
> report bDspEnabled=true)  :(((

Yeah, DSP_Enabled() seems to be called from many places based just on 
the DSP type.

I think the reason for doing things based on specific emulation 
settings, instead of machine type, is that machine type can change quite 
late in emulation (re-)initialization (when TOS is loaded), based on the 
TOS version (in tos.c).


> we should at least have this change :
>
> -- a/src/falcon/dsp.c
> +++ b/src/falcon/dsp.c
> @@ -191,8 +191,11 @@ void DSP_Reset(void)
>   void DSP_Enable(void)
>   {
>   #if ENABLE_DSP_EMU
> -       bDspEnabled = true;
> -       DSP_CyclesGlobalClockCounter = CyclesGlobalClockCounter;
> +       if ( Config_IsMachineFalcon() )
> +       {
> +               bDspEnabled = true;
> +               DSP_CyclesGlobalClockCounter = CyclesGlobalClockCounter;
> +       }
>   #endif
>   }
>
> Can you try ? it should not call DSP_Run() in TT mode, maybe improving 
> your tests  (to be complete, we should do the same in DSP_Init and 
> DSP_UnInit, but aprat from calling un-needed DSP functions when not in 
> falcon mode, it should not give problems)

Ok, you already pushed the change.  I tested it and it seems to work OK. 
 I also updated the release-notes description a bit.

(And fixed another issue AddressSanitizer noticed.)


	- Eero