PacketStream protocol, converters, UART stuff and async FIFO #51

Akribes · 2024-03-29T07:46:15Z

I'm reopening #46 here from a new branch, so we can merge code into develop without affecting this PR.

We have made some progress that we think is ready for review.

We defined a PacketStream Clash Protocols protocol for circuits (issues Define a streaming protocol #5, Create tailored Eq instance for PacketStreamM2S #39 and Change Drivable instance for PacketStream #44)

We made up and down converters (issues Create stream up converter #4 and Create stream down converter #14)

Async FIFO (Issue Create clash-protocols async FIFO #1)

For debugging over UART later, we wrapped Clash's UART components in the PacketStream protocol (issues Wrap existing Clash.Cores.UART transmit in PacketStream Protocol #3 and Wrap existing Clash.Cores.UART receive in PacketStream Protocol #22)

This PR also includes CI, but it disabled for now, because it sometimes takes a lot of GiPHouse's CI minutes.

…alled every time

2 setup ci

…nctions (belongs in /Test/.../DownConverter

Create stream down converter

…erter merge interpacket gap inserter into develop

…curious index out of bounds error.

lmbollen

Added some comments. Furthermore I'd like the commit history to consist of logical chunks of features or "work". It makes pull requests far easier to review allows us to understand the commit history better.

As a guideline also each commit should compile and pass the test-suite.

For this pull request I suggest squashing commits that belong to the same functionality or creating a new branch where you selectively create commits of logical functionalities.

Also, when will CI be re-enabled?

lmbollen · 2024-04-03T06:49:17Z

clash-eth.cabal

    Clash.Lattice.ECP5.Colorlight.TopEntity
    Clash.Cores.Ethernet.RGMII
+    Clash.Cores.Ethernet.PacketStream
+    Clash.Cores.Ethernet.UpConverter
+    Clash.Cores.Ethernet.DownConverter
+    Clash.Cores.Ethernet.Util
+    Clash.Cores.Ethernet.InterpacketGapInserter
+    Clash.Cores.Ethernet.AsyncFIFO
    Clash.Lattice.ECP5.Prims
+    Clash.Lattice.ECP5.UART
    Clash.Lattice.ECP5.Colorlight.CRG


lmbollen · 2024-04-03T06:50:53Z

clash-eth.cabal

@@ -33,6 +33,7 @@ common common-options
    TypeFamilies
    TypeOperators
    ViewPatterns
+    ImportQualifiedPost


lmbollen · 2024-04-03T06:56:40Z

src/Clash/Cores/Ethernet/DownConverter.hs

+      , _abort = _dcAborted
+      }
+
+downConverter


This is user API, please add a haddock comment shortly explaining what it does and how it behaves.
Mainly latency and throughput seem relevant.

lmbollen · 2024-04-03T06:56:56Z

src/Clash/Cores/Ethernet/DownConverter.hs

+                          }
+
+
+downConverterC


Also haddock here

lmbollen · 2024-04-03T07:14:59Z

src/Clash/Cores/Ethernet/DownConverter.hs

+  => ( Signal dom (Maybe (PacketStreamM2S dataWidth ()))
+     , Signal dom PacketStreamS2M
+     )
+  -- ^ Input packet stream from the source and backpressure from the sink
+  -> ( Signal dom PacketStreamS2M
+     , Signal dom (Maybe (PacketStreamM2S 1 ()))
+     )
+  -- ^ Output backpressure to the source


Why no support for meta data routing?

For now, we don't need require this.

lmbollen · 2024-04-03T08:34:59Z

src/Clash/Lattice/ECP5/UART.hs

+{-# language FlexibleContexts #-}
+{-# language NamedFieldPuns #-}
+
+module Clash.Lattice.ECP5.UART


Functions that drive a protocol with backpressure, but don't handle the backpressure should be prefixed with unsafe

Does that hold for things that use CSignal? They are not really unsafe imo. They don't handle backpressure but you also can't connect components to them that assert backpressure. There are no function in this module that are unsafe by that definition.

lmbollen · 2024-04-03T08:39:22Z

tests/Test/Cores/Ethernet/Util.hs

@@ -0,0 +1,66 @@
+{-# LANGUAGE RecordWildCards #-}
+{-# LANGUAGE FlexibleContexts #-}
+module Test.Cores.Ethernet.Util where


Add haddock comments to the functions to briefly explain what they do

lmbollen · 2024-04-03T08:40:55Z

tests/Test/Lattice/ECP5/UART.hs

+genVec :: (C.KnownNat n, 1 <= n) => Gen a -> Gen (C.Vec n a)
+genVec gen = sequence (C.repeat gen)


Why not use existing generators?

We're not using these, because we're getting a dependency conflict between clash-prelude-hedgehog and clash-protocols.

lmbollen · 2024-04-03T08:42:08Z

tests/Test/Lattice/ECP5/UART.hs

+    genPackets =
+      PacketStreamM2S <$>
+      genVec Gen.enumBounded <*>
+      pure Nothing <*>
+      Gen.enumBounded <*>
+      pure False


Make a separate re-usable generator

This one is quite specific because _last should always be Nothing and the _abort always false.

lmbollen · 2024-04-03T08:43:30Z

tests/Test/Lattice/ECP5/UART.hs

+        PacketStreamM2S <$>
+        genVec Gen.enumBounded <*>
+        Gen.maybe Gen.enumBounded <*>
+        Gen.enumBounded <*>
+        Gen.enumBounded


use re-usable generator

Akribes · 2024-04-11T14:11:34Z

About CI: there's an issue on this sprint to re-enable CI.

rowanG077 · 2024-05-17T22:12:33Z

Closed. It's way to outdated currently

Baublesaurus and others added 30 commits February 23, 2024 09:52

Added a first nix environment to test working with nix-shell

a312860

This should use the shell.nix from the repo

227a8b9

Added caching of the nix environment, otherwise nix has to be re-inst…

58bb379

…alled every time

Run tests during CI, uses the make test command

45f1899

added a name to a step

fe441a8

removed step to find error

dbab722

added build and test

b1f76db

spelling error

1c6d0d8

added git checkout

7e4e594

removed cache action

92043ec

last working version

f6b1dbc

adds caching

13512f8

added default.nix file to enable nix-build

558d34a

add nix build to the pipeline

38ca212

add caching again

8ea3637

remove caching for now, to see if speed is OK

8552666

magic nix cache is needed for speedy tests

dc3cdcf

Added names, and make verilog

8551684

Indentation fix

c7b6ff2

More names for clarity of process

0549e4d

Add make netlist

4eaf630

Added test for illegal whitspace file endings

77d3da9

Missing keyword

b65bb96

added run permissions to the .sh file

ead6b9f

update checkout versions

2d21be4

Adds make pnr, fixes file endings

4e8b72d

add run permissions for the bash files

4a6afef

fixed the end of file :(

1ac53f5

Merge pull request #10 from GiPHouse/2-setup-ci

42652cc

2 setup ci

Add hoogle and int commands to Makefile

8c9e340

MatthijsMu and others added 19 commits March 19, 2024 14:14

construct downConverterC with forceResetEnable => tests succeed

4e6b424

begin of a Util file, as requested in comment of toMaybe

aa36d3c

remove toMaybe (not in Util), redundant exports, unit tests, model fu…

129273b

…nctions (belongs in /Test/.../DownConverter

bye redundant imports, bye warnings, hello make format!

5fe1514

Small formatting fix

d8230ea

Merge pull request #15 from GiPHouse/14-create-stream-down-converter

b68789b

Create stream down converter

initial version

409badb

added interpacket gap inserter tests and fixed state transition function

5047c74

merged changes from develop

f68278b

fixed tests and style changes

a9c7b6d

removed gapSize, hardcoded 12

827656c

use Index 12 instead of Unsigned 4 + style changes

54fab18

configurable gapSize instead of hardcoded 12 cycles

16c3566

Merge pull request #48 from GiPHouse/28-implement-interpacket-gap-ins…

59ba1bf

…erter merge interpacket gap inserter into develop

initial version of async FIFO + tests. Tests currently fail due to a …

57dec2b

…curious index out of bounds error.

Added a check that the AsyncFifo is non empty before it is ready to read

9b4a28e

AsyncFifo gefixed met Rowan's hulp. Passed nu de tests

e86ad49

Removed unnecessary comments

081e837

final style and comment changes

721205d

Akribes requested a review from lmbollen March 29, 2024 07:46

Akribes mentioned this pull request Mar 29, 2024

PacketStream protocol, converters and UART #46

Closed

lmbollen suggested changes Apr 3, 2024

View reviewed changes

rowanG077 mentioned this pull request Apr 4, 2024

Handle client review 1 #59

Closed

Akribes requested a review from lmbollen April 11, 2024 14:11

Akribes force-pushed the packetstream-converters-uart-async-fifo branch from 570b730 to fd33464 Compare April 11, 2024 14:13

Handle Lucas's review comments

ae8af02

Akribes force-pushed the packetstream-converters-uart-async-fifo branch from fd33464 to ae8af02 Compare April 11, 2024 14:15

rowanG077 closed this May 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PacketStream protocol, converters, UART stuff and async FIFO #51

PacketStream protocol, converters, UART stuff and async FIFO #51

Akribes commented Mar 29, 2024

lmbollen left a comment

lmbollen Apr 3, 2024

lmbollen Apr 3, 2024

lmbollen Apr 3, 2024

lmbollen Apr 3, 2024

lmbollen Apr 3, 2024

Akribes Apr 11, 2024

lmbollen Apr 3, 2024

rowanG077 Apr 3, 2024 •

edited

Loading

lmbollen Apr 3, 2024

lmbollen Apr 3, 2024

Akribes Apr 11, 2024

lmbollen Apr 3, 2024

Akribes Apr 11, 2024

lmbollen Apr 3, 2024

Akribes commented Apr 11, 2024

rowanG077 commented May 17, 2024

		genVec :: (C.KnownNat n, 1 <= n) => Gen a -> Gen (C.Vec n a)
		genVec gen = sequence (C.repeat gen)

PacketStream protocol, converters, UART stuff and async FIFO #51

PacketStream protocol, converters, UART stuff and async FIFO #51

Conversation

Akribes commented Mar 29, 2024

lmbollen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rowanG077 Apr 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Akribes commented Apr 11, 2024

rowanG077 commented May 17, 2024

rowanG077 Apr 3, 2024 •

edited

Loading