Make and use verilog test benches to test upcoming AMDC release (daisy chain)

[elsevers]

Context

Related Issues / PRs:

• Modify AMDC Firmware to support new DCP updates #504
• Add AMDC Firmware changes to implement the DCP #500
• Fix additional delay in acquisition time for non-daisy chain configured AMDS #502
• AMDS docs: Add table to AMDS daisy chain article mapping sensor card numbers to USART port, headers, and firmware defines docs.amdc.dev#158
• Assess how challenging it would be to update docs.amdc.dev to allow selecting between website versions docs.amdc.dev#156

We are adding daisy chain capability to the AMDS link on the AMDC in #500. This is requiring an overhaul of the hardware (verilog). We need to test the new hardware via verilog testbenches to make sure all cases are properly handled.

Note that in #502, we have reason to believe that timeouts may not be properly handled for the case of only 1 channel of data being expected from an AMDS.

Approach

Create verilog test benches and check:

1. Baseline case of 1x AMDS transmitting 8 channels
2. Case of 3x AMDS's, each transmitting 8 channels
3. Case of 3x 2S boards, each transmitting 2 channels
4. Case of 1x AMDS transmitting only 1 channel
5. These same cases but with incomplete data, parity errors, and delays that should cause timeouts.
6. Other msc cases that come up

Carefully confirm that the timeouts and data corruption detection are working properly, both in the sense that we are detecting problems, but also in the sense that we are not adding additional delays by unnecessarily waiting for channels that we do not expect to have arrive.

[AwesomeTornado]

Hi @elsevers, I'm having some difficulty setting up an environment where I can run Verilog testbench simulations. Could you help me set these up?

[elsevers]

Thanks for digging in to this @AwesomeTornado. Is there a specific question, or more so just how to get going?

I am happy to go over things together in the lab. Unfortunately, it has been approx. 10 years or so since I have done this myself, and so my memory is pretty rusty when it comes to specifics. I do remember the broader picture of how things looked / what we were able to do (more recently from looking over students' shoulders).

[AwesomeTornado]

@elsevers I've been working through setting everything up, at this point I think I just need some help with compiler macro defines and understanding the current testbench. We do actually already have a bunch of testbenches, but there is no documentation on how to set up and modify them.

[elsevers]

Sounds good. I'll be in the lab 10 - 12 today. Let's connect during that window and go over things.

[AwesomeTornado]

I've successfully simulated a singular byte of data being received by the AMDC! Our UART code could probably be improved, we are waiting ten clock cycles per bit of data, but we aren't doing any supersampling, so it feels like we should be able to get better data integrity or even speed up our transmission speed.

I was mentioning that we could disobey the UART standard and not wait for the stop bits to go faster, but it turns out we are already doing that.

It also looks like we are sampling right at the start of each bit transmission, which is poor for data integrity. I'm not sure if I'm just misunderstanding the code here, but it seems like there is a lot to improve.

For reference, this is uart_rx.v

[AwesomeTornado]

@elsevers It's starting to look like the Verilog isn't the issue. I have a full testbench simulating 1 sensor card, 1 amds, and 1 amdc, and the timing is looking very good.

I'll work on testing the other cases tomorrow.

[elsevers]

Thanks @AwesomeTornado.

I think I understand that this is testing one instantiation of adc_uart_rx, that you are inputting to that instantiation one 1 sensor card of data and we are seeing its assert_done get asserted. Good!

The question that I have is what is the other adc_uart_rx instantiation doing? (i.e., the one that doesn't have any data going into it from the AMDS and doesn't expect any AMDS data). Is it properly setting assert_done immediately (as no data is expected)?

Finally --> in this case, what I really want to know is does amdc_amds_v1_0_S00_AXI.vcorrectly assert done as soon as the only adc_uart_rx that is expecting data sets assert_done?

In other words, your screenshot shows that in 625 ns assert_done is activated. How long does it take for amdc_amdc_v1_0_S00_AXI.v to set up ch_valid_reg to the correct values? (see here and here). Does it do it at ~625 ns? Or.... is some of the logic getting stuck in a waitstate because it doesn't understand no data is expected on the other adc_uart_rx?

[AwesomeTornado]

@elsevers

I think you were correct. in module amdc_amds_v2_0_S00_AXI the FPGA is waiting for a timeout before setting ch_valid_reg to the correct value. I don't have time to look any deeper than that today, but I can confidently say that the timeout is adding an extra ~3.4us to data collection.

[elsevers]

Okay, great, nice progress.

This raises two items:

1. Of course, figuring out a fix
2. BUT, before fixing: why isn't the counter of the number of timed out packets incrementing? (I am pretty sure Mohamed-Dek was checking this and it wasn't incrementing)

For item 2:

See

AMDC-Firmware/ip_repo/amdc_amds_1.0/hdl/amdc_amds_v1_0_S00_AXI.v

Line 728 in d90ed1f

5'h1D : reg_data_out <= {counter_line1_data_timed_out, counter_line0_data_timed_out};

• I think this is not incrementing (but it should be if there was a timeout)

The reason I think this is that I am pretty sure we were checking it via

AMDC-Firmware/sdk/app_cpu1/common/drv/amds.c

Lines 146 to 159 in 6eea3d6

	void amds_print_counters(uint8_t port)
	{
	uint32_t base_addr = amds_port_to_base_addr(port);
	if (base_addr == 0) {
	// This means an invalid port argument was passed
	cmd_resp_printf("AMDS: Invalid Port Argument\r\n");
	} else {
	cmd_resp_printf("Valid: %08X\r\n", Xil_In32(base_addr + AMDS_BYTES_VALID_REG_OFFSET));
	cmd_resp_printf("Corrupt: %08X\r\n", Xil_In32(base_addr + AMDS_BYTES_CORRUPT_REG_OFFSET));
	cmd_resp_printf("Timed out bytes: %08X\r\n", Xil_In32(base_addr + AMDS_BYTES_TIMED_OUT_REG_OFFSET));
	cmd_resp_printf("Timed out data: %08X\r\n", Xil_In32(base_addr + AMDS_DATA_TIMED_OUT_REG_OFFSET));
	}
	}

by issuing an amds command line command:

AMDC-Firmware/sdk/app_cpu1/common/sys/cmd/cmd_amds.c

Lines 66 to 70 in 6eea3d6

	// Handle 'amds <port> counters' command
	if (argc == 3 && STREQ("counters", argv[2])) {
	amds_print_counters(port);
	return CMD_SUCCESS;
	}

[AwesomeTornado]

@elsevers, I think I have a pretty good grasp of the problem currently.

We have 3 important modules.

• amdc_amds_v2_0_S00_AXI which I will refer to as the AXI logic
• adc_uart_rx which I will refer to as the Packet logic
• uart_rx which I will refer to as UART logic

The AXI module has a register describing which sensors are enabled on every AMDS. This is sent to the Packet module so that it only reads data from the enabled registers. The Packet module uses the UART logic to receive data from the AMDS.

In a normal configuration:

• AXI logic sees start bit
• AXI logic tells Packet logic it found a start bit.
• Packet logic starts searching for start byte
• Packet logic tells UART logic to look for start bit
• UART logic looks for start bit, reads data, sends it back up the chain.

In the event that there are no sensor cards, the logic is set up to automatically enable the done pin so that it doesn't halt other communications.

This issue stems from the fact that every module is looking for the start bit on its own. Each module can only execute logic once they have been notified that there is an incoming packet. When a channel has no data being sent on it, the AXI logic will never notify the Packet logic that there is data to read, which will never allow the Packet logic to instantly signal "done due to no data". This forces the AXI logic to wait for a timeout.

//Some code omitted for clarity.
`SM_IDLE: begin
            if (start_rx) begin // Harley: start_rx means that AXI module *already detected a start bit*
                if (|is_dout_enabled) begin// Harley: This is the check for the "no sensors enabled" edge case.
                    // We have a trigger and at least one channel is enabled
                    
                    // Incoming UART transmission, so start UART RX module
                    uart_start_rx = 1; // Harley: This signal allows the UART module to *start looking for a start bit*

                end
                else begin // Harley: Notice that this "else" block only runs if we received a "start_rx" pulse!
                    // Trigger received, but the line is disabled. 
                    // Immediately signal "done" so the AXI wrapper doesn't wait/timeout.
                    assert_done = 1;
                end
            end
        end

The AXI, Packet, and UART modules all have their own, independent, logic for detecting timeouts. When the Packet logic never responds, the AXI module eventually gives up and times out.

Important

The Packet module is never "enabled" when there is no data, so it never times out! This stops bytes_timed_out from ever incrementing

But the AXI module still times out, and in my simulation the timeout counter also works, so I am unsure as to why the counter was not ticking up for you.

[AwesomeTornado]

By making things more asynchronous, I was able to get the following simulation. Notice that the data is received the instant it is done transmitting, and that the valid data register updates to reflect this. There is a timeout that mistakenly gets registered, but that can be fixed by resetting the timeout timer upon confirmation that all the data is received.

[AwesomeTornado]

@elsevers I have created a test bench, let me know if there are any conditions that I am missing.

For each of the following scenarios:

• Sequential packet transmission min->max
• Sequential packet transmission max->min
• All 12 sensors enabled
• No sensors enabled
• One sensor enabled
• Random sensors enabled
• Corrupt data
• Corrupt headers
• Timed out data
• Timed out headers

Assert that the following tests are valid:

• Done is asserted after all valid data
• Done is asserted immediately after any timed out data
• Valid data is read as valid
• Invalid data is read as invalid
• Done is not asserted before transmission complete
• Output register matches transmitted data

[elsevers]

Thanks @AwesomeTornado. At first pass, this looks right. Probably best we talk it through -- I'll try to make a point to chat with you during my office hours this morning.