Clock input settings improvements #152
No reviewers
Labels
No Milestone
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: M-Labs/artiq-zynq#152
Loading…
Reference in New Issue
There is no content yet.
Delete Branch "mwojcik/artiq-zynq:clock_input_improv"
Deleting a branch is permanent. Although the deleted branch may exist for a short time before cleaning up, in most cases it CANNOT be undone. Continue?
As mentioned in https://github.com/m-labs/artiq/issues/1735 - this is the Zynq version.
The ARTIQ part was also created but I couldn't test it yet, so I did the Zynq part (logically it's pretty much the same). All builds compile, I tested this with zc706 of course, on standalone nist_qc2 (which has rtio_crg_clk_sel but no si5324) and master (si5324 but no rtio_crg_clk_sel) and of course since there's no external clock connected PLL won't lock. *
As currently there are only two configurations available for Zynq (125MHz internal or bypass, kind of - bypass/"external" was available for crg_clk_sel but not for si5324, so I added that akin to mainline ARTIQ), there wasn't much to test.
I'm also not sure on the terminology. I only inferred that before, "internal" (or "i" in ARTIQ) referred to anything that didn't bypass the PLL or CRG, and "external" (or "e") was bypassing whatever there was. Now when you think about it, there's external bypass, internal (crg?), external reference synthesized, and external from crystal (si5324) and I'm not sure if it makes much of a difference.
The
RtioClockenum in rtio_clocking.rs also reflects my findings from ARTIQ as I started working on that first. I could upload the ARTIQ branch as a WIP pull request, but I'd rather have all questions answered and fixed here and move all the changes at once.* there's also the issue that if PLL won't lock, program will not continues, nothing else will run, and thus changing the setting with
artiq_coremgtis not possible, the only option beingrecompiling the firmware to a version that ignores the settingchanging the setting manually on the SD card (or connecting external bypass clock). I mean, it was already a present issue, but with limited scope (only two options) and to change them you had to take out the SD card anyway. Could ignore it, or try to mitigate it (e.g. by trying to lock the PLL for x seconds, and returning to default int_125 if that doesn't work, with a stern warning).@ -0,0 +97,4 @@#[cfg(has_si5324)]fn setup_si5324(i2c: &mut I2c, timer: &mut GlobalTimer, clk: RtioClock) {let si5324_settings = match clk {_ => { // 125MHz output, from crystal, 7 Hz, default, also covers RtioClock::Int_125This isn't right - if the clock setting (e.g. int_100, ext0_synth0_125to125, ...) is unrecognized, it should error out, not silently select one Si5324 configuration.
It is theoretically possible to support exactly the same clock settings on KC705, ZC706, Kasli and Kasli-SoC. For KC705 and ZC706 you'd need to forward the external clock input to the Si5324 and clock RTIO from the Si5324. But I don't think NIST (@dhslichter) want this change and we should only support the existing options (which would be
int_125andext0_bypass_125- andext0_bypass_100as per their new request) on KC705 and ZC706.Also the only reason why all the mainline ARTIQ Si5324 settings aren't in artiq-zynq is they haven't been ported yet.
True that. Unlike in mainline, there are now options that can pass by unnoticed, and this should be taken care of. I'll break it down and add a warning.
I assume then since they're not necessary yet, porting them makes no sense at this point? Even though it would be the matter of just moving these settings as is?
I know you mentioned exactly
ext0_bypass_125in artiq #1735, but from the perspective of this code I can't find a functional difference between bypass_125 and _100. Would you like to have them added as separate, recognised options (that would map to just bypass internally) so it's more explicit?The situation here has absolutely no difference with mainline.
Mainline has KC705 (external/internal) and Kasli (via Si5324).
Here we have ZC706 (external/internal) and Kasli-SoC (via Si5324).
It makes sense for Kasli-SoC.
Right, just match any of them (explicitly) and select the bypass.
For our KC705 and ZC706, we use an external clock at the SMA pins, so it does not pass through the Si5324. It would need to be looped back through the FPGA to the Si5324 and then sent back in to the FPGA from there, I suppose? However, all of this seems like an inferior solution to just using the clock directly from the SMA, and skipping the Si5324 entirely, if KC705 and ZC706 are the DRTIO root.
Am I understanding your question correctly on this @sb10q?
I would add that for debugging purposes, it's useful for us to have
int_100available as well, so that we can do bench tests and verify performance at 100 MHz DRTIO clock without having to hook up to an external signal source to supply that clock.This should be covered now:
a82e7a332b/src/runtime/src/rtio_clocking.rs (L158)They don't have Kasli-SoC, they have ZC706. The root clock doesn't go through the Si5324 there and
int_100would be more complicated - needs DRP of a FPGA PLL. I don't think it's worth the trouble.Ah, I thought it goes through Si5324 on DRTIO variants - Standalone of course wouldn't work. But yeah, getting DRP to work would be more work than it's worth.
Only for the satellites.
At some point we probably want a similar clock config entry for the satellites, though I think only the RTIO frequency needs to be set. In all systems currently, the clock source is always the recovered transceiver clock. If that's doable at all - there are FPGA PLLs that need to be kept within operating specs and the simplest thing in this case would be to define the clock frequency at gateware compilation time (as is done now).
If there is no
int_100that's not a huge deal for us -- we can always find ways to supply an external 100 MHz clock. It's a convenience feature only, and if it's a lot of work it's not worth it.