YosysHQ / nextpnr

nextpnr portable FPGA place and route tool
ISC License
1.29k stars 242 forks source link

Is there a way to define I/O-timing constraints? #399

Open maikmerten opened 4 years ago

maikmerten commented 4 years ago

I wonder if nextpnr has a way to put constraints on I/O-timing. In Quartus, there's set_input_delay and set_output_delay (I found http://billauer.co.il/blog/2017/04/io-timing-constraints-meaning/ to be very helpful to get a quick overview).

I found https://github.com/YosysHQ/nextpnr/blob/master/docs/constraints.md - but the constraints mentioned there do not seem to deliver similar functionality. Are there other means to constrain I/O-timing?

maikmerten commented 4 years ago

Related to #390?

edit: Probably not, that one is talking about timing between clock domains within the FPGA itself.

smunaut commented 4 years ago

No there isn't anyway to constraints IO path at the moment. I'm not even sure if the timing data for this has been reversed at all.

Your best best at the moment is using IO registers (so the IO timing are not build dependent) and then use the official lattice tools to do the IO timing analysis once so you get the numbers "manually".

maikmerten commented 4 years ago

Thank you for your insightful answer!

madscientist159 commented 4 years ago

@smunaut Out of curiosity, is NextPNR smart enough to understand a manually instantiated SB_IO block with the output register flag set doesn't need an intermediate register, and that the register inside the SB_IO should be part of the overall timing analysis? I suspect this is true, but since I'm chasing down a timing related issue that smells strongly of CLK to OUT delay changing with unrelated HDL changes, I need to verify.

Thanks!

daveshah1 commented 4 years ago

The setup on the input is analysed, the clock to out isn't but it should not change based on the design. However, if you don't use a SB_GB_IO, then design changes might mean that the delay from clock input to global network might change due to different GB placement or routing which would change clock to out from an external point of view.

nextpnr does not insert registers in any case.

smunaut commented 4 years ago

@madscientist159 If your design is public somewhere I can have a quick look to see if I can spot anything that would cause randomized IO timings.

madscientist159 commented 4 years ago

@smunaut Sure, the timing fluctuation seems to be on the LPC buffers with outgoing signals (incoming seems fine):

https://gitlab.raptorengineering.com/raptor-engineering-public/lpc-spi-bridge-fpga/blob/master/main.v#L335

As you can tell from the comments, this is an old design dating back to Arachne, but it's always exhibited a quirk where most of the time the outgoing signals will meet setup / hold on the bus, but ~5-10% of the time an unrelated HDL change is required to restore operation.

Since the parameters are #define-controlled, the #defines are here: https://gitlab.raptorengineering.com/raptor-engineering-public/lpc-spi-bridge-fpga/blob/master/main.v#L6

smunaut commented 4 years ago

Well you're not using the IO register for the output side of the IO ....

madscientist159 commented 4 years ago

@smunaut You're right! I looked at that several times over several days but each time missed the fact that bits [5:2] define the output type, not bits [3:0]. I need a better rubber duck. :smiley:

smunaut commented 4 years ago

Also you might want to look at the verilog array instanciation syntax, things like :

https://github.com/smunaut/ice40-playground/blob/usb/cores/hub75/rtl/hub75_phy.v#L135

madscientist159 commented 4 years ago

@smunaut Thanks for the pointer, will do. The HDL in question actually dates from ~2015 when the tools were fairly raw, and the less advanced features one tried to use the better. If I recall correctly, when this HDL was written there was hope the toolchain would pick up tristate buffer inference but so far as I know that has not happened yet?

smunaut commented 4 years ago

No and it wouldn't implement IO registers if it did anyway. The IO registers have very specific constrain associated with them so they remain a manual thing to do since the designer better be aware of what he's doing.

madscientist159 commented 4 years ago

@smunaut Fair enough, though I dimly recall the old Xilinx tools performing a step called "register packing" where they would try to use the IO buffer registers if doing so was legal within the constraints set by the HDL.

If you don't mind me asking, what would be the issue with inferring an I/O buffer register from (pseudo)HDL like this? I do have some knowledge of the SB internal architecture but I may very well be missing the specific bit of knowledge that stops this working.

(
input wire clock,
input wire data_in,
input wire tristate,
output reg buffered_signal
)

always @(posedge clock) begin
    if (tristate) begin
        buffered_signal <= 1'bz;
    end else begin
        buffered_signal <= data_in;
    end
end
smunaut commented 4 years ago

They do have a feature for that. And if it can't because you randomly changed something in your design resulting in that FF no longer being implementable in the IO buffer they just don't do it anymore and throw a warning buried among the other 600 warnings and suddenly your design doesn't work anymore because the IO timings have just changed drastically ...

That totally didn't happen to me 3 days ago ....

whitequark commented 4 years ago

And if it can't because you randomly changed something in your design resulting in that FF no longer being implementable in the IO buffer they just don't do it anymore and throw a warning buried among the other 600 warnings and suddenly your design doesn't work anymore because the IO timings have just changed drastically ...

This is exactly why nMigen, if you request a registered IO buffer, always instantiates one explicitly instead of relying on inference.

madscientist159 commented 4 years ago

Understood. Thanks for the insights all!

madscientist159 commented 4 years ago

I'm currently moving a design over to an ECP5 from an iCE40 and might be starting to run into this problem again. On the ECP5, if I'm reading the I/O buffer design documents correctly, it's up to the routing software (e.g. NextPNR) to insert registers from general logic resources to create buffered I/O resources. I have a non-public design where as I push the resource use up it's fairly obvious the clock to out delays are starting to randomize, and I'm wondering what the correct way to counteract this would be in the NextPNR flow?

Thanks!

whitequark commented 4 years ago

You can instantiate SB_IO directly.

madscientist159 commented 4 years ago

@whitequark On an ECP5? How does that work?

whitequark commented 4 years ago

Oh sorry I misread. On ECP5 you are supposed to instantiate IFS1P3DX or OFS1P3DX, but nextpnr would not pack them into an IOB at the moment.

smunaut commented 4 years ago

Huh ? AFAIK this is supposed to work.

Yosys will convert the IFS* and OFS* to TRELLIS_FF with special attributes on them ( syn_useioff / ioff_dir ) and nextpnr will pack those into the IO.

daveshah1 commented 4 years ago

Yes, IFS and OFS should be packed if they meet the constraints.

whitequark commented 4 years ago

Oh oops, looks like my information is outdated.

madscientist159 commented 4 years ago

What is the correct primitive to use for a tristateable I/O buffer with registered inputs and outputs? I'm still not 100% sure where you found the IFS1P3DX info, so a pointer would be appreciated.

Thanks!

madscientist159 commented 4 years ago

@smunaut Where would those attributes show up? The .json file?

Reason I ask is with a design modified to use IFS1P3DX / OFS1P3DX on ECP5 I don't think the FFs are being packed (timing is varying all over the place with predictably bad results) and a quick grep on the output files for syn_useioff / ioff_dir isn't showing those keywords.

smunaut commented 4 years ago

Yea in the .json ... if they are not there, check your yosys is not ancient.

madscientist159 commented 4 years ago

@smunaut Yosys was built from GIT hash 8a4841d78690313a91af97e8c6d9aa3e65a3e491, so not exactly ancient. Now that I know what I'm looking for, let me dig around a bit and see what's going on.

madscientist159 commented 4 years ago

@smunaut Found it! This feature doesn't work with -abc9 passed to synth_ecp5.

Is that intentional?

smunaut commented 4 years ago

Most definitely not ! And it used to work ... but just rechecked here and indeed seems broken.

daveshah1 commented 4 years ago

There are some known issues with attributes and abc9, see YosysHQ/yosys#2176

I'll have a look and see what is really going on tomorrow.

daveshah1 commented 4 years ago

Based on a minimal test it is working for me on Yosys master now. If you still have issues then please share a test-case.

smunaut commented 4 years ago

Ok, I updated my yosys and it seems to indeed work with master. My previous yosys build was from mid-late june, so I guess it got fixed meanwhile.

madscientist159 commented 4 years ago

Looks like updating to master fixed it on my side as well.