FPGA-Research-Manchester / FABulous

Fabric generator and CAD tools
https://fabulous.readthedocs.io/en/latest/
Apache License 2.0
148 stars 34 forks source link

Configuration FrameStrobe Widths #168

Open RobB720 opened 7 months ago

RobB720 commented 7 months ago

Hi,

Looking at the generated code the Frame_Select block instances have FrameStrobe inputs and outputs: .FrameStrobe_I(FrameAddressRegister[MaxFramesPerCol-1:0]), .FrameStrobe_O(FrameSelect[1MaxFramesPerCol+MaxFramesPerCol-1:1MaxFramesPerCol]),

FrameAddressRegister is defined by the parameter MaxFramesPerCol.

However, at the top of eFPGA_top FrameAddressRegister is defined as a wire in terms of FrameBitsPerRow. wire[FrameBitsPerRow-1:0] FrameAddressRegister;

In my design, I have the parameters from my configuration: parameter FrameBitsPerRow=32, parameter MaxFramesPerCol=116,

So we therefore have mismatched bus widths in the code. Can this code be fixed?

Best regards, Rob.

KelvinChung2000 commented 7 months ago

I have done the fix in https://github.com/KelvinChung2000/FABulous/commit/98b8d5dad96c404f32e756db35703718ca062b63 I checked it and changed the generated top wrapper to what you intended. But I am unsure if any side effects might cause problems.

If this works fine, please let me know, and I will merge this directly with the master.

RobB720 commented 6 months ago

Just got around to following this up - thanks the fix looks good.

However, the FrameStrobe_I[MaxFramesPerCol-1:0] bus on Frame_Select block is connected to the FrameAddressRegister bus in eFPGA_top. This is the driven by eFPGA_Config, inside which FrameAddressRegister width is set by the parameter FrameBitsPerRow - i.e. 32 bits wide.

This code seems to force the FrameStrobe buses to be a maximum of 32 bits wide and we want 116 bits.

If we allow the FrameAddressRegister to be 116 wide by changing the parameters, will this break the eFPGA_Config RTL and the configuration mechanism?

Best regards, Rob.

KelvinChung2000 commented 6 months ago

I have designed the current version of FABulous so that if you change the parameter at the very top level, the parameter should propagate correctly to all the sub-modules. However, I am unsure if the configuration module is designed to handle the varying parameters. I have not looked into that side of the project.

RobB720 commented 6 months ago

One idea we have had is to insert a decoder between the eFPGA_config block and the Frame_Select blocks. We could then have a 7-bit field within the FrameAddress register that is decoded to a 116-bit one hot bus that is then selectively driven by the Frame_Select blocks.

Would this work? Are we correct presume the FrameStrobe buses driven by the Frame_Select blocks are one-hot?

Could the configuration software be changed to encode the Frame_Select value?