Hi Akhil,
> -----Original Message-----
> From: Akhil Goyal <gak...@marvell.com>
> Sent: Thursday, September 22, 2022 7:20 AM
> To: Chautru, Nicolas <nicolas.chau...@intel.com>; dev@dpdk.org;
> tho...@monjalon.net; hemant.agra...@nxp.com
> Cc: maxime.coque...@redhat.com; t...@redhat.com; m...@ashroe.eu;
> Richardson, Bruce <bruce.richard...@intel.com>;
> david.march...@redhat.com; step...@networkplumber.org; Zhang,
> Mingshan <mingshan.zh...@intel.com>
> Subject: RE: [EXT] [PATCH v7 5/7] bbdev: add new operation for FFT
> processing
>
> > Hi Akhil,
> >
> > > -----Original Message-----
> > > From: Akhil Goyal <gak...@marvell.com>
> > >
> > > > Extension of bbdev operation to support FFT based operations.
> > > >
> > > > Signed-off-by: Nicolas Chautru <nicolas.chau...@intel.com>
> > > > Acked-by: Hemant Agrawal <hemant.agra...@nxp.com>
> > > > Acked-by: Maxime Coquelin <maxime.coque...@redhat.com>
> > > > ---
> > > > doc/guides/prog_guide/bbdev.rst | 130
> > > > +++++++++++++++++++++++++++++++++++
> > > > lib/bbdev/rte_bbdev.c | 10 ++-
> > > > lib/bbdev/rte_bbdev.h | 76 ++++++++++++++++++++
> > > > lib/bbdev/rte_bbdev_op.h | 149
> > > > ++++++++++++++++++++++++++++++++++++++++
> > > > lib/bbdev/version.map | 4 ++
> > > > 5 files changed, 368 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/doc/guides/prog_guide/bbdev.rst
> > > > b/doc/guides/prog_guide/bbdev.rst index 70fa01a..5dcc7b5 100644
> > > > --- a/doc/guides/prog_guide/bbdev.rst
> > > > +++ b/doc/guides/prog_guide/bbdev.rst
> > > > @@ -1118,6 +1118,136 @@ Figure :numref:`figure_turbo_tb_decode`
> > > above
> > > > showing the Turbo decoding of CBs using BBDEV interface in TB-mode
> > > > is also valid for LDPC decode.
> > > >
> > > > +BBDEV FFT Operation
> > > > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > +
> > > > +This operation allows to run a combination of DFT and/or IDFT
> > > > +and/or
> > > > +time-
> > > > domain windowing.
> > > > +These can be used in a modular fashion (using bypass modes) or as
> > > > +a processing
> > > > pipeline
> > > > +which can be used for FFT-based baseband signal processing.
> > > > +In more details it allows :
> > > > +- to process the data first through an IDFT of adjustable size
> > > > +and padding;
> > > > +- to perform the windowing as a programmable cyclic shift offset
> > > > +of the data
> > > > followed by a
> > > > +pointwise multiplication by a time domain window;
> > > > +- to process the related data through a DFT of adjustable size
> > > > +and de-padding
> > > > for each such cyclic
> > > > +shift output.
> > > > +
> > > > +A flexible number of Rx antennas are being processed in parallel
> > > > +with the same
> > > > configuration.
> > > > +The API allows more generally for flexibility in what the PMD may
> > > > +support
> > > > (capability flags) and
> > > > +flexibility to adjust some of the parameters of the processing.
> > > > +
> > > > +The operation/capability flags that can be set for each FFT
> > > > +operation are given
> > > > below.
> > > > +
> > > > + **NOTE:** The actual operation flags that may be used with a
> > > > + specific BBDEV PMD are dependent on the driver capabilities as
> > > > + reported via ``rte_bbdev_info_get()``, and may be a subset of
> > > > + those
> > > below.
> > > > +
> > > > ++--------------------------------------------------------------------+
> > > > +|Description of FFT capability flags |
> > > >
> > >
> ++============================================================
> > > ===
> > > > =====+
> > > > +|RTE_BBDEV_FFT_WINDOWING |
> > > > +| Set to enable/support windowing in time domain |
> > > > ++--------------------------------------------------------------------+
> > > > +|RTE_BBDEV_FFT_CS_ADJUSTMENT |
> > > > +| Set to enable/support the cyclic shift time offset adjustment |
> > > > ++--------------------------------------------------------------------+
> > > > +|RTE_BBDEV_FFT_DFT_BYPASS |
> > > > +| Set to bypass the DFT and use directly the IDFT as an option |
> > > > ++--------------------------------------------------------------------+
> > > > +|RTE_BBDEV_FFT_IDFT_BYPASS |
> > > > +| Set to bypass the IDFT and use directly the DFT as an option |
> > > > ++--------------------------------------------------------------------+
> > > > +|RTE_BBDEV_FFT_WINDOWING_BYPASS |
> > > > +| Set to bypass the time domain windowing as an option |
> > > > ++--------------------------------------------------------------------+
> > > > +|RTE_BBDEV_FFT_POWER_MEAS |
> > > > +| Set to provide an optional power measurement of the DFT output
> |
> > > > ++--------------------------------------------------------------------+
> > > > +|RTE_BBDEV_FFT_FP16_INPUT |
> > > > +| Set if the input data shall use FP16 format instead of INT16 |
> > > > ++--------------------------------------------------------------------+
> > > > +|RTE_BBDEV_FFT_FP16_OUTPUT |
> > > > +| Set if the output data shall use FP16 format instead of INT16 |
> > > > ++--------------------------------------------------------------------+
> > > > +
> > > > +The structure passed for each FFT operation is given below, with
> > > > +the operation flags forming a bitmask in the ``op_flags`` field.
> > > > +
> > > > +.. code-block:: c
> > > > +
> > > > + struct rte_bbdev_op_fft {
> > > > + struct rte_bbdev_op_data base_input;
> > > > + struct rte_bbdev_op_data base_output;
> > > > + struct rte_bbdev_op_data power_meas_output;
> > > > + uint32_t op_flags;
> > > > + uint16_t input_sequence_size;
> > > > + uint16_t input_leading_padding;
> > > > + uint16_t output_sequence_size;
> > > > + uint16_t output_leading_depadding;
> > > > + uint8_t window_index[RTE_BBDEV_MAX_CS_2];
> > > > + uint16_t cs_bitmap;
> > > > + uint8_t num_antennas_log2;
> > > > + uint8_t idft_log2;
> > > > + uint8_t dft_log2;
> > > > + int8_t cs_time_adjustment;
> > > > + int8_t idft_shift;
> > > > + int8_t dft_shift;
> > > > + uint16_t ncs_reciprocal;
> > > > + uint16_t power_shift;
> > > > + uint16_t fp16_exp_adjust;
> > > > + };
> > >
> > > Why is this codeblock added in this guide? Isn't it covered in the
> > > doxygen API doc?
> >
> > It is but here this detailed in context and with additional details.
> > Note that this is the exact same format being used for all the other
> > existing operations.
> > Are you okay to keep as is?
>
> I would suggest to remove it and others as well. I think we can add more info
> In the API guide if needed.
> Otherwise it will be difficult to maintain/track in future for any changes
> that
> are done in these structs.
Do we really want to do this now?
That documentation is tightly tied with the API, so the doc would always have
to be updated when the API structure is modified.
If you read this through most of the documentation would become barely readable
without direct pointer to the relevant structure and related parameters (not
just for FFT but also for the existing operations).
I see also some code snippets insertions for other parallel documentation when
relevant.
I can see your point about avoid duplicates but in that very case it is
arguably sensible to be readable and would personally not change it in so far
as I am concerned.
Are you okay to keep current documentation flow for now as is, and we can look
into improving/updating documentation as a future item based on agreed guidance?
Thanks
Nic
