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

[ibex,fpga] Apply Ibex FPGA patches #25480

Merged
merged 2 commits into from
Dec 9, 2024
Merged

[ibex,fpga] Apply Ibex FPGA patches #25480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
370ca8d
[ibex,fpga] Fix FPGA related Ibex RF issue
nasahlpa Nov 29, 2024
89ea9f8
[ibex,fpga] Fix FPGA related Ibex counter issue
nasahlpa Dec 2, 2024
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
8 changes: 6 additions & 2 deletions hw/vendor/lowrisc_ibex/rtl/ibex_counter.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,12 @@ module ibex_counter #(
localparam int DspPragma = CounterWidth < 49 ? "yes" : "no";
(* use_dsp = DspPragma *) logic [CounterWidth-1:0] counter_q;

// DSP output register requires synchronous reset.
`define COUNTER_FLOP_RST posedge clk_i
if (CounterWidth < 49) begin : g_dsp_counter
// DSP output register requires synchronous reset.
`define COUNTER_FLOP_RST posedge clk_i
end else begin : g_no_dsp_counter
`define COUNTER_FLOP_RST posedge clk_i or negedge rst_ni
end
Comment on lines +58 to +63
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a note that this code relying on a pre-processor `define isn't generally correct, because the pre-processor would resolve the `define before elaboration resolves the parameterized if-else construct. However, this only affects Xilinx code (it's inside an `ifdef FPGA_XILINX) and @nasahlpa tested that this works as intended in Vivado.

On master a proper fix has been applied (#25458), but that would also modify ASIC RTL, which we don't allow on this branch.

`else
logic [CounterWidth-1:0] counter_q;

Expand Down
8 changes: 4 additions & 4 deletions hw/vendor/lowrisc_ibex/rtl/ibex_register_file_fpga.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,14 +147,14 @@ module ibex_register_file_fpga #(
.out_o (mem_o_b)
);

assign rdata_a_o = (raddr_a_i == '0) ? '0 : mem_o_a;
assign rdata_b_o = (raddr_b_i == '0) ? '0 : mem_o_b;
assign rdata_a_o = (raddr_a_i == '0) ? WordZeroVal : mem_o_a;
assign rdata_b_o = (raddr_b_i == '0) ? WordZeroVal : mem_o_b;
end else begin : gen_no_rdata_mux_check
// async_read a
assign rdata_a_o = (raddr_a_i == '0) ? '0 : mem[raddr_a_i];
assign rdata_a_o = (raddr_a_i == '0) ? WordZeroVal : mem[raddr_a_i];

// async_read b
assign rdata_b_o = (raddr_b_i == '0) ? '0 : mem[raddr_b_i];
assign rdata_b_o = (raddr_b_i == '0) ? WordZeroVal : mem[raddr_b_i];
end

// we select
Expand Down
23 changes: 23 additions & 0 deletions hw/vendor/patches/lowrisc_ibex/rtl/0001-Fix-fpga-register-file.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
diff --git a/ibex_register_file_fpga.sv b/ibex_register_file_fpga.sv
index 7a0ae34b15..73e0afab2f 100644
--- a/ibex_register_file_fpga.sv
+++ b/ibex_register_file_fpga.sv
@@ -147,14 +147,14 @@ module ibex_register_file_fpga #(
.out_o (mem_o_b)
);

- assign rdata_a_o = (raddr_a_i == '0) ? '0 : mem_o_a;
- assign rdata_b_o = (raddr_b_i == '0) ? '0 : mem_o_b;
+ assign rdata_a_o = (raddr_a_i == '0) ? WordZeroVal : mem_o_a;
+ assign rdata_b_o = (raddr_b_i == '0) ? WordZeroVal : mem_o_b;
end else begin : gen_no_rdata_mux_check
// async_read a
- assign rdata_a_o = (raddr_a_i == '0) ? '0 : mem[raddr_a_i];
+ assign rdata_a_o = (raddr_a_i == '0) ? WordZeroVal : mem[raddr_a_i];

// async_read b
- assign rdata_b_o = (raddr_b_i == '0) ? '0 : mem[raddr_b_i];
+ assign rdata_b_o = (raddr_b_i == '0) ? WordZeroVal : mem[raddr_b_i];
end

// we select
19 changes: 19 additions & 0 deletions hw/vendor/patches/lowrisc_ibex/rtl/0002-Fix-fpga-counter-reset.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
diff --git a/ibex_counter.sv b/ibex_counter.sv
index c78e510ee4..b4dc7ec347 100644
--- a/ibex_counter.sv
+++ b/ibex_counter.sv
@@ -55,8 +55,12 @@ module ibex_counter #(
localparam int DspPragma = CounterWidth < 49 ? "yes" : "no";
(* use_dsp = DspPragma *) logic [CounterWidth-1:0] counter_q;

- // DSP output register requires synchronous reset.
- `define COUNTER_FLOP_RST posedge clk_i
+ if (CounterWidth < 49) begin : g_dsp_counter
+ // DSP output register requires synchronous reset.
+ `define COUNTER_FLOP_RST posedge clk_i
+ end else begin : g_no_dsp_counter
+ `define COUNTER_FLOP_RST posedge clk_i or negedge rst_ni
+ end
`else
logic [CounterWidth-1:0] counter_q;

Loading