sbourdeauducq
commented
2 months ago Also noticed a mistake I've made before: I assumed the maximum packet size was 512B, while in reality it's 1024B. ? However, implementing that in firmware for maximum payload size causes linker errors, and 768B caused store faults (possibly needed more stack size), so I have left it as is for now (actual practical maximum value is TBD),
We need to understand this and make good use of the available packet memory - sounds like a low-hanging fruit to get better performances. But this can be the next PR.
which also keeps backwards compatibility.
It is not relevant. ARTIQ-8 is not supposed to interoperate with ARTIQ-7, other things will be broken already, and within the development/beta branch we don't keep compatibility either.
ARTIQ Pull Request
Description of Changes
2382 was fairly complex (but more powerful), this is a far simpler solution to the problem from #2302.
In both cases the extra DRTIO packet to indicate async packets was removed. Firmware changes are also somewhat reserved - checking for asynchronous packets must be done at all (post setup) transactions, and rerouted accordingly.
Instead of redoing the protocol, what was done was the aux receiver buffer was extended eight-fold, and made into a circular buffer, with two pointers, for writing (for gateware), and reading (for CPU). This allows to hold more messages before throwing an error, and since asynchronous packets are rather sparse (can imagine at most getting a subkernel message, finish, ddma finish at the same time), this should solve the issue.
Of course in this case there will be no benefits of having potentially asynchronous transactions, keeping them by their ID etc., but this is a far simpler system to understand and maintain.
Also noticed a mistake I've made before: I assumed the maximum packet size was 512B, while in reality it's 1024B. However, implementing that in firmware for maximum payload size causes linker errors, and 768B caused store faults (possibly needed more stack size), so I have left it as is for now (actual practical maximum value is TBD), which also keeps backwards compatibility.
Tested with gateware simulation, and in practice with two Kaslis 2.0 with subkernel and DDMA tests I've had before.
Zynq port requires only minor changes.
Related Issue
Closes #2302
Type of Changes
Steps (Choose relevant, delete irrelevant before submitting)
All Pull Requests
Code Changes
flake8to check code style (follow PEP-8 style).flake8has issues with parsing Migen/gateware code, ignore as necessary.Git Logistics
git rebase --interactive). Merge/squash/fixup commits that just fix or amend previous commits. Remove unintended changes & cleanup. See tutorial.[x] Write short & meaningful commit messages. Review each commit for messages (
git show). Format:Licensing
See copyright & licensing for more info. ARTIQ files that do not contain a license header are copyrighted by M-Labs Limited and are licensed under LGPLv3+.