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

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

Open
8 of 11 tasks
Juninho99 opened this issue Jun 8, 2024 · 10 comments · Fixed by #22
Open
8 of 11 tasks

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

Juninho99 opened this issue Jun 8, 2024 · 10 comments · Fixed by #22
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@Juninho99
Copy link
Collaborator

Juninho99 commented Jun 8, 2024

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

  • Create a basic function to generate random signals for cam_dphy_dat and cam_dphy_clk
    • This function should produce simple signal patterns to observe their behavior in the simulation.
    • Verify the correct behavior and timing of these signals in the simulation environment.
  • Add an option for Incrementing Data Pattern
    • Implement a mode to generate incrementing data patterns to facilitate downstream data integrity checks.
    • This option will help follow image transformations more easily when analyzing waveforms.

2. Camera Simulation Class

  • Design and implement a Python class to simulate camera behavior
    • The class should handle the generation of differential signals for cam_dphy_dat and cam_dphy_clk.
    • Implement methods to set these signals to desired values for different test scenarios.

3. Data Transmission Functions

  • Implement functions to send specific bits and bytes
    • Create methods in the camera simulation class to send individual bits and bytes over the differential signals.
    • Ensure these functions respect the number of data lanes specified as a parameter.

4. Line and Frame Control Functions

  • Develop functions for sending start/end of line and frame signals
    • Add methods to handle the transmission of signals indicating the start and end of lines and frames.
    • Implement mechanisms for sending line and frame data according to the CSI-2 protocol.
  • Provide an option to specify Resolution and FPS
    • Implement support for resolutions and frame rates defined in the current RTL:
      • define HDMI_720p60
      • define HDMI_1080p30
      • define HDMI_1080p60

5. ECC Calculation

  • Implement ECC Calculation Function
    • Develop a function to calculate the ECC for the transmitted data based on the CSI-2 specification.
    • Ensure the function can be easily integrated and called within methods such as start_frame, end_frame, and send_line.

6. Generate Errors and Corner Cases

  • Introduce options for generating errors and corner cases
    • Implement options for inserting lane-to-clock skew and lane-to-lane skew.
    • Add support for clock jitter.
    • These options should be specified as the maximum deviation from the nominal, with random deviations within the specified range.

7. Verilator Adaptations

  • Adapt Primitives for Verilator Compatibility
    • Identify and adapt RTL primitives that require modification to work correctly with Verilator.
    • Ensure the simulation runs smoothly without issues related to these primitives.

8. Integration and Testing

  • Integrate the class and functions into the testbench environment
    • Ensure that the class can be easily instantiated and used within Cocotb tests.
    • Write initial tests to validate the functionality of the class and its methods.

9. Documentation

  • Document the class and its usage
    • Provide comprehensive documentation on how to use the class and its methods.
    • Include example test cases and expected outputs.

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
Copy link
Owner

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:

  • lane-to-clock skew
  • lane-to-lane skew
  • clock jitter

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:

  • `define HDMI_720p60
  • `define HDMI_1080p30
  • `define HDMI_1080p60 , see Issue#1 and @suarezvictor comment

@Juninho99
Copy link
Collaborator Author

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
Copy link
Owner

chili-chips-ba commented Jun 14, 2024

1

  • For readability, consider using Hex instead of binary format in stimulus. That would not only shorten the line, but also allow removing b8b8 from the comment:
    ([0b10111000, 0b01000111], 20), # Sync bytes b8b8

2

  • What's the plan for scaling the stimulus from 2 lanes to 4 lanes?
  • Can it be made scalable from the get-go from one place, by one-and-only-one control knob?!

3

  • Any luck with bringing up this sim on Verilator too?
  • Is Icarus the reason why typedef enums of state declaration are not shown using their symbolic values
    image

4

  • Add a Makefile target for GTKWave launch (instead of having to manually type this command gtkwave sim_build/top.fst)

5

  • As a bonus, when everything else is done, try to also enable Surfer viewer. Surfer has recently undergone a major upgrade to version 0.2.0 and, as another NLnet project, our project is here to help @TheZoq2 by providing comprehensive feedback and stress-testing his design

@Juninho99
Copy link
Collaborator Author

Juninho99 commented Jun 25, 2024

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
Copy link
Collaborator Author

Juninho99 commented Jun 25, 2024

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
Copy link
Collaborator Author

Juninho99 commented Jun 29, 2024

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
Copy link
Collaborator Author

Juninho99 commented Jun 30, 2024

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
Copy link
Owner

chili-chips-ba commented Jul 2, 2024

@Juninho99 great work on creating:

  • the first-ever opensource model for the CSI2 camera
  • simulation framework and bring up

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
Copy link
Owner

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
Copy link
Collaborator Author

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.

@chili-chips-ba chili-chips-ba added documentation Improvements or additions to documentation enhancement New feature or request labels Jul 10, 2024
@Juninho99 Juninho99 linked a pull request Jul 15, 2024 that will close this issue
@Juninho99 Juninho99 reopened this Jul 16, 2024
@chili-chips-ba chili-chips-ba changed the title issue#5 issue#5 - cocotb spec, dev and status Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants