<zyp[m]>
the xdr1 processes are fairly slow, but they work:
<zyp[m]>
fairly slow being a 3s runtime for 10k cycles with five registers, entirely usable for shorter tests still
<whitequark[cis]>
Settle :/
<whitequark[cis]>
yeah I... don't think I like this async API at all
<whitequark[cis]>
simulator commands in their current form must die (and be replaced by calls to an object provided to the process), and I don't think I like adding so many names that are useless in synthesis to Value
<whitequark[cis]>
well, Settle in particular should just be removed, and Passive/Active should probably be replaced with something like an async with block
<zyp[m]>
yeah, a context manager for passive/active makes a lot of sense
<whitequark[cis]>
I remember discussing removing the latter two entirely, long ago, and the objection to having add_process(..., passive=True) was brought up that you sometimes want a process that's only active/passive (kind of a terrible name too) during certain periods, e.g. active iff there is an ongoing transaction
<whitequark[cis]>
I feel like add_process(..., passive=True) plus async with sim.active(): is the way to go
<zyp[m]>
yeah, that sounds reasonable to me
<whitequark[cis]>
I'm not quite sure what to do about get/set/changed
<whitequark[cis]>
namely, which namespace should they be in (the global sim module? the object passed to the process? the Value?) and whether the set of primitives is right
<whitequark[cis]>
for one I think I want wait that doesn't look at the past value
<zyp[m]>
I think I'd argue for keeping get/set as methods, considering the usefulness in having value castables implementing them
<zyp[m]>
but changed/wait/whatever could go elsewhere
<whitequark[cis]>
okay, that is a compelling argument
<whitequark[cis]>
I'm tentatively OK adding these two names to Value
<zyp[m]>
I wrote this the other day, which would work well with value castables implementing set/get: https://paste.jvnv.net/view/wYI9w
<zyp[m]>
and yeah, Settle needs to die
<whitequark[cis]>
yes, this is a very compelling example
<whitequark[cis]>
okay, it sounds like we're mostly on the same page then; great
<whitequark[cis]>
I think it's OK to land an initial async testbench support without having changed/wait/etc
<whitequark[cis]>
we will have to land those pretty quickly afterwards because of cxxsim's needs
<whitequark[cis]>
since it's basically impossible to get decent performance in cxxsim without those primitives
<whitequark[cis]>
but CXXRTL is not ready for that yet anyway (it would need a reactor in C++, and I've not even started on that)
<whitequark[cis]>
not sure what to do with Tick()
<whitequark[cis]>
await ClockSignal().changed(1) is ... long
<whitequark[cis]>
and to be fully generic it would even have to be something like
<whitequark[cis]>
await ClockSignal().changed(ClockSignal().active_level) or something
<whitequark[cis]>
I think I know another reason why I want wait primitives out of Value
<whitequark[cis]>
because it makes sense to compose wait primitives, unlike with get/set
<whitequark[cis]>
it is perfectly reasonable and very useful to do sth like await wait(bus.cyc == 1, bus.we == 1)
<zyp[m]>
yeah, I've been thinking that a wait object would effectively be a trigger (which Tick() and .changed() is) and optionally a condition, which is an expression
<zyp[m]>
so `while not (await self.valid.get()): await Tick()` would be something like `await Tick() & self.valid`
<zyp[m]>
hmm, yours reads better
<whitequark[cis]>
(I'm not sure if yours even works out due to precedence)
<zyp[m]>
would it make sense to have a clock context manager? we don't want to have to pass in the clock domain to e.g. StreamInterface.recv()
<whitequark[cis]>
no
<whitequark[cis]>
I think StreamInterface.recv should be looking up the associated clock somehow
<whitequark[cis]>
the driver of valid
<whitequark[cis]>
though, thinking more about that: maybe that's not always possible
<zyp[m]>
I think that direction would require a general solution to having clock domain information in signatures
<whitequark[cis]>
mh, I'm not sure
<zyp[m]>
by the way, what's the current intention for RFC #27?
<whitequark[cis]>
uh, I still need to revise it
<whitequark[cis]>
a request for a detailed explanatory section was made on the meeting, and writing these sections is time consuming, so I haven't
<whitequark[cis]>
but it's something to do after the 0.4 release
<zyp[m]>
I'm kinda thinking it could make sense to combine it with an initial async implementation, so we don't introduce a new API and deprecate it right away
<whitequark[cis]>
mmm, I'm unsure, people will have old testbenches they can't convert
<whitequark[cis]>
we should be able to remove Settle() asap, but getting everyone to migrate everything (and it has to be everything I think) to async might be more difficult
<whitequark[cis]>
and the yield->async deprecation cycle should probably be several releases long
<whitequark[cis]>
considering that migration to Settle() is just "remove it and swap to add_testbench_process"
<zyp[m]>
would you add yet another .add_whatever() for async testbenches, or autodetect which kind of function is passed? I currently do the latter, but having a dedicated method would be more robust
<whitequark[cis]>
latter
<zyp[m]>
I'm not sure I like that if you're also determining whether to pass the simulator context argument based on that
<whitequark[cis]>
I'd probably pass the context argument to any function that accepts it, if we go that way
<zyp[m]>
fair
vegard_e[m] has quit [Quit: Idle timeout reached: 172800s]
<_whitenotifier-3>
[amaranth] whitequark opened issue #988: `PureInterface.__repr__` should use the actual class name rather than always using "PureInterface" - https://github.com/amaranth-lang/amaranth/issues/988
<_whitenotifier-3>
[amaranth] whitequark closed issue #988: `PureInterface.__repr__` should use the actual class name rather than always using "PureInterface" - https://github.com/amaranth-lang/amaranth/issues/988
<_whitenotifier-3>
[amaranth-lang/amaranth-lang.github.io] github-merge-queue[bot] 10424c1 - Deploying to main from @ amaranth-lang/amaranth@120375dabeceaeeb1cf3fcaff2d9217b3e513cb3 🚀
<cr1901>
I get "Constraint must be one of Pins, DiffPairs, Subsignal, Attrs, or Clock, not [(pins io pmod_0:1), (pins io pmod_0:2), (pins io pmod_0:3), (pins io pmod_0:4)]"
<whitequark[cis]>
remove the inner []
<whitequark[cis]>
that being said, the correct way to create a "pmod" resource is to not do it
<whitequark[cis]>
a pmod is not a peripheral; only peripherals have corresponding resources. a pmod is a connector
<cr1901>
TypeError("Pins and DiffPairs are incompatible with other location or "
<cr1901>
TypeError: Pins and DiffPairs are incompatible with other location or subsignal constraints, but (pins io pmod_0:2) appears after (pins io pmod_0:1)
<whitequark[cis]>
you probably have something like a GPIO "peripheral" on your pmod, so it would be called "gpio"
<cr1901>
I have no idea what the hell this means
<cr1901>
ack, will change
<whitequark[cis]>
only one Pins() is allowed per a Resource or Subsignal
<whitequark[cis]>
so to make a compound resource, you want to put Pins inside Subsignal inside Resource
<whitequark[cis]>
(no, you can't put a Subsignal inside Subsignal, which is kind of silly)
<cr1901>
And my forgetful ass should've remember that from the old days
<whitequark[cis]>
the whole thing needs to be scrapped and replaced with something akin to lib.wiring, of course
<whitequark[cis]>
with resource type definitions imported from amaranth-stdio
<cr1901>
Subsignal requires a string, which isn't what I had in mind
<cr1901>
I wanted to index into pmod (which I will change into gpio) like pmod[0].{o, i, oe}
<whitequark[cis]>
that's not possible
<cr1901>
Then I'll make them individual resources
<whitequark[cis]>
you could make a series of Resource("gpio", n, Pins(...))
<whitequark[cis]>
yep that's fine
<whitequark[cis]>
I'd even call that idiomatic
<cr1901>
Ack.
<cr1901>
The idea is to add a slow-as-hell bitbang i2c impl to sentinel (if the hardware/software fits)
<whitequark[cis]>
interesting
<cr1901>
for i in range(8):
<cr1901>
m.d.sync += self.bus.dat_r.eq(self.gpio[i].i) <-- Oh. I see the problem.
<cr1901>
I mean it's great for a GPIO port that always returns the value on the 8th pin, but that's not what I want