Verilog FIR filter

Question

Hello I am implementing an FIR filter in Verilog, using the DE2 board. For some reason the output out of the speakers is full of static, although it does appear to filter out some frequencies. Here is the code for the FIR:

// Local wires.
    wire read_ready, write_ready, read, write;
    wire [23:0] readdata_left, readdata_right;
    wire [23:0] writedata_left, writedata_right;

    assign writedata_left = output_sample;
    
    assign writedata_right = output_sample;
    assign read = 1;
    assign write = 1;
    
    wire [23:0] input_sample = readdata_left;
    
    reg [23:0] output_sample;

the input sample is put through the FIR, and the output sample is put to both left and right speakers for simplicity.

//The FIR filter
parameter N = 40;
reg signed[23:0] coeffs[39:0];
reg [23:0] holderBefore[39:0];

wire [23:0] toAdd[39:0];

// -- 1000-1100
always @(*)
begin
    coeffs[0]=24'b100000000110101001111110; // -- 1
    coeffs[1]=24'b100000000110100011011011; // -- 2
    coeffs[2]=24'b100000000111000100001100; // -- 3
    coeffs[3]=24'b100000000111111000101000;// -- 4
    coeffs[4]=24'b100000001000011111111100;// -- 5
    coeffs[5]=24'b100000001000011001011001;// -- 6
    coeffs[6]=24'b100000000111010001010011;// -- 7
    coeffs[7]=24'b100000000100100110111010;// -- 8
    coeffs[8]=24'b100000000000011010001101;// -- 9
    coeffs[9]=24'b000000000101101111000000;// -- 10
    coeffs[10]=24'b000000001101100001000100;// -- 11
    coeffs[11]=24'b000000010110111100000000;// -- 12
    coeffs[12]=24'b000000100001011111000001;// -- 13
     coeffs[13]=24'b000000101100101001010111;// -- 14
    coeffs[14]=24'b000000111000000000110100;// -- 15
    coeffs[15]=24'b000001000010101010011001;// -- 16
    coeffs[16]=24'b000001001100001011111000;// -- 17
    coeffs[17]=24'b000001010011111101111100;// -- 18
    coeffs[18]=24'b000001011001011001010010;// -- 19
    coeffs[19]=24'b000001011100010000110010;// -- 20
    coeffs[20]=24'b000001011100010000110010;// -- 20
    coeffs[21]=24'b000001011001011001010010;// -- 19
    coeffs[22]=24'b000001001100001011111000;// -- 18
    coeffs[23]=24'b000001001100001011111000;// -- 17
    coeffs[24]=24'b000001000010101010011001;// -- 16
    coeffs[25]=24'b000000111000000000110100;// -- 15
     coeffs[26]=24'b000000101100101001010111;// -- 14
    coeffs[27]=24'b000000100001011111000001;// -- 13
    coeffs[28]=24'b000000010110111100000000;// -- 12
    coeffs[29]=24'b000000001101100001000100;// -- 11
    coeffs[30]=24'b000000000101101111000000;// -- 10
    coeffs[31]=24'b100000000000011010001101;// -- 9
    coeffs[32]=24'b100000000100100110111010;// -- 8
    coeffs[33]=24'b100000000111010001010011;// -- 7
    coeffs[34]=24'b100000001000011001011001;// -- 6
    coeffs[35]=24'b100000001000011111111100;// -- 5
    coeffs[36]=24'b100000000111111000101000;// -- 4
    coeffs[37]=24'b100000000111000100001100;// -- 3
     coeffs[38]=24'b100000000110100011011011;// -- 2
    coeffs[39]=24'b100000000110101001111110;// -- 1
end

genvar i;

generate
for (i=0; i<N; i=i+1)
    begin: mult
        multiplier mult1(
          .dataa(coeffs[i]),
          .datab(holderBefore[i]),
          .out(toAdd[i]));
    end
endgenerate

always @(posedge CLOCK_50 or posedge reset)
begin
    if(reset)
        begin
            holderBefore[39]     <= 0;
            holderBefore[38]     <= 0;
            holderBefore[37]     <= 0;
            holderBefore[36]     <= 0;
            holderBefore[35]     <= 0;
            holderBefore[34]     <= 0;
            holderBefore[33]     <= 0;
            holderBefore[32]     <= 0;
            holderBefore[31]     <= 0;
            holderBefore[30]     <= 0;
                holderBefore[29]    <= 0;
                holderBefore[28]    <= 0;
            holderBefore[27]    <= 0;
            holderBefore[26]    <= 0;
            holderBefore[25]    <= 0;
            holderBefore[24]     <= 0;
            holderBefore[23]     <= 0;
            holderBefore[22]     <= 0;
            holderBefore[21]     <= 0;
            holderBefore[20]     <= 0;
            holderBefore[19]     <= 0;
            holderBefore[18]     <= 0;
            holderBefore[17]     <= 0;
            holderBefore[16]     <= 0;
            holderBefore[15]     <= 0;
                holderBefore[14]    <= 0;
                holderBefore[13]    <= 0;
            holderBefore[12]    <= 0;
            holderBefore[11]    <= 0;
            holderBefore[10]    <= 0;
            holderBefore[9]     <= 0;
            holderBefore[8]     <= 0;
            holderBefore[7]     <= 0;
            holderBefore[6]     <= 0;
            holderBefore[5]     <= 0;
            holderBefore[4]     <= 0;
            holderBefore[3]     <= 0;
            holderBefore[2]     <= 0;
            holderBefore[1]     <= 0;
            holderBefore[0]     <= 0;
            output_sample       <= 0;
        end
    else
        begin
            holderBefore[39]     <= holderBefore[38];
            holderBefore[38]     <= holderBefore[37];
            holderBefore[37]     <= holderBefore[36];
            holderBefore[36]    <= holderBefore[35];
            holderBefore[35]    <= holderBefore[34];
            holderBefore[34]    <= holderBefore[33];
            holderBefore[33]     <= holderBefore[32];
            holderBefore[32]     <= holderBefore[31];
            holderBefore[31]     <= holderBefore[30];
            holderBefore[30]     <= holderBefore[29];
            holderBefore[29]     <= holderBefore[28];
            holderBefore[28]     <= holderBefore[27];
            holderBefore[27]     <= holderBefore[26];
            holderBefore[26]     <= holderBefore[25];
            holderBefore[25]     <= holderBefore[24];
                holderBefore[24]    <= holderBefore[23];
            holderBefore[23]    <= holderBefore[22];
            holderBefore[22]    <= holderBefore[21];
            holderBefore[21]     <= holderBefore[20];
            holderBefore[20]     <= holderBefore[19];
            holderBefore[19]     <= holderBefore[18];
            holderBefore[18]     <= holderBefore[17];
            holderBefore[17]     <= holderBefore[16];
            holderBefore[16]     <= holderBefore[15];
            holderBefore[15]     <= holderBefore[14];
            holderBefore[14]     <= holderBefore[13];
            holderBefore[13]     <= holderBefore[12];
            holderBefore[12]    <= holderBefore[11];
            holderBefore[11]    <= holderBefore[10];
            holderBefore[10]    <= holderBefore[9];
            holderBefore[9]     <= holderBefore[8];
            holderBefore[8]     <= holderBefore[7];
            holderBefore[7]     <= holderBefore[6];
            holderBefore[6]     <= holderBefore[5];
            holderBefore[5]     <= holderBefore[4];
            holderBefore[4]     <= holderBefore[3];
            holderBefore[3]     <= holderBefore[2];
            holderBefore[2]     <= holderBefore[1];
            holderBefore[1]     <= holderBefore[0];
            holderBefore[0]     <= input_sample;
            output_sample <= (input_sample + toAdd[0] + toAdd[1] + 
                              toAdd[2] + toAdd[3] + toAdd[4] + toAdd[5] +
                              toAdd[6] + toAdd[7] + toAdd[8] + toAdd[9] + 
                              toAdd[10] + toAdd[11] + toAdd[12]+ toAdd[13] + toAdd[14] + 
                              toAdd[15] + toAdd[16] + toAdd[17] + toAdd[18] +
                              toAdd[19] + toAdd[20] + toAdd[21] + toAdd[22] + 
                              toAdd[23] + toAdd[24] + toAdd[25] +toAdd[26] + toAdd[27] + toAdd[28] + toAdd[29] +
                              toAdd[19] + toAdd[20] + toAdd[21] + toAdd[22] + 
                              toAdd[30] + toAdd[31] + toAdd[32]+ toAdd[33] + toAdd[34] + toAdd[35] + toAdd[36] +
                              toAdd[37] + toAdd[38] + toAdd[39]);
        end
end

//The multiplier
module multiplier (dataa,datab,out);
input [23:0]dataa;
input [23:0]datab;
reg [47:0]result;
output[23:0]out;
always@(*)begin
    result = dataa*datab;
end
assign out = result[46:24]; 
endmodule

Granted that the coefficients are correct, is there something wrong with the code? I assume there is a problem with the representation of the coefficients in binary, or the multiplier is wrong but I can't figure it out.

Answer 1

The multiplier is not performing signed multiplication.

Verilog defaults to unsigned, if any part of an equation is unsigned it will be come unsigned. If a bit selection is made (even if it is the full width) the arithmetic will be come unsigned.

The following code should perform a signed arithmetic.

module multiplier (
  input      signed [23:0] dataa,
  input      signed [23:0] datab,
  output reg signed [23:0] out
);
reg signed [47:0] result;

  always @* begin
    result = dataa*datab;
    out    = result[46:24];
  end  
endmodule

Your not capturing the the MSB of result into out which would look like a gain error on unsigned or positive numbers, but might could loose the sign of negative numbers.

When you perform the sum into output_sample there is a possibility that the numbers overflow. for every addition you should add 1 bit of headroom, then limit. May be add some flags to record if it is overflowing/clipping at this stage.