Fixes to allow reduced FPGA1 project to run with agreement with emulation#75
Merged
Fixes to allow reduced FPGA1 project to run with agreement with emulation#75
Conversation
aehart
reviewed
Sep 9, 2025
Comment on lines
+274
to
+293
| #horrible hack to fix order of modules | ||
| ia=-1; | ||
| ib=-1; | ||
| for mem in memList: | ||
| if "_BE" in mem.var() : | ||
| ia=memList.index(mem); | ||
| if "_BF" in mem.var() : | ||
| ib=memList.index(mem); | ||
| if ia != -1 and ib != -1 and ib < ia : | ||
| memList[ia], memList[ib] = memList[ib], memList[ia] | ||
|
|
||
| ia=-1; | ||
| ib=-1; | ||
| for mem in memList: | ||
| if "L1PHIH_BC" in mem.var() : | ||
| ia=memList.index(mem); | ||
| if "L1PHIH_BD" in mem.var() : | ||
| ib=memList.index(mem); | ||
| if ia != -1 and ib != -1 and ib < ia : | ||
| memList[ia], memList[ib] = memList[ib], memList[ia] |
Contributor
There was a problem hiding this comment.
@aryd I'm assuming the order here comes from the order in the wiring files, so should the order ultimately be fixed in the TrackletConfigBuilder in CMSSW? I'm not proposing doing this immediately, but it seems like it would be better to eventually fix the issue where it originates, if possible.
aehart
approved these changes
Sep 9, 2025
Contributor
aehart
left a comment
There was a problem hiding this comment.
Aside from my one question, which is just out of curiosity, this looks good, so I will approve and merge.
Contributor
Author
|
Andrew,
I think you are correct that this could be fixed in the TrackletConfigBuilder, though I’m not sure that we should depend on the specific order. At some point we should review this, but I think this would go with a bigger rethink about the project_generation_scripts.
-Anders
Anders Ryd
***@***.******@***.***>
On Sep 9, 2025, at 1:10 PM, Andrew Hart ***@***.******@***.***>> wrote:
@aehart commented on this pull request.
________________________________
In WriteHDLUtils.py<#75 (comment)>:
+ #horrible hack to fix order of modules
+ ia=-1;
+ ib=-1;
+ for mem in memList:
+ if "_BE" in mem.var() :
+ ia=memList.index(mem);
+ if "_BF" in mem.var() :
+ ib=memList.index(mem);
+ if ia != -1 and ib != -1 and ib < ia :
+ memList[ia], memList[ib] = memList[ib], memList[ia]
+
+ ia=-1;
+ ib=-1;
+ for mem in memList:
+ if "L1PHIH_BC" in mem.var() :
+ ia=memList.index(mem);
+ if "L1PHIH_BD" in mem.var() :
+ ib=memList.index(mem);
+ if ia != -1 and ib != -1 and ib < ia :
+ memList[ia], memList[ib] = memList[ib], memList[ia]
@aryd<https://github.com/aryd> I'm assuming the order here comes from the order in the wiring files, so should the order ultimately be fixed in the TrackletConfigBuilder in CMSSW? I'm not proposing doing this immediately, but it seems like it would be better to eventually fix the issue where it originates, if possible.
—
Reply to this email directly, view it on GitHub<#75 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABI4LTMI3VYT6G52JQKW4PL3R2YSLAVCNFSM6AAAAACFNBER26VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTEMBQHE3DIOBXGI>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR consists of a set of updates to both project_generation_scripts and firmware_hls to solve several disagreements between the reduced FPGA1 project and the CMSSW C++ emulation. Many of the fixes here should be considered "patches" - e.g. the filewriting should be moved into the memory modules to make sure that we accurately print the correct debug information. Now there is a fair bit of manual adjustment to get the debug printout to allign with the memory writing.
project_generation_scripts:
To avoid an overwrite of the InputLink memories I changed them to use 4 pages instead of two. Could probably have used two page after adding delays in the memory writing. This is particularly a problem here as we read multiple input memories in the VMR and the last memory to be read was over written in one of the 100 events.
Add delay in the FileWriterFIFO modules to align debug writing with correct BX. Similarly align debug printout for VMSTE, IL, and AS
firmware_hls:
Add delay in the FileWriterFIFO.vhd
tf_merge_streamer.vhd has some significant rewrites to process data on each BX. The logic is a bit convoluted as the bx_in_vld signal is not aligned with the change in bx. A delay of three clocks has been added to fix this. This should probably be fixed in a cleaner way.
Fixes to CompareMemPrintsFW.py to handle MPAR and AS memories. This could be avoided if the debug printout followed the memory writing.
Make the IL memory use 4 pages instead of 2.