Skip to content
This repository has been archived by the owner on Apr 25, 2024. It is now read-only.

Added corrected GDS for five standard cells with known DRC errors #8

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

RTimothyEdwards
Copy link

Added corrected GDS for five standard cells (a2111o_1, a2111oi_0, buf_16, clkdlybuf4s15_1, and clkdlybuf4s18_1) which are known to have incorrect layout that violates DRC rules (namely the poly surround distance for poly contacts). All violations are by a minimal 5nm distance, and all corrections are a trivial extension of the poly layer to meet the DRC requirement. All corrections lie within the existing layer boundaries and so pose no risk for interactions with other standard cells. These corrections have been in open_pdks as a patch to the skywater-pdk repository, and have been used in tapeouts.

…_16,

clkdlybuf4s15_1, and clkdlybuf4s18_1) which are known to have incorrect
layout that violates DRC rules (namely the poly surround distance for
poly contacts).  All violations are by a minimal 5nm distance, and all
corrections are a trivial extension of the poly layer to meet the DRC
requirement.  All corrections lie within the existing layer boundaries
and so pose no risk for interactions with other standard cells.  These
corrections have been in open_pdks as a patch to the skywater-pdk
repository, and have been used in tapeouts.
@RTimothyEdwards
Copy link
Author

RTimothyEdwards commented Sep 17, 2022

This pull request fixes issue #261 (in the skywater-pdk issue tracker)

@proppy
Copy link
Member

proppy commented Sep 19, 2022

Should we do a XOR imaging of the GDS to verify what are the affected blocks?

@RTimothyEdwards
Copy link
Author

@proppy : Or you can just trust that I know what I'm doing, because if we have to spend several days of back-and-forth messages over the verification of the contents of every pull request, then it will be several years before we finish correcting all the known errors in the PDK.

@proppy
Copy link
Member

proppy commented Sep 19, 2022

Sorry if I phrased this poorly, I do trust your expertise, I was more after us documenting the changes happening to the layout themselves so that we're able to trace back (and potentially "replay" those) in case of an upstream update.

I'll let @QuantamHD handle this.

@RTimothyEdwards
Copy link
Author

@proppy : In that case, yes, an XOR would be the appropriate (and probably only) method of confirmation. The only other method I can think of would be to use a tool that converts GDS to ASCII format and run a diff on the ASCII output.

@proppy
Copy link
Member

proppy commented Sep 19, 2022

could a serie of magic commands to go from the upstream to the fixed version also be a way to document the change (and potentially make it reproducible?)

@QuantamHD
Copy link

I think we should merge this in, but an ASCII format for GDS sounds easy enough to build, and rather useful

@RTimothyEdwards
Copy link
Author

@proppy : I did not use magic to make the modification, so there is no set of magic commands that would fix the error.

@proppy
Copy link
Member

proppy commented Sep 19, 2022

Generated this visual (and textual diff):
proppy@9779594#diff-9ad50b50b69a73381ebf2334b17372dd908acf0712facfb9d20d3b2c7620d762

by converting the gds to SVG before and after the change:

git diff HEAD^ --name-only | xargs -n1 -i@ env/bin/python -c "import sys; import gdstk; path = '@'; print(path); gdstk.read_gds(path).top_level()[0].write_svg(f'{path}.svg')"

(GitHub has some doc on how to visualize the diff here: https://github.blog/2014-10-06-svg-viewing-diffing/)

For this shows that for sky130_fd_sc_hd__a2111o_1:

  • 1x polygon moved by (0.10, -0.18)
  • 1x text label (VNB) moved by (0, 0.02)

Hope that can help in case we want to document the precise change made to the geometries.

@RTimothyEdwards
Copy link
Author

@proppy : I think all layout should be in a human-readable format (CIF is good for that, too). But the diff-by-SVG seems to work fine. How did you get the text report of the difference?

@proppy
Copy link
Member

proppy commented Sep 19, 2022

From here:
proppy@9779594#diff-9ad50b50b69a73381ebf2334b17372dd908acf0712facfb9d20d3b2c7620d762
You can click here:
image
to see the raw text diff:
image

@RTimothyEdwards
Copy link
Author

@proppy @QuantamHD : Four days and counting and still this has not been merged.

@QuantamHD
Copy link

@RTimothyEdwards Still waiting on write access, but Proppy's visual diff also update the gds layout views?

@proppy
Copy link
Member

proppy commented Sep 20, 2022

CIF is good for that, too

Gave it a try here: proppy@754a984

With the following command:

git diff HEAD^ --name-only | xargs -n1 -i@ echo 'gds read @; cif write @' | PDK_ROOT=conda-env/share/pdk PDKPATH=conda-env/share/pdk/sky130A conda-env/bin/magic -dnull -noconsole -rcfile conda-env/share/pdk/sky130A/libs.tech/magic/sky130A.magicrc 

This give a good overview of the changes:

  • sky130_fd_sc_hd__a2111o_1: change to VNB label position
  • sky130_fd_sc_hd__a2111oi_0:
    • POLY layer
      • change to 4x boxes width and xpos
      • addition of 1x box
      • change to 1x box height and ypos
  • sky130_fd_sc_hd__buf_16:
    • POLY layer
      • change to 1x box width and xpos
  • sky130_fd_sc_hd__clkdlybuf4s15_1
    • POLY layer
      • change to 4x boxes width and xpos
    • LCONT layer
      • change to 4x boxes xpos
    • NPC layer
      • change to 2x boxes width and xpos
  • sky130_fd_sc_hd__clkdlybuf4s18_1
    • POLY layer
    • change to 1x box width and xpos

Given that not all the files are affected in the same way (ex: no apparent POLY change for sky130_fd_sc_hd__a2111o_1 and LCONT changes for sky130_fd_sc_hd__clkdlybuf4s15_1), it might be good to add a small note to the commit/PR description with some additional details for documentating each changes so that we can more easily replicate them in case the original GDS files gets updated?

I think all layout should be in a human-readable format

Does that means that we should store the file in cif instead, and build the gds as part of the PDK build process? That would follow @mithro recent recommendation on google/globalfoundries-pdk-libs-gf180mcu_fd_pr#57 (comment) to:

prioritize humans and maintenance not software

@mithro
Copy link
Contributor

mithro commented Sep 20, 2022

FYI - This change might conflict with some other work that Efabless wants us to do....

@proppy
Copy link
Member

proppy commented Oct 5, 2022

FYI - This change might conflict with some other work that Efabless wants us to do....

@mithro are those conflicting changes documented somewhere?

no apparent POLY change for sky130_fd_sc_hd__a2111o_1 and LCONT changes for sky130_fd_sc_hd__clkdlybuf4s15_1

@RTimothyEdwards is that expected?

@RTimothyEdwards
Copy link
Author

@proppy : Yes, in some cells it was necessary to extend the poly, but in some cells there was sufficient space just to move the contact, which was a simpler fix.

@proppy
Copy link
Member

proppy commented Oct 17, 2022

@RTimothyEdwards I tried to verify the DRC errors with the following script, I still some things are those unrelated to the fixes you're proposing here?

gds read $::env(GDS_FILE)
set cell  [lindex [split [lindex [split cells/a2111o/sky130_fd_sc_hd__a2111o_1.gds /] 2] .] 0]
load $cell
select top cell
what
drc euclidean on
drc style drc(full)
drc check
puts [drc listall why]

sky130_fd_sc_hd__a2111o_1

🍊 GDS_FILE=cells/a2111o/sky130_fd_sc_hd__a2111o_1.gds magic -dnull -noconsole -rcfile $PDK_ROOT/sky130A/libs.tech/magic/sky130A.magicrc < pr8.tcl
Magic 8.3 revision 329 - Compiled on Sat Oct  8 23:33:28 UTC 2022.
Starting magic under Tcl interpreter
Using the terminal as the console.
Using NULL graphics device.
Processing system .magicrc file
Sourcing design .magicrc for technology sky130A ...
2 Magic internal units = 1 Lambda
Input style sky130(vendor): scaleFactor=2, multiplier=2
The following types are not handled by extraction and will be treated as non-electrical types:
    ubm 
Scaled tech values by 2 / 1 to match internal grid scaling
Loading sky130A Device Generator Menu ...
Using technology "sky130A", version 1.0.342-0-gde752ec
Warning: Calma reading is not undoable!  I hope that's OK.
Library written using GDS-II Release 3.0
Library name: sky130_fd_sc_hd__a2111o_1
Reading "sky130_fd_sc_hd__a2111o_1".
Selected subcell(s):
    Instance "Topmost cell in the window" of cell "sky130_fd_sc_hd__a2111o_1"
DRC style is now "drc(full)"
Loading DRC CIF style.
{All nwells must contain metal-connected N+ taps (nwell.4)} {{-38 261 0 582} {0 261 866 582}} {P-diff distance to N-tap must be < 15.0um (LU.3)} {{27 297 85 497} {115 297 168 497} {227 297 304 497} {334 297 384 497} {414 297 486 497} {516 297 628 497} {658 297 712 497} {742 297 800 497}} {N-diff distance to P-tap must be < 15.0um (LU.2)} {{27 47 93 177} {123 47 292 177} {322 47 384 177} {414 47 486 177} {516 47 630 177} {660 47 711 177} {741 47 799 177}}

sky130_fd_sc_hd__a2111oi_0

🍙 GDS_FILE=cells/a2111oi/sky130_fd_sc_hd__a2111oi_0.gds magic -dnull -noconsole -rcfile $PDK_ROOT/sky130A/libs.tech/magic/sky130A.magicrc < pr8.tcl
Magic 8.3 revision 329 - Compiled on Sat Oct  8 23:33:28 UTC 2022.
Starting magic under Tcl interpreter
Using the terminal as the console.
Using NULL graphics device.
Processing system .magicrc file
Sourcing design .magicrc for technology sky130A ...
2 Magic internal units = 1 Lambda
Input style sky130(vendor): scaleFactor=2, multiplier=2
The following types are not handled by extraction and will be treated as non-electrical types:
    ubm 
Scaled tech values by 2 / 1 to match internal grid scaling
Loading sky130A Device Generator Menu ...
Using technology "sky130A", version 1.0.342-0-gde752ec
Warning: Calma reading is not undoable!  I hope that's OK.
Library written using GDS-II Release 3.0
Library name: sky130_fd_sc_hd__a2111oi_0
Reading "sky130_fd_sc_hd__a2111oi_0".
Selected subcell(s):
    Instance "Topmost cell in the window" of cell "sky130_fd_sc_hd__a2111o_1"
DRC style is now "drc(full)"
Loading DRC CIF style.
{All nwells must contain metal-connected N+ taps (nwell.4)} {{-38 261 0 582} {0 261 866 582}} {P-diff distance to N-tap must be < 15.0um (LU.3)} {{27 297 85 497} {115 297 168 497} {227 297 304 497} {334 297 384 497} {414 297 486 497} {516 297 628 497} {658 297 712 497} {742 297 800 497}} {N-diff distance to P-tap must be < 15.0um (LU.2)} {{27 47 93 177} {123 47 292 177} {322 47 384 177} {414 47 486 177} {516 47 630 177} {660 47 711 177} {741 47 799 177}}

sky130_fd_sc_hd__buf_16

🍙 GDS_FILE=cells/buf/sky130_fd_sc_hd__buf_16.gds magic -dnull -noconsole -rcfile $PDK_ROOT/sky130A/libs.tech/magic/sky130A.magicrc < pr8.tcl
Magic 8.3 revision 329 - Compiled on Sat Oct  8 23:33:28 UTC 2022.
Starting magic under Tcl interpreter
Using the terminal as the console.
Using NULL graphics device.
Processing system .magicrc file
Sourcing design .magicrc for technology sky130A ...
2 Magic internal units = 1 Lambda
Input style sky130(vendor): scaleFactor=2, multiplier=2
The following types are not handled by extraction and will be treated as non-electrical types:
    ubm 
Scaled tech values by 2 / 1 to match internal grid scaling
Loading sky130A Device Generator Menu ...
Using technology "sky130A", version 1.0.342-0-gde752ec
Warning: Calma reading is not undoable!  I hope that's OK.
Library written using GDS-II Release 3.0
Library name: sky130_fd_sc_hd__buf_16
Reading "sky130_fd_sc_hd__buf_16".
Selected subcell(s):
    Instance "Topmost cell in the window" of cell "sky130_fd_sc_hd__a2111o_1"
DRC style is now "drc(full)"
Loading DRC CIF style.
{All nwells must contain metal-connected N+ taps (nwell.4)} {{-38 261 0 582} {0 261 866 582}} {P-diff distance to N-tap must be < 15.0um (LU.3)} {{27 297 85 497} {115 297 168 497} {227 297 304 497} {334 297 384 497} {414 297 486 497} {516 297 628 497} {658 297 712 497} {742 297 800 497}} {N-diff distance to P-tap must be < 15.0um (LU.2)} {{27 47 93 177} {123 47 292 177} {322 47 384 177} {414 47 486 177} {516 47 630 177} {660 47 711 177} {741 47 799 177}}

sky130_fd_sc_hd__clkdlybuf4s15_1

🍊 GDS_FILE=cells/clkdlybuf4s15/sky130_fd_sc_hd__clkdlybuf4s15_1.gds magic -dnull -noconsole -rcfile $PDK_ROOT/sky130A/libs.tech/magic/sky130A.magicrc < pr8.tcl
Magic 8.3 revision 329 - Compiled on Sat Oct  8 23:33:28 UTC 2022.
Starting magic under Tcl interpreter
Using the terminal as the console.
Using NULL graphics device.
Processing system .magicrc file
Sourcing design .magicrc for technology sky130A ...
2 Magic internal units = 1 Lambda
Input style sky130(vendor): scaleFactor=2, multiplier=2
The following types are not handled by extraction and will be treated as non-electrical types:
    ubm 
Scaled tech values by 2 / 1 to match internal grid scaling
Loading sky130A Device Generator Menu ...
Using technology "sky130A", version 1.0.342-0-gde752ec
Warning: Calma reading is not undoable!  I hope that's OK.
Library written using GDS-II Release 3.0
Library name: sky130_fd_sc_hd__clkdlybuf4s15_1
Reading "sky130_fd_sc_hd__clkdlybuf4s15_1".
Selected subcell(s):
    Instance "Topmost cell in the window" of cell "sky130_fd_sc_hd__a2111o_1"
DRC style is now "drc(full)"
Loading DRC CIF style.
{All nwells must contain metal-connected N+ taps (nwell.4)} {{-38 261 0 582} {0 261 866 582}} {P-diff distance to N-tap must be < 15.0um (LU.3)} {{27 297 85 497} {115 297 168 497} {227 297 304 497} {334 297 384 497} {414 297 486 497} {516 297 628 497} {658 297 712 497} {742 297 800 497}} {N-diff distance to P-tap must be < 15.0um (LU.2)} {{27 47 93 177} {123 47 292 177} {322 47 384 177} {414 47 486 177} {516 47 630 177} {660 47 711 177} {741 47 799 177}}

sky130_fd_sc_hd__clkdlybuf4s18_1

🍙 GDS_FILE=cells/clkdlybuf4s18/sky130_fd_sc_hd__clkdlybuf4s18_1.gds magic -dnull -noconsole -rcfile $PDK_ROOT/sky130A/libs.tech/magic/sky130A.magicrc < pr8.tcl
Magic 8.3 revision 329 - Compiled on Sat Oct  8 23:33:28 UTC 2022.
Starting magic under Tcl interpreter
Using the terminal as the console.
Using NULL graphics device.
Processing system .magicrc file
Sourcing design .magicrc for technology sky130A ...
2 Magic internal units = 1 Lambda
Input style sky130(vendor): scaleFactor=2, multiplier=2
The following types are not handled by extraction and will be treated as non-electrical types:
    ubm 
Scaled tech values by 2 / 1 to match internal grid scaling
Loading sky130A Device Generator Menu ...
Using technology "sky130A", version 1.0.342-0-gde752ec
Warning: Calma reading is not undoable!  I hope that's OK.
Library written using GDS-II Release 3.0
Library name: sky130_fd_sc_hd__clkdlybuf4s18_1
Reading "sky130_fd_sc_hd__clkdlybuf4s18_1".
Selected subcell(s):
    Instance "Topmost cell in the window" of cell "sky130_fd_sc_hd__a2111o_1"
DRC style is now "drc(full)"
Loading DRC CIF style.
{All nwells must contain metal-connected N+ taps (nwell.4)} {{-38 261 0 582} {0 261 866 582}} {P-diff distance to N-tap must be < 15.0um (LU.3)} {{27 297 85 497} {115 297 168 497} {227 297 304 497} {334 297 384 497} {414 297 486 497} {516 297 628 497} {658 297 712 497} {742 297 800 497}} {N-diff distance to P-tap must be < 15.0um (LU.2)} {{27 47 93 177} {123 47 292 177} {322 47 384 177} {414 47 486 177} {516 47 630 177} {660 47 711 177} {741 47 799 177}}

@atorkmabrains
Copy link

@proppy To get the difference between 2 GDS files, the best way to do that is using XOR. Any alternative method might not be as reliable. We have added this script that supports GDS to text records and text records to GDS and we have tried going to text and back and did an xor everything seems to be working great:

google/globalfoundries-pdk-libs-gf180mcu_fd_pr@main...mabrains:globalfoundries-pdk-libs-gf180mcu_fd_pr:gds2txt

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants