Review of VideoBusRx

[JimLewis]

In the code segment, it seems that there can be a timing issue if the guard cycles are all 0. It seems like the code requires a minimum guard cycle of 1.

    wait until rising_edge(vid_fval_in);
    Log(ModelID, "New frame started in VideoBusRx!", INFO); 
    pixelCount := 0;
    lineCount := 0;
    while (vid_fval_in = '1') loop
        wait until rising_edge(Clk);
        if (vid_lval_in = '1') then
            pixelCount := 0;
            while (vid_lval_in = '1') loop
                wait until rising_edge(Clk);
                if (vid_dval_in = '1') then

Generally, I like keeping the whole VC aligned to clock, so the first wait would be replaced by:

  wait on Clk until rising_edge(Clk) and vid_fval_in = '1' ;

However, this would just further contribute to delays. OTOH, I think this and my above concerns would be addressed by moving the I suspect though that the remaining wait until rising_edge(Clk) may need to be after the end if rather than before. That would allow the 0 guard cycles.

[JimLewis]

In the code segments:

            AffirmIf(ModelID, (pixelCount = VIDEO_WIDTH), "pixelCount: " & to_string(pixelCount) & " /= VIDEO_WIDTH: " & to_string(VIDEO_WIDTH));
            -- Fill missing pixels
            for i in 1 to (VIDEO_WIDTH - pixelCount) loop 
                RxData := (others => 'U');
                Push(ReceiveFifo, RxData);
                Increment(ReceiveCount);
            end loop;
            lineCount := lineCount + 1;
        end if;
    end loop;
    AffirmIf(ModelID, (lineCount = VIDEO_HEIGHT), "lineCount:  " & to_string(lineCount) & " /= VIDEO_HEIGHT: " & to_string(VIDEO_HEIGHT));
    -- Fill missing pixels
    for j in 1 to (VIDEO_HEIGHT - lineCount) loop
        for i in 1 to VIDEO_WIDTH loop
            RxData := (others => 'U');
            Push(ReceiveFifo, RxData);
            Increment(ReceiveCount);
        end loop;   
    end loop;

Increment(ReceiveCount) is being called, but time is not passing. Hence, ReceiveCount will only increment once for this.

[Louadria]

About your first comment, is this what you mean?

    wait on Clk until rising_edge(Clk) and vid_fval_in = '1' ;
    Log(ModelID, "New frame started in VideoBusRx!", INFO); 
    pixelCount := 0;
    lineCount := 0;
    while (vid_fval_in = '1') loop
        if (vid_lval_in = '1') then
            pixelCount := 0;
            while (vid_lval_in = '1') loop
                if (vid_dval_in = '1') then
                     ...
                end if;
                wait until rising_edge(Clk);
            end loop;
            ...
        end if;
        wait until rising_edge(Clk);
    end loop;

Regarding the second comment, do you suggest adding a "wait for 0 ns;"?

[JimLewis]

A wait for 0 ns could be a disaster if there are 1000's of values transferred during this time. Since these are being pushed into the FIFO in 0 time, I think instead of waiting for a delta cycle each time, instead, do the update to ReceiveCount after all of the pushes, hence,

            for i in 1 to (VIDEO_WIDTH - pixelCount) loop 
                RxData := (others => 'U');
                Push(ReceiveFifo, RxData);
                -- Increment(ReceiveCount);  -- will not increment due to delta cycles
            end loop;
            ReceiveCount <= ReceiveCount + (VIDEO_WIDTH - pixelCount) ; -- accumulate values just pushed

[Louadria]

I have made a pull request to fix these issues, see #6.