It is currently Sat Oct 21, 2017 10:50 am

All times are UTC - 7 hours





Post new topic Reply to topic  [ 108 posts ]  Go to page Previous  1 ... 3, 4, 5, 6, 7, 8  Next
Author Message
PostPosted: Wed Aug 06, 2014 7:08 pm 
Offline

Joined: Fri May 21, 2010 4:10 pm
Posts: 260
Thank you for your valuable input perilsensitive and also thank you for posting the fix for burai fighter as well. :beer:

I sent you a pm by the way as i am curious about something else you said regarding ninja gaiden and figured it would be easier to pm you as to not derail the thread.

I would have never guessed in a million years that the problem with ninja gaiden (U) would be something like that. Thats why im a noob though, and its great to see the community come together and share fixes, information and the like.

Ive learned a lot from everyone and appreciate anyone who has taken the time to post their thoughts on anything i have ever asked. :D


Top
 Profile  
 
PostPosted: Wed Aug 06, 2014 7:54 pm 
Offline

Joined: Fri May 21, 2010 4:10 pm
Posts: 260
sorry for the double post just wanted to add this real quick. I merged in your fix for burai fighter, but i noticed this overwrites james fix for mickey's in nstppu.cpp he shared a while back.

NST_FORCE_INLINE void Ppu::UpdateScrollAddressLine()
{
if (io.line)
io.line.Toggle( scroll.address & 0x3FFF, cpu.GetCycles() );
}


to


NST_FORCE_INLINE void Ppu::UpdateScrollAddressLine()
{
if (io.line)
{
int a12_mask = ~((scroll.address & 0x2000) >> 1);
io.line.Toggle( (scroll.address & a12_mask) & 0x3FFF, cpu.GetCycles() );
}
}


This part was overwritten so mickeys return back to the shaky status bar. Is there a way to put this back in for both games to coexist peacefully?


Top
 Profile  
 
PostPosted: Wed Aug 06, 2014 8:40 pm 
Offline

Joined: Mon Dec 12, 2011 8:15 pm
Posts: 304
*Spitfire_NES* wrote:
sorry for the double post just wanted to add this real quick. I merged in your fix for burai fighter, but i noticed this overwrites james fix for mickey's in nstppu.cpp he shared a while back.

NST_FORCE_INLINE void Ppu::UpdateScrollAddressLine()
{
if (io.line)
io.line.Toggle( scroll.address & 0x3FFF, cpu.GetCycles() );
}


to


NST_FORCE_INLINE void Ppu::UpdateScrollAddressLine()
{
if (io.line)
{
int a12_mask = ~((scroll.address & 0x2000) >> 1);
io.line.Toggle( (scroll.address & a12_mask) & 0x3FFF, cpu.GetCycles() );
}
}


This part was overwritten so mickeys return back to the shaky status bar. Is there a way to put this back in for both games to coexist peacefully?

Integration code

if (!(regs.ctrl[1] & Regs::CTRL1_BG_SP_ENABLED) ||
(scanline == SCANLINE_VBLANK)) {
UpdateAddressLine(scroll.address & 0x3fff);
}

to UpdateScrollAddressLine;


Top
 Profile  
 
PostPosted: Wed Aug 06, 2014 10:03 pm 
Offline

Joined: Mon Jul 15, 2013 3:35 pm
Posts: 13
*Spitfire_NES* wrote:
sorry for the double post just wanted to add this real quick. I merged in your fix for burai fighter, but i noticed this overwrites james fix for mickey's in nstppu.cpp he shared a while back.

NST_FORCE_INLINE void Ppu::UpdateScrollAddressLine()
{
if (io.line)
io.line.Toggle( scroll.address & 0x3FFF, cpu.GetCycles() );
}


to


NST_FORCE_INLINE void Ppu::UpdateScrollAddressLine()
{
if (io.line)
{
int a12_mask = ~((scroll.address & 0x2000) >> 1);
io.line.Toggle( (scroll.address & a12_mask) & 0x3FFF, cpu.GetCycles() );
}
}


This part was overwritten so mickeys return back to the shaky status bar. Is there a way to put this back in for both games to coexist peacefully?


I actually eliminated UpdateScrollAddressLine() in my patch because it doesn't change the value of io.address, which is the value currently on the address bus. It really should though, as the actual bus value does change according to Visual 2C02. Since UpdateAddressLine already does that (and it seems the point of UpdateScrollAddressLine() was to *not* update the bus), it didn't make sense to leave it in.

As for the shaky status bar in Safari in Letterland, the fix James posted requires that the PPU never put addresses >= $3000 on the address bus (specifically, he forces A12 to 0 for addresses in that range before putting them on the bus, preventing the counter from being clocked), but that doesn't seem to match what Visual 2C02 does so I didn't include that change in my patch. The point of my patch was to show that the problem with Burai Fighter had a different (although somewhat related) root cause, the fix for which didn't require any special handling of $3000-$3FFF.

You could integrate the two patches by making a similar change to the one James made in his post, but instead of doing it to UpdateScrollAddressLine() (which no longer exists) do it directly to the address passed to UpdateAddressLine() in the implementations of $2006 and $2007. I'm too lazy to do this myself though. :-)

It may (or may not) be worth noting that Safari in Letterland uses an MC-ACC, which James has shown has similar but different IRQ behavior to that of the MMC3. The difference James documented in that thread doesn't really make a difference for Safari in Letterland, but it makes me wonder if there are other differences we're missing that do. :-/


Top
 Profile  
 
PostPosted: Thu Aug 07, 2014 5:39 am 
Offline

Joined: Mon Dec 12, 2011 8:15 pm
Posts: 304
in NstApu.cpp 2138
if (!readAddress)
{
cpu.StealCycles( cpu.GetClock(cpu.IsWriteCycle(clock) ? 2 : 3) );
}
to
if (!readAddress)
{
cpu.StealCycles( cpu.GetClock(cpu.IsWriteCycle(clock) ? 3 : 4) );
}
Attachment:
[[)$6TQ]WWWJR@[G}554ESY.jpg
[[)$6TQ]WWWJR@[G}554ESY.jpg [ 137.24 KiB | Viewed 1274 times ]

The need to observe and test


Top
 Profile  
 
PostPosted: Thu Aug 07, 2014 7:51 am 
Offline

Joined: Mon Jul 15, 2013 3:35 pm
Posts: 13
zxbdragon wrote:
in NstApu.cpp 2138
if (!readAddress)
{
cpu.StealCycles( cpu.GetClock(cpu.IsWriteCycle(clock) ? 2 : 3) );
}
to
if (!readAddress)
{
cpu.StealCycles( cpu.GetClock(cpu.IsWriteCycle(clock) ? 3 : 4) );
}



This won't work; it prevents the attribute glitch but breaks DMC DMA timing. For example, the sprdma_and_dmc_dma test roms hang before producing output with this change, which makes DMA take one extra cycle. blargg demonstrated that at most DMC DMA will steal 3 cycles if it falls on a write cycle and 4 if it falls on a read. The first 2 cycles on a write or 3 cycles on a read are where the CPU is paused, doing nothing. The last cycle is the actual DMA transfer; the CPU is still paused but the DMA unit is reading from memory. Your change adds an extra cycle before the transfer starts, making DMA now steal 4 cycles on a write and 5 cycles on a read. This may break games that require precise DMC DMA timing.


Top
 Profile  
 
PostPosted: Thu Aug 07, 2014 7:58 am 
Offline

Joined: Mon Dec 12, 2011 8:15 pm
Posts: 304
perilsensitive wrote:
zxbdragon wrote:
in NstApu.cpp 2138
if (!readAddress)
{
cpu.StealCycles( cpu.GetClock(cpu.IsWriteCycle(clock) ? 2 : 3) );
}
to
if (!readAddress)
{
cpu.StealCycles( cpu.GetClock(cpu.IsWriteCycle(clock) ? 3 : 4) );
}



This won't work; it prevents the attribute glitch but breaks DMC DMA timing. For example, the sprdma_and_dmc_dma test roms hang before producing output with this change, which makes DMA take one extra cycle. blargg demonstrated that at most DMC DMA will steal 3 cycles if it falls on a write cycle and 4 if it falls on a read. The first 2 cycles on a write or 3 cycles on a read are where the CPU is paused, doing nothing. The last cycle is the actual DMA transfer; the CPU is still paused but the DMA unit is reading from memory. Your change adds an extra cycle before the transfer starts, making DMA now steal 4 cycles on a write and 5 cycles on a read. This may break games that require precise DMC DMA timing.

You find a better solution ?


Top
 Profile  
 
PostPosted: Thu Aug 07, 2014 8:30 am 
Offline
User avatar

Joined: Sat Jan 22, 2005 8:51 am
Posts: 427
Location: Chicago, IL
perilsensitive wrote:
I tested this in Visual 2C02...while rendering the bus is controlled by the rendering logic, and therefore not directly affected by the change to loopy_v)

Nice job digging into this!

perilsensitive wrote:
It may (or may not) be worth noting that Safari in Letterland uses an MC-ACC, which James has shown has similar but different IRQ behavior to that of the MMC3. The difference James documented in that thread doesn't really make a difference for Safari in Letterland, but it makes me wonder if there are other differences we're missing that do. :-/

I played around with this a bit, but didn't have much luck either. It'll be interesting to find out what the issue is here... I'll try to take a look at it on the hardware sometime soon.

_________________
get nemulator
http://nemulator.com


Top
 Profile  
 
PostPosted: Thu Aug 07, 2014 8:44 am 
Offline

Joined: Mon Jul 15, 2013 3:35 pm
Posts: 13
zxbdragon wrote:
You find a better solution ?


I don't have a solution yet. Every emulator that implements these extra reads caused by DMA has similar glitches, so *nobody* has a solution yet. You could disable the extra reads altogether, although this would still be a hack. No games depend on this behavior, and many emulators simply don't implement it at all (FCEUX, for example). It is of interest to NES developers though, especially when it causes extra reads of the controller ports.

You could do something like this in NstApu.cpp to disable the reads if the glitch really bothers you that much:

Code:
diff --git a/source/core/NstApu.cpp b/source/core/NstApu.cpp
index cf1f3df..cd14e6c 100644
--- a/source/core/NstApu.cpp
+++ b/source/core/NstApu.cpp
@@ -2137,19 +2137,19 @@ namespace Nes
                        {
                                cpu.StealCycles( cpu.GetClock(3) );
                        }
-                       else
-                       {
-                               NST_DEBUG_MSG("DMA/Read conflict!");
+                       // else
+                       // {
+                       //      NST_DEBUG_MSG("DMA/Read conflict!");
 
-                               cpu.StealCycles( cpu.GetClock(1) );
+                       //      cpu.StealCycles( cpu.GetClock(1) );
 
-                               if ((readAddress & 0xF000) != 0x4000)
-                                       cpu.Peek( readAddress );
+                       //      if ((readAddress & 0xF000) != 0x4000)
+                       //              cpu.Peek( readAddress );
 
-                               cpu.StealCycles( cpu.GetClock(1) );
-                               cpu.Peek( readAddress );
-                               cpu.StealCycles( cpu.GetClock(1) );
-                       }
+                       //      cpu.StealCycles( cpu.GetClock(1) );
+                       //      cpu.Peek( readAddress );
+                       //      cpu.StealCycles( cpu.GetClock(1) );
+                       // }
 
                        dma.buffer = cpu.Peek( dma.address );
                        cpu.StealCycles( cpu.GetClock() );


Top
 Profile  
 
PostPosted: Thu Aug 07, 2014 2:33 pm 
Offline

Joined: Fri May 21, 2010 4:10 pm
Posts: 260
zxbdragon wrote:
*Spitfire_NES* wrote:
sorry for the double post just wanted to add this real quick. I merged in your fix for burai fighter, but i noticed this overwrites james fix for mickey's in nstppu.cpp he shared a while back.

NST_FORCE_INLINE void Ppu::UpdateScrollAddressLine()
{
if (io.line)
io.line.Toggle( scroll.address & 0x3FFF, cpu.GetCycles() );
}


to


NST_FORCE_INLINE void Ppu::UpdateScrollAddressLine()
{
if (io.line)
{
int a12_mask = ~((scroll.address & 0x2000) >> 1);
io.line.Toggle( (scroll.address & a12_mask) & 0x3FFF, cpu.GetCycles() );
}
}


This part was overwritten so mickeys return back to the shaky status bar. Is there a way to put this back in for both games to coexist peacefully?

Integration code

if (!(regs.ctrl[1] & Regs::CTRL1_BG_SP_ENABLED) ||
(scanline == SCANLINE_VBLANK)) {
UpdateAddressLine(scroll.address & 0x3fff);
}

to UpdateScrollAddressLine;


I tried your fix zxbdragon to get the mickeys fix reverted back to this:

if ((regs.ctrl[1] & Regs::CTRL1_BG_SP_ENABLED) && !(data & Regs::CTRL1_BG_SP_ENABLED))
UpdateScrollAddressLine(scroll.address & 0x3fff);

and mickeys did not revert back. What did you do exactly and what is the best way to integrate james fix and your patch?

Thanks again for all your contributions to this. Looks like we are all learning something here lol.

As for ninja gaiden ill add this in and give it a whirl and see where it goes. I dont think anything will get broken in the process as you seem to know your ins and outs far better than me. :D :beer: :beer:


Top
 Profile  
 
PostPosted: Thu Aug 07, 2014 2:53 pm 
Offline
Formerly Fx3
User avatar

Joined: Fri Nov 12, 2004 4:59 pm
Posts: 3064
Location: Brazil
@perilsensitive

Please, you must rephrase what you wrote about the IRQ fix. It's quite confusing to me, since you mixed it with tepple's question.


Top
 Profile  
 
PostPosted: Thu Aug 07, 2014 3:20 pm 
Offline

Joined: Mon Jul 15, 2013 3:35 pm
Posts: 13
I've been looking into the problem with Safari in Letterland a bit more after briefly discussing the issue with James via PM, and I have data explaining why the shaking happens, as well as a possible theory for what the actual fix is. Of course, it's possible that somebody has already tested this aspect of the MC-ACC and my theory is wrong, but I can't think of a better explanation at the moment. :-/

The status bar shakes due to the IRQ firing early on some frames. It is supposed to fire on scanline 205, but occasionally fires on scanline 204 (James pointed out some $2006 updates that occur when rendering; these happen one scanline after the IRQ). It fires on scanline 204 instead due to an extra clock occurring around cycle 300 on scanline 188. That clock happens because the game disables rendering around cycle 276 (putting vramaddr_v onto the address bus), then writes a palette address to $2006 around cycle 300 (putting the $3Fxx address onto the bus). If the vertical scroll value was even at the time rendering was disabled, this triggers an A12 rise. The difference in PPU clocks between when A12 falls and then rises again is between 24 and 30 clocks, clearly more than 15, so the IRQ counter gets clocked.

The only way around this (assuming that the PPU does put the palette address onto the bus and that nothing else is being emulated incorrectly), is to require more time between A12 rises in order to clock the counter. This doesn't seem unreasonable, especially since we already know that the MC-ACC doesn't *quite* behave like an MMC3. The required time between rises must be at least the same as required by the MMC3, but there's no reason it couldn't be greater. I tried various values and found that a minimum of 39 PPU cycles (or 13 M2 cycles, since the cart doesn't have access to the PPU clock signal) are required to fix the shaky status bar. The actual required time between rises may be larger than this, but there's no way to know what it is without testing an actual MC-ACC.

I've tested this theory in my emulator as well as Nestopia, and the shakes completely disappear (and the status bar looks correct: no scanline missing from the middle). I don't have a clean patch for Nestopia yet though, and simply changing the value of CLOCK_FILTER in NstBoardMmc3.hpp (as I did to test this theory) will definitely break actual MMC3 games. This is all purely academic if my theory is wrong though, so I'm hesitant to create a patch until this is confirmed (if it is confirmed). :-)


Top
 Profile  
 
PostPosted: Thu Aug 07, 2014 3:49 pm 
Offline

Joined: Mon Jul 15, 2013 3:35 pm
Posts: 13
Zepper wrote:
@perilsensitive

Please, you must rephrase what you wrote about the IRQ fix. It's quite confusing to me, since you mixed it with tepple's question.


I will reply via PM to avoid repeating myself in the thread. :-)


Top
 Profile  
 
PostPosted: Thu Aug 07, 2014 6:10 pm 
Offline

Joined: Fri May 21, 2010 4:10 pm
Posts: 260
hey peril, in the meantime what is the easiest way to merge the 2 patches together until a proper fix is found if you dont mind me asking? :)


Top
 Profile  
 
PostPosted: Thu Aug 07, 2014 10:32 pm 
Offline

Joined: Mon Dec 12, 2011 8:15 pm
Posts: 304
perilsensitive wrote:
zxbdragon wrote:
You find a better solution ?


I don't have a solution yet. Every emulator that implements these extra reads caused by DMA has similar glitches, so *nobody* has a solution yet. You could disable the extra reads altogether, although this would still be a hack. No games depend on this behavior, and many emulators simply don't implement it at all (FCEUX, for example). It is of interest to NES developers though, especially when it causes extra reads of the controller ports.

You could do something like this in NstApu.cpp to disable the reads if the glitch really bothers you that much:

Code:
diff --git a/source/core/NstApu.cpp b/source/core/NstApu.cpp
index cf1f3df..cd14e6c 100644
--- a/source/core/NstApu.cpp
+++ b/source/core/NstApu.cpp
@@ -2137,19 +2137,19 @@ namespace Nes
                        {
                                cpu.StealCycles( cpu.GetClock(3) );
                        }
-                       else
-                       {
-                               NST_DEBUG_MSG("DMA/Read conflict!");
+                       // else
+                       // {
+                       //      NST_DEBUG_MSG("DMA/Read conflict!");
 
-                               cpu.StealCycles( cpu.GetClock(1) );
+                       //      cpu.StealCycles( cpu.GetClock(1) );
 
-                               if ((readAddress & 0xF000) != 0x4000)
-                                       cpu.Peek( readAddress );
+                       //      if ((readAddress & 0xF000) != 0x4000)
+                       //              cpu.Peek( readAddress );
 
-                               cpu.StealCycles( cpu.GetClock(1) );
-                               cpu.Peek( readAddress );
-                               cpu.StealCycles( cpu.GetClock(1) );
-                       }
+                       //      cpu.StealCycles( cpu.GetClock(1) );
+                       //      cpu.Peek( readAddress );
+                       //      cpu.StealCycles( cpu.GetClock(1) );
+                       // }
 
                        dma.buffer = cpu.Peek( dma.address );
                        cpu.StealCycles( cpu.GetClock() );


thank you,I know how to solve the!


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 108 posts ]  Go to page Previous  1 ... 3, 4, 5, 6, 7, 8  Next

All times are UTC - 7 hours


Who is online

Users browsing this forum: No registered users and 4 guests


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
Powered by phpBB® Forum Software © phpBB Group