cancel
Showing results for 
Show  only  | Search instead for 
Did you mean: 
andrewcb
Explorer
Explorer
1,553 Views
Registered: ‎07-03-2008

Adding AXI Write wait states

Hi,

I'm sure this should be easy, but I can't get it to work. I am trying to add two wait-states to an AXI write in the auto generate AXI IP created by Vivado 2019.1.

I tried to delay AXI_WREADY but two clock, but it's obvoiusly not that simple. I have also struggled to find information on this, there must be loads out there?

I'd appreciate any help.

Thank-you

Andrew

Tags (3)
0 Kudos
20 Replies
adem369
Contributor
Contributor
1,542 Views
Registered: ‎02-18-2019

You can and wvalid signal with an another signal which goes low 2 clock cycle after a successfull write operation. This should work.

 

0 Kudos
dgisselq
Scholar
Scholar
1,529 Views
Registered: ‎05-21-2015

@andrewcb ,

I wouldn't recommend adding wait states to Xilinx's demo AXI logic.  Indeed, I wouldn't recommend using Xilinx's demo logic at all.  It's been buggy since 2016 and in spite of Xilinx's efforts to fix it it's known for hanging your logic when you aren't expecting it.  (Example: Adjust your microblaze or ARM software, and then your AXI bus fails--you'll be left believing the software is at fault.)  Even when Xilinx's demo works, it gets at best 33% throughput on writes, and it doesn't follow ARM's AXI performance guidelines at all.   (i.e. it holds xREADY low until VALID shows up--ARM recommends keeping READY high if possible.)

I'd recommend starting with this demonstration slave instead.  It's actually fairly easy to work with, can achieve 100% throughput, and once you get past the skid buffers, you can add whatever delays you want to the combinatorial ready signal.  See these lines for an example.

Dan

0 Kudos
andrewcb
Explorer
Explorer
1,521 Views
Registered: ‎07-03-2008

Thank-you @adem369 , it's definitely not that simple! The default Xilinx code uses other address flags that need to be taken care off, as I've found out!

Thanks for your input @dgisselq , I'll have to brush up on my Verilog for that. I'd really like to fix what I have, this code has been fine under 2015.4 but the bus timings are different in 2019.1 and I can't get the design to work - I don't really want to start from scratch.

I tried using the AXI Veriifcation IP, but even the example simulations don't work

Andrew

0 Kudos
adem369
Contributor
Contributor
1,502 Views
Registered: ‎02-18-2019

I dont think that you need to take care of that flag because when you modify wvalid signal, it behaves like the master delays write operation 2 clock cycle after each write. 

0 Kudos
dgisselq
Scholar
Scholar
1,493 Views
Registered: ‎05-21-2015

@andrewcb ,

I'd be glad to help you fix what you have if you'd be willing to share it here.

Dan

0 Kudos
andrewcb
Explorer
Explorer
1,481 Views
Registered: ‎07-03-2008

Hi @dgisselq that is very kind of you.
I have included the code. On line 452 I tried to prevent axi_wready going high for two clocks, bus this mod completely messes things up.
We had a contractor working for us a while back who used an ILA to monitor signal and make something work, which meant the addreaa and data channels were no longer separate - I think I removed most of that code.
Thank-you for your time.
It wouldn't be so bad if I could get a test bench running

The code just decodes 3 memory regions and outputs 3 Rd and Wr strobes plus Addr and Data.

I beleive the original code was generated by ISE 14.5.

Andrew

0 Kudos
andrewcb
Explorer
Explorer
1,415 Views
Registered: ‎07-03-2008

I managed to get the AXI Validation running with my IP


For me it's the  "axi_awv_awr_flag" not being correctly cleared that's causing the trouble.

Andrew

0 Kudos
andrewcb
Explorer
Explorer
1,413 Views
Registered: ‎07-03-2008

I think line 381 should be "axi_wready" not "axi_awready" ?
0 Kudos
dgisselq
Scholar
Scholar
1,395 Views
Registered: ‎05-21-2015

@andrewcb ,

That's only one of *many* bugs in this design.  The design as a whole looks very similar to this one I examined earlier, copying a lot of Xilinx's bugs along the way.  Don't be surprised if your bus locks up hard when using this core, as it is far from AXI compliant.  I was hoping to have a bug list for you by now, but a power outage got in the way--so I'm still working on it.

Dan

0 Kudos
andrewcb
Explorer
Explorer
1,389 Views
Registered: ‎07-03-2008

Thank-you @dgisselq,


I have found one or two issues myself, although it is difficult to get the AXI Verification IP to generate the signals in the same order we are seeing in the FPGA.
It's not easy to see from these two plots, the first one is the "normal" transaction cycle (the one we normally see) and that works fine. The second one happens 1 in 10 times and this is when things go wrong, the data from the previous first write is used in the second write...

andrewcb_0-1604337838300.jpeg

andrewcb_1-1604337856719.jpeg

 

 

0 Kudos
andrewcb
Explorer
Explorer
1,373 Views
Registered: ‎07-03-2008

Doh, just spotted the wrong write address signal is used in the address decode!
0 Kudos
andrewcb
Explorer
Explorer
1,367 Views
Registered: ‎07-03-2008

Although it works in our FPGA hardware - kind of.
In the FPGA the MASTER keeps S_AXI_WADDR constant (valid) even after the slave asserts AXI_AWREADY.
The Validation IP the address is taken away when AXI_AWREADY is asserted.
So my IP will fail validation but work in hardware
0 Kudos
dgisselq
Scholar
Scholar
1,363 Views
Registered: ‎05-21-2015

@andrewcb ,

I've counted nine bugs so far and I'm not yet at the bottom of the barrel.

  1. AWID is not registered.  Once AWVALID && AWREADY have been true, AWID is free to change to anything it wants to be while you are processing the burst.  However, this design--in what appears to be a copy of Xilinx's bug--sets BID combinatorially to AWID even after AWID is no longer valid.  This might cause you to return a BID for a packet you haven't yet received.  Check out Fig. 10 in the link I sent you to above for a picture of this.
  2. ARID has the same problem.
  3. AWLEN isn't registered either.  Yet your design depends upon the value of AWLEN when counting how many items are in the burst--possibly depending (erroneously) upon a subsequent bursts AWLEN value.
  4. ARLEN has the same problem.  This will cause RLAST to be generated erroneously.
  5. The design adjusts the axi_awv_awr_flag based upon WLAST && WREADY without also checking WVALID.  The problem is that if WVALID isn't also true, then WLAST is a don't care.  This might cause you to return a response early, or lock up on subsequent data requests.
  6. This design can't handle both read and write bursts concurrently, judging by the fact that you only have one WrRd_Addr which gets applied for both read and write channels.  This isn't a problem in itself.  The problem is that design will allow both read and write bursts to take place concurrently, possibly causing any reads to be applied to the write address.
  7. RDATA is not registered, but rather read directly from the external channel.  This allows RDATA to potentially change while the R* channel is stalled (RVALID && !RREADY)--a protocol violation which might break some masters.
  8. This design cannot handle back pressure on BVALID && !BREADY..  In the presence of back pressure, the design will drop packets--causing the bus to hang.
  9. You only update WrRd_Addr when the burst is first requested, but not mid-burst.  The address should change mid-burst as the read or write beats take place.

Unfortunately, I haven't really gotten to your delay issue--since you've had so many other bugs within the logic.  I'm sure it doesn't help that the Xilinx code you copied from had many of these bugs in it initially, nor will it help to know that Xilinx has yet to fix all of these bugs in their demo code as of even Vivado 2020.1.  Still, I managed to generate this trace and ... it doesn't look like I think you would want it to--especially since the second write besat isn't getting the delay you had requested applied to it.

write-cover.png

Yours,

Dan

0 Kudos
andrewcb
Explorer
Explorer
1,349 Views
Registered: ‎07-03-2008

Thanks for your help Dan.
I'll have a more detialed read of your webpage, which has many interesting findings.

Andrew

0 Kudos
dgisselq
Scholar
Scholar
1,110 Views
Registered: ‎05-21-2015

@andrewcb ,

I'm not sure I get this.  You want to add wait states to the writes.  Why?  Does each write operation take two extra clocks?  Why not pipeline these writes, so that you can return to the 100% throughput AXI was designed for?  Can your design not handle 100% throughput?

If your design can't handle 100% throughput, then why would it allow something like this trace:

write-cover-bursts.png

If you could pipeline write transactions, so that you could perform a trace like the one above (which was generated by your core) ... then why delay the writes at all?  Why not just accept them as fast as the core can allow?  Each write data value could enter a downstream pipeline and you could return BVALID even before the write operation is complete.  I mean, holding back BVALID makes sense ... if either you have synchronization requirements or you are waiting on a particular BRESP return.  In this design, however, BRESP will only ever be 2'b00.  Worse, because you only allow a single outgoing address, you (should) already be synchronized and so don't need to wait for that either.

Looking at the read side,

read-cover-bursts.png

I'd have to ask the same question: Can't you pipeline reads?  Sure, the AXI protocol gets in the way a bit, but a read that takes two clock cycles should still be able to be accomplished with 100% duty cycle.  You'd just need to handle a FIFO count going into the read pipeline, and then place the results into a FIFO at the end of the pipeline.  That FIFO would then feed RVALID, RID, RDATA, and RLAST.

If you want an example of this last architecture, consider this one.  In that example there's a read pipeline which takes a couple of clocks, gathers the read result from one of (potentially many) sources, and places the result into a return buffer suitable for driving the R* channel.

Perhaps I have a bit of a bias.  I tend to figure you bought your FPGA for a reason, and you are using AXI for a reason.  The most common such reason is high performance.  If that's your reason, then what you want is throughput--not throttling.  You just need to ... figure out how to get there from here, and Xilinx's examples don't really help you get there.

One other point: the traces I've been generating above, together with the bugs, are all due to using a tool known as SymbiYosys to formally verify your design.  Formal property verification failures generate traces illustrating bugs.  Similarly, the two traces above are formal property successes illustrating the throughput that your design can (currently, as posted above with minor bug fixes) achieve.

Dan

0 Kudos
andrewcb
Explorer
Explorer
947 Views
Registered: ‎07-03-2008

Hi Dan,

The reason for the 'slow' write (and read) is that WDATA goes to approx 10000 loads and we added a multicycle clock to this data path of 3 clocks to help routing and meet timing, (I think we could make it two now).

We used AXI because we use the Microblaze processor.

Speeding up the design would be a good thing, but high through-put is not our biggest issue and we are not in a position to redisegin the interface at the moment. It is what it is and it will have to stay that way until next year, unless there are some simple low risk modifications that can be made.

Last night's build (with some modifications made in response to your comments) met timing and seems to work, we have several projects on hold at the moment due to this issue, so as soon as management hear it works, they will want to test and ship product - hope they are not reading this!!!

Andrew

0 Kudos
dgisselq
Scholar
Scholar
912 Views
Registered: ‎05-21-2015

@andrewcb ,

If that's the case, then the WREADY logic you posted above is broken as illustrated by the write trace I posted previously.  Don't forget to fix that one as well then.

Dan

0 Kudos
andrewcb
Explorer
Explorer
884 Views
Registered: ‎07-03-2008

Hi @dgisselq ,

Yes I fixed the WREADY

It's still not perfect though.....

Andrew

 

0 Kudos
andrewcb
Explorer
Explorer
877 Views
Registered: ‎07-03-2008

So apart from the fix to WREADY, which I have already corrected (now that I hve the AXI Validation IP working), and the fact that the read bus is not registered (which is OK, because outside of this block read data is not allowed to changed while read is active), all of the other points in your list are bugs copied over from the original Xilinx code?

I don't feel quite so bad now

Something is still not quite right though.... I guess it could be one of the latent 'Xilinx bugs'?

Andrew

0 Kudos
dgisselq
Scholar
Scholar
871 Views
Registered: ‎05-21-2015

@andrewcb ,


all of the other points in your list are bugs copied over from the original Xilinx code?

I can't be sure.  Many of them are bugs copied over from the Xilinx code I tested back with Vivado 2016.3.  Not all of them are copied bugs, and I can't be sure how many bugs were in the ISE 14.5 version you copied it from.  Xilinx has changed their demo code over time, although ... last I checked they still hadn't fixed several of those bugs.


Something is still not quite right though.... I guess it could be one of the latent 'Xilinx bugs'?

From my perspective, it's hard to tell.  I don't have access to your design or your changes to know if you managed to fix those bugs or not.  You know the old phrase, "99 bugs in the design.  Take one down, patch it up, 107 bugs in the design"?  Without the formal proof, it'd be hard to know if things are better or worse.

I should also point out that I hadn't finished finding all the bugs yet when I first wrote back to you.  I had just found enough of a handful that it was becoming difficult to look for more without changing your design to fix the ones I had found.  As we've discussed here, since that time I've found even more--such as the fact that the core would hold WREADY high.

If you want me to take another pass, feel free to send a private message and we can arrange terms to work this into the ground until it is guaranteed.  If not, good luck, and I'm glad I had the opportunity to help out.

Dan

0 Kudos