chipsalliance / chisel

Chisel: A Modern Hardware Design Language
https://www.chisel-lang.org/
Apache License 2.0
3.92k stars 589 forks source link

Digital Top with Analog Black Box #525

Open shunshou opened 7 years ago

shunshou commented 7 years ago

I think I made a really poor assumption several months ago that I'm hoping someone with mixed signal design experience can clarify. If you have a digital top and you want to add in, say, an ADC black box, how would one go about doing that in Chisel? I assume you really just need to map the instance ports correctly, but in that case, is there still a notion of Width for AnalogType? And is inout really sufficient? Say you wanted to model your ADC with actual SystemVerilog and simulate via VCS. In that case, you'd have a 1-wide inout (or in) port that is of type "real", right? Anyways, my point is, fundamentally, AnalogType should not be of the same kind as UInt, SInt. If Analog is referring to in/out/inout behavior, it should be orthogonal to UInt, SInt, and ideally, Real. And I'm confused how the blackbox should be hooked up in Verilog.

ben-k commented 7 years ago

The only concrete answer that I can give to the above is that AnalogType does have a Width, and you can bulk-connect Analog IOs with attach().

Regarding your broader question (as I understand it), you may be stretching the Chisel use case beyond its original intent. It seems like you're requesting that a multi-bit Analog bus be (optionally?) emitted as a single-port real for verification purposes. Given that Chisel/firrtl is meant to emit only synthesizable verilog, I'm not sure we want Chisel emitting reals under any circumstances.

In our project, we connect our BlackBoxes in the straightforward way (normal Chisel wiring when possible, AnalogType Attach when inouts are necessary). But we do not use these more sophisticated SystemVerilog models that you're proposing, and I'm not sure how it would be possible in a unified, Chisel-top environment.

I imagine that this idea of having a different port interface for system integration vs. testing poses challenges under any circumstance. Do you have any idea how this is done currently? What exactly is the status quo that we'd like to improve upon?

shunshou commented 7 years ago

Actually for DSP, we are already using Reals and it works quite well. I just haven't thought enough about if the exact same thing can be applied toward analog models -- at least through Verilator. I think it works. I think there should be some switch that lets Chisel output either a completely black boxed thing to be swapped with some analog macro or a fake SystemVerilog style blackbox that does some simple Real based modeling.

shunshou commented 7 years ago

@chick @jackkoenig Revisiting this question -- For both of the chips going out in <1 month, we're trying to do some SystemVerilog + Chisel-generated Verilog modelling.

Can someone work on a way to potentially tag certain analogType ports (annotations?) as "real"?

As I mentioned before, directionality is orthogonal to functionality, and there's more to support than just stuff that can be "Z."

Additionally, Verilator supports real -- In that sense, I don't understand why we can't peek/poke into analogTypes at least for the Verilator + VCS backends.

And finally, can someone please add -sverilog to the VCSBackend? Without it, none of the dsptools DspReal stuff will simulate.

@stevobailey

chick commented 7 years ago

@stevobailey I don't think I know enough to do the analog port tagging without some coaching I have created a chisel-testers branch add-verilog-flag-to-vcs-backend. I don't have a way of testing it. Can you try it and if it works I'll PR it.

jackkoenig commented 7 years ago

@shunshou I'm not 100% sure what you're asking for. What would you like the "real" tag to do? It's easy enough to annotate them but obviously you want to do something with this information.

Directionality is orthogonal to functionality, but not for Chisel Analog. They can only represent bidirectional bit vectors.

For real, does it not make more sense just to use UInts that you interpret correctly in the blackboxes where you do your real operations?

shunshou commented 7 years ago

Yea that's what I figured would be best: See

https://github.com/ucb-bar/dsptools/issues/92

But you need to optionally tag the UInt to spit out input real signalName for simulation and input signalName for synthesis (w/ analog black box).

colinschmidt commented 7 years ago

Does an ifdef SYNTHESIS based on the annotation solve the problem? Is it that you can't consume annotations in the firrtl verilog emitter or some other technical limitation that is the root of this problem?

shunshou commented 7 years ago

So Stevo/Paul's thing (hack! not a long-term solution) does an ifdef SYNTHESIS, but there's some annoyance with Verilog "Real" not being supported by "Inout" so I think they hack it so that all of their annotated AnalogTypes are "In" and "Real" for simulation and I guess still "In" but not "Real" for synthesis (since it really is an ADC input...). It's a problem of confusing directionality and representation...