Merge pull request #2335 from nelsobe/master
Change MMCME2 advanced COMPENSATION parameter to have a default of INTERNAL
diff --git a/xc/xc7/techmap/cells_map.v b/xc/xc7/techmap/cells_map.v
index e421d5a..51554c7 100644
--- a/xc/xc7/techmap/cells_map.v
+++ b/xc/xc7/techmap/cells_map.v
@@ -4790,7 +4790,17 @@
parameter BANDWIDTH = "OPTIMIZED";
parameter STARTUP_WAIT = "FALSE";
- parameter COMPENSATION = "ZHOLD";
+
+// Previously, the default COMPENSATION value was ZHOLD, resulting in non-functional
+// bitstreams when the feedback loop is closed on-chip.
+ // Setting it to INTERNAL as the default creates working bitstreams for that case.
+// This bug was not previously uncovered since the MMCM tests all explicitly
+// specified a COMPENSATION value so the ZHOLD default was never used.
+// Setting it here in the techmapper means that existing code using on-chip
+// MMCM feedback without specifying a COMPENSATION value can be ported
+// unmodified into the toolflow and will result in functional bitstreams.
+// A test was added to test the case when relying on the default COMPENSATION value.
+ parameter COMPENSATION = "INTERNAL";
parameter CLKIN1_PERIOD = 0.0;
parameter REF_JITTER1 = 0.01;
diff --git a/xc/xc7/tests/mmcm/CMakeLists.txt b/xc/xc7/tests/mmcm/CMakeLists.txt
index 85f7f79..2eaa87a 100644
--- a/xc/xc7/tests/mmcm/CMakeLists.txt
+++ b/xc/xc7/tests/mmcm/CMakeLists.txt
@@ -57,6 +57,7 @@
add_file_target(FILE mmcme2_test.v SCANNER_TYPE verilog)
add_file_target(FILE mmcm_int_basys3_bottom.v SCANNER_TYPE verilog)
+add_file_target(FILE mmcm_none_basys3_bottom.v SCANNER_TYPE verilog)
add_file_target(FILE mmcm_buf_basys3_bottom.v SCANNER_TYPE verilog)
add_file_target(FILE mmcm_ext_basys3_bottom.v SCANNER_TYPE verilog)
add_file_target(FILE mmcm_int_frac_basys3_bottom.v SCANNER_TYPE verilog)
@@ -78,6 +79,14 @@
)
add_fpga_target(
+ NAME mmcm_none_basys3
+ BOARD basys3-bottom
+ SOURCES mmcm_none_basys3_bottom.v
+ INPUT_IO_FILE ${COMMON}/basys3_bottom_pmod.pcf
+ EXPLICIT_ADD_FILE_TARGET
+ )
+
+add_fpga_target(
NAME mmcm_buf_basys3
BOARD basys3-bottom
SOURCES mmcm_buf_basys3_bottom.v
@@ -117,6 +126,13 @@
)
add_vivado_target(
+ NAME mmcm_none_basys3_vivado
+ PARENT_NAME mmcm_none_basys3
+ CLOCK_PINS clk
+ CLOCK_PERIODS 10.0
+ )
+
+add_vivado_target(
NAME mmcm_buf_basys3_vivado
PARENT_NAME mmcm_buf_basys3
CLOCK_PINS clk
@@ -146,6 +162,7 @@
set(TEST_TARGETS
mmcm_packing
mmcm_int_basys3
+ mmcm_none_basys3
mmcm_int_frac_basys3
mmcm_buf_basys3
mmcm_ext_basys3
diff --git a/xc/xc7/tests/mmcm/README.md b/xc/xc7/tests/mmcm/README.md
index 3b10962..2aecf82 100644
--- a/xc/xc7/tests/mmcm/README.md
+++ b/xc/xc7/tests/mmcm/README.md
@@ -20,6 +20,7 @@
There are 3 test variants:
- `mmcm_int_basys3` - Internal feedback
+- `mmcm_int_basys3` - Internal feedback but no COMPENSATION specified in order to test default value in techmapper
- `mmcm_buf_basys3` - Feedback through a BUFG
- `mmcm_ext_basys3` - External feedback. Need to short `JC.1` and `JC.2` on the Basys3 board.
diff --git a/xc/xc7/tests/mmcm/mmcm_none_basys3_bottom.v b/xc/xc7/tests/mmcm/mmcm_none_basys3_bottom.v
new file mode 100644
index 0000000..e51f45f
--- /dev/null
+++ b/xc/xc7/tests/mmcm/mmcm_none_basys3_bottom.v
@@ -0,0 +1,62 @@
+`include "mmcme2_test.v"
+
+`default_nettype none
+
+// ============================================================================
+
+module top
+(
+input wire clk,
+
+input wire [7:0] sw,
+output wire [7:0] led,
+
+input wire jc1, // unused
+output wire jc2,
+input wire jc3, // unused
+input wire jc4
+);
+
+// ============================================================================
+// Reset generator
+wire CLK;
+BUFG bufg(.I(clk), .O(CLK));
+
+reg [3:0] rst_sr;
+initial rst_sr <= 4'hF;
+
+always @(posedge CLK)
+ if (sw[0])
+ rst_sr <= 4'hF;
+ else
+ rst_sr <= rst_sr >> 1;
+
+wire RST = rst_sr[0];
+
+// ============================================================================
+// The tester
+
+mmcme2_test #
+(
+.FEEDBACK ("NONE")
+)
+mmcme2_test
+(
+.CLK (clk),
+.RST (RST),
+
+.CLKFBOUT (),
+.CLKFBIN (),
+
+.I_PWRDWN (sw[1]),
+.I_CLKINSEL (sw[2]),
+
+.O_LOCKED (led[6]),
+.O_CNT (led[5:0])
+);
+
+assign led [7] = |sw[7:3];
+assign jc2 = jc4;
+
+endmodule
+
diff --git a/xc/xc7/tests/mmcm/mmcme2_test.v b/xc/xc7/tests/mmcm/mmcme2_test.v
index 814133a..e4c9a5f 100644
--- a/xc/xc7/tests/mmcm/mmcme2_test.v
+++ b/xc/xc7/tests/mmcm/mmcme2_test.v
@@ -44,68 +44,133 @@
wire [5:0] clk;
wire [5:0] gclk;
-MMCME2_ADV #
-(
-.BANDWIDTH ("HIGH"),
-.COMPENSATION ((FEEDBACK == "EXTERNAL") ? "EXTERNAL" : "INTERNAL"),
+generate if (FEEDBACK == "NONE") begin
-.CLKIN1_PERIOD (20.0), // 50MHz
-.CLKIN2_PERIOD (10.0), // 100MHz
+ MMCME2_ADV #
+ (
+ .BANDWIDTH ("HIGH"),
-.CLKFBOUT_MULT_F (CLKFBOUT_MULT_F),
-.CLKFBOUT_PHASE (0),
+ .CLKIN1_PERIOD (20.0), // 50MHz
+ .CLKIN2_PERIOD (10.0), // 100MHz
-.CLKOUT0_DIVIDE_F (CLKOUT0_DIVIDE_F),
-.CLKOUT0_DUTY_CYCLE (0.50),
-.CLKOUT0_PHASE (45.0),
+ .CLKFBOUT_MULT_F (CLKFBOUT_MULT_F),
+ .CLKFBOUT_PHASE (0),
-.CLKOUT1_DIVIDE (32),
-.CLKOUT1_DUTY_CYCLE (0.53125),
-.CLKOUT1_PHASE (90.0),
+ .CLKOUT0_DIVIDE_F (CLKOUT0_DIVIDE_F),
+ .CLKOUT0_DUTY_CYCLE (0.50),
+ .CLKOUT0_PHASE (45.0),
-.CLKOUT2_DIVIDE (48),
-.CLKOUT2_DUTY_CYCLE (0.50),
-.CLKOUT2_PHASE (135.0),
+ .CLKOUT1_DIVIDE (32),
+ .CLKOUT1_DUTY_CYCLE (0.53125),
+ .CLKOUT1_PHASE (90.0),
-.CLKOUT3_DIVIDE (64),
-.CLKOUT3_DUTY_CYCLE (0.50),
-.CLKOUT3_PHASE (45.0),
+ .CLKOUT2_DIVIDE (48),
+ .CLKOUT2_DUTY_CYCLE (0.50),
+ .CLKOUT2_PHASE (135.0),
-.CLKOUT4_DIVIDE (80),
-.CLKOUT4_DUTY_CYCLE (0.50),
-.CLKOUT4_PHASE (90.0),
+ .CLKOUT3_DIVIDE (64),
+ .CLKOUT3_DUTY_CYCLE (0.50),
+ .CLKOUT3_PHASE (45.0),
-.CLKOUT5_DIVIDE (96),
-.CLKOUT5_DUTY_CYCLE (0.50),
-.CLKOUT5_PHASE (135.0),
+ .CLKOUT4_DIVIDE (80),
+ .CLKOUT4_DUTY_CYCLE (0.50),
+ .CLKOUT4_PHASE (90.0),
-.CLKOUT6_DIVIDE (1),
-.CLKOUT6_DUTY_CYCLE (0.50),
-.CLKOUT6_PHASE (0.0),
+ .CLKOUT5_DIVIDE (96),
+ .CLKOUT5_DUTY_CYCLE (0.50),
+ .CLKOUT5_PHASE (135.0),
-.STARTUP_WAIT ("FALSE")
-)
-mmcm
-(
-.CLKIN1 (clk50),
-.CLKIN2 (clk100),
-.CLKINSEL (I_CLKINSEL),
+ .CLKOUT6_DIVIDE (1),
+ .CLKOUT6_DUTY_CYCLE (0.50),
+ .CLKOUT6_PHASE (0.0),
-.RST (RST),
-.PWRDWN (I_PWRDWN),
-.LOCKED (O_LOCKED),
+ .STARTUP_WAIT ("FALSE")
+ )
+ mmcm
+ (
+ .CLKIN1 (clk50),
+ .CLKIN2 (clk100),
+ .CLKINSEL (I_CLKINSEL),
-.CLKFBIN (clk_fb_i),
-.CLKFBOUT (clk_fb_o),
+ .RST (RST),
+ .PWRDWN (I_PWRDWN),
+ .LOCKED (O_LOCKED),
-.CLKOUT0 (clk[0]),
-.CLKOUT1 (clk[1]),
-.CLKOUT2 (clk[2]),
-.CLKOUT3 (clk[3]),
-.CLKOUT4 (clk[4]),
-.CLKOUT5 (clk[5]),
-.CLKOUT6 () // Deliberately disconnected
-);
+ .CLKFBIN (clk_fb_i),
+ .CLKFBOUT (clk_fb_o),
+
+ .CLKOUT0 (clk[0]),
+ .CLKOUT1 (clk[1]),
+ .CLKOUT2 (clk[2]),
+ .CLKOUT3 (clk[3]),
+ .CLKOUT4 (clk[4]),
+ .CLKOUT5 (clk[5]),
+ .CLKOUT6 () // Deliberately disconnected
+ );
+end else begin
+ MMCME2_ADV #
+ (
+ .BANDWIDTH ("HIGH"),
+ .COMPENSATION ((FEEDBACK == "EXTERNAL") ? "EXTERNAL" : "INTERNAL"),
+
+ .CLKIN1_PERIOD (20.0), // 50MHz
+ .CLKIN2_PERIOD (10.0), // 100MHz
+
+ .CLKFBOUT_MULT_F (CLKFBOUT_MULT_F),
+ .CLKFBOUT_PHASE (0),
+
+ .CLKOUT0_DIVIDE_F (CLKOUT0_DIVIDE_F),
+ .CLKOUT0_DUTY_CYCLE (0.50),
+ .CLKOUT0_PHASE (45.0),
+
+ .CLKOUT1_DIVIDE (32),
+ .CLKOUT1_DUTY_CYCLE (0.53125),
+ .CLKOUT1_PHASE (90.0),
+
+ .CLKOUT2_DIVIDE (48),
+ .CLKOUT2_DUTY_CYCLE (0.50),
+ .CLKOUT2_PHASE (135.0),
+
+ .CLKOUT3_DIVIDE (64),
+ .CLKOUT3_DUTY_CYCLE (0.50),
+ .CLKOUT3_PHASE (45.0),
+
+ .CLKOUT4_DIVIDE (80),
+ .CLKOUT4_DUTY_CYCLE (0.50),
+ .CLKOUT4_PHASE (90.0),
+
+ .CLKOUT5_DIVIDE (96),
+ .CLKOUT5_DUTY_CYCLE (0.50),
+ .CLKOUT5_PHASE (135.0),
+
+ .CLKOUT6_DIVIDE (1),
+ .CLKOUT6_DUTY_CYCLE (0.50),
+ .CLKOUT6_PHASE (0.0),
+
+ .STARTUP_WAIT ("FALSE")
+ )
+ mmcm
+ (
+ .CLKIN1 (clk50),
+ .CLKIN2 (clk100),
+ .CLKINSEL (I_CLKINSEL),
+
+ .RST (RST),
+ .PWRDWN (I_PWRDWN),
+ .LOCKED (O_LOCKED),
+
+ .CLKFBIN (clk_fb_i),
+ .CLKFBOUT (clk_fb_o),
+
+ .CLKOUT0 (clk[0]),
+ .CLKOUT1 (clk[1]),
+ .CLKOUT2 (clk[2]),
+ .CLKOUT3 (clk[3]),
+ .CLKOUT4 (clk[4]),
+ .CLKOUT5 (clk[5]),
+ .CLKOUT6 () // Deliberately disconnected
+ );
+end endgenerate
generate if (FEEDBACK == "INTERNAL") begin
assign clk_fb_i = clk_fb_o;
