OH Cluster finding updates
Overall this MR reduces latency by ~1.75 bx, and relaxes timing in the cluster finder, and improves maintainability of the cluster finding with a much more robust test bench.
- Completely overhaul the cluster finding test bench
- Cluster sorting improvements:
- Replace the cluster finding sorter with a v1 optimized version. lower resource, relaxed timing (-0.5 bx)
- Replace the cluster finding sorter with a v2 even more optimized version. lower latency (-1.0 bx)
- Move the sorters to their own wrapper module
- Fix issue with TMR control of the cluster finder
- Replace the s-bit oneshot module, reduce latency but remove the disabled option for programmable deadtime (-0.25 bx)
- S-bit strip remapping is done on the 160MHz clock instead of 40MHz, reducing latency (-0.5 bx)
- Cleanup some misc cruft in the repo
- Cluster finder "phase selection" (dav bit extraction) is now automatic and set at the arrival of the first s-bit
- An "invalid cluster" trigger was added to the internal OH s-bit monitor
Merge request reports
Activity
added 18 commits
-
2a180872...1f0ad9f5 - 3 commits from branch
devel
- 0036057b - cluster finding test bench improvements
- 405f9086 - tb: fix sorting for ge21 oh
- cff0ad93 - tb: unconfuse pytest
- 267c74af - tb: cleanup cluster finder tb
- cc9bc9d3 - tb: save the bitmask in a constant
- 3308db39 - tb: add verbose mode
- 08aee245 - tb: add a routine to pulse specific channels
- 14f7c9d3 - tb: improve cluster printout
- ac7d9205 - oh: slightly tweak the strobe generator
- e8edf07c - oh: remove cluster reverse option
- fd12e38b - oh: fix clusterizer tmr control
- 1019209e - remove remnant tcl mode for hog source files
- 49abf19f - oh: save the with-sorting find clusters for later
- 8ceebf49 - oh: fix merge cruft
- 05b1822c - oh: add in a new more efficient sorter
Toggle commit list-
2a180872...1f0ad9f5 - 3 commits from branch
added 2 commits
Hi @apeck, thank you for all the changes in this MR.
Do you think there is any purpose in testing it on hardware, maybe at p5, already?
I also saw that you removed the cluster reverse option in commit e8edf07c. Although it is not necessary for operations at p5, it proved very useful on the GE1/1 integration setup to test all orientation possibilities with the OTMB (long vs short and positive vs negative end-cap). From my understanding, due to the CSC geometry, this is the only way to test the 4 combinations in b904.
Thanks!
added 51 commits
-
1ba7a86e...b9911626 - 49 commits from branch
devel
- 990aaf8a - oh: enable register stage for cluster output
- f9e80d11 - Merge remote-tracking branch 'origin/devel' into cluster-tb
-
1ba7a86e...b9911626 - 49 commits from branch
Hi Laurent,
If you have some spare cycles it would be great to test. I revived an old version of the cluster sorting module, it improves the timing slack in the sorting stage by quite a bit.
I have no idea if this will fix anything, but it was a change I made in another (latency optimization) branch.
The choice of sorter is controlled by a compile time flag, so both still exist in the firmware and can be easily toggled back and forth.
The simulation passes for both versions.
I also made some improvements to a module which takes the 40 MHz clock and extracts a 160 MHz pulse (every 4th clock cycle) that is used to transfer from 40MHz to 160MHz domain where clustering is done. It involved some interaction between the 40 and 160 MHz clock domain.. the two clocks are from the same MMCM so I think the tools should handle it correctly on their own but trying this just in case..
Since the problem shows up in the s-bit monitor, which also operates on the 40 MHz clock from the same MMCM, there should be no "real" clock domain crossings that aren't timing analyzed.. so I'm stumped.
The timing slack in the sorting was very small, 0.01 ns.. perhaps temperature / voltage variation is higher in some chambers than expected? I have no idea what else would explain something like this that only appears on some chambers besides timing issues / clock crossing..
Regarding the cluster reverse, I could put it back in if desired. I must have misunderstood, I thought before that it turned out to be unnecessary, but didn't know about the B904 special case. Is it only needed in GE11?
Edited by Andrew Peckadded 48 commits
-
f9e80d11...3d80d30f - 46 commits from branch
devel
- b957086d - oh: implement a new prototype sorter
- 07a78160 - Merge remote-tracking branch 'origin/devel' into cluster-tb
-
f9e80d11...3d80d30f - 46 commits from branch
added 7 commits
- 0c048356 - oh: add tb method that prints latency to the console
- a5d64f64 - oh: Revert "oh: remove cluster reverse option"
- f37478a3 - oh: re-run update_oh_base
- ba152e1e - oh: modify cluster finding oneshot
- 927e8450 - oh: improve cluster test bench
- 5cdde982 - oh: time in new sorter w/ correct alignment
- 2ec71f65 - oh: less verbose test bench printout
Toggle commit list