12:33 <zyp[m]> I managed to crash Sentinel by swapping out the `WBMemory` from the example with the `amaranth_soc.wishbone.sram.WishboneSRAM` that jfng added yesterday, it appears to be driving ACK wrong

12:34 <jfng[m]> WB targets are allowed to deassert ACK later than CYC

12:34 <jfng[m]> i think this is a sentinel bug

12:35 <zyp[m]> it might be, I'm not completely sure what the wishbone rules are

12:36 <zyp[m]> but the issue appears to be that ACK is still asserted when Sentinel starts fetching the next instruction, so it interprets it as the data being available already

12:37 <jfng[m]> in the "Next instruction fetch", i think the bug comes from the fact that this is a classic WB cycle, so the initiator must deassert STB immediately after ACK, and wait at least one clock cycle before reasserting it agait

12:38 <jfng[m]> @libera_cr1901_:catircservices.org ^

12:42 <zyp[m]> I'm not sure the wishbone spec agrees with your interpretation

12:43 <jfng[m]> ah, lawyering time, 1sec

12:43 <whitequark[cis]> this is a classic wishbone moment (no pun intended)

12:43 <whitequark[cis]> i really feel like we should consider migrating to AXI asap

12:43 <whitequark[cis]> AXI seems scary from a distance but it's just 5 streams

12:52 <jfng[m]> damn, i can't rule out that WishboneSRAM is actually wrong

12:53 <jfng[m]> i.e. that there must be a wait state of one clock cycle between deassertion and reassertion of STB in classic mode

12:53 <jfng[m]> if yes, this is a sentinel bug, otherwise it is the SRAM

12:53 <whitequark[cis]> that actually sounds right to me

13:04 <jfng[m]> ok, this is actually apparent in the figure zyp posted; STB is held high

13:04 <jfng[m]> this is a WishboneSRAM bug

15:07 <tpw_rules> whitequark[cis]: i would love to see a reliable set of AXI primitives

15:07 <tpw_rules> (and contribute to some degree)

15:07 <whitequark[cis]> I think everyone would :p

15:07 <whitequark[cis]> we'll get there

15:07 <whitequark[cis]> no promises as to when

15:07 <tpw_rules> of course

15:08 <tpw_rules> i had assembled a couple for my project. i think next target will be an arbiter

15:08 <tpw_rules> i need to upgrade that stuff to real streams

15:27 <cr1901_> jfng[m]: Right now, there is exactly one case where Sentinel will not deassert CYC when ACK is received; when a memory store is immediately followed by an insn fetch

15:27 <cr1901_> deassert CYC and STB*

15:28 <cr1901_> This is a WB block cycle; every other mem access is a classic cycle

15:29 <cr1901_> I could modify the microcode to do a classic cycle in that case, and I may even need to: https://github.com/cr1901/sentinel?tab=readme-ov-file#using-pdmpyprojecttoml-from-this-package See italics

15:29 <jfng[m]> i think this is valid in non-pipelined mode (i.e. without the optional STALL signal), so WishboneSRAM should accomodate it

15:32 <cr1901_> Speaking of ACK being allowed to deassert later than STB/CYC, I don't recall reading that in the spec

15:32 <cr1901_> https://catircservices.org/_matrix/media/v3/download/matrix.org/LymjPsKsrrWzXjYrgheZWlGm/image.png Why is ACK not lowering at the same cycle that CYC/STB lowers

15:34 <whitequark[cis]> if CYC&STB is deasserted no other signal is valid

15:34 <whitequark[cis]> so it doesn't matter what ACK is doing in that case

15:35 <_whitenotifier-9> [amaranth-soc] jfng opened issue #96: WishboneSRAM misbehaves during back-to-back classic cycles - https://github.com/amaranth-lang/amaranth-soc/issues/96

15:37 <jfng[m]> cr1901_: this is the case for classic cycles in pipelined mode, where the initiator has to deassert STB upon receiveing ACK, iirc

15:38 <jfng[m]> or something like that.. anw i'll reread the spec in detail and fix this

15:39 <cr1901_> Yea, I can't find anything that suggests that ACK has to lower when CYC/STB lowers- just that ACK,RTY,ERR, or some other signal has to come up in response to CYC/STb

15:39 <cr1901_> zyp[m]: wb_bus_we going low before cyc/stb goes low looks like a bug on my end tho

15:41 <cr1901_> Oh wait, no it's not

15:50 <cr1901_> jfng[m]: Nope, there's a bug on my end too: https://github.com/cr1901/sentinel/blob/next/src/sentinel/top.py#L157-L158. This is fine if I'm not doing block cycles, but since I am for exactly one case (for that juicy one cycle savings), it's wrong. I'll have to fix this

15:50 <cr1901_> zyp[m]: ^

15:51 <cr1901_> (Can I see your testbench/repo code as well?)

16:02 <tpw_rules> hm how would you round robin AXI given that each stream is kinda independent

16:02 <whitequark[cis]> there's many approaches with different tradeoffs

16:02 <whitequark[cis]> AXI is harder to build interconnect for

16:04 <cr1901_> (Actually, Idk if what I'm doing is wrong. WB spec says for block cycles "MASTER negates [STB_O] to introduce a wait state (-WSM-)". Which past-me interpreted as "if xfer initiator doesn't need wait states, then it doesn't need to deassert STB during a block write"

16:10 <zyp[m]> <cr1901_> "(Can I see your testbench/repo..." <- the code is pretty much the same as yesterday, just with `WishboneSRAM` replacing `WBMemory` and a `Decoder` thrown in to adapt the `addr_width` between the cpu and the mem

16:10 <zyp[m]> https://paste.jvnv.net/view/opreJ

16:11 <cr1901_> I wanted the pdm lock so I know which version of sentinel you're using (main or next)

16:11 <cr1901_> pyproject.toml/pdm.lock*

16:14 <zyp[m]> oh, whatever was main yesterday I guess, I haven't specified a particular ref

16:14 <cr1901_> ahhh cool

16:16 <zyp[m]> but does it even matter? at a glance I can't see any HDL differences between main and next

16:17 <cr1901_> There might not be :P... I haven't worked on it much

16:17 <cr1901_> (since the last release)

16:52 <cr1901_> jfng[m]: Just for future me when I forget again, "PERMISSION 3.40" in WB spec 4 is what I'm (ab)using in Sentinel. Sentinel never generates wait states, so I tie CYC to STB. 1/2

16:55 <cr1901_> I managed to confuse myself b/c I didn't think WB block xfers could get a throughput of 1 data xfer per clock cycle, but after rereading, I'm realizing "yes, it can, if the responder always has ACK ready in response to STB/CYC". Makes me wonder why pipelined mode exists at all

18:50 <zyp[m]> pipelined mode as in bursts? exists because it's not always feasible to have data returned in the same cycle it's requested

19:27 <_whitenotifier-9> [amaranth] bl0x commented on pull request #1164: hdl._dsl: Change FSM codegen to avoid mutating AST nodes. - https://github.com/amaranth-lang/amaranth/pull/1164#issuecomment-2209484986

19:37 <_whitenotifier-9> [amaranth] whitequark commented on pull request #1164: hdl._dsl: Change FSM codegen to avoid mutating AST nodes. - https://github.com/amaranth-lang/amaranth/pull/1164#issuecomment-2209493049

20:36 <_whitenotifier-9> [amaranth] bl0x commented on pull request #1164: hdl._dsl: Change FSM codegen to avoid mutating AST nodes. - https://github.com/amaranth-lang/amaranth/pull/1164#issuecomment-2209541126

20:39 <bl0x[m]> Amazing sed: sed -i 's/Repl(\([a-zA-Z].*\), \([a-z0-9]*\))/\1.replicate(\2)/g;s/Repl(\([0-9].*\), \([a-z0-9]*\))/Const(\1).replicate(\2)/g' *.py