AXI arvalid signal issue

[promach]

I have question about AXI arvalid signal used for interfacing with DDR axi controller in Zynq.

if I do not remove i_axi_arready , I will have deadlock issue.
If I remove i_axi_arready , I will have issue arising from DDR periodic mechanism such as auto-refresh.

What could I do ?

 

[promach]

Someone told me the following which I do not understand:

VALID signal needs to be set (initially) independent of READY signal, and then only ever adjusted if !(VALID && !READY)

---

Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
14
always @(posedge clk) 
begin   
    if(reset) o_axi_arvalid <= 0;
 
    // AXI specification: A3.3.1 Dependencies between channel handshake signal
    // the VALID signal of the AXI interface sending information must not be dependent on 
    // the READY signal of the AXI interface receiving that information
    // this is to prevent deadlock 
    // since AXI slave could waits for i_axi_arvalid to be true before setting o_axi_arready true.
    // Note: VALID cannot be dependent upon READY, but READY can be dependent upon VALID
    //       VALID signal needs to be set (initially) independent of READY signal, 
    //       and then only ever adjusted if !(VALID && !READY)
    else o_axi_arvalid <= /*i_axi_arready &&*/ (!cache_is_full);
end

---

 

[TrickyDicky]

VALID signal needs to be set (initially) independent of READY signal, and then only ever adjusted if !(VALID && !READY)

This is correct. it is ILLEGAL to assert valid based on the ready signal. You may de-assert valid when ready is asserted. but ready MAY depend on valid. To this end, is is legal to connect ready directly to valid. This is true for ALL AXI channels.
Also, once valid is asserted, is MUST be kept asserted until ready is asserted.

You need to forget about the DDR backend, and simply think about the AXI channels. It is the DDR cotrollers problem when it is ready to recieve address requests. You simply send them when you can.
It also may be in your interest to think about pipelineing. It is fairly common to send several read requests before they are completed. This is down to the DDR controller how many address requests it can store in its pipeline.

 

[ads-ee]

This is correct. it is ILLEGAL to assert valid based on the ready signal.

if I do not remove i_axi_arready , I will have deadlock issue.

Asserting valid based on ready can result in a deadlock issue as neither side ever asserts their respective signal based on waiting for the other side to assert theirs first. That is why the spec say that the master asserts valid anytime it wants to do a transfer irregardless of the state of ready.

 

[promach]

Now, I have modified the code to satisfy AXI4 requirements, but I am not sure how many more AXI4 bugs I have.

So, should I use Xilinx AXI VIP to verify my AXI4 bus protocol correctness ?

 

[TrickyDicky]

Yes you can. Ive never used it as I have my own written in VHDL. You can also get the SVA directly from ARM
**broken link removed**

 

[vGoodtimes]

I'm not sure if any remaining errors are specifically axi errors. the two-ish stage pipeline, mix of conditions, and second backpressure signal make me very suspicious. there aren't comments about why the logic is different. or why it needs to be copy/pasted.

I would make a testbench and use one or more lfsrs to generate the ready/cache_full inputs. and I guess try different reset cases /wrt these two. it looks like this starts immediately after reset. so I'm guessing I can't assume cache_full is low after reset.

and having an axi tester is useful. if you make changes for any reason you might re-break the axi aspects.

 

[promach]

the two-ish stage pipeline, mix of conditions, and second backpressure signal make me very suspicious. there aren't comments about why the logic is different. or why it needs to be copy/pasted.

Would you mind telling which lines of code that are suspicious ?

What do you exactly mean by copy/pasted ?

 

[vGoodtimes]

I don't think specific lines of code are suspicious. I think the design has flaws.

for https://gist.github.com/promach/251cbb3c9c9af401bf712dc4ccb76fb3, lines 144-149, 187, 200-201, 209, 217 seem suspect. But that is every interesting line -- not a quickfix scenario.

you have a few main predicates. But you insist on copy/pasting them. They are important but you can't give a meaning to them. And you comment on random implementation details instead. And the interactions between the predicates is non-obvious and highly nuances and non-commented. It doesn't make me think that anything that works correctly was planned. It makes me suspicious.

Also, I get why you have this coding style. But you could move into the 2002 timeframe. not a typo. Verilog2001 is certainly a thing at this point.

 

[promach]

lines 144-149, 187, 200-201, 209, 217 seem suspect. But that is every interesting line -- not a quickfix scenario.

I have multiple verilog files in the link. Which file were you referring to ?

 

[vGoodtimes]

oh. I only looked at address_generator.v . There might be other issues in the other files.

 

[promach]

I don't think specific lines of code are suspicious. I think the design has flaws.

AXI Protocol Checker IP reports errors on AXI_ERRS_RID and AXI_AUXM_RCAM_OVERFLOW which are pc_status bits 59 and 78 respectively.

My AXI code is located at here (need to uncomment code block lines 107 to line 199 as well as line 374)

Could anyone advise ?

 

[TrickyDicky]

The document tells you what the errors mean. Maybe you should investigate...

Looking at your waveform, you previously mentioned you are connected to DDR - but all your reads are for 1 word. This is hugely innefficient with DDR - for best efficiency you should be doing reads/write of at least 1k bytes.

 

[promach]

As long as the master can hold RREADY high, which most masters can do, then the master doesn't need a skid buffer. (The same is true of BREADY)
The 50% loss comes from the fact that it takes a clock to set ARREADY high after any transaction completes
We are not allowed to set ARREADY combinatorially
(This is in the slave now)
If we were to set ARREADY combinatorially, we might set it to ARREADY = (!RVALID || RREADY);
But because ARREADY *must* be registered as per spec, it takes a clock to capture the stall
During that clock, either data can come in and get kept some where (a.k.a. skid buffer), or we have to make certain ARREADY is already low (50% throughput)

Any comments about this ?

 

[TrickyDicky]

What's the problem?

 

[promach]

Why As long as the master can hold RREADY high, which most masters can do, then the master doesn't need a skid buffer. (The same is true of BREADY) ?

An explanation on the need for skid buffer in AXI bus protocol

 

[TrickyDicky]

because you wont assert backpressure, and the slave wont need to halt part midway during a burst, or is able to immediately start a new burst. A skid buffer is basically a 1 word deep fifo.

 

[promach]

because you wont assert backpressure, and the slave wont need to halt part midway during a burst, or is able to immediately start a new burst.

No backpressure ?

Why only 1 word deep fifo (skid buffer) ? what if the master keep holding RREADY low

 

[promach]

What do you guys think about the following with regards to the skid buffer ?

A skid buffer lets you be optimistically wrong for one cycle in a row.

when something should be combinatorial, but must be registered, it's nice to be able to be optimistic. but registering might make you wrong once in a row.

conceptually, to be safe you have to "look before you leap". that is not ideal.
It is better to say "leap" and then be able to be wrong once (in a row) as long as you aren't wrong twice in a row.

 

[TrickyDicky]

No backpressure ?

Why only 1 word deep fifo (skid buffer) ? what if the master keep holding RREADY low

A skid buffer is purely a 1 deep fifo. But because of the way axi works its perfectly possible to use any first-word-fall-through fifo direcly on the bus - so its pretty common to have bigger buffers to hadle one or more bursts.
It all depends on architecture. I highly recommend you read the AMBA AXI4 spec.