Origen-SDK / o2

MIT License
4 stars 0 forks source link

Timing prototype #63

Closed coreyeng closed 4 years ago

coreyeng commented 4 years ago

Hello,

I've been focused pretty solely on pins recently so I decided to start messing around with timing to break up the monotony a bit.

This PR adds simple timesets to the dut. I've got some placeholder stuff for complex timing elements, but they'll be a future item. See the tests for what's currently supported.

I followed the design pattern from #40 more closely here (unlike pins, which are kinda doing their own thing). I'm also experimenting with code reuse. I started at the pyO3 <-> Python boundary with a DictLikeAPI trait, which isn't going as well as I'd like because of the limitations of using pyo3's macros with traits. But I've been able to remove much of the boilerplate and the remaining bits are easily copy-pasted with a few easy functions to implement. I added a PyTest class as well, so you can see this trait "implements" these tests. The timeset specific setup for the trait is here, with the Timeset-specific test subclassing the general PyTest one. The PyTest class to verify the DictLikeAPI is pretty elementary, though should be a decent starting point.

As far as the timesets go, this is just a simple timeset implementation to build on in the future. Since there's no tester, there's not much timesets can do outside of just getting instantiated, but it's a start.

Thanks!

ginty commented 4 years ago

Hi @coreyeng, the dict trait stuff is really cool and definitely helpful, I think you may even consider contributing that to PYO3?

Could we separate that from the timing additions for merge?

As far as the timing goes, what's here is fine but I think it would be better if we based the internal model of it on something established/proven and I would vote for the STIL spec rather than being influenced too much by O1's simple model of timing.

I think we want our API/model to be able to express and handle everything that you can do in a STIL Timing and WaveformTable block.

Once we have that solid model of timing under our belt that should shape how we approach vector generation and that will hopefully align nicely with the infrastructure that @pderouen is currently building for that.

This is basically the approach I'm taking with regs. On the face of it, it seems like half of what it the IP-XACT equivalent for regs contains is a waste of time, but hopefully supporting it means that O2 will be able to handle anything thrown at it in future.

So for timing, I think its important that O2 will be able to handle timing scenarios that are as complex as they get, so we should build on the back of the years of thought that have gone into the STIL model.

Also another thing from O1 is that having ns as the smallest unit of time was cutting it fine, and I've already wished I could make an edge between ns in some simulations. So here we should probably align to whatever SystemVerilog's base time unit is.

ginty commented 4 years ago

Here's the STIL spec btw - https://ieeexplore.ieee.org/document/4432411

coreyeng commented 4 years ago

Hi @ginty,

Thanks! I can look into contributing to pyo3. It'd be fun to get involved there is well!

For timing, I've only done a simple timeset, but I can redirect efforts to supporting STIL's timing instead of just what O1 could do. I also found this which has some good examples.

Is what's in there now too far gone? I'm all for supporting everything STIL can do, but I think for just setting a simple timeset, what's there now should be sufficient. Taking a quick look at the above link, it should be pretty easy to mold what I've got now into that while still allowing a simple one-liner to instantiate a simple timeset. I can even do what we've done with Reg vs. SimpleReg.

ginty commented 4 years ago

I'm definitely OK with supporting a simple O1-like API on top for users that only need to declare a name and a period. Main thing I just wanted to highlight is that this should be our direction and that we weren't making any fundamental decisions here that would conflict with that. I'm OK if you want to merge this and then continue to add STIL-like support.