Skip to content

Add ReqResp Protocol #83

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add ReqResp Protocol #83

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clash-protocols.cabal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ library
Protocols.Internal.Units.TH
Protocols.Plugin
Protocols.Plugin.Internal
Protocols.ReqResp
Protocols.Wishbone
Protocols.Wishbone.Standard
Protocols.Wishbone.Standard.Hedgehog
Expand Down
53 changes: 53 additions & 0 deletions src/Protocols/ReqResp.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
{- |
Simplest possible protocol for request-response communication.

The forward channel channel has type @Signal dom (Maybe req)@ and is used to send requests.
The backward channel has type @Signal dom (Maybe resp)@ and is used to send responses.

The protocol must obey the following rules:
* When the forward channel is @Just a@, it must not change until the transaction is completed.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implies that this protocol can not be pipelined. We can relax this constraint and do two versions of ReqResp (pipelined and not pipelined). Or we can use DSignal to enforce different pipeline depths on the type level.

Copy link
Member

@rowanG077 rowanG077 Jul 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why couldn't it be pipelined? I think you can write a component that handles this. Essentially store forward in a register. Send contents of the register to the sink. When the sink responds either send it directly to the source and clear the register or put the response in a register as well. The main problem is that you no longer can have full throughput between source and sink.

Copy link
Member

@christiaanb christiaanb Jul 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should then just have

type ReqRespPipelined (dom :: C.Domain) (req :: Type) (resp :: Type) = (Df dom req, Reverse (Df dom resp))

a kind of Axi4LiteLite

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enforcing pipeline depth on the type level seems to be more useful than just a blanket declaration "you can send stuff, which we call a request, and you can receive stuff, which we call a response", which seems to be what Christiaans ReqRespPipelined is.

Avalon Stream does pre-negotiated pipelining. You can configure that when you drop ready, you can still receive data in the next n cycles, so the pipeline of the sender can empty. Constructions like that. Maybe it can serve as inspiration.

Alternatively, we can tackle pipelining in a different PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think separating the channels is a good idea as it directly contradicts the purpose of circuit-notation (Combining forward and backward channels in a single binder).

How about separating request acknowledgement and response timing?:

data ReqResp (dom :: C.Domain) (req :: Type) (resp :: Type)

instance Protocol (ReqResp dom req resp) where
  -- | Forward channel for ReqResp protocol
  type Fwd (ReqResp dom req resp) = C.Signal dom (Maybe req)

  -- | Backward channel for ReqResp protocol
  type Bwd (ReqResp dom req resp) = C.Signal dom (Bool, Maybe resp)

Here the Bool would indicate a request acknowledgement and Maybe resp could be Just resp later.
Combinatorial circuits could then return (isJust resp, resp).

Copy link
Member

@DigitalBrains1 DigitalBrains1 Jul 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would you still be able to tell which response corresponds to which request?

Never mind, that's not a very clever comment.

It does seem to me that you need a whole bunch more protocol specification for two pipelined implementations to be able to communicate. Where's the guarantees? I want to know I can still get rid of all the data currently in my pipeline; if I can't get a guarantee I can deliver it all, I will need buffers to deal with the case that I cannot, or I need to be able to stall the whole pipeline at any random moment, and we're back at square one.

Personally, I think a "simple" protocol that doesn't allow pipelining at the protocol level¹ and a separate more complex protocol that tackles pipelining in a neat and efficient way is a perfectly fine solution. This PR can be the simple variant.

¹ As Rowan indicates above, you can still accomodate pipelining with constructions like skid buffers, but the protocol would be oblivious to this and it would in general be less efficient than pipelining being part of the protocol.

* The forward channel can not depend on the backward channel.
* When the forward channel is @Nothing@, the backward channel may be undefined.

This protocol can not be pipelined, for pipelined request-response communication see `Protocols.BiDf`.
-}
module Protocols.ReqResp where

import qualified Clash.Prelude as C
import Data.Kind (Type)
import Protocols
import Protocols.Internal.Classes
import Prelude as P

{- |
Simplest possible protocol for request-response communication.

The forward channel channel has type @Signal dom (Maybe req)@ and is used to send requests.
The backward channel has type @Signal dom (Maybe resp)@ and is used to send responses.

The protocol must obey the following rules:
* When the forward channel is @Just a@, it must not change until the transaction is completed.
* The forward channel can not depend on the backward channel.
* When the forward channel is @Nothing@, the backward channel may be undefined.
-}
data ReqResp (dom :: C.Domain) (req :: Type) (resp :: Type)

instance Protocol (ReqResp dom req resp) where
-- \| Forward channel for ReqResp protocol:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
-- \| Forward channel for ReqResp protocol:
-- | Forward channel for ReqResp protocol:

What happened here?

type Fwd (ReqResp dom req resp) = C.Signal dom (Maybe req)

-- \| Backward channel for ReqResp protocol:
type Bwd (ReqResp dom req resp) = C.Signal dom (Maybe resp)

instance IdleCircuit (ReqResp dom req resp) where
idleFwd _ = pure Nothing
idleBwd _ = pure Nothing

{- | Force a @Nothing@ on the backward channel and @Nothing@ on the forward
channel if reset is asserted.
-}
forceResetSanity ::
forall dom req resp.
(C.HiddenReset dom) =>
Circuit (ReqResp dom req resp) (ReqResp dom req resp)
forceResetSanity = forceResetSanityGeneric