Hi Brian,

I finally got a chance to look at that change in a bit more detail. I
need to test out this theory out but we may have added a regression
with the throttle commit I pointed out earlier. The issue would apply
to "sink" blocks only. We assert the clear_tx_seqnum signal when the
block is initialized but we only deassert it when the NEXT_DST_SID
register is set, which never happens for a block that doesn't have a
downstream destination i.e. a TX radio. In our testing we validate
both directions of the radio block so we we end up getting lucky
because the RX side is always connected. I'm guessing your DmaFIFO ->
Radio graph was TX only, right? For now, I'm going to revert the
noc_shell throttle commit (b48975ef) and we will address the
underlying issue with a software + FPGA fix.

The issue does not change the guidance on when the clear signal should
be used though. It is asserted when the flow-graph in UHD is about to
change (this includes streamer creation), and all the data-path state
machines need to reset. An example of when this would happen is when
you switch applications in GR. If you download an FPGA image, run flow
graph A, then without reloading the image you want to run flow graph B
with a different topology then the clear signal will ensure that
things are cleaned up properly between A and B. Do you really have to
switch applications to reconfigure the topology? In theory, no, but we
have some work to do on the graph resolution side in software to allow
changing the topology on the fly. That feature isn't as high a
priority because it is simple enough to just tear down and recreate
the graph/device3 in UHD. We also don't really support time
multiplexing blocks in the FPGA because each block has a single flow
control buffer and ACKs can only be sent to a single upstream block.
That is probably something that we are unlikely to support. The best
practice I would recommend is to reset any logic that has state
associated with the data stream, like sample counters, packet
counters, accumulators, etc using the clear_tx_seqnum signal. For all
other stream-agnostic logic, you can use a reset register. If
clear_tx_seqnum is high, then configuration and settings changes
should be allowed. The reason they weren't functional was, again, due
to the bug in the commit above.

In order to unblock you, I would suggest reverting commit b48975ef. It
should be safe and you won't lose any functionality. We will circle
back and come up with a solution for the underlying issue which has a
much lower severity. Thanks for the feedback and apologies for the
lost time.

Cheers,
Ashish

On Wed, May 2, 2018 at 4:15 PM, Brian Padalino <bpadal...@gmail.com> wrote:
> Hey Ashish,
>
> On Wed, May 2, 2018 at 7:02 PM Ashish Chaudhari <ashish.chaudh...@ettus.com>
> wrote:
>>
>> Hi Brian,
>>
>> >> > Moreover, it seems like axi_wrapper.v uses clear_tx_seqnum to pass
>> >> > out
>> >> > config bus messages, so now that clear_tx_seqnum is set I am not able
>> >> > to
>> >> > use
>> >> > the config bus from the axi_wrapper.
>> >>
>> >> I think this is a side effect of the assumption that data cannot flow
>> >> through the block when it is not in a graph. The AXI-Stream config bus
>> >> uses clear_tx_seqnum to hold itself in reset to you cannot use that
>> >> either when the data-path is disabled. Do you need to use the config
>> >> bus before you connect your block downstream? If so, you can try (as a
>> >> debugging step) to tied the clear signal to 0 here:
>> >>
>> >>
>> >> https://github.com/EttusResearch/fpga/blob/rfnoc-devel/usrp3/lib/rfnoc/axi_wrapper.v#L200.
>> >> If that makes your block work then it would confirm that the
>> >> aforementioned assumption is breaking your block. If so, we will go
>> >> back re-evaluate if we need to apply the clear_tx_seqnum to just the
>> >> data path and leave the control path alone.
>> >
>> >
>> > I re-created the old strobe methodology and fed that version into
>> > axi_wrapper and it fixed my issue, so it's definitely the thing that was
>> > gating me.
>>
>> OK, so I looked though the code and clear_tx_seqnum should not be
>> clearing the the config FIFO. That is the only place where it bleeds
>> into the control path. The primary purpose of that signal was to clear
>> the data path only. We will push the following patch into master (and
>> rfnoc-devel) to remedy that issue. In the meanwhile can you check if
>> that fixes your issue? You should not need to revert back to the
>> strobe approach or change anything in software.
>>
>> diff --git a/usrp3/lib/rfnoc/axi_wrapper.v b/usrp3/lib/rfnoc/axi_wrapper.v
>> index f438587..1193692 100755
>> --- a/usrp3/lib/rfnoc/axi_wrapper.v
>> +++ b/usrp3/lib/rfnoc/axi_wrapper.v
>> @@ -197,7 +197,7 @@ module axi_wrapper
>>     generate
>>        for (k = 0; k < NUM_AXI_CONFIG_BUS; k = k + 1) begin
>>           axi_fifo #(.WIDTH(33), .SIZE(CONFIG_BUS_FIFO_DEPTH))
>> config_stream
>> -           (.clk(clk), .reset(reset), .clear(clear_tx_seqnum),
>> +           (.clk(clk), .reset(reset), .clear(1'b0),
>>              .i_tdata({(set_addr ==
>> (SR_AXI_CONFIG_BASE+2*k+1)),set_data}),
>>              .i_tvalid(set_stb & ((set_addr ==
>> (SR_AXI_CONFIG_BASE+2*k))|(set_addr == (SR_AXI_CONFIG_BASE+2*k+1)))),
>>              .i_tready(),
>>
>
> It bleeds over in some other places too.  I had an old simple TX graph that
> I was using that would write from DmaFIFO -> Radio, and that stopped
> working.  No little red LED to show I was transmitting and no output power.
>
> Looking at radio_datapath_core.v, there is an input for clear_tx and
> clear_tx which both feed clear signals in the tx_control_gen3.
>
> Feeding the strobed version of the clear fixed the issue of no transmission
> from my simple TX or my siggen block.
>
>>
>>
>> >> > The most frustrating part is that simulation does not exhibit this
>> >> > behavior,
>> >> > so I can't trust my simulation and have to rely on the ILA to give me
>> >> > insight into these issues.
>> >>
>> >> The RFNOC_CONNECT macro in the simulation infrastructure is emulating
>> >> the software initialization and graph connect operations atomically so
>> >> you unfortunately don't see similar behavior as software. This is one
>> >> of those things where we are trying to emulate software as much as
>> >> possible while still trying to keep the simulation stuff lightweight,
>> >> which leads to some disparity in behavior.
>> >
>> >
>> > But it isn't doing it as much as possible, or at least not in the same
>> > way
>> > the C++ has it exposed.
>> >
>> > Is the preferred flow then:
>> >
>> >   - Get the appropriate control and blockid's of the blocks to connect
>> >   - Connect all the blocks in the way they are expected to be connected
>> >   - Apply any argument calls to the blocks after the connect() calls
>> > have
>> > been made
>> >   - Send issue_stream_cmd() in the case of RX blocks
>> > Can you write a simple C++ program which uses the Radio -> Window -> FFT
>> > ->
>> > Host, and uses the default window and attach it to a reply?
>> >
>> > In cases of TX blocks like the siggen where all setting and enabling is
>> > done
>> > through settings registers, is there anything special that needs to be
>> > done
>> > to transmit samples?
>>
>> As I mentioned above, you don't need to change software if you apply the
>> patch.
>>
>> >> > Can someone tell me how clear_tx_seqnum should be working and how it
>> >> > should
>> >> > be used?  Should it be a single strobe?  Should it be a long lasting
>> >> > signal?
>> >> > Can I use it to indicate to my block that it should be in reset?
>> >>
>> >> clear_tx_seqnum used to be a single cycle strobe in the ce_clk domain
>> >> but it is now a long lasting signal in the ce_clk domain.
>> >> clear_tx_seqnum will be asserted for at least one clock cycle, with no
>> >> upper-bound on the deassertion time. You can use it hold your control
>> >> and data-path logic in a quiescent state without having to change the
>> >> software-configured settings like settings reg values, etc.
>> >
>> >
>> > I'm not sure the helpfulness of this change at all, in all honesty.
>> >
>> > If no packets are flowing through the block, why does the signal need to
>> > be
>> > asserted at all?  It seems like it was used as a reset/clear in the
>> > past,
>> > and now it's meant as a hold?
>>
>> By quiescent I meant "idle between applications". It's still a
>> reset/clear signal that tells noc_shell, axi_wrapper and the client
>> logic to move its state machines (and any relevant registers) to the
>> idle state to await packets from a new stream. In noc_shell, this
>> signal resets all flow control information. In axi_wrapper it resets
>> the CHDR framer and deframer. Depending on what's in the client logic
>> in the noc_block, you may or may not care about this signal. For
>> example, if you have a simple multiply-by-const block as the client
>> logic, then the clear_tx_seqnum signal can pretty much be ignored
>> because you don't care if a the next packet/sample is from a new
>> stream. For an FIR filter, you do care because you need to zero out
>> your accumulators when a stream is restarted. In rfnoc, we make a
>> distinction between reset and clear. A "reset" is meant to reset
>> everything in a module to the power-up state. A "clear" only resets
>> things that are not software controlled, like settings regs. The
>> expectation is that you can configure your block, clear it to re-arm
>> packet processing engines, etc and still preserve the settings you
>> configured.
>
>
> Thanks for the explanation.  Can you give a use case when this might happen?
> Can you time division multiplex blocks in the FPGA for processing with
> multiple, different streams without reconfiguring the whole flowgraph?
> Under what circumstances does clear_tx_seqnum get deasserted, and where in
> the stream lifecycle does that happen?  I've been putting my own reset
> register to strobe at the beginning of the constructor for my block.  Is
> this best practice?
>
> I'm really curious what the lifecycle of the block is supposed to be both
> from a hardware/stream point of view as well as well as the concept of host
> software and flowgraph lifecycle.  If clear_tx_seqnum is high, should
> configuration and settings changes be disallowed or allowed?
>
> Thanks,
> Brian

_______________________________________________
USRP-users mailing list
USRP-users@lists.ettus.com
http://lists.ettus.com/mailman/listinfo/usrp-users_lists.ettus.com

Reply via email to