Continue to Site

Welcome to EDAboard.com

Welcome to our site! EDAboard.com is an international Electronics Discussion Forum focused on EDA software, circuits, schematics, books, theory, papers, asic, pld, 8051, DSP, Network, RF, Analog Design, PCB, Service Manuals... and a whole lot more! To participate you need to register. Registration is free. Click here to register now.

AXI arvalid signal issue

Status
Not open for further replies.
bram_axi_controller -- arready is always high after reset.
 

There is extreme possibility that arready could always be asserted high after reset, thus we need to take this possible scenario into account.

Please correct if wrong.

Besides, I have backpressure built in to o_axi_arvalid signal of AXI master in the previous post
 

you should try to make sure each module has a correct axi interface. it gets difficult to read the code when there are assumptions about the axi interfaces being used in specific ways.
 

I suppose Error bit 59 is due to inconsistent logic for ARADDR and RDATA

The read data must always follow the address that it relates to.

bram_axi_controller.v (AXI Slave)


Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
10
always @(posedge clk)
begin
    if(reset) o_axi_rdata <= 0;
 
    // the following assumes that (i_axi_arlen = 0) && (i_axi_arsize = C_AXI_DATA_WIDTH/NUM_OF_BITS_PER_BYTES)
    // If (i_axi_arlen = 0), then value of i_axi_arburst does not matter at all
    // memory data is read out correctly one clock cycle after data request
    // memory data is valid only when requested with valid read address
    else if(!(o_axi_rvalid && !i_axi_rready)) o_axi_rdata <= mem[i_axi_araddr]; 
end




read_instructions.v (AXI Master)


Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
10
always @(posedge clk)
begin
    if(reset) o_axi_araddr <= 0;
 
    // continues the next reading only when DDR memory is ready && 
    // destination cache has space &&
    // not exceeding designated DDR memory address
    else if(i_axi_arready && o_axi_rready) 
        o_axi_araddr <= o_axi_araddr + 1; // increments DDR address to read instructions from
end

 

Why are you incrementing the address based on rready?
I think you are confusing yourself as you are mixing up the control signals between the channels.
If you control the channels independent of one another, then your design will be easier.

ARVALID + ARREADY = next ARDDR on master

ARVALID + ARREADY = produce RDATA + Assert RVALID

Hold RVALID and RDATA until RREADY (Hold off arready)

This way, the master can produce and asserrt a new address without worrying about RDATA channel.
 

Code Verilog - [expand]
1
2
else if(i_axi_arready && o_axi_arvalid) 
        o_axi_araddr <= o_axi_araddr + 1;



However, if I modify the logic for ARADDR as above with o_axi_arvalid being a reg , then o_axi_araddr will be off by 1 clock cycle compared to o_axi_arvalid
 


Code Verilog - [expand]
1
2
else if(i_axi_arready && o_axi_arvalid) 
        o_axi_araddr <= o_axi_araddr + 1;



However, if I modify the logic for ARADDR as above with o_axi_arvalid being a reg , then o_axi_araddr will be off by 1 clock cycle compared to o_axi_arvalid

You clearly are not understanding the AXI spec and seem to be trying to read between the lines. There are only a couple of places that can be misinterpreted and those are on rarely used features from what I've seen.

When ARVALID & ARREADY are both high the next ARADDR can be generated because the current address for the current transfer is now complete (ARVALID & ARREADY). The address isn't a clock cycle behind compared to ARVALID it's an entirely new address for the next transfer. If there the master has another address to send then it can keep ARVALID high after the ARVALID & ARREADY both high or if there is no subsequent address the the master can set ARVALID low.

All the channels have the exact same behavior.

As I mentioned in your now duplicate thread you should draw your timing diagrams as channels not as master/slave signals, which only helps confuse things. The basic signals of the channel are VALID, READY, and DATA keep those grouped together for each channel.

The basic handshake for all channels is simply:
1. VALID can be asserted at any time and does not depend on READY
2. READY can be asserted at any time and does not depend on VALID
3. Both VALID and READY high completes the current channel transfer
4. The other channels are not dependent on the above transfer handshake, e.g. you don't have to start the RDATA/RVALID transfer on ARVALID/ARREADY you might insert 10 wait states to allow a flash device to get the command, address, command, and access time before returning valid data.

Of course there are various other signals that can affect this handshake if you are doing bursts/locked etc transfers.
 
ok, I looked more at this. There are a mix of issues. The first is that the signals have axi names, but at some point it looks like you gave up on implementing axi. so the interfaces look wrong when held to axi standards.

but they almost work in the specific system. I'm guessing you are doing something like design-by-simulation. Tweaking the code to get waveforms to look better. I could see how this design could result from that.

The almost looks like the classic 1-extra delay for a control signal. if you have cache_is_full (rready) go low for 1 cycle you should be able to observe the ar channel transaction being accepted but not impacting rvalid/rdata. arvalid goes low on the next cycle, but by then it is too late. this can be solved in multiple ways depending on if you want to meet the axi spec or just want to use the names.

(also, the read instruction module has a comparison that is always true. the count can never read the terminal value as it is 1bit too small. The counter will overflow.)
 

@TrickyDicky

ARVALID + ARREADY = produce RDATA + Assert RVALID

What do you exactly mean by the above ?


bram_axi_controller.v (AXI Slave)


Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
always @(posedge clk)
begin
    if(reset) o_axi_rdata <= 0;
 
    // the following assumes that (i_axi_arlen = 0) && (i_axi_arsize = C_AXI_DATA_WIDTH/NUM_OF_BITS_PER_BYTES)
    // If (i_axi_arlen = 0), then value of i_axi_arburst does not matter at all
    // memory data is read out correctly one clock cycle after data request
    // memory data is valid only when requested with valid read address
    else if(!(o_axi_rvalid && !i_axi_rready)) o_axi_rdata <= mem[i_axi_araddr]; 
end
 
always @(posedge clk)
begin
    if(reset) o_axi_rvalid <= 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 wait for i_axi_arvalid to be true before setting o_axi_arready true.
    // Note: For same interface, VALID cannot depend upon READY, but READY can depends upon VALID
    // Note: Once VALID is asserted, it MUST be kept asserted until READY is asserted.
    //       VALID signal needs to be set (initially) independent of READY signal, 
    //       and then only ever adjusted if !(VALID && !READY)
    // Note: the slave must not wait for the master to assert RREADY before asserting RVALID
    // Note: (!(o_axi_rvalid && !i_axi_rready)) == (!rvalid || rready) 
    //       == (!rvalid || (rvalid && rready)). 
    //       it means "no transaction in progress or transaction accepted"
    // Note: the slave must wait for both ARVALID and ARREADY to be asserted before 
    //       it asserts RVALID to indicate that valid data is available
    else if(!(o_axi_rvalid && !i_axi_rready))
            o_axi_rvalid <= i_axi_arvalid && o_axi_arready; 
end




Note: I have already modified the logic for ARADDR as follows :

read_instructions.v (AXI Master)


Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
always @(posedge clk)
begin
    if(reset) o_axi_araddr <= 0;
 
    // When ARVALID & ARREADY are both high the next ARADDR can be generated 
    // because the current address for the current transfer is now complete (ARVALID & ARREADY).
    else if(i_axi_arready && o_axi_arvalid) 
        o_axi_araddr <= o_axi_araddr + 1; // increments DDR address to read instructions from
end

 
Last edited:

Code:
// simplified logic with interpretation and commentary.  resets omitted.  Code
// is from two files.
always @(posedge clk)
begin
  // this part is fine. (bram controller)
  // if (no current r transaction or current r transaction accepted)
  //   rvalid <= ar transaction accepted.
  //   rdata <= mem selected with araddr
  if(!(rvalid && !rready)) begin
    rvalid <= arvalid && arready; 
    rdata <= mem[araddr]; 
  end

  // this part is not. (bram controller)
  // always accept ar transactions.
  // lets say rready is 0 and stays 0 for many cycles.
  // i_axi_arvalid becomes high.  this is fine -- the above logic works
  // correct.  but one cycle later, what happens if arvalid is high again?
  // the ar transaction will be accepted, but the r channel is not affected.
  // and the above logic doesn't have any way to handle this.
  arready <= 1;

  // this part is weird (read instruction)
  // from the above commentary, any time rready is high, arvalid must stop
  // immediately because the transaction would be accepted and dropped.
  // making part/all of this combinatorial probably fixes the issue, but isn't
  // axi.  It might work fine for this specific case.
  arvalid <= (ddr_address_range_is_valid) && rready;
end
// this part is weird (read instruction)
// it is not registered and comes from another module.
// perhaps this is ok?
rready <= !cache_is_full; // from another module

This is how I read the code from these two modules. Along with some commentary.
 
You accept the read address, then some time later, you produce the read data.

I think I had done this with the following code segment in bram_axi_controller.v (AXI master) :


Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
always @(posedge clk)
begin
    if(reset) o_axi_rdata <= 0;
 
    // the following assumes that (i_axi_arlen = 0) && (i_axi_arsize = C_AXI_DATA_WIDTH/NUM_OF_BITS_PER_BYTES)
    // If (i_axi_arlen = 0), then value of i_axi_arburst does not matter at all
    // memory data is read out correctly one clock cycle after data request
    // memory data is valid only when requested with valid read address
    else if(!(o_axi_rvalid && !i_axi_rready)) o_axi_rdata <= mem[i_axi_araddr]; 
end
 
always @(posedge clk)
begin
    if(reset) o_axi_rvalid <= 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 wait for i_axi_arvalid to be true before setting o_axi_arready true.
    // Note: For same interface, VALID cannot depend upon READY, but READY can depends upon VALID
    // Note: Once VALID is asserted, it MUST be kept asserted until READY is asserted.
    //       VALID signal needs to be set (initially) independent of READY signal, 
    //       and then only ever adjusted if !(VALID && !READY)
    // Note: the slave must not wait for the master to assert RREADY before asserting RVALID
    // Note: (!(o_axi_rvalid && !i_axi_rready)) == (!rvalid || rready) 
    //       == (!rvalid || (rvalid && rready)). 
    //       it means "no transaction in progress or transaction accepted"
    // Note: the slave must wait for both ARVALID and ARREADY to be asserted before 
    //       it asserts RVALID to indicate that valid data is available
    else if(!(o_axi_rvalid && !i_axi_rready))
            o_axi_rvalid <= i_axi_arvalid && o_axi_arready; 
end



- - - Updated - - -

I think I found out why RDATA does not align with ARADDR as pointed out by AXI Protocol Checker IP.

However, even after I have modifed the AXI master logic for o_axi_araddr as follows , I still have error from AXI Protocol Checker IP


Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
always @(posedge clk)
begin
    if(reset) o_axi_araddr <= 0;
 
    // When ARVALID & ARREADY are both high the next ARADDR can be generated 
    // because the current address for the current transfer is now complete (ARVALID & ARREADY).
    else if(i_axi_arready && o_axi_arvalid && o_axi_rready) 
        o_axi_araddr <= o_axi_araddr + 1; // increments DDR address to read instructions from
end



qD7PrUE.png
 
Last edited:


Code Verilog - [expand]
1
else if(i_axi_arready && o_axi_arvalid && o_axi_rready)



This will not work in all situations. ARREADY and ARVALID don't have to be high when RREADY is high, they are from different channels.

1) when ARVALID goes high the master is saying I have an new address to transfer.
2) slave sends an ARREADY high to tell the master it is done with the current address, i.e. it has latched it or used it already. master can now change the address if it wants to.
3) slave can now start a read operation using the address captured during 2 (ignoring the RADDR), while doing the read,the slave would not respond (with an ARREADY) to another ARVALID unless it can support pending transactions.
4) slave sends the RDATA and a RVALID to end the transaction and return the read data to the master.

You should not be mixing signals from different channels to control individual channels. All the channels are self contained, only an FSM controlling the back end of the AXI bus would use signals from different channels to determine things like the address has been receive and the data needs to be fetched/written to something.

I would also avoid what vGoodtimes pointed out that you have created what look like AXI signals that are actually signals on your backend interface to the rest of your logic. o_axi_araddr is a perfect example of this.

As you seem to want to do an auto-increment address (for bursts?).
a) latch the AXI bus RADDR with ARREADY && ARVALID
b) increment the address using RREADY && RVALID, this updates the auto-increment burst address with a new read address.
 
  • Like
Reactions: promach

    V

    Points: 2
    Helpful Answer Positive Rating

    promach

    Points: 2
    Helpful Answer Positive Rating
I think the updated code is based on both ends having errors that cancel out. eg, one side drops the transaction and the other side doesn't advance even though there was a transaction. but in terms of a protocol checker, this is confusing.

I'm still not sure if the intent is to use axi or just use axi names. In any case, if you wanted to use axi, you would likely use a skid buffer in the bram_axi_controller. Then arready can correctly apply backpressure. this frees up all other interfaces to use the correct logic for axi.
 
Then arready can correctly apply backpressure.

@vGoodtimes : Why use ARREADY ? Why can't we use the following backpressure mechanism ?


Code Verilog - [expand]
1
o_axi_arvalid <= o_axi_rready;




3) slave can now start a read operation using the address captured during 2 (ignoring the RADDR), while doing the read,the slave would not respond (with an ARREADY) to another ARVALID unless it can support pending transactions.

@ads-ee : this literally means AXI master cannot issue another read request to AXI slave if RVALID had not been received by AXI master ? Is that what you mean ?


As you seem to want to do an auto-increment address (for bursts?).
a) latch the AXI bus RADDR with ARREADY && ARVALID
b) increment the address using RREADY && RVALID, this updates the auto-increment burst address with a new read address.

@ads-ee : wait, I do not get the difference between a) and b) above ?
 
Last edited:

that might be possible, but it isn't axi.

you should be able to look at each module and determine if it works. you should not need to look at two modules to determine if one of them works.

if you use arready for backpressure, you know a valid axi master won't send transactions you can't handle. you only need look at one file to determine correctness of the interface.

if you use rready for backpressure on the ar channel, maybe it does work in some cases. for example, if the axi master uses rready to generate arvalid.

But the axi master is not required to use rready for anything other than accepting transactions on the r channel. you no longer have an axi interface -- you have an axi-ish interface. axi + some additional restrictions/assumptions. this is annoying because you can no longer connect a normal axi interface to your axi-ish interface without investigating if it is compatible.
 
if you use arready for backpressure, you know a valid axi master won't send transactions you can't handle.

@vGoodtimes : but ARVALID cannot depend on ARREADY .....
 

ok? that is expected.

--edit: if arvalid doesn't care about arready, this is not an issue. arready can be used to apply backpressure to prevent additional ar channel transactions.
 
Last edited:

@vGoodtimes : but ARVALID cannot depend on ARREADY .....

Yes it can - it just cannot be asserted based purely on ARREADY being asserted. There must be some way to assert ARVALID if ARREADY is low, and must hold ARVALID high if arready has been held low. There may be a condition where ARVALID is de-asserted based on ARREADY.


Code VHDL - [expand]
1
2
3
4
5
6
7
if start_of_burst then
  arvalid <= '1';
 
elsif end_of_burst and arready = '1' then
  arvalid <= '0';
 
end if;



- - - Updated - - -

Another issue you may be having is directly registering your outputs. AXI is really an unregistered control system, so its usually best to create READY signal unregistered internally and connect them to a AXI reg stage or skid buffer. It only specifies that OUTPUTS have to be registered.
 

According to 'Read transaction dependencies' in section A3.3.1 of AXI spec, ARREADY and ARVALID must be of clocked-reg type

AXI is really an unregistered control system, so its usually best to create READY signal unregistered internally and connect them to a AXI reg stage or skid buffer.

@TrickyDicky : Note the **broken link removed**.

- - - Updated - - -

As you seem to want to do an auto-increment address (for bursts?).
a) latch the AXI bus RADDR with ARREADY && ARVALID
b) increment the address using RREADY && RVALID, this updates the auto-increment burst address with a new read address.

@ads-ee : Which of the following logic should I use for o_axi_araddr ?



Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
10
11
wire transaction_is_accepted = (i_axi_rvalid) && (o_axi_rready);
 
always @(posedge clk)
begin
    if(reset) o_axi_araddr <= 0;
 
    // When RVALID & RREADY are both high the next ARADDR can be generated 
    // because the current address for the current transfer is now complete (RVALID & RREADY).
    else if(transaction_is_accepted) 
        o_axi_araddr <= o_axi_araddr + 1; // increments DDR address to read instructions from
end





Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
10
11
wire transaction_is_accepted = (o_axi_arvalid) && (i_axi_arready);
 
always @(posedge clk)
begin
    if(reset) o_axi_araddr <= 0;
 
    // When ARVALID & ARREADY are both high the next ARADDR can be generated 
    // because the current address for the current transfer is now complete (ARVALID & ARREADY).
    else if(transaction_is_accepted) 
        o_axi_araddr <= o_axi_araddr + 1; // increments DDR address to read instructions from
end



- - - Updated - - -

@@TrickyDicky : Your VHDL code has a small mistake which should be modified as follows :


Code VHDL - [expand]
1
2
3
4
5
6
7
if start_of_burst then
  arvalid <= '1';
 
elsif end_of_burst and arready = '0' then
  arvalid <= '0';
 
end if;



Besides, could you define start_of_burst and end_of_burst in terms of AXI signals ?
 

Status
Not open for further replies.

Part and Inventory Search

Welcome to EDABoard.com

Sponsor

Back
Top