04:20 <sajattack> matrix bridge broken?

04:22 <sajattack> anyway, I'm getting an error message I don't understand from yosys as well

04:22 <sajattack> Warning: Async reset value `\r_TX_Byte [7]' is not constant!

04:22 <sajattack> https://github.com/nandland/spi-slave/blob/master/Verilog/source/SPI_Slave.v

04:24 <sajattack> I get that warning and then yosys hangs

05:02 <az0re> https://github.com/nandland/spi-slave/blob/017e2c5516f71d67c7ab1b5028fad76d8f2db2d2/Verilog/source/SPI_Slave.v#L174 -- looks like an async reset to me

05:03 <az0re> I didn't look in the yosys source or anything. Dunno if it's supposed to hang.

05:03 <az0re> But you should probably get rid of the async reset lol ;)

05:05 <az0re> sajattack: ^

05:23 <sajattack> ah ok

05:27 <sajattack> now I'm getting "Multiple edge sensitive events found for this signal!" but it's not telling me which signal

05:30 <sajattack> that seems like a bug

05:32 <sajattack> oh it just went to my logfile instead of my terminal

05:39 <sajattack> now I can't tell if it was actually hung because I'm looking at the logfile and it's doing stuff

05:40 <sajattack> but it seems like maybe the same thing over and over

05:40 <sajattack> nope it's just my design is big enough to not compile instantly now I guess

05:40 <sajattack> oops

05:42 <sajattack> oh

05:42 <sajattack> it seems like somehow my memory got inferred as LUTs

05:42 <sajattack> and wires

05:45 <sajattack> I guess this is like when quartus infers latches?

06:17 <sajattack> hmm, my if's have elses and my cases have default, what else could be wrong?

06:42 <sajattack> haha, my memory has two different addresses

06:42 <sajattack> I guess that's a problem

06:42 <sajattack> sorry to bother y'all

15:16 <ross_s> Hello, is anyone able to shed some light on some unexpected behaviour of the DP16KD primitive for the ECP5 devices?

15:16 <ross_s> Here is a gist containing the critical path report for a clock that I'm using to drive an RGMII interface, and so needs to run at 125MHz:

15:16 <ross_s> https://gist.github.com/rschlaikjer/720d2755b7fcb432b1ef652bffdc8787

15:16 <ross_s> We have some crc and output data paths that make sense to me, but also a whopping 5.8ns total time spent in `pkt_data.data.0.0.0.DOB7`, which is an inferred block ram containing the ethernet frame data to be sent.

15:16 <ross_s> Based on these propagation delays for the -6 speed grade ECP5:

15:16 <ross_s> https://symbiflow.github.io/prjtrellis-db/ECP5/timing/cell_timing_6.html#DP16KD:REGMODE_A=NOREG,REGMODE_B=NOREG

15:16 <ross_s> It looks like the 5.8ns value lines up for a DP16KD with NOREG output mode, which is the default. So, based on that, I'd assume that manually instantiating the DP16KD primitive with `.REGMODE_B("OUTREG")` would take the critical path delay down from the 5.8ns value to just below 1ns, looking at the timings lower down with the `OUTREG` option selected:

15:16 <ross_s> https://symbiflow.github.io/prjtrellis-db/ECP5/timing/cell_timing_6.html#DP16KD:REGMODE_A=NOREG,REGMODE_B=OUTREG

15:16 <ross_s> Alas, when I use this instantiation:

15:16 <ross_s> My only theory so far is that the timings for DP16KD parameterized by WRITEMODE are clobbering the timings parameterized by OUTREG? I don't really know enough about the internals to know whether this is barking up the wrong tree, I just notice that it is an extra group under cells.json.

15:16 <ross_s> https://gist.github.com/rschlaikjer/37e605571beb1a9ca0b0700328aa76ec

15:16 <ross_s> I see no change to the timing output. Is the OUTREG parameter not supported in the yosys flow (yet)? Or am I missing some detail?

15:17 <ZirconiumX> daveshah: ^

15:17 <daveshah> I don't think nextpnr takes OUTREG into account, yet

15:17 <daveshah> because its not inferred by Yosys

15:18 <daveshah> Bitstream generation should handle it correctly though

15:18 <ross_s> hmm, strange that it doesn't work with the direct primitive instantiation then

15:18 <ross_s> is there a way to verify the parameters used by nextpnr are correct?

15:19 <ZirconiumX> ross_s: It won't work with direct primitive instantiation because nextpnr does not know about OUTREG

15:19 <ZirconiumX> nextpnr doesn't care whether it's inferred or directly instantiated

15:19 <ross_s> gotcha. Is there a way to work around that?

15:20 <ZirconiumX> I suspect daveshah will possibly write a patch for it, or else you can file a bug report on nextpnr

15:21 <daveshah> Yeah, file a bug report so I don't forget and I'll look into it

15:22 <ross_s> OK, will do. If there's anything I can do to help out with the patch let me know.

15:29 <ross_s> Alright, created issue #422 https://github.com/YosysHQ/nextpnr/issues/422

17:50 <ross_s> I took a look through the nextpnr source to see if I could spot what to change - this patch seems to fix the timing analysis: https://github.com/YosysHQ/nextpnr/compare/master...rschlaikjer:rschlaikjer-regmode-timing-database?expand=1

17:50 <ross_s> But I can't spot if/where to alter the bitstream generation

17:51 <daveshah> ross_s: hang on, I'll be around in an hour or so

17:52 <daveshah> At a glance that patch looks correct but needs a performance improvement (using ArchCellInfo)

17:52 <ross_s> ok cool, I'll look into that in the meantime. thanks for taking a peek.

18:13 <ross_s> alright; I think I've moved that logic over correctly. No rush: https://github.com/YosysHQ/nextpnr/pull/423

18:29 <daveshah> ross_s: approach in the PR looks good, left some small style comments

18:30 <daveshah> Looks like the bitstream side should be done already - https://github.com/YosysHQ/nextpnr/blob/master/ecp5/bitstream.cc#L1004

18:42 <ross_s> OK, I thought that might be taking care of it but wasn't sure. Patch updated per your comments.