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

Wrong initial flip-flop state #175

Closed
rfuest opened this issue Apr 23, 2023 · 5 comments · Fixed by #176
Closed

Wrong initial flip-flop state #175

rfuest opened this issue Apr 23, 2023 · 5 comments · Fixed by #176

Comments

@rfuest
Copy link

rfuest commented Apr 23, 2023

I've just started playing with Yosys/nextpnr on a Tang Nano 9K and encountered an issue which I couldn't explain. And I'm unsure if I did something wrong or this is a bug or current limitation of the open source toolchain. The same design works as expected when synthesized with the vendor tools.

The minimized example which triggers the issue is simply turning on an LED when the corresponding button is pressed. Both the LEDs and buttons are active low on the Tang Nano 9K. The only difference between led1 and led2 is that the internal state is inverted.

With the vendor tools led1 and led2 are off after the bitstream is loaded and can be turned on with their corresponding button. But with the open source toolchain led1 is already turned on after the bitstream is loaded, without any button presses. led2 works as expected.

I did also run a post synthesis simulation and both buttons worked as expected in the simulation.

Source files

top.v

module top (
    input clk,
    input button1,
    input button2,
    output [5:0] leds
);

reg led1 = 1'b0;
reg led2 = 1'b1;

always @(posedge clk) begin
    if (button1 == 1'b0)
        led1 <= 1'b1;

    if (button2 == 1'b0)
        led2 <= 1'b0;
end

assign leds = { led2, button2, 2'b11, button1, !led1 };

endmodule

top.cst

IO_LOC "leds[5]" 16;
IO_LOC "leds[4]" 15;
IO_LOC "leds[3]" 14;
IO_LOC "leds[2]" 13;
IO_LOC "leds[1]" 11;
IO_LOC "leds[0]" 10;

IO_LOC "button1" 4;
IO_LOC "button2" 3;

IO_LOC "clk" 52;

Commands used to synthesize the design

yosys -p "read_verilog top.v; synth_gowin -json top.json"
nextpnr-gowin --family GW1N-9C --device "GW1NR-LV9QN88PC6/I5" --cst top.cst --json top.json --write top_pnr.json
gowin_pack --device GW1N-9C -o top.fs top_pnr.json
openFPGALoader top.fs

Toolchain

oss-cad-suite-linux-x64-20230422

@yrabbit
Copy link
Collaborator

yrabbit commented Apr 23, 2023

Yes, the phenomenon is confirmed.
I should only note that "initial state" is a deceptive phrase: there is no such concept for flip-flops in the binary image.
As a matter of fact, everything ends even before the data is sent to nextpnr - at the yosys level, the flip-flop type is selected so that it is initially in the desired state.
Unfortunately, my competence is not enough at this level.

@rfuest
Copy link
Author

rfuest commented Apr 24, 2023

Thanks for the info. Can this problem be related to the NODFFE problem? I didn't immediately find any explanation what the known problem with DFFE is, but I could track my issue back to the DFFE:

If I replace DFFE with DFFRE (with RESET = 0) in the Yosys output my design works as expected. Connecting all inputs of a DFFE to 0 produces a 1 output in the open source toolchan and a 0 using the vendor tools. The same experiment with a DFFRE did produce the expected 0 output with both toolchains.

@yrabbit
Copy link
Collaborator

yrabbit commented Apr 24, 2023

The "NODFFE" error is called so conventionally because specifying the --nodffe key when calling yosys (synth_gowin --nodffe) fixes the problem.

But it does not define the nature of the error itself.

Your case is good in the sense that it is very small in size and I think I'll spend today on experiments with INIT.

@yrabbit
Copy link
Collaborator

yrabbit commented Apr 25, 2023

Here it turned out to be a reaction to a global reset.
Too bad, NODFF remains a mystery, but you can fix a couple lines in gowin_pack.py and see if that solves the problem :)

PR: #176

dff.mp4

@rfuest
Copy link
Author

rfuest commented Apr 25, 2023

Thanks a lot, I've tried the new version of gowin_pack and it works much better now. It did not only fix the basic design in this issue, but also seems to have fixed a more complex design, which I was working on when I noticed the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants