Kasli-soc: add WRPLL clock recovery #282
No reviewers
Labels
No Milestone
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: M-Labs/artiq-zynq#282
Loading…
Reference in New Issue
No description provided.
Delete Branch "morgan/artiq-zynq:WRPLL"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Prerequisite
Summary
This PR implement WRPLL with Si549 DCXOs on kasli-soc.
The WRPLL first use a frequency counter to pull in the frequency and acquired a
BASE_ADPLL
.Then two PLL will be enabled:
f_{helper} = \dfrac{f_{GTX} * 32767}{32768}
Currently, the PR supports most existing function provided by Si5324:
calibrate_wrpll_skew
featureChangelog
Gateware
kasli-soc
ddmtd (in helper clock domain)
si549
wrpll
Firmware
rtio_clocking
runtime main
satman main
io_expander
si549
FIQ
cargo template
Measurement
TODO
@ -0,0 +72,4 @@
]
class Collector(Module):
Maybe this should move to firmware?
I tried moving the gateware collector to firmware with a gtx & main tag interrupts setup. But the CPU is not fast enough to sample two consecutive gtx tags while executing the helper PLL + main PLL. If a firmware collector is prefered, the main & helper PLL need to run alternatively to avoid cycle slipping for the beating period error calculation.
Firmware collector idea:
collector 2 gtx tags -> helper PLL -> collector gtx & main -> main PLL -> repeat
On a 1GHz Cortex A9 I find it hard to believe. Most likely this can be solved by proper design of the CPU/gateware interface i.e. still collect the related tags in gateware, but do not process them any further and just send them as-is to the CPU.
@ -206,3 +208,3 @@
class GenericMaster(SoCCore):
def __init__(self, description, acpki=False):
def __init__(self, description, acpki=False, si5324=False):
Si5324 should be the default until WRPLL is completed.
Historically this flag was called --with-wrpll.
@ -13,2 +13,4 @@
}
#[cfg(has_si549)]
const USE_SI549: u8 = 0xFF;
Something called USE_ should be a bool. Rename or restructure.
Remove Q# from the diagram. It doesn't add anything and is just clutter.
0a4c34b1f2
to7dc57dc24e
7dc57dc24e
to2f57ccf617
2f57ccf617
toec506fd0ce
Force push to address above comments, add an I term for main PLL, improve deglitcher, general clean up and update commit messages. Ready to review.
ec506fd0ce
to3ec537a5da
Force push changelog:
n * ideal_beating_period - GTX_TAG(n)
to use the same KP/KI pair for both main & helper PLL3ec537a5da
tob7694af295
Force push changelog:
125Mhz phase noise measurement on satellite MMCX J3
100Mhz phase noise measurement on satellite MMCX J3
@ -0,0 +1,551 @@
use core::result::Result::Ok;
I don't see this line anywhere else in the codebase.
Yea, no need for that line. Will be removed in next push. I think it was added by rust-analyzer plugin from vscodium.
@ -0,0 +291,4 @@
static M_INTEGRATOR: Mutex<i32> = Mutex::new(0);
#[derive(Clone, Copy)]
pub enum FIQ {
I wouldn't call it FIQ because we're going to port that to MiSoC/Kasli where there's no FIQ, and the firmware divergence should be minimized (ideally, artiq-zynq and artiq firmware should be merged at some point).
We should probably move enable-wrpll to the JSON so it can be easily enabled on AFWS builds.
b7694af295
to673e2fa6de
673e2fa6de
to0d0f9b5a6b
0d0f9b5a6b
tocbe48f9412
Force push to rebase, cleanup and add support for runtime wrpll. Internal 150Mhz si549 and PLL bypass mode are not supported.
Changelog
gtx
to the more genericref
has_wrpll
in favor of just usinghas_si549
dummy_fiq_handler
feature to fix compilation errorfiq
to the more genericisr
gtx
to the more genericref
DividerConfig
is used as reference following si5324wrpll_ref_clk="SMA_CLKIN"
sma clkin--> mmcm --> 125Mhz refclk--> wrpll
has_virtual_leds
as standalone don't need to change virtual ledsNetwork testing
Phase noise measurement
Standalone WRPLL performance compare to standalone si5324 & clocker
Effect of difference reference sources on Standalone WRPLL
cbe48f9412
toff68521bfd
ff68521bfd
to1acd9da095
Force push changelog
enable_wrpll
json fieldenable_wrpll
field in jsonhas_wrpll
cfgset_adpll
,helper_setup
outside of wrpll to separate si549 driver from wrpll moduleTAG_OFFSET
for Satman WRPLL to meet the setup/hold constraints of rx synchronizercalibrate_wrpll_skew
featurecalibrate_wrpll_skew
featureSkew calibration
--features=target_$(TARGET),calibrate_wrpll_skew
19000
19050
is used as the calibrated tag offset, it's the average result of 10 consecutive runs on one of the card.GTX_CDR -> GT_CDR
We'll want to run this on other boards with different transceiver types.
@ -0,0 +691,4 @@
fn one_clock_cycle(timer: &mut GlobalTimer) {
unsafe {
csr::wrpll_refclk::mmcm_dclk_write(1);
timer.delay_us(1);
Maximum DCLK frequency is 200MHz as per https://www.mouser.com/datasheet/2/903/ds191_XC7Z030_XC7Z045_data_sheet-1923769.pdf
You can remove the timer and delays.
1acd9da095
to6cc02cc460
6cc02cc460
to005f96a924
Force push changelog
satellite: add WRPLL clock recoveryto Kasli-soc: add WRPLL clock recovery@ -47,6 +48,19 @@ eem_iostandard_dict = {
def eem_iostandard(eem):
return IOStandard(eem_iostandard_dict[eem])
class ClockSynthesis(Module):
This is a poorly chosen name since there isn't a PLL or anything like it inside. Maybe just inline it anyway? It's just IBUFGDS really and some constraints that may be redundant and/or are dependent on the board.
005f96a924
to8fd524d36b
8fd524d36b
to137489da93
Force push changelog
ClockSynthesis
refactorclk_synth
,IBUFGDS
&period constraint
for master and satellite@ -0,0 +133,4 @@
)
]
# PL->PS interrupt
On Kasli we won't have PS/PL. But this can be addressed later.
@ -0,0 +304,4 @@
csr::wrpll::main_dcxo_adpll_stb_write(1);
csr::wrpll::main_dcxo_adpll_stb_write(0);
if csr::wrpll::main_dcxo_nack_read() == 1 {
This test can only be done after the transaction is completed and seems misplaced. Should be right after the while loop I guess, and the core should probably reset the nack status on stb.
@ -0,0 +302,4 @@
csr::wrpll::main_dcxo_adpll_write(adpll as u32);
csr::wrpll::main_dcxo_adpll_stb_write(1);
csr::wrpll::main_dcxo_adpll_stb_write(0);
Writing 0 is superfluous if you use CSR() in the core.
Wording of messages and code comments could be improved.
@ -0,0 +364,4 @@
static REF_TAG: Mutex<u32> = Mutex::new(0);
static REF_TAG_READY: Mutex<bool> = Mutex::new(false);
static MAIN_TAG: Mutex<u32> = Mutex::new(0);
static MAIN_TAG_READY: Mutex<bool> = Mutex::new(false);
Why do we need all those mutexes? Only one thread is accessing those, no?
Yes, will change it to
static mut
@ -0,0 +650,4 @@
info!("warming up refclk...");
// refclk need a couple seconds for freq counter to read it properly
timer.delay_us(20_000_000);
That's 20 seconds, not "a couple".
What is refclk and why do we need to wait so long?
it's waiting for the gtx_cdr, sometimes the cdr is not locked/stable yet and the frequency counter will read the wrong value.
Wouldn't it be better to just wait for the CDR lock then?
When is select_recovered_clock() called?
Yes, that will also save time. Let me look in it
The select_recovered_clock(true) is called directly after drtiosat_link_rx_up
@ -0,0 +799,4 @@
let locked = unsafe { csr::wrpll_refclk::mmcm_locked_read() == 1 };
if !locked {
return Err("mmcm failed to generate 125Mhz ref clock from SMA CLKIN");
MHz
20732c0de1
toaa4bb8bae8
aa4bb8bae8
to14fa038118
Force pushed changelog
busy
CSRstatusmutex
withstatic mut
main_dcxo_adpll_stb_write(0)
nack
check just afterbusy
busy
CSRstatusTest