StanfordVLSI / dragonphy2

Open Source PHY v2
Apache License 2.0
22 stars 2 forks source link

Small fixes #126

Closed sgherbst closed 3 years ago

sgherbst commented 3 years ago

Summary

This PR addresses three open issues:

  1. Option to trigger SRAM dump without external pin (#13)
  2. Option to clamp CDR update to +/- 1 (#20)
  3. Fix X-optimism in PI local encoder (#63)

Details

  1. I ran out of registers while implementing these features, so some of the control bits are lumped into a new 32-bit register called misc_ctrl_bits. There's plenty of extra room in this register for future expansion -- only 5 of the bits are currently being used.
  2. To trigger SRAM dumping internally: set misc_ctrl_bits[2] to 1 (to enable the feature), then set/clear misc_ctrl_bits[3] to toggle the internal dump_start pin.
  3. To clamp CDR updates, set misc_ctrl_bits[3] to 1 to enable the feature. The clamp amount can be adjusted, if needed, via a 32-bit signed register called cdr_clamp_amt. It defaults to +/-1, though, so you may not need to change it.
  4. For the X-optimism issue, I swapped in the code suggested from issue #63. Furthermore, I used formal verification to prove that it is logically equivalent to the previous implementation. That verification code (using JasperGold) is in experiments/pi_local_encoder.
  5. I found an issue where build errors encountered while running JusTAG were not reported by make.py, so I fixed that, too.
sgherbst commented 3 years ago

I will update this branch to remove the misc_ctrl_bits register, now that the allowed number of JTAG registers has been increased.

sjkim85 commented 3 years ago

It looks a new test "loopback_sram_int" is added and cpu regression fails in the test. Can you add 30ps of delay to the input clocks (ext_clk) in "/cpu_system_tests/loopback_sram_int/test.sv"?

sgherbst commented 3 years ago

Thanks @sjkim85 for catching that -- I just updated the code. The other test I added is mm_cdr_slew, which is slightly modified from mm_cdr -- did you need to make any changes to that test for the analog_core_updates branch?

sjkim85 commented 3 years ago

@sgherbst , I think I don't need to make change to that test. After this PR (#126) is merged to master I will push analog_core_update again with adding QTM of PI and slight change of V2T (which are not relevant to the new tests you added)