[dv,pwm] DV improvements for PWM

hw/dv/sv/pwm_monitor/pwm_if.sv

@@ -3,13 +3,19 @@
 // SPDX-License-Identifier: Apache-2.0
 
 interface pwm_if (
+  // PWM core clock and reset.
   input logic clk,
   input logic rst_n,
+  // Phase counter starting within DUT; this signal is asserted for a single core clock
+  // cycle and marks the first beat of the first phase of the counter.
+  input logic start_cntr,
+  // PWM output to be monitored.
   input logic pwm
 );
 
   clocking cb @(posedge clk);
     input pwm;
+    input start_cntr;
   endclocking
 
 endinterface : pwm_if

hw/dv/sv/pwm_monitor/pwm_item.sv

@@ -5,12 +5,16 @@
 class pwm_item extends uvm_sequence_item;
 
   int monitor_id    = 0; // for debugging purpose only
-  int period        = 0; // clks in a beat
-  int duty_cycle    = 0; // high vs low cnt
-  int active_cnt    = 0; // number of clocks pwm was high
-  int inactive_cnt  = 0; // number of clocks pwm was low
-  int phase         = 0; // what clock cnt did the pulse start
-  bit invert        = 0; // (1)active low (0) active high
+  int period        = 0; // clks in a pulse cycle
+  int duty_cycle    = 0; // 0.16 fixed-point fraction for which output was asserted
+  int active_cnt    = 0; // number of clocks pwm was asserted
+  int inactive_cnt  = 0; // number of clocks pwm was deasserted
+  int phase         = 0; // phase at which output was asserted (0-ffff)
+  bit invert        = 0; // (1) active low (0) active high
+
+  // May be assigned to `phase` to indicate that the pwm phase cannot be ascertained because
+  // the signal has never been asserted.
+  static int PhaseUnknown = -1;
 
   `uvm_object_utils_begin(pwm_item)
     `uvm_field_int(period, UVM_DEFAULT)
@@ -29,7 +33,7 @@ class pwm_item extends uvm_sequence_item;
       txt = "\n------| PWM ITEM |------";
       txt = { txt, $sformatf("\n Item from monitor %d", monitor_id) };
       txt = { txt, $sformatf("\n Period %d clocks", period) };
-      txt = { txt, $sformatf("\n Duty cycle %0d pct ", duty_cycle) };
+      txt = { txt, $sformatf("\n Duty cycle %04x", duty_cycle) };
       txt = { txt, $sformatf("\n inverted %0b", invert) };
       txt = { txt, $sformatf("\n # of active cycles %d", active_cnt) };
       txt = { txt, $sformatf("\n # of inactive cycles %d", inactive_cnt) };
@@ -38,9 +42,9 @@ class pwm_item extends uvm_sequence_item;
     endfunction : convert2string
 
   function int get_duty_cycle();
-    real dc = 0;
-    dc = (invert) ? (real'(inactive_cnt) / real'(period) * 100)
-                  : (real'(active_cnt) / real'(period) * 100);
-    return dc;
+    // 16-bit fraction for which the duty cycle is asserted; 0xffff nearly always asserted;
+    // the DUT cannot produce a continuously asserted output except by using inversion and
+    // a duty cycle of 0.
+    return (active_cnt << 16) / period;
   endfunction : get_duty_cycle
 endclass

hw/dv/sv/pwm_monitor/pwm_monitor.sv

@@ -8,9 +8,38 @@ class pwm_monitor extends dv_base_monitor #(
   );
   `uvm_component_utils(pwm_monitor)
 
+  // Allow for (i) single cycle to reset phase counter and (ii) single register delay of
+  // output signal.
+  //
+  // (This relies upon the internal details of the DUT because we're checking the phase
+  //  delay exactly; alernatively we could just check the relative phases of the PWM
+  //  output transitions, or allow a margin of error.)
+  uint output_delay = 2;
+
+  // Elapsed time in core clocks at the start of the most recent pulse cycle.
+  int unsigned pulse_cycle_start = 0;
+  // Duration of a pulse cycle in core clocks.
+  int unsigned pulse_cycle_clks = 1;
+  // Current elapsed time; core clocks.
+  int unsigned clk_elapsed = 0;
+  // Phase counter configuration.
+  int unsigned clk_div = 0;
+  int unsigned dc_resn = 0;
+  // Are we still waiting to the first transition to the active state?
+  bit first_activation = 1'b1;
+  // Have we received valid configuration and are we monitoring the PWM output?
+  bit monitoring = 1'b0;
+
   extern function new(string name = "", uvm_component parent = null);
   extern function void build_phase(uvm_phase phase);
 
+  // One or more of the phase counter configuration parameters has been changed; ensure that our
+  // measurement of elapsed pulse cycles is updated accordingly.
+  extern function void phase_config_changed();
+
+  // Send a sequence item to the scoreboard for checking.
+  extern function void send_item(int unsigned inactive_cycles, int unsigned active_cycles);
+
   // collect transactions forever - already forked in dv_base_monitor::run_phase
   extern protected task collect_trans();
 
@@ -35,45 +64,141 @@ function void pwm_monitor::build_phase(uvm_phase phase);
   end
 endfunction
 
+// One or more of the phase counter configuration parameters has been changed; ensure that our
+// measurement of elapsed pulse cycles is updated accordingly.
+function void pwm_monitor::phase_config_changed();
+  `uvm_info(`gfn, $sformatf("Collected phase counter config: clk_div 0x%0x dc_resn 0x%0x",
+                            clk_div, dc_resn), UVM_HIGH)
+  clk_elapsed = 0;
+  // Remember that we have not yet seen an activation, because this means that we cannot know the
+  // phase.
+  first_activation = 1'b1;
+  // Calculate how many core clocks a single pulse cycle takes; we require this information so
+  // that we may detect a pulse cycle in which the PWM output is not asserted at all
+  // (0% duty cycle).
+  pulse_cycle_clks = (1 + clk_div) * (2 ** (dc_resn + 1));
+  // Remember the start time of this pulse cycle in core clocks, but at the _output_ of the PWM
+  // channel, ie. delayed by two cycles.
+  pulse_cycle_start = output_delay;
+  `uvm_info(`gfn, $sformatf("Core clocks/pulse cycle = 0x%0x", pulse_cycle_clks), UVM_HIGH)
+endfunction
+
+// Send a sequence item to the scoreboard for checking.
+function void pwm_monitor::send_item(int unsigned inactive_cycles, int unsigned active_cycles);
+  // Measure phase delay of PWM output assertion.
+  uint phase_delay;
+
+  pwm_item item = pwm_item::type_id::create("item");
+  item.invert       = cfg.invert;
+  item.monitor_id   = cfg.monitor_id;
+  item.active_cnt   = active_cycles;
+  item.inactive_cnt = inactive_cycles;
+  item.period       = inactive_cycles + active_cycles;
+  item.duty_cycle   = item.get_duty_cycle();
+
+  if (first_activation && ~|active_cycles) begin
+    // We cannot detect the phase of the PWM output if it has never been asserted. A zero duty
+    // cycle can occur part-way through a sequence in the context of blinking/heartmode modes,
+    // however, in which case we do have phase information still.
+    item.phase = pwm_item::PhaseUnknown;
+  end else begin
+    // Each PWM pulse cycle is divided into 2^DC_RESN+1 beats, per beat the 16-bit
+    // phase counter increments by 2^(16-DC_RESN-1)(modulo 65536)
+    phase_delay = ((clk_elapsed - output_delay) / (1 + clk_div)) * (2 ** (15 - dc_resn));
+    item.phase = phase_delay[15:0];
+  end
+  `uvm_info(`gfn, $sformatf("clk_elapsed %0x phase_delay %0x", clk_elapsed, item.phase),
+            UVM_HIGH)
+
+  analysis_port.write(item);
+endfunction
+
 task pwm_monitor::collect_trans();
+  // Counting of core clocks for measuring the inactive and active phases of the PWM output.
   uint count_cycles, active_cycles;
   logic pwm_prev = 0;
 
-  wait(cfg.vif.rst_n);
   forever begin
-    if (!cfg.active) begin
-      wait (cfg.active);
-      count_cycles = 0;
-      active_cycles = 0;
+    if (!cfg.vif.rst_n) begin
+      // Wait until DUT comes out of reset.
+      wait (cfg.vif.rst_n);
+      // This is the reset state of the phase counter configuration.
+      clk_div = 'h8000;
+      dc_resn = 7;
+      // Ensure that our other state is updated accordingly.
+      phase_config_changed();
+      monitoring = 1'b0;
     end
 
     @(cfg.vif.cb);
-    count_cycles++;
-    if (cfg.vif.cb.pwm != pwm_prev) begin
-      `uvm_info(`gfn, $sformatf("Detected edge: %0b->%0b at %0d cycles (from last edge)",
-                                pwm_prev, cfg.vif.cb.pwm, count_cycles), UVM_HIGH)
-      pwm_prev = cfg.vif.cb.pwm;
-      if (cfg.vif.cb.pwm == cfg.invert) begin
-        // We got to the first (active) half duty cycle point. Save the count and restart.
-        active_cycles = count_cycles;
-      end else begin
-        uint phase_count;
-        pwm_item item = pwm_item::type_id::create("item");
-        item.invert       = cfg.invert;
-        item.monitor_id   = cfg.monitor_id;
-        item.active_cnt   = active_cycles;
-        item.inactive_cnt = count_cycles;
-        item.period       = count_cycles + active_cycles;
-        item.duty_cycle   = item.get_duty_cycle();
-
-        // Each PWM pulse cycle is divided into 2^DC_RESN+1 beats, per beat the 16-bit
-        // phase counter increments by 2^(16-DC_RESN-1)(modulo 65536)
-        phase_count = ((item.period / (2 ** (cfg.resolution + 1))) *
-                       (2 ** (16 - (cfg.resolution - 1))));
-        item.phase = (phase_count % 65536);
-        analysis_port.write(item);
+
+    // Do nothing until the phase counter has started; note that this does not necessarily mean
+    // that the channel is enabled. We must monitor disabled channels too for completeness.
+    if (cfg.active && cfg.vif.cb.start_cntr) begin
+      // Pick up the new phase counter configuration.
+      clk_div = cfg.clk_div;
+      dc_resn = cfg.resolution;
+      // Update other state relating to phase counter/pulse cycles.
+      phase_config_changed();
+      monitoring = 1'b1;
+    end else begin
+      // We need to keep track of the elapsed core clock cycles in order to ascertain
+      // (i) when a complete pulse cycle has elapsed but no transition of the pwm output occurred.
+      // (ii) the phase of a pwm activation within the pulse cycle.
+      clk_elapsed++;
+    end
+
+    if (monitoring) begin
+      count_cycles++;
+      // Has the state of the PWM output changed?
+      if (cfg.vif.cb.pwm != pwm_prev) begin
+        `uvm_info(`gfn, $sformatf("Detected edge: %0b->%0b at %0d cycles (from last edge)",
+                                  pwm_prev, cfg.vif.cb.pwm, count_cycles), UVM_HIGH)
+        `uvm_info(`gfn, $sformatf(" (elapsed core clk cycles %0d)", clk_elapsed), UVM_HIGH)
+        pwm_prev = cfg.vif.cb.pwm;
+        if (cfg.vif.cb.pwm == cfg.invert) begin
+          // The PWM output is now inactive. Save the count of 'active' cycles and start counting
+          // the 'inactive' cycles.
+          active_cycles = count_cycles;
+        end else begin
+          // The PWM output is now active, marking the end of the 'inactive' period.
+          if (first_activation) begin
+            // Swallow the first 'inactive' period because this is the switch on delay and any
+            // phase delay up until the first assertion of the PWM output.
+            first_activation = 1'b0;
+          end else begin
+            // Send the sequence item to the scoreboard for checking.
+            send_item(count_cycles, active_cycles);
+          end
+          // Remember the start of the active phase as the beginning of a pulse cycle for this PWM
+          // channel; if an entire pulse cycle elapses with no assertion this constitutes a cycle
+          // for which the duty cycle is zero.
+          pulse_cycle_start = clk_elapsed;
+        end
+        count_cycles = 0;
+      end else if (clk_elapsed >= pulse_cycle_start + pulse_cycle_clks) begin
+        `uvm_info(`gfn, $sformatf("Idle PWM output detected (0x%0x cycle(s))", pulse_cycle_clks),
+                  UVM_HIGH)
+        // Send a pulse cycle showing no activity.
+        send_item(pulse_cycle_clks, 0);
+        pulse_cycle_start = clk_elapsed;
+        active_cycles = 0;
+        count_cycles = 0;
       end
+    end else begin
       count_cycles = 0;
+      active_cycles = 0;
+      // Capture the initial state of the PWM output signal; this is the 'inactive' state of the
+      // PWM output, shortly before the channel is enabled.
+      pwm_prev = cfg.vif.cb.pwm;
+      first_activation = 1'b1;
+    end
+
+    // We still need to become inactive when requested....
+    // May need to steal another signal from the DUT here to make this work reliably; 'cfg.active'
+    // is set before the counter is disabled.
+    if (!cfg.active) begin
+      monitoring = 1'b0;
     end
   end
 endtask
@@ -83,6 +208,6 @@ endtask
 task pwm_monitor::monitor_ready_to_end();
   forever begin
     @(cfg.vif.cb);
-    ok_to_end = ~cfg.active;
+    ok_to_end = ~monitoring;
   end
 endtask

hw/dv/sv/pwm_monitor/pwm_monitor_cfg.sv

@@ -5,10 +5,10 @@
 class pwm_monitor_cfg  extends dv_base_agent_cfg;
 
   int monitor_id = 0;
-  bit en_monitor = 1'b1; // enable monitor
   bit invert     = 1'b0; // 0: active high,  1: active low
-  bit active     = 1'b0; // 1: collect items 0: ignore
+  bit active     = 1'b0; // 1: valid configuration, collect items 0: ignore
   int resolution = 0;
+  int clk_div    = 0;
 
   // interface handle
   virtual pwm_if vif;

hw/ip/pwm/dv/README.md

@@ -65,7 +65,12 @@ parameter uint NUM_PWM_CHANNELS = 6;
   typedef struct packed {
     bit [15:0]   B;
     bit [15:0]   A;
-  } dc_blink_t;
+  } duty_cycle_t;
+
+  typedef struct packed {
+    bit [15:0]   Y;
+    bit [15:0]   X;
+  } blink_param_t;
 ```
 ### TL_agent
 PWM instantiates (already handled in CIP base env) [tl_agent](../../../dv/sv/tl_agent/README.md)
@@ -99,8 +104,8 @@ Some of the most commonly-used tasks / functions are as follows:
 * set_reg_en(pwm_status_e state): enable registers for writing
 * set_cfg_reg(cfg_reg_t cfg_reg): program global configuration  (ClkDiv/DcResn/CntrEn))
 * set_ch_enables(bit [PWM_NUM_CHANNELS-1:0] enables): used to enable and disable the different channels
-* set_duty_cycle(bit [$bits(PWM_NUM_CHANNELS)-1:0] channel, dc_blink_t value ,bit [3:0] resn) set the A and B values for channel
-* set_blink(bit [$bits(PWM_NUM_CHANNELS)-1:0] channel, dc_blink_t value): set X and Y value for pulse and heart bit
+* set_duty_cycle(bit [$bits(PWM_NUM_CHANNELS)-1:0] channel, duty_cycle_t value ,bit [3:0] resn) set the A and B values for channel
+* set_blink(bit [$bits(PWM_NUM_CHANNELS)-1:0] channel, blink_param_t value): set the X and Y values for blinking modes
 * set_param(bit [$bits(PWM_NUM_CHANNELS)-1:0] channel, param_reg_t value): set channel configuration (blink/heartbeat/phase)
 * shutdown_dut(): this will disable all channels as an indication for the scoreboard to verify all remaining items.
 
@@ -116,7 +121,7 @@ It creates the following analysis ports to retrieve the data monitored by corres
 * exp_item                   : It is used to store the expected item constructed from tl address and data channels.
 
 when a channel is configured to start sending pulses the first expected item is generated.
-Because of the way the PWM IP is design the first and the last pulse might not match the configuration settings.
+Because of the way the PWM IP is designed the first and the last pulse might not match the configuration settings.
 Therefore the scoreboard will wait until a channel is disabled before checking the output.
 Once a channel is disabled it will first discard the first two items received from the monitor.
 The is send because the channel was enabled and has no valid information.

hw/ip/pwm/dv/env/pwm_env_cfg.sv

@@ -41,6 +41,14 @@ function void pwm_env_cfg::initialize(bit [31:0] csr_base_addr = '1);
 
   // only support 1 outstanding TL items in tlul_adapter
   m_tl_agent_cfg.max_outstanding_req = 1;
+
+  // Switch the alert agent to use the TL-UL clock rather than asynchronous clocking because
+  // otherwise we shall incur ping timeouts when stopping the TL-UL clock for an extended period to
+  // exercise low power mode.
+  foreach(list_of_alerts[i]) begin
+    string alert_name = list_of_alerts[i];
+    m_alert_agent_cfgs[alert_name].is_async = 0;
+  end
 endfunction
 
 function int pwm_env_cfg::get_clk_core_freq();

hw/ip/pwm/dv/env/pwm_env_pkg.sv

@@ -50,10 +50,17 @@ package pwm_env_pkg;
     bit [15:0]   PhaseDelay;
   } param_reg_t;
 
+  // Duty cycles (DUTY_CYCLE_i register).
   typedef struct packed {
     bit [15:0]   B;
     bit [15:0]   A;
-  } dc_blink_t;
+  } duty_cycle_t;
+
+  // Blink mode parameters (BLINK_PARAM_i register).
+  typedef struct packed {
+    bit [15:0]   Y;
+    bit [15:0]   X;
+  } blink_param_t;
 
   // the index of multi-reg is at the last char of the name
   function automatic int get_multireg_idx(string name);