Constructing Wishbone interface with a single memory window is overly complex / uses redundant parameters

[galibert]

For the simple but very usual case of a memory range mapping a device that does not need internal decoding (ram, rom, that kind of stuff) the boilerplate is a little annoying. Specifically it is:

    self.bus = wishbone.Interface(addr_width = self.ram_width-2, data_width = 32, granularity = 8, name="ram")
    map = memory.MemoryMap(addr_width = self.ram_width, data_width=8, name="ram")
    map.add_resource(self.ram, name="ram", size=self.ram_size)
    self.bus.memory_map = map

The redundancy I suspect makes things error-prone, especially with the data_width/granularity issues between the Interface and the MemoryMap. It probably could be done in one helper function call, not sure what it should look like though.

[whitequark]

RAM and ROM we should have in the standard library already, so I think this wouldn't come up very much.

[galibert]

you have internal ram, external sram on pins, external sdram on pins, and external ddr through Intel's HMC? :-)

[whitequark]

you have internal ram

We should have this and it's easy.

external sram on pins

We should have this and it's easy.

external sdram on pins

We should have this, though it's a bit complicated.

external ddr through Intel's HMC

I'd have to look into it. Is that a SoC thing?

[galibert]

I have handy a mister and a terasic c5g, both Cyclone V, the first with embedded arm.

The mister has sdram on free pins, and ddr3 on the arm side that's accessible from the fpga through a soc-specific hps-sdram interface which is mostly 6 axi3/avalon (configurable) read or write ports. Inside it goes through a hmc (hardware memory controller) on the arm side but we don't need to care. The memory training/calibration seems to be done in arm code.

The c5g is not a soc, it has the sram on free pins and the lpddr2 on pins controlled by an hmc (altera_mem_if_hard_memory_controller_top_cyclonev). The example code to handle it is dementedly complicated and also uses pll, delay-locked-loop, termination control and who knows what else. It may be too intel-specific at some point to be part of -soc.

[whitequark]

Ah, yeah, HMC is definitely out of scope for -soc.

[galibert]

So, well, that's an example of what could exist and will not fit in -soc :-)

[whitequark]

I think if you're interfacing with HMC then using four lines instead of two is a significant concern...

[galibert]

Most definitely! But what I don't really like is the code smell of the redundancy of parameters. It looks to me that the use-case "window on something" doesn't really exist, interface-wise. Maybe it shouldn't, I don't really know.

[whitequark]

But what I don't really like is the code smell of the redundancy of parameters.

Oh yeah that makes sense!

[whitequark]

I think this is as simple as adding a function on wishbone.Interface so you can do e.g. self.bus.set_full_size_memory_map(self) (name subject to bikeshedding).

[galibert]

I think so too, doesn't need to be more than that.

[tannewt]

When I was experimenting with lambda SoC definitions I used a decoder object with fixed address ranges. Each range was accessible through subscripting ([0]) and passed into the peripheral that occupied that address range.

https://github.com/systemonachip/systemonachip/blob/6440b7ad7648a1affa1e6ddbdbf8d6fe76f57df7/examples/basic.py#L27-L43

        self.bus = wishbone.Interface(addr_width=30, data_width=32, granularity=8,
                                      features={"cti", "bte"})

        self._decoder = Decoder(self.bus, 0x10000000) # Every 0x10000000
        """Memory decoder that splits the 32-bit address space into 16 0x10000000 byte chunks. Each
           chunk can be passed into a sub-bus or peripheral."""

        self.cpu = MinervaCPU(reset_address=0x00000000,
                              instruction_bus=self._arbiter,
                              data_bus=self._arbiter)
        """Central processing unit."""

        self.rom = RandomAccessMemory(self._decoder[0x0], size=rom_size, writable=False)
        """Core read-only memory. At 0x00000000."""

        self.ram = RandomAccessMemory(self._decoder[0x2], size=ram_size)
        """Single random access memory. At 0x20000000."""

These experiments went further and switched to a more "outside in" approach. Passing the memory range into the peripheral class meant that you could pass another type of accessor for the memory range and actually use the object. The Timer peripheral shows this:

    timer0 = Timer(sbus)
    with sim.write_vcd("timer.vcd"):
        print(timer0.width) # Should be 2
        timer0.reload_ = 0xf
        timer0.value = 0xe
        print(timer0.enable) # Should be False
        timer0.enable = True
        for _ in range(50):
            sim.advance()
        print(timer0.enable)
        print(timer0.value)
        # wait 5 cycles
        print(timer0.zero)
        print(timer0.value)