On 9/28/23 01:46, Chautru, Nicolas wrote:
Hi Maxime,

-----Original Message-----
From: Maxime Coquelin <maxime.coque...@redhat.com>
Sent: Wednesday, September 27, 2023 6:40 AM
To: Chautru, Nicolas <nicolas.chau...@intel.com>; dev@dpdk.org
Cc: hemant.agra...@nxp.com; david.march...@redhat.com; Vargas, Hernan
<hernan.var...@intel.com>
Subject: Re: [PATCH v2 6/7] baseband/acc: introduce the new VRB2 variant



On 9/21/23 22:43, Nicolas Chautru wrote:
This extends the unified driver to support both the
VRB1 and VRB2 implementations of Intel vRAN Boost.

Signed-off-by: Nicolas Chautru <nicolas.chau...@intel.com>
---
   doc/guides/bbdevs/index.rst            |    1 +
   doc/guides/bbdevs/vrb2.rst             |  269 +++++
   doc/guides/rel_notes/release_23_11.rst |    3 +
   drivers/baseband/acc/acc_common.h      |   84 +-
   drivers/baseband/acc/rte_acc100_pmd.c  |    4 +-
   drivers/baseband/acc/rte_vrb_pmd.c     | 1442 +++++++++++++++++++++---
   drivers/baseband/acc/vrb1_pf_enum.h    |   17 +-
   drivers/baseband/acc/vrb2_pf_enum.h    |  124 ++
   drivers/baseband/acc/vrb2_vf_enum.h    |  121 ++
   drivers/baseband/acc/vrb_pmd.h         |  161 ++-
   10 files changed, 2062 insertions(+), 164 deletions(-)
   create mode 100644 doc/guides/bbdevs/vrb2.rst
   create mode 100644 drivers/baseband/acc/vrb2_pf_enum.h
   create mode 100644 drivers/baseband/acc/vrb2_vf_enum.h


As a general comment, the review process would be easier if the patch was
split by features introduced:
- Base init, no capabilities exposed
- Add LDPC capabilities and related code
- ADD FFT capabilities and related code
- MLD-TS ...

We had this very discussion last week at the techboard for a Net PMD, and it
was a shared opinion across the board.

Yes I realize there is more code that I first thought. Will do in v3.

Thanks, much appreciated!


diff --git a/doc/guides/bbdevs/index.rst b/doc/guides/bbdevs/index.rst
index 77d4c54664..269157d77f 100644
--- a/doc/guides/bbdevs/index.rst
+++ b/doc/guides/bbdevs/index.rst
@@ -15,4 +15,5 @@ Baseband Device Drivers
       fpga_5gnr_fec
       acc100
       vrb1
+    vrb2
       la12xx
diff --git a/doc/guides/bbdevs/vrb2.rst b/doc/guides/bbdevs/vrb2.rst
new file mode 100644 index 0000000000..8d8e094660
--- /dev/null
+++ b/doc/guides/bbdevs/vrb2.rst
@@ -0,0 +1,269 @@
+..  SPDX-License-Identifier: BSD-3-Clause
+    Copyright(c) 2023 Intel Corporation
+
+.. include:: <isonum.txt>
+
+Intel\ |reg| vRAN Boost v2 Poll Mode Driver (PMD)
+=================================================
+
+The Intel\ |reg| vRAN Boost integrated accelerator enables
+cost-effective 4G and 5G next-generation virtualized Radio Access
+Network (vRAN) solutions.
+The Intel vRAN Boost v2.0 (VRB2 in the code) is specifically
+integrated on the Intel\ |reg| Xeon\ |reg| Granite Rapids-D Process (GNR-
D).
+
+Features
+--------
+
+Intel vRAN Boost v2.0 includes a 5G Low Density Parity Check (LDPC)
+encoder/decoder, rate match/dematch, Hybrid Automatic Repeat Request
+(HARQ) with access to DDR memory for buffer management, a 4G Turbo
+encoder/decoder, a Fast Fourier Transform (FFT) block providing
+DFT/iDFT processing offload for the 5G Sounding Reference Signal
+(SRS), a MLD-TS accelerator, a Queue Manager (QMGR), and a DMA
subsystem.
+There is no dedicated on-card memory for HARQ, the coherent memory on
the CPU side is being used.
+
+These hardware blocks provide the following features exposed by the PMD:
+
+- LDPC Encode in the Downlink (5GNR)
+- LDPC Decode in the Uplink (5GNR)
+- Turbo Encode in the Downlink (4G)
+- Turbo Decode in the Uplink (4G)
+- FFT processing
+- MLD-TS processing
+- Single Root I/O Virtualization (SR-IOV) with 16 Virtual Functions
+(VFs) per Physical Function (PF)
+- Maximum of 2048 queues per VF
+- Message Signaled Interrupts (MSIs)
+
+The Intel vRAN Boost v2.0 PMD supports the following bbdev capabilities:
+
+* For the LDPC encode operation:
+   - ``RTE_BBDEV_LDPC_CRC_24B_ATTACH``: set to attach CRC24B to CB(s).
+   - ``RTE_BBDEV_LDPC_RATE_MATCH``: if set then do not do Rate Match
bypass.
+   - ``RTE_BBDEV_LDPC_INTERLEAVER_BYPASS``: if set then bypass
interleaver.
+   - ``RTE_BBDEV_LDPC_ENC_SCATTER_GATHER``: supports scatter-gather
for input/output data.
+   - ``RTE_BBDEV_LDPC_ENC_CONCATENATION``: concatenate code blocks
with bit granularity.
+
+* For the LDPC decode operation:
+   - ``RTE_BBDEV_LDPC_CRC_TYPE_24B_CHECK``: check CRC24B from CB(s).
+   - ``RTE_BBDEV_LDPC_CRC_TYPE_24B_DROP``: drops CRC24B bits
appended while decoding.
+   - ``RTE_BBDEV_LDPC_CRC_TYPE_24A_CHECK``: check CRC24A from CB(s).
+   - ``RTE_BBDEV_LDPC_CRC_TYPE_16_CHECK``: check CRC16 from CB(s).
+   - ``RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE``: provides an input for
HARQ combining.
+   - ``RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE``: provides an input
for HARQ combining.
+   - ``RTE_BBDEV_LDPC_ITERATION_STOP_ENABLE``: disable early
termination.
+   - ``RTE_BBDEV_LDPC_DEC_SCATTER_GATHER``: supports scatter-gather
for input/output data.
+   - ``RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION``: supports
compression of the HARQ input/output.
+   - ``RTE_BBDEV_LDPC_LLR_COMPRESSION``: supports LLR input
compression.
+   - ``RTE_BBDEV_LDPC_HARQ_4BIT_COMPRESSION``: supports
compression of the HARQ input/output.
+   - ``RTE_BBDEV_LDPC_SOFT_OUT_ENABLE``: set the APP LLR soft output.
+   - ``RTE_BBDEV_LDPC_SOFT_OUT_RM_BYPASS``: set the APP LLR soft
output after rate-matching.
+   - ``RTE_BBDEV_LDPC_SOFT_OUT_DEINTERLEAVER_BYPASS``: disables the
de-interleaver.
+
+* For the turbo encode operation:
+   - ``RTE_BBDEV_TURBO_CRC_24B_ATTACH``: set to attach CRC24B to
CB(s).
+   - ``RTE_BBDEV_TURBO_RATE_MATCH``: if set then do not do Rate Match
bypass.
+   - ``RTE_BBDEV_TURBO_ENC_INTERRUPTS``: set for encoder dequeue
interrupts.
+   - ``RTE_BBDEV_TURBO_RV_INDEX_BYPASS``: set to bypass RV index.
+   - ``RTE_BBDEV_TURBO_ENC_SCATTER_GATHER``: supports scatter-
gather for input/output data.
+
+* For the turbo decode operation:
+   - ``RTE_BBDEV_TURBO_CRC_TYPE_24B``: check CRC24B from CB(s).
+   - ``RTE_BBDEV_TURBO_SUBBLOCK_DEINTERLEAVE``: perform subblock
de-interleave.
+   - ``RTE_BBDEV_TURBO_DEC_INTERRUPTS``: set for decoder dequeue
interrupts.
+   - ``RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN``: set if negative LLR input is
supported.
+   - ``RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP``: keep CRC24B bits
appended while decoding.
+   - ``RTE_BBDEV_TURBO_DEC_CRC_24B_DROP``: option to drop the code
block CRC after decoding.
+   - ``RTE_BBDEV_TURBO_EARLY_TERMINATION``: set early termination
feature.
+   - ``RTE_BBDEV_TURBO_DEC_SCATTER_GATHER``: supports scatter-
gather for input/output data.
+   - ``RTE_BBDEV_TURBO_HALF_ITERATION_EVEN``: set half iteration
granularity.
+   - ``RTE_BBDEV_TURBO_SOFT_OUTPUT``: set the APP LLR soft output.
+   - ``RTE_BBDEV_TURBO_EQUALIZER``: set the turbo equalizer feature.
+   - ``RTE_BBDEV_TURBO_SOFT_OUT_SATURATE``: set the soft output
saturation.
+   - ``RTE_BBDEV_TURBO_CONTINUE_CRC_MATCH``: set to run an extra
odd iteration after CRC match.
+   - ``RTE_BBDEV_TURBO_NEG_LLR_1_BIT_SOFT_OUT``: set if negative APP
LLR output supported.
+   - ``RTE_BBDEV_TURBO_MAP_DEC``: supports flexible parallel MAP
engine decoding.
+
+* For the FFT operation:
+   - ``RTE_BBDEV_FFT_WINDOWING``: flexible windowing capability.
+   - ``RTE_BBDEV_FFT_CS_ADJUSTMENT``: flexible adjustment of Cyclic Shift
time offset.
+   - ``RTE_BBDEV_FFT_DFT_BYPASS``: set for bypass the DFT and get
directly into iDFT input.
+   - ``RTE_BBDEV_FFT_IDFT_BYPASS``: set for bypass the IDFT and get
directly the DFT output.
+   - ``RTE_BBDEV_FFT_WINDOWING_BYPASS``: set for bypass time domain
windowing.
+
+* For the MLD-TS operation:
+   - ``RTE_BBDEV_MLDTS_REP``: set to repeat and reuse channel across
operations.
+
+Installation
+------------
+
+Section 3 of the DPDK manual provides instructions on installing and
compiling DPDK.
+
+DPDK requires hugepages to be configured as detailed in section 2 of the
DPDK manual.
+The bbdev test application has been tested with a configuration 40 x 1GB
hugepages.
+The hugepage configuration of a server may be examined using:
+
+.. code-block:: console
+
+   grep Huge* /proc/meminfo
+
+
+Initialization
+--------------
+
+When the device first powers up, its PCI Physical Functions (PF) can
+be listed through these commands for Intel vRAN Boost v2:
+
+.. code-block:: console
+
+   sudo lspci -vd8086:57c2
+
+The physical and virtual functions are compatible with Linux UIO drivers:
+``vfio`` and ``igb_uio``.
+However, in order to work the 5G/4G FEC device first needs to be
+bound to one of these Linux drivers through DPDK.
+
+
+Bind PF UIO driver(s)
+~~~~~~~~~~~~~~~~~~~~~


Please document with VFIO instead of IGB_UIO, which is being deprecated.

There are still some deployment with igb_uio, so keeping for a bit. Then we can 
remove in all documentations.

This is a patch for upcoming v23.11 relase, not something that has
already been deployed.

I've had internal reports of collegues trying to experiment with BBDEV,
and trying to set it up with unsecure IGB UIO instead of VFIO because the DPDK documentation only provided guidance for IGB UIO.

The upstream documentation should document the best practice, which is,
AFAICS, VFIO token for your usecase.

But as suggests David, better to just point to "linux drivers" chapter
of the documentation, which already explains that and avoid duplication.



+Install the DPDK igb_uio driver, bind it with the PF PCI device ID
+and use ``lspci`` to confirm the PF device is under use by ``igb_uio`` DPDK
UIO driver.
+
+The igb_uio driver may be bound to the PF PCI device using one of two
+methods for Intel vRAN Boost v2:
+
+#. PCI functions (physical or virtual, depending on the use case) can
+be bound to the UIO driver by repeating this command for every function.
+
+.. code-block:: console
+
+   cd <dpdk-top-level-directory>
+   insmod build/kmod/igb_uio.ko
+   echo "8086 57c2" > /sys/bus/pci/drivers/igb_uio/new_id
+   lspci -vd8086:57c2
+
+#. Another way to bind PF with DPDK UIO driver is by using the
+``dpdk-devbind.py`` tool
+
+.. code-block:: console
+
+   cd <dpdk-top-level-directory>
+   usertools/dpdk-devbind.py -b igb_uio 0000:f7:00.0
+
+where the PCI device ID (example: 0000:f7:00.0) is obtained using ``lspci -
vd8086:57c2``.
+
+In a similar way the PF may be bound with vfio-pci as any PCIe device.
+
+
+Enable Virtual Functions
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+Now, it should be visible in the printouts that PCI PF is under
+igb_uio control "``Kernel driver in use: igb_uio``"
+
+To show the number of available VFs on the device, read ``sriov_totalvfs``
file.
+
+.. code-block:: console
+
+   cat /sys/bus/pci/devices/0000\:<b>\:<d>.<f>/sriov_totalvfs
+
+where ``0000\:<b>\:<d>.<f>`` is the PCI device ID
+
+To enable VFs via igb_uio, echo the number of virtual functions
+intended to enable to ``max_vfs`` file.
+
+.. code-block:: console
+
+   echo <num-of-vfs> >
+ /sys/bus/pci/devices/0000\:<b>\:<d>.<f>/max_vfs
+
+Afterwards, all VFs must be bound to appropriate UIO drivers as
+required, same way it was done with the physical function previously.
+
+Enabling SR-IOV via VFIO driver is pretty much the same, except that
+the file name is different:
+
+.. code-block:: console
+
+   echo <num-of-vfs> >
+ /sys/bus/pci/devices/0000\:<b>\:<d>.<f>/sriov_numvfs
+
+
+Configure the VFs through PF
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The PCI virtual functions must be configured before working or
+getting assigned to VMs/Containers.
+The configuration involves allocating the number of hardware queues,
+priorities, load balance, bandwidth and other settings necessary for
+the device to perform FEC functions.
+
+This configuration needs to be executed at least once after reboot or
+PCI FLR and can be achieved by using the functions
+``rte_acc_configure()``, which sets up the parameters defined in the
compatible ``rte_acc_conf`` structure.
+
+
+Test Application
+----------------
+
+The bbdev class is provided with a test application,
+``test-bbdev.py`` and range of test data for testing the
+functionality of the device, depending on the device's capabilities.
+The test application is located under app/test-bbdev folder and has
+the following options:
+
+.. code-block:: console
+
+   "-p", "--testapp-path": specifies path to the bbdev test app.
+   "-e", "--eal-params": EAL arguments which are passed to the test app.
+   "-t", "--timeout": Timeout in seconds (default=300).
+   "-c", "--test-cases": Defines test cases to run. Run all if not specified.
+   "-v", "--test-vector": Test vector path.
+   "-n", "--num-ops": Number of operations to process on device
(default=32).
+   "-b", "--burst-size": Operations enqueue/dequeue burst size (default=32).
+   "-s", "--snr": SNR in dB used when generating LLRs for bler tests.
+   "-s", "--iter_max": Number of iterations for LDPC decoder.
+   "-l", "--num-lcores": Number of lcores to run (default=16).
+   "-i", "--init-device": Initialise PF device with default values.
+
+
+To execute the test application tool using simple decode or encode
+data, type one of the following:
+
+.. code-block:: console
+
+  ./test-bbdev.py -c validation -n 64 -b 1 -v ./ldpc_dec_default.data
+ ./test-bbdev.py -c validation -n 64 -b 1 -v ./ldpc_enc_default.data
+
+
+The test application ``test-bbdev.py``, supports the ability to
+configure the PF device with a default set of values, if the "-i" or
+"- -init-device" option is included. The default values are defined in
test_bbdev_perf.c.
+
+
+Test Vectors
+~~~~~~~~~~~~
+
+In addition to the simple LDPC decoder and LDPC encoder tests, bbdev
+also provides a range of additional tests under the test_vectors
+folder, which may be useful.
+The results of these tests will depend on the device capabilities
+which may cause some test cases to be skipped, but no failure should be
reported.
+
+
+Alternate Baseband Device configuration tool
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+On top of the embedded configuration feature supported in test-bbdev
+using
+"- -init-device" option mentioned above, there is also a tool
+available to perform that device configuration using a companion
application.
+The ``pf_bb_config`` application notably enables then to run
+bbdev-test from the VF and not only limited to the PF as captured above.
+
+See for more details: https://github.com/intel/pf-bb-config
+
+Specifically for the bbdev Intel vRAN Boost v2 PMD, the command below
+can be used (note that ACC200 was used previously to refer to VRB2):
+
+.. code-block:: console
+
+   pf_bb_config VRB2 -c ./vrb2/vrb2_config_vf_5g.cfg
+   test-bbdev.py -e="-c 0xff0 -a${VF_PCI_ADDR}" -c validation -n 64
+ -b 64 -l 1 -v ./ldpc_dec_default.data
diff --git a/doc/guides/rel_notes/release_23_11.rst
b/doc/guides/rel_notes/release_23_11.rst
index 333e1d95a2..668dd58ee3 100644
--- a/doc/guides/rel_notes/release_23_11.rst
+++ b/doc/guides/rel_notes/release_23_11.rst
@@ -78,6 +78,9 @@ New Features
   * build: Optional libraries can now be selected with the new ``enable_libs``
     build option similarly to the existing ``enable_drivers`` build option.

+* **Updated Intel vRAN Boost bbdev PMD.**
+
+  Added support for the new Intel vRAN Boost v2 device variant (GNR-D)
within the unified driver.

   Removed Items
   -------------
diff --git a/drivers/baseband/acc/acc_common.h
b/drivers/baseband/acc/acc_common.h
index 5de58dbe36..b71292af94 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -18,6 +18,7 @@
   #define ACC_DMA_BLKID_OUT_HARQ      3
   #define ACC_DMA_BLKID_IN_HARQ       3
   #define ACC_DMA_BLKID_IN_MLD_R      3
+#define ACC_DMA_BLKID_DEWIN_IN      3

   /* Values used in filling in decode FCWs */
   #define ACC_FCW_TD_VER              1
@@ -103,6 +104,9 @@
   #define ACC_MAX_NUM_QGRPS              32
   #define ACC_RING_SIZE_GRANULARITY      64
   #define ACC_MAX_FCW_SIZE              128
+#define ACC_IQ_SIZE                    4
+
+#define ACC_FCW_FFT_BLEN_3             28

   /* Constants from K0 computation from 3GPP 38.212 Table 5.4.2.1-2 */
   #define ACC_N_ZC_1 66 /* N = 66 Zc for BG 1 */ @@ -132,6 +136,11 @@
   #define ACC_LIM_21 14 /* 0.21 */
   #define ACC_LIM_31 20 /* 0.31 */
   #define ACC_MAX_E (128 * 1024 - 2)
+#define ACC_MAX_CS 12
+
+#define ACC100_VARIANT          0
+#define VRB1_VARIANT           2
+#define VRB2_VARIANT           3

   /* Helper macro for logging */
   #define rte_acc_log(level, fmt, ...) \ @@ -332,6 +341,37 @@ struct
__rte_packed acc_fcw_fft {
                res:19;
   };

+/* FFT Frame Control Word. */
+struct __rte_packed acc_fcw_fft_3 {
+       uint32_t in_frame_size:16,
+               leading_pad_size:16;
+       uint32_t out_frame_size:16,
+               leading_depad_size:16;
+       uint32_t cs_window_sel;
+       uint32_t cs_window_sel2:16,
+               cs_enable_bmap:16;
+       uint32_t num_antennas:8,
+               idft_size:8,
+               dft_size:8,
+               cs_offset:8;
+       uint32_t idft_shift:8,
+               dft_shift:8,
+               cs_multiplier:16;
+       uint32_t bypass:2,
+               fp16_in:1,
+               fp16_out:1,
+               exp_adj:4,
+               power_shift:4,
+               power_en:1,
+               enable_dewin:1,
+               freq_resample_mode:2,
+               depad_ouput_size:16;
+       uint16_t cs_theta_0[ACC_MAX_CS];
+       uint32_t cs_theta_d[ACC_MAX_CS];
+       int8_t cs_time_offset[ACC_MAX_CS];
+};
+
+
   /* MLD-TS Frame Control Word */
   struct __rte_packed acc_fcw_mldts {
        uint32_t fcw_version:4,
@@ -473,14 +513,14 @@ union acc_info_ring_data {
                uint16_t valid: 1;
        };
        struct {
-               uint32_t aq_id_3: 6;
-               uint32_t qg_id_3: 5;
-               uint32_t vf_id_3: 6;
-               uint32_t int_nb_3: 6;
-               uint32_t msi_0_3: 1;
-               uint32_t vf2pf_3: 6;
-               uint32_t loop_3: 1;
-               uint32_t valid_3: 1;
+               uint32_t aq_id_vrb2: 6;
+               uint32_t qg_id_vrb2: 5;
+               uint32_t vf_id_vrb2: 6;
+               uint32_t int_nb_vrb2: 6;
+               uint32_t msi_0_vrb2: 1;
+               uint32_t vf2pf_vrb2: 6;
+               uint32_t loop_vrb2: 1;
+               uint32_t valid_vrb2: 1;
        };
   } __rte_packed;

@@ -765,16 +805,20 @@ alloc_sw_rings_min_mem(struct rte_bbdev *dev,
struct acc_device *d,
    */
   static inline uint16_t
   get_queue_id_from_ring_info(struct rte_bbdev_data *data,
-               const union acc_info_ring_data ring_data)
+               const union acc_info_ring_data ring_data, uint16_t
device_variant)

The device variant ID should be derived from the data, which stores device
private pointer. No need to add an extra parameter.

the device variant comes from directly forom dev-> not from dev->data.

Ok, then just pass dev as first argument? It is always passed as
dev->data currently.



   {
        uint16_t queue_id;
+       struct acc_queue *acc_q;
+       uint16_t vf_id = (device_variant == VRB2_VARIANT) ?
ring_data.vf_id_vrb2 : ring_data.vf_id;
+       uint16_t aq_id = (device_variant == VRB2_VARIANT) ?
ring_data.aq_id_vrb2 : ring_data.aq_id;
+       uint16_t qg_id = (device_variant == VRB2_VARIANT) ?
+ring_data.qg_id_vrb2 : ring_data.qg_id;

Wouldn't it be better to have an intermediate representation, and each variant
implements a callback to fill it?

It will become messy otherwise when other variants will be added.

Something like the existing queue_offset callback would be great.

ok fair enough.


Great, thanks.




        for (queue_id = 0; queue_id < data->num_queues; ++queue_id) {
-               struct acc_queue *acc_q =
-                               data->queues[queue_id].queue_private;
-               if (acc_q != NULL && acc_q->aq_id == ring_data.aq_id &&
-                               acc_q->qgrp_id == ring_data.qg_id &&
-                               acc_q->vf_id == ring_data.vf_id)
+               acc_q = data->queues[queue_id].queue_private;
+
+               if (acc_q != NULL && acc_q->aq_id == aq_id &&
+                               acc_q->qgrp_id == qg_id &&
+                               acc_q->vf_id == vf_id)
                        return queue_id;
        }

@@ -1436,4 +1480,16 @@ get_num_cbs_in_tb_ldpc_enc(struct
rte_bbdev_op_ldpc_enc *ldpc_enc)
        return cbs_in_tb;
   }

+#ifdef RTE_LIBRTE_BBDEV_DEBUG
+static inline void
+acc_memdump(const char *string, void *buf, uint16_t bytes) {
+       printf("%s\n", string);
+       uint32_t *data = buf;
+       uint16_t i;
+       for (i = 0; i < bytes / 4; i++)
+               printf("0x%08X\n", data[i]);
+}

For consistency and to avoid code dumplication, please use rte_memdump()
directly as you did in ACC100.

I can do for now, it is just that for format is quite poor. Will change.

Thanks, and by the way this is better for consistency as rte_memdump()
calls are outputed on stderr while acc_memdump() is on stdout.



+#endif
+
   #endif /* _ACC_COMMON_H_ */
diff --git a/drivers/baseband/acc/rte_acc100_pmd.c
b/drivers/baseband/acc/rte_acc100_pmd.c
index 5362d39c30..7f8d05b5a9 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -294,7 +294,7 @@ acc100_pf_interrupt_handler(struct rte_bbdev *dev)
                case ACC100_PF_INT_DMA_UL5G_DESC_IRQ:
                case ACC100_PF_INT_DMA_DL5G_DESC_IRQ:
                        deq_intr_det.queue_id =
get_queue_id_from_ring_info(
-                                       dev->data, *ring_data);
+                                       dev->data, *ring_data, acc100_dev-
device_variant);
                        if (deq_intr_det.queue_id == UINT16_MAX) {
                                rte_bbdev_log(ERR,
                                                "Couldn't find queue: aq_id:
%u, qg_id: %u, vf_id: %u", @@
-348,7 +348,7 @@ acc100_vf_interrupt_handler(struct rte_bbdev *dev)
                         */
                        ring_data->vf_id = 0;
                        deq_intr_det.queue_id =
get_queue_id_from_ring_info(
-                                       dev->data, *ring_data);
+                                       dev->data, *ring_data, acc100_dev-
device_variant);
                        if (deq_intr_det.queue_id == UINT16_MAX) {
                                rte_bbdev_log(ERR,
                                                "Couldn't find queue: aq_id:
%u, qg_id: %u", diff --git
a/drivers/baseband/acc/rte_vrb_pmd.c
b/drivers/baseband/acc/rte_vrb_pmd.c
index e82ed55ca7..a787592ec9 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -37,6 +37,15 @@ vrb1_queue_offset(bool pf_device, uint8_t vf_id,
uint8_t qgrp_id, uint16_t aq_id
                return ((qgrp_id << 7) + (aq_id << 3) +
VRB1_VfQmgrIngressAq);
   }

+static inline uint32_t
+vrb2_queue_offset(bool pf_device, uint8_t vf_id, uint8_t qgrp_id,
+uint16_t aq_id) {
+       if (pf_device)
+               return ((vf_id << 14) + (qgrp_id << 9) + (aq_id << 3) +
VRB2_PfQmgrIngressAq);
+       else
+               return ((qgrp_id << 9) + (aq_id << 3) +
VRB2_VfQmgrIngressAq); }
+
   enum {UL_4G = 0, UL_5G, DL_4G, DL_5G, FFT, MLD, NUM_ACC};

   /* Return the accelerator enum for a Queue Group Index. */ @@ -197,7
+206,7 @@ fetch_acc_config(struct rte_bbdev *dev)
        struct acc_device *d = dev->data->dev_private;
        struct rte_acc_conf *acc_conf = &d->acc_conf;
        uint8_t acc, qg;
-       uint32_t reg_aq, reg_len0, reg_len1, reg0, reg1;
+       uint32_t reg_aq, reg_len0, reg_len1, reg_len2, reg_len3, reg0, reg1,
+reg2, reg3;
        uint32_t reg_mode, idx;
        struct rte_acc_queue_topology *q_top = NULL;
        int qman_func_id[VRB_NUM_ACCS] = {ACC_ACCMAP_0,
ACC_ACCMAP_1, @@
-219,32 +228,81 @@ fetch_acc_config(struct rte_bbdev *dev)
        acc_conf->num_vf_bundles = 1;
        initQTop(acc_conf);

-       reg0 = acc_reg_read(d, d->reg_addr->qman_group_func);
-       reg1 = acc_reg_read(d, d->reg_addr->qman_group_func + 4);
-       for (qg = 0; qg < d->num_qgroups; qg++) {
-               reg_aq = acc_reg_read(d, d->queue_offset(d->pf_device, 0,
qg, 0));
-               if (reg_aq & ACC_QUEUE_ENABLE) {
-                       if (qg < ACC_NUM_QGRPS_PER_WORD)
-                               idx = (reg0 >> (qg * 4)) & 0x7;
+       if (d->device_variant == VRB1_VARIANT) {
+               reg0 = acc_reg_read(d, d->reg_addr->qman_group_func);
+               reg1 = acc_reg_read(d, d->reg_addr->qman_group_func + 4);
+               for (qg = 0; qg < d->num_qgroups; qg++) {
+                       reg_aq = acc_reg_read(d, d->queue_offset(d-
pf_device, 0, qg, 0));
+                       if (reg_aq & ACC_QUEUE_ENABLE) {
+                               if (qg < ACC_NUM_QGRPS_PER_WORD)
+                                       idx = (reg0 >> (qg * 4)) & 0x7;
+                               else
+                                       idx = (reg1 >> ((qg -
ACC_NUM_QGRPS_PER_WORD) * 4)) & 0x7;
+                               if (idx < VRB1_NUM_ACCS) {
+                                       acc = qman_func_id[idx];
+                                       updateQtop(acc, qg, acc_conf, d);
+                               }
+                       }
+               }
+
+               /* Check the depth of the AQs. */
+               reg_len0 = acc_reg_read(d, d->reg_addr->depth_log0_offset);
+               reg_len1 = acc_reg_read(d, d->reg_addr->depth_log1_offset);
+               for (acc = 0; acc < NUM_ACC; acc++) {
+                       qtopFromAcc(&q_top, acc, acc_conf);
+                       if (q_top->first_qgroup_index <
ACC_NUM_QGRPS_PER_WORD)
+                               q_top->aq_depth_log2 =
+                                               (reg_len0 >> (q_top-
first_qgroup_index * 4)) & 0xF;
                        else
-                               idx = (reg1 >> ((qg -
ACC_NUM_QGRPS_PER_WORD) * 4)) & 0x7;
-                       if (idx < VRB1_NUM_ACCS) {
-                               acc = qman_func_id[idx];
-                               updateQtop(acc, qg, acc_conf, d);
+                               q_top->aq_depth_log2 = (reg_len1 >> ((q_top-
first_qgroup_index -
+
        ACC_NUM_QGRPS_PER_WORD) * 4)) & 0xF;
+               }
+       } else {
+               reg0 = acc_reg_read(d, d->reg_addr->qman_group_func);
+               reg1 = acc_reg_read(d, d->reg_addr->qman_group_func + 4);
+               reg2 = acc_reg_read(d, d->reg_addr->qman_group_func + 8);
+               reg3 = acc_reg_read(d, d->reg_addr->qman_group_func + 12);
+               /* printf("Debug Function %08x %08x %08x %08x\n", reg0,
reg1, reg2, reg3);*/
+               for (qg = 0; qg < VRB2_NUM_QGRPS; qg++) {
+                       reg_aq = acc_reg_read(d, vrb2_queue_offset(d-
pf_device, 0, qg, 0));
+                       if (reg_aq & ACC_QUEUE_ENABLE) {
+                               /* printf("Qg enabled %d %x\n", qg,
reg_aq);*/
+                               if (qg / ACC_NUM_QGRPS_PER_WORD == 0)
+                                       idx = (reg0 >> ((qg %
ACC_NUM_QGRPS_PER_WORD) * 4)) & 0x7;
+                               else if (qg / ACC_NUM_QGRPS_PER_WORD ==
1)
+                                       idx = (reg1 >> ((qg %
ACC_NUM_QGRPS_PER_WORD) * 4)) & 0x7;
+                               else if (qg / ACC_NUM_QGRPS_PER_WORD ==
2)
+                                       idx = (reg2 >> ((qg %
ACC_NUM_QGRPS_PER_WORD) * 4)) & 0x7;
+                               else
+                                       idx = (reg3 >> ((qg %
ACC_NUM_QGRPS_PER_WORD) * 4)) & 0x7;
+                               if (idx < VRB_NUM_ACCS) {
+                                       acc = qman_func_id[idx];
+                                       updateQtop(acc, qg, acc_conf, d);
+                               }
                        }
                }
-       }

-       /* Check the depth of the AQs. */
-       reg_len0 = acc_reg_read(d, d->reg_addr->depth_log0_offset);
-       reg_len1 = acc_reg_read(d, d->reg_addr->depth_log1_offset);
-       for (acc = 0; acc < NUM_ACC; acc++) {
-               qtopFromAcc(&q_top, acc, acc_conf);
-               if (q_top->first_qgroup_index <
ACC_NUM_QGRPS_PER_WORD)
-                       q_top->aq_depth_log2 = (reg_len0 >> (q_top-
first_qgroup_index * 4)) & 0xF;
-               else
-                       q_top->aq_depth_log2 = (reg_len1 >> ((q_top-
first_qgroup_index -
-                                       ACC_NUM_QGRPS_PER_WORD) * 4))
& 0xF;
+               /* Check the depth of the AQs. */
+               reg_len0 = acc_reg_read(d, d->reg_addr->depth_log0_offset);
+               reg_len1 = acc_reg_read(d, d->reg_addr->depth_log0_offset +
4);
+               reg_len2 = acc_reg_read(d, d->reg_addr->depth_log0_offset +
8);
+               reg_len3 = acc_reg_read(d, d->reg_addr->depth_log0_offset +
12);
+
+               for (acc = 0; acc < NUM_ACC; acc++) {
+                       qtopFromAcc(&q_top, acc, acc_conf);
+                       if (q_top->first_qgroup_index /
ACC_NUM_QGRPS_PER_WORD == 0)
+                               q_top->aq_depth_log2 = (reg_len0 >> ((q_top-
first_qgroup_index %
+
        ACC_NUM_QGRPS_PER_WORD) * 4)) & 0xF;
+                       else if (q_top->first_qgroup_index /
ACC_NUM_QGRPS_PER_WORD == 1)
+                               q_top->aq_depth_log2 = (reg_len1 >> ((q_top-
first_qgroup_index %
+
        ACC_NUM_QGRPS_PER_WORD) * 4)) & 0xF;
+                       else if (q_top->first_qgroup_index /
ACC_NUM_QGRPS_PER_WORD == 2)
+                               q_top->aq_depth_log2 = (reg_len2 >> ((q_top-
first_qgroup_index %
+
        ACC_NUM_QGRPS_PER_WORD) * 4)) & 0xF;
+                       else
+                               q_top->aq_depth_log2 = (reg_len3 >> ((q_top-
first_qgroup_index %
+
        ACC_NUM_QGRPS_PER_WORD) * 4)) & 0xF;
+               }
        }

        /* Read PF mode. */
@@ -341,18 +399,29 @@ vrb_check_ir(struct acc_device *acc_dev)
        ring_data = acc_dev->info_ring + (acc_dev->info_ring_head &
ACC_INFO_RING_MASK);

        while (ring_data->valid) {
-               if ((ring_data->int_nb < ACC_PF_INT_DMA_DL_DESC_IRQ) || (
-                               ring_data->int_nb >
ACC_PF_INT_DMA_MLD_DESC_IRQ)) {
-                       rte_bbdev_log(WARNING, "InfoRing: ITR:%d
Info:0x%x",
-                                       ring_data->int_nb, ring_data-
detailed_info);
-                       /* Initialize Info Ring entry and move forward. */
-                       ring_data->val = 0;
+               if (acc_dev->device_variant == VRB1_VARIANT) {
+                       if ((ring_data->int_nb <
ACC_PF_INT_DMA_DL_DESC_IRQ) || (
+                                       ring_data->int_nb >
ACC_PF_INT_DMA_MLD_DESC_IRQ)) {
+                               rte_bbdev_log(WARNING, "InfoRing: ITR:%d
Info:0x%x",
+                                               ring_data->int_nb, ring_data-
detailed_info);
+                               /* Initialize Info Ring entry and move forward.
*/
+                               ring_data->val = 0;
+                       }
+               } else { /* VRB2_VARIANT */
+                       if ((ring_data->int_nb_vrb2 <
ACC_PF_INT_DMA_DL_DESC_IRQ) || (
+                                       ring_data->int_nb_vrb2 >
ACC_PF_INT_DMA_MLD_DESC_IRQ)) {
+                               rte_bbdev_log(WARNING, "InfoRing: ITR:%d
Info:0x%x",
+                                               ring_data->int_nb_vrb2,
ring_data->val);
+                               /* Initialize Info Ring entry and move forward.
*/
+                               ring_data->val = 0;
+                       }

Same as get_queue_id_from_ring_info().
Having a callback to get the different parameters (int_nb, vf_id, ...)
implemented by each variant would avoid duplication like above.

It would make it more easier to add new variants, i.e. just implement the
callback without needing to patch every function making use of these info.

ok, fair enough.

Thanks!




                }
                info_ring_head++;
                ring_data = acc_dev->info_ring + (info_ring_head &
ACC_INFO_RING_MASK);
        }
   }

+

Remove new line.

ok


   /* Interrupt handler triggered by dev for handling specific interrupt. */
   static void
   vrb_dev_interrupt_handler(void *cb_arg) @@ -361,16 +430,22 @@
vrb_dev_interrupt_handler(void *cb_arg)
        struct acc_device *acc_dev = dev->data->dev_private;
        volatile union acc_info_ring_data *ring_data;
        struct acc_deq_intr_details deq_intr_det;
+       uint16_t vf_id, aq_id, qg_id, int_nb;
+       bool isVrb1 = (acc_dev->device_variant == VRB1_VARIANT);

        ring_data = acc_dev->info_ring + (acc_dev->info_ring_head &
ACC_INFO_RING_MASK);

        while (ring_data->valid) {
+               vf_id = isVrb1 ? ring_data->vf_id : ring_data->vf_id_vrb2;
+               aq_id = isVrb1 ? ring_data->aq_id : ring_data->aq_id_vrb2;
+               qg_id = isVrb1 ? ring_data->qg_id : ring_data->qg_id_vrb2; > +
                int_nb = isVrb1 ? ring_data->int_nb : ring_data->int_nb_vrb2;

Same comment as above.

ok


                if (acc_dev->pf_device) {
                        rte_bbdev_log_debug(
-                                       "VRB1 PF Interrupt received, Info Ring
data: 0x%x -> %d",
-                                       ring_data->val, ring_data->int_nb);
+                                       "PF Interrupt received, Info Ring data:
0x%x -> %d",
+                                       ring_data->val, int_nb);

-                       switch (ring_data->int_nb) {
+                       switch (int_nb) {
                        case ACC_PF_INT_DMA_DL_DESC_IRQ:
                        case ACC_PF_INT_DMA_UL_DESC_IRQ:
                        case ACC_PF_INT_DMA_FFT_DESC_IRQ:
@@ -378,13 +453,11 @@ vrb_dev_interrupt_handler(void *cb_arg)
                        case ACC_PF_INT_DMA_DL5G_DESC_IRQ:
                        case ACC_PF_INT_DMA_MLD_DESC_IRQ:
                                deq_intr_det.queue_id =
get_queue_id_from_ring_info(
-                                               dev->data, *ring_data);
+                                               dev->data, *ring_data,
acc_dev->device_variant);
                                if (deq_intr_det.queue_id == UINT16_MAX) {
                                        rte_bbdev_log(ERR,
                                                        "Couldn't find queue:
aq_id: %u, qg_id: %u, vf_id: %u",
-                                                       ring_data->aq_id,
-                                                       ring_data->qg_id,
-                                                       ring_data->vf_id);
+                                                       aq_id, qg_id, vf_id);
                                        return;
                                }
                                rte_bbdev_pmd_callback_process(dev,
@@ -396,9 +469,9 @@ vrb_dev_interrupt_handler(void *cb_arg)
                        }
                } else {
                        rte_bbdev_log_debug(
-                                       "VRB1 VF Interrupt received, Info Ring
data: 0x%x\n",
+                                       "VRB VF Interrupt received, Info Ring
data: 0x%x\n",
                                        ring_data->val);
-                       switch (ring_data->int_nb) {
+                       switch (int_nb) {
                        case ACC_VF_INT_DMA_DL_DESC_IRQ:
                        case ACC_VF_INT_DMA_UL_DESC_IRQ:
                        case ACC_VF_INT_DMA_FFT_DESC_IRQ:
@@ -406,14 +479,16 @@ vrb_dev_interrupt_handler(void *cb_arg)
                        case ACC_VF_INT_DMA_DL5G_DESC_IRQ:
                        case ACC_VF_INT_DMA_MLD_DESC_IRQ:
                                /* VFs are not aware of their vf_id - it's set 
to
0.  */
-                               ring_data->vf_id = 0;
+                               if (acc_dev->device_variant ==
VRB1_VARIANT)
+                                       ring_data->vf_id = 0;
+                               else
+                                       ring_data->vf_id_vrb2 = 0;

Ditto, you also need a way to set these fields in a generic way.

ok


                                deq_intr_det.queue_id =
get_queue_id_from_ring_info(
-                                               dev->data, *ring_data);
+                                               dev->data, *ring_data,
acc_dev->device_variant);
                                if (deq_intr_det.queue_id == UINT16_MAX) {
                                        rte_bbdev_log(ERR,
                                                        "Couldn't find queue:
aq_id: %u, qg_id: %u",
-                                                       ring_data->aq_id,
-                                                       ring_data->qg_id);
+                                                       aq_id, qg_id);
                                        return;
                                }
                                rte_bbdev_pmd_callback_process(dev,
@@ -428,8 +503,7 @@ vrb_dev_interrupt_handler(void *cb_arg)
                /* Initialize Info Ring entry and move forward. */
                ring_data->val = 0;
                ++acc_dev->info_ring_head;
-               ring_data = acc_dev->info_ring +
-                               (acc_dev->info_ring_head &
ACC_INFO_RING_MASK);
+               ring_data = acc_dev->info_ring + (acc_dev->info_ring_head &
+ACC_INFO_RING_MASK);
        }
   }

@@ -461,7 +535,10 @@ allocate_info_ring(struct rte_bbdev *dev)
        phys_low  = (uint32_t)(info_ring_iova);
        acc_reg_write(d, d->reg_addr->info_ring_hi, phys_high);
        acc_reg_write(d, d->reg_addr->info_ring_lo, phys_low);
-       acc_reg_write(d, d->reg_addr->info_ring_en,
VRB1_REG_IRQ_EN_ALL);
+       if (d->device_variant == VRB1_VARIANT)
+               acc_reg_write(d, d->reg_addr->info_ring_en,
VRB1_REG_IRQ_EN_ALL);
+       else
+               acc_reg_write(d, d->reg_addr->info_ring_en,
VRB2_REG_IRQ_EN_ALL);

My guess is the mask value difference between the two is the VRB2 added
MLD-TS related interrupts, is it?

If so, it would be more future proof to configure this based on the device
capabilities instead of its variant?

There are some differences in these implementations, best to to keep them 
separate.

OK.



        d->info_ring_head = (acc_reg_read(d, d->reg_addr->info_ring_ptr) &
                        0xFFF) / sizeof(union acc_info_ring_data);
        return 0;
@@ -516,6 +593,7 @@ vrb_setup_queues(struct rte_bbdev *dev, uint16_t
num_queues, int socket_id)
        phys_high = (uint32_t)(d->sw_rings_iova >> 32);
        phys_low  = (uint32_t)(d->sw_rings_iova & ~(ACC_SIZE_64MBYTE-1));

+

Remove new line.

ok


        /* Read the populated cfg from device registers. */
        fetch_acc_config(dev);

@@ -540,6 +618,10 @@ vrb_setup_queues(struct rte_bbdev *dev, uint16_t
num_queues, int socket_id)
        acc_reg_write(d, d->reg_addr->dma_ring_dl4g_lo, phys_low);
        acc_reg_write(d, d->reg_addr->dma_ring_fft_hi, phys_high);
        acc_reg_write(d, d->reg_addr->dma_ring_fft_lo, phys_low);
+       if (d->device_variant == VRB2_VARIANT) {
+               acc_reg_write(d, d->reg_addr->dma_ring_mld_hi, phys_high);
+               acc_reg_write(d, d->reg_addr->dma_ring_mld_lo, phys_low);

Same, wouldn't it be more future-proof to set this based on the capabilities of
the device and not its variant?

Checking capability on the fly would be cumbersome I think.
I dont see problem related to next gen extension for this.

Something like this is not possible?
If (has_caps(dev, RTE_BBDEV_OP_MLDTS))

Thanks


+       }
        /*
         * Configure Ring Size to the max queue ring size
         * (used for wrapping purpose).
@@ -549,8 +631,7 @@ vrb_setup_queues(struct rte_bbdev *dev, uint16_t
num_queues, int socket_id)

        /* Configure tail pointer for use when SDONE enabled. */
        if (d->tail_ptrs == NULL)
-               d->tail_ptrs = rte_zmalloc_socket(
-                               dev->device->driver->name,
+               d->tail_ptrs = rte_zmalloc_socket(dev->device->driver->name,
                                VRB_MAX_QGRPS * VRB_MAX_AQS *
sizeof(uint32_t),
                                RTE_CACHE_LINE_SIZE, socket_id);
        if (d->tail_ptrs == NULL) {
@@ -574,6 +655,10 @@ vrb_setup_queues(struct rte_bbdev *dev, uint16_t
num_queues, int socket_id)
        acc_reg_write(d, d->reg_addr->tail_ptrs_dl4g_lo, phys_low);
        acc_reg_write(d, d->reg_addr->tail_ptrs_fft_hi, phys_high);
        acc_reg_write(d, d->reg_addr->tail_ptrs_fft_lo, phys_low);
+       if (d->device_variant == VRB2_VARIANT) {
+               acc_reg_write(d, d->reg_addr->tail_ptrs_mld_hi, phys_high);
+               acc_reg_write(d, d->reg_addr->tail_ptrs_mld_lo, phys_low);
+       }

Ditto.


        ret = allocate_info_ring(dev);
        if (ret < 0) {
@@ -671,10 +756,17 @@ vrb_intr_enable(struct rte_bbdev *dev)
                        return ret;
                }

-               if (acc_dev->pf_device)
-                       max_queues = VRB1_MAX_PF_MSIX;
-               else
-                       max_queues = VRB1_MAX_VF_MSIX;
+               if (d->device_variant == VRB1_VARIANT) {
+                       if (acc_dev->pf_device)
+                               max_queues = VRB1_MAX_PF_MSIX;
+                       else
+                               max_queues = VRB1_MAX_VF_MSIX;
+               } else {
+                       if (acc_dev->pf_device)
+                               max_queues = VRB2_MAX_PF_MSIX;
+                       else
+                               max_queues = VRB2_MAX_VF_MSIX;
+               }

                if (rte_intr_efd_enable(dev->intr_handle, max_queues)) {
                        rte_bbdev_log(ERR, "Failed to create fds for %u
queues", @@
-776,7 +868,10 @@ vrb_find_free_queue_idx(struct rte_bbdev *dev,
                        /* Mark the Queue as assigned. */
                        d->q_assigned_bit_map[group_idx] |= (1ULL <<
aq_idx);
                        /* Report the AQ Index. */
-                       return (group_idx << VRB1_GRP_ID_SHIFT) + aq_idx;
+                       if (d->device_variant == VRB1_VARIANT)
+                               return (group_idx << VRB1_GRP_ID_SHIFT) +
aq_idx;
+                       else
+                               return (group_idx << VRB2_GRP_ID_SHIFT) +
aq_idx;

This is the same bitmap as in acc_info_ring_data, isn't it?
We could maybe share some code to to the decoding as suggested ealier in
this patch.

you mean just to add an inline function for the grp_id_shift(variant) right?
fair enough

A per-variant callback like I suggested for acc_info_ring_data decoder,
yes.




                }
        }
        rte_bbdev_log(INFO, "Failed to find free queue on %s, priority %u",
@@ -819,6 +914,9 @@ vrb_queue_setup(struct rte_bbdev *dev, uint16_t
queue_id,
                        ACC_FCW_LD_BLEN : (conf->op_type ==
RTE_BBDEV_OP_FFT ?
                        ACC_FCW_FFT_BLEN : ACC_FCW_MLDTS_BLEN))));

+       if ((q->d->device_variant == VRB2_VARIANT) && (conf->op_type ==
RTE_BBDEV_OP_FFT))
+               fcw_len = ACC_FCW_FFT_BLEN_3;
+
        for (desc_idx = 0; desc_idx < d->sw_ring_max_depth; desc_idx++) {
                desc = q->ring_addr + desc_idx;
                desc->req.word0 = ACC_DMA_DESC_TYPE; @@ -915,9
+1013,16 @@
vrb_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
                }
        }

-       q->qgrp_id = (q_idx >> VRB1_GRP_ID_SHIFT) & 0xF;
-       q->vf_id = (q_idx >> VRB1_VF_ID_SHIFT)  & 0x3F;
-       q->aq_id = q_idx & 0xF;
+       if (d->device_variant == VRB1_VARIANT) {
+               q->qgrp_id = (q_idx >> VRB1_GRP_ID_SHIFT) & 0xF;
+               q->vf_id = (q_idx >> VRB1_VF_ID_SHIFT)  & 0x3F;
+               q->aq_id = q_idx & 0xF;
+       } else {
+               q->qgrp_id = (q_idx >> VRB2_GRP_ID_SHIFT) & 0x1F;
+               q->vf_id = (q_idx >> VRB2_VF_ID_SHIFT)  & 0x3F;
+               q->aq_id = q_idx & 0x3F;
+       }
+

Same remark as before.

yes, still valid


        q->aq_depth = 0;
        if (conf->op_type ==  RTE_BBDEV_OP_TURBO_DEC)
                q->aq_depth = (1 << d->acc_conf.q_ul_4g.aq_depth_log2);
@@ -1149,6 +1254,127 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct
rte_bbdev_driver_info *dev_info)
                RTE_BBDEV_END_OF_CAPABILITIES_LIST()
        };

+       static const struct rte_bbdev_op_cap vrb2_bbdev_capabilities[] = {
+               {
+                       .type = RTE_BBDEV_OP_TURBO_DEC,
+                       .cap.turbo_dec = {
+                               .capability_flags =
+
        RTE_BBDEV_TURBO_SUBBLOCK_DEINTERLEAVE |
+                                       RTE_BBDEV_TURBO_CRC_TYPE_24B |
+
        RTE_BBDEV_TURBO_DEC_CRC_24B_DROP |
+                                       RTE_BBDEV_TURBO_EQUALIZER |
+
        RTE_BBDEV_TURBO_SOFT_OUT_SATURATE |
+
        RTE_BBDEV_TURBO_HALF_ITERATION_EVEN |
+
        RTE_BBDEV_TURBO_CONTINUE_CRC_MATCH |
+                                       RTE_BBDEV_TURBO_SOFT_OUTPUT |
+
        RTE_BBDEV_TURBO_EARLY_TERMINATION |
+
        RTE_BBDEV_TURBO_DEC_INTERRUPTS |
+
        RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN |
+
        RTE_BBDEV_TURBO_NEG_LLR_1_BIT_SOFT_OUT |
+                                       RTE_BBDEV_TURBO_MAP_DEC |
+
        RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP |
+
        RTE_BBDEV_TURBO_DEC_SCATTER_GATHER,
+                               .max_llr_modulus = INT8_MAX,
+                               .num_buffers_src =
+
        RTE_BBDEV_TURBO_MAX_CODE_BLOCKS,
+                               .num_buffers_hard_out =
+
        RTE_BBDEV_TURBO_MAX_CODE_BLOCKS,
+                               .num_buffers_soft_out =
+
        RTE_BBDEV_TURBO_MAX_CODE_BLOCKS,
+                       }
+               },
+               {
+                       .type = RTE_BBDEV_OP_TURBO_ENC,
+                       .cap.turbo_enc = {
+                               .capability_flags =
+
        RTE_BBDEV_TURBO_CRC_24B_ATTACH |
+
        RTE_BBDEV_TURBO_RV_INDEX_BYPASS |
+                                       RTE_BBDEV_TURBO_RATE_MATCH |
+
        RTE_BBDEV_TURBO_ENC_INTERRUPTS |
+
        RTE_BBDEV_TURBO_ENC_SCATTER_GATHER,
+                               .num_buffers_src =
+
        RTE_BBDEV_TURBO_MAX_CODE_BLOCKS,
+                               .num_buffers_dst =
+
        RTE_BBDEV_TURBO_MAX_CODE_BLOCKS,
+                       }
+               },
+               {
+                       .type   = RTE_BBDEV_OP_LDPC_ENC,
+                       .cap.ldpc_enc = {
+                               .capability_flags =
+                                       RTE_BBDEV_LDPC_RATE_MATCH |
+                                       RTE_BBDEV_LDPC_CRC_24B_ATTACH
|
+
        RTE_BBDEV_LDPC_INTERLEAVER_BYPASS |
+                                       RTE_BBDEV_LDPC_ENC_INTERRUPTS
|
+
        RTE_BBDEV_LDPC_ENC_SCATTER_GATHER |
+
        RTE_BBDEV_LDPC_ENC_CONCATENATION,
+                               .num_buffers_src =
+
        RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
+                               .num_buffers_dst =
+
        RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
+                       }
+               },
+               {
+                       .type   = RTE_BBDEV_OP_LDPC_DEC,
+                       .cap.ldpc_dec = {
+                       .capability_flags =
+                               RTE_BBDEV_LDPC_CRC_TYPE_24B_CHECK |
+                               RTE_BBDEV_LDPC_CRC_TYPE_24B_DROP |
+                               RTE_BBDEV_LDPC_CRC_TYPE_24A_CHECK |
+                               RTE_BBDEV_LDPC_CRC_TYPE_16_CHECK |
+                               RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE
|
+
        RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE |
+                               RTE_BBDEV_LDPC_ITERATION_STOP_ENABLE
|
+                               RTE_BBDEV_LDPC_DEINTERLEAVER_BYPASS |
+                               RTE_BBDEV_LDPC_DEC_SCATTER_GATHER |
+
        RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION |
+
        RTE_BBDEV_LDPC_HARQ_4BIT_COMPRESSION |
+                               RTE_BBDEV_LDPC_LLR_COMPRESSION |
+                               RTE_BBDEV_LDPC_SOFT_OUT_ENABLE |
+                               RTE_BBDEV_LDPC_SOFT_OUT_RM_BYPASS |
+
        RTE_BBDEV_LDPC_SOFT_OUT_DEINTERLEAVER_BYPASS |
+                               RTE_BBDEV_LDPC_DEC_INTERRUPTS,
+                       .llr_size = 8,
+                       .llr_decimals = 2,
+                       .num_buffers_src =
+
        RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
+                       .num_buffers_hard_out =
+
        RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
+                       .num_buffers_soft_out = 0,
+                       }
+               },
+               {
+                       .type   = RTE_BBDEV_OP_FFT,
+                       .cap.fft = {
+                               .capability_flags =
+
        RTE_BBDEV_FFT_WINDOWING |
+
        RTE_BBDEV_FFT_CS_ADJUSTMENT |
+
        RTE_BBDEV_FFT_DFT_BYPASS |
+
        RTE_BBDEV_FFT_IDFT_BYPASS |
+                                               RTE_BBDEV_FFT_FP16_INPUT
|
+
        RTE_BBDEV_FFT_FP16_OUTPUT |
+
        RTE_BBDEV_FFT_POWER_MEAS |
+
        RTE_BBDEV_FFT_WINDOWING_BYPASS,
+                               .num_buffers_src =
+
        RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
+                               .num_buffers_dst =
+
        RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,

Is that a copy/pasta error or is it expected to have LDPC defines in FFT
capabilities?

I need to change that, this is not quite correct actually even previously.

Ok, that indeed looks fishy.

Probably through other serie though. No one uses this currently.

Or at the top of the series, so that we can backport it more easily.



+                       }
+               },
+               {
+                       .type   = RTE_BBDEV_OP_MLDTS,
+                       .cap.fft = {
+                               .capability_flags =
+                                               RTE_BBDEV_MLDTS_REP,
+                               .num_buffers_src =
+
        RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
+                               .num_buffers_dst =
+
        RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,

Same question here, that looks suspicious but maybe I'm wrong.

+                       }
+               },
+               RTE_BBDEV_END_OF_CAPABILITIES_LIST()
+       };
+
        static struct rte_bbdev_queue_conf default_queue_conf;
        default_queue_conf.socket = dev->data->socket_id;
        default_queue_conf.queue_size = ACC_MAX_QUEUE_DEPTH; @@ -
1193,7
+1419,10 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct
rte_bbdev_driver_info *dev_info)
        dev_info->default_queue_conf = default_queue_conf;
        dev_info->cpu_flag_reqs = NULL;
        dev_info->min_alignment = 1;
-       dev_info->capabilities = vrb1_bbdev_capabilities;
+       if (d->device_variant == VRB1_VARIANT)
+               dev_info->capabilities = vrb1_bbdev_capabilities;
+       else
+               dev_info->capabilities = vrb2_bbdev_capabilities;
        dev_info->harq_buffer_size = 0;

        vrb_check_ir(d);
@@ -1242,6 +1471,9 @@ static struct rte_pci_id pci_id_vrb_pf_map[] = {
        {
                RTE_PCI_DEVICE(RTE_VRB1_VENDOR_ID,
RTE_VRB1_PF_DEVICE_ID)
        },
+       {
+               RTE_PCI_DEVICE(RTE_VRB2_VENDOR_ID,
RTE_VRB2_PF_DEVICE_ID)
+       },
        {.device_id = 0},
   };

@@ -1250,6 +1482,9 @@ static struct rte_pci_id pci_id_vrb_vf_map[] = {
        {
                RTE_PCI_DEVICE(RTE_VRB1_VENDOR_ID,
RTE_VRB1_VF_DEVICE_ID)
        },
+       {
+               RTE_PCI_DEVICE(RTE_VRB2_VENDOR_ID,
RTE_VRB2_VF_DEVICE_ID)
+       },
        {.device_id = 0},
   };


I'm stopping the review here for now.

Thanks expecting a v3 this week based on your suggestions.

Great, thanks.
Maxime



Thanks,
Maxime



Reply via email to