cocotbext-axi icon indicating copy to clipboard operation
cocotbext-axi copied to clipboard

Manually set a bus signal as optional even if present

Open patrickrst opened this issue 2 years ago • 5 comments

Hi, this is a usecase I believe may be useful to have for SystemVerilog interfaces. Some designs have an interface that defines many signals, but with modules that may not be using all of those signals. An example below is a AXI Stream interface with tdata and tid, but with a module that does not assign tid.

Since tid is present on the bus, AxiStreamSink will try to resolve the value to an integer which won't work because it remains at 'x'. Would it make sense to have a way to specify that a signal should not be used/resolved for such cases?

Module:

interface axis_if #(
  parameter int DATA_WIDTH = 8,
  parameter int ID_WIDTH = 2
);
  logic                  tvalid;
  logic                  tready;
  logic [DATA_WIDTH-1:0] tdata;
  logic [ID_WIDTH-1:0]   tid;
endinterface

module top;
  logic clk;
  axis_if axis ();
  assign axis.tvalid = '1;
  assign axis.tdata = '1;  
endmodule

Testcase:

import cocotb
from cocotb.clock import Clock
from cocotbext.axi import AxiStreamBus, AxiStreamSink

@cocotb.test()
async def test_top(dut):
    cocotb.start_soon(Clock(dut.clk, 1, units="ns").start())
    receiver = AxiStreamSink(AxiStreamBus.from_entity(dut.axis), dut.clk)
    frame = await receiver.recv()

Log:

0.00ns INFO     cocotb                             Running on ModelSim for QuestaIntel FPGA Edition-64 version 2022.1 2022.01
0.00ns INFO     cocotb                             Running tests with cocotb v1.7.2 from /home/user/venv/lib64/python3.11/site-packages/cocotb
0.00ns INFO     cocotb                             Seeding Python random module with 1673369237
0.00ns INFO     cocotb.regression                  Found test test_top.test_top
0.00ns INFO     cocotb.regression                  running test_top (1/1)
0.00ns INFO     cocotb.axis.None                   AXI stream sink
0.00ns INFO     cocotb.axis.None                   cocotbext-axi version 0.1.18
0.00ns INFO     cocotb.axis.None                   Copyright (c) 2020 Alex Forencich
0.00ns INFO     cocotb.axis.None                   https://github.com/alexforencich/cocotbext-axi
0.00ns INFO     cocotb.axis.None                   AXI stream sink configuration:
0.00ns INFO     cocotb.axis.None                     Byte size: 8 bits
0.00ns INFO     cocotb.axis.None                     Data width: 8 bits (1 bytes)
0.00ns INFO     cocotb.axis.None                   AXI stream sink signals:
0.00ns INFO     cocotb.axis.None                     tdata width: 8 bits
0.00ns INFO     cocotb.axis.None                     tdest: not present
0.00ns INFO     cocotb.axis.None                     tid width: 2 bits
0.00ns INFO     cocotb.axis.None                     tkeep: not present
0.00ns INFO     cocotb.axis.None                     tlast: not present
0.00ns INFO     cocotb.axis.None                     tready width: 1 bits
0.00ns INFO     cocotb.axis.None                     tuser: not present
0.00ns INFO     cocotb.axis.None                     tvalid width: 1 bits
0.00ns INFO     cocotb.axis.None                   Reset de-asserted
1.00ns ERROR    cocotb.Task 1.AxiStreamSink._run   Exception raised by this forked coroutine
1.00ns INFO     cocotb.regression                  test_top failed
                                                   Traceback (most recent call last):
                                                     File "/home/user/venv/lib64/python3.11/site-packages/cocotbext/axi/axis.py", line 725, in _run
                                                       frame.tid.append(self.bus.tid.value.integer)
                                                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^
                                                     File "/home/user/venv/lib64/python3.11/site-packages/cocotb/binary.py", line 372, in integer
                                                       return self._convert_from(self._str)
                                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                                                     File "/home/user/venv/lib64/python3.11/site-packages/cocotb/binary.py", line 252, in _convert_from_unsigned
                                                       return int(x.translate(_resolve_table), 2)
                                                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^
                                                     File "/home/user/venv/lib64/python3.11/site-packages/cocotb/binary.py", line 84, in __missing__
                                                       return self.resolve_x(key)
                                                              ^^^^^^^^^^^^^^^^^^^
                                                     File "/home/user/venv/lib64/python3.11/site-packages/cocotb/binary.py", line 61, in resolve_error
                                                       raise ValueError(
                                                   ValueError: Unresolvable bit in binary string: 'x'
1.00ns INFO     cocotb.regression                  **************************************************************************************
                                                   ** TEST                          STATUS  SIM TIME (ns)  REAL TIME (s)  RATIO (ns/s) **
                                                   **************************************************************************************
                                                   ** test_top.test_top              FAIL           1.00           0.03         37.45  **
                                                   **************************************************************************************
                                                   ** TESTS=1 PASS=0 FAIL=1 SKIP=0                  1.00           0.18          5.51  **
                                                   **************************************************************************************

patrickrst avatar Jan 10 '23 17:01 patrickrst

Ultimately the solution here is to have a better container type that can be passed in to cocotbext-axi. Ideally, something that replaces/supersedes the Bus object from cocotb-bus, but is more flexible and supports things like manual deletion and renaming of signals to deal with non-standard interfaces, as well as operating on parts of signals.

But, I suppose another thing to think about is perhaps better handling of X and Z. This unfortunately ties into the screwed up BinaryValue object in cocotb, which I think they're planning on replacing/reworking at some point. Not sure what would make the most sense here necessarily, but it's possible that some adjustments to better deal with Z might be sufficient for your particular use case.

alexforencich avatar Mar 01 '23 02:03 alexforencich

Thanks for the possible solution ideas. I might have a look at what is been done on the Bus and BinaryValue object.

For now, with a Verilog wrapper, it is easy to workaround this by hardcoding the missing signal values. But indeed, an integrated solution in the long term would be nice.

patrickrst avatar Mar 08 '23 04:03 patrickrst

Hi,

I think we've run into a similar problem. We like to connect cocotbext-axi transactors from prefix:

self.axi_byp = AxiMasterWrite(AxiWriteBus.from_prefix(self.dut, "s_axi"), self.dut.clk, self.dut.rst)

Some of the IP we test have no ID (arid, awid, rid, bid) busses defined and this errors due to those missing signals. We have hacked cocotbext/axi/axi_channels.py to make the IDs optional. For example:

Write response channel

AxiBBus, AxiBTransaction, AxiBSource, AxiBSink, AxiBMonitor = define_stream("AxiB", signals=["bvalid", "bready"], optional_signals=["bresp", "buser", "bid"], signal_widths={"bresp": 2} )

But it seems like there should be a cleaner way that we can do this locally without having to hack the cocotb install. Are we missing something?

Thanks!

Steve Haynal

softerhardware avatar Oct 03 '23 00:10 softerhardware

Making more signals optional is something that I would like to do upstream at some point. I think the current setup is such that making the ID signals optional should be straightforward; I'll take a look at that. Probably the same goes for rlast/wlast. Are there any other signals that should be optional?

alexforencich avatar Oct 03 '23 08:10 alexforencich

For us it was mainly the ID signals. Thanks!

softerhardware avatar Oct 05 '23 14:10 softerhardware