YosysHQ / apicula

Project Apicula 🐝: bitstream documentation for Gowin FPGAs
MIT License
465 stars 66 forks source link

tangprimer20k: Design involving the clock doesn't run on hardware #269

Open Mai-Lapyst opened 1 month ago

Mai-Lapyst commented 1 month ago

When using a design that involves the clock of the tangprimer20k, the bitcode created does not properly work on the bord itself. When using the gowin toolchain (IDE or via gw_sh), it runs fine.

The code used is the blinking led example directly from the sipeed website: https://wiki.sipeed.com/hardware/en/tang/tang-primer-20k/examples/led.html#New-file

Commands used:

yosys -p "read_verilog led.v; synth_gowin -json led.json"
nextpnr-gowin --json led.json --write pnrled.json --device GW2A-LV18PG256C8/I7 --cst led.cst
gowin_pack -d GW2A-18C -o led.fs pnrled.json
openFPGALoader -b tangprimer20k led.fs

Versions used:

Bord: dock-ext v3713, core module v3961

`led.v`: ``` module led( input Clock, output IO_voltage ); /********** Counter **********/ //parameter Clock_frequency = 27_000_000; // Crystal oscillator frequency is 27Mhz parameter count_value = 13_499_999; // The number of times needed to time 0.5S reg [23:0] count_value_reg ; // counter_value reg count_value_flag; // IO change flag always @(posedge Clock) begin if ( count_value_reg <= count_value ) begin //not count to 0.5S count_value_reg <= count_value_reg + 1'b1; // Continue counting count_value_flag <= 1'b0 ; // No flip flag end else begin //Count to 0.5S count_value_reg <= 23'b0; // Clear counter,prepare for next time counting. count_value_flag <= 1'b1 ; // Flip flag end end /********** IO voltage flip **********/ reg IO_voltage_reg = 1'b0; // Initial state always @(posedge Clock) begin if ( count_value_flag ) // Flip flag IO_voltage_reg <= ~IO_voltage_reg; // IO voltage flip else // No flip flag IO_voltage_reg <= IO_voltage_reg; // IO voltage constant end /***** Add an extra line of code *****/ assign IO_voltage = IO_voltage_reg; endmodule ```
Constaints `led.cst` (or `fpga_project.cst` in the offical gowin IDE): ``` IO_LOC "IO_voltage" L14; IO_PORT "IO_voltage" IO_TYPE=LVCMOS18 PULL_MODE=UP DRIVE=8 BANK_VCCIO=1.8; IO_LOC "Clock" H11; IO_PORT "Clock" IO_TYPE=LVCMOS18 PULL_MODE=UP BANK_VCCIO=1.8; ```
yrabbit commented 1 month ago

I have a few comments on the commands you use:

#!/bin/sh
yosys -p "read_verilog led.v; synth_gowin -json led.json"
nextpnr-himbaechel --json led.json --write led.json --top led --device GW2A-LV18PG256C8/I7 --vopt family=GW2A-18 --vopt cst=led.cst
gowin_pack -d GW2A-18 -o led.fs led.json
openFPGALoader -b tangprimer20k led.fs

https://github.com/user-attachments/assets/03d3f448-8f7e-43a1-a247-5f887d4ca6e0

yrabbit commented 1 month ago

I just noticed that I myself read and write in led.json, this focus works in my OS (Dragonflybsd) but it may not work in others. So it is better to have different files :)

Mai-Lapyst commented 1 month ago
* You indicate nextpnr to write to the output file with the name pnrled.json, but gowin_pack you start with led.json;

Ah good catch; that was a typo while I copied arround the commands from my shell; In my tests I ofc used the correct pnrled.json. Have edited the initial text to reflect that.

* Tangprimer20k does not belong to the GW2A-18C family, which you indicate for gowin_pack- it belongs to the GW2A-18 family;

It does? The official IDE and a lot of other places suggest that GW2A-18C is correct: image

Regardless, it doesnt work with either of them.

* nextpnr-gowin is somewhat outdated and may contain errors that have long been fixed in nextpnr-himbaechel.

Oh okay, using nextpnr-himbaechel does indeed fix everything. Thanks a lot! Maybe then the gowin binary should either be dropped or a warning but somewhere? A lot of outside ressources sadly still point people to use nextpnr-gowin.

yrabbit commented 1 month ago

It does? The official IDE and a lot of other places suggest that GW2A-18C is correct:

Let's say carefully - Gowin has some difference between what is shown to the user and what is actually inside the chip and, the funniest, by those names of the series that are used by IDE itself :) You can see what we have to deal with: https://github.com/yosyshq/apicula/blob/master/doc/device_grouping.md

Look at the control amounts of the files that the chips describe - the letter R means only that there is a memory in the same case, but the chip itself still refers to that family. But the letter C is already a significant change in the insides of the chip and another family. And when Gowin IDE tell you that Tangnano20k and Tangprimer20k is GW2A (R) -18C, this is cunning-inside it uses the GW2A-18 base for Primer and GW2A-18C for Tangnano. I understand that it is indifferent to you as a user, but if you have a desire to participate as a programmer, you can make a PR with the new Parser of the names of the chips (and there will be a lot of similar rags).

55cfc48170a50c08f30b0b46e773669d  bin/gowin1.9.8/IDE/share/device/GW2A-18C/GW2A-18C.fse
55cfc48170a50c08f30b0b46e773669d  bin/gowin1.9.8/IDE/share/device/GW2ANR-18C/GW2ANR-18C.fse
55cfc48170a50c08f30b0b46e773669d  bin/gowin1.9.8/IDE/share/device/GW2AR-18C/GW2AR-18C.fse
9f5ef5e4a8530ea5bbda62cd746e675a  bin/gowin1.9.8/IDE/share/device/GW2A-18/GW2A-18.fse
9f5ef5e4a8530ea5bbda62cd746e675a  bin/gowin1.9.8/IDE/share/device/GW2AR-18/GW2AR-18.fse

Oh okay, using nextpnr-himbaechel does indeed fix everything. Thanks a lot! Maybe then the gowin binary should either be dropped or a warning but somewhere? A lot of outside ressources sadly still point people to use nextpnr-gowin.

Yes, this bothers me, I was going to destroy the Legacy version on May 1, 2024 and even warned those whom I knew about it (Lushay Labs, for example;)), but while the old version lives, is not updated, but is present :(

pepijndevos commented 1 month ago

Maybe we should add a deprecation warning though