Hi Dehim,
there are numerous better ways of implementing digital signal generators. That has already been said above so I'm not going to elaborate on that any further, but you may indeed find it interesting to learn about DDS. As a VHDL/FPGA learning exercise though, what you did is ok. Unfortunately, it doesn't work properly, but we'll see in a minute why.
First off, let me tell you what I think are good points in your VHDL code, and what I think is not that good.
The good:
- The use of IEEE's "numeric_std" library and its corresponding types and conversion functions is good practice. It's the only standard way of doing this. Please don't use the old Synopsys libraries for performing arithmetic on logic vectors, they're non-standard, have their quirks and are pretty much deprecated.
- The use of packages is also good practise if you want to write reusable code. In addition to functions and procedures, you can add component declarations in package files, so that you can reuse entities without having to declare the components in each source file that are going to use these entities.
- The use of the rising_edge and falling_edge functions is also good practice and more readable than their clunky counterparts clk'event and clk=xx. They are also more correct for simulation. I'll let you find out why.
The bad:
- There is no 'reset' signal in your clocked process. That's bad practise in my opinion. Initial values for signals (as with your 'timer' signal) will work with most FPGA's due to them usually having an internal global reset, but they won't work if synthesized on real silicon. So you should definitely add a reset signal.
- Your sin function is unmaintainable. Imagine you want to modify the resolution, you pretty much have to rewrite all of your comparisons. The use of all those if's could be changed to using an array, which would be shorter and easier to maintain. You could definitely make it more generic.
- You could also make your generator entity more generic, which would allow you to change the resolution in bits with just one parameter, for instance. If you don't know how to use generic parameters, you should definitely learn about them.
- I'm going to talk about that a little below, but I'm not sure you wrote a test bench and simulated your code before testing it on your FPGA. If you didn't, well... you should!
Speaking of simulation and test benches: apart from very trivial VHDL code, I suggest that you always write test benches and simulate your code. Behavioral simulation won't show you everything that can go bad in a VHDL design, but it will at least help you get the behavior right. (Things that can go bad and will usually not get caught by behavioral simulation include race conditions, issues when crossing clock domains... that's something to learn about as well!)
There are lots of commercial and free tools for VHDL simulation. I've used the open-source GHDL which I have found to be very good, especially the latest versions, even on complex designs. I use it along with GTKWave, both on Linux and on Windows. Commercial tools usually simulate much faster, but, except for the free versions (sometimes given away with some FPGA software packages), they can be pretty expensive.
And now, we're getting to the cause of the phase issue you have with the piece of code you posted. At first glance, there didn't seem to be anything inherently wrong with your code, so that was a bit tricky to pinpoint. Once I set up a simulation test bench, the result of the simulation was exactly what you saw on the oscilloscope.
There is nothing wrong with using integers if you take care of constraining their ranges properly, which you did.
The problem lies in the very first expression in your 'sin' function's body:
tH := t rem tRes/2;
The
intended expression looks fine, but this code doesn't actually compute what you intended, and this is because of
operators precedence ! In VHDL, '*', '/', 'rem' and 'mod' have the same precedence. If operators with the same precedence are encountered, they are evaluated left to right. (See:
http://vlsi-design-engineers.blogspot.fr/2015/07/vhdl-operators.html)
So what your code does is actually equivalent to:
tH := (t rem tRes)/2;
So the fix is to rewrite this line like so:
tH := t rem (tRes/2);
And here is the test bench code I used (to get you started):
--******************************************************************************
-- SineWaveGenerator
--
-- TestBench.vhd
-- Test Bench for SineWaveGenerator implementation.
-- https://www.eevblog.com/forum/microcontrollers/vhdl-sine-wave-generator-generates-only-half-a-sinusoid/
--
-- Author: SiliconWizard
-- Copyright (c) 2017
--
--******************************************************************************
library IEEE;
use IEEE.std_logic_1164.all;
use IEEE.numeric_std.all;
use WORK.trig.all;
--******************************************************************************
entity TestBench is
generic
(
MainClockPeriod : time := 100 ns -- Main Clock period
);
end entity;
--******************************************************************************
architecture Behavioral of TestBench is
--******************************************************************************
component TopModule is
Port
(
clk : in STD_LOGIC;
vga_g : out STD_LOGIC_VECTOR (5 downto 0)
);
end component;
--******************************************************************************
signal MainClock, Reset : std_logic;
signal Output : std_logic_vector(5 downto 0);
begin
SineWaveGenerator_Inst1: TopModule
port map
(
clk => MainClock,
vga_g => Output
);
-- Clocks.
Clock1: process
begin
MainClock <= '1';
wait for (MainClockPeriod / 2);
MainClock <= '0';
wait for (MainClockPeriod / 2);
end process;
-- Reset.
Reset1: process
begin
Reset <= '1';
wait for 10 us;
Reset <= '0';
wait;
end process;
end Behavioral;
--******************************************************************************
Attached are before fix/after fix simulation screenshots.
You can try that on your board and let me know how that goes.
For those interested, I maintain GTKWave and GHDL binaries (latest versions) for Windows.