Export jt49 output enables (used by Alpha Densi 68k), fix .qip file, fix ADPCM-A enablers for YM2610 (NeoGeo)

[gyurco]

Depends on https://github.com/jotego/jt49/pull/7

[gyurco]

There's also one outstanding issue. The following construct didn't work with ADPCM-A channel in NeoGeo: https://github.com/jotego/jt12/blob/37a00760eb20f9c5a198a779fd69f9c18eadb064/hdl/jt12_div.v#L89 Probably it doesn't like when cen_xxx and cen are out of phase.

The following patch solved it:

diff --git a/hdl/jt12_div.v b/hdl/jt12_div.v
index b9e5bc9..56fb390 100644
--- a/hdl/jt12_div.v
+++ b/hdl/jt12_div.v
@@ -86,8 +86,8 @@ always @(negedge clk) begin     // It's important to leave the negedge to use th
     clk_en     <= pre_clk_en;
     clk_en_2   <= pre_clk_en_2;
     clk_en_ssg <= pre_clk_en_ssg;
-    clk_en_666 <= pre_clk_en_666;
-    clk_en_111 <= pre_clk_en_111;
+    clk_en_666 <= cen && pre_clk_en_666;
+    clk_en_111 <= cen && pre_clk_en_111;
     clk_en_55  <= pre_clk_en_55;
 end

@@ -107,8 +107,8 @@ always @(posedge clk) begin
     pre_clk_en     <= cen & cen_int;
     pre_clk_en_2   <= cen && (div2==2'b00);
     pre_clk_en_ssg <= use_ssg ? (cen & cen_ssg_int) : 1'b0;
-    pre_clk_en_666 <= cen & cen_adpcm_int;
-    pre_clk_en_111 <= cen & cen_adpcm_int & cen_adpcm3_int;
+    pre_clk_en_666 <= cen_adpcm_int;
+    pre_clk_en_111 <= cen_adpcm_int & cen_adpcm3_int;
     pre_clk_en_55  <= cen & cen_adpcm_int & cen_adpcm3_int & cen_55_int;
     `endif
 end

BTW, why negedge is required for using the ENA input? I know it's used all over in MiSTer's code, but I didn't find any reference in FPGA manuals why it is needed. I only found some info about using it in the ASIC world for clock gating, but that's a totally different story. Checked in the netlist viewer, and looks like "cen" is happily used as ENA input without any tricks (on Cyclone III at least):

(The red wire is cen, just clipped off from the screenshot)

[jotego]

Is the cen patch coming from the MiSTer version?

About the negedge usage, when a signal gates the clock it should be latched at falling edge. The problem here is that Quartus seems to decide whether to use the cen signals as clock enable inputs in the LUT or as clock gating signals in a rather whimsical way. I read recently that it's better to generate the clock gating signal and then effectively add a clk & cen assignment to force it. I want to check whether that reduces LUT usage or not, but I haven't had the time to do it.

I'm ok with merging this but we need to solve the question about the removed jt49 mux. It changes the functionality of a pin and I'm afraid it may break something in other cores.

[gyurco]

It's basically reverting the pre_xxx version (where there are no pre_cen_xxx and cen_xxx versions). Do you think is it used as clock gating? My best knowledge is that clock gating on FPGAs are no-no, as it would affect the clock skew, and even if it's used, it requires special hardware elements, not just a simple gate. So clock enables are the common and recommended way.

I replied about the JT49 change, basically I can add a parameter to enable/disable the feedback logic.

[jotego]

I think most FPGAs have several clock gating structures, particularly at the root of each clock tree. That includes the Cyclone V series and I suppose Cyclone III. It's obvious that if you apply the signal once at the clock tree it will save resources. I just wasn't able to get consistent results a few years ago when I was trying it. I must try again, though, with the clk & cen syntax.

[gyurco]

As the number of clock networks are quite limited, and are described in the fitter output, I never seen any new global clock created from a clock-enable HDL construct. But I might be wrong here...

The most important part that ADPCM is broken with the current code in NeoGeo. Probably you can try it and hear it yourself (check Metal Slug intro for example, Gatling gun and "Metal Slug" announcement are both missing with the current jt12_div)

[jotego]

I don't maintain the code directly in the MiSTer repo. I've got a few merges from MiSTer people porting things back. Usually, anything that only affects JT10 gets accepted as I just trust them and I'm not using JT10 elsewhere.

Your point about the number of clock networks is right. So once cannot just pass everything as clock gating. But I think it will be worth using for the large modules, such as the CPUs and FM synthesizers. I think Cyclone V had 20 clock trees and a core may have 2 CPUs and one FM synthesizer so it may be a good choice to take them to clock gating structures.

[gyurco]

But now you can try it on MiST, too :) With the current jt12 (broken) or with this reverted (OK): https://github.com/jotego/jt12/commit/d4607e1ec4857c7aab4660162e2da53d02fa3890 (or just with the simple patch above)

Here's the link: https://github.com/gyurco/NeoGeo_MiSTer/tree/mist/mist

The theory about the clock gating might be OK, but as I said, I never seen any new global clock created from constructs like this. Only when a generated signal is used as clock, but that's a very different story, and should be avoided.