-
Notifications
You must be signed in to change notification settings - Fork 58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
vendor.altera: use buffer primitives #221
Conversation
Codecov Report
@@ Coverage Diff @@
## vendor.altera #221 +/- ##
==============================================
Coverage 82.34% 82.34%
==============================================
Files 33 33
Lines 5499 5499
Branches 1178 1178
==============================================
Hits 4528 4528
Misses 837 837
Partials 134 134 Continue to review full report at Codecov.
|
So, |
376180f
to
c8e6ec2
Compare
@ZirconiumX Looks like you can use the Note that you still need to verify whether those will work on |
See also this post. |
c8e6ec2
to
3c3ab4a
Compare
I fixed the mangled branch history, sorry. |
Looks good. The next steps would be to add DDR support and then factor out common code so that it's not quite as repetitive. (I think DDR suppor would influence the way you can refactor this.) |
Should I put that in a separate PR, or just continue this? |
Continue this. |
I've been working from UG83105, which also mentions the differential I/O primitives. However, I still need to hunt for what a useful DDR primitive would be, given that using IP blocks seems troublesome. |
I think you might be able to generate IP cores with |
9a5f677
to
7c1ff11
Compare
I'm unsure if this should advertise SDR support or not at the moment. Thoughts? |
It should. I won't merge the complete backend (i.e. the |
While writing f24137b a question occurred to me: what's the correct clock to use for an output-enable? |
The output clock. |
7c1ff11
to
fdb9937
Compare
Have you looked at how the other vendor packages solve this? E.g. Xilinx? I would expect that a similar code structure would work for Altera. |
I have, but I found them very confusing and difficult to understand. You're welcome to refactor my code to fit in better though. |
Sure, but you'll need to add DDR support first, even if it doesn't work. |
Writing ba3dc3c felt like wading through mud; did Altera need to make the cells this difficult to use? I've not done tristate/bidirectional stuff yet because I'm not sure I can summon the motivation for it; it's just painful to work on. |
I've done some more research over the |
|
Yeah it's not great, but I don't know of any better solutions. |
Well I'm not actually convinced yet that |
It doesn't like it. Given: module top(input clk, p_port, n_port, output i0, i1);
wire p_posedge, p_negedge, n_posedge, n_negedge;
altddio_in #(.width(1)) foo_ddr_p_0 (.datain(p_port), .inclock(clk), .dataout_h(p_posedge), .dataout_l(p_negedge));
altddio_in #(.width(1)) foo_ddr_n_0 (.datain(n_port), .inclock(clk), .dataout_h(n_posedge), .dataout_l(n_negedge));
altiobuf_in #(.NUMBER_OF_CHANNELS(1), .ENABLE_BUS_HOLD("FALSE"), .USE_DIFFERENTIAL_MODE("TRUE")) foo_buf_p_0 (.datain(p_posedge), .datain_b(n_posedge), .dataout(i0));
altiobuf_in #(.NUMBER_OF_CHANNELS(1), .ENABLE_BUS_HOLD("FALSE"), .USE_DIFFERENTIAL_MODE("TRUE")) foo_buf_n_0 (.datain(p_negedge), .datain_b(n_negedge), .dataout(i1));
endmodule I get
|
OK. Looks like it is indeed not supported. So you should set |
module top(clk50_0__io, p_port, n_port, i0, i1);
input clk50_0__io;
(* altera_attribute = "-name IO_STANDARD \"LVDS\" " *) input p_port, n_port;
output i0, i1;
wire buf_out;
altiobuf_in #(.NUMBER_OF_CHANNELS(1), .ENABLE_BUS_HOLD("FALSE"), .USE_DIFFERENTIAL_MODE("TRUE")) foo_buf_p_0 (.datain(p_port), .datain_b(n_port), .dataout(buf_out));
altddio_in #(.width(1)) foo_ddr_p_0 (.datain(buf_out), .inclock(clk50_0__io), .dataout_h(i0), .dataout_l(i1));
endmodule This works okay. Guess the altddio documentation is wrong here. |
Worth noting is that Quartus is extremely picky about the I/O standard here. Without the |
That's normal. It should already be applied by nMigen elsewhere in platform code. |
Presumably then I need to implement DDR tristate/bidirectional I/O for completeness? |
Okay, I believe this is feature-complete. The resulting Verilog needs to be checked and tested though. |
Looks good at a quick glance. Thank you for the contribution. I will check this on real hardware at some point soon, and then merge while adjusting anything that needs changing. |
This adds SDR support for Altera chips, or at least enough flops and buffers for it.