search

StanfordVLSI / dragonphy2

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

Mflowgen sgh #121

Closed zamyers closed 3 years ago

zamyers commented 3 years ago

This pull request involves merging over changes made to the mflowgen flow and to verilog during the May tape-out.

sgherbst commented 3 years ago

Thanks, could you double-check the changes in vlog/chip_src/digital_core/dsp_backend.sv? I noticed the following updates:

  1. code_pipeline_depth is reduced by 1
  2. pb_buffer_depth is reduced by 2
  3. est_seq_3 is removed
  4. comb_mlsd_dec_i.est_seq is now wired to est_seq_2

Since I'm not familiar with that code, I wanted to check with you that these changes are intended.

sgherbst commented 3 years ago

Looks like the mflowgen regression test is failing because the cadence-innovus-init step is missing.

zamyers commented 3 years ago

Thanks, could you double-check the changes in vlog/chip_src/digital_core/dsp_backend.sv? I noticed the following updates:

1. `code_pipeline_depth` is reduced by `1`

2. `pb_buffer_depth` is reduced by `2`

3. `est_seq_3` is removed

4. `comb_mlsd_dec_i.est_seq` is now wired to `est_seq_2`

Since I'm not familiar with that code, I wanted to check with you that these changes are intended.

These are just changing the pipeline depths to improve timing. I will revert them to the ones that existed on the server (as those are what are actually taped out). But given I am scrapping this backend, its just for reference for that chip design.

sgherbst commented 3 years ago

OK, thanks for clearing that up. There were two other questions I had on the code -- I'll copy them here in case they got hidden:

  1. There was a block of create_clock and set_max_transition commands that were removed in the synthesis constraints. I do remember there being an issue where lots of unnecessary clock buffers were being added, but just wanted to check that is what this change was about.
  2. I saw that the mflowgen connection from signoff to gdsmerge was removed. I remember hearing about problems with gdsmerge, so I wanted to check if this was intentionally removed because of those problems, or whether is was removed as part of the debugging process and hadn't been put back yet.
zamyers commented 3 years ago

OK, thanks for clearing that up. There were two other questions had on the code -- I'll copy them here in case they got hidden:

  1. There was a block of create_clock and set_max_transition commands that were removed in the synthesis constraints. I do remember there being an issue where lots of unnecessary clock buffers were being added, but just wanted to check that is what this change was about.
  2. I saw that the mflowgen connection from signoff to gdsmerge was removed. I remember hearing about problems with gdsmerge, so I wanted to check if this was intentionally removed because of those problems, or whether is was removed as part of the debugging process and hadn't been put back yet.

  1. I am not sure it has to do with unnecessary clock buffers. I vaguely recall there was some issues with the interfacing between the acore block and the rest of the digital logic. I think it is safe to reinsert these lines as I will be doing synthesis soon and I am not sure that these lines were the source of the acore error.

  2. We removed this as I did the gdsmerge manually. I believe there was a particular setting that I had to use. Let me double check that the merge settings that we used have been reflected back into the design folder. Otherwise, the gdsmerge and signoff should be reconnected.

zamyers commented 3 years ago

OK, thanks for clearing that up. There were two other questions had on the code -- I'll copy them here in case they got hidden:

  1. There was a block of create_clock and set_max_transition commands that were removed in the synthesis constraints. I do remember there being an issue where lots of unnecessary clock buffers were being added, but just wanted to check that is what this change was about.
  2. I saw that the mflowgen connection from signoff to gdsmerge was removed. I remember hearing about problems with gdsmerge, so I wanted to check if this was intentionally removed because of those problems, or whether is was removed as part of the debugging process and hadn't been put back yet.
1. I am not sure it has to do with unnecessary clock buffers. I vaguely recall there was some issues with the interfacing between the acore block and the rest of the digital logic. I think it is safe to reinsert these lines as I will be doing synthesis soon and I am not sure that these lines were the source of the acore error.

2. We removed this as I did the gdsmerge manually. I believe there was a particular setting that I had to use. Let me double check that the merge settings that we used have been reflected back into the design folder. Otherwise, the gdsmerge and signoff should be reconnected.

Okay so for #2, its okay to reconnect that. What I recalled happening involves a LVS/DRc discrepancy between Sung-Jin's and the flow's LVS/DRC commands. Due to the time constraints, we did LVS and DRC manually. Anyways, the GDS and signoff cut occurred when I was trying to debug something in the flow related to the LVS and DRC. I ended up having to do GDS manually as a result of this debugging failing.

Anyways, when Sung-Jin and I get to DRC/LVS this time around, I will reflect his lvs runset file over the current one in design (since I don't currently have it). I'll open an issue to make a note of that.