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.

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

[tlul/rtl] Fix SRAM timing #22588

Merged
merged 3 commits into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 4 additions & 3 deletions hw/ip/sram_ctrl/rtl/sram_ctrl.sv
Original file line number Diff line number Diff line change
Expand Up @@ -568,8 +568,9 @@ module sram_ctrl
u_tlul_adapter_sram.u_rspfifo.gen_normal_fifo.u_fifo_cnt.gen_secure_ptrs.u_rptr,
alert_tx_o[0])

// `tlul_gnt` doesn't factor in `sram_gnt` for timing reasons. This assertion checks that
// a `tlul_req` is always granted when `sram_gnt` has been asserted and we're not doing an init.
`ASSERT(TlulGntIsCorrect_A, tlul_req & sram_gnt & ~init_req |-> tlul_gnt)
// `tlul_gnt` doesn't factor in `sram_gnt` for timing reasons. This assertions checks that
// `tlul_gnt` is the same as `sram_gnt` when there's an active `tlul_req` that isn't being ignored
// because the SRAM is initializing.
`ASSERT(TlulGntIsCorrect_A, tlul_req |-> (sram_gnt & ~init_req) == tlul_gnt)

endmodule : sram_ctrl
20 changes: 19 additions & 1 deletion hw/ip/tlul/rtl/tlul_adapter_sram.sv
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,24 @@ module tlul_adapter_sram
(vld_rd_rsp) ? rspfifo_rdata.data_intg : // valid read
prim_secded_pkg::SecdedInv3932ZeroEcc; // valid write

// When an error is seen on an incoming transaction it gets an immediate response without
// performing an SRAM request. It may be the transaction receives a ready the first cycle it is
// seen, but if not we force a ready the following cycle. This avoids factoring the error
// calculation into the outgoing ready preventing a feedthrough path from the incoming tilelink
// signals to the outgoing tilelink signals.
logic missed_err_gnt_d, missed_err_gnt_q;

// Track whether we've seen an incoming transaction with an error that didn't get a ready
assign missed_err_gnt_d = error_internal & tl_i_int.a_valid & ~tl_o_int.a_ready;

always_ff @(posedge clk_i or negedge rst_ni) begin
if (!rst_ni) begin
missed_err_gnt_q <= 1'b0;
end else begin
missed_err_gnt_q <= missed_err_gnt_d;
end
end

assign tl_o_int = '{
d_valid : d_valid ,
d_opcode : (d_valid && reqfifo_rdata.op != OpRead) ? AccessAck : AccessAckData,
Expand All @@ -310,7 +328,7 @@ module tlul_adapter_sram
d_data : d_data,
d_user : '{default: '0, data_intg: data_intg},
d_error : d_valid && d_error,
a_ready : (gnt_i | error_internal) & reqfifo_wready & sramreqfifo_wready
a_ready : (gnt_i | missed_err_gnt_q) & reqfifo_wready & sramreqfifo_wready
Copy link
Contributor

Choose a reason for hiding this comment

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

One thing I had noticed while working on Sonata is that a missed error causes an a_ready to be sent but the error response never gets written to the request FIFO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Odd, the ready shouldn't be signalled unless there's space in the FIFO and the FIFO is written when you have a_ready and a_valid asserted. It would be good to understand the issue you'd be seeing in more detail.

};

// a_ready depends on the FIFO full condition and grant from SRAM (or SRAM arbiter)
Expand Down
24 changes: 7 additions & 17 deletions hw/ip/tlul/rtl/tlul_err_resp.sv
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ module tlul_err_resp (
tl_a_op_e err_opcode;
logic [$bits(tl_h_i.a_source)-1:0] err_source;
logic [$bits(tl_h_i.a_size)-1:0] err_size;
logic err_req_pending, err_rsp_pending;
logic err_rsp_pending;
mubi4_t err_instr_type;
tlul_pkg::tl_d2h_t tl_h_o_int;

Expand All @@ -32,24 +32,24 @@ module tlul_err_resp (

always_ff @(posedge clk_i or negedge rst_ni) begin
if (!rst_ni) begin
err_req_pending <= 1'b0;
err_rsp_pending <= 1'b0;
err_source <= {top_pkg::TL_AIW{1'b0}};
err_opcode <= Get;
err_size <= '0;
err_instr_type <= MuBi4False;
end else if (err_rsp_pending && tl_h_i.d_ready) begin
err_rsp_pending <= 1'b0;
end else if (tl_h_i.a_valid && tl_h_o_int.a_ready) begin
err_req_pending <= 1'b1;
err_rsp_pending <= 1'b1;
err_source <= tl_h_i.a_source;
err_opcode <= tl_h_i.a_opcode;
err_size <= tl_h_i.a_size;
err_instr_type <= tl_h_i.a_user.instr_type;
end else if (!err_rsp_pending) begin
err_req_pending <= 1'b0;
end
end

assign tl_h_o_int.a_ready = ~err_rsp_pending & ~(err_req_pending & ~tl_h_i.d_ready);
assign tl_h_o_int.d_valid = err_req_pending | err_rsp_pending;
assign tl_h_o_int.a_ready = ~err_rsp_pending;
assign tl_h_o_int.d_valid = err_rsp_pending;
assign tl_h_o_int.d_data = (mubi4_test_true_strict(err_instr_type)) ? DataWhenInstrError :
DataWhenError;
assign tl_h_o_int.d_source = err_source;
Expand All @@ -60,16 +60,6 @@ module tlul_err_resp (
assign tl_h_o_int.d_user = '0;
assign tl_h_o_int.d_error = 1'b1;

always_ff @(posedge clk_i or negedge rst_ni) begin
if (!rst_ni) begin
err_rsp_pending <= 1'b0;
end else if ((err_req_pending || err_rsp_pending) && !tl_h_i.d_ready) begin
err_rsp_pending <= 1'b1;
end else begin
err_rsp_pending <= 1'b0;
end
end

// Waive unused bits of tl_h_i
logic unused_tl_h;
assign unused_tl_h = ^tl_h_i;
Expand Down
Loading