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

Wishbone access from initiator bus with data_width smaller than the one of the subordinate bus. #18

Open
Fatsie opened this issue Jun 24, 2020 · 9 comments
Labels

Comments

@Fatsie
Copy link
Contributor

Fatsie commented Jun 24, 2020

I am working on my Retro_uC. In there I combine a 32-bit M68K and with two 8-bit CPUs (a MOS6502 and a Z80). I would like to have all three of them accessing the same memory map. I would like it that the M68K can fetch a 32-bit word in one bus cycle.
AFAICS, currently neither the Wishbone Arbiter or Decoder allows that the data_width of the initiator bus is smaller than the data_width of the subordinate bus(es) even if the granularity is the same.

I see different solutions to this problem:

  • extend Arbiter to support initiator buses with data_width smaller than the subordinate bus data_width. This would solve my request directly.
  • extend Decoder to support subordinate buses with data_width bigger than initiator bus. For my case this would be more indirect. I would then have an Arbiter on the two 8-bit CPU buses, then a decoder from the output 8-bit bus to a 32-bit data_width and then an Arbiter of that output with the 32-bit CPU bus.
    I still propose this solution as the feature may be more as decoding than arbitration.
  • do both; for my use case I would only use Arbiter so the Decoder implementation will not be tested by user code.
  • do it an a separate bridge class.

This issue can be assigned to me after it is clear what the preferred implementation is.

@jfng
Copy link
Member

jfng commented Jun 25, 2020

I think we should support this by implementing an up-converting bridge between two data widths. It would have the benefit of being reusable in other scenarios (e.g. a point-to-point connection between an initiator and its target).

@whitequark
Copy link
Member

I agree with JF.

@Fatsie
Copy link
Contributor Author

Fatsie commented Jun 29, 2020

OK, will implement it as a separate class.
Plan is to have it implemented in a class Connector. It would not only allow up-convert of data_width but also for example connecting a pipelined WB bus to a non-pipelined one etc.

@whitequark
Copy link
Member

whitequark commented Jun 29, 2020

Plan is to have it implemented in a class Connector. It would not only allow up-convert of data_width but also for example connecting a pipelined WB bus to a non-pipelined one etc.

Should this be the responsibility of the same class? I'm leaning towards having separate ones (easier to test, less code pulled in if you only need one kind of conversion), but I'm open to hearing reasons why it should be handled in a single one.

@Fatsie
Copy link
Contributor Author

Fatsie commented Jun 29, 2020

I was inspired by the Decoder and Arbiter class that are now already doing more than one feature:

  • The Decoder class now also does support both pipelined and non-pipelined Wishbone (e.g. with and without stall feature).
  • The Arbiter supports initiator buses with granularity bigger than the arbiter bus.

Being very strict on DataConversion bus support seems not consistent to me.
So the bus classes would then be:

  • Arbiter: multiple initiator, one subordinate
  • Decoder: single initiator, multiple subordinates
  • Connector: one initiator, one subordinate

And later:

  • Crossbar: multiple initiators, multiple subordinates

If you would support a non-datawidth converting Connector and a datawidth converting DataWidthExtend, would you then error when datawidth is the same on the latter one ?

@whitequark
Copy link
Member

Being very strict on DataConversion bus support seems not consistent to me.

I agree with this assessemnt. Please go on with your original plan.

@Fatsie
Copy link
Contributor Author

Fatsie commented Jul 1, 2020

As said in PR #21 I commit first version; maybe this can give some guidance on further discussion if also the initiator data width bigger than the subordinate one feature should be implemented in that class.

@Fatsie
Copy link
Contributor Author

Fatsie commented Jul 7, 2020

I am now porting Retro-uC to BlackIce which has external SRAM with 16 bit data_width so it seems the implementation of data_width upconverting (16->32) will be done faster than expected.

@chenz
Copy link

chenz commented Mar 27, 2023

Some unsolicited notes that came to mind while looking for a similar solution:

  • litex has simple data-width (Up/Down)Converter modules. These are then used as building blocks in the the construction of more complex adapters (e.g. also doing AXI <-> WB). So having simple modules is not mutually exclusive with (eventually) having a more intricate conversion. Meanwhile, they can already already be used as-is.
  • I agree that Connector is not a good name, a connection implies that pieces fit together. Converter or Adapter might be more appropriate.
  • the proposed implementation as well as the litex one directly connect two busses - but a use case I am thinking of would be adding an adapted sub bus to a Decoder. Would that then work like this?
sub_bus_16 = <existing interface with data_width=16>
decoder = Decoder(data_width=32, granularity=8, ...)
sub_bus_32 = Interface(data_width=32, ...)
connector = Connector(sub_bus32, sub_bus16)
decoder.add(sub_bus_32, ...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

4 participants