From 287d1f2bdd3717ca0931d364ed13ab6026d7672c Mon Sep 17 00:00:00 2001 From: Philippe Sauter Date: Fri, 11 Oct 2024 13:22:35 +0200 Subject: [PATCH 1/3] test: add cdc_4phase_tb adapted from cdc_2phase_tb --- Bender.yml | 1 + test/cdc_4phase_tb.sv | 267 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 268 insertions(+) create mode 100644 test/cdc_4phase_tb.sv diff --git a/Bender.yml b/Bender.yml index f5a5a21b..6e01d863 100644 --- a/Bender.yml +++ b/Bender.yml @@ -123,6 +123,7 @@ sources: - test/addr_decode_tb.sv - test/cb_filter_tb.sv - test/cdc_2phase_tb.sv + - test/cdc_4phase_tb.sv - test/cdc_2phase_clearable_tb.sv - test/cdc_fifo_tb.sv - test/cdc_fifo_clearable_tb.sv diff --git a/test/cdc_4phase_tb.sv b/test/cdc_4phase_tb.sv new file mode 100644 index 00000000..14178707 --- /dev/null +++ b/test/cdc_4phase_tb.sv @@ -0,0 +1,267 @@ +// Copyright 2024 ETH Zurich and University of Bologna. +// +// Copyright and related rights are licensed under the Solderpad Hardware +// License, Version 0.51 (the "License"); you may not use this file except in +// compliance with the License. You may obtain a copy of the License at +// http://solderpad.org/licenses/SHL-0.51. Unless required by applicable law +// or agreed to in writing, software, hardware and materials distributed under +// this License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR +// CONDITIONS OF ANY KIND, either express or implied. See the License for the +// specific language governing permissions and limitations under the License. +// +// Philippe Sauter + +module cdc_4phase_tb; + + parameter int UNTIL = 10_000_000; + parameter bit INJECT_DELAYS = 1; + parameter bit POST_SYNTHESIS = 0; + + time tck_src = 10ns; + time tck_dst = 10ns; + bit src_done = 0; + bit dst_done = 0; + bit done; + assign done = src_done & dst_done; + + // Signals of the design under test. + logic src_rst_ni = 1; + logic src_clk_i = 0; + logic [31:0] src_data_i = 0; + logic src_valid_i = 0; + logic src_ready_o; + + logic dst_rst_ni = 1; + logic dst_clk_i = 0; + logic [31:0] dst_data_o; + logic dst_valid_o; + logic dst_ready_i = 0; + + // Instantiate the design under test. + if (POST_SYNTHESIS) begin : g_dut + cdc_4phase_synth i_dut (.*); + end else if (INJECT_DELAYS) begin : g_dut + cdc_4phase_tb_delay_injector #(0.8ns) i_dut (.*); + end else begin : g_dut + cdc_4phase #(logic [31:0]) i_dut (.*); + end + + // Mailbox with expected items on destination side. + mailbox #(int) dst_mbox = new(); + int num_sent = 0; + int num_received = 0; + int num_failed = 0; + + // Clock generators. + initial begin + static int num_items, num_clks; + num_items = 10; + num_clks = 0; + #10ns; + src_rst_ni = 0; + #10ns; + src_rst_ni = 1; + #10ns; + while (!done) begin + src_clk_i = 1; + #(tck_src/2); + src_clk_i = 0; + #(tck_src/2); + + // Modulate the clock frequency. + num_clks++; + if (num_sent >= num_items && num_clks > 10) begin + num_items = num_sent + 10; + num_clks = 0; + tck_src = $urandom_range(1000, 100000) * 1ps; + assert(tck_src > 0); + end + end + end + + initial begin + static int num_items, num_clks; + num_items = 10; + num_clks = 0; + #10ns; + dst_rst_ni = 0; + #10ns; + dst_rst_ni = 1; + #10ns; + while (!done) begin + dst_clk_i = 1; + #(tck_dst/2); + dst_clk_i = 0; + #(tck_dst/2); + + // Modulate the clock frequency. + num_clks++; + if (num_received >= num_items && num_clks > 10) begin + num_items = num_received + 10; + num_clks = 0; + tck_dst = $urandom_range(1000, 100000) * 1ps; + assert(tck_dst > 0); + end + end + end + + // Source side sender. + task src_cycle_start; + #(tck_src*0.8); + endtask + + task src_cycle_end; + @(posedge src_clk_i); + endtask + + initial begin + @(negedge src_rst_ni); + @(posedge src_rst_ni); + repeat(3) @(posedge src_clk_i); + for (int i = 0; i < UNTIL; i++) begin + static integer stimulus; + stimulus = $random(); + src_data_i <= #(tck_src*0.2) stimulus; + src_valid_i <= #(tck_src*0.2) 1; + dst_mbox.put(stimulus); + num_sent++; + src_cycle_start(); + while (!src_ready_o) begin + src_cycle_end(); + src_cycle_start(); + end + src_cycle_end(); + src_valid_i <= #(tck_src*0.2) 0; + end + src_done = 1; + $display("src done"); + end + + // Destination side receiver. + task dst_cycle_start; + #(tck_dst*0.8); + endtask + + task dst_cycle_end; + @(posedge dst_clk_i); + endtask + + initial begin + @(negedge dst_rst_ni); + @(posedge dst_rst_ni); + repeat(3) @(posedge dst_clk_i); + while (!src_done || dst_mbox.num() > 0) begin + static integer expected, actual; + static int cooldown; + dst_ready_i <= #(tck_dst*0.2) 1; + dst_cycle_start(); + while (!dst_valid_o && !src_done) begin + dst_cycle_end(); + dst_cycle_start(); + end + if(src_done) + break; + actual = dst_data_o; + num_received++; + if (dst_mbox.num() == 0) begin + $error("unexpected transaction: data=%0h", actual); + num_failed++; + end else begin + dst_mbox.get(expected); + if (actual !== expected) begin + $error("transaction mismatch: exp=%0h, act=%0h", expected, actual); + num_failed++; + end + end + if(num_received%10000 == 0) begin + $display("transaction nr %d", num_received); + end + dst_cycle_end(); + dst_ready_i <= #(tck_dst*0.2) 0; + + // Insert a random cooldown period. + cooldown = $urandom_range(0, 40); + if (cooldown < 20) repeat(cooldown) @(posedge dst_clk_i); + end + $display("dst done"); + + if (num_sent != num_received) begin + $error("%0d items sent, but %0d items received", num_sent, num_received); + end + if (num_failed > 0) begin + $error("%0d/%0d items mismatched", num_failed, num_sent); + end else begin + $info("%0d items passed", num_sent); + end + dst_done = 1; + end + +endmodule + + +module cdc_4phase_tb_delay_injector #( + parameter time MAX_DELAY = 0ns +)( + input logic src_rst_ni, + input logic src_clk_i, + input logic [31:0] src_data_i, + input logic src_valid_i, + output logic src_ready_o, + + input logic dst_rst_ni, + input logic dst_clk_i, + output logic [31:0] dst_data_o, + output logic dst_valid_o, + input logic dst_ready_i +); + + logic async_req_o, async_req_i; + logic async_ack_o, async_ack_i; + logic [31:0] async_data_o, async_data_i; + + always @(async_req_o) begin + automatic time d = $urandom_range(0, MAX_DELAY); + async_req_i <= #d async_req_o; + end + + always @(async_ack_o) begin + automatic time d = $urandom_range(0, MAX_DELAY); + async_ack_i <= #d async_ack_o; + end + + for (genvar i = 0; i < 32; i++) begin + always @(async_data_o[i]) begin + automatic time d = $urandom_range(0, MAX_DELAY); + async_data_i[i] <= #d async_data_o[i]; + end + end + + cdc_4phase_src #( + .T(logic [31:0]), + .DECOUPLED(1'b0) + ) i_src ( + .rst_ni ( src_rst_ni ), + .clk_i ( src_clk_i ), + .data_i ( src_data_i ), + .valid_i ( src_valid_i ), + .ready_o ( src_ready_o ), + .async_req_o ( async_req_o ), + .async_ack_i ( async_ack_i ), + .async_data_o ( async_data_o ) + ); + + cdc_4phase_dst #( + .T(logic [31:0]), + .DECOUPLED(1'b0) + ) i_dst ( + .rst_ni ( dst_rst_ni ), + .clk_i ( dst_clk_i ), + .data_o ( dst_data_o ), + .valid_o ( dst_valid_o ), + .ready_i ( dst_ready_i ), + .async_req_i ( async_req_i ), + .async_ack_o ( async_ack_o ), + .async_data_i ( async_data_i ) + ); + +endmodule From 7d3af4b11bdac0376e410c18685c51f1495ddec4 Mon Sep 17 00:00:00 2001 From: Philippe Sauter Date: Fri, 11 Oct 2024 13:24:23 +0200 Subject: [PATCH 2/3] test: fix cdc_2phase_clearable_tb Previously this used some old (development?) version of the DUT module. Now it uses the same implementation as is currently used. Todo: This should test the module, not a copy of the module. --- test/cdc_2phase_clearable_tb.sv | 104 +++++++++++++++++++++----------- 1 file changed, 70 insertions(+), 34 deletions(-) diff --git a/test/cdc_2phase_clearable_tb.sv b/test/cdc_2phase_clearable_tb.sv index 5ca8fc8b..9dd9c3f4 100644 --- a/test/cdc_2phase_clearable_tb.sv +++ b/test/cdc_2phase_clearable_tb.sv @@ -13,7 +13,7 @@ module cdc_2phase_clearable_tb; - parameter int UNTIL = 100000; + parameter int UNTIL = 10_000_000; parameter bit INJECT_DELAYS = 1; parameter int CLEAR_PPM = 2000; parameter int SYNC_STAGES = 3; @@ -308,11 +308,16 @@ module cdc_2phase_clearable_tb_delay_injector #( logic async_ack_o, async_ack_i; logic [31:0] async_data_o, async_data_i; - - logic s_src_clear; + logic s_src_clear_req; + logic s_src_clear_ack_q; logic s_src_ready; - logic s_dst_clear; + logic s_src_isolate_req; + logic s_src_isolate_ack_q; + logic s_dst_clear_req; + logic s_dst_clear_ack_q; logic s_dst_valid; + logic s_dst_isolate_req; + logic s_dst_isolate_ack_q; always @(async_req_o) begin automatic time d = $urandom_range(1ps, MAX_DELAY); @@ -332,49 +337,80 @@ module cdc_2phase_clearable_tb_delay_injector #( end cdc_2phase_src_clearable #(logic [31:0], SYNC_STAGES) i_src ( - .rst_ni ( src_rst_ni ), - .clk_i ( src_clk_i ), - .clear_i ( s_src_clear ), - .data_i ( src_data_i ), - .valid_i ( src_valid_i & !s_src_clear ), - .ready_o ( s_src_ready ), - .async_req_o ( async_req_o ), - .async_ack_i ( async_ack_i ), - .async_data_o ( async_data_o ) + .rst_ni ( src_rst_ni ), + .clk_i ( src_clk_i ), + .clear_i ( s_src_clear_req ), + .data_i ( src_data_i ), + .valid_i ( src_valid_i & !s_src_isolate_req ), + .ready_o ( s_src_ready ), + .async_req_o ( async_req_o ), + .async_ack_i ( async_ack_i ), + .async_data_o ( async_data_o ) ); - assign src_ready_o = s_src_ready & !s_src_clear; + assign src_ready_o = s_src_ready & !s_src_isolate_req; cdc_2phase_dst_clearable #(logic [31:0], SYNC_STAGES) i_dst ( - .rst_ni ( dst_rst_ni ), - .clk_i ( dst_clk_i ), - .clear_i ( s_dst_clear ), - .data_o ( dst_data_o ), - .valid_o ( s_dst_valid ), - .ready_i ( dst_ready_i & !s_dst_clear ), - .async_req_i ( async_req_i ), - .async_ack_o ( async_ack_o ), - .async_data_i ( async_data_i ) + .rst_ni ( dst_rst_ni ), + .clk_i ( dst_clk_i ), + .clear_i ( s_dst_clear_req ), + .data_o ( dst_data_o ), + .valid_o ( s_dst_valid ), + .ready_i ( dst_ready_i & !s_dst_isolate_req ), + .async_req_i ( async_req_i ), + .async_ack_o ( async_ack_o ), + .async_data_i ( async_data_i ) ); - assign dst_valid_o = s_dst_valid & !s_dst_clear; + assign dst_valid_o = s_dst_valid & !s_dst_isolate_req; // Synchronize the clear and reset signaling in both directions (see header of // the cdc_reset_ctrlr module for more details.) cdc_reset_ctrlr #( .SYNC_STAGES(SYNC_STAGES-1) ) i_cdc_reset_ctrlr ( - .a_clk_i ( src_clk_i ), - .a_rst_ni ( src_rst_ni ), - .a_clear_i ( src_clear_i ), - .a_clear_o ( s_src_clear ), - .b_clk_i ( dst_clk_i ), - .b_rst_ni ( dst_rst_ni ), - .b_clear_i ( dst_clear_i ), - .b_clear_o ( s_dst_clear ) + .a_clk_i ( src_clk_i ), + .a_rst_ni ( src_rst_ni ), + .a_clear_i ( src_clear_i ), + .a_clear_o ( s_src_clear_req ), + .a_clear_ack_i ( s_src_clear_ack_q ), + .a_isolate_o ( s_src_isolate_req ), + .a_isolate_ack_i ( s_src_isolate_ack_q ), + .b_clk_i ( dst_clk_i ), + .b_rst_ni ( dst_rst_ni ), + .b_clear_i ( dst_clear_i ), + .b_clear_o ( s_dst_clear_req ), + .b_clear_ack_i ( s_dst_clear_ack_q ), + .b_isolate_o ( s_dst_isolate_req ), + .b_isolate_ack_i ( s_dst_isolate_ack_q ) ); - assign src_clear_pending_o = s_src_clear; - assign dst_clear_pending_o = s_dst_clear; + // Just delay the isolate request by one cycle. We can ensure isolation within + // one cycle by just deasserting valid and ready signals on both sides of the CDC. + always_ff @(posedge src_clk_i, negedge src_rst_ni) begin + if (!src_rst_ni) begin + s_src_isolate_ack_q <= 1'b0; + s_src_clear_ack_q <= 1'b0; + end else begin + s_src_isolate_ack_q <= s_src_isolate_req; + s_src_clear_ack_q <= s_src_clear_req; + end + end + + always_ff @(posedge dst_clk_i, negedge dst_rst_ni) begin + if (!dst_rst_ni) begin + s_dst_isolate_ack_q <= 1'b0; + s_dst_clear_ack_q <= 1'b0; + end else begin + s_dst_isolate_ack_q <= s_dst_isolate_req; + s_dst_clear_ack_q <= s_dst_clear_req; + end + end + + + assign src_clear_pending_o = s_src_isolate_req; // The isolate signal stays + // asserted during the whole + // clear sequence. + assign dst_clear_pending_o = s_dst_isolate_req; endmodule From 83e1e157785e40bfd29729032b563679632f0192 Mon Sep 17 00:00:00 2001 From: Philippe Sauter Date: Fri, 11 Oct 2024 13:28:11 +0200 Subject: [PATCH 3/3] cdc_4phase: remove synch bypass for DECOUPLED The previous implementation creates a non-synchronized path across the CDC, making it significantly more vulnerable to metastability. This does not seem to be necessary for DECOUPLED to work in the cdc_2phase_clearable (cdc_reset_ctrlr). The important thing seems to be when on the source side the ready signal is asserted, not how the destination side synchronizes data. --- src/cdc_4phase.sv | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/src/cdc_4phase.sv b/src/cdc_4phase.sv index bd4c65ae..96b08b40 100644 --- a/src/cdc_4phase.sv +++ b/src/cdc_4phase.sv @@ -298,27 +298,19 @@ module cdc_4phase_dst #( end end - if (DECOUPLED) begin : gen_decoupled - // Decouple the output from the asynchronous data bus without introducing - // additional latency by inserting a spill register - spill_register #( - .T(T), - .Bypass(1'b0) - ) i_spill_register ( - .clk_i, - .rst_ni, - .valid_i(data_valid), - .ready_o(output_ready), - .data_i(async_data_i), - .valid_o, - .ready_i, - .data_o - ); - end else begin : gen_not_decoupled - assign valid_o = data_valid; - assign output_ready = ready_i; - assign data_o = async_data_i; - end + spill_register #( + .T(T), + .Bypass(1'b0) + ) i_spill_register ( + .clk_i, + .rst_ni, + .valid_i(data_valid), + .ready_o(output_ready), + .data_i(async_data_i), + .valid_o, + .ready_i, + .data_o + ); // Output assignments. assign async_ack_o = ack_dst_q;