Add Cache Coherency support on Ultrascale projects #1527
Conversation
Signed-off-by: Ionut Podgoreanu <[email protected]>
podgori
requested review from
AndrDragomir,
IuliaCMoldovan,
AndreiGrozav,
gastmaier,
FilipG24,
bluncan,
alin724,
acostina,
cristianmihaipopa,
LBFFilho,
PopPaul2021 and
sarpadi
as code owners
There was a problem hiding this comment.
I believe it is better to have set CACHE_COHERENCY false set on the common bds instead of having the check if the variable exists condition.
Also, I would revert adi_bd.tcl since ad_mem_hpc0_interconnect ad_mem_hpc1_interconnect methods already exist and the only project that leverages it can call the methods directly without further condition checks.
| ad_mem_hp0_interconnect sys_cpu_clk sys_ps8/S_AXI_HP0 | ||
| ad_mem_hp0_interconnect sys_cpu_clk axi_ad9680_xcvr/m_axi | ||
| ad_mem_hp1_interconnect sys_dma_clk sys_ps7/S_AXI_HP1 | ||
| ad_mem_hp1_interconnect sys_dma_clk axi_ad9680_dma/m_dest_axi | ||
| ad_mem_hp3_interconnect sys_dma_clk sys_ps7/S_AXI_HP3 | ||
| ad_mem_hp3_interconnect sys_dma_clk axi_ad9152_dma/m_src_axi | ||
| ad_mem_hpc0_interconnect sys_dma_clk sys_ps8/S_AXI_HPC0 | ||
| ad_mem_hpc0_interconnect sys_dma_clk axi_ad9680_dma/m_dest_axi | ||
| ad_mem_hpc1_interconnect sys_dma_clk sys_ps8/S_AXI_HPC1 | ||
| ad_mem_hpc1_interconnect sys_dma_clk axi_ad9152_dma/m_src_axi |
There was a problem hiding this comment.
This is the only project that actually benefits from the cache coherence port and is not using the updated method.
There was a problem hiding this comment.
Jupiter also uses this approach, since it is based purely on Zynq Ultrascale; the same for adrv9009zu11eg.
There was a problem hiding this comment.
True, my point stands, the projects that leverages it call the methods directly without further condition checks, so I would not update adi_board.tcl hp methods
| ### SPDX short identifier: ADIBSD | ||
| ############################################################################### | ||
|
|
||
| set CACHE_COHERENCY true |
There was a problem hiding this comment.
Isn't it better to set
set CACHE_COHERENCY false
on the other common projects and drop the
if {![info exists CACHE_COHERENCY]} {
set CACHE_COHERENCY false
}
from every project//common/_bd.tcl ?
There was a problem hiding this comment.
You mean setting this parameter to false for every platform in projects/common/*, as in zcu102_system_bd.tcl, right? Yes, this can be done.
| if {$sys_zynq == -1} {ad_mem_hpx_interconnect "SIM" $p_clk $p_name} | ||
| if {$sys_zynq == 0} {ad_mem_hpx_interconnect "MEM" $p_clk $p_name} | ||
| if {$sys_zynq == 1} {ad_mem_hpx_interconnect "HP0" $p_clk $p_name} | ||
| if {$sys_zynq == 2} {ad_mem_hpx_interconnect "HP0" $p_clk $p_name} | ||
| if {$sys_zynq == 2} {ad_mem_hpx_interconnect [expr {$p_cc ? "HPC0" : "HP0"}] $p_clk $p_name} |
There was a problem hiding this comment.
I would revert adi_board.tcl, and have the user use ad_mem_hpc0/1_interconnect.
If he needs to dynamically use or not, he should at project//common/_bd.tcl:
if {![info exists CACHE_COHERENCY]} {
ad_mem_hp1_interconnect sys_dma_clk sys_ps8/S_AXI_HP1
} else {
ad_mem_hpc0_interconnect sys_dma_clk sys_ps8/S_AXI_HPC0
}
But considering the projects that will use cache coherence are only with ps8, this snippet will be at project/*/zcu102/system_bd.tcl anyway, not requiring the if.
There was a problem hiding this comment.
This was one of the 2 possible ways to implement the interface selection based on the CACHE_COHERENCY parameter, using an if in project/common. I chose the adi_board.tcl approach to try to reduce the added changes in the common/_bd files and move them in the background, in adi_board.tcl. Though I admit that using an if in common allows for a more precise selection of the coherent interfaces.
Also, as a note, Versal platforms may also implement cache coherency in the future.
PR Description
This commit enables Cache Coherency on Ultrascale+ projects, leaving it disabled for the other platforms.
Updated the adi_board.tcl script to automatically select the coherent interface, based on the CACHE_COHERENCY parameter.
PR Type
PR Checklist