Xilinx / RapidWright

Build Customized FPGA Implementations for Vivado
http://www.rapidwright.io
Other
292 stars 108 forks source link

Missing VCC/GND ties on some Cell BEL pin maps #143

Closed litghost closed 3 years ago

litghost commented 3 years ago

I'm currently debugging the Cell BEL pin maps for the RAMB18E1 on 7-series. The current mapping appears to be missing some VCC/GND ties on pins in some configurations.

For example, the pin map for WRITE_WIDTH_B = 9 is currently:

      - parametersSiteTypes:
          - bel: RAMB18E1
            siteType: RAMB18E1
            parameter:
              key: WRITE_WIDTH_B
              textValue: 1
          - bel: RAMB18E1
            siteType: RAMB18E1
            parameter:
              key: WRITE_WIDTH_B
              textValue: 2
          - bel: RAMB18E1
            siteType: RAMB18E1
            parameter:
              key: WRITE_WIDTH_B
              textValue: 4
          - bel: RAMB18E1
            siteType: RAMB18E1
            parameter:
              key: WRITE_WIDTH_B
              textValue: 9
        pins:
          - cellPin: 'WEBWE[0]'
            belPin: WEBWE0
          - cellPin: 'WEBWE[0]'
            belPin: WEBWE1
          - cellPin: 'WEBWE[0]'
            belPin: WEBWE2
          - cellPin: 'WEBWE[0]'
            belPin: WEBWE3

The WEBWE[0] 1 to many relationship is correct. However, I believe that there should be additional lines to tied WEBWE4 to WEBWE7 to GND. The current DeviceResourceWriter does not do this. So I believe the correct pins block would be:

        pins:
          - cellPin: 'WEBWE[0]'
            belPin: WEBWE0
          - cellPin: 'WEBWE[0]'
            belPin: WEBWE1
          - cellPin: 'WEBWE[0]'
            belPin: WEBWE2
          - cellPin: 'WEBWE[0]'
            belPin: WEBWE3
          - cellPin: GND
            belPin: WEBWE4
          - cellPin: GND
            belPin: WEBWE5
          - cellPin: GND
            belPin: WEBWE6
          - cellPin: GND
            belPin: WEBWE7

There also appears to be an issue where REGCLKARDRCLK and REGCLKB should be tied to VCC, but I haven't isolated where in the map that should be happening.

I'm going to continue to investigate to determine if I can find out where the issue is.

litghost commented 3 years ago

I wrote a simple tool to see if try to see if the problem was in DeviceResourceWriter or in RapidWright itself. Invoking the new tool for this exact case shows that the problem appears to be in RapidWright, rather than a latent bug in DeviceResourceWriter. In the output below, you can see that WEBWE4 to WEBWE7 are missing from the Cell.getPinMappingsP2L after the placement of the RAMB18E1. If there is a bug in PinMapTester, let me know.

# scripts/invoke_rapidwright.sh com.xilinx.rapidwright.tests.PinMapTester xc7a35tcpg236-1 RAMB18E1 RAMB18_X0Y19 RAMB18E1 RAMB18E1 WRITE_WIDTH_A=9 WRITE_WIDTH_B=9
Cell type RAMB18E1 at RAMB18_X0Y19/RAMB18E1 in part xc7a35tcpg236-1, pin map:
 - CLKBWRCLK <= CLKBWRCLK          
 - ADDRARDADDR0 <= ADDRARDADDR[0]
 - ADDRARDADDR1 <= ADDRARDADDR[1]  
 - DOBDO1 <= DOBDO[1]
 - WEBWE1 <= WEBWE[0]              
 - ADDRARDADDR2 <= ADDRARDADDR[2]
 - DOBDO0 <= DOBDO[0]              
 - ENBWREN <= ENBWREN
 - WEBWE0 <= WEBWE[0]  
 - DOBDO3 <= DOBDO[3]
 - WEBWE3 <= WEBWE[0]  
 - DOBDO2 <= DOBDO[2]    
 - WEBWE2 <= WEBWE[0]
 - DOBDO5 <= DOBDO[5]
 - DOBDO4 <= DOBDO[4]
 - DOBDO7 <= DOBDO[7]
 - DOBDO6 <= DOBDO[6]    
 - DOBDO9 <= DOBDO[9]    
 - DOBDO8 <= DOBDO[8]              
 - REGCEAREGCE <= REGCEAREGCE      
 - RSTREGB <= RSTREGB              
 - DIBDI12 <= DIBDI[12]            
 - DIBDI11 <= DIBDI[11]          
 - DOBDO10 <= DOBDO[10]          
 - DIBDI10 <= DIBDI[10]          
 - DOBDO11 <= DOBDO[11]
 - DOBDO12 <= DOBDO[12]          
 - DOBDO13 <= DOBDO[13]
 - DIBDI15 <= DIBDI[15]          
 - DOBDO14 <= DOBDO[14]          
 - DIBDI14 <= DIBDI[14]          
 - DOBDO15 <= DOBDO[15]          
 - DIBDI13 <= DIBDI[13]          
 - DIADI0 <= DIADI[0]            
 - RSTREGARSTREG <= RSTREGARSTREG
 - WEA3 <= WEA[0]                
 - DIADI9 <= DIADI[9]
 - DIADI5 <= DIADI[5]            
 - DIADI6 <= DIADI[6]    
 - DIADI7 <= DIADI[7]            
 - DIADI8 <= DIADI[8]            
 - DIADI1 <= DIADI[1]            
 - DIADI2 <= DIADI[2]    
 - DIADI3 <= DIADI[3]            
 - DIADI4 <= DIADI[4]
 - DIPBDIP0 <= DIPBDIP[0]
 - DIPBDIP1 <= DIPBDIP[1]
 - ADDRBTIEHIGH1 <= VCC
 - ADDRBTIEHIGH0 <= VCC
 - REGCLKB <= CLKBWRCLK
 - DOPADOP0 <= DOPADOP[0]
 - WEA0 <= WEA[0]    
 - RSTRAMARSTRAM <= RSTRAMARSTRAM
 - WEA1 <= WEA[0]
 - WEA2 <= WEA[0]
 - DOPADOP1 <= DOPADOP[1]
 - RSTRAMB <= RSTRAMB
 - DIADI15 <= DIADI[15]
 - DOADO12 <= DOADO[12]
 - DIADI14 <= DIADI[14]
 - DOADO13 <= DOADO[13]
 - DOADO10 <= DOADO[10]
 - DOADO11 <= DOADO[11]
 - DIADI11 <= DIADI[11]
 - DIADI10 <= DIADI[10]
 - DIADI13 <= DIADI[13]
 - DOADO0 <= DOADO[0]
 - DOADO14 <= DOADO[14]
 - DIADI12 <= DIADI[12]
 - DOADO15 <= DOADO[15]
 - ADDRBWRADDR11 <= ADDRBWRADDR[11]
 - DOADO2 <= DOADO[2]
 - ADDRBWRADDR10 <= ADDRBWRADDR[10]
 - DOADO1 <= DOADO[1]
 - ADDRBWRADDR13 <= ADDRBWRADDR[13]
 - DOADO4 <= DOADO[4]
 - ADDRBWRADDR12 <= ADDRBWRADDR[12]
 - DOADO3 <= DOADO[3]
 - ADDRATIEHIGH1 <= VCC
 - DOADO6 <= DOADO[6]
 - ADDRATIEHIGH0 <= VCC
 - CLKARDCLK <= CLKARDCLK
 - DOADO5 <= DOADO[5]
 - DOADO8 <= DOADO[8]
 - DOADO7 <= DOADO[7]
 - DOADO9 <= DOADO[9]
 - DOPBDOP0 <= DOPBDOP[0]
 - DOPBDOP1 <= DOPBDOP[1]
 - ADDRARDADDR11 <= ADDRARDADDR[11]
 - ADDRARDADDR10 <= ADDRARDADDR[10]
 - ADDRARDADDR13 <= ADDRARDADDR[13]
 - ADDRARDADDR12 <= ADDRARDADDR[12]
 - ADDRBWRADDR2 <= ADDRBWRADDR[2]
 - ADDRBWRADDR1 <= ADDRBWRADDR[1]
 - ADDRBWRADDR4 <= ADDRBWRADDR[4]
 - DIBDI1 <= DIBDI[1]
 - ADDRBWRADDR3 <= ADDRBWRADDR[3]
 - DIBDI0 <= DIBDI[0]
 - ADDRBWRADDR0 <= ADDRBWRADDR[0]
 - ADDRBWRADDR9 <= ADDRBWRADDR[9]
 - ADDRBWRADDR6 <= ADDRBWRADDR[6]
 - ADDRBWRADDR5 <= ADDRBWRADDR[5]
 - ADDRBWRADDR8 <= ADDRBWRADDR[8]
 - ADDRBWRADDR7 <= ADDRBWRADDR[7]
 - ADDRARDADDR3 <= ADDRARDADDR[3]
 - ADDRARDADDR4 <= ADDRARDADDR[4]
 - ENARDEN <= ENARDEN
 - ADDRARDADDR5 <= ADDRARDADDR[5]
 - DIPADIP1 <= DIPADIP[1]
 - ADDRARDADDR6 <= ADDRARDADDR[6]
 - ADDRARDADDR7 <= ADDRARDADDR[7]
 - ADDRARDADDR8 <= ADDRARDADDR[8]
 - DIPADIP0 <= DIPADIP[0]
 - ADDRARDADDR9 <= ADDRARDADDR[9]
 - REGCEB <= REGCEB
 - DIBDI7 <= DIBDI[7]
 - DIBDI6 <= DIBDI[6]
 - DIBDI9 <= DIBDI[9]
 - DIBDI8 <= DIBDI[8]
 - DIBDI3 <= DIBDI[3]
 - DIBDI2 <= DIBDI[2]
 - DIBDI5 <= DIBDI[5]
 - DIBDI4 <= DIBDI[4]
clavin-xlnx commented 3 years ago

I took a look to see what Vivado does when placing a RAMB18E1 and it does not add pin mappings to the pins of WEBWE4-WEBWE7 to GND. Those mappings do not exist in the source consumed by RapidWright for pin mappings from Vivado. It appears that there is another process that is routing those pins post placement to GND. It seems like it would make sense to add those pin mappings to GND in this instance, but I'm not sure how we could make this data-driven.

litghost commented 3 years ago

I took a look to see what Vivado does when placing a RAMB18E1 and it does not add pin mappings to the pins of WEBWE4-WEBWE7 to GND. Those mappings do not exist in the source consumed by RapidWright for pin mappings from Vivado. It appears that there is another process that is routing those pins post placement to GND. It seems like it would make sense to add those pin mappings to GND in this instance, but I'm not sure how we could make this data-driven.

So I'm confused, but Vivado 2019.2 definitely maps WEBWE4-WEBWE7 to GND (along with REGCLKARDRCLK and REGCLKB to VCC). The interesting thing here is that the TIEHIGH pins are correctly mapped to VCC, so whatever process is going on works for those BEL pins?

Screenshot from 2021-03-24 12-38-39

clavin-xlnx commented 3 years ago

Yes, clearly the pins do end up routed to GND and that is probably a DRC requirement. However, from what I can tell, the directive to route those pins to GND does not come from pin mappings. It seems to come from another process during route_design. The pin mapping template does not direct those pins to GND.

The REGCLKARDCLK and REGCLKB must get statically tied for a different reason than the WEBWE* pins. I might have to investigate further to understand the issue better.

litghost commented 3 years ago

Yes, clearly the pins do end up routed to GND and that is probably a DRC requirement. However, from what I can tell, the directive to route those pins to GND does not come from pin mappings. It seems to come from another process during route_design. The pin mapping template does not direct those pins to GND.

The REGCLKARDCLK and REGCLKB must get statically tied for a different reason than the WEBWE* pins. I might have to investigate further to understand the issue better.

The tie to VCC/GND appears to have immediately after site placement, so presumably it is before route_design.

clavin-xlnx commented 3 years ago

Ok, after examining it a bit more, I have a hypothesis. For BEL pins that do not have mappings (WEBWE4 does not have a WEBWE[4] on the logical cell, for example), it would seem that there is an implicit default mapping to GND. Perhaps this was intended to save on space/verbosity, etc. Would it be worthwhile to test this theory?

If it holds for other primitive cell mappings, we could explicitly encode the rule by adding GND pin mappings to all of the missing pins. Otherwise, we would have to modify the Interchange schema to somehow communicate this.

litghost commented 3 years ago

Ok, after examining it a bit more, I have a hypothesis. For BEL pins that do not have mappings (WEBWE4 does not have a WEBWE[4] on the logical cell, for example), it would seem that there is an implicit default mapping to GND. Perhaps this was intended to save on space/verbosity, etc. Would it be worthwhile to test this theory?

If it holds for other primitive cell mappings, we could explicitly encode the rule by adding GND pin mappings to all of the missing pins. Otherwise, we would have to modify the Interchange schema to somehow communicate this.

Okay, so the theory is:

clavin-xlnx commented 3 years ago

Correct, that is my current thinking. I'm searching for counter examples, but haven't found one yet.

litghost commented 3 years ago

If it holds for other primitive cell mappings, we could explicitly encode the rule by adding GND pin mappings to all of the missing pins. Otherwise, we would have to modify the Interchange schema to somehow communicate this.

I definitely believe in explicit over implicit here. One question for you is what should RapidWright's behavior be? I would argue that if there is an implicit tie to GND, RapidWright should populate getPinMappingsP2L with the GND ties, even if the underlying data does explicitly specify it.

clavin-xlnx commented 3 years ago

I definitely believe in explicit over implicit here. One question for you is what should RapidWright's behavior be? I would argue that if there is an implicit tie to GND, RapidWright should populate getPinMappingsP2L with the GND ties, even if the underlying data does explicitly specify it.

I agree as I think you meant "doesn't" explicitly specify it. RapidWright can fill in the gaps where the implicit GND ties exist with an explicit pin mapping to GND. To be clear, however, the GND and VCC pin mappings aren't treated the same as other pin mappings. They don't show up in the pin mapping list nor are the BEL pins highlighted in the GUI when viewed in Vivado.

litghost commented 3 years ago

I agree as I think you meant "doesn't" explicitly specify it.

Correct.

RapidWright can fill in the gaps where the implicit GND ties exist with an explicit pin mapping to GND. To be clear, however, the GND and VCC pin mappings aren't treated the same as other pin mappings. They don't show up in the pin mapping list nor are the BEL pins highlighted in the GUI when viewed in Vivado.

The key idea from my perspective is if you route a design based on the pin mappings currently returned from getPinMappingsP2L, you get a partially routed design. That means that getPinMappingsP2L is not valid without either changing the documentation or changing its behavior. Implicitly routing all unmapped physical pins to GND is a non-obvious design decision, especially given VCC pin mappings are explicitly returned.

clavin-xlnx commented 3 years ago

I just realized that the obvious place where we don't tie unused BEL input pins to GND is on LUTs and CARRYs. If you place a LUT2 onto a LUT5/6 BEL, Vivado doesn't route the other inputs to GND. Do we simply exclude SLICE-based BELs from this GND input convention?

litghost commented 3 years ago

I just realized that the obvious place where we don't tie unused BEL input pins to GND is on LUTs and CARRYs. If you place a LUT2 onto a LUT5/6 BEL, Vivado doesn't route the other inputs to GND. Do we simply exclude SLICE-based BELs from this GND input convention?

Very true. This is a strong reason to not leave this up to implementer of DeviceResource's. Not sure what the solution is yet.

litghost commented 3 years ago

We could consider adding a per cell type "default" connection. LUT's would be VCC, RAMB18E1 would be GND. I'm not sure how to generalize this though. Could you dig on your side to see if there is a cleaner way to expose this in RapidWright?

In particular, if getPinMappingsP2L does the right thing, then I can work to adjust the schema to reflect the latest mapping.

clavin-xlnx commented 3 years ago

I've created a new RapidWright jar that tries to do the right thing when creating cells and populating the map for getPinMappingsP2L(). You can access it here: https://github.com/Xilinx/RapidWright/releases/download/v2020.2.2-beta/rapidwright-api-lib-2020.2.2_update1.jar

It excludes any BELs in SLICELs and SLICEMs from the GND tie assumptions explained above. Perhaps RapidWright will just have to adjust/handle exceptional cases to find a way to communicate something consist.

litghost commented 3 years ago

I've created a new RapidWright jar that tries to do the right thing when creating cells and populating the map for getPinMappingsP2L(). You can access it here: https://github.com/Xilinx/RapidWright/releases/download/v2020.2.2-beta/rapidwright-api-lib-2020.2.2_update1.jar

It excludes any BELs in SLICELs and SLICEMs from the GND tie assumptions explained above. Perhaps RapidWright will just have to adjust/handle exceptional cases to find a way to communicate something consist.

Okay great. I'll work on fixing up DeviceResourceWriter to do the right thing. I believe that #145 is not the right approach, we'll see. Might need to extend the schema, might not.

litghost commented 3 years ago
scripts/invoke_rapidwright.sh com.xilinx.rapidwright.tests.PinMapTester xc7a35tcpg236-1 RAMB18E1 RAMB18_X0Y19 RAMB18E1 RAMB18E1 WRITE_WIDTH_A=9 WRITE_WIDTH_B=9 DOB_REG=0 DOA_REG=0 | grep GND
 - WEBWE5 <= GND
 - WEBWE4 <= GND
 - WEBWE7 <= GND
 - WEBWE6 <= GND
 - REGCLKARDRCLK <= GND

Looks like REGCLKB tie to GND is still missing?

litghost commented 3 years ago

Current output for REGCLKB BEL pin is:

scripts/invoke_rapidwright.sh com.xilinx.rapidwright.tests.PinMapTester xc7a35tcpg236-1 RAMB18E1 RAMB18_X0Y19 RAMB18E1 RAMB18E1 WRITE_WIDTH_A=9 WRITE_WIDTH_B=9 DOB_REG=0 DOA_REG=0 | grep REGCLKB
 - REGCLKB <= CLKBWRCLK
clavin-xlnx commented 3 years ago

scripts/invoke_rapidwright.sh com.xilinx.rapidwright.tests.PinMapTester xc7a35tcpg236-1 RAMB18E1 RAMB18_X0Y19 RAMB18E1 RAMB18E1 WRITE_WIDTH_A=9 WRITE_WIDTH_B=9 DOB_REG=0 DOA_REG=0 | grep REGCLKB

  • REGCLKB <= CLKBWRCLK

I can confirm this behavior. I have diagnosed it to an issue with interpreting source data, so this will be fixable on the RapidWright side. I'll report back when I have a fix.

litghost commented 3 years ago

I don't think the new map is right:

# scripts/invoke_rapidwright.sh com.xilinx.rapidwright.tests.PinMapTester xc7a35tcpg236-1 OBUF IOB_X0Y111 IOB33 OUTBUF
Cell type OBUF at IOB_X0Y111/OUTBUF in part xc7a35tcpg236-1, pin map:
 - IN <= I
 - TRI <= GND
 - OUT <= O

I believe that the TRI <= GND map is not right. Similiar situation with IBUF path:

Cell type IBUF at IOB_X0Y111/INBUF_EN in part xc7a35tcpg236-1, pin map:
 - PAD <= I
 - IBUFDISABLE <= GND
 - INTERMDISABLE <= GND
 - DIFFI_IN <= GND
 - OUT <= O

IBUFDISABLE and INTERMDISABLE are correct, but I think the DIFFI_IN is incorrect. Exception to the rule could be "tie to GND if <sig>USED buffer doesn't exist.

In the case of TRI and DIFFI_IN, there is TUSED and DIFFI_USED respectively.

clavin-xlnx commented 3 years ago

Yes, based on my new understanding of how the source data should be re-interpreted, we'll have a much more accurate set of GND/VCC ties. I'm hoping this next fix should resolve all the issues and exceptions we've discovered. My original hypothesis has several issues and probably will be discarded.

litghost commented 3 years ago

Yes, based on my new understanding of how the source data should be re-interpreted, we'll have a much more accurate set of GND/VCC ties. I'm hoping this next fix should resolve all the issues and exceptions we've discovered. My original hypothesis has several issues and probably will be discarded.

Oh, great! Looking forward to it. I'll investigate my other open issues for now then.

clavin-xlnx commented 3 years ago

I have created patch for RapidWright that should resolve the inconsistent pin mappings. Here are the steps to update your local RapidWright install with the patch:

cd $RAPIDWRIGHT_PATH
wget https://github.com/Xilinx/RapidWright/releases/download/v2020.2.2-beta/rapidwright-2020.2.2-patch2.zip
unzip -o rapidwright-2020.2.2-patch2.zip

It turns out that we have some pin mappings that are controlled by setting a combination of params together to specific values. This patch also addresses that behavior, at least for getPinMappingsP2L(). This will also likely necessitate a change in the Interchange schema to accommodate multiple parameter settings that yield a particular pin mapping. I can file some new issues to help more fully describe the behavior.

As long as there are no issues with the patch, I will create a new 2020.2.3 release soon. We'll continue exploring options for packaging RapidWright to make deployment easier.

litghost commented 3 years ago

I think you are clear to make the 2020.2.3 release when you are ready. After the release, this issue can be closed.