search

efabless / caravel

Caravel is a standard SoC harness with on chip resources to control and read/write operations from a user-dedicated space.
https://caravel-harness.readthedocs.io/
Apache License 2.0
295 stars 68 forks source link

user project floating signals handling #412

Closed mo-hosni closed 1 year ago

mo-hosni commented 1 year ago

I am currently re-implementing caravel using a hierarchy of (padframe + core_chip). The issue is that I am unable to re-implement the mgmt_protect (as a macro of flattened) without buffering the user signals and not having 1.5mm long wires.

So, can we handle this in any other way? or constrain the users to tie the floating outputs?

This was raised before in issue https://github.com/efabless/caravel/issues/352. I did fix this back then with a workaround that is no longer applicable using the latest OpenRoad.

RTimothyEdwards commented 1 year ago

You should not attempt to do this before having a full discussion between Mohamed Shalan, Andy, and me about the definition of the open frame design.

The quick answer to your question is that there should not be a management protect block because with only a padframe and a core, there is no boundary between process and user space to protect because there is no processor and no user space.

RTimothyEdwards commented 1 year ago

Or is this just refactoring the caravel chip design for sky130 in the same way that was done for gf180mcu? This wasn't an issue on gf180mcu?

RTimothyEdwards commented 1 year ago

I still don't get why this is a problem. Each of the user outputs goes directly to one input of a NAND gate. The NAND gate can be placed right at the boundary of the user project so that the wire length is essentially zero. Anything downstream of the NAND gate can be buffered as many times as it needs to be.

RTimothyEdwards commented 1 year ago

The solution found in #352 seems to be the correct one---it makes sure that the NAND gates are placed close to the user project area. Why is that solution "no longer applicable using the latest OpenROAD"?

mo-hosni commented 1 year ago

Or is this just refactoring the caravel chip design for sky130 in the same way that was done for gf180mcu? This wasn't an issue on gf180mcu?

Yes, that's exactly what I am doing. This was an issue in gf180mcu that we deferred. I remember our discussion on the power consumption of the buf_4 cells that were added for those nets. We decided to defer this, and I am now working on fixing it in caravel sky130 as I am currently implementing it.

I still don't get why this is a problem. Each of the user outputs goes directly to one input of a NAND gate. The NAND gate can be placed right at the boundary of the user project so that the wire length is essentially zero. Anything downstream of the NAND gate can be buffered as many times as it needs to be.

That's the ideal implementation; but I cannot put such constraints on the PnR tool on where to place some cells.

The solution found in #352 seems to be the correct one---it makes sure that the NAND gates are placed close to the user project area. Why is that solution "no longer applicable using the latest OpenROAD"?

What I did before in #352 is that I changed the placement to prioritize routability over timing and calibrated the placement density to have the cells placed very close to the pins. This was random and happened after some trials.

The issue is: OpenRoad now is not as heuristic as before. Using the current OpenRoad, I always get 1.5mm wires using whatever configuration when I set the user's output signals as 'DONT TOUCH'. So, I can't really replicate what I did before.

I could use an old OpenRoad version and get this done, but the implementation wouldn't really be reproducible. So, I was looking for other rigorous solutions if there are any.

RTimothyEdwards commented 1 year ago

Have you raised this as an issue on the OpenROAD github tracker?

mo-hosni commented 1 year ago

There is a variable in OpenRoad's global placement function that can be modified to change the initial global placement and reduce the wirelengths on these "don't touch" nets.

I opened a PR on OpenLane to add this variable (PL_WIRELENGTH_COEF) here: https://github.com/The-OpenROAD-Project/OpenLane/pull/1672

Setting PL_WIRELENGTH_COEF to 0.01 in the mgmt_protect implementation fixes this issue.