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

Regression from arachnepnr: failure to route SB_PLL40_2_PAD #126

Closed
swetland opened this issue Nov 17, 2018 · 11 comments
Closed

Regression from arachnepnr: failure to route SB_PLL40_2_PAD #126

swetland opened this issue Nov 17, 2018 · 11 comments

Comments

@swetland
Copy link

swetland commented Nov 17, 2018

Below is a test of using SB_PLL40_2_PAD which provides both the original clock and the synthesized clock via globals. This works in iceCube2 and with arachne-pnr but fails with nextpnr with:
ERROR: PLL 'pll0.pll_inst' is using multiple ports mapping to PLLOUT_A output of the PLL

pll-test.tar.gz

pll-test.sh

#!/bin/sh
set -ex

yosys -p 'synth_ice40 -top top -blif pll-test.blif -json pll-test.json' pll-test.v

# this succeeds
arachne-pnr -d 5k -p sg48 -o pll-test-apnr.asc -p pll-test.pcf pll-test.blif

# this fails
# ERROR: PLL 'pll0.pll_inst' is using multiple ports mapping to PLLOUT_A output of the PLL
nextpnr-ice40 --package sg48 --up5k --pcf pll-test.pcf --asc pll-test-npnr.asc --json pll-test.json

pll-test.pcf

set_io clk12m_in 35

set_io out1 3
set_io out2 4

pll-test.v

// instantiate pll with both original and generated clocks available
// pass clocks to test points out1, out2

module top(
	input clk12m_in,
	output out1,
	output out2
	);

wire clk25m;
wire clk12m;

pll pll0(
	.PACKAGEPIN(clk12m_in),
	.PLLOUTGLOBALA(clk12m),
	.PLLOUTGLOBALB(clk25m),
	.PLLOUTCOREA(),
	.PLLOUTCOREB(),
	.LOCK(),
	.RESET(1'b1)
	);

assign out1 = clk25m;
assign out2 = clk12m;

endmodule


// below generated by lattice icecube2

module pll(PACKAGEPIN,
           PLLOUTCOREA,
           PLLOUTCOREB,
           PLLOUTGLOBALA,
           PLLOUTGLOBALB,
           RESET,
           LOCK);

inout PACKAGEPIN;
input RESET;    /* To initialize the simulation properly, the RESET signal (Active Low) must be asserted at the beginning of the simulation */ 
output PLLOUTCOREA;
output PLLOUTCOREB;
output PLLOUTGLOBALA;
output PLLOUTGLOBALB;
output LOCK;

SB_PLL40_2_PAD pll_inst(.PACKAGEPIN(PACKAGEPIN),
                        .PLLOUTCOREA(PLLOUTCOREA),
                        .PLLOUTCOREB(PLLOUTCOREB),
                        .PLLOUTGLOBALA(PLLOUTGLOBALA),
                        .PLLOUTGLOBALB(PLLOUTGLOBALB),
                        .EXTFEEDBACK(),
                        .DYNAMICDELAY(),
                        .RESETB(RESET),
                        .BYPASS(1'b0),
                        .LATCHINPUTVALUE(),
                        .LOCK(LOCK),
                        .SDI(),
                        .SDO(),
                        .SCLK());

//\\ Fin=12, Fout=25;
defparam pll_inst.DIVR = 4'b0000;
defparam pll_inst.DIVF = 7'b1000010;
defparam pll_inst.DIVQ = 3'b101;
defparam pll_inst.FILTER_RANGE = 3'b001;
defparam pll_inst.FEEDBACK_PATH = "SIMPLE";
defparam pll_inst.DELAY_ADJUSTMENT_MODE_FEEDBACK = "FIXED";
defparam pll_inst.FDA_FEEDBACK = 4'b0000;
defparam pll_inst.SHIFTREG_DIV_MODE = 2'b00;
defparam pll_inst.PLLOUT_SELECT_PORTB = "GENCLK";
defparam pll_inst.ENABLE_ICEGATE_PORTA = 1'b0;
defparam pll_inst.ENABLE_ICEGATE_PORTB = 1'b0;

endmodule
@smunaut
Copy link
Contributor

smunaut commented Nov 17, 2018

Dupe of #69

Try https://github.com/smunaut/nextpnr/tree/ice40_work for a fix.

@swetland
Copy link
Author

Your branch does fix this specific testcase (yay) -- pnr succeeded, I verified 12M and 25M clocks at the testpoints on the board, everything's happy.

However for a nontrivial project (VGA character display, soft spi peripheral to read/write video memory) the pnr now gets "stuck" around here:
Info: at iteration #125: temp = 8.185582, cost = 4579, est tns = -162.50ns

It's been sitting there at 100% cpu for 5 minutes now apparently making no progress.

Of course this could be an unrelated bug. I'll do a bit more exploring.

@smunaut
Copy link
Contributor

smunaut commented Nov 17, 2018

Yes, that sounds like a distinct issue.
If you have a test case (or even just the source .json / .pcf) to reproduce, feel free to post that.

@swetland
Copy link
Author

I haven't narrowed it down to a specific test case yet. Here's the json+pcf:
stuck.tar.gz

$ nextpnr-ice40 --package sg48 --up5k --pcf top.pcf --asc top.asc --json top.json

@smunaut
Copy link
Contributor

smunaut commented Nov 17, 2018

Yeah it gets stuck in an infinite loop in random_bel_for_cell().

I would suggest opening another issue for this.

@smunaut
Copy link
Contributor

smunaut commented Nov 17, 2018

Although ... wait ... might actually be somewhat related since the cell it's trying to place is a SB_GB.

@smunaut
Copy link
Contributor

smunaut commented Nov 17, 2018

in ice40/pack.cc in function bool Arch::pack(), you can try commenting out promote_globals()

This works around it for now.

@swetland
Copy link
Author

Can confirm that the design works correctly (same as w/ arachne-pnr and icecube2) with that workaround. Thanks!

Happy to open a separate issue for this if that's useful/desirable.

@smunaut
Copy link
Contributor

smunaut commented Nov 17, 2018

Yeah, I would open a different issue. This random_bel_for_cell seems pretty bad for resources that are "rare" in the FPGA. (i.e. when there is only a few of them)

@smunaut
Copy link
Contributor

smunaut commented Nov 20, 2018

I think this should all be fixed now too.

@swetland
Copy link
Author

Confirmed -- tip of tree Yosys + NextPNR is working fine for both the small test case and some larger projects.

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