Report investigated paths and code changes.

applications/lofar2/doc/sdp_timing_closure.txt

+
+1) General:
+* In L2SDP-1005 the common_areset.vhd was improved with g_tree_len = 1. From Eric to Reinier in Slack 6 aug 2024:
+
+  Hoi Reinier, kun je deze voor mij reviewen https://git.astron.nl/rtsd/hdl/-/merge_requests/417 . Ik heb de
+  common_pipeline_sl vervangen door een common_async omdat dat beter past op het niveau van common_areset. Verder heb
+  ik default g_tree_len = 1 gezet voor minimale extra latency. Ik zag dat de regressie test op SDP ARTS faalde.
+  Misschien lag dat aan common_areset met te lange g_tree_len. Ik heb gezocht op waar common_areset wordt gebruikt
+  in onze code, met name in ctrl_unb2c_board.vhd, maar kon niet direct een oorzaak vinden. Missichien moet
+  default g_tree_len = 0 en dan alleen g_tree_len > 0 waar dat nodig is. Maar eerst wou ik het maar met minimale
+  g_tree_len = 1 default proberen.
+
+* ==> c_dp_sosi_rst with 'X' for data, info fields or use RESET_DP_SOSI_CTRL() [RTSD-123, RTSD-129]
+  Issue is that using 'X' or '-' in c_dp_sosi_rst causes lots of Modelsim warnings about conversion from slv to int
+
+* The PPS capture sometimes fails to meet timing. It is special because it uses also the falling edge of CLK. Sofar
+  we can ignore this because FPGA_pps_capture_cnt_R and FPGA_pps_error_cnt_R seem OK on hardware:
+
+  u_revision|u_ctrl|u_mms_ppsh|u_ppsh|u_in|u_ddio_in|gen_ip_arria10_e2sg.u0|gen_w[0].u_ip_arria10_e2sg_ddio_in_1|
+  gpio_0|core|i_loop[0].altera_gpio_bit_i|input_path.in_path_fr.buffer_data_in_fr_ddio~ddio_in_fr__nff to
+  u_revision|u_ctrl|u_mms_ppsh|u_ppsh|pps_ext_cap
+
+
+2) RTSD-124c 24 feb 2025 : Fix asynchronous reset timing violations
+
+The changes marked with DONE are sufficient to achieve timing closure for CLK 5 ns and applied in RTSD-124c merge.
+
+CLK timing violations (10000 paths in lofar2_unb2c_sdp_station_full, in disturb2_unb2c_sdp_station_full almost 10000
+fail):
+* common_areset_dp_rst/with_pipe/u_pipe/din_meta[0] to gen_cross in some MM reg
+  - u_revision|u_sdp_station|gen_use_xsub.u_xsub|u_sdp_crosslets_remote|u_mmp_dp_bsn_align_v2|
+    gen_bsn_mon_input.u_bsn_mon_input|gen_stream[6].u_reg|u_reg|gen_cross.gen_rd.u_in_vector|in_buf_reg[145]
+  - u_revision|u_sdp_station|gen_use_ring.gen_xst_ring.u_ring_lane_xst|u_ring_tx|
+    gen_bsn_monitors.u_mms_dp_bsn_monitor_v2|gen_stream[0].u_reg|u_reg|gen_cross.gen_rd.u_in_vector|in_buf_reg[35]
+  - u_revision|u_sdp_station|gen_use_bf.gen_bf[1].u_bf|u_sdp_bst_udp_offload|u_dp_offload_tx_v3|
+    gen_dp_field_blk[0].u_dp_field_blk|u_mm_fields_slv|u_common_reg_r_w_dc|gen_cross.u_out_reg|i_out_dat[1291]
+  - u_revision|u_ctrl|gen_eth.u_eth|u_rx_hdr_info|u_hdr_store|i_hdr_words_arr[3][29]
+  ==> TRIED: Use g_tree_len = 2 instead of 1 or fix it by adding pipeline to out_rst in common_reg_cross_clock_domain
+      and by using out_rst as synchronous rst. Same for in_rst, because timing violations also occur with in_rst and
+      in_buf_reg[].
+      Using g_tree_len = 2 does not help for timing closure in u_common_fifo_sc, the timing errors occur then from
+      din_meta[1].
+  ==> TRIED: Do not out_rst out_en in common_reg_cross_clock_domain
+  ==> DONE: Pipeline in_rst and out_rst in common_reg_cross_clock_domain fixes gen_cross errors
+  ==> NOT TRIED: Do not use in_rst and out_rst in common_reg_cross_clock_domain
+  ==> NOT TRIED: Use in_rst and out_rst as synchronous reset in common_reg_cross_clock_domain
+
+* common_areset_dp_rst/with_pipe/u_pipe/din_meta[0] to r.taps_out_vec in:
+ - u_revision|u_sdp_station|gen_use_fsub.gen_use_no_oversample.u_fsub|u_wpfb_unit_dev|
+   gen_pfb.gen_prefilter.u_filter|gen_fil_ppf_singles[0].u_fil_ppf_single|u_fil_ctrl|r.taps_out_vec[481]
+   ==> DONE: Do not rst r.taps_out_vec, fixes u_fil_ctrl errors
+
+* common_areset_dp_rst/with_pipe/u_pipe/din_meta[0] to u_dp_fifo_sc in:
+ - u_revision|u_sdp_station|gen_use_ring.gen_bf_ring.gen_beamset_ring[0].u_ring_lane_bf|u_ring_tx|u_dp_fifo_sc|
+   u_dp_fifo_core|gen_common_fifo_sc.u_common_fifo_sc|u_fifo|gen_ip_arria10_e2sg.u0|u_scfifo|auto_generated|
+   dpfifo|FIFOram|q_b[63]
+ - u_revision|u_sdp_station|gen_use_ring.gen_bf_ring.gen_beamset_ring[1].u_ring_lane_bf|u_ring_tx|u_dp_fifo_sc|
+   u_dp_fifo_core|gen_common_fifo_sc.u_common_fifo_sc|u_fifo|gen_ip_arria10_e2sg.u0|u_scfifo|auto_generated|
+   dpfifo|FIFOram|q_b[65]
+ - u_revision|u_sdp_station|gen_use_ring.gen_xst_ring.u_ring_lane_xst|u_ring_tx|u_dp_fifo_sc|
+   u_dp_fifo_core|gen_common_fifo_sc.u_common_fifo_sc|u_fifo|gen_ip_arria10_e2sg.u0|u_scfifo|auto_generated|
+   dpfifo|FIFOram|q_b[60]
+ - u_revision|u_sdp_station|gen_use_ring.gen_bf_ring.gen_beamset_ring[1].u_ring_lane_bf|u_ring_tx|u_dp_fifo_sc|
+   u_dp_fifo_core|gen_common_fifo_sc.u_common_fifo_sc|u_fifo|gen_ip_arria10_e2sg.u0|u_scfifo|auto_generated|
+   dpfifo|usedw_counter|counter_reg_bit[3]
+ - u_revision|u_sdp_station|gen_use_ring.gen_xst_ring.u_ring_lane_xst|u_ring_tx|u_dp_fifo_sc|
+   u_dp_fifo_core|gen_common_fifo_sc.u_common_fifo_sc|u_fifo|gen_ip_arria10_e2sg.u0|u_scfifo|auto_generated|
+   dpfifo|FIFOram|ram_block1a42~reg1
+ - u_revision|u_sdp_station|gen_use_ring.gen_xst_ring.u_ring_lane_xst|u_ring_tx|u_dp_fifo_sc|
+   u_dp_fifo_core|gen_common_fifo_sc.u_common_fifo_sc|u_fifo|gen_ip_arria10_e2sg.u0|u_scfifo|auto_generated|
+   dpfifo|FIFOram|ram_block1a19~reg1
+ ==> TRIED: pipeline rst --> in_rst --> fifo_rst in common_fifo_sc.vhd
+     Still the u_common_fifo_sc do occur, maybe due to path via wr_req input.
+ ==> TRIED: Pipeline tx_sosi in ring_tx.vhd --> pipeline not needed, only the tx_sosi.channel field has logic,
+     all other tx_sosi fields are wired.
+ ==> DONE: Pipeline tx_fifo_siso with dp_pipeline_ready_inp in dp_offload_tx_v3 in ring_tx.vhd fixes FIFOram errors.
+     For this I (Eric) added dp_pipeline_ready_inp and dp_pipeline_ready_outp to dp_offload_tx_v3.vhd.
+
+Following din_meta[0] timing paths with least slack were seen, but not further investigated because they achieved
+CLK 5 ns timing:
+
+* common_areset_dp_rst/with_pipe/u_pipe/din_meta[0] to u_dp_fifo_sc in:
+ - u_revision|u_sdp_station|gen_use_ring.gen_xst_ring.u_ring_lane_xst|u_ring_tx|u_dp_offload_tx_v3|
+   gen_dp_field_blk[0].u_dp_field_blk|u_dp_repack_data|gen_dp_repack_out.u_dp_repack_out|r.hold_out.eop
+ - u_revision|u_sdp_station|gen_use_xsub.u_xsub|u_sdp_xst_udp_offload|u_dp_offload_tx_v3|
+   gen_dp_field_blk[0].u_dp_field_blk|u_dp_repack_data|gen_dp_repack_in.u_dp_repack_in|r.src_out.bsn[29]
+ - u_revision|u_sdp_station|gen_use_xsub.u_xsub|u_sdp_crosslets_remote|u_dp_repack_data_rx|
+   gen_dp_repack_out.u_dp_repack_out|r.src_out.bsn[59]
+ - u_revision|u_sdp_station|gen_use_bf.gen_bf[0].u_bf|u_sdp_beamformer_remote|u_dp_repack_data_sum|
+   gen_dp_repack_out.u_dp_repack_out|r.src_out.channel[0]
+ - u_revision|u_sdp_station|gen_use_bf.gen_bf[1].u_bf|u_sdp_beamformer_output|u_dp_offload_tx_v3|
+   gen_packet[0].u_dp_field_blk|u_dp_repack_data|gen_dp_repack_out.u_dp_repack_out|r.src_out.empty[0]
+ - u_revision|u_sdp_station|gen_use_bf.gen_bf[1].u_bf|u_sdp_beamformer_remote|u_dp_repack_data_sum|
+   gen_dp_repack_in.u_dp_repack_in|gen_load.u_load|r.dat_bit_cnt[2]
+   ==> do not reset data fields in u_dp_repack_out
+   ==> do not reset data fields in u_dp_repack_in
+   ==> use RESET_DP_SOSI_CTRL()
+
+Some other timing paths with low slack:
+
+* u_revision|u_sdp_station|gen_use_xsub.u_xsub|u_sdp_xst_udp_offload|u_dp_offload_tx_v3|
+  gen_dp_concat[0].u_dp_concat|i_src_out.eop to
+  - u_revision|u_ctrl|gen_eth.u_eth|u_tx_mux|src_out_hi.eop
+  - u_revision|u_ctrl|gen_eth.u_eth|u_tx_mux|gen_input[2].u_hold|u_hold_sop|u_hld_ctrl|switch_level
+  ==> Put dp_pipeline_ready after dp_offload_tx_v3 in sdp_statistics_offload
+  ==> Put optional dp_pipeline_ready in gen_dp_concat
+* u_revision|u_sdp_station|gen_use_bf.gen_bf[0].u_bf|u_sdp_bst_udp_offload|u_dp_offload_tx_v3|
+  gen_dp_concat[0].u_dp_concat|i_src_out.sop to
+  - u_revision|u_sdp_station|gen_use_bf.gen_bf[1].u_bf|u_sdp_bst_udp_offload|u_dp_pipeline_ready|gen_out_rl.u_incr|
+    reg_ready[1]
+* u_revision|u_sdp_station|gen_use_bf.gen_bf[0].u_bf|u_sdp_beamformer_output|u_dp_fifo_data|u_dp_fifo_fill_eop|
+  u_dp_fifo_core|gen_common_fifo_sc.u_common_fifo_sc|rd_val to
+  - u_revision|u_sdp_station|gen_use_bf.gen_bf[0].u_bf|u_sdp_beamformer_output|u_dp_fifo_data|u_dp_fifo_fill_eop|
+    i_src_out.channel[2]
+* u_revision|u_sdp_station|u_ring_info|u_mm_fields|u_common_reg_r_w_dc|gen_cross.u_out_reg|i_out_dat[69] to
+  - u_revision|u_sdp_station|gen_use_ring.gen_bf_ring.gen_beamset_ring[0].u_ring_lane_bf|u_ring_tx|hdr_fields_in_reg[65]
+