Hi, since I am a beginner with fpgas and vhdl, I want to ask if some fpga-veteran can give me some hints how to improve my coding-style. Attached is my latest code (for controlling an LCD via LVDS), it works in the simulator and is not yet tested on the chip. regards, Benjamin -- lvds_tick is a clock at 7x pixel-clock (140 MHz) -- lvds_clk is the clock for the lvds bus, same as pixel clock, but not symmetric -- like this for one pixel period: --___-- process (lvds_tick,screen_reset) begin if screen_reset='1' then --load_lvds<='1'; lvds_div<=0; elsif rising_edge(lvds_tick) then if lvds_div = 0 then lvds_clk <='1'; --load_lvds<='0'; elsif lvds_div = 2 then lvds_clk<='0'; elsif lvds_div = 5 then lvds_clk<='1'; elsif lvds_div = 6 then lvds_div <= 0; end if; if lvds_div = 7 then lvds_div <= 0; --load_lvds<='1'; else lvds_div <= lvds_div + 1; end if; end if; end process; -- this process generates the load impuls for the shift register below process (lvds_tick,screen_reset) begin if screen_reset='1' then load_lvds<='1'; elsif falling_edge(lvds_tick) then if lvds_div = 7 then load_lvds<='1'; else load_lvds<='0'; end if; end if; end process; -- generates serial data for one lvds-pair lvds_1:process (lvds_tick) begin if load_lvds= '1' then reg1 <= lvds1; elsif rising_edge(lvds_tick) then reg1 <= reg1((7-2) downto 0) & '0'; end if; lvds_1_in <= reg1(7 - 1); end process;
how can I improve my code?
Started by ●April 29, 2005
Reply by ●April 29, 20052005-04-29
Benjamin Menk�c wrote:> Hi, > > since I am a beginner with fpgas and vhdl, I want to ask if some > fpga-veteran can give me some hints how to improve my coding-style. > > Attached is my latest code (for controlling an LCD via LVDS), it works > in the simulator and is not yet tested on the chip. > > regards, > Benjamin > > -- lvds_tick is a clock at 7x pixel-clock (140 MHz) > -- lvds_clk is the clock for the lvds bus, same as pixel clock, but not > symmetric > -- like this for one pixel period: --___-- > process (lvds_tick,screen_reset) > begin > if screen_reset='1' then > --load_lvds<='1'; > lvds_div<=0; > elsif rising_edge(lvds_tick) then > if lvds_div = 0 then > lvds_clk <='1'; > --load_lvds<='0'; > elsif lvds_div = 2 then > lvds_clk<='0'; > elsif lvds_div = 5 then > lvds_clk<='1'; > elsif lvds_div = 6 then > lvds_div <= 0; > end if; > if lvds_div = 7 then > lvds_div <= 0; > --load_lvds<='1'; > else > lvds_div <= lvds_div + 1; > end if; > end if; > end process; > > -- this process generates the load impuls for the shift register below > process (lvds_tick,screen_reset) > begin > if screen_reset='1' then > load_lvds<='1'; > elsif falling_edge(lvds_tick) then > if lvds_div = 7 then > load_lvds<='1'; > else > load_lvds<='0'; > end if; > end if; > end process; > > -- generates serial data for one lvds-pair > lvds_1:process (lvds_tick) > begin > if load_lvds= '1' then > reg1 <= lvds1; > elsif rising_edge(lvds_tick) then > reg1 <= reg1((7-2) downto 0) & '0'; > end if; > lvds_1_in <= reg1(7 - 1); > end process;I suggest you could start with : http://www.alse-fr.com/English/Archive/VHDL_Coding_eng.pdf Your processes have many errors mentioned in this document. process 1 : missing reset on lvds_clk process 2 : you use both edges of the clock ? (do you need this ?) process 3 : wrong sensitivity list, maybe not synthesizable depending on the target technology (asynchronous load), lvds_1_in is clocked by both edges and negatively clock enabled load_lvds ?? Bert Cuzeau
Reply by ●April 29, 20052005-04-29
"info_" <"info_"@\\nospam_no_underscore_alse-fr.com> wrote in message news:Qauce.51988$Of5.33143@nntpserver.swip.net...> http://www.alse-fr.com/English/Archive/VHDL_Coding_eng.pdf > > Your processes have many errors mentioned in this document. > > process 1 : missing reset on lvds_clk > process 2 : you use both edges of the clock ? (do you need this ?) > process 3 : wrong sensitivity list, maybe not synthesizable > depending on the target technology (asynchronous load), > lvds_1_in is clocked by both edges and negatively clock enabled > load_lvds ?? >...and learn about 'case' statements. Syms.
Reply by ●April 29, 20052005-04-29
Hi Bert, thanks for looking at the code.> I suggest you could start with :> process 1 : missing reset on lvds_clkwhat do you mean by that? should I set lvds_clk to '0' during reset?> process 2 : you use both edges of the clock ? (do you need this ?)I think I need both edges, because there is no pause between two bytes, so the shift register needs to be loaded with the new data between the last and first bit. Or is there a better way (probably...)?> process 3 : wrong sensitivity list, maybe not synthesizable > depending on the target technology (asynchronous load), > lvds_1_in is clocked by both edges and negatively clock enabled > load_lvds ??I have changed this process... I have my new code attached. However I have read that case statements are better, because they can be implemented parallel... I will change that. I have put the two processes that do the lvds_clk and the pixel counting into one process. Is it generally better to have one big process? regards, Benjamin -- lvds_tick is a clock at 7x pixel-clock (140 MHz) -- lvds_clk is the clock for the lvds bus, same as pixel clock, but not symmetric -- like this for one pixel period: --___-- process (lvds_tick,screen_reset) begin if screen_reset='1' then lvds_div<=0; row_count <= 0; col_count <= 0; DTMG <= '1'; VSYNC <= '0'; HSYNC <= '0'; elsif rising_edge(lvds_tick) then if lvds_div = 0 then lvds_clk <='1'; elsif lvds_div = 2 then lvds_clk<='0'; elsif lvds_div = 5 then lvds_clk<='1'; end if; if lvds_div = 6 then lvds_div <= 0; ------------ if col_count = 1023 then DTMG <= '0'; HSYNC <= '1'; elsif col_count = 1035 then HSYNC <= '0'; end if; if col_count = 1075 then col_count <= 0; if row_count < 767 then DTMG<='1'; elsif row_count = 767 then VSYNC <= '1'; elsif row_count = 769 then VSYNC <= '0'; end if; if row_count = 783 then row_count <= 0; DTMG<='1'; else row_count <= row_count +1; end if; else col_count <= col_count +1; end if; ------------- else lvds_div <= lvds_div + 1; end if; end if; end process; -- this process generates the load impuls for the shift register below process (lvds_tick,screen_reset) begin if screen_reset='1' then load_lvds<='1'; elsif falling_edge(lvds_tick) then if lvds_div = 0 then load_lvds<='1'; else load_lvds<='0'; end if; end if; end process; -- generates serial data for one lvds-pair lvds_1:process (lvds_tick) begin if rising_edge(lvds_tick) then if load_lvds='1' then reg1 <= lvds1; else reg1 <= reg1((5) downto 0) & '0'; end if; end if; lvds_1_in <= reg1(6); end process;
Reply by ●April 30, 20052005-04-30
Symon wrote:> > ...and learn about 'case' statements. > Syms. >I take advantage of your post (and of the WeekEnd) for a question to the group. I often see newbies writing things like : case my_std_logic is when '0' => when '1' => when others => -- they HAVE to write this indeed !! end case; They seem to believe it would be a better (more efficient !) style than : if my_std_logic='1' then else end if; ????? Since I've seen this a number of times, I'm curious to know who is telling them this ? and why ? a book ? And when things get more complex, even more people seem to say that the case statement is a miracle... and would magically turn a "serial" computation into a "parallel" one, more efficient, that one has a priority while the other would not etc etc.... I usually hear the very same persons say they don't know what the '-' (don't care) is about ! I'm _not_ saying here that the case statement is never to be preferred, but the designer must be aware of the reality. The case statement in VHDL also has it's unfriendly sides (you often have to qualify the case expression like in "case A&B is" + there is also the locally static issue + no way to use std_match in a case + case must be complete, etc...). The Verilog case statement, associated with parallel case pragma, is more versatile (like with a constant selector). Even using the case statement (and don't cares), obtaining the minimal logic sometimes requires efforts (as for the one hot minimal decoder). My recommendation is to usually favor the style which provides the best expressiveness even at the (hypothetical) cost of a few gates. I quickly wrote the example below. Note that I didn't merge the cases or the ifs so different decoding logic can easily be tested. It is also possible to remove some case(s) (thanks to the default statements). Has anyone different views or experience to share ? --------------------------------------------------------- -- Will the "case" statement really change your life :-) ? -- -- Please let me know if there is one synthesis tool which -- does provide a different QOR for the two architectures below. -- I would use the case statement myself, but not in the -- hope of better QOR. library IEEE; use IEEE.std_logic_1164.all; use IEEE.numeric_std.all; Entity casetest is port ( A : in std_logic_vector (3 downto 0); Q : out std_logic_vector (3 downto 0) -- MSB not used (= 0) ); end; -- ------------------------------------ architecture RTL_case of casetest is begin process (A) begin case A is when x"0" => Q <= x"0"; when x"1" => Q <= x"1"; when x"2" => Q <= x"2"; when x"3" => Q <= x"2"; when x"4" => Q <= x"3"; when x"5" => Q <= x"3"; when x"6" => Q <= x"3"; when x"7" => Q <= x"3"; when x"8" => -- could use when others here Q <= x"4"; when x"9" => Q <= x"4"; when x"A" => Q <= x"4"; when x"B" => Q <= x"4"; when x"C" => Q <= x"4"; when x"D" => Q <= x"4"; when x"E" => Q <= x"4"; when x"F" => Q <= x"4"; when others => Q <= "----"; end case; end process; end architecture RTL_case; -- -------------------------------------------- architecture RTL_if of casetest is begin process (A) begin if A = x"0" then Q <= x"0"; elsif A = x"1" then Q <= x"1"; elsif A = x"2" then Q <= x"2"; elsif A = x"3" then Q <= x"2"; elsif A = x"4" then Q <= x"3"; elsif A = x"5" then Q <= x"3"; elsif A = x"6" then Q <= x"3"; elsif A = x"7" then Q <= x"3"; elsif A = x"8" then -- could put an "else" here Q <= x"4"; elsif A = x"9" then Q <= x"4"; elsif A = x"A" then Q <= x"4"; elsif A = x"B" then Q <= x"4"; elsif A = x"C" then Q <= x"4"; elsif A = x"D" then Q <= x"4"; elsif A = x"E" then Q <= x"4"; elsif A = x"F" then Q <= x"4"; else Q <= "----"; end if; end process; end architecture RTL_if;
Reply by ●April 30, 20052005-04-30
Hi Info, I have read here, that case is more parallel: http://www.vhdl-online.de/tutorial/deutsch/t_201.htm regards, Benjamin
Reply by ●April 30, 20052005-04-30
Benjamin Menk�c wrote:> Hi Info, > > I have read here, that case is more parallel: > > http://www.vhdl-online.de/tutorial/deutsch/t_201.htm > > regards, > BenjaminI became the Saint Thomas of RTL syntyhesis :-) So let's take the example from your University course. -ich verstehe ein bischen Deutsch- I just modified the names to avoid reserved keywords and added the proper types : -- "case" statement tests -- Bert Cuzeau library IEEE; use IEEE.std_logic_1164.all; use IEEE.numeric_std.all; Entity casetest2 is port ( A,B,C,Ad : in std_logic_vector (4 downto 0); Q : out std_logic_vector (4 downto 0) ); end; -- -------------------------------------------- architecture RTL_if of casetest2 is -- 12 LEs begin process (Ad,A,B,C) begin if unsigned(Ad) > 17 then Q <= A ; elsif unsigned(Ad) < 17 then Q <= B ; else Q <= C ; end if ; end process; end architecture RTL_if; -- ------------------------------------ architecture RTL_case of casetest2 is -- 12 LEs ?! begin process (Ad,A,B,C) begin case to_integer(unsigned(Ad)) is when 0 to 16 => Q <= B ; when 17 => Q <= C ; when others => Q <= A ; end case; end process; end architecture RTL_case; Both RTL views "look" different indeed, but... both styles end up with exactly the same number of LEs ! (used Quartus II's internal synthesis) and identical timing performance. With XST (ISE 6.3.03), the case version is 7 slices vs 6 slices for the "if" version. We may hope it's slightly faster, but I wouldn't bet on that either. So I still don't see any proof that the case statement is the (only) way to go. But I am _not_ saying that arithmetic operator inference is something not to care about !!! Quite the contrary ! With larger operators, I would double check, consider using only one magnitude comparator, etc... (given a _strong_ motivation for area or timing perf, otherwise I wouldn't care much and I would spend my time more productively). It's just that "coding style QOR" is a more complex issue than it may seem, and is often very tool-dependant. Even with a given tool, optimization options may impact the QOR. ...and I believe what I see by myself, and I always verify the big "we are the best" statements from the tools vendors ;-) Bert Cuzeau
Reply by ●April 30, 20052005-04-30
HI Bert, Intersting topic to talk about ! .> I often see newbies writing things like :> [Snipped]..............> ????? Since I've seen this a number of times, I'm curious > to know who is telling them this ? and why ? a book ?I am a newbie(still at academic level) but I'll not accept from a book without saying why.> to know who is telling them this ?Here is the list which I have read ..... 1) RTL Implementaion Guide by Jack Marshall (from tera systems Inc). ( I got this from synopsys SNUG group,check it if u have account ) 2) Coding style from synopsys 3) http://www.utdallas.edu/~shankars/teaching/ee5325/foils/lectures/lecture17.pdf (Slid number 17) 4) http://min.ecn.purdue.edu/~ee495d/Lectures/synthesis.pdf All say that case infers a parallel logic and if-elsif-else infers a proirity encoder structure ... If situation does'nt demand me for a prority logic Why shall I choose it ???> The case statement in VHDL also has it's unfriendly sides > (you often have to qualify the case expression like in > "case A&B is" + there is also the locally static issue > + no way to use std_match in a case + case must be complete, etc...).problem of qulifying expression has been addressed by vhdl committee and it is going be fixed soon. completness of case is an advantage to find the bugs.Lets take an example.... Take the exmaple you mentioned above> case my_std_logic is > when '0' => > when '1' => > when others => -- they HAVE to write this indeed !! > end case;>They seem to believe it would be a better (more efficient !) >style than : > if my_std_logic='1' then > else > end if;Suppose my_std_logic is a conrol signal which must be initialized properly after reset. I cannot see that with your if - else code . but by having others clause I could watch all uninitialized 'U' signals by case statement.> Even using the case statement (and don't cares), obtaining > the minimal logic sometimes requires efforts (as for the one > hot minimal decoder).I dont know partiuclarly about one hot minimal decoder but Synthesis tools say case statments are good for area opptimizations.>My recommendation is to usually favor the style which provides >the best expressiveness even at the (hypothetical) cost of a >few gates.I dont think if-else provide more " expressiveness " then case. For parallel code case is more "expressiveness". At the end merits of hardware counts wheather it is in the form of few gates. Dont under estimate the number of gates as it is know causing more static power. > Has anyone different views or experience to share ? Thank you very much for starting such an interesting topic. -- Mohammed A Khader.
Reply by ●April 30, 20052005-04-30
Hi, it would be interesting to know, if the if-style gets worse compared to the case-style, when we have a very long if-chain (10x elsif or so). regards, Benjamin
Reply by ●April 30, 20052005-04-30
Just a few extra comments : Mohammed A khader wrote:> All say that case infers a parallel logic and if-elsif-else infers > a proirity encoder structure ...They don't quite say this (or they'd lie). "If ... elseif" has an implied priority (first test true -> next tests are not taken)... but that makes no sense when you describe a truth table ! (as in my first encoder example) Whatever the means to describe behaviorally the truth table, it will end up the same, and by way of logic reduction it will lead to implementations that may or may not be similar depending on synthesis tools optimization goals, constraints, internal logic reduction algorithm, etc... Things may become different when complex operators are inferred. The BIG difference is that, from a synthesis perspective, describing a truth table isn't the same as describing higher level structures ! What I mean is be wary of general rules about synthesis. There are more than one tool, and tens of years of research behind them... I think it's pretty dangerous to say "this does infer that". Or you have to be damn accurate : given code snippet, given tool, given version, given technology, given constraints, etc etc> problem of qulifying expression has been addressed by vhdl committee > and it is going be fixed soon.1. I didn't mean it was a problem ! You just need to know the language. Qualified expressions are just often unknown to newbies, but they are extremely useful. write (L,"hello"); for example. There's no problem in writing "case A&B" (a qualified expr does it). It's also possible to get directly the two MSBs of A + B (vectors). One just needs to know a bit more than the basics of VHDL. 2. What do you mean by "has been addressed..."? I don't want to start another controversy, but VHDL200X isn't out of the woods. We all just hope it will happen (out of IEEE and then into our tools) before the EDA community majority has switched to SystemVerilog. But there is absolutely no real need of a better VHDL for simple designs.> completness of case is an advantage to find the bugs.Lets take an > example....It's MUCH easier to follow good coding rules which ensure that the bad situation you mention will never happen in your design that trying to test every signal again 'U' ! If you want to do this, be consistent and write a test for every numeric vector, making sure it doesn't include any 'U'. Not cool. Simple gross mistakes are easier to prevent than to detect. I think this is the same with std_ulogic, which use was supposed to help detect multiple drivers situations (it did). But there are other ways to check this and so many other bad things are to be tested at synthesis that everybody has dropped now this unresolved type and obsolete style.> I dont know partiuclarly about one hot minimal decoder but > Synthesis tools say case statments are good for area opptimizations.It's called hearsay I think. Then, why doesn't it show up in the simple examples I gave ? (XST 6.3.03i creates in fact a larger design with the "case" version) I have reproduced below an old example that I sometimes used in my courses. I kept it simple minded (no fancy function). Why don't all synthesis tools give the simple OR gates ? - The case version "looks" nicer, but check the gates count (with your tool). - In the "if" solution : do you see a priority after synthesis ?? Chances are your synthesis tool will produce three times larger solution with the case than with the if. Don't believe ppt slides (nor me) !> Dont under estimate the number of gates as it is know > causing more static power.0. How do you know you have "saved" some gates and achieved and an optimal solution ? What do you compare against ? In the example below, was 15 LUTs acceptable or not ? (without the comments saying it was not). 1. The case doesn't always produce less gates, sometimes the contrary as proved here. I try to not underestimate anything ! I just check by myself as much as I can and I encourage you, if you're sensitive to gate count and QOR, to always verify and never simply "assume". 2. If design could be made significantly smaller by avoiding "ifs", then don't you think the Synthesis tools would automatically do the translation internally ? (they do, this and many other tricks) 3. There is one or two orders of magnitude higher potential gain by smart implementation and good design know-how : being a smart architect pays ! 4. Most synthesis tools are smarter and smarter, so a good understanding of your specific tool is also a good investment. 5. FPGAs and ASICs are two different worlds. What applies to one not necessarily applies to the other. Synthesizing to LUTs and to GATEs isn't the same. Don't forget Synopsys is an ASIC company. Driving DC ins't the same as doing FPGA synthesis. 6. Significant Power consumption reduction requires other techniques than using case instead of if (supposing that there is any gain at all doing so). 7. Gate count is definitely less and less an issue, even in the ASIC field ! Challenges have moved, and old books don't reflect this. My experience is : Synthesis tools are often smarter than the designer thinks, but there are also sometimes doing some apparently "stupid" things (at least looking like such for unexperienced designers). Just try Q <= A + B - A - B; -- unsigned vectors. And please do *NOT* avoid the case statement when it is appropriate !!! Again, HDL code should be as simple, clear and expressive as possible, easy to understand and maintain, and not error-prone. I assign a higher priority to these criteria in our designs. After a good ground learning, learn by trying. This is fun anyway. You must end up "thinking" like your synthesizer, then you'll be real efficient. Bert Cuzeau "Let's save the poor IF from damnation !" Commitee 12, Gate(s) Lane Mux Island :-) (donations accepted, FFs* and LUTs only) * FlipFlops, not French Francs ---------------------------- -- HOTDECOD.VHD -- ------------------------------------- -- One-Hot Decoder -- ------------------------------------- -- ALSE. http://www.alse-fr.com/english -- This design must be synthesized as 3 x OR gates (4-inputs) -- Any extra logic is unnecessary. -- library IEEE; use IEEE.std_logic_1164.all; -- ------------------------------------- Entity HOTDECOD is -- ------------------------------------- port ( A : in std_logic_vector(0 to 7); Q : out std_logic_vector(2 downto 0) ); end; -- ------------------------------------- Architecture RTL_case of HOTDECOD is -- ------------------------------------- -- check how many LUTS for this one... -- How do you code this as a generic size decoder ? begin process (A) begin case A is when "00000001" => Q <= "111"; when "00000010" => Q <= "110"; when "00000100" => Q <= "101"; when "00001000" => Q <= "100"; when "00010000" => Q <= "011"; when "00100000" => Q <= "010"; when "01000000" => Q <= "001"; when "10000000" => Q <= "000"; when others => Q <= "---"; -- don't care end case; end process; -- Note : the "case" above is auto-full and auto-parallel in Verilog sense. -- Quite obviously, one solution is : -- Q2 = A0 or A1 or A2 or A3 -- Q1 = A2 or A3 or A6 or A7 -- Q0 = A1 or A3 or A5 or A7 end RTL_case; -- ------------------------------------- Architecture RTL_if of HOTDECOD is -- ------------------------------------- -- and how many LUTS for this one ? -- Isn't this description easier to rewrite under -- the form of a (size-independant) function ? begin process (A) begin if A(7) = '1' then Q <= "111"; elsif A(6) = '1' then Q <= "110"; elsif A(5) = '1' then Q <= "101"; elsif A(4) = '1' then Q <= "100"; elsif A(3) = '1' then Q <= "011"; elsif A(2) = '1' then Q <= "010"; elsif A(1) = '1' then Q <= "001"; elsif A(0) = '1' then Q <= "000"; else Q <= "---"; -- don't care end if; end process; -- do you see a "priority" in the synthesized result ??? end RTL_if;