procedure problem
 

I have a statemachine that needs to maintain each current state for a
certain time, then switch to another state.
To save tedious typing, I wrote a helper procedure that I call
with
- the state signal instance,
- the next state value,
- the counter signal instance which is used to count the remaining
clock cycles,
- and the number of clock cycles to wait.

Here is the procedure:

procedure advance_state (
signal state : inout state_type;
constant next_state : in state_type;
signal counter : inout integer range 0 to 1000;
constant dt : in integer)
is
begin
if counter = dt then
state <= next_state;
counter <= 0;
end if;
end advance_state;

and here a snippet of the state machine code:

process(clock)
begin
if rising_edge(clock) then
case state is
when IDLE =>
if we = '1' then
state <= START_A;
counter <= 0;
end if;

when START_A => advance_state(state, START_B, counter, 2000);
....
when START_B => advance_state(state, START_C, counter, 2000);
....
when START_C => advance_state(state, WRITE_A, counter, 2000);
....

Code like this works fine when synthesized in ISE14.1 for a Spartan6
device, but it does not work for Spartan3.

Does anyone see a problem with the above?
Are there better ways to implement a state machine with state-duration?

Thanks,
Thomas

Re: procedure problem
 

I noticed that

signal counter : inout integer range 0 to 1000;

and

advance_state(state, START_B, counter, 2000);

2000 is not in range 0 to 1000, does this compile?

Where do you increment the counter?

-- Pontus

Re: procedure problem
 

Am 11.12.2012 16:33, schrieb Pontus:
> I noticed that
>
> signal counter : inout integer range 0 to 1000;
>
> and
>
> advance_state(state, START_B, counter, 2000);
>
> 2000 is not in range 0 to 1000, does this compile?

This is an error that I made when copying the code into
the message. I tried to simplify it a bit.
Yes, it does compile, and, as I said, it does even work
for Spartan 6.

> Where do you increment the counter?

The counter in incremented on every rising_edge(clock)
elsewhere in the process.

Thomas

Re: procedure problem
 

On 12/11/2012 10:22 AM, Thomas Heller wrote:
> Am 11.12.2012 16:33, schrieb Pontus:
>> I noticed that
>>
>> signal counter : inout integer range 0 to 1000;
>>
>> and
>>
>> advance_state(state, START_B, counter, 2000);
>>
>> 2000 is not in range 0 to 1000, does this compile?
>
> This is an error that I made when copying the code into
> the message. I tried to simplify it a bit.
> Yes, it does compile, and, as I said, it does even work
> for Spartan 6.
>
>> Where do you increment the counter?
>
> The counter in incremented on every rising_edge(clock)
> elsewhere in the process.
>
> Thomas
>

When you say compile, it synthesizes without errors and passes PaR with
no timing errors (timing errors don't error out the software). Unless
it is a resource issue, I can't see why a S3 vs. S6 would make a difference.

Chris

Re: procedure problem
 

Am 11.12.2012 17:53, schrieb Christopher Felton:
> On 12/11/2012 10:22 AM, Thomas Heller wrote:
>> Am 11.12.2012 16:33, schrieb Pontus:
>>> I noticed that
>>>
>>> signal counter : inout integer range 0 to 1000;
>>>
>>> and
>>>
>>> advance_state(state, START_B, counter, 2000);
>>>
>>> 2000 is not in range 0 to 1000, does this compile?
>>
>> This is an error that I made when copying the code into
>> the message. I tried to simplify it a bit.
>> Yes, it does compile, and, as I said, it does even work
>> for Spartan 6.
>>
>>> Where do you increment the counter?
>>
>> The counter in incremented on every rising_edge(clock)
>> elsewhere in the process.
>>
>> Thomas
>>
>
> When you say compile, it synthesizes without errors and passes PaR with
> no timing errors (timing errors don't error out the software). Unless
> it is a resource issue, I can't see why a S3 vs. S6 would make a
> difference.

Yes, no errors, and it fits timing.
I assume they use different parsers/map/par for s3 and s6.
Maybe that explains the issue.

How do others write statemachines with certain durations for the states?

Thomas

Re: procedure problem
 

On 12/11/2012 11:52 AM, Thomas Heller wrote:
> Am 11.12.2012 17:53, schrieb Christopher Felton:
>> On 12/11/2012 10:22 AM, Thomas Heller wrote:
>>> Am 11.12.2012 16:33, schrieb Pontus:
>>>> I noticed that
>>>>
>>>> signal counter : inout integer range 0 to 1000;
>>>>
>>>> and
>>>>
>>>> advance_state(state, START_B, counter, 2000);
>>>>
>>>> 2000 is not in range 0 to 1000, does this compile?
>>>
>>> This is an error that I made when copying the code into
>>> the message. I tried to simplify it a bit.
>>> Yes, it does compile, and, as I said, it does even work
>>> for Spartan 6.
>>>
>>>> Where do you increment the counter?
>>>
>>> The counter in incremented on every rising_edge(clock)
>>> elsewhere in the process.
>>>
>>> Thomas
>>>
>>
>> When you say compile, it synthesizes without errors and passes PaR with
>> no timing errors (timing errors don't error out the software). Unless
>> it is a resource issue, I can't see why a S3 vs. S6 would make a
>> difference.
>
> Yes, no errors, and it fits timing.
> I assume they use different parsers/map/par for s3 and s6.
> Maybe that explains the issue.

It shouldn't have a different parser, I believe the Xilinx synthesis is
device aware but typically synthesis will synthesize to generic
primitives (gates) and then the mapper will translate to device
specific. I would expect only the mapper to be significantly different.
But there should be some kind of warning or error that there are not
enough resources etc., hmmmm.

>
> How do others write statemachines with certain durations for the states?

If you mean simple time delays, I usually have a local counter,
something like your example.. Other times I will make a generic state
delay, where the return state (next state out of the delay state) is a
register and the delay is a register. Then it is like calling a
function in CPU programming, you have to program the delay and return
state before transitioning to the delay state. It is a reusable delay
with the extra overhead of the programmable delay (expiration) and
return state. And normally it is three states, delay_start, delay_wait,
delay_end.


Regards,
Chris

Regards,
Chris

Re: procedure problem
 

Christopher Felton wrote:
> On 12/11/2012 11:52 AM, Thomas Heller wrote:
>> Am 11.12.2012 17:53, schrieb Christopher Felton:
>>> On 12/11/2012 10:22 AM, Thomas Heller wrote:
>>>> Am 11.12.2012 16:33, schrieb Pontus:
>>>>> I noticed that
>>>>>
>>>>> signal counter : inout integer range 0 to 1000;
>>>>>
>>>>> and
>>>>>
>>>>> advance_state(state, START_B, counter, 2000);
>>>>>
>>>>> 2000 is not in range 0 to 1000, does this compile?
>>>>
>>>> This is an error that I made when copying the code into
>>>> the message. I tried to simplify it a bit.
>>>> Yes, it does compile, and, as I said, it does even work
>>>> for Spartan 6.
>>>>
>>>>> Where do you increment the counter?
>>>>
>>>> The counter in incremented on every rising_edge(clock)
>>>> elsewhere in the process.
>>>>
>>>> Thomas
>>>>
>>>
>>> When you say compile, it synthesizes without errors and passes PaR with
>>> no timing errors (timing errors don't error out the software). Unless
>>> it is a resource issue, I can't see why a S3 vs. S6 would make a
>>> difference.
>>
>> Yes, no errors, and it fits timing.
>> I assume they use different parsers/map/par for s3 and s6.
>> Maybe that explains the issue.
>
> It shouldn't have a different parser, I believe the Xilinx synthesis is
> device aware but typically synthesis will synthesize to generic
> primitives (gates) and then the mapper will translate to device
> specific. I would expect only the mapper to be significantly different.
> But there should be some kind of warning or error that there are not
> enough resources etc., hmmmm.
>
>>
>> How do others write statemachines with certain durations for the states?
>
> If you mean simple time delays, I usually have a local counter,
> something like your example.. Other times I will make a generic state
> delay, where the return state (next state out of the delay state) is a
> register and the delay is a register. Then it is like calling a
> function in CPU programming, you have to program the delay and return
> state before transitioning to the delay state. It is a reusable delay
> with the extra overhead of the programmable delay (expiration) and
> return state. And normally it is three states, delay_start, delay_wait,
> delay_end.
>
>
> Regards,
> Chris
>
> Regards,
> Chris
S6 and newer devices use the "new parser" for XST. Older devices
do not, unless you add "-use_new_parser yes" to the command line
options for XST. This could very well explain the difference in
behavior between the two. You need ISE 12 or newer to use the
new parser.

-- Gabor

Re: procedure problem
 

On Tuesday, December 11, 2012 9:52:41 AM UTC-5, Thomas Heller wrote:

- The state machine has no reset. There is nothing to get it into a valid state which then can be stepped through once reset completes. That alone would cause it to work in simulation but not always work in actual hardware.

- Signal state is an output only of the procedure. That shouldn't matter for the problem you're seeing (which is not clear what exactly is the problem...you didn't say).

- Since you asked, I would've written it like this...

if rising_edge(clock) then
if (reset = '1') then
state <= idle; -- I presume
counter = 0;
elsif (counter = dt) then
counter = 0;
case state is
when START_A => state <= START_B;
...
end case;
else
counter <= counter + 1;
end if;
end if;

Not that what you have is wrong, but you asked how others would write it that are 'better'. Whether you think yours is better or not is up to you based on your own criteria.

Kevin Jennings

Re: procedure problem
 

On 12/11/2012 3:37 PM, Gabor wrote:
> Christopher Felton wrote:
>> On 12/11/2012 11:52 AM, Thomas Heller wrote:
>>> Am 11.12.2012 17:53, schrieb Christopher Felton:
>>>> On 12/11/2012 10:22 AM, Thomas Heller wrote:
>>>>> Am 11.12.2012 16:33, schrieb Pontus:
>>>>>> I noticed that
>>>>>>
>>>>>> signal counter : inout integer range 0 to 1000;
>>>>>>
>>>>>> and
>>>>>>
>>>>>> advance_state(state, START_B, counter, 2000);
>>>>>>
>>>>>> 2000 is not in range 0 to 1000, does this compile?
>>>>>
>>>>> This is an error that I made when copying the code into
>>>>> the message. I tried to simplify it a bit.
>>>>> Yes, it does compile, and, as I said, it does even work
>>>>> for Spartan 6.
>>>>>
>>>>>> Where do you increment the counter?
>>>>>
>>>>> The counter in incremented on every rising_edge(clock)
>>>>> elsewhere in the process.
>>>>>
>>>>> Thomas
>>>>>
>>>>
>>>> When you say compile, it synthesizes without errors and passes PaR with
>>>> no timing errors (timing errors don't error out the software). Unless
>>>> it is a resource issue, I can't see why a S3 vs. S6 would make a
>>>> difference.
>>>
>>> Yes, no errors, and it fits timing.
>>> I assume they use different parsers/map/par for s3 and s6.
>>> Maybe that explains the issue.
>>
>> It shouldn't have a different parser, I believe the Xilinx synthesis
>> is device aware but typically synthesis will synthesize to generic
>> primitives (gates) and then the mapper will translate to device
>> specific. I would expect only the mapper to be significantly
>> different. But there should be some kind of warning or error that
>> there are not enough resources etc., hmmmm.
>>
>>>
>>> How do others write statemachines with certain durations for the states?
>>
>> If you mean simple time delays, I usually have a local counter,
>> something like your example.. Other times I will make a generic state
>> delay, where the return state (next state out of the delay state) is a
>> register and the delay is a register. Then it is like calling a
>> function in CPU programming, you have to program the delay and return
>> state before transitioning to the delay state. It is a reusable delay
>> with the extra overhead of the programmable delay (expiration) and
>> return state. And normally it is three states, delay_start,
>> delay_wait, delay_end.
>>
>>
>> Regards,
>> Chris
>>
>> Regards,
>> Chris
> S6 and newer devices use the "new parser" for XST. Older devices
> do not, unless you add "-use_new_parser yes" to the command line
> options for XST. This could very well explain the difference in
> behavior between the two. You need ISE 12 or newer to use the
> new parser.
>
> -- Gabor

Seems odd to me, wonder what the rational was. That is, a new release
of software to use a different parser (frontend of synthesis) for
different devices.

Regards,
Chris

Re: procedure problem
 

Christopher Felton wrote:
> On 12/11/2012 3:37 PM, Gabor wrote:
>> Christopher Felton wrote:
>>> On 12/11/2012 11:52 AM, Thomas Heller wrote:
>>>> Am 11.12.2012 17:53, schrieb Christopher Felton:
>>>>> On 12/11/2012 10:22 AM, Thomas Heller wrote:
>>>>>> Am 11.12.2012 16:33, schrieb Pontus:
>>>>>>> I noticed that
>>>>>>>
>>>>>>> signal counter : inout integer range 0 to 1000;
>>>>>>>
>>>>>>> and
>>>>>>>
>>>>>>> advance_state(state, START_B, counter, 2000);
>>>>>>>
>>>>>>> 2000 is not in range 0 to 1000, does this compile?
>>>>>>
>>>>>> This is an error that I made when copying the code into
>>>>>> the message. I tried to simplify it a bit.
>>>>>> Yes, it does compile, and, as I said, it does even work
>>>>>> for Spartan 6.
>>>>>>
>>>>>>> Where do you increment the counter?
>>>>>>
>>>>>> The counter in incremented on every rising_edge(clock)
>>>>>> elsewhere in the process.
>>>>>>
>>>>>> Thomas
>>>>>>
>>>>>
>>>>> When you say compile, it synthesizes without errors and passes PaR
>>>>> with
>>>>> no timing errors (timing errors don't error out the software). Unless
>>>>> it is a resource issue, I can't see why a S3 vs. S6 would make a
>>>>> difference.
>>>>
>>>> Yes, no errors, and it fits timing.
>>>> I assume they use different parsers/map/par for s3 and s6.
>>>> Maybe that explains the issue.
>>>
>>> It shouldn't have a different parser, I believe the Xilinx synthesis
>>> is device aware but typically synthesis will synthesize to generic
>>> primitives (gates) and then the mapper will translate to device
>>> specific. I would expect only the mapper to be significantly
>>> different. But there should be some kind of warning or error that
>>> there are not enough resources etc., hmmmm.
>>>
>>>>
>>>> How do others write statemachines with certain durations for the
>>>> states?
>>>
>>> If you mean simple time delays, I usually have a local counter,
>>> something like your example.. Other times I will make a generic state
>>> delay, where the return state (next state out of the delay state) is a
>>> register and the delay is a register. Then it is like calling a
>>> function in CPU programming, you have to program the delay and return
>>> state before transitioning to the delay state. It is a reusable delay
>>> with the extra overhead of the programmable delay (expiration) and
>>> return state. And normally it is three states, delay_start,
>>> delay_wait, delay_end.
>>>
>>>
>>> Regards,
>>> Chris
>>>
>>> Regards,
>>> Chris
>> S6 and newer devices use the "new parser" for XST. Older devices
>> do not, unless you add "-use_new_parser yes" to the command line
>> options for XST. This could very well explain the difference in
>> behavior between the two. You need ISE 12 or newer to use the
>> new parser.
>>
>> -- Gabor
>
> Seems odd to me, wonder what the rational was. That is, a new release
> of software to use a different parser (frontend of synthesis) for
> different devices.
>
> Regards,
> Chris

The rationale is very simple actually. They didn't want to run
regression testing on all of the older device families. They
give you a switch to turn on (or off) the new parser regardless of
the FPGA family, but if you decide to use the new parser on an
older device you should not expect support.

-- Gabor