Modularize CSR integration/code generation

[thirtythreeforty]

In my design, I need for CSRs to be synchronous with blocks in multiple clock domains; currently, CSRBankArray collects then all into the sys domain regardless of the primary clock domain of the block in question, so the design can't synchronously update registers or use pulses to start/stop state machines.

To fix this I will need to provide a new CSR collection implementation that provides a separate sub-CSR bus for each clock domain, with an async crossing between them. (Not proposing that this becomes the new default, although I do think it would be more or less free for those not using clock domains other than sys.) Roughly:

There's a couple issues with LiteX that means this is difficult to do modularly. Questions:

• the current CSR framework is tightly coupled into the SoC class, and there's not a good point to override in a subclass. This is easy enough to move into its own method. Any objections to this?
• The address of each register is implicitly calculated with a for-loop, rather than being provided by the CSR mechanism. This is especially a problem because the header generation code duplicates the implicit calculation. This needs to be moved into a field that's calculated by the CSR collector only once per register.
  • Corollary: have I missed any locations where this logic is duplicated?
• Opinion poll: it would be cheaper from a logic perspective to partition registers' address based in part on their clock domain, so that the address doesn't need to be checked for a miss in other clock domains. Is the address of a register within a bank constrained by aesthetic reasons to be the in sequential order that they were declared?

[enjoy-digital]

Thanks @thirtythreeforty, we could indeed improve support for multiple clock domains on the CSR. I'm generally putting the CDC logic between the CSRBankArray and the logic of the core. Before discussing the implementation details for the changes on the CSR collection/CDC, could you provide more information about the current limitations you are facing in your design? I'll maybe have suggestions to at least work around these.

[thirtythreeforty]

It's true that putting the CDC behind the CSRBankArray would be a smaller code change. The reason I was thinking it would be more appropriate to group all the modules in a domain would be that you'd only need a single clock domain crossing per clock domain, rather than one per component per clock domain. I can be persuaded on this, however.

Here's an example use case: I have a multi-bit command register that triggers a capture state machine. The state machine updates multiple other status registers every cycle as it runs (memory address, remaining samples, etc). When stopped, the CPU needs to be able to read and update these to re-arm the module.

It's true that I could probably work around this by using a bunch of single-bit clock domain crossings and PulseSynchronizer (although this wiki section makes me nervous). And then, declare that the CPU can only read registers while the module is stopped. But it makes the CSR dramatically more difficult to use; it's not a single class any more, the CPU has to do a bunch of writes that are logically one write, and the module has to consider all the async behavior of CSRs. It's not an isolated module, most of the chip runs faster than the RAM and CPU, so I figured the effort to retool CSRs would be similar and reusable.

[enjoy-digital]

Due to the nature of the CSR (simple bus with single cycle write, 2 cycles read, no handshake), the solution you are suggesting is not really possible. The CDC would have to happen on the Wishbone bus, before Wishbone2CSR. We would then need to have different groups of CSRBankArray (one for each clock domain) with one Wishbone2CSR/WishboneCDC for each.

This is however a non-trivial change and I would recommend putting the CDC behind the CSRBankArray for now.

You have different solutions for this:

• If you want to pass a full synchronized bus, you can also use the BusSynchronizer: https://github.com/m-labs/migen/blob/master/migen/genlib/cdc.py#L85
• You can also small AsyncFIFOs for this, for both directions.
• The CPU could also be the one triggering the update of the status register: 1 bit update write in sys_clk --> PulseSynchronizer --> latch status --> MultiReg --> status in sys_clk that would always be stabilized N cycles after the update write.

The right architecture on the control/datapath can generally greatly simplify CDC on such cases. Happy to work with you on your specific use case if you want to see if we can find a simple solution.