Skip to content

Commit

Permalink
[uart,rtl,dv] Switch watermark interrupts to status type
Browse files Browse the repository at this point in the history 
Fixes #16693

Signed-off-by: Greg Chadwick <[email protected]>
  • Loading branch information
@GregAC
GregAC committed Feb 22, 2024
1 parent 7e5b056 commit 2b03607
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 73 deletions.
4 changes: 3 additions & 1 deletion hw/ip/uart/data/uart.hjson
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,11 @@
{ name: "tx", desc: "Serial transmit bit" }
],
interrupt_list: [
{ name: "tx_watermark"
{ name: "tx_watermark",
type: "status",
desc: "raised if the transmit FIFO is past the high-water mark."}
{ name: "rx_watermark"
type: "status",
desc: "raised if the receive FIFO is past the high-water mark."}
{ name: "tx_empty"
desc: "raised if the transmit FIFO has emptied and no transmit is ongoing."}
Expand Down
57 changes: 37 additions & 20 deletions hw/ip/uart/dv/env/seq_lib/uart_intr_vseq.sv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,21 +55,25 @@ class uart_intr_vseq extends uart_base_vseq;
int level = ral.fifo_ctrl.txilvl.get_mirrored_value();
int watermark_bytes = get_watermark_bytes_by_level(level);
if (!en_tx) return;
// when tx is enabled, one extra item is in the data path
// when watermark_bytes==1, watermark interrupt is triggered before item is processed
if (en_tx && watermark_bytes > 1) watermark_bytes += 1;
// First byte is immediately popped from TX FIFO (for transmission) and watermark based upon
// TX FIFO level excluding in-tranmission byte. Add 1 to watermark_bytes here to give the
// number of bytes required to move over the watermark threshold.
watermark_bytes += 1;
drive_tx_bytes(.num_bytes(watermark_bytes - 1));
check_one_intr(.uart_intr(uart_intr), .exp(0));
check_one_intr(.uart_intr(uart_intr), .exp(1));
drive_tx_bytes(.num_bytes(1));
check_one_intr(.uart_intr(uart_intr), .exp(0));
// wait until it drops below watermark
csr_spinwait(.ptr(ral.fifo_status.txlvl),
.exp_data(get_watermark_bytes_by_level(level)),
.compare_op(CompareOpLt));
check_one_intr(.uart_intr(uart_intr), .exp(1));
cfg.m_uart_agent_cfg.vif.wait_for_tx_idle();
// check interrupt is non-sticky
// interrupt should remain asserted whilst FIFO level is below watermark, writes to
// intr_state to clear have no effect
csr_wr(.ptr(ral.intr_state), .value(1 << uart_intr));
drive_tx_bytes(.num_bytes(watermark_bytes - 1));
check_one_intr(.uart_intr(uart_intr), .exp(1));
drive_tx_bytes(.num_bytes(watermark_bytes + 1));
check_one_intr(.uart_intr(uart_intr), .exp(0));
cfg.m_uart_agent_cfg.vif.wait_for_tx_idle();
end
Expand All @@ -81,10 +85,11 @@ class uart_intr_vseq extends uart_base_vseq;
check_one_intr(.uart_intr(uart_intr), .exp(0));
drive_rx_bytes(.num_bytes(1));
check_one_intr(.uart_intr(uart_intr), .exp(en_rx));
// check interrupt is non-sticky
// interrupt should remain asserted whilst FIFO level is above watermark, writes to
// intr_state to clear have no effect
csr_wr(.ptr(ral.intr_state), .value(1 << uart_intr));
drive_rx_bytes(.num_bytes(1));
check_one_intr(.uart_intr(uart_intr), .exp(0));
check_one_intr(.uart_intr(uart_intr), .exp(en_rx));
end

TxEmpty: begin
Expand Down Expand Up @@ -123,6 +128,7 @@ class uart_intr_vseq extends uart_base_vseq;
// break at RXBLVL char-times
RxBreakErr: begin
bit [NumUartIntr-1:0] exp_intr_state;
bit [NumUartIntr-1:0] exp_intr_state_mask = '1;
int level = ral.ctrl.rxblvl.get_mirrored_value();
int break_bytes = get_break_bytes_by_level(level);

Expand All @@ -132,6 +138,11 @@ class uart_intr_vseq extends uart_base_vseq;
clear_fifos(.clear_tx_fifo(0), .clear_rx_fifo(1));
csr_wr(.ptr(ral.intr_state), .value('hff));

// Don't attempt to predict Tx/Rx watermark when testing RxBreakErr so we don't need to
// predict TX/RX FIFO levels for this test.
exp_intr_state_mask[TxWatermark] = 1'b0;
exp_intr_state_mask[RxWatermark] = 1'b0;

fork
begin
drive_rx_all_0s();
Expand All @@ -140,16 +151,18 @@ class uart_intr_vseq extends uart_base_vseq;
// < 10 cycles 0s, expect no interrupt
wait_for_baud_clock_cycles(9);
// check interrupt reg & pin but not affect timing of driving uart RX
nonblocking_check_all_intr(.exp(0), .do_clear(0));
nonblocking_check_all_intr(.exp(0), .do_clear(0), .exp_mask(exp_intr_state_mask));
// 10th cycle
wait_for_baud_clock_cycles(1);
exp_intr_state[RxFrameErr] = ~en_parity & en_rx;
nonblocking_check_all_intr(.exp(exp_intr_state), .do_clear(0));
nonblocking_check_all_intr(.exp(exp_intr_state), .do_clear(0),
.exp_mask(exp_intr_state_mask));
// 11th cycle
wait_for_baud_clock_cycles(1);
exp_intr_state[RxParityErr] = en_parity & en_rx & `GET_PARITY(0, odd_parity);
exp_intr_state[RxFrameErr] = en_rx;
nonblocking_check_all_intr(.exp(exp_intr_state), .do_clear(1));
nonblocking_check_all_intr(.exp(exp_intr_state), .do_clear(1),
.exp_mask(exp_intr_state_mask));
end
join

Expand All @@ -161,18 +174,20 @@ class uart_intr_vseq extends uart_base_vseq;
// from 11 to RXBLVL * char - 1
if (break_bytes > 2) begin // avoid negetive value
wait_for_baud_clock_cycles(bit_num_per_trans * (break_bytes - 1) - 11);
nonblocking_check_all_intr(.exp(exp_intr_state), .do_clear(1));
nonblocking_check_all_intr(.exp(exp_intr_state), .do_clear(1),
.exp_mask(exp_intr_state_mask));
end
// RXBLVL * char
wait_for_baud_clock_cycles(bit_num_per_trans);
exp_intr_state[RxBreakErr] = en_rx;
nonblocking_check_all_intr(.exp(exp_intr_state), .do_clear(1));
nonblocking_check_all_intr(.exp(exp_intr_state), .do_clear(1),
.exp_mask(exp_intr_state_mask));

// RXBLVL * char * 2
wait_for_baud_clock_cycles(bit_num_per_trans * break_bytes);
// check break intr doesn't occur again
exp_intr_state[RxBreakErr] = 0;
nonblocking_check_all_intr(.exp(exp_intr_state));
nonblocking_check_all_intr(.exp(exp_intr_state), .exp_mask(exp_intr_state_mask));

sync_up_rx_from_frame_err(bit_num_per_trans);
cfg.disable_scb_rx_parity_check = 0;
Expand Down Expand Up @@ -256,24 +271,26 @@ class uart_intr_vseq extends uart_base_vseq;
endtask : check_one_intr

// check all interrupt state and pin
task check_all_intr(bit [NumUartIntr-1:0] exp, bit do_clear = 0);
task check_all_intr(bit [NumUartIntr-1:0] exp, bit do_clear = 0,
bit [NumUartIntr-1:0] exp_mask = '1);
bit [NumUartIntr-1:0] act_intr_state;
bit [NumUartIntr-1:0] exp_pin;

csr_rd(.ptr(ral.intr_state), .value(act_intr_state));
if (!cfg.under_reset) `DV_CHECK_EQ(act_intr_state, exp)
if (!cfg.under_reset) `DV_CHECK_EQ(act_intr_state & exp_mask, exp)
exp_pin = exp & en_intr;
if (!cfg.under_reset) `DV_CHECK_EQ(cfg.intr_vif.pins[NumUartIntr-1:0], exp_pin, $sformatf(
"uart_intr val: %0h, en_intr: %0h", exp, en_intr))
if (!cfg.under_reset) `DV_CHECK_EQ(cfg.intr_vif.pins[NumUartIntr-1:0] & exp_mask, exp_pin,
$sformatf("uart_intr val: %0h, en_intr: %0h", exp, en_intr))

if (do_clear) begin
csr_wr(.ptr(ral.intr_state), .value(exp));
end
endtask : check_all_intr

task nonblocking_check_all_intr(bit [NumUartIntr-1:0] exp, bit do_clear = 0);
task nonblocking_check_all_intr(bit [NumUartIntr-1:0] exp, bit do_clear = 0,
bit [NumUartIntr-1:0] exp_mask = '1);
fork
check_all_intr(exp, do_clear);
check_all_intr(exp, do_clear, exp_mask);
join_none
endtask : nonblocking_check_all_intr

Expand Down
55 changes: 31 additions & 24 deletions hw/ip/uart/dv/env/uart_scoreboard.sv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,12 @@ class uart_scoreboard extends cip_base_scoreboard #(.CFG_T(uart_env_cfg),
local bit tx_full_exp, rx_full_exp, tx_empty_exp, rx_empty_exp, tx_idle_exp, rx_idle_exp;
local int txlvl_exp, rxlvl_exp;
local bit [NumUartIntr-1:0] intr_exp;
local bit [NumUartIntr-1:0] status_intr_test;
local bit [7:0] rdata_exp;
// store tx/rx_q at TL address phase
local int tx_q_size_at_addr_phase, rx_q_size_at_addr_phase;
local bit [NumUartIntr-1:0] intr_exp_at_addr_phase;

// non sticky interrupts are edge-triggered
// set it when interrupt is triggered, clear it when interrupt condition is no longer true
local bit tx_watermark_triggered = 1;
local bit rx_watermark_triggered = 0;

// TLM fifos to pick up the packets
uvm_tlm_analysis_fifo #(uart_item) uart_tx_fifo;
uvm_tlm_analysis_fifo #(uart_item) uart_rx_fifo;
Expand Down Expand Up @@ -101,7 +97,9 @@ class uart_scoreboard extends cip_base_scoreboard #(.CFG_T(uart_env_cfg),
// after an item is sent, check to see if size dipped below watermark
predict_tx_watermark_intr();

`uvm_info(`gfn, $sformatf("Processing FIFO %d %d", tx_q.size(), tx_processing_item_q.size()), UVM_LOW)

Check warning on line 100 in hw/ip/uart/dv/env/uart_scoreboard.sv

View workflow job for this annotation

GitHub Actions / verible-verilog-lint

[verible-verilog-lint] hw/ip/uart/dv/env/uart_scoreboard.sv#L100 

Line length exceeds max: 100; is: 108 [Style: line-length] [line-length]
Raw output 
message:"Line length exceeds max: 100; is: 108 [Style: line-length] [line-length]"  location:{path:"./hw/ip/uart/dv/env/uart_scoreboard.sv"  range:{start:{line:100  column:101}}}  severity:WARNING  source:{name:"verible-verilog-lint"  url:"https://github.com/chipsalliance/verible"}
if (tx_q.size() == 0 && tx_processing_item_q.size() == 0) begin
`uvm_info(`gfn, "Setting intr_exp[TxEmpty]", UVM_LOW)
intr_exp[TxEmpty] = 1;
end
end
Expand Down Expand Up @@ -142,29 +140,14 @@ class uart_scoreboard extends cip_base_scoreboard #(.CFG_T(uart_env_cfg),
end // forever
endtask

// when interrupt is non-sticky, interrupt will be triggered once unless it exits interrupt
// condition
virtual function bit get_non_sticky_interrupt(bit cur_intr, bit new_intr, ref bit triggered);
bit final_intr = cur_intr || (new_intr & ~triggered);
if (!new_intr) triggered = 0;
else if (final_intr) triggered = 1;

return final_intr;
endfunction

virtual function void predict_tx_watermark_intr(uint tx_q_size = tx_q.size);
uint watermark = get_watermark_bytes_by_level(ral.fifo_ctrl.txilvl.get_mirrored_value());
intr_exp[TxWatermark] = get_non_sticky_interrupt(.cur_intr(intr_exp[TxWatermark]),
.new_intr(tx_q_size < watermark),
.triggered(tx_watermark_triggered));
intr_exp[TxWatermark] = (tx_q_size < watermark) || status_intr_test[TxWatermark];
endfunction

virtual function void predict_rx_watermark_intr(uint rx_q_size = rx_q.size);
uint watermark = get_watermark_bytes_by_level(ral.fifo_ctrl.rxilvl.get_mirrored_value());
intr_exp[RxWatermark] = get_non_sticky_interrupt(
.cur_intr(intr_exp[RxWatermark]),
.new_intr(rx_q_size >= watermark && rx_enabled),
.triggered(rx_watermark_triggered));
intr_exp[RxWatermark] = (rx_q_size >= watermark) || status_intr_test[RxWatermark];
endfunction

// we don't model uart cycle-acurrately, ignore checking when item is just/almost finished
Expand Down Expand Up @@ -325,7 +308,21 @@ class uart_scoreboard extends cip_base_scoreboard #(.CFG_T(uart_env_cfg),
"intr_test": begin
if (write && channel == AddrChannel) begin
bit [TL_DW-1:0] intr_en = ral.intr_enable.get_mirrored_value();
// TxWatermark and RxWatermark are status type interrupts. When `intr_test` is set for
// these interrupts it acts as if the status for the interrupt is true. In turn when
// `intr_test` is cleared the interrupt will drop, but only if the underlying status isn't
// true. So record what's been written to `intr_test` for status type interrupts here and
// then call the prediction functions for those interrupts to determine the interrupt
// status.
status_intr_test[TxWatermark] = item.a_data[TxWatermark];
status_intr_test[RxWatermark] = item.a_data[RxWatermark];

intr_exp |= item.a_data;

predict_tx_watermark_intr();
predict_rx_watermark_intr();

`uvm_info(`gfn, $sformatf("intr_test write, intr_exp now %x", intr_exp), UVM_LOW)
if (cfg.en_cov) begin
foreach (intr_exp[i]) begin
cov.intr_test_cg.sample(i, item.a_data[i], intr_en[i], intr_exp[i]);
Expand Down Expand Up @@ -409,7 +406,12 @@ class uart_scoreboard extends cip_base_scoreboard #(.CFG_T(uart_env_cfg),
// add 1 cycle delay to avoid race condition when fifo changing and interrupt clearing
// occur simultaneously
cfg.clk_rst_vif.wait_clks(1);
// Watermark interrupt state based upon current level of TX/RX fifos. They cannot be
// cleared by writes to intr_state.
intr_wdata[TxWatermark] = 1'b0;
intr_wdata[RxWatermark] = 1'b0;
intr_exp &= ~intr_wdata;
`uvm_info(`gfn, $sformatf("intr_state write, intr_exp now %x", intr_exp), UVM_LOW)
end join_none
end else if (!write && channel == AddrChannel) begin // read & addr phase
intr_exp_at_addr_phase = intr_exp;
Expand Down Expand Up @@ -570,8 +572,6 @@ class uart_scoreboard extends cip_base_scoreboard #(.CFG_T(uart_env_cfg),
uart_rx_clk_pulses = 0;
tx_q_size_at_addr_phase = 0;
rx_q_size_at_addr_phase = 0;
tx_watermark_triggered = 1;
rx_watermark_triggered = 0;
tx_enabled = ral.ctrl.tx.get_reset();
rx_enabled = ral.ctrl.rx.get_reset();
tx_full_exp = ral.status.txfull.get_reset();
Expand All @@ -583,7 +583,14 @@ class uart_scoreboard extends cip_base_scoreboard #(.CFG_T(uart_env_cfg),
txlvl_exp = ral.fifo_status.txlvl.get_reset();
rxlvl_exp = ral.fifo_status.rxlvl.get_reset();
intr_exp = ral.intr_state.get_reset();
status_intr_test = '0;
rdata_exp = ral.rdata.get_reset();


// Predict watermark interrupts now as they're status types. This ensures the expected
// interrupts reflect the UART state on reset.
predict_tx_watermark_intr();
predict_rx_watermark_intr();
endfunction

function void check_phase(uvm_phase phase);
Expand Down
46 changes: 18 additions & 28 deletions hw/ip/uart/rtl/uart_core.sv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ module uart_core (
logic allzero_err, not_allzero_char;
logic event_tx_watermark, event_rx_watermark, event_tx_empty, event_rx_overflow;
logic event_rx_frame_err, event_rx_break_err, event_rx_timeout, event_rx_parity_err;
logic tx_watermark_d, tx_watermark_prev_q;
logic rx_watermark_d, rx_watermark_prev_q;
logic tx_uart_idle_q;

assign tx_enable = reg2hw.ctrl.tx.q;
Expand Down Expand Up @@ -309,18 +307,16 @@ module uart_core (

always_comb begin
unique case(uart_fifo_txilvl)
3'h0: tx_watermark_d = (tx_fifo_depth < 8'd1);
3'h1: tx_watermark_d = (tx_fifo_depth < 8'd2);
3'h2: tx_watermark_d = (tx_fifo_depth < 8'd4);
3'h3: tx_watermark_d = (tx_fifo_depth < 8'd8);
3'h4: tx_watermark_d = (tx_fifo_depth < 8'd16);
3'h5: tx_watermark_d = (tx_fifo_depth < 8'd32);
default: tx_watermark_d = (tx_fifo_depth < 8'd64);
3'h0: event_tx_watermark = (tx_fifo_depth < 8'd1);
3'h1: event_tx_watermark = (tx_fifo_depth < 8'd2);
3'h2: event_tx_watermark = (tx_fifo_depth < 8'd4);
3'h3: event_tx_watermark = (tx_fifo_depth < 8'd8);
3'h4: event_tx_watermark = (tx_fifo_depth < 8'd16);
3'h5: event_tx_watermark = (tx_fifo_depth < 8'd32);
default: event_tx_watermark = (tx_fifo_depth < 8'd64);
endcase
end

assign event_tx_watermark = tx_watermark_d & ~tx_watermark_prev_q;

// The empty condition handling is a bit different.
// If empty rising conditions were detected directly, then every first write of a burst
// would trigger an empty. This is due to the fact that the uart_tx fsm immediately
Expand All @@ -335,32 +331,26 @@ module uart_core (

always_ff @(posedge clk_i or negedge rst_ni) begin
if (!rst_ni) begin
tx_watermark_prev_q <= 1'b1; // by default watermark condition is true
rx_watermark_prev_q <= 1'b0; // by default watermark condition is false
tx_uart_idle_q <= 1'b1;
end else begin
tx_watermark_prev_q <= tx_watermark_d;
rx_watermark_prev_q <= rx_watermark_d;
tx_uart_idle_q <= tx_uart_idle;
end
end

always_comb begin
unique case(uart_fifo_rxilvl)
3'h0: rx_watermark_d = (rx_fifo_depth >= 8'd1);
3'h1: rx_watermark_d = (rx_fifo_depth >= 8'd2);
3'h2: rx_watermark_d = (rx_fifo_depth >= 8'd4);
3'h3: rx_watermark_d = (rx_fifo_depth >= 8'd8);
3'h4: rx_watermark_d = (rx_fifo_depth >= 8'd16);
3'h5: rx_watermark_d = (rx_fifo_depth >= 8'd32);
3'h6: rx_watermark_d = (rx_fifo_depth >= 8'd64);
3'h7: rx_watermark_d = (rx_fifo_depth >= 8'd126);
default: rx_watermark_d = 1'b0;
3'h0: event_rx_watermark = (rx_fifo_depth >= 8'd1);
3'h1: event_rx_watermark = (rx_fifo_depth >= 8'd2);
3'h2: event_rx_watermark = (rx_fifo_depth >= 8'd4);
3'h3: event_rx_watermark = (rx_fifo_depth >= 8'd8);
3'h4: event_rx_watermark = (rx_fifo_depth >= 8'd16);
3'h5: event_rx_watermark = (rx_fifo_depth >= 8'd32);
3'h6: event_rx_watermark = (rx_fifo_depth >= 8'd64);
3'h7: event_rx_watermark = (rx_fifo_depth >= 8'd126);
default: event_rx_watermark = 1'b0;
endcase
end

assign event_rx_watermark = rx_watermark_d & ~rx_watermark_prev_q;

// rx timeout interrupt
assign uart_rxto_en = reg2hw.timeout_ctrl.en.q;
assign uart_rxto_val = reg2hw.timeout_ctrl.val.q;
Expand Down Expand Up @@ -402,7 +392,7 @@ module uart_core (

// instantiate interrupt hardware primitives

prim_intr_hw #(.Width(1)) intr_hw_tx_watermark (
prim_intr_hw #(.Width(1), .IntrT("Status")) intr_hw_tx_watermark (
.clk_i,
.rst_ni,
.event_intr_i (event_tx_watermark),
Expand All @@ -415,7 +405,7 @@ module uart_core (
.intr_o (intr_tx_watermark_o)
);

prim_intr_hw #(.Width(1)) intr_hw_rx_watermark (
prim_intr_hw #(.Width(1), .IntrT("Status")) intr_hw_rx_watermark (
.clk_i,
.rst_ni,
.event_intr_i (event_rx_watermark),
Expand Down

0 comments on commit 2b03607

Please sign in to comment.