DAC would this be ok?

[jacko]

Hi

does the following code do what i think it does, as i have no means of
test at present. It will be for outputting 32 left/32 right sawtooth
oscillators (by time mux) with FM cascadeable selectable modulation.
(supprisingly easy)

Has my style improved?

-- indi16 vhdl
-- Delta Sigma DAC (Second Order)
-- (C)2007 K Ring Technologies Semiconductor
-- designed for 66MHz operation

LIBRARY ieee;
USE ieee.std_logic_1164.all;
use ieee.std_logic_unsigned.all;

ENTITY dac2 IS
PORT
(
Clk, CEN : IN STD_LOGIC;
Sample : IN STD_LOGIC_VECTOR(15 DOWNTO 0);
Output : OUT STD_LOGIC
);
END dac2;

ARCHITECTURE a OF dac2 IS
SIGNAL Residual, Delay : STD_LOGIC_VECTOR(16 DOWNTO 0);
BEGIN
PROCESS(Clk, CEN)
VARIABLE Diff : STD_LOGIC_VECTOR(16 DOWNTO 0);
CONSTANT Max : STD_LOGIC_VECTOR(16 DOWNTO 0) :=
"10000000000000000";
BEGIN
IF Clk'EVENT AND Clk = '1' AND CEN = '1' THEN
-- output 1 when +ve and 0 when -ve
Output <= NOT Diff(16);
-- calc the residual with sign (assumes +max = 0-(+max) for easier
calc)
-- residual is the overshooted by amount
Residual <= Max - Diff;
-- then add anti-phase ultra sonic undershoot (due to delay)
Delay <= (Sample(15) & Sample) + Residual;
Diff := Delay - Residual;
END IF;
END PROCESS;
END a;

 

[Martin Thompson]

Hi Jacko,

Comments below mainly on style, rather than function - part of my crusade
against std_logic_unsigned

Hi

does the following code do what i think it does, as i have no means of
test at present. It will be for outputting 32 left/32 right sawtooth
oscillators (by time mux) with FM cascadeable selectable modulation.
(supprisingly easy)

Has my style improved?

-- indi16 vhdl
-- Delta Sigma DAC (Second Order)
-- (C)2007 K Ring Technologies Semiconductor
-- designed for 66MHz operation

66MHz in what device?

LIBRARY ieee;
USE ieee.std_logic_1164.all;
use ieee.std_logic_unsigned.all;

Consider using numeric_std.all

ENTITY dac2 IS
PORT
(
Clk, CEN : IN STD_LOGIC;
Sample : IN STD_LOGIC_VECTOR(15 DOWNTO 0);
Output : OUT STD_LOGIC
);
END dac2;

ARCHITECTURE a OF dac2 IS
SIGNAL Residual, Delay : STD_LOGIC_VECTOR(16 DOWNTO
0);

use UNSIGNED not std_logic_vector

BEGIN
PROCESS(Clk, CEN)

Do you need to be sensitive to CEN? If it's a clock enable, it
doesn't belong in the sensitivity list. Also, to me CE would be the
standard name for a clock enable and putting an extra N on the end may
imply that it is active low (by some naming conventions). Call it
"clock_enable" and no-one will be in any doubt. I love Emacs' TAB
completion of long names

VARIABLE Diff : STD_LOGIC_VECTOR(16 DOWNTO 0);

use UNSIGNED not std_logic_vector

CONSTANT Max : STD_LOGIC_VECTOR(16 DOWNTO 0) :=
"10000000000000000";

and again

BEGIN
IF Clk'EVENT AND Clk = '1' AND CEN = '1' THEN

Some synths won't accept this form for a clock-enable - you may find
you have to nest another IF statement instead

-- output 1 when +ve and 0 when -ve
Output <= NOT Diff(16);
-- calc the residual with sign (assumes +max = 0-(+max) for easier
calc)
-- residual is the overshooted by amount
Residual <= Max - Diff;

On the first clock tick, Diff will be full of 'U's, so Residual will
also get 'U's, which will propogate to Delay and back to Diff. So it
will never get anything done.

You need either a reset or an explicit initialisation of Diff. If
you go for initialisation, ensure that your synth tools will take
notice of it and that your target is an FPGA which supports it...

-- then add anti-phase ultra sonic undershoot (due to delay)
Delay <= (Sample(15) & Sample) + Residual;

This looks like you are doing a sign-extension, which for "unsigned"
you probably don't want. Numeric_std has functions for resizing
vectors as well. You'll need to cast Sample as an "unsigned" in this
line as well.

Also, you are using the *previous* cycle's version of Residual here,
not the one you just calculated, is that the intention? Does the lag
matter? I haven't analysed your code deeply enough to tell for
myself, and my simulator is otherwise engaged...

Diff := Delay - Residual;

Again, you are using the *previous* cycle's version of Delay and Residual here,
not the one you just calculated.

END IF;
END PROCESS;
END a;

Hope this helps!

Cheers,
Martin

 

[Andy]

Hi

does the following code do what i think it does, as i have no means of
test at present. It will be for outputting 32 left/32 right sawtooth
oscillators (by time mux) with FM cascadeable selectable modulation.
(supprisingly easy)

Has my style improved?

-- indi16 vhdl
-- Delta Sigma DAC (Second Order)
-- (C)2007 K Ring Technologies Semiconductor
-- designed for 66MHz operation

LIBRARY ieee;
USE ieee.std_logic_1164.all;
use ieee.std_logic_unsigned.all;

ENTITY dac2 IS
PORT
(
Clk, CEN : IN STD_LOGIC;
Sample : IN STD_LOGIC_VECTOR(15 DOWNTO 0);
Output : OUT STD_LOGIC
);
END dac2;

ARCHITECTURE a OF dac2 IS
SIGNAL Residual, Delay : STD_LOGIC_VECTOR(16 DOWNTO 0);
BEGIN
PROCESS(Clk, CEN)
VARIABLE Diff : STD_LOGIC_VECTOR(16 DOWNTO 0);
CONSTANT Max : STD_LOGIC_VECTOR(16 DOWNTO 0) :=
"10000000000000000";
BEGIN
IF Clk'EVENT AND Clk = '1' AND CEN = '1' THEN
-- output 1 when +ve and 0 when -ve
Output <= NOT Diff(16);
-- calc the residual with sign (assumes +max = 0-(+max) for easier
calc)
-- residual is the overshooted by amount
Residual <= Max - Diff;
-- then add anti-phase ultra sonic undershoot (due to delay)
Delay <= (Sample(15) & Sample) + Residual;
Diff := Delay - Residual;
END IF;
END PROCESS;
END a;

Martin has given you very good advice.

As far as style comments:
I would use rising_edge(clk) instead of (clk'event and clk='1'). It is
a recognized standard function, and is more communicative of what you
want.

Whenever you need to declare a local variable or signal whose length
is related to that of a port vector, use the port's 'length attribute
to automatically calculate the length of the local object:

variable diff : unsigned(sample'length downto 0); -- 1 bit longer than
sample

This (and other tricks) make it easier to change the width of the
circuit based on changes to the width of sample. In fact, sample could
be an unconstrained SLV, set by whatever it is associated with when
the entity/component is instantiated.

When you need to set a vector constant, use to_unsigned(integer,
length), or something like (sample'length => '1', others => '0'), to
make it automatically handle other changes. I rarely use a bit string
literal for a long vector value; too much typing (and counting digits,
and too easy to make a mistake.

Other ideas:

Instead of unsigned (preferable over slv for numeric quantities) you
may want to try integers (my preference). See other threads here for
advantages/disadvantages of integers and vectors.

Regarding Martin's observations about delayed values of residual and
delay, if that is a problem, you can use variables to make it easier
to manage cycle delays. Keep in mind, this can also lead to long
combinatorial logic chains if you don't pay attention to it.
References to a variable value before it has been assigned within a
clock cycle result in a register to hold the value from the previous
clock cycle. For example, the references to diff are for registered
values. The assignment to Output represents an additional register
delay. With variables, write-before-read = combinatorial, read-before-
write = register. I like using variables because they (and the
synthesized circuit) always behave like the "software" reads.

Andy

 

[jacko]

Martin has given you very good advice.

As far as style comments:
I would use rising_edge(clk) instead of (clk'event and clk='1'). It is
a recognized standard function, and is more communicative of what you
want.

i'll start using rising_edge(clk)

Whenever you need to declare a local variable or signal whose length
is related to that of a port vector, use the port's 'length attribute
to automatically calculate the length of the local object:

variable diff : unsigned(sample'length downto 0); -- 1 bit longer than
sample

seems ok, how do you do 1 bit unsigned ?

This (and other tricks) make it easier to change the width of the
circuit based on changes to the width of sample. In fact, sample could
be an unconstrained SLV, set by whatever it is associated with when
the entity/component is instantiated.

When you need to set a vector constant, use to_unsigned(integer,
length), or something like (sample'length => '1', others => '0'), to
make it automatically handle other changes. I rarely use a bit string
literal for a long vector value; too much typing (and counting digits,
and too easy to make a mistake.

Other ideas:

Instead of unsigned (preferable over slv for numeric quantities) you
may want to try integers (my preference). See other threads here for
advantages/disadvantages of integers and vectors.

a type cast warning would be good, and i have some learning about
arrays to do.

Regarding Martin's observations about delayed values of residual and
delay, if that is a problem, you can use variables to make it easier
to manage cycle delays. Keep in mind, this can also lead to long
combinatorial logic chains if you don't pay attention to it.
References to a variable value before it has been assigned within a
clock cycle result in a register to hold the value from the previous
clock cycle. For example, the references to diff are for registered
values. The assignment to Output represents an additional register
delay. With variables, write-before-read = combinatorial, read-before-
write = register. I like using variables because they (and the
synthesized circuit) always behave like the "software" reads.

i would assume that := makes a combinational, and <= would give me a
register.
in this sense what i want is to remove the output <= and use := ??

the current residual is delayed and added/subtracted from the incoming
sample stream to make the 1 1/2 cycle utrasonic oscillation equal the
'error' amplitude.

i.e. current output is in error by residual (both in registers and
output)
diff or next output is in error by -residual (combinatorial yes
delayed residual)
delay or second next output is in error by residual (registered delay)

so are you saying i have to move Diff:=delay-residual to before the <=
assignments?? as both quantities are already registered as required.

i thought vhdl was a parallel evaluation language, and := would never
infer a register, although an infered latch may be useful.

removing the output delay may reduce flip flop count, but would
increase timing jitter on diff carry chain, and so loose ADC
resolution.

so will i get what i wanted?

 

[Mike Treseler]

i would assume that := makes a combinational, and <= would give me a
register.

Note that := requires a variable id on the left side.
and <= requires a port or signal id on the left side.

Signal values are updated at the end of the process
not in line, so only the final assignment matters.

Variable values are updated in line.
If I use a variable id above the update,
I get the value from last time, and a
register is inferred to hold this value.
If I use a variable id below the update,
I get the value from this time.

in this sense what i want is to remove the output <= and use := ??

No. See above.
I would recommend not mixing
variables and signals, at least
until you are good at using an
RTL viewer and simulator.

Here are some variable-only examples:
http://home.comcast.net/~mike_treseler/
Start with the template example.

so will i get what i wanted?

Try it and see.
Get a vhdl simulator and rtl viewer.

-- Mike Treseler

 

[Martin Thompson]

Martin has given you very good advice.

Thanks!

As far as style comments:
I would use rising_edge(clk) instead of (clk'event and clk='1'). It is
a recognized standard function, and is more communicative of what you
want.

Doh! Missed that one - that's another of my hobby horses

Cheers,
Martin

 

[industrial_one]

Btw jacko I responded to you in:
http://groups.google.ca/group/comp....read/8a99c6f52da4ba37/b818738a7a271f16?hl=en&