[PATCH v5 0/7] bbdev changes for 22.11

[Chautru, Nicolas]

Hi Hemant, 

Could you please provide a +1 for that serie please? This has been under review for a while but would like to get it merged soon if possible. I believe you had already reviewed and acked a previous version. 
Much appreciated, thanks, 

Nic

> -----Original Message-----
> From: Chautru, Nicolas <nicolas.chautru at intel.com>
> Sent: Wednesday, July 6, 2022 4:28 PM
> To: dev at dpdk.org; thomas at monjalon.net; gakhil at marvell.com;
> hemant.agrawal at nxp.com
> Cc: maxime.coquelin at redhat.com; trix at redhat.com; mdr at ashroe.eu;
> Richardson, Bruce <bruce.richardson at intel.com>;
> david.marchand at redhat.com; stephen at networkplumber.org; Chautru,
> Nicolas <nicolas.chautru at intel.com>
> Subject: [PATCH v5 0/7] bbdev changes for 22.11
> 
> v5: update base on review from Tom Rix. Number of typos reported and
> resolved, removed the commit related to rw_lock for now, added a commit
> for code clean up from review, resolved one rebase issue between 2
> commits, used size of array for some bound check implementation. Thanks.
> v4: update to the last 2 commits to include function to print the queue status
> and a fix to the rte_lock within the wrong structure
> v3: update to device status info to also use padded size for the related array.
> Adding also 2 additionals commits to allow the API struc to expose more
> information related to queues corner cases/warning as well as an optional
> rw lock.
> Hemant, Maxime, this is planned for DPDK 21.11 but would like review/ack
> early is possible to get this applied earlier and due to time off this summer.
> Thanks
> Nic
> 
> --
> 
> Hi,
> 
> Agregating together in a single serie a number of bbdev api changes
> previously submitted over the last few months and all targeted for 22.11 (4
> different series detailed below). Related deprecation notice being pushed in
> 22.07 in parallel.
> * bbdev: add device status info
> * bbdev: add new operation for FFT processing
> * bbdev: add device info on queue topology
> * bbdev: allow operation type enum for growth
> 
> v2: Update to the RTE_BBDEV_COUNT removal based on feedback from
> Thomas/Stephen : rejecting out of range op type and adjusting the new
> name for the padded maximum value used for fixed size arrays.
> 
> ---
> 
> Previous cover letters agregated below:
> 
> * bbdev: add device status info
> https://patches.dpdk.org/project/dpdk/list/?series=23367
> 
> The updated structure will allow PMDs to expose through info_get what be
> may the status of the underlying accelerator, notably in case an HW error
> event having happened.
> 
> * bbdev: add new operation for FFT processing
> https://patches.dpdk.org/project/dpdk/list/?series=22111
> 
> This contribution adds a new operation type to the existing ones already
> supported by the bbdev PMDs.
> This set of operation is FFT-based processing for 5GNR baseband processing
> acceleration. This operates in the same lookaside fashion as other existing
> bbdev operation with a dedicated set of capabilities and parameters (marked
> as experimental).
> 
> I plan to also include a new PMD supporting this operation (and most of the
> related capabilities) in the next couple of months (either in 22.06 or 22.09) as
> well as extending the related bbdev-test.
> 
> * bbdev: add device info on queue topology
> https://patches.dpdk.org/project/dpdk/list/?series=22076
> 
> Addressing an historical concern that the device info struct only imperfectly
> captured what queues are available on the device (number of operation and
> priority). This ended up being an iterative process for application to find each
> queue could be configured.
> 
> ie. the gap was captured as technical debt previously  in comments
> /* This isn't ideal because it reports the maximum number of queues but
>  * does not provide info on how many can be uplink/downlink or different
>  * priorities
>  */
> 
> This is now being exposed explictly based on the what the device actually
> supports using the existing info_get api
> 
> * bbdev: allow operation type enum for growth
> https://patches.dpdk.org/project/dpdk/list/?series=23509
> 
> This is related to the general intent to remove using MAX value for enums.
> There is consensus that we should avoid this for a while notably for future-
> proofed ABI concerns
> https://patches.dpdk.org/project/dpdk/patch/20200130142003.2645765-1-
> ferruh.yigit at intel.com/.
> But still there is arguably not yet an explicit best recommendation to handle
> this especially when we actualy need to expose array whose index is such an
> enum.
> As a specific example here I am refering to RTE_BBDEV_OP_TYPE_COUNT in
> enum rte_bbdev_op_type which is being extended for new operation type
> being support in bbdev (such as
> https://patches.dpdk.org/project/dpdk/patch/1646956157-245769-2-git-
> send-email-nicolas.chautru at intel.com/ adding new FFT operation)
> 
> There is also the intent to be able to expose information for each operation
> type through the bbdev api such as dynamically configured queues
> information per such operation type
> https://patches.dpdk.org/project/dpdk/patch/1646785355-168133-2-git-
> send-email-nicolas.chautru at intel.com/
> 
> Basically we are considering best way to accomodate for this, notably based
> on discussions with Ray Kinsella and Bruce Richardson, to handle such a case
> moving forward: specifically for the example with
> RTE_BBDEV_OP_TYPE_COUNT and also more generally.
> 
> One possible option is captured in that patchset and is basically based on the
> simple principle to allow for growth and prevent ABI breakage. Ie. the last
> value of the enum is set with a higher value than required so that to allow
> insertion of new enum outside of the major ABI versions.
> In that case the RTE_BBDEV_OP_TYPE_COUNT is still present and can be
> exposed and used while still allowing for addition thanks to the implicit
> padding-like room. As an alternate variant, instead of using that last enum
> value, that extended size could be exposed as an #define outside of the
> enum but would be fundamentally the same (public).
> 
> Another option would be to avoid array alltogether and use each time this a
> new dedicated API function (operation type enum being an input argument
> instead of an index to an array in an existing structure so that to get access
> to structure related to a given operation type enum) but that is arguably not
> well scalable within DPDK to use such a scheme for each enums and keep an
> uncluttered and clean API. In that very example that would be very odd
> indeed not to get this simply from info_get().
> 
> Some pros and cons, arguably the simple option in that patchset is a valid
> compromise option and a step in the right direction but we would like to
> know your view wrt best recommendation, or any other thought.
> 
> 
> 
> Nicolas Chautru (7):
>   bbdev: allow operation type enum for growth
>   bbdev: add device status info
>   bbdev: add device info on queue topology
>   drivers/baseband: update PMDs to expose queue per operation
>   bbdev: add new operation for FFT processing
>   bbdev: add queue related warning and status information
>   bbdev: remove unnecessary if-check
> 
>  app/test-bbdev/test_bbdev.c                        |   2 +-
>  app/test-bbdev/test_bbdev_perf.c                   |   6 +-
>  doc/guides/prog_guide/bbdev.rst                    | 130 +++++++++++++++++
>  drivers/baseband/acc100/rte_acc100_pmd.c           |  30 ++--
>  drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c |   9 ++
>  drivers/baseband/fpga_lte_fec/fpga_lte_fec.c       |   9 ++
>  drivers/baseband/la12xx/bbdev_la12xx.c             |  10 +-
>  drivers/baseband/null/bbdev_null.c                 |   1 +
>  drivers/baseband/turbo_sw/bbdev_turbo_software.c   |  12 ++
>  examples/bbdev_app/main.c                          |   2 +-
>  lib/bbdev/rte_bbdev.c                              |  57 +++++++-
>  lib/bbdev/rte_bbdev.h                              | 149 +++++++++++++++++++-
>  lib/bbdev/rte_bbdev_op.h                           | 156 ++++++++++++++++++++-
>  lib/bbdev/version.map                              |  11 ++
>  14 files changed, 555 insertions(+), 29 deletions(-)
> 
> --
> 1.8.3.1