SystemVerilog Combinational weighted priority selection routine.

[BrianHG]

Ok, this one for me is unusual as I am accustomed to 'always_comb' being purely for combinational statements where 2 '=' events after one another should just mess everything up.  Yest this simulates fine in ModelSim.

This is a code which selects which read port/channel, 0 through 15, has the next priority selection to my DDR3 memory controler:

Code: [Select]

always_comb begin

// **************************************************************************************************************************
// Combinational Read Request Priority Selection Logic: (1 of 2)
// Establish all the metrics of all the source read channel command input FIFOs.
// **************************************************************************************************************************
for (int i=0 ; i<PORT_R_TOTAL ; i ++ ) begin

    // Make a flag which identifies if the next FIFO queued read command address is within the current read cache's address.
    // Used to bypass any unnecessary read requests to the DDR3.
    RC_cache_hit[i]     = ( RC_raddr[i][PORT_ADDR_SIZE-1:CACHE_ADDR_WIDTH] == FIFO_raddr[i][PORT_ADDR_SIZE-1:CACHE_ADDR_WIDTH] ) && RC_ready[i] ;
   
    // Make a flag which identifies if the next FIFO queued read command address is within the same DDR3 row as the previous DDR3's accessed address.
    // Used to raise the priority of the current read channel so that sequential reads bursts within the same row address get coalesced together.
    RC_page_hit[i]      = ( SEQ_ADDR[PORT_ADDR_SIZE-1:CACHE_ROW_BASE] == FIFO_raddr[i][PORT_ADDR_SIZE-1:CACHE_ROW_BASE] );

    // Coalesces all the required DDR3 read requests into a single logic word.
    RC_ddr3_read_req[i]  = (read_req[i] && !RC_cache_hit[i] && !RC_ddr3_busy[i]) ;

    // Advance the read FIFO.  No advance should be allowed if any other read channels are busy waiting for the DDR3 to read.
    read_req_ack[i]     = (read_req[i] && RC_cache_hit[i]) || (SEQ_RDATA_RDY_t != last_rdata_rdy && SEQ_RDATA_VECT_IN[3:0] == 4'(i)) ;

    // Output a read ready pulse on the CMD/RC_read_ready[] port when DDR3_PHY read is done or a read cache hit occurs.
    RC_read_ready[i]    = RC_cache_ack[i] || RC_ddr3_ack[i] ;

end // for i

// **************************************************************************************************************************
// Combinational Read Request Priority Selection Logic: (2 of 2)
// Select which read_ack active channel goes next based on the previous DDR3 read access.
// **************************************************************************************************************************
any_read_req = (RC_ddr3_read_req != 0) ; // Go high if there are any DDR3 reads required.

    read_req_sel     = 4'd0 ;
    RC_pri_weight    = 4'd0 ;
    RC_pri_pos       = last_read_req_chan ;     // Begin the priority scan with the last DDR3 read request channel as the lowest priority by order.

for (int i=0 ; i<PORT_R_TOTAL ; i ++ ) begin    // Cycle around the available read ports.
                                                // Use the RC_pri_pos as the bottom priority.

    // Compare the weight of the read request channel against the channel's set parameter priority's weight of 0-7, with a +8 boost if the
    // read request is in the same row as the previous DDR3 access.  The +8 boost coalesces same port page bursts until the burst_limit has been reached.
    if ( RC_ddr3_read_req[RC_pri_pos] && ( (PORT_R_PRIORITY[RC_pri_pos] || ((RC_page_hit[RC_pri_pos] && !RC_burst_limit[8])<<3)) >= RC_pri_weight ) ) begin

        read_req_sel  = RC_pri_pos ; // Update the selection of the potential next DDR3 access read channel until the 'for i loop' is finished.

        // Set the current priority weight including a +8 boost for a write request within the same row unless the burst limit counter reaches -1, IE 256.
        RC_pri_weight = (PORT_R_PRIORITY[RC_pri_pos] || ((RC_page_hit[RC_pri_pos] && !RC_burst_limit[8])<<3)); // Set the current priority weight including a +8 boost for a read request within the same row.

    end

RC_pri_pos = (RC_pri_pos==0) ? (PORT_R_TOTAL-1) : (RC_pri_pos - 1) ; // Proceed to the next position, wrap around to the top once position 0 has been reached.

end // for i

end // always_comb

Part 1 of 2 is obviously perfectly fine.
Part 2 of 2 is where I'm in new ground.  I used to do this in synchronous logic to select between 2 to 4 ports with equal weight, however, for a variable number of ports with variable weights, combined with all the features stacked, it seemed it was time to try this combinational method.

At the end of it all, logic 'read_req_sel' should contain a number, 0 through 15, of which port should have access to the next DDR3 read.

Am I doing anything illegal here?
Can I expect this code to not work properly in the field?
Is this logic sound?
Has anyone coded like this before?

[BrianHG]

Ok, after extensive simulation, the same page priority boost:

Code: [Select]

( (PORT_R_PRIORITY[RC_pri_pos] || ((RC_page_hit[RC_pri_pos] && !RC_burst_limit[8])<<3))
Seems to be ignored even though the regs show up active and correct in the simulation.

It's like the 'RC_pri_weight' and the 'IF' aren't being taken into account, but the rotating 'RC_pri_pos' based on the last DDR3 read is.

Next, I'll try a 2 pass loop.

[BrianHG]

Ok, after extensive simulation, the same page priority boost:

Code: [Select]

( (PORT_R_PRIORITY[RC_pri_pos] || ((RC_page_hit[RC_pri_pos] && !RC_burst_limit[8])<<3))
Seems to be ignored even though the regs show up active and correct in the simulation.

It's like the 'RC_pri_weight' and the 'IF' aren't being taken into account, but the rotating 'RC_pri_pos' based on the last DDR3 read is.

Next, I'll try a 2 pass loop.

Seems to be priority sekecting now, the bug was elsewhere.  Ok, time to make it into a working project...

[BrianHG]

Ok, this new section 2 of 2 works perfectly:

Code: [Select]

// **************************************************************************************************************************
// Combinational Read Request Priority Selection Logic: (2 of 2)
// Select which read_ack active channel goes next based on the previous DDR3 read access.
// **************************************************************************************************************************
any_read_req = (RC_ddr3_read_req != 0) ; // Go high if there are any DDR3 reads required.

read_req_sel = 4'd0 ;
RC_break     = 0;

for (int p=15 ; p>=0 ; p-- ) begin                                                               // 'p' scans in order of highest priority to lowest.
    for (int i=(last_read_req_chan+1) ; i<(17+last_read_req_chan) ; i++ ) begin                  // 'i' scans at the next read channel port and counts up and around.
        if ( 4'(i)<PORT_R_TOTAL && !RC_break ) begin                                             // Only scan from the available read ports.
            if ( RC_ddr3_read_req[4'(i)] && ( (PORT_R_PRIORITY[4'(i)]<<1) + RC_page_hit[4'(i)] ) == 4'(p) ) begin // Add 1/2 weight to the priority level if the page_hit is true.
                                                                          read_req_sel = 4'(i);  // If there is a hit a read_req hit and the port priority is == P
                                                                          RC_break     = 1 ;     // Ignore the rest of the loop.
                                                                          end
        end
    end // for i
end // for p

The trick was the newly added 'RC_break = 1' which truly stops the rest of the loop from setting the 'read_req_sel' once the highest priority available read request in the pool has been found.

While the 'for (int i=(last_read_req_chan+1)...' begins the scan for that 1 position above the previous read sent to the DDR3 sequencer, wrapping around to that channel once again at the end.

The '(for p=15 ...--)'... begins the search for the highest read request in the pool and works its way down to the lowest priority.


ModelSim likes it, but what will Quartus do?  Give me 2 days to find out...