YosysHQ / nextpnr

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

Gowin. Add fix for Single Port BSRAM #1332

Closed yrabbit closed 6 days ago

yrabbit commented 1 week ago

Add description of BSRAM harness

In some cases, Gowin IDE adds a number of LUTs and DFFs to the BSRAM. Here we are trying to add similar elements.

More details with pictures: https://github.com/YosysHQ/apicula/blob/master/doc/bsram-fix.md

Compatible with older Apicula databases.

whitequark commented 1 week ago

Shouldn't this be done in Yosys during techmapping?

yrabbit commented 1 week ago

Shouldn't this be done in Yosys during techmapping?

For Tangnano4k no extra primitives are added, for Tangnano20k they are added that are different from Tangnano9k, and for szfga others are added. Is it possible to transfer to yosys the version of the Gowin chip for which to synthesize... :thinking: Do you think I should deal with this there?

gatecat commented 1 week ago

I think this kind of hardware-errata-level workaround is okay in nextpnr, if the vendor tools are also doing it during the place and route step.

I just want to double check, these aren't only being added during memory inference, but also when you instantiate the primitives directly in the vendor tools?

yrabbit commented 6 days ago

Definitely. Here is one of the working files that I am running through Gowin IDE. I use a pure primitive and not a single additional element. But in the resulting binary image, the output pin of the R1C42_OB chip is equipped with the R11C23_LUT4_0 LUT, which works in conjunction with R11C23_DFFE0. In addition, the input signals WRE and CE do not pass directly, but through the LUTs R9C23_LUT4_7 and R9C23_LUT4_6.

And yes, in the new (relatively) families of chips used in Tangnano9k and Tangnano20k, some defect with the WRE and CE signals was corrected and these elements are not added, but it seems that the built-in output register was broken and therefore DFFs are still added, but according to a different scheme and for another reason.

`default_nettype none
module top(
    input wire clk,
    input wire rst_i,
    input wire wre,
    input wire ce,
    input wire oce,
    input wire [7:0]  in,
    input wire [10:0] addr,
    output wire [7:0] led
);

    wire gnd, vcc;
    assign gnd = 1'b0;
    assign vcc = 1'b1;

    //wire [27:0] dummy;
    wire [34:0] dummy;

    SPX9 mem(
        .DO({dummy, led[0]}),
        .DI({{28{gnd}}, in}),
        .AD({addr, gnd, gnd, gnd}),
        .CLK(clk),
        .CE(ce),
        .WRE(wre),
        .OCE(oce),
        .BLKSEL(3'b000),
        .RESET(rst_i)
    );
    defparam mem.READ_MODE = 1'b0;
    defparam mem.WRITE_MODE = 2'b01;
    defparam mem.BIT_WIDTH = 9;
    defparam mem.BLK_SEL = 3'b000;
    defparam mem.RESET_MODE = "ASYNC";

endmodule

shot-0

whitequark commented 6 days ago

Thanks for clarification. In this case I feel like you have one of the two reasonable options:

  1. Do it in nextpnr and give up uptimization opportunities.
  2. Do it in Yosys by creating eg an internal GOWIN_BSRAM cell that represents a "bare" cell, and a user-facing eg SPX9 cell that adds the LUTs.
yrabbit commented 6 days ago

For now, I’ll stick to the option with nextpnr - first we’ll work out all the errors without optimization.