I'm trying to create a VGA driver in VHDL.
I'm going for 640x480 @ 60 Hz, so I need 25 MHz and 31.5 KHz clocks. divider_h process is driven by 50 MHz clock and results in 25 MHz clock. On each tick of 25 MHz clock h_counter is incremented by process h_sync and when it reaches some value (H_FRONT + H_SYNC - 1), divider_v process is fired and sets clock_v to 1 for a brief period of time.
Quartus II's timing analysis fails and a warning appears: Can't achieve minimum setup and hold requirement clock along 1 path(s). Clock hold section in compilation report points out that MSB of v_counter is the culprit, with minimum slack time of -0.050 ns. Second lowest slack time is 0.218 ns, which is fine.
I tried to use long 1 state for clock_v with brief 0 state and minimum slack time has increased to -0.019 ns, which is still unacceptable.
As I understand, hold clock issue means that inputs are changed before they are processed correctly, so I tried to make both 1 and 0 appear for roughly same period of time. To my surprise, over 40 paths turned red with the same error.
Commenting out v_clock process fixes the problem.
Why is slack time for MSB so much higher than for other bits? What can I do to fix this issue?
Here is my code:
library ieee;
use ieee.std_logic_1164.all;
library Famikon;
use Famikon.Types.all;
use Famikon.VgaNames.all;
entity VgaDriver is
port (
-- inputs
clock_50: in STD_LOGIC; -- 50 MHz
reset: in bit; -- '1' resets
r, g, b: in screen_pixel; -- bit_vector subtype
-- outputs
vga_x, vga_y: out hw_coord; -- integer subtype
vga_drawing: out bit;
hw_r, hw_g, hw_b: out hw_pixel; -- bit_vector subtype
vga_hsync, vga_vsync: out bit;
vga_blank, vga_clock: out bit
);
end entity;
architecture Arch of VgaDriver is
signal clock_h, clock_v: std_logic;
signal h_counter, v_counter: vga_counter; -- integer subtype
signal h_coord, v_coord: hw_coord; -- integer subtype
signal h_active, v_active: bit;
begin
-- some irrelevant code --
h_active <= '1' when (h_counter >= H_BLANK) else '0';
v_active <= '1' when (v_counter >= V_BLANK) else '0';
divider_h: process (clock_50) begin
if (rising_edge(clock_50)) then
clock_h <= not clock_h;
end if;
end process;
divider_v: process (h_counter) begin
if (h_counter /= H_FRONT + H_SYNC - 1) then
clock_v <= '0';
else
clock_v <= '1';
end if;
end process;
h_clock: process (clock_h, reset) begin
if (rising_edge(clock_h)) then
if (reset = '1') then
h_counter <= 0;
elsif (h_counter = H_TOTAL - 1) then
h_counter <= 0;
else
h_counter <= h_counter + 1;
end if;
end if;
end process;
v_clock: process (clock_v, reset) begin
if (rising_edge(clock_v)) then
if (reset = '1') then
v_counter <= 0;
elsif (v_counter = V_TOTAL - 1) then
v_counter <= 0;
else
v_counter <= v_counter + 1;
end if;
end if;
end process;
end architecture;
Details of the failing path:
- From:
v_counter[9] - To:
v_counter[9] - From Clock:
clock - To Clock:
clock - Required Hold Relationship: 0.000 ns
- Required Shortest P2P Time: 0.618 ns
- Actual Shortes P2P Time: 0.568 ns
- Minimum Slack: -0.050 ns
v_counter is a bit_vector(9 downto 0). When I use Locate in Design on this path, Quartus points to the first line of v_clock process (one with rising_edge(clock_v)).
There's also a warning about ripple clocks which mentions all h_counter bits, clock_h and three gated clocks: Equal0, Equal0~1 and Equal0~0.
It would help if you provided the start and end point of the failing path. It looks like the main problem is that you're generating different clocks and have cross domain timing issues or incomplete constraints establishing the relationship between the different domains. Presumably
resetcomes from theclock_50domain and it shouldn't be directly used in the other two domains. A more robust solution would use a single clock with synchronous enables to get the same effect asclock_vandclock_h.When generated clocks are necessary you should not be attempting to build them in the FPGA fabric but rather use an onboard PLL/DLL to manage them. The skew between domains is better controlled that way and the timing analyzer will handle the situation better. Every domain needs to have its own reset signal that is synchronized to its own clock.
It may also be a good idea to register
v_activeandh_activeto separate the comparison logic from whatever they feed into. Plus you are using synchronous resets so there is no need to have them in the sensitivity lists.