{"id":667,"date":"2015-05-04T08:12:11","date_gmt":"2015-05-04T16:12:11","guid":{"rendered":"http:\/\/www.beyond-circuits.com\/wordpress\/?page_id=667"},"modified":"2016-09-23T14:44:42","modified_gmt":"2016-09-23T22:44:42","slug":"discussion","status":"publish","type":"page","link":"https:\/\/www.beyond-circuits.com\/wordpress\/tutorial\/tutorial18\/part2\/discussion\/","title":{"rendered":"Tutorial 18, Part 2: Discussion"},"content":{"rendered":"

Spoiler alert!<\/h1>\n

Don’t read any further unless you have gotten your design working for part two of tutorial 18. Well, at least the spirit of tutorial 18 is that you should really try to work it out on your own. This section will walk through some code examples in detail.<\/p>\n

The literal implementation<\/h1>\n

You can go to the block diagram and just literally code everything that appears there. First we have the module declaration:<\/p>\n

`timescale 1ns\/1ns\r\nmodule i2s_receive2a\r\n  (\r\n   input sck,\r\n   input ws,\r\n   input sd,\r\n   output reg [31:0] data_left,\r\n   output reg [31:0] data_right\r\n   );\r\n<\/code><\/pre>\n
Next, we need to implement the two flip-flops which delay the WS<\/em> signal by a clock. The part of the diagram that implements this is shown to the right.\"I2S_receiver2_detail1\"<\/a> We will of course need two register values. One is called wsd<\/em>, presumably for WS<\/em> delayed. The second has no name in the figure and I just called it wsdd<\/em>, for wsd<\/em> delayed. I have also chosen to initialize wsd<\/em> to zero. The code for this is really quite simple.<\/p>\n
  reg wsd = 0;\r\n  always @(posedge sck)\r\n    wsd <= ws;\r\n\r\n  reg wsdd;\r\n  always @(posedge sck)\r\n    wsdd <= wsd;\r\n<\/code><\/pre>\n
We also need a combinational XOR of wsd<\/em> and wsdd<\/em> and call that wsp<\/em>. I do that like this:<\/p>\n
wire wsp = wsd ^ wsdd;<\/code><\/pre>\n
This uses the Verilog exclusive OR operator ^<\/code>. We could also write it like this:<\/p>\n
wire wsp = wsd != wsdd;<\/code><\/pre>\n
This might be a little more expressive of our intent. We want a single clock pulse when the two signals are different. Ne\"I2S_receiver2_detail2\"xt, we see that we need 32 enable signals for the flip-flops that capture the SD<\/em> data input. Actually, if you look at the counter, you see that we need an additional one for when the counter is 32 to keep it from counting further.<\/p>\n
Let’s declare those:<\/p>\n
wire en0;\r\nwire en1;\r\nwire en2;\r\n<\/code>...\r\nwire en31;\r\nwire en32;\r\n<\/code><\/pre>\n
\"I2S_receiver2_detail2\"Next, we’ll\u00a0focus on the counter. It needs to count from zero to 32. We declare this to be a 6-bit register. Remember, six bits are needed to represent 0 to 32. Here is the counter. Also notice that it is clocked on the negative edge of SCK:<\/em><\/p>\n
  reg [5:0] counter;\r\n  always @(negedge sck)\r\n    if (wsp)\r\n      counter <= 0;\r\n    else if (!en32)\r\n      counter <= counter+1;\r\n<\/code><\/pre>\n
Then we need to also assign to the enable signals. Here is that code:<\/p>\n
  assign en0 = counter == 0;\r\n  assign en1 = counter == 1;\r\n  assign en2 = counter == 2;\r\n  <\/code>...\r\n  assign en31 = counter == 31;\r\n  assign en32 = counter == 32;\r\n<\/code><\/pre>\n
Next, let’s focus on the register that captures the data. In the previous example, the register was called shift<\/em>. We will stick with that name, even though it’s no longer technically a shift register. It still kind of functions that way, so I think the name still makes sense. \"I2S_receiver2_detail3\"Notice that EN0 enables the most significant bit to be written. and EN31 enables the least significant bit. That means we naturally want the MSB of shift to be bit zero and the LSB of shift to be bit 31. So we declare shift as follows.<\/p>\n
reg [0:31] shift;<\/code><\/pre>\n
Also, notice that bit zero is different from the other bits. It has no reset input while the others do. Let’s code that bit first:<\/p>\n
always @(posedge sck)\r\n  if (en0)\r\n    shift[0] <= sd;\r\n<\/code><\/pre>\n
And for the other registers which are reset by wsp<\/em>, we have the following code:<\/p>\n
always @(posedge sck)\r\n  if (wsp)\r\n    shift[1] <= 0;\r\n  else if (en1)\r\n    shift[1] <= sd;\r\n  <\/code>...\r\n  always @(posedge sck)\r\n    if (wsp)\r\n      shift[31] <= 0;\r\n    else if (en31)\r\n      shift[31] <= sd;\r\n<\/code><\/pre>\n
The final part of the design is to write to the data_left<\/em> and data_right<\/em> output registers. Here is the relevant portion of the diagram. Basically the wsp<\/em> signal indicates that one of the two registers should be written and wsd<\/em> indicates which one should be written. When wsp<\/em> is high, the left register is written, and when wsp<\/em> is low, the right register is written.\"I2S_receiver2_detail4\"<\/a><\/p>\n
Here is the code for that:<\/p>\n
always @(posedge sck)\r\n  if (wsd && wsp)\r\n    data_left <= shift;\r\n\r\nalways @(posedge sck)\r\n  if (!wsd && wsp)\r\n    data_right <= shift;\r\n<\/code><\/pre>\n
Don’t forget the endmodule<\/code> to close it off.<\/p>\n
Thoughts<\/h1>\n
So there you have it. 296\u00a0lines of Verilog code to implement our I2S receiver. Are you happy with it? Probably half of all FPGA designers would pat themselves on the back for a job well done and go on to the next assignment. But really, didn’t you think while typing all those lines of code that it was really repetitive? Didn’t it seem like you should have a computer program to help you type all those lines of code which are almost all the same? Those are signs that we can do a better job.<\/p>\n
The Rev B Design<\/h1>\n
OK, well first we have 33 nets called enn<\/strong><\/em>. That is just silly. The first thing we need to do is to make that be a vector. Let’s change the declaration to this.<\/p>\n
wire [32:0] en;<\/code><\/pre>\n
We also need to change all the places that have enn<\/b><\/em> to en[n<\/b>]<\/em>. By my count that gets us down to 264 lines. Still not too much in terms of removing the repetitive nature of things.<\/p>\n
The Power of for<\/em> Loops<\/h1>\n
Many logic designers don’t understand that you can use Verilog for<\/strong> loops in synthesized RTL code. There is just one simple restriction on their use. The loop you write has to be able to be completely unrolled. What that means is that the loop essentially needs to be run when the code is parsed. It has to take a fixed number of iterations through the loop. The number of iterations has to be known when the code is synthesized.<\/p>\n
The Rev C Design<\/h1>\n
Let’s attack those 33 lines where we make the assignment to the enable bits. First, the enable assignments were continuous assignments to en<\/em>\u00a0which was declared as a wire<\/strong>. We first need to change that to a reg so we can assign to it in a combinational always block.<\/p>\n
reg [32:0] en;<\/code><\/pre>\n
Now, if we don’t use the loop, we would need to code the assignments to EN<\/em> like this.<\/p>\n
always @(*)\r\n  en[0] = count == 0;\r\n\r\nalways @(*)\r\n  en[1] = count == 1;\r\n\r\n...\r\n\r\nalways @(*)\r\n  en[32] = count == 32;\r\n<\/code><\/pre>\n
Even more lines of code! But with a for<\/b> loop we can code them like this.<\/p>\n
integer i;\r\nalways @(*)\r\n  for(i=0; i<=32; i=i+1)\r\n    en[i] = counter == i;\r\n<\/code><\/pre>\n
That’s it. A big space savings. And it has some other advantages we will discuss later. Let’s attack those assignments to shift<\/em> while we’re at it.<\/p>\n
Bit zero of shift has no reset so it is different. We will leave that alone. But bits 1 through 32 will get written using a for<\/strong> loop.<\/p>\n
  always @(posedge sck)\r\n    for (i=1; i<32; i=i+1)\r\n      if (wsp)\r\n\tshift[i] <= 0;\r\n      else if (en[i])\r\n\tshift[i] <= sd;\r\n<\/code><\/pre>\n
Note that we don’t need to declare i<\/em> again. We are using the same i<\/em> variable as the previous for<\/strong> loop.<\/p>\n
This reduces our design down to 44 lines of Verilog code. That’s a real improvement. But our improvements aren’t over yet.<\/p>\n
Parameterization<\/h1>\n
Notice that the width of our module is hardwired to 32 bits. In the first two designs that is not really an issue because our code was hardwired to 32 bits as well. You couldn’t change the number without adding lines of code to the enable and shift assignments. But with the improvements in version C we now just have numbers scattered around the code which we can<\/em> derive from a parameter.<\/p>\n
In general, I believe you should try to always write your code such that it is parameterized. This more than any other rule will guide you to writing good Verilog code. You should go ahead and change the design to take a parameter called width<\/em> with a default value of 32.<\/p>\n
Remember to use the $clog2<\/strong> function to determine the number of bits needed for the counter. It might take a little thought to get the expression for the width of the counter exactly right. For a width of 32 the counter should be six bits wide. But if width is 31 the counter should only be five bits. If width is 63 then the counter should still only be six bits.<\/p>\n
We will call this version D of the design. But there are more improvements to come.<\/p>\n
A Better Encoder<\/h1>\n
If you run a simulation of the previous design and look at the counter<\/em> value and the en<\/em> value you will notice that en<\/em> is just two raised to the counter<\/em>. In other words they have the following relationship.<\/p>\n
\n\r\n\r\n
counter<\/em><\/th>\nen<\/em><\/th>\nen<\/em>(binary)<\/th>\n<\/tr>\n<\/thead>\n
0<\/td>\n1<\/td>\n0000000012<\/sub><\/td>\n<\/tr>\n\r\n
1<\/td>\n2<\/td>\n0000000102<\/sub><\/td>\n<\/tr>\n\r\n
2<\/td>\n4<\/td>\n0000001002<\/sub><\/td>\n<\/tr>\n\r\n
3<\/td>\n8<\/td>\n0000010002<\/sub><\/td>\n<\/tr>\n\r\n
4<\/td>\n16<\/td>\n0000100002<\/sub><\/td>\n<\/tr>\n\r\n
5<\/td>\n32<\/td>\n0001000002<\/sub><\/td>\n<\/tr>\n\r\n
6<\/td>\n64<\/td>\n0010000002<\/sub><\/td>\n<\/tr>\n\r\n
7<\/td>\n128<\/td>\n0100000002<\/sub><\/td>\n<\/tr>\n\r\n
8<\/td>\n256<\/td>\n1000000002<\/sub><\/td>\n<\/tr>\n<\/tbody><\/table><\/div>\n
A common way to implement a decoder like this is to just shift 1 left by the count<\/em> amount. So we can replace our for<\/strong> loop with just the following.<\/p>\n
always @(*)\r\n  en = 1<<counter;\r\n<\/code><\/pre>\n
Simplifying the shift<\/em> Assignment<\/h1>\n
Getting back to the D version of the design, you may recall that we had two always blocks to assign to the shift<\/em> variable. One used a for loop to assign to bits 1 through 31 because they needed a reset. And a separate one to assign to bit zero where there was no reset. Here is the code for that again.<\/p>\n
  reg [0:width-1] shift;\r\n  always @(posedge sck)\r\n    if (en[0]) shift[0] <= sd;\r\n\r\n  integer i;\r\n  always @(posedge sck)\r\n    for (i=1; i<width; i=i+1)\r\n      if (wsp)\r\n\tshift[i] <= 0;\r\n      else if (en[i])\r\n\tshift[i] <= sd;\r\n<\/code><\/pre>\n
We can actually merge these two always blocks. If you look at a timing diagram from an existing simulation you will see that wsp is only true then counter is equal to zero. Why? Because wsp actually resets counter. This means when wsp<\/em> is high, en<\/em>[0] is also high. If another bit of en<\/em> is set, then wsp<\/em> must be low. This means we can remove the else<\/strong> from the second always block. It also means that we can extend the for loop to include index zero now. In the zero case we reset shift<\/em>[0] in the first if<\/strong>, but we immediately write sd<\/em> into it with the second if<\/strong>. This has the effect of not resetting it at all. So the new code looks like this.<\/p>\n
  reg [0:width-1] shift;\r\n  integer i;\r\n  always @(posedge sck)\r\n    for (i=0; i<width; i=i+1)\r\n      begin\r\n\tif (wsp)\r\n\t  shift[i] <= 0;\r\n\tif (en[i])\r\n\t  shift[i] <= sd;\r\n      end\r\n<\/code><\/pre>\n
Another Option for the Enable<\/h1>\n
Instead of shifting 1 left there is another way we can implement the en<\/em> signal. Let’s change it back into a reg signal and think about the following. As we showed in the last section if you write to the same bit multiple times in an always<\/strong> block it is the last write that actually takes effect. We can make an always<\/strong> block which sets the en<\/em> value to zero and then just sets the bit indexed by counter<\/em> to one. That code looks like this.<\/p>\n
  always @(*)\r\n    begin\r\n      en = 0;\r\n      en[counter] = 1;\r\n    end\r\n<\/code><\/pre>\n
The Biggest Leap<\/h1>\n
Now let’s out the thinking from the last insight to work for the final thoughts on setting the shift<\/em> value. First, let’s look at what en is really telling us. Bit n<\/em> of en<\/em> is true if counter<\/em> equals n<\/em>. So, let’s rewrite the always block where we assign shift to just use the expression for the active bit in en<\/em>.<\/p>\n
  reg [0:width-1] shift;\r\n  integer i;\r\n  always @(posedge sck)\r\n    for (i=0; i<width; i=i+1)\r\n      begin\r\n\tif (wsp)\r\n\t  shift[i] <= 0;\r\n\tif (counter == i)\r\n\t  shift[i] <= sd;\r\n      end\r\n<\/code><\/pre>\n
Next, let’s factor the reset value for shift to outside the for<\/strong> loop.<\/p>\n
  reg [0:width-1] shift;\r\n  integer i;\r\n  always @(posedge sck)\r\n    begin\r\n      if (wsp) shift <= 0;\r\n      for (i=0; i<width; i=i+1)\r\n\tif (counter == i)\r\n\t  shift[i] <= sd;\r\n    end\r\n<\/code><\/pre>\n
Now, if you look at that for loop it should look like one of the dumbest loops ever written. Why is it a loop at all? Let’s fix that once and for all.<\/p>\n
  reg [0:width-1] shift;\r\n  always @(posedge sck)\r\n    begin\r\n      if (wsp) shift <= 0;\r\n      shift[counter] <= sd;\r\n    end\r\n<\/code><\/pre>\n
Notice that we don’t even need the en<\/em> signal at all any more.<\/p>\n
A Very Subtle Problem<\/h1>\n
There is a very subtle problem with this design however. It has to do with synthesis. What happens when counter<\/em> is equal to 32? Well, shift<\/em> only has bits 0 through 31. So, from an RTL perspective we are assigning to a nonexistent bit. Unfortunately, there is a bug in the Vivado synthesis tool through at least version 2015.1 where this is not what happens. Because of this bug, the counter<\/em> value is essentially truncated and will set bit zero of shift<\/em> when counter<\/em> is 32.\u00a0You must be extremely careful when indexing into bit vectors and arrays (RAMs) that you don’t exceed the bounds of the vector or array. The synthesis tool will happily wrap those accesses such that they actually modify the vector or array.<\/em><\/span><\/p>\n
You can test this by running the simulation using the synthesized results. You will see that in about half of the cases where the sample size is greater than 32 bits the test will fail.<\/p>\n
So we must modify that final always block just a little bit. Here is a fix for the issue.<\/p>\n
  reg [0:width-1] shift;\r\n  always @(posedge sck)\r\n    begin\r\n      if (wsp) shift <= 0;\r\n      if (counter < width) shift[counter] <= sd;\r\n    end\r\n<\/code><\/pre>\n
This will synthesize correctly since we have explicitly added the check which the synthesis tool should have done automatically.<\/p>\n
Synthesis<\/h1>\n
Speaking of synthesis, I have synthesized each of these designs to see how they compared size wise. Here are the results.<\/p>\n
\n\r\n\r\n
module<\/th>\n cells<\/th>\n ports<\/th>\n nets<\/th>\n luts<\/th>\n registers<\/th>\n<\/tr>\n<\/thead>\n
i2s_receive2a<\/td>\n218<\/td>\n67<\/td>\n223<\/td>\n42<\/td>\n104<\/td>\n<\/tr>\n\r\n
i2s_receive2b<\/td>\n241<\/td>\n67<\/td>\n246<\/td>\n54<\/td>\n104<\/td>\n<\/tr>\n\r\n
i2s_receive2c<\/td>\n218<\/td>\n67<\/td>\n223<\/td>\n42<\/td>\n104<\/td>\n<\/tr>\n\r\n
i2s_receive2d<\/td>\n218<\/td>\n67<\/td>\n223<\/td>\n42<\/td>\n104<\/td>\n<\/tr>\n\r\n
i2s_receive2e<\/td>\n218<\/td>\n67<\/td>\n223<\/td>\n42<\/td>\n104<\/td>\n<\/tr>\n\r\n
i2s_receive2f<\/td>\n230<\/td>\n67<\/td>\n235<\/td>\n48<\/td>\n104<\/td>\n<\/tr>\n\r\n
i2s_receive2g<\/td>\n230<\/td>\n67<\/td>\n235<\/td>\n48<\/td>\n104<\/td>\n<\/tr>\n\r\n
i2s_receive2h<\/td>\n241<\/td>\n67<\/td>\n246<\/td>\n54<\/td>\n104<\/td>\n<\/tr>\n\r\n
i2s_receive2*<\/td>\n222<\/td>\n67<\/td>\n227<\/td>\n44<\/td>\n104<\/td>\n<\/tr>\n\r\n
i2s_receive2<\/td>\n230<\/td>\n67<\/td>\n235<\/td>\n48<\/td>\n104<\/td>\n<\/tr>\n<\/tbody><\/table><\/div>\n
The design with the asterisk is the final design without the bounds check. The final design is a little larger, but I like the RTL code. It is pretty clean with no for loops and it is quite short. But it is not the smallest in terms of area.<\/p>\n
I have linked to each of the designs below.<\/p>\n