Skip to content
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.

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

Add CreditedIO to support register-register interfacing for AXI4 and TL #2555

Merged
merged 9 commits into from
Jul 29, 2020

Conversation

terpstra
Copy link
Contributor

@terpstra terpstra commented Jul 14, 2020

This PR adds a new kind of flow control, CreditedIO, which fills the same role for credit/debit systems that DecoupledIO fills for ready/valid systems. Included in this PR are conversion methods to and from Decoupled as well as adapters for TileLink and AXI4.

The advantage of a credit/debit system is that you can add an arbitrary number of fanout=1 fanin=1 pipeline stages in both the data delivery and credit return paths. By comparison, adding ready/valid pipeline stages invariably includes at least a fan of degree 2.

See the following design diagram: https://app.lucidchart.com/invitations/accept/8271ee7b-98ba-44a4-ab3e-e8ae5af9fb43

Type of change: feature request
Impact: API addition (no impact on existing code)
Development Phase: implementation

@terpstra terpstra requested review from hcook and aswaterman July 14, 2020 21:34
We must clear the credit flops during reset.
Copy link
Member

@aswaterman aswaterman left a comment

Choose a reason for hiding this comment

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

(I only looked at CreditedIO.scala)

@terpstra
Copy link
Contributor Author

I'll hold off on merging this a bit to see what @hcook and @rmac-sifive think. I've noticed that adding a new ClockCrossingType causes quite a number of unmatched case warnings in sifive-blocks. It seems like that codebase used a lot of copy-paste. If I refactor that, the number of places that actually need to be touched in the larger ecosystem seems to only be 3.

val bits = Output(genType)

/** Provide a DecoupledIO interface for sending CreditedIO[Data].
* Convert an IrrevocableIO input to DecoupledIO via Decoupled().
Copy link
Member

Choose a reason for hiding this comment

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

IrrevocableIO?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What are you asking? You want me to remove that line of commentary?
While the sender function returns a DecoupledIO, you may want to connect an IrrevocableIO.

Copy link
Member

Choose a reason for hiding this comment

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

I just didn't see IrrevocableIO mentioned anywhere in this particular function. this is CreditedIO and as you the say the return type is DecoupledIO. I guess I would find "To connect an IrrevocableIO sender, first convert it with Decoupled()" to be clearer phrasing to me?

@@ -27,6 +30,8 @@ case class IntOutwardCrossingHelper(name: String, scope: LazyScope, node: IntOut
IntSyncRationalCrossingSink() :*=* IntSyncNameNode(name) :*=* scope { IntSyncNameNode(name) :*=* IntSyncCrossingSource(alreadyRegistered) } :*=* node
case SynchronousCrossing(buffer) =>
IntSyncSyncCrossingSink() :*=* IntSyncNameNode(name) :*=* scope { IntSyncNameNode(name) :*=* IntSyncCrossingSource(alreadyRegistered) } :*=* node
case CreditedCrossing(CreditedDelay(sourceDebit, _), CreditedDelay(sinkDebit, _)) =>
IntSyncSyncCrossingSink(/*sinkDebit==0*/) :*=* IntSyncNameNode(name) :*=* scope { IntSyncNameNode(name) :*=* IntSyncCrossingSource(sourceDebit==0) } :*=* node
Copy link
Member

Choose a reason for hiding this comment

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

why is sinkDebit ignored here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The IntSyncCrossingSink does not support a register stage. I could change that. Should I?

Copy link
Member

@hcook hcook Jul 27, 2020

Choose a reason for hiding this comment

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

No I guess I would just leave it, though I confess I don't really understand the semantics of IntSync are as it relates to what circuits outcomes represent "safe" crossings. Would a user explicitly controlling sinkDebit be surprised to not get a buffer?

@rmac-sifive rmac-sifive merged commit 97b8255 into master Jul 29, 2020
@rmac-sifive rmac-sifive deleted the credited-io branch July 29, 2020 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants