Re: [hatari-devel] DSP patch suggestion

[ Thread Index | Date Index | More lists.tuxfamily.org/hatari-devel Archives ]


Le 03/12/2023 à 13:51, Andreas Grabher a écrit :


Am 03.12.2023 um 12:32 schrieb Andreas Grabher <andreas_g86@xxxxxxxxxx>:


Am 03.12.2023 um 12:09 schrieb Nicolas Pomarède <npomarede@xxxxxxxxxxxx>:

Le 03/12/2023 à 11:09, Andreas Grabher a écrit :
Hello all,
I created a new patch that will cleanly merge with Hatari. It contains these changes:
Changes without (noticeable) effect:
* dsp.c: Changing "P ram" to "P rom“ in spaces[2][1]. This element of
   the array is unused by Hatari and the change therefore has zero
   effect but makes things future proof in case one wants to add the
   bootstrap ROM later.
* dsp.c: Moving the check for program space above the check for
   internal RAM. This change has no functional effect but makes things
   future proof in case one wants to add the bootstrap ROM later. Very
   minor side effect is that one additional condition is checked in
   case of X and Y internal RAM access. Effect on performance is not
   noticeable (if one wants to optimise for performance many other
   things could be improved). No other accesses are affected.
* dsp_core.c: Removing duplicate HSR initialisation. This has zero effect.
* dsp_core.c: Moving dsp_host_interrupt initialisation to a different
   place inside the function. This is just cleaning and has zero effect.
* dsp_core.h: Adding rom[2] (P ROM). This change has no functional
   effect but makes things future proof in case one wants to add the
   bootstrap ROM later.
* dsp_cpu.c: Moving check for program space above check for internal
   RAM. See comments in point 2.
Changes with effect:
* dsp_core.c: Adding host interface initialisation. This function of
   the DSP was not simulated previously but is required for some DSP
   code on the NeXT. I don’t know if any Hatari program makes use of
   this function but I think it should be included anyway to match real
   DSP more closely. I highly recommend this addition.
* dsp_core.c: Adding early abort of the hacked bootstrap loader. The
   real bootstrap loader permanently checks HF0. If HF0 is set, it
   exits early even if a limited amount of bytes have been transferred.
   NeXT makes heavy use of this feature. This is required to accurately
   simulate the function of the bootstrap loader programmed to the real
   DSP. This addition is highly recommended and has zero negative side
   effects.
* dsp_core.c: The real bootstrap loader sets R0 to the amount of bytes
   copied and OMR to 2. This needs to be added to accurately simulate
   the bootstrap program and match real DSP. This change is highly
   recommended and has zero negative side effects.
* dsp_core.c: Adding host received data interrupt. This is missing
   from the current Hatari code. It must be a mistake. This is highly
   recommended to match real DSP.
Changes not included:
* DMA interface: Atari obviously does not use the DMA interface of the
   DSP host port. Therefore I didn’t include my implementation of it.
* Bootstrap ROM: This feature is not necessary. It is more a question
   of wanting to avoid using hacks (dsp_core.running et al). I did not
   include this in my patch because it requires extra CPU power without
   functional benefits.
* Starting with different modes: The DSP has 4 different modes. Hatari
   is hard coded to always use mode 2 (normal expanded mode). Real
   simulation of the modes would be required for using bootstrap ROM.
   Although in theory there could be other programs making use of this,
   there seem to be none on the NeXT and probably also none on Atari.
   Therefore I did not include this.
There seem to be a few white space issues, because tabs are used between code and comments and because changing from SDL types to normal fixed length types messed things a bit. If you want me to fix this, I can provide a patch (replacing tabs with spaces).
Feel free to ask any questions.

Hi

thanks for your patch.
since it implies several changes, could it be possible for you to split it at least in two ? one patch for not noticeable effect and one for possible effects ?

I understand these work under Previous, but just in case it breaks sthg in Hatari for some games/demos, it would make bisescting easier if we don't commit one single whole patch.

If not possible, the single patch can be applied anyway as there's no one really working on dsp in hatari at the moment, any improvement/fix should be merged.

Nicolas

Yes, of course! I have split it into three patches. And yes, the functional changes (patch „improving“) have been part of Previous for a long time and have not caused any regressions. On the contrary, they were necessary to solve problems. Before making changes I always checked the data sheet of the 56001 to confirm that my changes match the specifications.

Regards
Andreas

<dsp_patch_improving.diff><dsp_patch_cleaning.diff><dsp_patch_futureproofing.diff>

I just recognised that my patches had the wrong level and different directory names. Appended patches are fixed. They can be applied with patch -p1.


Thanks for splitting your patches and sharing them (I just had to remove the reference to "dsp_core.dma_address_counter=0", Hatari has no dma_address_counter member)

I applied to Hatari, for regular Falcon users (Laurent :) ), please check you don't see any regression

Nicolas




Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/