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. Add fix for Single Port BSRAM #1332

Merged
merged 2 commits into from
Jun 25, 2024
Merged

Conversation

yrabbit
Copy link
Contributor

@yrabbit yrabbit commented Jun 23, 2024

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.

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

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

Shouldn't this be done in Yosys during techmapping?

@yrabbit
Copy link
Contributor Author

yrabbit commented Jun 23, 2024

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... 🤔
Do you think I should deal with this there?

@gatecat
Copy link
Member

gatecat commented Jun 24, 2024

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

yrabbit commented Jun 24, 2024

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

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.

It seems that the internal registers on the BSRAM output pins in
READ_MODE=1'b1 (pipeline) mode do not function properly because in the
images generated by Gowin IDE an external register is added to each pin,
and the BSRAM itself switches to READ_MODE=1'b0 (bypass) mode .

This is observed on Tangnano9k and Tangnano20k boards.

Here we repeat this fix.

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

yrabbit commented Jun 25, 2024

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

@gatecat gatecat merged commit 2e8280a into YosysHQ:master Jun 25, 2024
8 checks passed
@yrabbit yrabbit deleted the bsram-fix-sp branch June 25, 2024 10:10
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

Successfully merging this pull request may close these issues.

3 participants