-
Notifications
You must be signed in to change notification settings - Fork 807
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
[membkdr] Provide a tile adapter to access internal rows #26006
base: master
Are you sure you want to change the base?
Conversation
61b80e2
to
f22c654
Compare
2f22457
to
4c7e9aa
Compare
d071b91
to
408d8dc
Compare
036b6a6
to
6f4b47a
Compare
cbafd0f
to
58a72db
Compare
At the moment I am not sure why I get a Null error here |
58a72db
to
5f63639
Compare
@rswarbrick I am not sure how to fix that. Can you help me on that? if (row_adapter != null) begin
this.row_adapter = row_adapter;
end else begin
this.row_adapter = new();
end And later on, the row adapter is used like: this.width = (n_bits / depth) - this.row_adapter.get_num_extra_bits(); However, this causes the following error (line 132 is the line from above when calling
|
Hmm, that seems really odd to me: I'm not sure what's going on. Can I suggest debugging by adding some print statements inside the |
5f63639
to
b88d6b9
Compare
Stupid me! After the if/else, the default assignment from the parameter to the member variable was still there. This overwrote the previously allocated object with null again. @rswarbrick Fixed now, please take a look. |
Ha! I didn't notice it either :-) I'll have a proper look now. |
4708c15
to
bc91835
Compare
if (!check_addr_valid(addr)) return 'x; | ||
index = addr >> addr_lsb; | ||
ram_tile = index / tile_depth; | ||
res = uvm_hdl_read($sformatf("%0s[%0d]", get_full_path(ram_tile), index), data); | ||
res = uvm_hdl_read($sformatf("%0s[%0d]", get_full_path(ram_tile), index), encoded_row); | ||
data = row_adapter.decode_row(encoded_row); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now I've understood a bit more, I'm not sure this is quite right. I think this is returning several words of data packed together? But the read
function is supposed to return just one word, isn't it?
Don't we need to extract just one lane here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on how the functions and variables are currently named, I agree with @rswarbrick. This code seems to decode a row and assign that to data. @Razer6: Doesn't this function need to select a word from the decoded row based on addr
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the later function read8
(which calls read
) needs a proper access function to select the right word?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see. I think what confused me is the short description of this function ("Read the entire word at the given address."). I suggest we change word to memory row, because it may contain more than one word with parity bits, depending on the memory architecture. Does that make sense?
res = uvm_hdl_deposit($sformatf("%0s[%0d]", get_full_path(ram_tile), index), data); | ||
index = addr >> addr_lsb; | ||
ram_tile = index / tile_depth; | ||
encoded_row = row_adapter.encode_row(data); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this has the same problem as the read
function above. If we've got multiple words in the row, won't this need to do a read-modify-write?
9e4a0a6
to
f5dc3a5
Compare
if (!check_addr_valid(addr)) return 'x; | ||
index = addr >> addr_lsb; | ||
ram_tile = index / tile_depth; | ||
res = uvm_hdl_read($sformatf("%0s[%0d]", get_full_path(ram_tile), index), data); | ||
res = uvm_hdl_read($sformatf("%0s[%0d]", get_full_path(ram_tile), index), encoded_row); | ||
data = row_adapter.decode_row(encoded_row); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on how the functions and variables are currently named, I agree with @rswarbrick. This code seems to decode a row and assign that to data. @Razer6: Doesn't this function need to select a word from the decoded row based on addr
?
f5dc3a5
to
a5bdde7
Compare
Signed-off-by: Robert Schilling <[email protected]>
a5bdde7
to
ac2cc08
Compare
This PR adds a custom row adapter that is being used by the memory backdoor util to access and manipulate rows of data. Integrators might are using SRAM primitives with a different internal organization than
Width x Depth
. They can provide a custom row adapter to the memory backdoor utility to for the needs of their memory prims.