YosysHQ / nextpnr

nextpnr portable FPGA place and route tool
ISC License
1.32k stars 245 forks source link

`nextpnr-ice40` is failing to route PLL clock #71

Closed cr1901 closed 6 years ago

cr1901 commented 6 years ago

Actual Behavior

The following Verilog module:

/* Machine-generated using Migen */
module top(
    input clk16,
    input usb_d_p,
    input usb_d_n,
    output usb_pullup,
    output user_led
);

wire sys_clk;
reg sys_rst = 1'd0;
wire clk16_1;
wire clk16b;
reg [23:0] count = 24'd0;

// Adding a dummy event (using a dummy signal 'dummy_s') to get the simulator
// to run the combinatorial process once at the beginning.
// synthesis translate_off
reg dummy_s;
initial dummy_s <= 1'd0;
// synthesis translate_on

assign clk16_1 = clk16;
assign usb_pullup = 1'd0;
assign user_led = count[22];

always @(posedge sys_clk) begin
    count <= (count + 1'd1);
    if (sys_rst) begin
        count <= 24'd0;
    end
end

SB_PLL40_CORE #(
    .DIVF(6'd63),
    .DIVQ(3'd6),
    .DIVR(1'd0),
    .FILTER_RANGE(3'd1)
) SB_PLL40_CORE (
    .REFERENCECLK(clk16_1),
    .RESETB(1'd1),
    .PLLOUTCORE(clk16b)
);

SB_GB SB_GB(
    .USER_SIGNAL_TO_GLOBAL_BUFFER(clk16b),
    .GLOBAL_BUFFER_OUTPUT(sys_clk)
);

results in a design which passes timing according to nextpnr, but results in a nonfunctional bitstream (no LED blinking). Modifying the above Verilog module to remove the PLL and route the system clock directly to the logic creates a functional bitstream (LED blinking). I have reproduced it below:

/* Machine-generated using Migen */
module top(
    input clk16,
    input usb_d_p,
    input usb_d_n,
    output usb_pullup,
    output user_led
);

wire sys_clk;
reg sys_rst = 1'd0;
wire clk16_1;
reg [23:0] count = 24'd0;

// Adding a dummy event (using a dummy signal 'dummy_s') to get the simulator
// to run the combinatorial process once at the beginning.
// synthesis translate_off
reg dummy_s;
initial dummy_s <= 1'd0;
// synthesis translate_on

assign sys_clk = clk16_1;
assign clk16_1 = clk16;
assign usb_pullup = 1'd0;
assign user_led = count[22];

always @(posedge sys_clk) begin
    count <= (count + 1'd1);
    if (sys_rst) begin
        count <= 24'd0;
    end
end

endmodule

Expected behavior

As both the above modules are driven by a 16MHz clock- one from the PLL output, the other from a pin- I would expect the above two modules to behave identically.

Other information

William@William-THINK MINGW64 ~/src/tinyfpga-soc
$ uname -a
MINGW64_NT-6.1 William-THINK 2.10.0(0.325/5/3) 2018-02-09 15:25 x86_64 Msys

MCVE

MVCE is here: pll-crash2.zip

smunaut commented 6 years ago

@cr1901 Do you still have this issue ? I just tried a similar design (PLL with output manually connected to a SB_GB) and it works fine here.

It's still not the "right" thing (wasing a buffer instead of using the one dedicated to the PLL), but that should still be working, at least for a led blink.

cr1901 commented 6 years ago

@smunaut I still have this issue as of commit 9472b6d (using the first snippet of Verilog code with the manually instantiated SB_GB).

smunaut commented 6 years ago

Just for the record, issue was identified as next-pnr placing the PLL instance in the bottom PLL BEL ... which is not available in that package because it doesn't process the LOCKED attributes from the chipdb.

I have a couple of commits to allow to workaround that and other fixes in my 'work' tree https://github.com/smunaut/nextpnr/tree/ice40_work I'll properly submit those once the PLL global stuff has been reviewed.

smunaut commented 6 years ago

I think this can be closed. Works fine on my tinyfpga BX now.