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

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

Open
Mai-Lapyst opened this issue Aug 11, 2024 · 5 comments
Open

Comments

@Mai-Lapyst
Copy link

Mai-Lapyst commented Aug 11, 2024

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
Copy link
Collaborator

yrabbit commented Aug 11, 2024

I have a few comments on the commands you use:

  • You indicate nextpnr to write to the output file with the name pnrled.json, but gowin_pack you start with led.json;
  • Tangprimer20k does not belong to the GW2A-18C family, which you indicate for gowin_pack- it belongs to the GW2A-18 family;
  • nextpnr-gowin is somewhat outdated and may contain errors that have long been fixed in nextpnr-himbaechel.
#!/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
pr20k.mp4

@yrabbit
Copy link
Collaborator

yrabbit commented Aug 11, 2024

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
Copy link
Author

* 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
Copy link
Collaborator

yrabbit commented Aug 11, 2024

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
Copy link
Member

Maybe we should add a deprecation warning though

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

No branches or pull requests

3 participants