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

Gowin: failed route io with ddr #1275

Open
andryblack opened this issue Jan 20, 2024 · 4 comments
Open

Gowin: failed route io with ddr #1275

andryblack opened this issue Jan 20, 2024 · 4 comments

Comments

@andryblack
Copy link

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 added a commit to yrabbit/apicula that referenced this issue Jan 21, 2024
There's actually very little going on on the apicula side, but a future
nextpnr fix will split each of IOLOGICA and IOLOGICB into two
directions: IOLOGIC in and IOLOGIC out. This allows us to combine two
IOLOGICs at once to work with IOBUF, which addresses issue
YosysHQ/nextpnr#1275

The nature of the fixes in this PR is such that it will work with the
old version of nextpnr and everything that successfully passes through
netxpnr will work.

Signed-off-by: YRabbit <rabbit@yrabbit.cyou>
yrabbit added a commit to yrabbit/nextpnr that referenced this issue Jan 21, 2024
Corrects the situation when it is impossible to use IOBUF with two
IOLOGIC elements at the same time - input and output.

Addresses YosysHQ#1275

This is done by dividing one IOLOGIC Bel into two - input IOLOGIC and
output IOLOGIC plus checking for compatibility of the cells located
there.

At the moment, this check is simple and allows only the combination of
DDR and DDRC primitives.

Signed-off-by: YRabbit <rabbit@yrabbit.cyou>
@yrabbit
Copy link
Contributor

yrabbit commented Jan 21, 2024

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.

YosysHQ/apicula#227

yrabbit@1ccd4e5

@andryblack
Copy link
Author

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

yrabbit commented Jan 22, 2024

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

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)

yrabbit added a commit to yrabbit/nextpnr that referenced this issue Feb 9, 2024
Corrects the situation when it is impossible to use IOBUF with two
IOLOGIC elements at the same time - input and output.

Addresses YosysHQ#1275

This is done by dividing one IOLOGIC Bel into two - input IOLOGIC and
output IOLOGIC plus checking for compatibility of the cells located
there.

At the moment, this check is simple and allows only the combination of
DDR and DDRC primitives.

Signed-off-by: YRabbit <rabbit@yrabbit.cyou>
gatecat pushed a commit that referenced this issue Mar 13, 2024
Corrects the situation when it is impossible to use IOBUF with two
IOLOGIC elements at the same time - input and output.

Addresses #1275

This is done by dividing one IOLOGIC Bel into two - input IOLOGIC and
output IOLOGIC plus checking for compatibility of the cells located
there.

At the moment, this check is simple and allows only the combination of
DDR and DDRC primitives.

Signed-off-by: YRabbit <rabbit@yrabbit.cyou>
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

2 participants