PowerPak MMC3 fixes: accurate IRQ, MC-ACC submapper, iNES 2 CHR-RAM

Discuss hardware-related topics, such as development cartridges, CopyNES, PowerPak, EPROMs, or whatever.

Moderator: Moderators

User avatar
rainwarrior
Posts: 8732
Joined: Sun Jan 22, 2012 12:03 pm
Location: Canada
Contact:

PowerPak MMC3 fixes: accurate IRQ, MC-ACC submapper, iNES 2 CHR-RAM

Post by rainwarrior »

In 2011, Loopy made a supplemental mapper pack for the PowerPak (powerpak_loopy.zip), but the MMC3 mapper always caused the scroll splits to jitter up and down for me.

I used TheFox's MMC3 mapper as an alternative until now, but I always preferred the original plain mappers with game genie support to having that savestate menu inserted.

.

Edit: After a few more revisions it was improved even more, see attempt 13 below instead.

Original post continues below this line.


.

Anyway, I think I've fixed it:
loopy_mmc3_fix.zip
(25.12 KiB) Downloaded 47 times

Source code for some of these mappers was found: here.

This was a 1 line change to the a12 filter:

Code: Select all

    reg a12;
    wire a12clear;
    always@(posedge chrain[12], posedge a12clear)
        if(a12clear)
            a12<=0;
        else
            a12<=1;

    reg [2:0] a12count;
    always@(posedge m2)
        if(chrain[12])
            a12count<=0;
        else if(a12count!=7)
            a12count<=a12count+1;
    assign a12clear=a12count==7; // fix: ==6 was causing jitter
//    assign a12clear=a12count==6;
So, previously "a12clear" would rise and fall at step 6, but instead I've made it hold steady at step 7. I'm guessing there was some timing conflict with the more transient version of the filter, causing a12 to never return to 0? Basically the IRQs looked like they were happening 1 line too late, erratically.

Also, as an aside, it has this warning, which I don't know enough about the Xilinx stuff to understand. Not sure if it's something that should be addressed:

Code: Select all

WARNING:PhysDesignRules:372 - Gated clock. Clock net mapper/irqtrip is sourced
   by a combinatorial pin. This is not good design practice. Use the CE pin to
   control the loading of data into the flip-flop.
Last edited by rainwarrior on Sun Dec 04, 2022 12:56 pm, edited 5 times in total.
Trirosmos
Posts: 50
Joined: Mon Aug 01, 2016 4:01 am
Location: Brinstar, Zebes
Contact:

Re: PowerPak MMC3 jitter fix for loopy's mapper

Post by Trirosmos »

That's kind of a weird filter. Seems to me like it'd filter out negative A12 edges, but any positive edge would still trigger the counter.

I believe an implementation closer to how the MMC3 handles it would be:

Code: Select all

reg [2:0] a12_delay_line;
wire a12 = a12_delay_line == 0 && chrain[12];

always @(posedge m2) begin
    a12_delay_line <= {a12_delay_line[1:0],chrain[12]};
end
rainwarrior wrote: Sun Oct 30, 2022 7:02 pm Also, as an aside, it has this warning, which I don't know enough about the Xilinx stuff to understand. Not sure if it's something that should be addressed:

Code: Select all

WARNING:PhysDesignRules:372 - Gated clock. Clock net mapper/irqtrip is sourced
   by a combinatorial pin. This is not good design practice. Use the CE pin to
   control the loading of data into the flip-flop.
That's probably not too worrisome. I mean, if it works.

That is caused by:

Code: Select all

wire irqtrip=~|count;
wire irqclear=nesprg_we & m2_n & {prgain[15:13],prgain[0]}==4'b1110;
    
always@(posedge irqtrip, posedge irqclear)
    if(irqclear) irq<=0;
    else irq<=irqen;
Generally, you'd want to avoid having two signals trigger a procedural block. Clocking a flip flop with a signal that is generated by logic in the FPGA instead of directly from an external clock or by the FPGA's PLL can also be wonky.

In this case, irqtrip is synchronous to A12 since it only changes when count does, so all that is required is an asynchronous reset:

Code: Select all

reg [7:0] count;
wire irqclear=nesprg_we & m2_n & {prgain[15:13],prgain[0]}==4'b1110;

always@(posedge a12, posedge irqclear) begin
    if(irqclear) irq <= 0;
    else if(!count) irq <= irqen;

    if(!count | reload) count <= irqlatch;
    else count <= count-1;
end
It might still complain about irqclear, tho
User avatar
rainwarrior
Posts: 8732
Joined: Sun Jan 22, 2012 12:03 pm
Location: Canada
Contact:

Re: PowerPak MMC3 jitter fix for loopy's mapper

Post by rainwarrior »

Thanks for the notes. I'm not quite up for testing a more significant change like that at the moment, but I'll keep it in mind for the future.

In the shorter term, I'm going to try to organize all the current PowerPak mapper additions into a coherent package so that it isn't scattered on so many different forum threads and other places.
lidnariq
Posts: 11432
Joined: Sun Apr 13, 2008 11:12 am

Re: PowerPak MMC3 jitter fix for loopy's mapper

Post by lidnariq »

Trirosmos wrote: Tue Nov 01, 2022 10:25 am I believe an implementation closer to how the MMC3 handles it would be:
Rereading Furrtek's schematic, I think it's negedge
User avatar
rainwarrior
Posts: 8732
Joined: Sun Jan 22, 2012 12:03 pm
Location: Canada
Contact:

Re: PowerPak MMC3 jitter fix for loopy's mapper

Post by rainwarrior »

I tried Crystalis with my replacement mapper and it seems to have one vertical position you can stand in the field north of the town that will cause a double IRQ at the first split, causing it to place the status bar high up on the screen whenever you're standing in that spot. (Easy to find by just walking up and down in the field.)
Trirosmos wrote: Tue Nov 01, 2022 10:25 am

Code: Select all

reg [2:0] a12_delay_line;
wire a12 = a12_delay_line == 0 && chrain[12];

always @(posedge m2) begin
    a12_delay_line <= {a12_delay_line[1:0],chrain[12]};
end
I replaced the snippet in my OP with this. It seemed to create larger problems. It caused SMB3's bottom bar to bounce wildly (about 0-8 lines early). I tried different sizes for a12_delay_line. At 5:0 it became stable for SMB3. However, at 8:0 it was bouncing again (but higher up)... so, increasing the filter length alone doesn't seem to help.

Crystalis is even worse. It bounces just like SMB3 but I couldn't find any delay amount that worked for it.

Also tried negedge m2... which made Crystalis perfect at 2:0 but SMB3 bounces... but increasing the filter causes Crystalis to bounce.

I also tried the second suggestion but this line gives an error:

Code: Select all

    if(!count | reload) count <= irqlatch;

The logic for <count> does not match a known FF or Latch template. The description style you are using to describe a register or latch is not supported in the current software release.

So... I guess this MMC3 mapper IRQ needs more work. The version in my OP seems to make most games happy, the occasional high-status-bar flash in Crystalis is the only problem I've been able to notice with it. Replacing just the delay line code in the way suggested doesn't seem to be a correct solution by itself, but maybe it's badly interacting with some other assumed timings in the IRQ implementation.
User avatar
rainwarrior
Posts: 8732
Joined: Sun Jan 22, 2012 12:03 pm
Location: Canada
Contact:

Re: PowerPak MMC3 jitter fix for loopy's mapper

Post by rainwarrior »

Hmm, I tried inverting the logic:

Code: Select all

    reg [2:0] a12_delay_line;
    wire a12 = a12_delay_line != 0 || chrain[12];

    always @(negedge m2) begin
        a12_delay_line <= {a12_delay_line[1:0],chrain[12]};
    end
i.e. I think this means: filtered-a12 can't go low until A12 has been low for long enough.

Is that correct? It seems to be passing both SMB3 and Crystalis now! Trying a few other games...

Edit: Nope, now Recca has a partially glitchy title screen logo, and a bouncy status bar. :(
User avatar
rainwarrior
Posts: 8732
Joined: Sun Jan 22, 2012 12:03 pm
Location: Canada
Contact:

Re: PowerPak MMC3 jitter fix for loopy's mapper

Post by rainwarrior »

Okay, here's a version that's a bit better.

Download:
loopy_mmc3_fix2.zip
(25.48 KiB) Downloaded 26 times
What I did was filter it so that both ways are delayed for stability. Needs a 000 or 111 to transition the filtered A12:

Code: Select all

    reg [2:0] a12_delay_line;
    always @(negedge m2) begin
        a12_delay_line <= {a12_delay_line[1:0],chrain[12]};
    end

    reg a12;
    wire a12d0 = a12_delay_line == 3'b000;
    wire a12d1 = a12_delay_line == 3'b111;
    always@(posedge a12d0, posedge a12d1) begin
        if (a12d0) a12 <= 0;
        else if (a12d1) a12 <= 1;
    end
I noticed no issues in a short play of SMB3, Crystalis, and Recca, Sword Master, Tecmo Super Bowl.

Startropics seems to have issues, though. Many of the sprites seem to flicker for no reason, and the status bar palette change looks like it's too late and creeps into the frame a little on the left. :( I wonder what's affecting the sprites though, that's really odd.

To be clear, if I had to choose between a hardware-accurate mapper and a robust mapper, I would take the latter. I'm trying to make something that runs existing games well and without problems. There is of course a place for a separate, more accurate version as well, if these things are in opposition... I'd like it to be as accurate as it can be, but the primary goal is just to have a nice clean low-fuss user experience with the mapper.

I think ideally I'd like to be able to duplicate what TheFox did in the PowerMappers, but all I have to go on is this vague description:
PowerMappers RREADME wrote:

Code: Select all

- v23 (2015-12-28)
  * New MMC3 IRQ implementation (RAMBO-1 style, based on M2). Doesn't
  exactly match real MMC3 timings, but should be more robust than the
  previous implementation.
PowerMappers seem to have the same problem with Startropics, FWIW, though it might be slightly less severe?

Bunnyboy's v1.34 PowerPak MMC3 mapper didn't have a jitter problem... though it can't run Startropics, and Crystalis has a weird scroll-seam issue (not sure if IRQ related... it seems like maybe something else). I dunno how that was implemented either though.

Also, aaaargh, the first fixed version I posted above has no problems with Startropics. :S Do I have to chose between Startropics and Crystalis...?

Anyway, this seems the best I can do for today.
User avatar
rainwarrior
Posts: 8732
Joined: Sun Jan 22, 2012 12:03 pm
Location: Canada
Contact:

Re: PowerPak MMC3 jitter fix for loopy's mapper

Post by rainwarrior »

Attempt #3. Going back to the previous filter, but rewriting the reload logic.

Download:
loopy_mmc3_fix3.zip
(25.39 KiB) Downloaded 23 times

Code: Select all

 // old version
 
    wire rset=nesprg_we & m2_n & {prgain[15:13],prgain[0]}==4'b1101;
    reg reload;
    always@(posedge a12, posedge rset)
        if(rset) reload<=1;
        else reload<=0;

    reg [7:0] count;
    always@(posedge a12) begin
        if(!count | reload)
            count<=irqlatch;
        else
            count<=count-1;
    end

// new version

    reg reload;
    reg [7:0] count;

    wire reload_ack = (count == irqlatch);
    wire rset=nesprg_we & m2_n & {prgain[15:13],prgain[0]}==4'b1101;

    always@(posedge reload_ack, posedge rset)
        if (reload_ack)
            reload <= 0;
        else
            reload <= 1;
What was bothering me about the old reload was that it gets cleared by posedge a12, which is also how the counter gets clocked. It felt like there might be a timing conflict between the two.

Anyway, this version looks perfect in all the mentioned games. Startropics remainds an outlier, but is better here than before.

StarTropics has a flickering Yoyo sprite in the status bar, and on the world map the player sprite tends to flicker a little. It's less severe than in the other versions (or TheFox's version)... I feel like there might be some bus conflict with the CHR banking or something that is interfering with the sprites somehow... but the severity seems to change every time I modify the mapper. :S
User avatar
rainwarrior
Posts: 8732
Joined: Sun Jan 22, 2012 12:03 pm
Location: Canada
Contact:

Re: PowerPak MMC3 jitter fix for loopy's mapper

Post by rainwarrior »

Attempt #4. While investigating StarTropics, I was experimenting with delays on the filtered A12, and I noticed an interesting behaviour... which seemed to match exactly how it looks on the original cartridge.

Download:
loopy_mmc3_fix4.zip
(25.32 KiB) Downloaded 25 times
So, this version delays A12 by 1 fall of M2, and requires 4 M2 in a row of 0 to clear. It looks like this now:

Code: Select all

    parameter A12_FILTER = 4; // length of filter in M2 clocks
    parameter A12_DELAY  = 1; // delay of A12 rise in M2 clocks
    reg [A12_FILTER-1:0] a12_delay_line;
    wire a12 = (a12_delay_line[A12_FILTER-1:A12_DELAY] != 0);
    always @(negedge m2) begin
        a12_delay_line <= {a12_delay_line[A12_FILTER-2:0],chrain[12]};
    end

    reg reload;
    reg [7:0] count;

    wire reload_ack = (count == irqlatch);
    wire rset=nesprg_we & m2_n & {prgain[15:13],prgain[0]}==4'b1101;

    always@(posedge reload_ack, posedge rset)
        if (reload_ack)
            reload <= 0;
        else
            reload <= 1;

    always@(posedge a12) begin
        if(!count | reload)
            count<=irqlatch;
        else
            count<=count-1;
    end

    wire irqtrip=~|count;
    wire irqclear=nesprg_we & m2_n & {prgain[15:13],prgain[0]}==4'b1110;
    always@(posedge irqtrip, posedge irqclear)
        if(irqclear) irq<=0;
        else irq<=irqen;
This seems to work perfectly for SMB3, Crystalis, Recca, Sword Master, and finally StarTropics stops having sprite corruption issues (except the one that specifically does happen on the real cart).

...Tecmo Super Bowl has a small problem. During the introduction sequence, when it scrolls down on the sky and stadium scene, when it hits the bottom of that scroll the stadium seems to jitter up and down by 1 line on alternating frames.

So, that's the widest success and mildest failure I've got so far.

I now have the "gated clock" warning on all of "irqtrip", "a12", and "rset". Still not sure if it matters.

I did find Blargg's mmc3_test_2.zip, and there are some indicated failures there... but I haven't yet been able to interpret what to do about the errors. I had much less successful versions of the mapper that passed more of these tests. :S
User avatar
rainwarrior
Posts: 8732
Joined: Sun Jan 22, 2012 12:03 pm
Location: Canada
Contact:

Re: PowerPak MMC3 jitter fix for loopy's mapper

Post by rainwarrior »

Attempt #5. Now all the games I mentioned, plus a bunch of others, appear to be flawless. It passes some of Blargg's test ROMs, but not all.

Download:
loopy_mmc3_fix5.zip
(25.17 KiB) Downloaded 23 times
.

So, I consolidated the IRQ a bit, trying to make some of the state interactions less complicated.

Code: Select all

    parameter A12_FILTER = 4; // length of filter in M2 clocks
    parameter A12_DELAY  = 1; // delay of A12 rise in M2 clocks
    reg [A12_FILTER-1:0] a12_delay_line;
    wire a12 = (a12_delay_line[A12_FILTER-1:A12_DELAY] != 0);
    always @(negedge m2) begin
        a12_delay_line <= {a12_delay_line[A12_FILTER-2:0],chrain[12]};
    end

    wire write_c001 = nesprg_we & m2_n & {prgain[15:13],prgain[0]}==4'b1101;
    wire write_e000 = nesprg_we & m2_n & {prgain[15:13],prgain[0]}==4'b1110;

    reg reload;
    reg [7:0] count;

    always@(posedge a12, posedge write_c001, posedge write_e000) begin
        if(write_c001)
            reload <= 1;
        else if(write_e000)
            irq <= 0;
        else if(count == 0) begin
            reload <= 0;
            count <= irqlatch;
        end else if(reload) begin
            reload <= 0;
            count <= irqlatch;
            irq <= irqen & (irqlatch == 0);
        end else if (count == 1) begin
            count <= 0;
            irq <= irqen;
        end else
            count <= count - 1;
    end
It passes the first 3 of Blargg's test ROMs, but 4-scanline_timing gives a no IRQ error on its #2 test (I had to modify the ROM to find out which test was having no-IRQ). I'm not sure what causes its IRQ to be missed.

Otherwise, I am wondering if this consolidated procedural block would cause a write to C001 or E000 at the exact same time as A12 posedge to cancel the clock for that scanline... but it seems unlikely for this case to come up, given that these things would normally be written in NMI or in response to the IRQ itself. Without consolidating, I otherwise couldn't figure out a way to control "irq" and "reload" in a coordinated way with the clocking of "count". (This could just be due to my limited experience with Verilog.) Still getting the "Gated clock" warning with a12 in this version, also.

Anyway, the results seem good with actual games. It'd maybe be nice to pass Blargg's tests too, but I can't actually think of a way to proceed with them any further right now. Everything I tried just seemed to cause new errors in the first 3 tests.


Edit: A few more games tested revealed some minor errors. Jurassic Park title screen wavy Ocean logo sparkles noisily. Star Trek 25th Anniversary has a glitched frame dialog scenes (requires specific MMC3 variant behaviour?)
Trirosmos
Posts: 50
Joined: Mon Aug 01, 2016 4:01 am
Location: Brinstar, Zebes
Contact:

Re: PowerPak MMC3 jitter fix for loopy's mapper

Post by Trirosmos »

lidnariq wrote: Tue Nov 01, 2022 11:03 am
Trirosmos wrote: Tue Nov 01, 2022 10:25 am I believe an implementation closer to how the MMC3 handles it would be:
Rereading Furrtek's schematic, I think it's negedge
M2 is inverted on the PowerPak.
Trirosmos
Posts: 50
Joined: Mon Aug 01, 2016 4:01 am
Location: Brinstar, Zebes
Contact:

Re: PowerPak MMC3 jitter fix for loopy's mapper

Post by Trirosmos »

rainwarrior wrote: Tue Nov 01, 2022 4:09 pm To be clear, if I had to choose between a hardware-accurate mapper and a robust mapper, I would take the latter. I'm trying to make something that runs existing games well and without problems. There is of course a place for a separate, more accurate version as well, if these things are in opposition... I'd like it to be as accurate as it can be, but the primary goal is just to have a nice clean low-fuss user experience with the mapper.
An accurate mapper should be robust as far as licensed games are concerned. If it isn't, then the issue probably lies in the PowerPak itself.
If using the correct A12 filter leads to spurious triggering, then one approach might be to pre-filter chrain[12] (which is what the EverDrive does, btw):

Code: Select all

reg a12_pre_filtered;
reg [1:0] a12_pre_delay_line;

always @(posedge clk20) begin
    a12_pre_delay_line <= {a12_pre_delay_line[0],chrain[12]};

    if(a12_pre_delay_line == 3) a12_pre_filtered <= 1;
    if(a12_pre_delay_line == 0) a12_pre_filtered <= 0;
end

reg [2:0] a12_delay_line;
wire a12 = a12_delay_line == 0 && a12_pre_filtered;

always @(posedge m2) begin
    a12_delay_line <= {a12_delay_line[1:0],a12_pre_filtered};
end
This should ignore any transition faster than 100 ns. The pre-filter can be tweaked to work with the PowerPak while keeping the correct IRQ logic.
User avatar
rainwarrior
Posts: 8732
Joined: Sun Jan 22, 2012 12:03 pm
Location: Canada
Contact:

Re: PowerPak MMC3 jitter fix for loopy's mapper

Post by rainwarrior »

Trirosmos wrote: Wed Nov 02, 2022 3:45 amM2 is inverted on the PowerPak.
Okay, thanks for that clarification. That does explain why loopy's mappers are always using posedge. I will put this in my notes.

Ultimately the only thing I'm changing about loopy's original mapper related to direct use of M2 is the A12 filter.

As far as the filter goes, you can see what I settled on in attempt #5 above. I tried many combinations of filter length and pre-delay, and negedge vs. posedge. That version seems to work the best out of everything I tried.

The main thing the filter should be affecting is the specific onset timing. Unfortunately I don't know if anyone has made a test ROM that can measure MMC3 IRQ timing directly (Blargg's tests seem agnostic to a little bit of delay), but the real MMC3 carts I have on hand vs. this implementation look visually identical to me in terms of horizontal positioning. In particular the startropics glitch I discovered is faithfully reproduced, and that one seems to rest on a knife edge where once cycle earlier or later I get something different.

So, it isn't quite the filter you suggested in your first post on this thread. I did try it like that, but never found a successful result with it.

Otherwise the remaining known issues with attempt #5:
  • Jurassic Park's title screen has some wrong-looking scanlines in the wobbly Ocean logo. It doesn't seem to be jittering up or down at all, just some of the lines are erratically placed, like their scroll is wrong I guess.
  • Passes the first 3 of blargg's test ROMs. Rom 4-scanline_timing gets a no-IRQ error in its #2 test, and I'm not sure why.
  • Rom 5-MMC3 or 6-MMC3_alt do not pass either. Ultimately I guess one or the other needs to be chosen as the default behaviour. It sounds like Star Trek 25th anniversary might require the alt behaviour based on the tricky-to-emulate games article.
I assume the Blargg test issues at least should be due to the logic portion and not the filter. I'm finding that trickier to accomplish, but I'm not well versed in how to step around the limitations involved with the procedural blocks. Nintendulator seems to be the only emulator that passes these tests (and it also seems to have chosen MMC3 alt).
User avatar
rainwarrior
Posts: 8732
Joined: Sun Jan 22, 2012 12:03 pm
Location: Canada
Contact:

Re: PowerPak MMC3 jitter fix for loopy's mapper

Post by rainwarrior »

Attempt #6 fixes the Jurassic Park issue, thanks to Trirosmos.

Download:
loopy_mmc3_fix6.zip
(25.54 KiB) Downloaded 27 times
Trirosmos wrote: Wed Nov 02, 2022 4:36 amIf using the correct A12 filter leads to spurious triggering, then one approach might be to pre-filter chrain[12] (which is what the EverDrive does, btw):
(code)
Ah, I put this in and it works great!

So instead of the M2-based pre-filter I had made, yours uses the PowerPak's itnernal 20MHz clock! I hadn't thought of that as an option.

This appears to fix the issue with Jurassic Park!

Everything else I'm testing so far looks the same. Same Blargg test results, and still probably need that "MMC3_alt" behaviour for Star Trek 25th Anniversary.
User avatar
rainwarrior
Posts: 8732
Joined: Sun Jan 22, 2012 12:03 pm
Location: Canada
Contact:

Re: PowerPak MMC3 jitter fix for loopy's mapper

Post by rainwarrior »

Attempt #7. I've found zero issues testing with many games.

Download:
loopy_mmc3_fix7.zip
(25.48 KiB) Downloaded 25 times
This last revision lets a reload of 0 fire the IRQ, which makes Star Trek 25th Anniversary happy:

Code: Select all

    wire write_c001 = nesprg_we & m2_n & {prgain[15:13],prgain[0]}==4'b1101;
    wire write_e000 = nesprg_we & m2_n & {prgain[15:13],prgain[0]}==4'b1110;

    reg reload;
    reg [7:0] count;

    always@(posedge a12, posedge write_c001, posedge write_e000) begin
        if(write_c001)
            reload <= 1;
        else if(write_e000)
            irq <= 0;
        else if(reload | (count == 0)) begin
            reload <= 0;
            count <= irqlatch;
            irq <= irqen & (irqlatch == 0);
        end else if (count == 1) begin
            count <= 0;
            irq <= irqen;
        end else
            count <= count - 1;
    end
Still not fully passing Blargg's tests. They give the following reports:
  • 1-clocking: #4 Writing to $C000 shouldn't cause reload
  • 4-scanline: #3 Scanline 0 IRQ should occur sooner when $2000=$08
  • 6-MMC3_alt: #2 IRQ shouldn't be set when reloading to 0 due to counter naturally reaching 0 previously.
However, I'm at a point where the description of these tests doesn't seem to match my attempted alterations to the logic. :S With reload not getting an IRQ, it passed 1-clocking, but Star Trek needed it.

So, maybe another rewrite of the logic with a different approach could get it together, but I don't have a good idea how to proceed at the moment. So... unless there are some new ideas, maybe this is a fine place to leave it. It seems perfect with every game I tested, and it fixes issues found in every other available MMC3 PowerPak mapper (v1.43, Loopy's version, TheFox's PowerMappers).
Post Reply