-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RAM32M/RAM64M not initialized with INIT_A/INIT_B/INIT_C/INIT_D parameters #28
Comments
Reverted hansemro@dc23177 as I mistakenly used hansemro@262ae45 reverts back to using
Prior (with hansemro@dc23177), it looked like the following:
|
Pushed a fix for above (hansemro@7f459c9), where I chose to interleave bits in reverse. |
Hmm, I get this:
I pushed my config files to the prmitives-tests repo. |
@hansfbaier Thanks for testing! Make sure to build the test with |
Now I get:
I use the hansemro/fix-ram32m-ram64m-init-v2 branch for nextpnr-xilinx |
Great! It matches Vivado results (feel free to confirm this yourself). |
I did not build the vivado version yet. |
Shouldn't it show this?
I still don't understand most of the code. Had very little time with it yet. |
Really cool! Looks like it works!
|
@hansemro
|
This is expected. RAM64M only has 4 data ports whereas RAM32M has 8. |
Ah I understand, great, thanks! |
Thanks for asking since I did find some errors while trying to look into unshuffling the values from openocd. In short, INIT_{A:D} parameters should be 64-bits, and I flipped parameter assignments (INIT_D<->INIT_A; INIT_C<->INIT_B). Because I did mess up how INIT patterns were assigned, let me share first how we can readback the original lower 128-bits of the INIT using RAM32M. Focusing on ports and parameters with the _D suffix in https://github.com/openXC7/primitive-tests/blob/07430a895ae9b5b804acbf4e0cd060e33662be57/lutram-tests/jtag-test/lutram_inst.vh#L270-L291, we can see that DOD[1:0] maps to lutram_do[1:0] and that INIT_D maps to INIT[127:96]=32'h01234567=32'b00_00_00_01_00_10_00_11_01_00_01_01_01_10_01_11 [*]. So let's try reading this back first. Reading address 5'h0, lutram_do[1:0] fetches INIT[97:96]=2'b11. lutram_do[0] reads back every even bits of INIT_D (starting at bit 0) and lutram_do[1] reads back every odd bits (starting at bit 1) The same principle applies to memory groups A, B, and C. [*] Two mistakes made here: (1) INIT_{A:D} are 64-bits and not 32-bits; (2) Flipped parameter assignments: INIT_D swapped with INIT_A and INIT_C swapped with INIT_B. Reading INIT_D in OpenOCD
Openocd output:
|
openXC7/primitive-tests#2 changes INIT_{A:D} parameters, so here is the new expected RAM32M vivado result:
Fix by #29 is still valid (thankfully). |
Issue Description
As discovered in #20 (comment), nextpnr packer was checking the wrong INIT parameter without the underscore and produce fasm result with INIT parameters unset.
Additionally, in the following post in #20 (comment), I became more concerned after finding INIT pattern discrepancies between Vivado and NextPNR fasm results.
Indeed, with the recently merged LUTRAM over JTAG Primitive Test, it becomes more clear that my initial fix (https://github.com/hansemro/nextpnr-xilinx/tree/fix-ram32m-ram64m-init) would have functional errors due to invalid INIT patterns.
Affects OpenXC7 toolchain 0.8.2 and older.
Steps to Reproduce
[Affects RAM32M+RAM64M] Missing INIT underscore:
Update target parameters in Makefile (if not using SQRL Acorn CLE215(+)).
Build LUTRAM over JTAG test with RAM32M using openXC7 toolchain:
INIT
parameters for transformedbe.ram32m/*
cells intop.pack.json
.[Affects RAM32M only] Invalid INIT patterns after correcting underscore typo
fix-ram32m-ram64m-init-0.8.2
branch in my nextpnr-xilinx fork:fix-ram32m-ram64m-init-0.8.2
branch of nextpnr-xilinx[REFERENCE] Building JTAG over LUTRAM Using Vivado Toolchain
build-vivado.tcl
insideprimitive-tests/lutram-tests/jtag-test/
with the following:build-vivado.sh
insideprimitive-tests/lutram-tests/jtag-test/
with the following:build-vivado.sh
:<BOARD>_<PART>_<LUTRAM>/top.vivado.bit
to the board.The text was updated successfully, but these errors were encountered: