6
\$\begingroup\$

I wrote my first module in Verilog. The purpose is to maintain two counters and emit signals corresponding to VGA's HSync and VSync, as well as HBlank and VBlank pulses to be used by a video controller for the blanking intervals. The only input is a clock signal of 25.175 MHz. All timing information was acquired from here.

module sync (clk, hblank, hsync, vblank, vsync) ;
  input clk;
  output reg hblank = 0, hsync = 1, vblank = 0, vsync = 1;
  reg [9:0] hcnt = 0, vcnt = 0;

  always @(posedge clk) begin
    case (hcnt)
      640: hblank <= 1;
      656: hsync <= 0;
      752: hsync <= 1;
      800: begin
        hblank <= 0;
        hcnt <= 0;
        vcnt <= vcnt + 1;
      end
    endcase
    case (vcnt)
      480: vblank <= 1;
      490: vsync <= 0;
      492: vsync <= 1;
      525: begin
        vblank <= 0;
        vcnt <= 0;
      end
    endcase
    hcnt <= hcnt + 1;
  end
endmodule
\$\endgroup\$
0
\$\begingroup\$

If you had ran simulation or loaded onto FPGA, you would noticed you didn't get the expected behavior. Run simulation and look at waveforms before loading to FPGA.

You have hcnt <= hcnt + 1; at the bottom of your always block, this will override hcnt <= 0; which is not what you want. <= is a non-blocking assignment which means it will be evaluated immediately but the value will not be updated until the end of the timestep. Order matters.

The simplest solution is to move hcnt <= hcnt + 1; above the case statement.

Be aware that your hcnt are counting from 0 to 800 which is 801 clocks. You may want to consider subtracting 1 from each case condition or initiating/resetting hcnt (and vcnt) as 1 instead of 0.

Your vcnt will only be 525 for one clock, which don't look intentional. Consider moving it inside the case(hcnt)'s condition 800.

I will recommend adding a reset input. And also recommend using an ANSI header Non-ANSI is required for Verilog-95 and pre IEEE1364. Since Verilog-2001, ANSI style is preferred mostly because it reduces the amount of typing.

Below is my suggestion. Note I haven't tested that all functional requirements are met (that should be done in your testbench).

module sync (
  input clk, rst_n,                       // <-- ANSI header
  output reg hblank, hsync, vblank, vsync
);
  reg [9:0] hcnt, vcnt;

  always @(posedge clk) begin
    if (!rst_n) begin // <-- synchronous reset logic
      hblank <= 1'b0;
      hsync <= 1'b1;
      vblank <= 1'b0;
      vsync <= 1'b1;
      hcnt <= 10'h001;  // <-- init as 1 so case index doesn't need to change
      vcnt <= 10'h001;  // <-- same as vcnt
    end
    else begin
      hcnt <= hcnt + 10'h001; // <-- default assignment, will be updated after the clock
      case (hcnt) // <-- uses the sampled value, not the result of the above line
        10'd640: hblank <= 1'b1;
        10'd656: hsync <= 1'b0;
        10'd752: hsync <= 1'b1;
        10'd800: begin
          hblank <= 1'b0;
          hcnt <= 10'h001; // <-- reset as 1, last assignment wins
          vcnt <= vcnt + 10'h001;
          case (vcnt)
            10'd480: vblank <= 1'b1;
            10'd490: vsync <= 1'b0;
            10'd492: vsync <= 1'b1;
            10'd525: begin
              vblank <= 1'b0;
              vcnt <= 10'h001; // <-- reset as 1, last assignment wins
            end
          endcase
        end
      endcase
    end
  end
endmodule

You may want to consider using the 2-alway block coding style. It does require more lines of code small designs (usually reduces lines of code for large/complex desings). The main benefit is you can access to the present state and next state of a flop.

// sequential logic (uses non-blocking assignment and is synchronous)
always @(posedge clk) begin
  if (!rst_n) begin
    hblank <= 1'b0;
    hsync <= 1'b1;
    vblank <= 1'b0;
    vsync <= 1'b1;
    hcnt <= 10'h001;
    vcnt <= 10'h001;
  end
  else begin
    hblank <= next_hblank;
    hsync <= next_hsync;
    vblank <= next_vblank;
    vsync <= next_vsync;
    hcnt <= next_hcnt;
    vcnt <= next_vcnt;
  end
end

// combinational logic (uses blocking assignment and is asynchronous)
always @* begin
  next_hblank = hblank; // <-- default keep previous
  next_hsync = hsync;
  next_vblank = vblank;
  next_vsync = vsync;
  next_hcnt = hcnt + 10'h001; // <-- default increment
  next_vcnt = vcnt;

  // calc next values, update as needed
  case (hcnt)
    10'd640: next_hblank = 1'b1;
    10'd656: next_hsync = 1'b0;
    10'd752: next_hsync = 1'b1;
    10'd800: begin
      next_hblank = 1'b0;
      next_hcnt = 10'h001;
      next_vcnt = vcnt + 10'h001;
      case (vcnt)
        10'd480: next_vblank = 1'b1;
        10'd490: next_vsync = 1'b0;
        10'd492: next_vsync = 1'b1;
        10'd525: begin
          next_vblank = 1'b0;
          next_vcnt = 10'h001;
        end
      endcase
    end
  endcase
end
\$\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.