MI_INIT_MODE bug ?

Discuss emulation or program issues here.
Post Reply
User avatar
bsmiles32
Posts: 8
Joined: Tue Jul 29, 2014 1:58 pm

MI_INIT_MODE bug ?

Post by bsmiles32 » Tue Aug 12, 2014 1:49 pm

Hi,

I was reading the source code, and I think thre might be mistake in vr4300/interface.c :

Code: Select all

75    result = word & 0x7F;
76
77     if (word & 0x0080)
78       result &= ~MI_INIT_MODE;
79     else if (word & 0x0100)
80       result |= MI_INIT_MODE;
81
82     if (word & 0x0200)
83       result &= ~MI_EBUS_TEST_MODE;
84     else if (word & 0x0400)
85       result |= MI_EBUS_TEST_MODE;
86
87     if (word & 0x0800) {
88       vr4300->mi_regs[MI_INTR_REG] &= ~MI_INTR_DP;
89       check_for_interrupts(vr4300); // TODO/FIXME: ???
90     }
91
92     if (word & 0x1000)
93       result &= ~MI_RDRAM_REG_MODE;
94     else if (word & 0x2000)
95       result |= MI_RDRAM_REG_MODE;
96
97     vr4300->mi_regs[MI_INIT_MODE_REG] = result;
From that code, bits MI_INIT_MODE, MI_EBUS_TEST_MODE and MI_RDRAM_REG_MODE are always cleared, whatever value "word" has.
I guess, not using the temporary "result" would fix it.

Anyway, congratulations for this very clean code base... I'm kind of jealous, because the emulator I normally contribute to doesn't have this well designed architecture.
Keep up the good work !

beannaich
Posts: 149
Joined: Mon Oct 21, 2013 2:43 pm

Re: MI_INIT_MODE bug ?

Post by beannaich » Tue Aug 12, 2014 8:58 pm

My reading of that code doesn't give me the same impression at all.

User avatar
MarathonMan
Site Admin
Posts: 692
Joined: Fri Oct 04, 2013 4:49 pm

Re: MI_INIT_MODE bug ?

Post by MarathonMan » Tue Aug 12, 2014 9:07 pm

beannaich wrote:My reading of that code doesn't give me the same impression at all.
Same here... scratching my head. :?:

Thanks for the kudos though. :D

User avatar
bsmiles32
Posts: 8
Joined: Tue Jul 29, 2014 1:58 pm

Re: MI_INIT_MODE bug ?

Post by bsmiles32 » Wed Aug 13, 2014 12:14 pm

Sorry, bad explanation.
What I meant was that MI_INIT_MODE, MI_EBUS_TEST_MODE and MI_RDRAM_REG_MODE bits can be cleared even when not explicitly asked.

Example:
* initial value: MI_INIT_MODE_REG = 0x80; // MI_INIT_MODE set
* input : word = 0x00; // ie should change nothing

the current code ends up with MI_INIT_MODE_REG = 0x00 instead of 0x80

The same apply to MI_EBUS_TEST_MODE and MI_RDRAM_REG_MODE.

User avatar
grivy
Posts: 6
Joined: Sat Oct 05, 2013 5:33 am

Re: MI_INIT_MODE bug ?

Post by grivy » Wed Aug 13, 2014 1:49 pm

With word = 0x00, the last statement would become:
vr4300->mi_regs[MI_INIT_MODE_REG] = 0x00;
But I can't say whether that be good or bad :)

beannaich
Posts: 149
Joined: Mon Oct 21, 2013 2:43 pm

Re: MI_INIT_MODE bug ?

Post by beannaich » Wed Aug 13, 2014 8:36 pm

I think I see what you're seeing:

Code: Select all

result = (vr4300->mi_regs[MI_INIT_MODE_REG] & ~0x7f) | (word & 0x7f); // <-- is this a fix to the problem you noticed?

if (word & 0x0080)
  result &= ~MI_INIT_MODE;
else if (word & 0x0100)
  result |= MI_INIT_MODE;

if (word & 0x0200)
  result &= ~MI_EBUS_TEST_MODE;
else if (word & 0x0400)
  result |= MI_EBUS_TEST_MODE;

if (word & 0x0800) {
  vr4300->mi_regs[MI_INTR_REG] &= ~MI_INTR_DP;
  check_for_interrupts(vr4300); // TODO/FIXME: ???
}

if (word & 0x1000)
  result &= ~MI_RDRAM_REG_MODE;
else if (word & 0x2000)
  result |= MI_RDRAM_REG_MODE;

vr4300->mi_regs[MI_INIT_MODE_REG] = result;
@marathonman: is there a name associated with the bottom 7 bits of MI_INIT_MODE_REG? If so, can they be #defined? :) I'd look into it myself and pull request, but, you know.. I can't.
@krom: test ROM time? :)

krom
Posts: 72
Joined: Sat Oct 05, 2013 2:19 am

Re: MI_INIT_MODE bug ?

Post by krom » Thu Aug 14, 2014 1:23 am

beannaich wrote:@krom: test ROM time?
I think I'll wait to see what MarathonMan thinks before I jump into making a test ROM for this, but it should not be too hard to cook up something to check out the correct behavior =D

User avatar
bsmiles32
Posts: 8
Joined: Tue Jul 29, 2014 1:58 pm

Re: MI_INIT_MODE bug ?

Post by bsmiles32 » Thu Aug 14, 2014 1:41 am

is this a fix to the problem you noticed?
Yes :)

User avatar
MarathonMan
Site Admin
Posts: 692
Joined: Fri Oct 04, 2013 4:49 pm

Re: MI_INIT_MODE bug ?

Post by MarathonMan » Wed Aug 20, 2014 1:47 pm

Ahh, I see... I'll try to look at this in the next day or so and patch as required.

Thanks for the report!

Post Reply

Who is online

Users browsing this forum: No registered users and 1 guest