Skip to content

Conversation

@disdi
Copy link
Contributor

@disdi disdi commented Jan 11, 2025

Add support for PLIC interrupt controller as per RISC-V PLIC specs.

Copy link
Contributor

@marnovandermaas marnovandermaas left a comment

Choose a reason for hiding this comment

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

Thank you so much for your contribution. Just a few comments based on a quick review. How have you tested this? What are the area costs of adding the PLIC to the system?

parameter logic [31:0] SIM_CTRL_MASK = ~(SIM_CTRL_SIZE-1);

localparam logic [31:0] PLIC_SIZE = 4 * 1024; // 4 KiB
localparam logic [31:0] PLIC_START = 32'h80005000;
Copy link
Contributor

Choose a reason for hiding this comment

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

It's probably best to keep the 8000X000 addresses for those that are only 1 KiB in size. Maybe create it at 0x8010000?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I corrected this in latest commit.

.irq_external_i(1'b0),
.irq_fast_i ({14'b0, uart_irq}),
.irq_external_i(irq_external),
.irq_fast_i (1'b0),
Copy link
Contributor

Choose a reason for hiding this comment

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

This should still be 15 bits right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected in the latest commit.

assign ndmreset_req = 1'b0;
end

assign irq_sources = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is irq_sources defined somewhere? If so what is its width?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I declared irq_sources now.


assign rdata_o = reg_rdata;

endmodule No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: fix end line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected.

parameter logic [31:0] SIM_CTRL_START = 32'h20000;
parameter logic [31:0] SIM_CTRL_MASK = ~(SIM_CTRL_SIZE-1);

localparam logic [31:0] PLIC_SIZE = 4 * 1024; // 4 KiB
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this enough? I thought this needed to be 64 MiB?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected.

@disdi
Copy link
Contributor Author

disdi commented Jan 18, 2025

How have you tested this?

I am still not able to get UART interrupt working with PLIC. I think my sw driver needs some change.

What are the area costs of adding the PLIC to the system?

LUTs increased from 6686 to 6790, FFs increased from 6279 to 6411.

@marnovandermaas
Copy link
Contributor

I'm converting this PR to draft until you have a successful UART interrupt firing.

@marnovandermaas marnovandermaas marked this pull request as draft February 5, 2025 12:36
@disdi disdi force-pushed the plic_inc_support branch from ea9ab44 to 4ad06a4 Compare April 5, 2025 06:39
@disdi disdi force-pushed the plic_inc_support branch from 4ad06a4 to 02dfe2f Compare April 5, 2025 06:41
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

Successfully merging this pull request may close these issues.

2 participants