Skip to content

Support for outstanding HCI transactions#71

Draft
mbertuletti wants to merge 10 commits into
masterfrom
mbertuletti/outstanding_hci
Draft

Support for outstanding HCI transactions#71
mbertuletti wants to merge 10 commits into
masterfrom
mbertuletti/outstanding_hci

Conversation

@mbertuletti
Copy link
Copy Markdown

Adds a request valid-ready response valid-ready protocol, compatible with the variable latency cluster interconnect. Adds reorder buffer module for out-of-order responses.

@FrancescoConti
Copy link
Copy Markdown
Member

@mbertuletti it will take a bit of time for this one. I would like to deeply understand where it diverges from "standard" HCI; my feeling is that since as we discussed part of these features are already in the "standard", it might make more sense to integrate the new ones one-by-one instead as af a "variant" hci_outstanding. But I need to have the time to dig to let you know, in the mean time thanks for the PR! :)

@FrancescoConti
Copy link
Copy Markdown
Member

OK after a long while, I managed to find some time for a bit of review - I also postponed till we merged some of the pending PRs, particularly Sergio's large one. I am very much on the edge here, because I see these modules really as duplications of the existing ones. As far as I can see (but of course I can be wrong) the real distinguishing difference between the hci_outstanding interfaces and the existing hci_core ones is that the req/gnt handshake on the request side is replaced by a valid/ready.
The semantics of the hci_core is described here: https://hwpe-doc.readthedocs.io/en/latest/protocols.html#hci-core (this actually needs a refresh! it is not fully up-to-date with HCI v2 in terms of signal names)

If I am correct, the valid/ready handshake you use in hci_outstanding follows the same semantics as hwpe_stream, then I think that duplicating the full set of hci_core modules into hci_outstanding ones is unneeded, and could lead to unintended consequences (in particular the risk is that an updated to the hci_core modules is not propagated to hci_outstanding, or vice versa -- or that it is propagated incorrectly).
I understand the need to plug into the variable latency interconnect, but in my view this is better served through a (handshake) protocol converter. Specifically

  • request req in hci_core and req_valid in hci_outstanding, as far as I can understand have the same semantics.
  • request gnt in hci_core and req_ready in hci_outstanding are tied by the fact that gnt = req_valid & req_ready.
  • response r_valid/r_ready (hci_core) and resp_valid/resp_ready (hci_outstanding) are simply renamed but with same semantics.

So my idea would be the following:

  1. we keep the addition of the hci_outstanding protocol in hci_interfaces (although I would change the name, as supporting outstanding transactions is not a specific characteristic of this protocol) if you need to expose a SV interface. One proposal could be hci_mempool?
  2. we add core2outstanding and outstanding2core protocol converters (intf2intf or intf2struct, depending how you prefer on the variable_latency_interconnect side).
  3. we drop the duplication of sink/source/fifo/mux/assign. I think especially for sink and source you have some changes compared to hci_core, we should evaluate these individually for integration.
  4. we port the rob to hci_core.

Consider me open to a discussion on any technical ground, and feel free to correct me if any of my understanding above is wrong.

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.

2 participants