pepijndevos changed the topic of #apicula to: Project Apicula: bitstream documentation and tooling for Gowin FPGAs https://github.com/YosysHQ/apicula -- logs https://freenode.irclog.whitequark.org/apicula
mmicko has quit [Ping timeout: 264 seconds]
mmicko has joined #apicula
<pepijndevos_> kbeckmann, one thing you could try is remove the -vout part, run splitnets again, and then manually copy the write_verilog part. That would tell us if splitnets is simply in the wrong place or there is a deeper issue.
<kbeckmann> pepijndevos_: that worked!
<pepijndevos_> Cool! Now the hard part is figuring out why...
<kbeckmann> wait sorry i might have celebrated too early
<kbeckmann> but splitnets definitely did some changes. for some reason i can't reproduce the original issue now.
<pepijndevos_> What I'd do is run synth_gowin up until the splitnets call, and then run it in small steps with -run <from_label>[:<to_label>] or by manually copy-pasting commands, and then with show figure out when a bus was introduced
<pepijndevos_> So you have another bug now?
<kbeckmann> ok sorry i celebrated too early
<kbeckmann> doing what you suggested, calling splitnets and then running write_verilog, produces different output. however it still does the wide assignment, just in a different way
<kbeckmann> top_raw.vg is the output from -vout
<kbeckmann> this looks better but i guess it still has to be on two separate lines:
<kbeckmann> + assign { \cd_sync.timer[2]_LUT3_I0_F[3] , \cd_sync.timer[2]_LUT3_I0_F[2] } = { \cd_sync.timer[5] , \cd_sync.timer[3] };
<kbeckmann> also i think this is illegal verilog per the discussion about \ yesterday. shouldn't there be a space between the name and the array operator?
<trabucayre> kbeckmann: when pergola will be publicly available? (if it's planned)
<kbeckmann> oh soon
<kbeckmann> i have procrastinated on that project for so long
<kbeckmann> the issue is that i have sporadic jtag programming failures which i can't seem to figure out why they happen
<kbeckmann> and i don't want to release something half-assed
<trabucayre> hardware issues or software ?
<kbeckmann> honestly i don't know
<trabucayre> through imx ?
<kbeckmann> also i want to finish the usb 2.0 HS interface so you can send data to/from the fpga quickly. that shouldn't be much work but i have not yet implemented it.
<kbeckmann> yep
<trabucayre> interesting
<kbeckmann> i am using Apollo (from LUNA) now
<kbeckmann> and basically bitbanging jtag
<kbeckmann> and for some reason, it works most of the time, but not always
<kbeckmann> and sometimes it seems that some specific bitstreams are more problematic than others
<trabucayre> strange
<trabucayre> too big pullup for imx?
<kbeckmann> i have 4k7 pull-ups on all lines except for tck that has a 4k7 pull down
<trabucayre> have you tried without (imx issue).
<kbeckmann> no, but i could try that
<kbeckmann> reading the sysConfig datasheet again for the ECP5 and it seems they only suggest a 4k7 pull-down on TCK, nothing on the other lines.
<kbeckmann> because the chip has internal weak pull-ups
<trabucayre> it's maybe stupid but orangeCrab has no pull(up|down), I've added a couple for similar issue with usbBlaster, result is worse
<kbeckmann> huh interesting
<kbeckmann> good that it's easy to remove them
<kbeckmann> (harder to add when you don't have a footprint!)
<trabucayre> but i'm not expert in electronic :)
<pepijndevos_> kbeckmann, Okay so there is definitely an issue with splitnets but also still a but where yosys doesn't break up the assigns.
<kbeckmann> okay
<pepijndevos_> well not really a bug... more of... missing feature I guess
<kbeckmann> ah
<kbeckmann> anything i can do to help out?
<pepijndevos_> fix yosys XD
<pepijndevos_> Well, what you can try is what I suggested where you execute synth_gowin line by line and see why it doesn't get split up properly.
<pepijndevos_> I'm guessing it may need to be moved to after all the techmap stuff.
<pepijndevos_> or just yolo move it to the end of map_cells and see if that fixes it.
<pepijndevos_> It won't fix the assignment but it should split the net at least
<pepijndevos_> I'm thinking what's happening is that the course cells have bus inputs themselves so it can't split them.
<kbeckmann> heh, fixing yosys would be cool but i am far from able to do that, as of now at least... i can try the iterative approach that you suggested.
<pepijndevos_> Yea fixing yosys... I'm also not sure what it'd take.
<pepijndevos_> I think it should be pretty simple to make a test case for that...
<pepijndevos_> kbeckmann, ^
<pepijndevos_> I think moving splitnets down would still be a good PR if that works for you
<kbeckmann> alright, i will try some more. i finally made a "tiny" reproducible test case: https://gist.github.com/kbeckmann/db5ca074c5926c78dedf53dd0e9d5bdd
<kbeckmann> the verilog input code is small at least
<kbeckmann> seems to be triggered by the comparison, `assign led = counter == 8'd42;`
<kbeckmann> and it only starts happening once the size of counter is [8:0] or larger
<kbeckmann> simplified it a bit more.. `assign led = counter == 1'b0;` also triggers the bug
<pepijndevos_> kbeckmann, but this is the top-level case, right? Not the internal one that should be flattened and split.
<pepijndevos_> I'll give your initial nmigen case a try...
<kbeckmann> no, the top level in my example has only clk24 (single wire) and led (single wire). the assignment of the internal signal counter should be flattened
<kbeckmann> if you take a look at the last gist i posted a few lines above, the verilog is quite simple.
<pepijndevos_> uhu
<kbeckmann> or actually, it seems it's assigning a generated signal called led_OBUF_O_I_LUT4_F_I2
<pepijndevos_> yea... I have a hunch what's going on
<kbeckmann> nice!
<pepijndevos_> It's running iopadmap after splitnets, and since splitnets does not have the -ports option it does not touch the output net
<pepijndevos_> this fixes your example for me
<pepijndevos_> please test if the original code still fails
<kbeckmann> awesome, i am building and testing now
<kbeckmann> seems to work...!
<pepijndevos_> So maybe no need to to tricky fixes in yosys after all?? I mean... technically Yosys could generate invalid code, but if stuff is properly flattened, I'm not sure it ever will.
<pepijndevos_> "invalid"=="code that Gowin PnR can't parse"
<kbeckmann> ok it fixed it for the blinky case, however it didn't solve it for my lcd example
<kbeckmann> assign lcd_0__b__io = { \pin_lcd_0__b.lcd_0__b__io[4] , \pin_lcd_0__b.lcd_0__b__io[3] , \pin_lcd_0__b.lcd_0__b__io[2] , \pin_lcd_0__b.lcd_0__b__io[1] , \pin_lcd_0__b.lcd_0__b__io[0] };
<kbeckmann> phew
<kbeckmann> this will be hairy to debug
<kbeckmann> ah
<kbeckmann> doh sorry.. this code uses wide io ports
<kbeckmann> so let's take one step at a time.. the PR solves the problem.
<pepijndevos_> which... should be supported.
<kbeckmann> hmm
<pepijndevos_> Okay, can you try compiling with splitnets -ports and see if that fixes it? I don't really want to do that as it messes up constraints, but it could be a workaround.
<kbeckmann> will try
<pepijndevos_> (by mean "should be suppported" I mean that it's most definitely broken now, but should be fixed.
<pepijndevos_> )
<kbeckmann> ah gotcha
<pepijndevos_> I'm thinking... in the constraints you specify a wire port as foo[0] right? And... that's a valid verilog variable name?? Yosys seems to generate wire names with [] in it.
<pepijndevos_> In that case -ports would not actually break constraints and I would be in favour of it.
<kbeckmann> in the lcd case, yes
<kbeckmann> IO_LOC "lcd_0__b__io[0]" 41;
<kbeckmann> IO_LOC "lcd_0__b__io[1]" 42;
<kbeckmann> IO_LOC "lcd_0__b__io[2]" 43;
<kbeckmann> maybe i shouldn't quote them?
<kbeckmann> maybe i should write this instead
<kbeckmann> IO_LOC lcd_0__b__io[0] 41;
<pepijndevos_> Dunno the details of their constraint file format
<pepijndevos_> the default is '[]:'.
<pepijndevos_> So hey that would actually be fine
<pepijndevos_> So the big question is, does it solve your issue
<kbeckmann> run("splitnets -ports");
<kbeckmann> this worked for me
<pepijndevos_> woooo
<pepijndevos_> okay, I'll add that to my pr
<kbeckmann> now my lcd code runs on the device and looks pretty
<pepijndevos_> coooool
<kbeckmann> the pin names should be quoted, if they are not i get a syntax error
<kbeckmann> top.cst":5 | 'syntax error' near token '['
<kbeckmann> Physical Constraint parsed completed with errors
<pepijndevos_> good to know
<kbeckmann> thanks again for the fix in yosys, good stuff!
<kbeckmann> now, time for me to go back and support TBUF and IOBUF. the seem to still be a bit troublesome when used through nMigen.
<pepijndevos_> Glad to help. Really happy to see Gowin work with nMigen, and looking forward to Apicula support :))))
<kbeckmann> yeah!
<kbeckmann> i made a quick and dirty one earlier, not sure if it still works
<pepijndevos_> Yea, better to wait until nextpnr works properly, including constraints
<kbeckmann> my main gripe were the constrants, yeah
<kbeckmann> quite the hack but it worked
<kbeckmann> my goal right now is to make a best effort platform file for the official toolchain and upstream it to nmigen, and then we can add apicula and try to reach feature parity eventually.
<pepijndevos_> yesss
<kbeckmann> urgh. it seems that the official PnR tool doesn't support wires in the IOBUF and TBUF primitives
<kbeckmann> wires to the actual ports/pins that come from the top module. it seems it requires you to specify those directly. i have no idea how to work around this...