search

chili-chips-ba / openeye-CamSI

A truly opensource camera serial interface. No frills. No backdoors that compromise security. Outstanding signal integrity. Hi-rez video pipeline with remote connectivity. For Sony, Series7 & open FPGA makers on limited budget. Augments openXC7 CI/CD, challenging its timing-savvy. Promotes the lesser-known EU boards.
https://nlnet.nl/project/TISG
BSD 3-Clause "New" or "Revised" License
25 stars 5 forks source link

issue#5 - cocotb spec, dev and status #19

Open Juninho99 opened 5 months ago

Juninho99 commented 5 months ago

Issue: Develop Testbench Environment for Testing CSI-2 Camera Model in Cocotb

Description

The goal of this issue is to develop a testbench environment for testing a CSI-2 camera model using Cocotb. The primary objective is to create a Python class that simulates the actual operation of a camera by generating and sending cam_dphy_dat and cam_dphy_clk signals. These signals are differential inputs to the top module, and the class will be responsible for setting these signals to specific values.

Tasks

1. Basic Signal Generation

2. Camera Simulation Class

3. Data Transmission Functions

4. Line and Frame Control Functions

5. ECC Calculation

6. Generate Errors and Corner Cases

7. Verilator Adaptations

8. Integration and Testing

9. Documentation

Expected Outcome

By completing this issue, we expect to have a fully functional testbench environment capable of simulating the CSI-2 camera model's behavior. This environment will facilitate thorough testing and validation of the top module's interaction with the camera signals, ensuring robust and reliable operation.

chili-chips-ba commented 5 months ago

In addition to generating random data patterns, add an option for Incrementing Data Pattern. Such option would checking the downstream data integrity easier. It would also facilitate the process of following image transformations when looking at the waves.

Also add a new task category: Generate Errors and Corner Cases. For the starters, within it, add options for inserting:

These options can be specified as the max deviation from the nominal, and the model would then randomly deviate within that range.

Within Line and Frame Control Functions category, provide an option to specify Resolution and FPS. For the starters, this set needs to include at least the rates supported by our current RTL:

Juninho99 commented 5 months ago

On current branch issue#5 we have implemented basic functions for control signals cam_dphy_dat and cam_dphy_clk. We can send a sequence of bytes on the data lane, and everything is received properly through RTL. For example, through cocotb we send this sequence:

   sequence = [
      ([0b00000000, 0b11111111], 40), # Some random data at the start
      ([0b10111000, 0b01000111], 20), # Sync bytes b8b8
      ([0b00010010, 0b11101101], 20)  # Start of long packet
   ]

After the simulation is done, we can obtain results and see that everything works fine, except for the part of the packet_handler where RTL didn't set long_packet signal in right way. In the red block, we can see that all bytes have been received properly.

image

From folder /2.sim/cocotb/top you can run the simulation with the command: make SIM=icarus WAVES=1 TESTCASE=test_1 Also from the same path you can open gtkwave with: gtkwave sim_build/top.fst and inside it, you can read csi-2.gtkw from the `/2.sim/cocotb/top folder.

chili-chips-ba commented 5 months ago

1

2

3

5

Juninho99 commented 5 months ago

First of all, the simulation is now working pretty much good. Everything was done using Verilator. Of course, there were many problems regarding the choice of the simulator, but from this point CSI model can be continued.

So, as we can see from the image below, csi_in_frame and csi_in_line signals are there and they behave as expected. All sync bytes are cought properly, and all data from python script are received.

image

Current functions in python script are not so edited nor readable. However, the first task from this issue is done and tested successfully. Next tasks are related to the class and much prettier way of sending data.

For curiosity, here are some lines of code that send sequence of data to the differential signal cam_dphy_dat:

   sequence = [
      ([0x00, 0xFF], 2),    # Some random data at the start
      ([0xB8, 0x47], 1),    # Sync bytes b8b8
      ([0x12, 0xFF], 1),    # Start of frame (FF inv. -> 00)
      ([0x81, 0x7E], 3),    # Random data
      ([0xFF, 0x00], 2),    # Random data
      ([0x00, 0xFF], 2),    # Random data
      ([0xB8, 0x47], 1),    # Sync bytes b8b8
      ([0x32, 0xED], 1),    # Start of long packet (32h bytes, ED inv. -> 12) - EMBEDDED DATA
      ([0x11, 0xFF], 1),    # (FF inv. -> 00), combined with previous 32 --> 0032 bytes to read
      ([0xFF, 0x00], 32),   # Data for read
      ([0x00, 0xFF], 2),    # Random data
      ([0xB8, 0x47], 1),    # Sync bytes b8b8
      ([0x32, 0xD7], 1),    # Start of long packet (32h bytes, D7 inv. -> 28) - START OF LINE
      ([0x11, 0xFF], 1),    # (FF inv. -> 00), combined with previous 32 --> 0032 bytes to read
      ([0x22, 0xDD], 32),   # Data for read
   ]

Definitely, this can be done more efficiently and that will be done in the next tasks.

About your previous suggestions @chili-chips-ba:

  1. Surely more readable with 0x..... and comments will be removed later
  2. We will get to it. With class and methods, I think it won't be a problem
  3. More luck with Verilator, but also with some problems, which will be discussed in the next comments
  4. Yes. It is already there, we can make it with make g, but definitely I'll make Makefile more clearly
  5. Stay tuned 😉
Juninho99 commented 5 months ago

I'll mention some of issues I encountered with the simulators.

Icarus Issues: Development of testbench with Icarus is at this point completely ceased. There are multiple reasons for that. One of the main ones is that our RTL uses more advanced Verilog and SystemVerilog constructs than Icarus can digest. Moreover, we found that Icarus was having problems even with the basic constructs, such as multi-dimensional arrays, "for-loops" and indexing within them.

Additionally, when it comes to the csi_rx_align_word.sv module, the signals word_in_pipe and valid_in_pipe do not show in Icarus simulation waves at all. We found that "packed" arrays were the reason for that pecularity. Interestingly, "unpacked" arrays are displayed properly.

Having tried to overcome these problems by adding `ifdef branches for Icarus where simpler, more primitive Verilog constructs were used, we have soon realized that such approach was not maintainable, not scalable and not worth further investments in our time and effort. Consequently, Icarus dev track was abandoned.

Verilator Issues: Several problems also emerged with Verilator. The first and major one comes from Verilator 2-state/cycle-based simulation engine, which make it difficult to understand 'X and 'Z, and also make sub-cycle # delays harder to work with. That renders most XILINX primitives' models non-simulatable in Verilator. Specifically, the IDELAYE2 component is currently ignored, and it will need to be manually rewritten in a Verilator-friendly way.

We've also experienced problems in cocotb interaction with Verilator. A good example of that is cam_dphy_dat input. Verilator simply did not allow assigning it as intuitively as with Icarus, where the following natural form worked off the bat:

signal[lane*2 + 1].value = bit_value  # Positive bit
signal[lane*2].value = ~bit_value & 0x1  # Negative bit (inverse)

Verilator, on the other hand, was stubbornly not cooperative on this front, not even when forces were used. Using trial-and-error iterative method, after some time we've managed to find a form that works, which is by assigning all bits in one-and-only-one co-routine, such as:

bit_a = (byte0 >> bit) & 0x1
bit_b = ~bit_a & 0x1
bit_c = (byte1 >> bit) & 0x1
bit_d = ~bit_c & 0x1

# Set the signal values for both lanes
signal.value = (bit_b << 3) | (bit_a << 2) | (bit_d << 1) | bit_c

Additionally, below is the image showing the specific error when the simple and straightforward Icarus form was used with Verilator. image

Having pulled quite a bit of hair, and thinking at first that some small, unimposing detail was overlooked, we then started looking around. It did not take long to uncover that these issues were also encountered by the others, who left a solid track record of similar problems:

Verilator Pull Request #2728 Cocotb Issue #2380 Verilator Issue #2727

Juninho99 commented 4 months ago

Update: Implementation of CSI Class for Camera Simulation

A whole new CSI Python class is written as the foundational component for subsequent validation of our video pipeline. This class is thoroughly tested using Verilator. It includes methods such as send_combined_bytes, send_data_pattern, start_frame, end_frame, send_embedded_data, send_line, send_frame, and run.

For simulation, this class was instantiated with parameters such as: 

num_lane = 2
dinvert = [0, 1] if num_lane == 2 else [0, 1, 0, 1]

csi = CSI(dut, dut.cam_dphy_dat, 
               period_ns=1.096, fps_Hz=60, line_length=1280, frame_length=720, frame_blank=130, 
               num_lane=num_lane, dinvert=dinvert)

await csi.run(3)  # Sending 3 frames as an example

(*) For more, see the bottom of test_2 in 2.sim/cocotb/top/test_top.py.

Extensive debug and test time effort was put into validation of this function. Attached waveform snapshot illustrates three frames sent, each containing 5 lines (a reduced number for testing purposes). image

The basic video functionality was fully validated for 2-lane mode. The dinvert feature was tested and behaves as expected. Simulation and implementation of the 4-lane mode is on-going. As a matter of fact, we have it almost functional on the issue#5 branch. Corner-case testing and development of documentation are also in the pipeline.

To run the test use the following procedure:

cd 2.sim/cocotb/top
make WAVES=1 TESTCASE=test_2

When sim completes, open GTKWave with gtkwave dump.fst in the same folder. The csi-2-verilator.gtkw file in GTKWave brings in all necessary signals.

Juninho99 commented 4 months ago

ECC successfully tested

Module csi_rx_packet_handler was reverted to original code with ECC part now also successfully tested. In the CSI class, a part for calculating ecc has been added. Note: This code section should be adapted to the specifics of the camera and the data being sent. On the test example, everything works as expected. Here is a function that calculates ECC in python:

def calculate_ecc(self, byte1, byte2, byte3):
      """
      Calculate the ECC for the given three bytes.
      The ECC is computed over the 24 bits formed by concatenating the three bytes.
      """
      data = (byte1 << 16) | (byte2 << 8) | byte3

      ecc = 0

      ecc_bit5 = ((data >> 10) & 1) ^ ((data >> 11) & 1) ^ ((data >> 12) & 1) ^ ((data >> 13) & 1) ^ ((data >> 14) & 1) ^ ((data >> 15) & 1) ^ ((data >> 16) & 1) ^ ((data >> 17) & 1) ^ ((data >> 18) & 1) ^ ((data >> 19) & 1) ^ ((data >> 21) & 1) ^ ((data >> 22) & 1) ^ ((data >> 23) & 1)
      ecc |= ecc_bit5 << 5

      ecc_bit4 = ((data >> 4) & 1) ^ ((data >> 5) & 1) ^ ((data >> 6) & 1) ^ ((data >> 7) & 1) ^ ((data >> 8) & 1) ^ ((data >> 9) & 1) ^ ((data >> 16) & 1) ^ ((data >> 17) & 1) ^ ((data >> 18) & 1) ^ ((data >> 19) & 1) ^ ((data >> 20) & 1) ^ ((data >> 22) & 1) ^ ((data >> 23) & 1)
      ecc |= ecc_bit4 << 4

      ecc_bit3 = ((data >> 1) & 1) ^ ((data >> 2) & 1) ^ ((data >> 3) & 1) ^ ((data >> 7) & 1) ^ ((data >> 8) & 1) ^ ((data >> 9) & 1) ^ ((data >> 13) & 1) ^ ((data >> 14) & 1) ^ ((data >> 15) & 1) ^ ((data >> 19) & 1) ^ ((data >> 20) & 1) ^ ((data >> 21) & 1) ^ ((data >> 23) & 1)
      ecc |= ecc_bit3 << 3

      ecc_bit2 = ((data >> 0) & 1) ^ ((data >> 2) & 1) ^ ((data >> 3) & 1) ^ ((data >> 5) & 1) ^ ((data >> 6) & 1) ^ ((data >> 9) & 1) ^ ((data >> 11) & 1) ^ ((data >> 12) & 1) ^ ((data >> 15) & 1) ^ ((data >> 18) & 1) ^ ((data >> 20) & 1) ^ ((data >> 21) & 1) ^ ((data >> 22) & 1)
      ecc |= ecc_bit2 << 2

      ecc_bit1 = ((data >> 0) & 1) ^ ((data >> 1) & 1) ^ ((data >> 3) & 1) ^ ((data >> 4) & 1) ^ ((data >> 6) & 1) ^ ((data >> 8) & 1) ^ ((data >> 10) & 1) ^ ((data >> 12) & 1) ^ ((data >> 14) & 1) ^ ((data >> 17) & 1) ^ ((data >> 20) & 1) ^ ((data >> 21) & 1) ^ ((data >> 22) & 1) ^ ((data >> 23) & 1)
      ecc |= ecc_bit1 << 1

      ecc_bit0 = ((data >> 0) & 1) ^ ((data >> 1) & 1) ^ ((data >> 2) & 1) ^ ((data >> 4) & 1) ^ ((data >> 5) & 1) ^ ((data >> 7) & 1) ^ ((data >> 10) & 1) ^ ((data >> 11) & 1) ^ ((data >> 13) & 1) ^ ((data >> 16) & 1) ^ ((data >> 20) & 1) ^ ((data >> 21) & 1) ^ ((data >> 22) & 1) ^ ((data >> 23) & 1)
      ecc |= ecc_bit0

      return ecc
chili-chips-ba commented 4 months ago

@Juninho99 great work on creating:

The next step is to distill some of this info into a brief, yet informative README.md section on how to run the simulation and modify it to per-user needs, with a few examples, as you've shown above.

Please, also bring up the issues you've found with Verilator, cocotb and Icarus to their relevant owners.

chili-chips-ba commented 4 months ago

Let's also make the tests auto-checking, so that the designer does not need to eye-ball the waveforms for every sim run, but instead the test itself finds and reports issues in RTL

Juninho99 commented 4 months ago

Yes, definitely we need to provide some tests that will be auto-checked. It will be much easier for users to see where the issue is. Also we will briefly explain everything according to our model and scripts, but first of all we want to make sure that all the tasks with this issue are done.