Re: [hatari-devel] DSP addressing bug

On 6 November 2015 at 21:49, Laurent Sallafranque <laurent.sallafranque@xxxxxxx> wrote:

Hi Andreas,

It's OK, I'm just waiting for doug's test to validate this patch.

I still have to integrate your other patches about interrupts, but it always fails when I try (SSI all the time is KO).
I'm waiting to have more time to retry it globaly.

Regards
Laurent

Le 06/11/2015 21:33, Andreas Grabher a écrit :

Laurent, thank you for adding the patch for testing purpose. I have a very limited amount of DSP software to test with. It seems none of it uses the special addressing with Nn = P*2^k.

About P being positive:

The datasheet is quite confusing about this.

"If an offset, Nn, is used in the address calculations, the 16-bit absolute value, |Nn|, must be less than or equal to M for proper modulo addressing. If Nn>M, the result is data dependent and unpredictable, except for the special case where Nn = P x 2^k, a multiple of the block size where P is a positive integer."

It is not clear if Nn in the function Nn = P x 2^k is the absolute value or the signed one. At least when comparing Nn>M the variable Nn seems to stand for the absolute value, while missing the ||.

Douglas' message from october 23rd indicates that the special case also needs to work for (Rn)-Nn addressing (as it does since your second patch). If i understand correctly he tested it on real hardware. With (Rn)-Nn addressing and signed Nn, in the above function P would be negative. So for now i think that the function really is |Nn| = P * 2^k and therefore P = |Nn| / 2^k, which means P is always positive.

I was confused by this for quite some time. But i hope i got it right now.

Note: There is a low chance that the datasheet is to understand in a way that if singed Nn<0 the addressing always works, even if Nn<-M. I think this is unlikely.

Am 05.11.2015 um 23:03 schrieb Laurent Sallafranque <laurent.sallafranque@xxxxxxx>:

Hi Andreas,

I've done some tests with your patch and noticed no regression.
My patch seems to work too.

So, I've uploaded your patch for more tests.
Doug and the others, if you could test your favorite Falcon games / demos / utils with the latest patch, it would help.

Doug, is your current devs still work ?

If so, we have 2 versions of the patch (yours that is currently in the head code and mine that was just before).
It would be easy to return to mine for testing purpose if needed.

I've already tested :

bad mood
bound1
bound2
bound3
eko_system
sololuminescentz

Regards

Laurent

Le 05/11/2015 19:24, Andreas Grabher a écrit :

After some testing i came to the conclusion, that the appended patch should be correct. I'm not sure if the function currently in Hatari works in all cases. This is the modified function:

static void dsp_update_rn_modulo(Uint32 numreg, Sint16 modifier)

{

Uint16 bufsize, bufmask, modulo, abs_modifier;

Uint32 r_reg, lobound, hibound;

r_reg = dsp_core.registers[DSP_REG_R0+numreg]|0x10000;

modulo = dsp_core.registers[DSP_REG_M0+numreg]+1;

bufsize = 1;

while (bufsize < modulo) {

bufsize <<= 1;

}

bufmask = bufsize - 1;

lobound = r_reg - (r_reg&bufmask);

hibound = lobound + modulo - 1;

if (modifier<0) {

abs_modifier = -modifier;

} else {

abs_modifier = modifier;

}

if (abs_modifier>modulo) {

if (abs_modifier&bufmask) {

fprintf(stderr,"Dsp: Modulo addressing result unpredictable\n");

} else {

r_reg += modifier;

}

} else {

r_reg += modifier;

if (r_reg>hibound) {

r_reg -= modulo;

} else if (r_reg<lobound) {

r_reg += modulo;

}

}

dsp_core.registers[DSP_REG_R0+numreg] = r_reg & BITMASK(16);

}

Am 29.10.2015 um 20:49 schrieb Andreas Grabher <andreas.grabher@xxxxxxxxxxxx>:

I had a look at the patch and i'm still not sure, if everything is correct about it.

I read the data sheet over and over and i made two new variants. A few things that i don't understand about the original code:

r_reg beeing Sint16:

if dsp_core.registers[DSP_REG_R0+numreg] is greater than 0x7FFF, r_reg will be negative, while the unsigned lobound and hibound will be positive. Won't that cause problems comparing them?

The calculation that is done, if orig_modifier > modulo, but Nn = P * 2^k is not true: Where did you get this from?

Why only doing the modulo operation, if orig_modifier!=modulo. Shouldn't r_reg be always updated to modulo bounds?

first variant:

static void dsp_update_rn_modulo(Uint32 numreg, Sint16 modifier)

{

Uint16 bufsize, modulo, lobound, hibound, bufmask, abs_modifier;

Sint16 r_reg;

modulo = dsp_core.registers[DSP_REG_M0+numreg]+1;

bufsize = 1;

while (bufsize < modulo) {

bufsize <<= 1;

}

bufmask = bufsize - 1;

lobound = dsp_core.registers[DSP_REG_R0+numreg] & (~bufmask);

hibound = lobound + modulo - 1;

r_reg = (Sint16) dsp_core.registers[DSP_REG_R0+numreg];

if (modifier<0) {

abs_modifier = -modifier;

} else {

abs_modifier = modifier;

}

if (abs_modifier>modulo) {

if (abs_modifier&bufmask) {

fprintf(stderr,"Dsp: Modulo addressing result unpredictable\n");

} else {

r_reg += modifier;

}

} else {

r_reg += modifier;

if (r_reg>hibound) {

r_reg -= modulo;

} else if (r_reg<lobound) {

r_reg += modulo;

}

}

dsp_core.registers[DSP_REG_R0+numreg] = ((Uint32) r_reg) & BITMASK(16);

}

second variant:

static void dsp_update_rn_modulo(Uint32 numreg, Sint16 modifier)

{

Uint16 bufsize, modulo, bufmask, abs_modifier;

Uint32 r_reg, lobound, hibound;

modulo = dsp_core.registers[DSP_REG_M0+numreg]+1;

r_reg = dsp_core.registers[DSP_REG_R0+numreg];

bufsize = 1;

while (bufsize < modulo) {

bufsize <<= 1;

}

bufmask = bufsize - 1;

lobound = r_reg & (~bufmask);

hibound = lobound + modulo - 1;

if (modifier<0) {

abs_modifier = -modifier;

} else {

abs_modifier = modifier;

}

if (abs_modifier>modulo) {

if (abs_modifier&bufmask) {

fprintf(stderr,"Dsp: Modulo addressing result unpredictable\n");

return;

} else {

lobound += modifier;

hibound += modifier;

}

}

r_reg += modifier;

if (r_reg>hibound) {

r_reg -= modulo;

} else if (r_reg<lobound) {

r_reg += modulo;

}

dsp_core.registers[DSP_REG_R0+numreg] = r_reg & BITMASK(16);

}

Am 28.10.2015 um 19:13 schrieb Laurent Sallafranque <laurent.sallafranque@xxxxxxx>:

Great news ;)

Regards

Laurent

Le 28/10/2015 17:51, Douglas Little a écrit :

Hi,

Still no problems here - so I think the patch is working, at least for the cases I described before :-)

Best,

D.

On 27 October 2015 at 18:20, Laurent Sallafranque <laurent.sallafranque@xxxxxxx> wrote:

Hi Doug,

Did you have some time to test it ?

Regards

Laurent

Le 25/10/2015 20:41, Douglas Little a écrit :

Thanks, I will try and report back.

Best,

D.

On 25 October 2015 at 18:28, Laurent Sallafranque <laurent.sallafranque@xxxxxxx> wrote:

Hi all,

I've pushed a new patch that should work also with (Rn)-Nn.

I've tested a few demos, prgs and games (Eko system, Raimbow 2, Running, Tere Ra'i, ...) and everything seems OK.

Doug, it's up to you to tell me if this works now.
If not, I'll be OK to have another look on wednesday.

Regards

Laurent

Le 24/10/2015 17:14, Laurent Sallafranque a écrit :

I'll have some time tomorrow evening to send a patch.
I know how to do it, but I haven't got the time today (too much people at home, it's my daughter's birthday ;)

Regards

Laurent

Le 24/10/2015 15:42, Andreas Grabher a écrit :

After some more investigation it seems my patch is faulty. So please ignore it for now.

Am 24.10.2015 um 00:50 schrieb Andreas Grabher <andreas.grabher@xxxxxxxxxxxx>:

If Nn is -8 and modulo is less than 8 (--> k <= 4, P >= 1) it will decrement r_reg by 8 (r_reg += -8 is same as r_reg -= 8):
e.g. Nn = -8, M = 3:
--> k = 2
--> blocksize = 2^k = 4

P = Nn / 2^k = -8 / 2^2 = -2
Rn += P * blocksize = -2 * 4 = -8 (jump 2 blocks down)

Of course i might be totally wrong. The description in the data sheet is very confusing. I absolutely agree that this needs to be tested.

In the meantime i slightly improved the patch to better suit the coding conventions of the DSP part. It seems it consequently avoids using the modulo operator. So my new patch works without it and is also slightly faster.

<experimental_dsp2.diff>

Am 24.10.2015 um 00:25 schrieb laurent.sallafranque@xxxxxxx:

Hi

I haven't got the time to test but at first look i think the patch is wrong.

What If i set -8 into Nn ?

I Know How to fix this but it Will be in 3 days

Regards

----- Mail d'origine -----
De: Douglas Little <doug694@xxxxxxxxxxxxxx>
À: hatari-devel@xxxxxxxxxxxxxxxxxxx
Envoyé: Fri, 23 Oct 2015 20:22:12 +0200 (CEST)
Objet: Re: [hatari-devel] DSP addressing bug

Hi,

I'm not very familiar with that part of Hatari so I'm not sure just by
looking at the diff (at least not without more study). Laurent will know
better.

I will say that the exceptional case occurs when Nn is exactly a multiple
of the 2^k ringbuffer bound. If it is less, it performs modulo addressing.
If greater, the results seem to be undefined i.e. not necessarily modulo.

This happens for (Rn)+Nn, (Rn)-Nn *and also (Rn+Nn) i.e. offset only, with
no update to addr register.*

Best,
D

On 23 October 2015 at 19:07, Andreas Grabher <andreas.grabher@xxxxxxxxxxxx>
wrote:

Laurent and Doug,

what do you think about the appended patch? I'm not sure about it, but it
might work. Maybe worth to test it?

Regards,

Andreas

Am 23.10.2015 um 13:52 schrieb laurent.sallafranque@xxxxxxx:

Hi Doug,

Quick question Laurent - does the patch also work for (Rn)-Nn

addressing?

It should ;)

But I think I understand precisely the problem now (in hatari, -Nn is

passed as a negative argument to the function when the addressing mode is
(Rn)-Nn, I'll have to rewrite it by adding a sign variable and keeping Nn
unchanged).

Don't expec this fix before tuesday or wednesday (we'll have family at

home in the next days).

At least, I see how to fix it definitively now.

Regards

Laurent

----- Mail original -----
De: "Douglas Little" <doug694@xxxxxxxxxxxxxx>
À: hatari-devel@xxxxxxxxxxxxxxxxxxx
Envoyé: Vendredi 23 Octobre 2015 12:20:16
Objet: Re: [hatari-devel] DSP addressing bug

Quick question Laurent - does the patch also work for (Rn)-Nn addressing?

If I keep my workaround just for this case and remove it for the (Rn)+Nn

cases the remaining problem seems to go away...

D

On 23 October 2015 at 11:11, Douglas Little < doug694@xxxxxxxxxxxxxx >

wrote:

Yep definitely something is slightly different on real HW. I'm getting

selectively black pixels in Hatari where they look correct on HW.
Previously I was getting a completely destroyed image.

It's going to take effort to figure out what's going on but I still see

some kind of problem. I'll try to narrow it down a bit.

D.

On 23 October 2015 at 10:54, Douglas Little < doug694@xxxxxxxxxxxxxx >

wrote:

Perhaps I spoke too soon. There is something funny still happening - but

I'll need to do more checks to make sure it's not my fault. I'll update
soon when i have more info....

Doug.

On 23 October 2015 at 10:13, Douglas Little < doug694@xxxxxxxxxxxxxx >

wrote:

It works for me - everything looks good!

Thanks :)

I'll report if I run into any problems with it but I don't think so.

D.

On 22 October 2015 at 22:10, Laurent Sallafranque <

laurent.sallafranque@xxxxxxx > wrote:

I've already tested :

EKO_systems : still working to the end
TERE RA'I ;) : still working to the end
Sololuminezcent : still working to the end

underscore demo seems to crash now but it also crash with the n-1

version of hatari (ie without my patch).

There's a regression with this demo, it was at least working to the 4rth

screen before.

I continue some tests but the patch seems OK to me.

Regards

Laurent

Le 22/10/2015 23:02, Nicolas Pomarède a écrit :

Le 22/10/2015 22:57, Douglas Little a écrit :

Woo, that was quick. Thanks!

If the patch has made it into the repo, I can just collect tomorrow's
nightly build and see if that works for me. Sounds like it probably
will. Otherwise I'll try a local build tomorrow.

Hi,

unfortunately, it seems builds at http://antarctica.no/~hatari/ are not

regularly updated anymore (and it even returns a HTTP 503 error at the
moment :( )

Nicolas