Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fixed clock alignment in axi_stream_master vc #420

Merged
merged 4 commits into from
Dec 10, 2018
Merged

Fixed clock alignment in axi_stream_master vc #420

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0500e20
Split axi stream master process into two separate processes, as was d…
Dec 10, 2018
1e10b14
removed whitespaces and extra empty line
Dec 10, 2018
238718f
added delete of msg after it was handled and before a new msg is popp…
Dec 10, 2018
49aedb0
wait for valid to become 1, so that "not_valid" checks do not interfe…
Dec 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 60 additions & 29 deletions vunit/vhdl/verification_components/src/axi_stream_master.vhd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,28 @@ entity axi_stream_master is
end entity;

architecture a of axi_stream_master is
constant message_queue : queue_t := new_queue;
begin
main : process
variable request_msg : msg_t;
variable msg_type : msg_type_t;
begin
receive(net, master.p_actor, request_msg);
msg_type := message_type(request_msg);

if msg_type = stream_push_msg or msg_type = push_axi_stream_msg then
push(message_queue, request_msg);
elsif msg_type = wait_for_time_msg then
push(message_queue, request_msg);
elsif msg_type = wait_until_idle_msg then
wait until tvalid = '0' and is_empty(message_queue) and rising_edge(aclk);
handle_wait_until_idle(net, msg_type, request_msg);
else
unexpected_msg_type(msg_type);
end if;
end process;

bus_process : process
variable msg : msg_t;
variable msg_type : msg_type_t;
begin
Expand All @@ -50,39 +70,50 @@ begin
tuser <= (others => drive_invalid_val_user);
end if;

receive(net, master.p_actor, msg);
msg_type := message_type(msg);
-- Wait for messages to arrive on the queue, posted by the process above
wait until rising_edge(aclk) and not is_empty(message_queue);

handle_sync_message(net, msg_type, msg);
while not is_empty(message_queue) loop

if msg_type = stream_push_msg or msg_type = push_axi_stream_msg then
tvalid <= '1';
tdata <= pop_std_ulogic_vector(msg);
if msg_type = push_axi_stream_msg then
tlast <= pop_std_ulogic(msg);
tkeep <= pop_std_ulogic_vector(msg);
tstrb <= pop_std_ulogic_vector(msg);
tid <= pop_std_ulogic_vector(msg);
tdest <= pop_std_ulogic_vector(msg);
tuser <= pop_std_ulogic_vector(msg);
else
if pop_boolean(msg) then
tlast <= '1';
msg := pop(message_queue);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The msg needs to be deallocated unless memory will leak. Previously the msg was deallocated by being passed into the receive procedure in the main process. Now the msg is nulled in the main process when being pushed into the queue since ownership is transfered and thus it must be explicitly deallocated in the bus_process.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree, will change that.. Technically it also needs to be done after the unexpected_msg_type call in the main process I guess?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it could have been done explicitly in the main process as well but the receive procedure will deallocate its input argument if it is non-null so there was no memory leak in the original code. Maybe it is confusing and could be more readable with an explicit deallocate.

msg_type := message_type(msg);

if msg_type = wait_for_time_msg then
handle_sync_message(net, msg_type, msg);
-- Re-align with the clock when a wait for time message was handled, because this breaks edge alignment.
wait until rising_edge(aclk);
elsif msg_type = stream_push_msg or msg_type = push_axi_stream_msg then
tvalid <= '1';
tdata <= pop_std_ulogic_vector(msg);
if msg_type = push_axi_stream_msg then
tlast <= pop_std_ulogic(msg);
tkeep <= pop_std_ulogic_vector(msg);
tstrb <= pop_std_ulogic_vector(msg);
tid <= pop_std_ulogic_vector(msg);
tdest <= pop_std_ulogic_vector(msg);
tuser <= pop_std_ulogic_vector(msg);
else
tlast <= '0';
if pop_boolean(msg) then
tlast <= '1';
else
tlast <= '0';
end if;
tkeep <= (others => '1');
tstrb <= (others => '1');
tid <= (others => '0');
tdest <= (others => '0');
tuser <= (others => '0');
end if;
tkeep <= (others => '1');
tstrb <= (others => '1');
tid <= (others => '0');
tdest <= (others => '0');
tuser <= (others => '0');
wait until (tvalid and tready) = '1' and rising_edge(aclk);
tvalid <= '0';
tlast <= '0';
else
unexpected_msg_type(msg_type);
end if;
wait until (tvalid and tready) = '1' and rising_edge(aclk);
tvalid <= '0';
tlast <= '0';
else
unexpected_msg_type(msg_type);
end if;

delete(msg);
end loop;

end process;

axi_stream_monitor_generate : if master.p_monitor /= null_axi_stream_monitor generate
Expand Down Expand Up @@ -123,4 +154,4 @@ begin
);
end generate axi_stream_protocol_checker_generate;

end architecture;
end architecture;
30 changes: 27 additions & 3 deletions vunit/vhdl/verification_components/test/tb_axi_stream.vhd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,7 @@ begin
pop_stream(net, slave_stream, data, last_bool);
check_equal(data, std_logic_vector'(x"77"), result("for pop stream data"));
check_true(last_bool, result("for pop stream last"));
check_equal(now - 10 ns, timestamp + 30 ns, result("for push wait time"));

check_equal(now - 10 ns, timestamp + 50 ns, result("for push wait time")); -- two extra cycles inserted by alignment
for i in 1 to n_monitors loop
get_axi_stream_transaction(axi_stream_transaction);
check_equal(
Expand Down Expand Up @@ -225,6 +224,25 @@ begin
check_true(axi_stream_transaction.tlast, result("for axi_stream_transaction.tlast"));
end loop;

elsif run("test single stalled push and pop with non-multiple of clock period") then
wait until rising_edge(aclk);
wait_for_time(net, master_sync, 29 ns);
timestamp := now;
push_stream(net, master_stream, x"77", true);
pop_stream(net, slave_stream, data, last_bool);
check_equal(data, std_logic_vector'(x"77"), result("for pop stream data"));
check_true(last_bool, result("for pop stream last"));
check_equal(now - 10 ns, timestamp + 40 ns, result("for push wait time")); -- Aligned to clock edge again
for i in 1 to n_monitors loop
get_axi_stream_transaction(axi_stream_transaction);
check_equal(
axi_stream_transaction.tdata,
std_logic_vector'(x"77"),
result("for axi_stream_transaction.tdata")
);
check_true(axi_stream_transaction.tlast, result("for axi_stream_transaction.tlast"));
end loop;

elsif run("test pop before push") then
for i in 0 to 7 loop
pop_stream(net, slave_stream, reference);
Expand All @@ -235,6 +253,8 @@ begin
push_stream(net, master_stream, std_logic_vector(to_unsigned(i + 1, data'length)), true);
end loop;

wait_until_idle(net, master_sync); -- wait until all transfers are done before checking them

for i in 0 to 7 loop
reference := pop(reference_queue);
await_pop_stream_reply(net, reference, data);
Expand All @@ -259,10 +279,14 @@ begin
check_axi_stream(net, slave_axi_stream, x"12", '1', msg => "reduced checking axi stream");

elsif run("test failing check") then

push_axi_stream(net, master_axi_stream, x"11", tlast => '0', tkeep => "0", tstrb => "0", tid => x"22", tdest => x"33", tuser => x"44");
-- Delay mocking the logger to prevent 'invalid checks' from failing the checks below
wait until rising_edge (aclk) and tvalid = '1';

mocklogger := get_logger("check");
mock(mocklogger);

push_axi_stream(net, master_axi_stream, x"11", tlast => '0', tkeep => "0", tstrb => "0", tid => x"22", tdest => x"33", tuser => x"44");
check_axi_stream(net, slave_axi_stream, x"12", '1', "1", "1", x"23", x"34", x"45", "checking axi stream");
check_log(mocklogger, "checking axi stream - Got 0001_0001 (17). Expected 0001_0010 (18).", error);
check_log(mocklogger, "checking axi stream - Got 0. Expected 1.", error);
Expand Down