sipeed / TangNano-4K-example

TangNano-4K-example project
89 stars 27 forks source link

HDMI example regression in Gowin IDE edu 1.9.9beta4 in regard to Gowind IDE 1.9.8 and 1.9.8.03 #10

Open Popolon opened 8 months ago

Popolon commented 8 months ago

The default presynthesized HDMI example is pobably synthesized with version 1.9.9beta4 of Gowin IDE edu.

https://github.com/sipeed/TangNano-4K-example/blob/main/hdmi_720p/hdmi.fs

I upload the bitstream with OpenFPGAloader

I synthesized the HDMI example with Gowin IDE edu 19.9.8 and it worked perfectly as in second screenshot, after upgrading, and resynthesized I have the same bug than in pre-synthesized example.

I uploaded the bitstream placed in hdmi_720p/impl/pnr/hdmi.fs after synthesizing. Bug_1 9 9beta4 OK_1 9 8

Popolon commented 8 months ago

There are some wargning in both case during synthesizing about line 67 of svo_htmi.v:

   always @(posedge clk)
        locked_clk_q <= {locked_clk_q, locked};

locked_clk_q is a 4 bits vector, should isn't be a logic operation, or a wrong order, I'm a bit beginner in Verilog, but I'm not sure the result of 4 bits <= 4bits + 1 bit after synthesizing is really predictable.

Works fine and no more warning, with:

locked_clk_q <= {locked_clk_q[2:0], locked};

The registers vectors definitions:

   reg [3:0] locked_clk_q;
    reg [3:0] resetn_clk_pixel_q;

And the same warning occur for line 70:

   always @(posedge clk_pixel)
        resetn_clk_pixel_q <= {resetn_clk_pixel_q, resetn};

But this is more probably in svo_tcard.v: WARN (EX3791) : Expression size 32 truncated to fit in target size 24("/data/fpga/TangNano-4K-example/hdmi_720p/src/hdmi/svo_tcard.v":237)

               out_axis_tdata <= bolt_bitmap[{yoff,  xoff}] ? ~0 : 0;

Also line 251,253,254,257,259,260, in this segment of code:

           if (hcursor == SVO_HOR_PIXELS-1) begin
                hcursor <= 0;
                x <= 0;
                xoff <= HOFFSET;
                if (vcursor == SVO_VER_PIXELS-1) begin
                    vcursor <= 0;
                    y <= 0;
                    yoff <= VOFFSET;
                end else begin
                    vcursor <= vcursor + 1;
                    if (&yoff)
                        y <= y + 1;
                    yoff <= yoff + 1;
                end
            end else begin
                hcursor <= hcursor + 1;
                if (&xoff)
                    x <= x + 1;
                xoff <= xoff + 1;
            end
.
Popolon commented 8 months ago

The bug in the code is confirmed with Gw IDE 1.9.9.01 (non edu, registered version).

And here is the patch, this is related to ctrl_fifo_wraddr, that need 3 bits (8 values) instead of 2 bits (4 values) here:

diff --git a/hdmi_720p/src/hdmi/svo_enc.v b/hdmi_720p/src/hdmi/svo_enc.v
index 83b4e85..d5b71a5 100644
--- a/hdmi_720p/src/hdmi/svo_enc.v
+++ b/hdmi_720p/src/hdmi/svo_enc.v
@@ -46,7 +46,7 @@ module svo_enc #( `SVO_DEFAULT_PARAMS ) (
        reg [`SVO_XYBITS-1:0] vcursor;

        reg [3:0] ctrl_fifo [0:3];
-       reg [1:0] ctrl_fifo_wraddr, ctrl_fifo_rdaddr;
+       reg [2:0] ctrl_fifo_wraddr, ctrl_fifo_rdaddr;

        reg [SVO_BITS_PER_PIXEL:0] pixel_fifo [0:7];
        reg [2:0] pixel_fifo_wraddr, pixel_fifo_rdaddr;

Here, a bit less blurry picture of the bug: Sipeed_TangNano4K_HDMI_720p_bug

Popolon commented 8 months ago

Found another case, where this value should be defined instead, a more complete patch is (sorry the previous time I made diff between 2 tests, here is the diff with current git on this file:

diff --git a/hdmi_720p/src/hdmi/svo_enc.v b/hdmi_720p/src/hdmi/svo_enc.v
index 83b4e85..8e8c5d4 100644
--- a/hdmi_720p/src/hdmi/svo_enc.v
+++ b/hdmi_720p/src/hdmi/svo_enc.v
@@ -46,7 +46,7 @@ module svo_enc #( `SVO_DEFAULT_PARAMS ) (
        reg [`SVO_XYBITS-1:0] vcursor;

        reg [3:0] ctrl_fifo [0:3];
-       reg [1:0] ctrl_fifo_wraddr, ctrl_fifo_rdaddr;
+       reg [3:0] ctrl_fifo_wraddr, ctrl_fifo_rdaddr;

        reg [SVO_BITS_PER_PIXEL:0] pixel_fifo [0:7];
        reg [2:0] pixel_fifo_wraddr, pixel_fifo_rdaddr;
@@ -54,7 +54,7 @@ module svo_enc #( `SVO_DEFAULT_PARAMS ) (
        reg [SVO_BITS_PER_PIXEL+3:0] out_fifo [0:3];
        reg [1:0] out_fifo_wraddr, out_fifo_rdaddr;

-       wire [1:0]  ctrl_fifo_fill =  ctrl_fifo_wraddr -  ctrl_fifo_rdaddr;
+       wire [3:0]  ctrl_fifo_fill =  ctrl_fifo_wraddr -  ctrl_fifo_rdaddr;
        wire [2:0] pixel_fifo_fill = pixel_fifo_wraddr - pixel_fifo_rdaddr;
        wire [1:0]   out_fifo_fill =   out_fifo_wraddr -   out_fifo_rdaddr;
Popolon commented 8 months ago

I played with some more cases, and there is a case were at this line wire[2:0] work better than [3:0], I thnk there are probably some other problems elsewhere, probably linked to limits and overruns:

wonderfullook commented 8 months ago

This project is based on SimpleVOut, and different version of GOWIN IDE may lead different results. Check your timing result first, and then check others.