PCIe on UltraScalePlus with 256 bit width is broken

enjoy-digital / litepcie

Small footprint and configurable PCIe core

Other

468 stars 116 forks source link

PCIe on UltraScalePlus with 256 bit width is broken #128

Closed smunaut closed 7 months ago

smunaut commented 8 months ago

Commit 526a3d05aafcf8877b7cb9f893de82f3bd85a6e4 breaks it.

Not only does this not account if you have address_width=32 (see associated PR to fix that issue), but even after that or if you use address_width=64, it doesn't work.

The documentation explicitely states "The user application must keep the s_axis_rq_tvalid signal asserted over the duration of the packet. " but the new code removed the FIFO that was ensuring this.

smunaut commented 8 months ago

Another issue is with tkeep.

It's "per-byte" at the input but at the output it's expected to be "per-double-word". So for one it's too wide ( 32b instead of 8b ) but it's also just plain wrong value ...

This was introduced in the "cleanup" AFAICT.

enjoy-digital commented 7 months ago

Thanks, I'll spent time next week looking at this and try to get it working with your config. Strangely, I've been using this code in the last months on at least two different systems also on Ultrascale+ and Gen3 X8, so in addition to your finding, there is probably a subtle difference.

smunaut commented 7 months ago

Factors that help not see the issues :

address_width=64,
Use only DMAReader (and not DMAWriter),
sys_clk is fast enough ( >= 250M ) to not cause discontinuity
Transfers of the exact good length to have tkeep match by "chance"

The right combination of the above will result on a working system ... but you have to be lucky.

enjoy-digital commented 7 months ago

The designs are using DMAReader and DMAWriter but indeed in the "right" combination for the other parameters. I'll merge your PR next week and will do more tests to help/fix things. I'm also setting up a HW CI setup for LiteX to help catching regressions on these wrappers.