1
\$\begingroup\$

I am trying to implement a rolling average of an array of 12 bit samples in SystemVerilog. New samples are generated and shift into an array via a clocked flip flop. The goal is to have a register that represents the sum of all of the current samples, so I can then take the average with a right-shift.

enter image description here

I've attempted to implement this with the following mix of sequential logic to update the array of data, and combinational logic to total all the array elements. Note that the number of measurements to average will eventually be more like 128-1024:

localparam      numMeasurementsToAverage        =   8'd4;

reg [11:0]  voltages    [numMeasurementsToAverage:0];

// shift in latest voltage measurement on rising clock
always_ff @ (posedge clk)
    if(~rst_n) begin
        voltages <= '{default:11'd0};
    end
    else begin 
        voltages <= {voltages[numMeasurementsToAverage-1:0], v2p_out_voltage};
    end

// calculate the current sum of voltages array
always_comb
    if(~rst_n) begin
        sumOfVoltages <= 0;
    end
    else begin
        for (int i = 0; i < numMeasurementsToAverage; i=i+1) begin
            sumOfVoltages += voltages[i];
        end
    end

This runs, but the sumOfVoltages keeps increasing... the new array total is added to the array total calculated on the previous clock edge. I need the sumOfVoltages to only reflect the total of the elements currently in the array.

Because the voltages[] array is updating on the clock, the sum should also only update on the clock edge, so I can see an argument for moving the summing logic to a clocked flip flop. However, I don't think I can use a for loop for sequential logic.

I like the for loop approach because it makes it way easier to modify the number of elements in the array, compared to hardcoding addition operators between each element in the array. Is there a shorthand for summing all values within an array, using sequential logic instead of a for loop?

Or alternately, is there a clean way to reset sumOfVoltages to zero on each clock edge, without creating a race condition between the sequential and combinational logic? The target is a Xilinx Zynq FPGA, so I need synthesizable code, not just test bench.

\$\endgroup\$
  • \$\begingroup\$ This looks like you're working your way up to a CIC filter -- if so, there's a lot of literature out there on how to do it. \$\endgroup\$ – TimWescott Mar 27 at 19:33
4
\$\begingroup\$

Your approach is extremely wasteful of resources. You don't need to keep adding up the numbers in the middle of the FIFO. Instead, you add the numbers to an accumulator as they go in, and you subtract them back out of the accumulator when they come out of the FIFO.

enter image description here

This requires just two adders, regardless of the amount of data, and a dual-port block RAM can be used for the FIFO, since you no longer need to access all of the data in it on every clock cycle.

\$\endgroup\$
1
\$\begingroup\$

I think your reset technique is the problem. When you hit reset you clear both the voltage array and the voltage sum. Then, as you shift voltages in they get added to the voltage sum at every clock, even as they are being shifted in. You are also using non-blocking assignments in your combinational adder, which doesn't make sense.

I would suggest setting sumOfVoltages = voltages[0] (blocking assignment, it's combinational logic) and then having your for loop start at i = 1.

I don't see any need to reset the array of voltages, the sum will not be valid until you fill the array anyway.

\$\endgroup\$
1
\$\begingroup\$

I elaborated your code snippet in Vivado to generate a schematic. By using the sumOfVoltage in the loop you are causing the output to be used again on the next iteration. You can see the feedback into sumOfVoltage. This explains what you are seeing. Also note the latch because of the reset in the combinatorial block.

feedback into sumOfVoltage

I updated your code. I believe this is what you want. Please note, that serially adding this way can be quite slow and you should consider putting the adder in a synchronous block. Also, if you do not really need pure average you can think of running average as Dave Tweed indicated.

without feedback Here is the updated code.

module rollingsum 
(input logic clk, input logic rst_n, input logic [11:0] v2p_out_voltage,
 output logic [13:0] sumOut
);

localparam      numMeasurementsToAverage        =   8'd4;

reg  [11:0]  voltages  [numMeasurementsToAverage:0] ;
logic [13:0]  sumOfVoltages;

// shift in latest voltage measurement on rising clock
always_ff @ (posedge clk)
    if(~rst_n) begin
        voltages <= '{default:11'd0};
    end
    else begin 
        voltages <= {voltages[numMeasurementsToAverage-1:0], v2p_out_voltage};
    end

// calculate the current sum of voltages array
logic [13:0]  vsum [numMeasurementsToAverage:0];
always_comb
/*
    if(~rst_n) begin
        sumOfVoltages <= 0;
    end
    else */ begin

        for (int i = 0; i < numMeasurementsToAverage; i=i+1) begin
            //sumOfVoltages += voltages[i];
            vsum[i+1] = vsum[i] + voltages[i];
        end
    end

assign sumOut = vsum[numMeasurementsToAverage];//sumOfVoltages;
endmodule
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.