Skip to content

Fixes and CI version bumps #45

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

Open
micprog wants to merge 3 commits into
base: main
Choose a base branch
from
+16 −11
Open

Fixes and CI version bumps #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
85bb5cd
Don't mix continuous and procedural connections
micprog Jul 10, 2026
cd86f87
Bump CI versions
micprog Jul 10, 2026
5fcbcd6
Simplify demux signalling from review
micprog Aug 3, 2026
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
6 changes: 3 additions & 3 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
name: Check license
runs-on: ubuntu-latest
steps:
- uses: pulp-platform/pulp-actions/lint-license@v2
- uses: pulp-platform/pulp-actions/lint-license@v2.5.1
with:
license: |
Copyright (\d{4}(-\d{4})?\s)?ETH Zurich and University of Bologna.
Expand All @@ -27,7 +27,7 @@ jobs:
lint-verilog:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v7
- uses: chipsalliance/verible-linter-action@main
with:
paths: |
Expand All @@ -42,7 +42,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Mirror and check
uses: pulp-platform/pulp-actions/gitlab-ci@v2
uses: pulp-platform/pulp-actions/gitlab-ci@v2.5.1
# Skip on forks or pull requests from forks due to missing secrets.
if: >
github.repository == 'pulp-platform/obi' &&
Expand Down
2 changes: 1 addition & 1 deletion .gitlab-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ stages:
- sim

variables:
VSIM: "questa-2022.3 vsim"
VSIM: "questa-2026.1 vsim"

build:
stage: build
Expand Down
19 changes: 12 additions & 7 deletions src/obi_demux.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,25 +41,25 @@ module obi_demux #(
logic sbr_port_gnt;
logic sbr_port_rready;
logic rsp_phase_stalled;
logic [NumMgrPorts-1:0] mgr_reqs;
logic [NumMgrPorts-1:0] mgr_connect;

select_t select_d, select_q;

always_comb begin : proc_req
select_d = select_q;
cnt_up = 1'b0;
for (int i = 0; i < NumMgrPorts; i++) begin
mgr_ports_req_o[i].req = 1'b0;
mgr_ports_req_o[i].a = '0;
end
mgr_reqs = '0;
mgr_connect = '0;
sbr_port_gnt = 1'b0;

if (!overflow) begin
// R-4.1.1: block source changes while a stalled R phase is active
if (sbr_port_select_i == select_q || (!rsp_phase_stalled &&
(in_flight == '0 || (in_flight == 1 && cnt_down)))) begin

@LucaRufer LucaRufer Jul 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

An issue I see (not related to this PR), is that
mgr_ports_req_o[i].req depends on mgr_reqs[i] depends on cnt_down, which depends on sbr_port_rsp_o.rvalid, depends on mgr_ports_rsp_i[select_q].rvalid, which is not legal as per OBI v1.6.0 R-21.1 ("For a manager, req shall not combinatorially depend on gnt or rvalid (nor on gntpar or rvalidpar").
So the (in_flight == 1 && cnt_down)) condition would have to be removed to resolve this dependency.

@micprog micprog Aug 3, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, I see. This was added to reduce stalling, but I see that it can be an issue. Let's properly address this in a separate issue, and/or PR!

@LucaRufer LucaRufer Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I created #46 to address this issue.

mgr_ports_req_o[sbr_port_select_i].req = sbr_port_req_i.req;
mgr_ports_req_o[sbr_port_select_i].a = sbr_port_req_i.a;
sbr_port_gnt = mgr_ports_rsp_i[sbr_port_select_i].gnt;
mgr_reqs[sbr_port_select_i] = sbr_port_req_i.req;
mgr_connect[sbr_port_select_i] = 1'b1;
sbr_port_gnt = mgr_ports_rsp_i[sbr_port_select_i].gnt;
end
end

Expand All @@ -69,6 +69,11 @@ module obi_demux #(
end
end

for (genvar i = 0; i < NumMgrPorts; i++) begin : gen_req_assign
assign mgr_ports_req_o[i].req = mgr_reqs[i];
assign mgr_ports_req_o[i].a = mgr_connect[i] ? sbr_port_req_i.a : '0;
end

assign sbr_port_rsp_o.gnt = sbr_port_gnt;
assign sbr_port_rsp_o.r = mgr_ports_rsp_i[select_q].r;
assign sbr_port_rsp_o.rvalid = mgr_ports_rsp_i[select_q].rvalid;
Expand Down
Loading