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.chau...@intel.com> > Sent: Wednesday, July 6, 2022 4:28 PM > To: dev@dpdk.org; tho...@monjalon.net; gak...@marvell.com; > 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; Chautru, > Nicolas <nicolas.chau...@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.yi...@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.chau...@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.chau...@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
