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

Unused PE slaves #25

Open
micprog opened this issue Jan 25, 2021 · 4 comments
Open

Unused PE slaves #25

micprog opened this issue Jan 25, 2021 · 4 comments

Comments

@micprog
Copy link
Member

micprog commented Jan 25, 2021

If I am not mistaken, there are currently several unused peripheral slave signals connected to the PE_XBAR in cluster_interconnect_wrap, with speriph_bus linked to cluster_peripherals. pulp declares NB_SPERIPHS=10, while only 8 are actually used, with ID=3 and ID=8 unused. Below the definition of peripheral slave IDs (missing = 3, SPER_DECOMP_ID unused/set to 0).

// position of peripherals on slave port of periph interconnect
parameter SPER_EOC_ID = 0;
parameter SPER_TIMER_ID = 1;
parameter SPER_EVENT_U_ID = 2;
parameter SPER_HWPE_ID = 4;
parameter SPER_ICACHE_CTRL = 5;
parameter SPER_DMA_CL_ID = 6;
parameter SPER_DMA_FC_ID = 7;
parameter SPER_DECOMP_ID = 8;
parameter SPER_EXT_ID = 9;

assign speriph_slave[SPER_DECOMP_ID].gnt = '0;
assign speriph_slave[SPER_DECOMP_ID].r_rdata = '0;
assign speriph_slave[SPER_DECOMP_ID].r_opc = '0;
assign speriph_slave[SPER_DECOMP_ID].r_id = '0;
assign speriph_slave[SPER_DECOMP_ID].r_valid = '0;

Are the missing peripherals intended to be added? Or can the signals be removed and the other IDs properly aligned to reduce the PE_XBAR size? Or am I missing something?

@FrancescoConti
Copy link
Member

FrancescoConti commented Jan 25, 2021

Changing the peripheral slave IDs means changing the peripheral address map; so far we have tried to keep compatibility even when some peripheral slave are actually disconnected. E.g., in many PULP instantiations there are no HWPEs, but we keep the related memory space reserved.
Moreover, since the missing slaves are grounded, I do not think you will see any particularly meaningful gain in removing the slaves -- the synthesis tool should be smart enough to reduce the xbar's size on its own.
I suggest keeping this as it is.

EDIT: forgot to mention a couple of points. For what concerns the decompressor, it is an IP that is used in some PULPs and might be added in the future. For what concerns periph ID 3, I am not sure why the hole is there, I do not think there is any peripheral using this address space; we should consider it "free" for new possible peripherals.

@micprog
Copy link
Member Author

micprog commented Jan 25, 2021

Thank you for the info, keeping compatibility makes sense. If I'm not mistaken, the periph ID=3 is not grounded, it is never referenced anywhere, maybe adding a parameter SPER_UNUSED_ID = 3 and grounding it would make sense, if you want I can add this.

@FrancescoConti
Copy link
Member

I think it's a good idea if it is not connected elsewhere.

@micprog
Copy link
Member Author

micprog commented Jan 25, 2021

It seems the ID=3 is used for the Event unit, my bad for not seeing this, but it is not that transparent. I will add a comment to the pulp_cluster_package.sv to clarify this.

// combine number of required slave ports for event unit
generate
for (genvar I = 0; I < NB_SPERIPH_PLUGS_EU; I++ ) begin
assign speriph_slave[SPER_EVENT_U_ID+I].gnt = speriph_slave_eu_comb.gnt;
assign speriph_slave[SPER_EVENT_U_ID+I].r_valid = speriph_slave_eu_comb.r_valid;
assign speriph_slave[SPER_EVENT_U_ID+I].r_opc = speriph_slave_eu_comb.r_opc;
assign speriph_slave[SPER_EVENT_U_ID+I].r_id = speriph_slave_eu_comb.r_id;
assign speriph_slave[SPER_EVENT_U_ID+I].r_rdata = speriph_slave_eu_comb.r_rdata;
assign eu_speriph_plug_req[I] = speriph_slave[SPER_EVENT_U_ID+I].req;
assign eu_speriph_plug_add[I] = speriph_slave[SPER_EVENT_U_ID+I].add;
assign eu_speriph_plug_wen[I] = speriph_slave[SPER_EVENT_U_ID+I].wen;
assign eu_speriph_plug_wdata[I] = speriph_slave[SPER_EVENT_U_ID+I].wdata;
assign eu_speriph_plug_be[I] = speriph_slave[SPER_EVENT_U_ID+I].be;
assign eu_speriph_plug_id[I] = speriph_slave[SPER_EVENT_U_ID+I].id;
end
endgenerate
assign speriph_slave_eu_comb.req = |eu_speriph_plug_req;
assign speriph_slave_eu_comb.add = (eu_speriph_plug_req == 2'b10) ? eu_speriph_plug_add[1] : eu_speriph_plug_add[0];
assign speriph_slave_eu_comb.wen = (eu_speriph_plug_req == 2'b10) ? eu_speriph_plug_wen[1] : eu_speriph_plug_wen[0];
assign speriph_slave_eu_comb.wdata = (eu_speriph_plug_req == 2'b10) ? eu_speriph_plug_wdata[1] : eu_speriph_plug_wdata[0];
assign speriph_slave_eu_comb.be = (eu_speriph_plug_req == 2'b10) ? eu_speriph_plug_be[1] : eu_speriph_plug_be[0];
assign speriph_slave_eu_comb.id = (eu_speriph_plug_req == 2'b10) ? eu_speriph_plug_id[1] : eu_speriph_plug_id[0];

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants