dragonphy2 icon indicating copy to clipboard operation
dragonphy2 copied to clipboard

Review initial state of pi_ctl_cdr

Open sgherbst opened this issue 5 years ago • 2 comments

Based on our discussion today, we want to make sure that there is not a start-up issue here (x's or values that prevent proper starting of clk_adc)

sgherbst avatar Apr 30 '20 01:04 sgherbst

This is the code snippet showing how the PI code is set: https://github.com/StanfordVLSI/dragonphy2/blob/2d280b99c7884aedabea331e1e2498c6bb49c457/vlog/new_chip_src/digital_core/digital_core.sv#L163-L171

Since en_ext_max_sel_mux currently defaults to 0, it relies on the analog core to have a reasonable value for max_sel_mux. The analog core, in turn, computes max_sel_mux from Qperi, which currently defaults to int_Qperi. Finally, int_Qperi is computed from combo logic on arb_out, which is all x's on startup.

At a minimum, I think we should change the default for en_ext_max_sel_mux to 1. But we might also consider solving through #62 (be able to set PI codes directly from the outside), and default to external codes on startup.

Last thing -- why does this currently work in simulation? The reason has to do with the way that Verilog simulators handle x's in conditional statements. Consider this code that computes int_Qperi: https://github.com/StanfordVLSI/dragonphy2/blob/2d280b99c7884aedabea331e1e2498c6bb49c457/vlog/new_chip_src/analog_core/PI_local_encoder.sv#L84-L92

When the boolean condition evalues to x, the simulator evaluates the the else condition, setting int_Qperi to Nunit-1. So x's are blocked at this point, resulting in a misleading sense of security...

sgherbst avatar May 01 '20 19:05 sgherbst

I'll assign this @zamyers since it seems both solutions mentioned above involve updates to digital_core or its register defaults. But it would be good to get some feedback from @sjkim85 on this too.

sgherbst avatar May 01 '20 19:05 sgherbst