YosysHQ / nextpnr

nextpnr portable FPGA place and route tool
ISC License
1.24k stars 237 forks source link

Gowin: failed route io with ddr #1275

Open andryblack opened 5 months ago

andryblack commented 5 months ago

failed route IOBUF with ODDR and IDDR together:

module test_io (
    input sys_clk,  // 27 Mhz, crystal clock from board
    input sys_resetn,
    input button,   // 0 when pressed

    output [5:0] led,

    inout test_io
);

wire test_i,test_o,test_oen;
IOBUF io_buf(
    .I(test_i),
    .O(test_o),
    .IO(test_io),
    .OEN(test_oen)
);
ODDR oddr_rwds(
    .CLK(sys_clk), .D0(button), .D1(~button), .TX(button), .Q0(test_i), .Q1(test_oen)
);
IDDR iddr_rwds(
    .CLK(sys_clk), .D(test_o), .Q0(led[0]), .Q1(led[1])
);
endmodule

nextpnr-himbaechel:

ERROR: Can't place iddr_rwds at X0Y16/IOLOGICA because it's already taken by oddr_rwds

nextpnr-gowin:

ERROR: Cell 'iddr_rwds' cannot be bound to bel 'R29C15_IOLOGICB' since it is already bound to cell 'oddr_rwds'

yrabbit commented 5 months ago

Your problem has been considered and patches for apicula and nextpnr-himbaechel have been prepared. Such complex changes will require some dancing with the sequence of accepting a PR in one water product, releasing and accepting a PR in another.

It's slow official way. But you can manually fix gowin_pack yourself, and also fix and compile nextpnr.

https://github.com/YosysHQ/apicula/pull/227

https://github.com/yrabbit/nextpnr/commit/1ccd4e56da3b753c61d94bc66b049cf091fa728b

andryblack commented 5 months ago

Thanks, its build input output with manual placed IOBUF, but now crashed with generated:

module test_io (
    input sys_clk,  // 27 Mhz, crystal clock from board
    input sys_resetn,
    input button,   // 0 when pressed

    output [5:0] led,

    inout test_io
);

wire test_i,test_o,test_oen;
ODDR oddr_rwds(
    .CLK(sys_clk), .D0(button), .D1(~button), .TX(button), .Q0(test_i), .Q1(test_oen)
);
IDDR iddr_rwds(
    .CLK(sys_clk), .D(test_o), .Q0(led[0]), .Q1(led[1])
);
// IOBUF io_buf(
//     .I(test_i),
//     .O(test_o),
//     .IO(test_io),
//     .OEN(test_oen)
// );
assign test_io = ~test_oen ? 1'bz : test_i;
assign test_o = test_io;

endmodule

nextpnr_himbaechel::assertion_failure: Assertion failure: out_iob == net_only_drives(ctx, ci.ports.at(tx_port).net, is_iob, id_OEN, true) (/Users/andry/hobby/fpga/nextpnr/himbaechel/uarch/gowin/pack.cc:458)

yrabbit commented 5 months ago

yeah, about that. The point here is that DDR and other IOLOGIC are located directly in the IO cell and have clear, non-switched wires to IOBUF. I can't put anything in there in the middle between DDR and IO.

And if we look at what you want to implement, let’s say, using VOUT:

yosys -p "read_verilog top.v; synth_gowin -vout top.vg"

inv

Then we will see the inverter on the OEN wire. If you use DDR, then this is no longer a wire that can be cut and an inverter inserted there. So I'm afraid I can't help much here.

andryblack commented 5 months ago

Yes, sample is wrong, without inversion it is builded. I think, need clear error message instead assertion failure. Another assertion emitted if IDDR not connected to io:

wire test_i,test_o,test_oen;
// ODDR oddr_rwds(
//     .CLK(sys_clk), .D0(button), .D1(~button), .TX(button), .Q0(test_i), .Q1(test_oen)
// );
IDDR iddr_rwds(
    .CLK(sys_clk), .D(test_o), .Q0(led[0]), .Q1(led[1])
);
//assign test_io = test_oen ? 1'bz : test_i;
//assign test_o = test_io;

endmodule

nextpnr_himbaechel::assertion_failure: Assertion failure: in_iob != nullptr && in_iob->bel != BelId() (/Users/andry/hobby/fpga/nextpnr/himbaechel/uarch/gowin/pack.cc:591)