Bug 57

Summary: chain of buffered pipeline followed by unbuffered pipeline fails
Product: Libre-SOC's first SoC Reporter: Luke Kenneth Casson Leighton <lkcl>
Component: Source CodeAssignee: Luke Kenneth Casson Leighton <lkcl>
Status: RESOLVED INVALID    
Severity: blocker CC: libre-soc-bugs
Priority: ---    
Version: unspecified   
Hardware: PC   
OS: Linux   
NLnet milestone: NLnet.2019.02.012 total budget (EUR) for completion of task and all subtasks: 0
budget (EUR) for this task, excluding subtasks' budget: 0 parent task for budget allocation:
child tasks for budget allocation: The table of payments (in EUR) for this task; TOML format:
Bug Depends on:    
Bug Blocks: 64    
Attachments: gtkwave screenshot showing p_o_ready not in sync

Description Luke Kenneth Casson Leighton 2019-04-06 05:37:14 BST 
* a BufferedPipeline followed by
* an UnbufferedPipeline (or UnbufferedPipeline2 which is the logic from BreakReadyChainStage)

will FAIL for no good reason that i can see.

*all other permutations work fine*.

* BufferedPipeline followed by BufferedPipeline: fine
* UnBufferedPipeline followed by BufferedPipeline: fine
* UnBufferedPipeline2 followed by BufferedPipeline: fine
* UnBufferedPipeline followed by UnbufferedPipeline: fine
* UnBufferedPipeline2 followed by UnbufferedPipeline2: fine

i haven't tried permutations of UnbufferedPipeline with UnbufferedPipeline2.

this bug occurred *before* the addition of the dynamic data valid/ready logic, i just hadn't written a unit test that caused the above failure until writing the dynamic data valid/ready code.  as it was interfering with the development i went back to a version of the code from a day ago, and *confirmed* that the bug is still present *before* the dynamic data valid/ready logic was added.
Comment 2 Luke Kenneth Casson Leighton 2019-04-06 10:55:27 BST 
p_o_ready is combinatorial and is an entire cycle *too early*, relative to the data.

 so the BufferedPipeline is receiving a n_i_ready signal indicating that the next stage (the UnbufferedPipeline) is ready when it is not.

 the *other way round* is fine... because there is a delay introduced.
Comment 4 Luke Kenneth Casson Leighton 2019-04-07 01:06:39 BST 
created a new mode to BufferedPipeline which takes out the register
(saving gates), and preserves the synchronised logic.

still have to work out what to do about UnbufferedPipeline.
Comment 5 Luke Kenneth Casson Leighton 2019-04-08 12:05:33 BST 
Created attachment 7 [details]
gtkwave screenshot showing p_o_ready not in sync

p.o_ready is not properly in sync with data, it is being set just
beyond the clk.  this means that only a combinatorial chain will
be ok... right up until the point where it's connected to a sync'd
valid/ready unit.

by contrast, SimpleHandshake and BufferedHandshake send the data
*and* its associated p.o_ready on the clock.
Comment 6 Luke Kenneth Casson Leighton 2019-04-10 03:39:23 BST 
        Truth Table

        Inputs  Temp  Output
        -------   -   -----
        P P N N ~NiR&  N P
        i o i o  NoV   o o
        V R R V        V R

        -------   -    - -
        0 0 0 0   0    0 1
        0 0 0 1   1    1 0
        0 0 1 0   0    0 1
        0 0 1 1   0    0 1
        -------   -    - -
        0 1 0 0   0    0 1
        0 1 0 1   1    1 0
        0 1 1 0   0    0 1
        0 1 1 1   0    0 1
        -------   -    - -
        1 0 0 0   0    1 1
        1 0 0 1   1    1 0
        1 0 1 0   0    1 1
        1 0 1 1   0    1 1
        -------   -    - -
        1 1 0 0   0    1 1
        1 1 0 1   1    1 0
        1 1 1 0   0    1 1
        1 1 1 1   0    1 1
        -------   -    - -

Note: PoR is *NOT* involved in the above decision-making. If p.o_ready
is ignored, it has no way to determine if, during the previous cycle,
it told the previous stage "sending data is okay".

This is what is resulting in data corruption.
Comment 7 Luke Kenneth Casson Leighton 2021-10-02 11:35:50 BST 
different types of pipelined and non-pipelined objects cannot be
chained together.

if one is pipelined but the other is not then whilst one can signal
to other to stall, it cannot actually do so.

closing as invalid