[dv/clkmgr] Update docs and minor refactor
Add coverpoints in the testplan.
Remove extclk_sel_regwen from clkmgr_if.
Fix the type of extclk_sel in clkmgr_if.
Replace clkmgr_vif.wait_clks by clk_rst_vif.wait_clks.
Simplify the constraint for lc_tx_t variable.
Start monitor_ip_clk_en task in clkmgr_scoreboard.
Signed-off-by: Guillermo Maturana <maturana@google.com>
diff --git a/hw/ip/clkmgr/data/clkmgr_testplan.hjson b/hw/ip/clkmgr/data/clkmgr_testplan.hjson
index 479f465..c56a4a9 100644
--- a/hw/ip/clkmgr/data/clkmgr_testplan.hjson
+++ b/hw/ip/clkmgr/data/clkmgr_testplan.hjson
@@ -111,9 +111,33 @@
tests: []
}
]
- // The covergroups are arrays, which are yet to be supported by the tools.
covergroups: [
- // peri_cg_wrap cover the dependencies of each peripheral clock.
- // trans_cg_wrap cover the dependencies of each transactional unit clock.
+ {
+ name: peri_cg
+ desc: '''
+ Collects coverage for each peripheral clock.
+
+ The peripheral clocks depend on a bit in the clk_enables CSR,
+ the ip_clk_en input from pwrmgr, and the scanmode input.
+ This collects the cross of them for each peripheral.
+
+ FIXME This is collected in an array, one instance for each clock,
+ but the dvsim coverage flow doesn't yet support arrays.
+ '''
+ }
+ {
+ name: trans_cg
+ desc: '''
+ Collects coverage for each transactional unit clock.
+
+ The transactional unit clocks depend on a bit in the clk_hints CSR,
+ the ip_clk_en input from pwrmgr, the respective idle input bit from
+ the unit, and the scanmode input.
+ This collects the cross of them for each transactional unit.
+
+ FIXME This is collected in an array, one instance for each clock,
+ but the dvsim coverage flow doesn't yet support arrays.
+ '''
+ }
]
}
diff --git a/hw/ip/clkmgr/dv/env/clkmgr_env_pkg.sv b/hw/ip/clkmgr/dv/env/clkmgr_env_pkg.sv
index 7b6da06..6f0e788 100644
--- a/hw/ip/clkmgr/dv/env/clkmgr_env_pkg.sv
+++ b/hw/ip/clkmgr/dv/env/clkmgr_env_pkg.sv
@@ -14,6 +14,10 @@
import csr_utils_pkg::*;
import clkmgr_ral_pkg::*;
+ import lc_ctrl_pkg::lc_tx_t;
+ import lc_ctrl_pkg::On;
+ import lc_ctrl_pkg::Off;
+
// macro includes
`include "uvm_macros.svh"
`include "dv_macros.svh"
diff --git a/hw/ip/clkmgr/dv/env/clkmgr_if.sv b/hw/ip/clkmgr/dv/env/clkmgr_if.sv
index 67af1a2..1a4d88f 100644
--- a/hw/ip/clkmgr/dv/env/clkmgr_if.sv
+++ b/hw/ip/clkmgr/dv/env/clkmgr_if.sv
@@ -52,31 +52,22 @@
} clk_hints_t;
// The CSR values from the testbench side.
- clk_enables_t clk_enables;
- clk_hints_t clk_hints;
- clk_hints_t clk_hints_status;
- logic extclk_sel_regwen;
- logic extclk_sel;
- logic jitter_enable;
+ clk_enables_t clk_enables;
+ clk_hints_t clk_hints;
+ clk_hints_t clk_hints_status;
+ lc_ctrl_pkg::lc_tx_t extclk_sel;
+ logic jitter_enable;
- task automatic wait_clks(int cycles);
- repeat (cycles) @(posedge clk);
- endtask
-
- function automatic void update_extclk_sel_regwen(logic sel_regwen);
- extclk_sel_regwen = sel_regwen;
+ function automatic void update_extclk_sel(lc_ctrl_pkg::lc_tx_t value);
+ extclk_sel = value;
endfunction
- function automatic void update_extclk_sel(logic sel);
- extclk_sel = sel;
+ function automatic void update_clk_enables(clk_enables_t value);
+ clk_enables = value;
endfunction
- function automatic void update_clk_enables(clk_enables_t ens);
- clk_enables = ens;
- endfunction
-
- function automatic void update_clk_hints(clk_hints_t hints);
- clk_hints = hints;
+ function automatic void update_clk_hints(clk_hints_t value);
+ clk_hints = value;
endfunction
function automatic void update_idle(bit [NUM_TRANS-1:0] value);
@@ -87,8 +78,8 @@
pwr_i.ip_clk_en = value;
endfunction
- function automatic void update_scanmode(lc_ctrl_pkg::lc_tx_t scm);
- scanmode_i = scm;
+ function automatic void update_scanmode(lc_ctrl_pkg::lc_tx_t value);
+ scanmode_i = value;
endfunction
function automatic logic get_clk_status();
diff --git a/hw/ip/clkmgr/dv/env/clkmgr_scoreboard.sv b/hw/ip/clkmgr/dv/env/clkmgr_scoreboard.sv
index 9a7c5f9..b41ea1b 100644
--- a/hw/ip/clkmgr/dv/env/clkmgr_scoreboard.sv
+++ b/hw/ip/clkmgr/dv/env/clkmgr_scoreboard.sv
@@ -29,6 +29,7 @@
super.run_phase(phase);
fork
monitor_idle();
+ monitor_ip_clk_en();
monitor_scanmode();
begin : post_reset
fork
@@ -179,6 +180,8 @@
string access_str = write ? "write" : "read";
string channel_str = channel == AddrChannel ? "address" : "data";
+ logic extclk_sel_regwen = ral.extclk_sel_regwen.get_reset();
+
// if access was to a valid csr, get the csr handle
if (csr_addr inside {cfg.csr_addrs[ral_name]}) begin
csr = ral.default_map.get_reg_by_offset(csr_addr);
@@ -211,10 +214,10 @@
end
"extclk_sel_regwen":
if (addr_phase_write) begin
- cfg.clkmgr_vif.update_extclk_sel_regwen(item.a_data);
+ extclk_sel_regwen = item.a_data;
end
"extclk_sel":
- if (addr_phase_write) begin
+ if (addr_phase_write && extclk_sel_regwen) begin
cfg.clkmgr_vif.update_extclk_sel(item.a_data);
end
"jitter": begin
diff --git a/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_base_vseq.sv b/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_base_vseq.sv
index 56867ee..179ad75 100644
--- a/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_base_vseq.sv
+++ b/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_base_vseq.sv
@@ -10,18 +10,30 @@
);
`uvm_object_utils(clkmgr_base_vseq)
+ typedef enum {LcTxTSelOn, LcTxTSelOff, LcTxTSelOther} lc_tx_t_sel_e;
+
+ // This simplifies the constraint blocks.
+ function lc_tx_t get_lc_tx_t_from_sel(lc_tx_t_sel_e sel, lc_tx_t other);
+ case (sel)
+ LcTxTSelOn: return On;
+ LcTxTSelOff: return Off;
+ LcTxTSelOther: return other;
+ endcase
+ endfunction
+
rand bit ip_clk_en;
rand bit [NUM_TRANS-1:0] idle;
- // This selects scanmode according to scanmode_sel, which is randomized with weights.
- rand bit [$bits(lc_ctrl_pkg::lc_tx_t)-1:0] scanmode;
- typedef enum {SC_ON, SC_OFF, SC_OTHER} scanmode_sel_e;
- rand scanmode_sel_e scanmode_sel;
- constraint scanmode_values {
- (scanmode_sel == SC_ON) -> scanmode == lc_ctrl_pkg::On;
- (scanmode_sel == SC_OFF) -> scanmode == lc_ctrl_pkg::Off;
- (scanmode_sel == SC_OTHER) -> !(scanmode inside {lc_ctrl_pkg::On, lc_ctrl_pkg::Off});
- scanmode_sel dist {SC_ON := 4, SC_OFF := 2, SC_OTHER := 2};
+ // This selects scanmode according to sel_scanmode, which is randomized with weights.
+ rand lc_tx_t scanmode;
+ rand lc_tx_t scanmode_other;
+ rand lc_tx_t_sel_e sel_scanmode;
+ int scanmode_on_weight = 8;
+
+ constraint scanmode_c {
+ sel_scanmode dist {LcTxTSelOn := scanmode_on_weight, LcTxTSelOff := 4, LcTxTSelOther := 4};
+ !(scanmode_other inside {On, Off});
+ scanmode == get_lc_tx_t_from_sel(sel_scanmode, scanmode_other);
}
// various knobs to enable certain routines
@@ -96,7 +108,6 @@
function void update_csrs_with_reset_values();
cfg.clkmgr_vif.update_clk_enables(ral.clk_enables.get_reset());
cfg.clkmgr_vif.update_clk_hints(ral.clk_hints.get_reset());
- cfg.clkmgr_vif.update_extclk_sel_regwen(ral.extclk_sel_regwen.get_reset());
cfg.clkmgr_vif.update_extclk_sel(ral.extclk_sel.get_reset());
endfunction
diff --git a/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_peri_vseq.sv b/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_peri_vseq.sv
index 1cb0f8a..7347f16 100644
--- a/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_peri_vseq.sv
+++ b/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_peri_vseq.sv
@@ -18,7 +18,7 @@
for (int i = 0; i < num_trans; ++i) begin
logic [NUM_PERI-1:0] flipped_enables;
`DV_CHECK_RANDOMIZE_FATAL(this)
- cfg.clkmgr_vif.init(.idle(idle), .ip_clk_en(ip_clk_en), .scanmode(scanmode));
+ cfg.clkmgr_vif.init(.idle(idle), .ip_clk_en(ip_clk_en), .scanmode(scanmode));
csr_wr(.ptr(ral.clk_enables), .value(initial_enables));
diff --git a/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_smoke_vseq.sv b/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_smoke_vseq.sv
index 1ff77c2..a3ad7ee 100644
--- a/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_smoke_vseq.sv
+++ b/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_smoke_vseq.sv
@@ -10,7 +10,7 @@
constraint enable_ip_clk_en { ip_clk_en == 1'b1; }
constraint all_busy { idle == '0; }
- constraint scanmode_off { scanmode_sel == SC_OFF; }
+ constraint scanmode_off { sel_scanmode == LcTxTSelOff; }
task body();
update_csrs_with_reset_values();
@@ -63,7 +63,7 @@
$sformatf("%s hint value cannot drop while busy", descriptor.unit.name()))
`uvm_info(`gfn, $sformatf("Setting %s idle bit", descriptor.unit.name), UVM_MEDIUM)
- cfg.clkmgr_vif.wait_clks(1);
+ cfg.clk_rst_vif.wait_clks(1);
idle[trans] = 1'b1;
cfg.clkmgr_vif.update_idle(idle);
// Some cycles for the logic to settle.
diff --git a/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_trans_vseq.sv b/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_trans_vseq.sv
index fa4c72d..e8e3bcb 100644
--- a/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_trans_vseq.sv
+++ b/hw/ip/clkmgr/dv/env/seq_lib/clkmgr_trans_vseq.sv
@@ -27,20 +27,20 @@
`uvm_info(`gfn, $sformatf("Updating hints to 0x%0x", initial_hints), UVM_MEDIUM)
csr_wr(.ptr(ral.clk_hints), .value(initial_hints));
- cfg.clkmgr_vif.wait_clks(5);
+ cfg.clk_rst_vif.wait_clks(5);
// We expect the status to be determined by hints and idle.
csr_rd(.ptr(ral.clk_hints_status), .value(value));
`DV_CHECK_EQ(value, initial_hints | ~idle, "Busy units have status high")
// Clearing idle should make hint_status match hints.
cfg.clkmgr_vif.update_idle('1);
- cfg.clkmgr_vif.wait_clks(2);
+ cfg.clk_rst_vif.wait_clks(2);
csr_rd(.ptr(ral.clk_hints_status), .value(value));
`DV_CHECK_EQ(value, initial_hints, "All idle: units status matches hints")
// Now enable them all, and the status should also be all ones.
csr_wr(.ptr(ral.clk_hints), .value('1));
- cfg.clkmgr_vif.wait_clks(2);
+ cfg.clk_rst_vif.wait_clks(2);
csr_rd(.ptr(ral.clk_hints_status), .value(value));
// We expect all units to be on.
`DV_CHECK_EQ(value, '1, "All idle and all hints high: units status should be high")
