openpiton icon indicating copy to clipboard operation
openpiton copied to clipboard

Ariane : Translation Shift fix with Piton UART Stream

Open Raghavendra-Srinivas opened this issue 5 years ago • 3 comments

Hi,

On Genesys2 with MIG with AXI interface: Discussed with Jon on problem of AXI address from NoC with UART boot option. As per his suggestion, It seems on this line, for ariane, it might to be shift by 6 bits right to achieve null translation eventually for pitonstream to load test and work correctly. Test worked properly with the fix. https://github.com/PrincetonUniversity/openpiton/blob/cddf5416fc6a7e39976fb11838254ba8c41cbd46/piton/design/chipset/rtl/storage_addr_trans_unified.v.pyv#L66

Thanks, Raghav

Raghavendra-Srinivas avatar May 30 '20 19:05 Raghavendra-Srinivas

@grigoriy-chirkov I'm looking at the definition of the ADDR_TRANS_PHYS_WIDTH_ALIGN macro and wondering if Michael chose the wrong ones here. What do you think?

https://github.com/PrincetonUniversity/openpiton/blob/3eefdef42e03d1df14242e551cda2ef8c964bcf6/piton/design/chipset/include/chipset_define.vh#L78-L90

At some point the shifts were done more manually: https://github.com/PrincetonUniversity/openpiton/blob/ec2437c11577d7fef939c80ea8ee66c29cc3c5a5/piton/design/chipset/rtl/storage_addr_trans.v#L46-L76

Are all of the boards off by some factor like Raghav is seeing?

@Raghavendra-Srinivas How much additional shift did you have to add versus what was already there? By which I mean, how much was the code above off by? Just off-by-one? Or was it 6 like you said?

Jbalkind avatar Jun 08 '20 22:06 Jbalkind

Hi @Jbalkind ,

ADDR_TRANS_PHYS_WIDTH_ALIGN is related to phy. I did not look much into it.

But My intention was to to give null translation for only ARIANE case while storing the image through UART (Pitostream flow). so "storage_addr_trans_unique" is the right file. In this file, currently it using the above define which is doing right shift by 5, then again you left shift by 3 , so eventually it giving affect of effective right shift by 2. and then later on there is another left shift by 3 in noc_bridge_write/read.sv files. so , overall it was NOT giving the intended effect of null translation.

So, what I did is right shift by ONE more additional bit like below in file "storage_addr_trans_unified.v.pyv". https://github.com/Raghavendra-Srinivas/openpitonHawk/blob/9e68f75791ebb153d0354a82b9c45716cf9190c5/piton/design/chipset/rtl/storage_addr_trans_unified.v.pyv#L67

Test cases passed on FPGA with this change.

PS : I have enabled AXI based Memory controller (MIG) for Genesys2 board at my end with Ariane. So above data path for genesys2 with ariane was tested for first time it seems as default openpiton is not supported with AXI based MIG for genesys2. So, may be that's why you did not see any such issue.

Thanks, Raghav

Raghavendra-Srinivas avatar Jun 09 '20 15:06 Raghavendra-Srinivas

The shift inside "storage_addr_trans_unified" is fine for MIG with Native interface. My issue and solution as mentioned above is applicable to MIG with AXI interface,

Raghavendra-Srinivas avatar Jun 10 '20 22:06 Raghavendra-Srinivas