Skip to content
Snippets Groups Projects

OH Cluster finding updates

Merged Andrew Peck requested to merge cluster-tb into devel

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
Edited by Andrew Peck

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Andrew Peck added 18 commits

    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

    Compare with previous version

  • Andrew Peck changed the description

    changed the description

  • Andrew Peck added 2 commits

    added 2 commits

    • 751d30dc - oh: use different sorters for ge11 vs. ge21
    • 1ba7a86e - oh: fix out of bounds loop

    Compare with previous version

  • Hi @apeck, thank you for all the changes in this MR. :smiley:

    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!

  • Andrew Peck added 51 commits

    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

    Compare with previous version

  • Author Owner

    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 Peck
  • Andrew Peck added 48 commits

    added 48 commits

    Compare with previous version

  • Andrew Peck added 3 commits

    added 3 commits

    • 943be1f2 - oh: add a pipeline register to the new sorter
    • 097cf55f - oh: refactor cluster sorting into its own module
    • e0a9dc08 - oh: add a pipeline stage to the v2 sorter

    Compare with previous version

  • Andrew Peck added 1 commit

    added 1 commit

    • e7538d69 - oh: update ge21 iodelay frequency

    Compare with previous version

  • Andrew Peck changed the description

    changed the description

  • Andrew Peck changed the description

    changed the description

  • Andrew Peck added 7 commits

    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

    Compare with previous version

  • Andrew Peck changed the description

    changed the description

  • Andrew Peck changed title from Cluster finding test bench updates + improvements to OH Cluster finding updates

    changed title from Cluster finding test bench updates + improvements to OH Cluster finding updates

  • Andrew Peck changed the description

    changed the description

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
Please register or sign in to reply
Loading