Auto-suggest helps you quickly narrow down your search results by suggesting possible matches as you type.

- Community Forums
- :
- Forums
- :
- Vivado RTL Development
- :
- Timing Analysis
- :
- Re: timing error -- in a counter --- Artix 7

- Subscribe to RSS Feed
- Mark Topic as New
- Mark Topic as Read
- Float this Topic for Current User
- Bookmark
- Subscribe
- Mute
- Printer Friendly Page

rakeshm55

Explorer

- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Email to a Friend
- Report Inappropriate Content

08-08-2016 12:48 AM

7,047 Views

Registered:
10-27-2013

Hi,

I have a design which without any timing errors on virtex 6.... Recently I tried porting this design to Artix 7.... Here i encounter timing errors in simple synchronous counter....

Attached is the details of timing errors..... it says 31 levels of logic ......

here is the brief of my implementation......

My clock speed is 92.16Mhz.....So I was trying to generate a one hot pulse of period 100us...

Is my implementation wrong??.....

signal counter : natural range 0 to 9216 ;--9216 := 0;

----- Generate 100us---- 100us generated here to bring in precise control over timing....

process (adc_clk)

begin

if rising_edge ( adc_clk) then

if (T_109_timer_RESET = '1') then

counter <= 0;

clk_100us <= '0';

else

counter <= (counter + 1) mod 9217;--10;--9217;

if (counter = 9216) then--9) then -- 9216

clk_100us <= '1';

else

clk_100us <= '0';

end if;

end if;

end if;

end process ;

1 Solution

Accepted Solutions

avrumw

Expert

- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Email to a Friend
- Report Inappropriate Content

08-08-2016 07:32 AM - edited 08-08-2016 07:38 AM

12,711 Views

Registered:
01-23-2009

Does this have any thing to do with **mod** operation??

Yes, absolutely.

This is an excellent example of why you need to "think hardware".

Lets look at your original line

counter <= (counter + 1) mod 9217

The tools break this down as follows:

- compute counter+1

- this is a 14 bit adder

- check if it is greater than or equal to 9217

- this is a 14 bit subtractor and an equality check

- the equality check is either a direct result from the carry, or a 14 bit AND gate

- if it is, subtract 9217 from the result of the addition

- this is a 14 bit subtractor

All three of these addition/subtraction operations need to be done in series. Thus the critical path goes through all three adder/subtractors.

The tools actually seem to have done worse - it says there are 21 CARRY4 elements on the critical path. Each 14 bit addition/subtraction would require 4 CARRY4 elements, so the 21 on the chain represents slightly more than 5 operations of 14 bits. It may be complicated by the fact that 9217 (in your "mod 9217") may be being interpreted as an integer (rather than a number in the range) and hence the operation ends up being 32 bits...

Now lets look at the new code

- check if the old value is 9216

- this is a 14 bit AND gate with local inversions

- have a MUX that chooses the new value of either 0 or counter+1

- this is a 14 bit 2:1 MUX

- one input is a constant

- the other input is the result of a 14 bit adder

So, the critical path here is through the 14 bit adder plus one 2:1 MUX. Almost certainly the first part (check if the old value is equal to 9216) is faster than the adder. Since it is done in parallel with the adder, it is not on the critical path.

So, it is highly expected (and proven through your code) that the second implementation will by MUCH faster - probably 3x the speed of the first (maybe even 5x+).

Furthermore, this operation (a 14 bit counter that is modulo something other than 2^N) should be able to EASILY meet timing in this technology (in fact, much larger counters should be expected to meet timing easily).

Avrum

8 Replies

vijayak

Xilinx Employee

- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Email to a Friend
- Report Inappropriate Content

08-08-2016 12:52 AM

7,046 Views

Registered:
10-24-2013

Hi @rakeshm55

You may try reducing the logic levels. Below are the links with suggestions.

http://www.xilinx.com/support/answers/9417.html

Thanks,Vijay

--------------------------------------------------------------------------------------------

Please mark the post as an answer "Accept as solution" in case it helped resolve your query.

Give kudos in case a post in case it guided to the solution.

--------------------------------------------------------------------------------------------

Please mark the post as an answer "Accept as solution" in case it helped resolve your query.

Give kudos in case a post in case it guided to the solution.

nagabhar

Xilinx Employee

- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Email to a Friend
- Report Inappropriate Content

08-08-2016 01:24 AM

7,037 Views

Registered:
05-07-2015

HI @rakeshm55

The end point is reset input of counter register (hence RTL code you showed might not be relevant to this path.

whats driving "T_109_timer_RESET" signal? Look at the combinations logic through which it coming through.

you can get a better idea by viewing the schematic of the failing path.

Note: Always show full path report when posting timing related queries. in the snapshot you attached, it does not show the datapath. Anyway , it is evident that the issue is due to too many logic levels.

Thanks

Bharath

----------------------------------------------------------------------------------------------

Please mark the Answer as "Accept as solution" if information provided addresses your query/concern.

Give Kudos to a post which you think is helpful.

---------------------------------------------------------------------------------------------

Bharath

----------------------------------------------------------------------------------------------

Please mark the Answer as "Accept as solution" if information provided addresses your query/concern.

Give Kudos to a post which you think is helpful.

---------------------------------------------------------------------------------------------

rakeshm55

Explorer

- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Email to a Friend
- Report Inappropriate Content

08-08-2016 06:54 AM

7,016 Views

Registered:
10-27-2013

Hi,

I modified the counter as below and the error went off.... there were 3 other counters with similar logic also threw up same error.....

process (adc_clk)

begin

if rising_edge ( adc_clk) then

if (T_109_timer_RESET = '1') then

counter <= 0;

clk_100us <= '0';

else

if (counter =< 9216) then--9) then -- 9216

counter <= 0;

clk_100us <= '1';

else

clk_100us <= '0';

counter <= (counter + 1) ;

end if;

end if;

end if;

end process ;

Once same style was followed timing error on counters vanished..... Does this have any thing to do with **mod** operation?? please help

avrumw

Expert

- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Email to a Friend
- Report Inappropriate Content

08-08-2016 07:32 AM - edited 08-08-2016 07:38 AM

12,712 Views

Registered:
01-23-2009

Does this have any thing to do with **mod** operation??

Yes, absolutely.

This is an excellent example of why you need to "think hardware".

Lets look at your original line

counter <= (counter + 1) mod 9217

The tools break this down as follows:

- compute counter+1

- this is a 14 bit adder

- check if it is greater than or equal to 9217

- this is a 14 bit subtractor and an equality check

- the equality check is either a direct result from the carry, or a 14 bit AND gate

- if it is, subtract 9217 from the result of the addition

- this is a 14 bit subtractor

All three of these addition/subtraction operations need to be done in series. Thus the critical path goes through all three adder/subtractors.

The tools actually seem to have done worse - it says there are 21 CARRY4 elements on the critical path. Each 14 bit addition/subtraction would require 4 CARRY4 elements, so the 21 on the chain represents slightly more than 5 operations of 14 bits. It may be complicated by the fact that 9217 (in your "mod 9217") may be being interpreted as an integer (rather than a number in the range) and hence the operation ends up being 32 bits...

Now lets look at the new code

- check if the old value is 9216

- this is a 14 bit AND gate with local inversions

- have a MUX that chooses the new value of either 0 or counter+1

- this is a 14 bit 2:1 MUX

- one input is a constant

- the other input is the result of a 14 bit adder

So, the critical path here is through the 14 bit adder plus one 2:1 MUX. Almost certainly the first part (check if the old value is equal to 9216) is faster than the adder. Since it is done in parallel with the adder, it is not on the critical path.

So, it is highly expected (and proven through your code) that the second implementation will by MUCH faster - probably 3x the speed of the first (maybe even 5x+).

Furthermore, this operation (a 14 bit counter that is modulo something other than 2^N) should be able to EASILY meet timing in this technology (in fact, much larger counters should be expected to meet timing easily).

Avrum

gszakacs

Instructor

- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Email to a Friend
- Report Inappropriate Content

08-08-2016 07:51 AM

7,002 Views

Registered:
08-14-2007

For a simple divide-by-n pulse generator, I typically add a pipeline stage to the max count detection. This means that the signal that resets the count comes directly from a flop. One way to code it is to use the max count detection as your output pulse (pulse comes a cycle sooner than as you coded it).

process (adc_clk)

begin

if rising_edge ( adc_clk) then

if (T_109_timer_RESET = '1') then

counter <= 0;

clk_100us <= '0';

else

if (counter = 9215) then -- on when count is 9216

clk_100us <= '1';

else

clk_100us <= '0';

end if;

if (clk_100us= '1') then -- on when count is 9216

counter <= 0;

else

counter <= (counter + 1) ;

end if;

end if;

end if;

end process ;

-- Gabor

avrumw

Expert

- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Email to a Friend
- Report Inappropriate Content

08-08-2016 08:57 AM

6,991 Views

Registered:
01-23-2009

This means that the signal that resets the count comes directly from a flop.

(I have done this too)

But now that I am thinking about it, I am not convinced it actually buys anything...

Continuing in my "think hardware" thread...

In my previous e-mail, I documented what the critical path was - it was the

- the calculation of counter+1 and

- the 2-1 MUX afterward

By pipelining the comparison (as @gszakacs suggests) it does, indeed make the enable of the MUX significantly faster, but that will only make a difference if the comparison is slower than the addition, which is done in parallel. Due to the complexity of addition versus comparison to a constant, I suspect that it won't - the addition will be slower. So, it seems likely that this won't actually speed up the overall design.

On the other hand it's "mostly harmless". This will use one more flip-flop, but flip-flops are virtually free in the FPGA. The only other potential issue is if the counter is parameterized; the 9217 is coded as a parameter (say N), causing the counter to count from 0 to N-1. The original code will work for any value of N, including N=1 (which means that it will count from 0 to 0, meaning all clocks are the "terminal count" and N=2 (oscillating between 0 and 1). With the new code N=1 won't work and I don't think N=2 will either (I would have to think a bit harder to be sure) - at least not without some coding changes to take these values into account.

So, it would be interesting to see if this pipelining helps... (If someone wants to try and report back, we would be interested).

Avrum

gszakacs

Instructor

- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Email to a Friend
- Report Inappropriate Content

08-08-2016 11:25 AM

6,983 Views

Registered:
08-14-2007

Actually, as I coded it there are the same number of flops. The "pipeline" register is the same register as the output, which was already there in the original code. As you point out, the number for the comparison becomes N-2 instead of N-1, which means you can't go down to "divide by 1." On the other hand, I don't generally code things like this unless N is already fairly large.

-- Gabor

rakeshm55

Explorer

- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Email to a Friend
- Report Inappropriate Content

08-08-2016 09:56 PM

6,956 Views

Registered:
10-27-2013

Wrong design took

257 LUT

17 REG

Modified one took

20 LUT

17 REG

*Is this design modification you are suggesting?? if not pls let me know*

*if so it takes*

*19 LUT *

*18 REG*

* ----- Geneerate 100us---- process (adc_clk) begin if rising_edge ( adc_clk) then if (DD_reset = '1') then counter <= 0; clk_100us <= '0'; else if (reset_count = '1') then -- 9216 clk_100us <= '1'; counter <= 0; else clk_100us <= '0'; counter <= (counter + 1); end if; end if;end if;end process ; process (adc_clk) begin if rising_edge ( adc_clk) then if ( counter = 9215) then reset_count <= '1'; else reset_count <= '0'; end if; end if; end process;*

*--------*