[PATCH v3 00/12] acc baseband PMD fix and updates for 24.11

[PATCH v3 00/12] acc baseband PMD fix and updates for 24.11

[Hernan Vargas]

This series targets 24.11.
v3: Updated code with community recommendations. Added 2 commits for rte_free refactor and clean up of VRB1 capabilities.
v2: Rebased to the latest next-baseband-for-main which includes needed rte_bbdev lib updates.
v1: It includes a memory access fix, refactoring of queue allocation and general improvements.

Hernan Vargas (12):
  baseband/acc: fix access to deallocated mem
  baseband/acc: fix soft output bypass RM
  baseband/acc: queue allocation refactor
  baseband/acc: configure max queues per device
  baseband/acc: future proof structure comparison
  baseband/acc: enhance SW ring alignment
  baseband/acc: algorithm tuning for LDPC decoder
  baseband/acc: remove check on HARQ memory
  baseband/acc: reset ring data valid bit
  baseband/acc: cosmetic changes
  baseband/acc: rte free refactor
  baseband/acc: clean up of VRB1 capabilities

 doc/guides/bbdevs/vrb1.rst            |   3 -
 drivers/baseband/acc/acc_common.h     |  16 +-
 drivers/baseband/acc/rte_acc100_pmd.c |  61 ++---
 drivers/baseband/acc/rte_vrb_pmd.c    | 335 ++++++++++++++++----------
 4 files changed, 227 insertions(+), 188 deletions(-)

-- 
2.37.1

[PATCH v3 01/12] baseband/acc: fix access to deallocated mem

[Hernan Vargas]

Prevent op_addr access during queue_stop operation, as this memory may
have been deallocated.

Fixes: e640f6cdfa84 ("baseband/acc200: add LDPC processing")
Cc: stable@dpdk.org

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 36 ----------------------
 drivers/baseband/acc/rte_vrb_pmd.c    | 44 +--------------------------
 2 files changed, 1 insertion(+), 79 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index 5e6ee85e1321..c690d1492ba3 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -838,51 +838,15 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
 	return ret;
 }
 
-static inline void
-acc100_print_op(struct rte_bbdev_dec_op *op, enum rte_bbdev_op_type op_type,
-		uint16_t index)
-{
-	if (op == NULL)
-		return;
-	if (op_type == RTE_BBDEV_OP_LDPC_DEC)
-		rte_bbdev_log(DEBUG,
-			"  Op 5GUL %d %d %d %d %d %d %d %d %d %d %d %d",
-			index,
-			op->ldpc_dec.basegraph, op->ldpc_dec.z_c,
-			op->ldpc_dec.n_cb, op->ldpc_dec.q_m,
-			op->ldpc_dec.n_filler, op->ldpc_dec.cb_params.e,
-			op->ldpc_dec.op_flags, op->ldpc_dec.rv_index,
-			op->ldpc_dec.iter_max, op->ldpc_dec.iter_count,
-			op->ldpc_dec.harq_combined_input.length
-			);
-	else if (op_type == RTE_BBDEV_OP_LDPC_ENC) {
-		struct rte_bbdev_enc_op *op_dl = (struct rte_bbdev_enc_op *) op;
-		rte_bbdev_log(DEBUG,
-			"  Op 5GDL %d %d %d %d %d %d %d %d %d",
-			index,
-			op_dl->ldpc_enc.basegraph, op_dl->ldpc_enc.z_c,
-			op_dl->ldpc_enc.n_cb, op_dl->ldpc_enc.q_m,
-			op_dl->ldpc_enc.n_filler, op_dl->ldpc_enc.cb_params.e,
-			op_dl->ldpc_enc.op_flags, op_dl->ldpc_enc.rv_index
-			);
-	}
-}
-
 static int
 acc100_queue_stop(struct rte_bbdev *dev, uint16_t queue_id)
 {
 	struct acc_queue *q;
-	struct rte_bbdev_dec_op *op;
-	uint16_t i;
 
 	q = dev->data->queues[queue_id].queue_private;
 	rte_bbdev_log(INFO, "Queue Stop %d H/T/D %d %d %x OpType %d",
 			queue_id, q->sw_ring_head, q->sw_ring_tail,
 			q->sw_ring_depth, q->op_type);
-	for (i = 0; i < q->sw_ring_depth; ++i) {
-		op = (q->ring_addr + i)->req.op_addr;
-		acc100_print_op(op, q->op_type, i);
-	}
 	/* ignore all operations in flight and clear counters */
 	q->sw_ring_tail = q->sw_ring_head;
 	q->aq_enqueued = 0;
diff --git a/drivers/baseband/acc/rte_vrb_pmd.c b/drivers/baseband/acc/rte_vrb_pmd.c
index 646c12ad5cac..e3f98d6e421c 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -1048,58 +1048,16 @@ vrb_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
 	return ret;
 }
 
-static inline void
-vrb_print_op(struct rte_bbdev_dec_op *op, enum rte_bbdev_op_type op_type,
-		uint16_t index)
-{
-	if (op == NULL)
-		return;
-	if (op_type == RTE_BBDEV_OP_LDPC_DEC)
-		rte_bbdev_log(INFO,
-			"  Op 5GUL %d %d %d %d %d %d %d %d %d %d %d %d",
-			index,
-			op->ldpc_dec.basegraph, op->ldpc_dec.z_c,
-			op->ldpc_dec.n_cb, op->ldpc_dec.q_m,
-			op->ldpc_dec.n_filler, op->ldpc_dec.cb_params.e,
-			op->ldpc_dec.op_flags, op->ldpc_dec.rv_index,
-			op->ldpc_dec.iter_max, op->ldpc_dec.iter_count,
-			op->ldpc_dec.harq_combined_input.length
-			);
-	else if (op_type == RTE_BBDEV_OP_LDPC_ENC) {
-		struct rte_bbdev_enc_op *op_dl = (struct rte_bbdev_enc_op *) op;
-		rte_bbdev_log(INFO,
-			"  Op 5GDL %d %d %d %d %d %d %d %d %d",
-			index,
-			op_dl->ldpc_enc.basegraph, op_dl->ldpc_enc.z_c,
-			op_dl->ldpc_enc.n_cb, op_dl->ldpc_enc.q_m,
-			op_dl->ldpc_enc.n_filler, op_dl->ldpc_enc.cb_params.e,
-			op_dl->ldpc_enc.op_flags, op_dl->ldpc_enc.rv_index
-			);
-	} else if (op_type == RTE_BBDEV_OP_MLDTS) {
-		struct rte_bbdev_mldts_op *op_mldts = (struct rte_bbdev_mldts_op *) op;
-		rte_bbdev_log(INFO, "  Op MLD %d RBs %d NL %d Rp %d %d %x",
-				index,
-				op_mldts->mldts.num_rbs, op_mldts->mldts.num_layers,
-				op_mldts->mldts.r_rep,
-				op_mldts->mldts.c_rep, op_mldts->mldts.op_flags);
-	}
-}
-
 /* Stop queue and clear counters. */
 static int
 vrb_queue_stop(struct rte_bbdev *dev, uint16_t queue_id)
 {
 	struct acc_queue *q;
-	struct rte_bbdev_dec_op *op;
-	uint16_t i;
+
 	q = dev->data->queues[queue_id].queue_private;
 	rte_bbdev_log(INFO, "Queue Stop %d H/T/D %d %d %x OpType %d",
 			queue_id, q->sw_ring_head, q->sw_ring_tail,
 			q->sw_ring_depth, q->op_type);
-	for (i = 0; i < q->sw_ring_depth; ++i) {
-		op = (q->ring_addr + i)->req.op_addr;
-		vrb_print_op(op, q->op_type, i);
-	}
 	/* ignore all operations in flight and clear counters */
 	q->sw_ring_tail = q->sw_ring_head;
 	q->aq_enqueued = 0;
-- 
2.37.1

[PATCH v3 02/12] baseband/acc: fix soft output bypass RM

[Hernan Vargas]

Removing soft output bypass RM capability due to VRB2 device
limitations.

Fixes: b49fe052f9cd ("baseband/acc: add FEC capabilities for VRB2 variant")
Cc: stable@dpdk.org

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 drivers/baseband/acc/rte_vrb_pmd.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/baseband/acc/rte_vrb_pmd.c b/drivers/baseband/acc/rte_vrb_pmd.c
index e3f98d6e421c..52a683e4e49b 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -1272,7 +1272,6 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct rte_bbdev_driver_info *dev_info)
 				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,
@@ -1643,18 +1642,18 @@ vrb_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
 		fcw->so_en = check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_SOFT_OUT_ENABLE);
 		fcw->so_bypass_intlv = check_bit(op->ldpc_dec.op_flags,
 				RTE_BBDEV_LDPC_SOFT_OUT_DEINTERLEAVER_BYPASS);
-		fcw->so_bypass_rm = check_bit(op->ldpc_dec.op_flags,
-				RTE_BBDEV_LDPC_SOFT_OUT_RM_BYPASS);
+		fcw->so_bypass_rm = 0;
 		fcw->minsum_offset = 1;
 		fcw->dec_llrclip   = 2;
 	}
 
 	/*
-	 * These are all implicitly set
+	 * These are all implicitly set:
 	 * fcw->synd_post = 0;
 	 * fcw->dec_convllr = 0;
 	 * fcw->hcout_convllr = 0;
 	 * fcw->hcout_size1 = 0;
+	 * fcw->so_it = 0;
 	 * fcw->hcout_offset = 0;
 	 * fcw->negstop_th = 0;
 	 * fcw->negstop_it = 0;
-- 
2.37.1

Re: [PATCH v3 02/12] baseband/acc: fix soft output bypass RM

[Maxime Coquelin]

On 10/9/24 23:12, Hernan Vargas wrote:
> Removing soft output bypass RM capability due to VRB2 device
> limitations.
> 
> Fixes: b49fe052f9cd ("baseband/acc: add FEC capabilities for VRB2 variant")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   drivers/baseband/acc/rte_vrb_pmd.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

[PATCH v3 03/12] baseband/acc: queue allocation refactor

[Hernan Vargas]

Refactor to manage queue memory per operation more flexibly for VRB
devices.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 drivers/baseband/acc/acc_common.h  |   5 +
 drivers/baseband/acc/rte_vrb_pmd.c | 214 ++++++++++++++++++++---------
 2 files changed, 157 insertions(+), 62 deletions(-)

diff --git a/drivers/baseband/acc/acc_common.h b/drivers/baseband/acc/acc_common.h
index 38870c6458c4..70028fb5d235 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -149,6 +149,8 @@
 #define VRB2_VF_ID_SHIFT     6
 
 #define ACC_MAX_FFT_WIN      16
+#define ACC_MAX_RING_BUFFER  64
+#define VRB2_MAX_Q_PER_OP 256
 
 extern int acc_common_logtype;
 #define RTE_LOGTYPE_ACC_COMMON acc_common_logtype
@@ -581,6 +583,9 @@ struct acc_device {
 	void *sw_rings_base;  /* Base addr of un-aligned memory for sw rings */
 	void *sw_rings;  /* 64MBs of 64MB aligned memory for sw rings */
 	rte_iova_t sw_rings_iova;  /* IOVA address of sw_rings */
+	void *sw_rings_array[ACC_MAX_RING_BUFFER];  /* Array of aligned memory for sw rings. */
+	rte_iova_t sw_rings_iova_array[ACC_MAX_RING_BUFFER];  /* Array of sw_rings IOVA. */
+	uint32_t queue_index[ACC_MAX_RING_BUFFER]; /* Tracking queue index per ring buffer. */
 	/* Virtual address of the info memory routed to the this function under
 	 * operation, whether it is PF or VF.
 	 * HW may DMA information data at this location asynchronously
diff --git a/drivers/baseband/acc/rte_vrb_pmd.c b/drivers/baseband/acc/rte_vrb_pmd.c
index 52a683e4e49b..7e967ba97ac2 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -282,7 +282,7 @@ fetch_acc_config(struct rte_bbdev *dev)
 		/* 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++) {
+		for (acc = 0; acc < VRB1_NUM_ACCS; acc++) {
 			qtopFromAcc(&q_top, acc, acc_conf);
 			if (q_top->first_qgroup_index < ACC_NUM_QGRPS_PER_WORD)
 				q_top->aq_depth_log2 =
@@ -291,7 +291,7 @@ fetch_acc_config(struct rte_bbdev *dev)
 				q_top->aq_depth_log2 = (reg_len1 >> ((q_top->first_qgroup_index -
 						ACC_NUM_QGRPS_PER_WORD) * 4)) & 0xF;
 		}
-	} else {
+	} else if (d->device_variant == VRB2_VARIANT) {
 		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);
@@ -309,7 +309,7 @@ fetch_acc_config(struct rte_bbdev *dev)
 					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) {
+				if (idx < VRB2_NUM_ACCS) {
 					acc = qman_func_id[idx];
 					updateQtop(acc, qg, acc_conf, d);
 				}
@@ -322,7 +322,7 @@ fetch_acc_config(struct rte_bbdev *dev)
 		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++) {
+		for (acc = 0; acc < VRB2_NUM_ACCS; 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 %
@@ -544,6 +544,7 @@ vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
 {
 	uint32_t phys_low, phys_high, value;
 	struct acc_device *d = dev->data->dev_private;
+	uint16_t queues_per_op, i;
 	int ret;
 
 	if (d->pf_device && !d->acc_conf.pf_mode_en) {
@@ -565,27 +566,37 @@ vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
 		return -ENODEV;
 	}
 
-	alloc_sw_rings_min_mem(dev, d, num_queues, socket_id);
+	if (d->device_variant == VRB1_VARIANT) {
+		alloc_sw_rings_min_mem(dev, d, num_queues, socket_id);
 
-	/* If minimal memory space approach failed, then allocate
-	 * the 2 * 64MB block for the sw rings.
-	 */
-	if (d->sw_rings == NULL)
-		alloc_2x64mb_sw_rings_mem(dev, d, socket_id);
+		/* If minimal memory space approach failed, then allocate
+		 * the 2 * 64MB block for the sw rings.
+		 */
+		if (d->sw_rings == NULL)
+			alloc_2x64mb_sw_rings_mem(dev, d, socket_id);
 
-	if (d->sw_rings == NULL) {
-		rte_bbdev_log(NOTICE,
-				"Failure allocating sw_rings memory");
-		return -ENOMEM;
+		if (d->sw_rings == NULL) {
+			rte_bbdev_log(NOTICE, "Failure allocating sw_rings memory");
+			return -ENOMEM;
+		}
+	} else if (d->device_variant == VRB2_VARIANT) {
+		queues_per_op = RTE_MIN(VRB2_MAX_Q_PER_OP, num_queues);
+		for (i = 0; i <= RTE_BBDEV_OP_MLDTS; i++) {
+			alloc_sw_rings_min_mem(dev, d, queues_per_op, socket_id);
+			if (d->sw_rings == NULL) {
+				rte_bbdev_log(NOTICE, "Failure allocating sw_rings memory %d", i);
+				return -ENOMEM;
+			}
+			/* Moves the pointer to the relevant array. */
+			d->sw_rings_array[i] = d->sw_rings;
+			d->sw_rings_iova_array[i] = d->sw_rings_iova;
+			d->sw_rings = NULL;
+			d->sw_rings_base = NULL;
+			d->sw_rings_iova = 0;
+			d->queue_index[i] = 0;
+		}
 	}
 
-	/* Configure device with the base address for DMA descriptor rings.
-	 * Same descriptor rings used for UL and DL DMA Engines.
-	 * Note : Assuming only VF0 bundle is used for PF mode.
-	 */
-	phys_high = (uint32_t)(d->sw_rings_iova >> 32);
-	phys_low  = (uint32_t)(d->sw_rings_iova & ~(ACC_SIZE_64MBYTE-1));
-
 	/* Read the populated cfg from device registers. */
 	fetch_acc_config(dev);
 
@@ -600,20 +611,60 @@ vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
 	if (d->pf_device)
 		acc_reg_write(d, VRB1_PfDmaAxiControl, 1);
 
-	acc_reg_write(d, d->reg_addr->dma_ring_ul5g_hi, phys_high);
-	acc_reg_write(d, d->reg_addr->dma_ring_ul5g_lo, phys_low);
-	acc_reg_write(d, d->reg_addr->dma_ring_dl5g_hi, phys_high);
-	acc_reg_write(d, d->reg_addr->dma_ring_dl5g_lo, phys_low);
-	acc_reg_write(d, d->reg_addr->dma_ring_ul4g_hi, phys_high);
-	acc_reg_write(d, d->reg_addr->dma_ring_ul4g_lo, phys_low);
-	acc_reg_write(d, d->reg_addr->dma_ring_dl4g_hi, phys_high);
-	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);
+	if (d->device_variant == VRB1_VARIANT) {
+		/* Configure device with the base address for DMA descriptor rings.
+		 * Same descriptor rings used for UL and DL DMA Engines.
+		 * Note : Assuming only VF0 bundle is used for PF mode.
+		 */
+		phys_high = (uint32_t)(d->sw_rings_iova >> 32);
+		phys_low  = (uint32_t)(d->sw_rings_iova & ~(ACC_SIZE_64MBYTE-1));
+		acc_reg_write(d, d->reg_addr->dma_ring_ul5g_hi, phys_high);
+		acc_reg_write(d, d->reg_addr->dma_ring_ul5g_lo, phys_low);
+		acc_reg_write(d, d->reg_addr->dma_ring_dl5g_hi, phys_high);
+		acc_reg_write(d, d->reg_addr->dma_ring_dl5g_lo, phys_low);
+		acc_reg_write(d, d->reg_addr->dma_ring_ul4g_hi, phys_high);
+		acc_reg_write(d, d->reg_addr->dma_ring_ul4g_lo, phys_low);
+		acc_reg_write(d, d->reg_addr->dma_ring_dl4g_hi, phys_high);
+		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);
+	} else if (d->device_variant == VRB2_VARIANT) {
+		/* Configure device with the base address for DMA descriptor rings.
+		 * Different ring buffer used for each operation type.
+		 * Note : Assuming only VF0 bundle is used for PF mode.
+		 */
+		acc_reg_write(d, d->reg_addr->dma_ring_ul5g_hi,
+				(uint32_t)(d->sw_rings_iova_array[RTE_BBDEV_OP_LDPC_DEC] >> 32));
+		acc_reg_write(d, d->reg_addr->dma_ring_ul5g_lo,
+				(uint32_t)(d->sw_rings_iova_array[RTE_BBDEV_OP_LDPC_DEC]
+				& ~(ACC_SIZE_64MBYTE - 1)));
+		acc_reg_write(d, d->reg_addr->dma_ring_dl5g_hi,
+				(uint32_t)(d->sw_rings_iova_array[RTE_BBDEV_OP_LDPC_ENC] >> 32));
+		acc_reg_write(d, d->reg_addr->dma_ring_dl5g_lo,
+				(uint32_t)(d->sw_rings_iova_array[RTE_BBDEV_OP_LDPC_ENC]
+				& ~(ACC_SIZE_64MBYTE - 1)));
+		acc_reg_write(d, d->reg_addr->dma_ring_ul4g_hi,
+				(uint32_t)(d->sw_rings_iova_array[RTE_BBDEV_OP_TURBO_DEC] >> 32));
+		acc_reg_write(d, d->reg_addr->dma_ring_ul4g_lo,
+				(uint32_t)(d->sw_rings_iova_array[RTE_BBDEV_OP_TURBO_DEC]
+				& ~(ACC_SIZE_64MBYTE - 1)));
+		acc_reg_write(d, d->reg_addr->dma_ring_dl4g_hi,
+				(uint32_t)(d->sw_rings_iova_array[RTE_BBDEV_OP_TURBO_ENC] >> 32));
+		acc_reg_write(d, d->reg_addr->dma_ring_dl4g_lo,
+				(uint32_t)(d->sw_rings_iova_array[RTE_BBDEV_OP_TURBO_ENC]
+				& ~(ACC_SIZE_64MBYTE - 1)));
+		acc_reg_write(d, d->reg_addr->dma_ring_fft_hi,
+				(uint32_t)(d->sw_rings_iova_array[RTE_BBDEV_OP_FFT] >> 32));
+		acc_reg_write(d, d->reg_addr->dma_ring_fft_lo,
+				(uint32_t)(d->sw_rings_iova_array[RTE_BBDEV_OP_FFT]
+				& ~(ACC_SIZE_64MBYTE - 1)));
+		acc_reg_write(d, d->reg_addr->dma_ring_mld_hi,
+				(uint32_t)(d->sw_rings_iova_array[RTE_BBDEV_OP_MLDTS] >> 32));
+		acc_reg_write(d, d->reg_addr->dma_ring_mld_lo,
+				(uint32_t)(d->sw_rings_iova_array[RTE_BBDEV_OP_MLDTS]
+				& ~(ACC_SIZE_64MBYTE - 1)));
 	}
+
 	/*
 	 * Configure Ring Size to the max queue ring size
 	 * (used for wrapping purpose).
@@ -637,19 +688,21 @@ vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
 
 	phys_high = (uint32_t)(d->tail_ptr_iova >> 32);
 	phys_low  = (uint32_t)(d->tail_ptr_iova);
-	acc_reg_write(d, d->reg_addr->tail_ptrs_ul5g_hi, phys_high);
-	acc_reg_write(d, d->reg_addr->tail_ptrs_ul5g_lo, phys_low);
-	acc_reg_write(d, d->reg_addr->tail_ptrs_dl5g_hi, phys_high);
-	acc_reg_write(d, d->reg_addr->tail_ptrs_dl5g_lo, phys_low);
-	acc_reg_write(d, d->reg_addr->tail_ptrs_ul4g_hi, phys_high);
-	acc_reg_write(d, d->reg_addr->tail_ptrs_ul4g_lo, phys_low);
-	acc_reg_write(d, d->reg_addr->tail_ptrs_dl4g_hi, phys_high);
-	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);
+	{
+		acc_reg_write(d, d->reg_addr->tail_ptrs_ul5g_hi, phys_high);
+		acc_reg_write(d, d->reg_addr->tail_ptrs_ul5g_lo, phys_low);
+		acc_reg_write(d, d->reg_addr->tail_ptrs_dl5g_hi, phys_high);
+		acc_reg_write(d, d->reg_addr->tail_ptrs_dl5g_lo, phys_low);
+		acc_reg_write(d, d->reg_addr->tail_ptrs_ul4g_hi, phys_high);
+		acc_reg_write(d, d->reg_addr->tail_ptrs_ul4g_lo, phys_low);
+		acc_reg_write(d, d->reg_addr->tail_ptrs_dl4g_hi, phys_high);
+		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);
+		}
 	}
 
 	ret = allocate_info_ring(dev);
@@ -685,8 +738,13 @@ vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
 	rte_free(d->tail_ptrs);
 	d->tail_ptrs = NULL;
 free_sw_rings:
-	rte_free(d->sw_rings_base);
-	d->sw_rings = NULL;
+	if (d->device_variant == VRB1_VARIANT) {
+		rte_free(d->sw_rings_base);
+		d->sw_rings = NULL;
+	} else if (d->device_variant == VRB2_VARIANT) {
+		for (i = 0; i <= RTE_BBDEV_OP_MLDTS; i++)
+			rte_free(d->sw_rings_array[i]);
+	}
 
 	return ret;
 }
@@ -810,17 +868,34 @@ vrb_intr_enable(struct rte_bbdev *dev)
 static int
 vrb_dev_close(struct rte_bbdev *dev)
 {
+	int i;
 	struct acc_device *d = dev->data->dev_private;
+
 	vrb_check_ir(d);
-	if (d->sw_rings_base != NULL) {
-		rte_free(d->tail_ptrs);
-		rte_free(d->info_ring);
-		rte_free(d->sw_rings_base);
-		rte_free(d->harq_layout);
-		d->tail_ptrs = NULL;
-		d->info_ring = NULL;
-		d->sw_rings_base = NULL;
-		d->harq_layout = NULL;
+	if (d->device_variant == VRB1_VARIANT) {
+		if (d->sw_rings_base != NULL) {
+			rte_free(d->tail_ptrs);
+			rte_free(d->info_ring);
+			rte_free(d->sw_rings_base);
+			rte_free(d->harq_layout);
+			d->tail_ptrs = NULL;
+			d->info_ring = NULL;
+			d->sw_rings_base = NULL;
+			d->harq_layout = NULL;
+		}
+	} else if (d->device_variant == VRB2_VARIANT) {
+		if (d->sw_rings_array[1] != NULL) {
+			rte_free(d->tail_ptrs);
+			rte_free(d->info_ring);
+			rte_free(d->harq_layout);
+			d->tail_ptrs = NULL;
+			d->info_ring = NULL;
+			d->harq_layout = NULL;
+			for (i = 0; i <= RTE_BBDEV_OP_MLDTS; i++) {
+				rte_free(d->sw_rings_array[i]);
+				d->sw_rings_array[i] = NULL;
+			}
+		}
 	}
 	/* Ensure all in flight HW transactions are completed. */
 	usleep(ACC_LONG_WAIT);
@@ -891,8 +966,16 @@ vrb_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
 	}
 
 	q->d = d;
-	q->ring_addr = RTE_PTR_ADD(d->sw_rings, (d->sw_ring_size * queue_id));
-	q->ring_addr_iova = d->sw_rings_iova + (d->sw_ring_size * queue_id);
+	if (d->device_variant == VRB1_VARIANT) {
+		q->ring_addr = RTE_PTR_ADD(d->sw_rings, (d->sw_ring_size * queue_id));
+		q->ring_addr_iova = d->sw_rings_iova + (d->sw_ring_size * queue_id);
+	} else if (d->device_variant == VRB2_VARIANT) {
+		q->ring_addr = RTE_PTR_ADD(d->sw_rings_array[conf->op_type],
+				(d->sw_ring_size * d->queue_index[conf->op_type]));
+		q->ring_addr_iova = d->sw_rings_iova_array[conf->op_type] +
+				(d->sw_ring_size * d->queue_index[conf->op_type]);
+		d->queue_index[conf->op_type]++;
+	}
 
 	/* Prepare the Ring with default descriptor format. */
 	union acc_dma_desc *desc = NULL;
@@ -1347,8 +1430,14 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct rte_bbdev_driver_info *dev_info)
 	dev_info->queue_priority[RTE_BBDEV_OP_FFT] = d->acc_conf.q_fft.num_qgroups;
 	dev_info->queue_priority[RTE_BBDEV_OP_MLDTS] = d->acc_conf.q_mld.num_qgroups;
 	dev_info->max_num_queues = 0;
-	for (i = RTE_BBDEV_OP_NONE; i <= RTE_BBDEV_OP_MLDTS; i++)
+	for (i = RTE_BBDEV_OP_NONE; i <= RTE_BBDEV_OP_MLDTS; i++) {
+		if (unlikely(dev_info->num_queues[i] > VRB2_MAX_Q_PER_OP)) {
+			rte_bbdev_log(ERR, "Unexpected number of queues %d exposed for op %d",
+					dev_info->num_queues[i], i);
+			dev_info->num_queues[i] = VRB2_MAX_Q_PER_OP;
+		}
 		dev_info->max_num_queues += dev_info->num_queues[i];
+	}
 	dev_info->queue_size_lim = ACC_MAX_QUEUE_DEPTH;
 	dev_info->hardware_accelerated = true;
 	dev_info->max_dl_queue_priority =
@@ -4239,7 +4328,8 @@ vrb_bbdev_init(struct rte_bbdev *dev, struct rte_pci_driver *drv)
 			d->reg_addr = &vrb1_pf_reg_addr;
 		else
 			d->reg_addr = &vrb1_vf_reg_addr;
-	} else {
+	} else if ((pci_dev->id.device_id == RTE_VRB2_PF_DEVICE_ID) ||
+			(pci_dev->id.device_id == RTE_VRB2_VF_DEVICE_ID)) {
 		d->device_variant = VRB2_VARIANT;
 		d->queue_offset = vrb2_queue_offset;
 		d->num_qgroups = VRB2_NUM_QGRPS;
-- 
2.37.1

Re: [PATCH v3 03/12] baseband/acc: queue allocation refactor

[Maxime Coquelin]

On 10/9/24 23:12, Hernan Vargas wrote:
> Refactor to manage queue memory per operation more flexibly for VRB
> devices.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   drivers/baseband/acc/acc_common.h  |   5 +
>   drivers/baseband/acc/rte_vrb_pmd.c | 214 ++++++++++++++++++++---------
>   2 files changed, 157 insertions(+), 62 deletions(-)
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

[PATCH v3 04/12] baseband/acc: configure max queues per device

[Hernan Vargas]

Configure max_queues based on the number of queue groups and numbers of
AQS per device variant.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/baseband/acc/rte_vrb_pmd.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/baseband/acc/rte_vrb_pmd.c b/drivers/baseband/acc/rte_vrb_pmd.c
index 7e967ba97ac2..519385e82ed3 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -545,7 +545,7 @@ vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
 	uint32_t phys_low, phys_high, value;
 	struct acc_device *d = dev->data->dev_private;
 	uint16_t queues_per_op, i;
-	int ret;
+	int ret, max_queues = 0;
 
 	if (d->pf_device && !d->acc_conf.pf_mode_en) {
 		rte_bbdev_log(NOTICE,
@@ -672,10 +672,15 @@ vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
 	value = log2_basic(d->sw_ring_size / ACC_RING_SIZE_GRANULARITY);
 	acc_reg_write(d, d->reg_addr->ring_size, value);
 
+	if (d->device_variant == VRB1_VARIANT)
+		max_queues = VRB1_NUM_QGRPS * VRB1_NUM_AQS;
+	else if (d->device_variant == VRB2_VARIANT)
+		max_queues = VRB2_NUM_QGRPS * VRB2_NUM_AQS;
+
 	/* Configure tail pointer for use when SDONE enabled. */
 	if (d->tail_ptrs == NULL)
 		d->tail_ptrs = rte_zmalloc_socket(dev->device->driver->name,
-				VRB_MAX_QGRPS * VRB_MAX_AQS * sizeof(uint32_t),
+				max_queues * sizeof(uint32_t),
 				RTE_CACHE_LINE_SIZE, socket_id);
 	if (d->tail_ptrs == NULL) {
 		rte_bbdev_log(ERR, "Failed to allocate tail ptr for %s:%u",
-- 
2.37.1

[PATCH v3 05/12] baseband/acc: future proof structure comparison

[Hernan Vargas]

Some implementation in the PMD is based on some size assumption from
the bbdev structure, which should use sizeof instead to be more future
proof in case these structures change.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/baseband/acc/acc_common.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/baseband/acc/acc_common.h b/drivers/baseband/acc/acc_common.h
index 70028fb5d235..1d8fd24ba008 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -95,8 +95,8 @@
 #define ACC_COMPANION_PTRS             8
 #define ACC_FCW_VER                    2
 #define ACC_MUX_5GDL_DESC              6
-#define ACC_CMP_ENC_SIZE               20
-#define ACC_CMP_DEC_SIZE               24
+#define ACC_CMP_ENC_SIZE               (sizeof(struct rte_bbdev_op_ldpc_enc) - ACC_ENC_OFFSET)
+#define ACC_CMP_DEC_SIZE               (sizeof(struct rte_bbdev_op_ldpc_dec) - ACC_DEC_OFFSET)
 #define ACC_ENC_OFFSET                (32)
 #define ACC_DEC_OFFSET                (80)
 #define ACC_LIMIT_DL_MUX_BITS          534
-- 
2.37.1

[PATCH v3 06/12] baseband/acc: enhance SW ring alignment

[Hernan Vargas]

Calculate the aligned total size required for queue rings, ensuring that
the size is a power of two for proper memory allocation.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 drivers/baseband/acc/acc_common.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/baseband/acc/acc_common.h b/drivers/baseband/acc/acc_common.h
index 1d8fd24ba008..0c249d5b93fd 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -767,6 +767,7 @@ alloc_sw_rings_min_mem(struct rte_bbdev *dev, struct acc_device *d,
 	int i = 0;
 	uint32_t q_sw_ring_size = ACC_MAX_QUEUE_DEPTH * get_desc_len();
 	uint32_t dev_sw_ring_size = q_sw_ring_size * num_queues;
+	uint32_t alignment = q_sw_ring_size * rte_align32pow2(num_queues);
 	/* Free first in case this is a reconfiguration */
 	rte_free(d->sw_rings_base);
 
@@ -774,12 +775,12 @@ alloc_sw_rings_min_mem(struct rte_bbdev *dev, struct acc_device *d,
 	while (i < ACC_SW_RING_MEM_ALLOC_ATTEMPTS) {
 		/*
 		 * sw_ring allocated memory is guaranteed to be aligned to
-		 * q_sw_ring_size at the condition that the requested size is
-		 * less than the page size
+		 * the variable 'alignment' at the condition that the requested
+		 * size is less than the page size
 		 */
 		sw_rings_base = rte_zmalloc_socket(
 				dev->device->driver->name,
-				dev_sw_ring_size, q_sw_ring_size, socket);
+				dev_sw_ring_size, alignment, socket);
 
 		if (sw_rings_base == NULL) {
 			rte_acc_log(ERR,
-- 
2.37.1

Re: [PATCH v3 06/12] baseband/acc: enhance SW ring alignment

[Maxime Coquelin]

On 10/9/24 23:12, Hernan Vargas wrote:
> Calculate the aligned total size required for queue rings, ensuring that
> the size is a power of two for proper memory allocation.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   drivers/baseband/acc/acc_common.h | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

[PATCH v3 07/12] baseband/acc: algorithm tuning for LDPC decoder

[Hernan Vargas]

Reverting to MS1 version of the algorithm to improve MU1 fading
conditions.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/baseband/acc/rte_vrb_pmd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/baseband/acc/rte_vrb_pmd.c b/drivers/baseband/acc/rte_vrb_pmd.c
index 519385e82ed3..4d7535e9d99f 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -1363,7 +1363,7 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct rte_bbdev_driver_info *dev_info)
 				RTE_BBDEV_LDPC_SOFT_OUT_DEINTERLEAVER_BYPASS |
 				RTE_BBDEV_LDPC_DEC_INTERRUPTS,
 			.llr_size = 8,
-			.llr_decimals = 2,
+			.llr_decimals = 1,
 			.num_buffers_src =
 					RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
 			.num_buffers_hard_out =
@@ -1737,8 +1737,8 @@ vrb_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
 		fcw->so_bypass_intlv = check_bit(op->ldpc_dec.op_flags,
 				RTE_BBDEV_LDPC_SOFT_OUT_DEINTERLEAVER_BYPASS);
 		fcw->so_bypass_rm = 0;
-		fcw->minsum_offset = 1;
-		fcw->dec_llrclip   = 2;
+		fcw->minsum_offset = 0;
+		fcw->dec_llrclip   = 0;
 	}
 
 	/*
-- 
2.37.1

[PATCH v3 08/12] baseband/acc: remove check on HARQ memory

[Hernan Vargas]

Automatically reset HARQ memory to prevent errors and simplify usage.
In a way we can assume that the HARQ output operation will always
overwrite the buffer, so we can reset this from the driver to prevent
an error being reported when application fails to do this explicitly.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/baseband/acc/rte_vrb_pmd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/baseband/acc/rte_vrb_pmd.c b/drivers/baseband/acc/rte_vrb_pmd.c
index 4d7535e9d99f..f7a120688f5a 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -2596,8 +2596,9 @@ vrb_enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 	/* Hard output. */
 	mbuf_append(h_output_head, h_output, h_out_length);
 	if (op->ldpc_dec.harq_combined_output.length > 0) {
-		/* Push the HARQ output into host memory. */
+		/* Push the HARQ output into host memory overwriting existing data. */
 		struct rte_mbuf *hq_output_head, *hq_output;
+		op->ldpc_dec.harq_combined_output.data->data_len = 0;
 		hq_output_head = op->ldpc_dec.harq_combined_output.data;
 		hq_output = op->ldpc_dec.harq_combined_output.data;
 		hq_len = op->ldpc_dec.harq_combined_output.length;
-- 
2.37.1

[PATCH v3 09/12] baseband/acc: reset ring data valid bit

[Hernan Vargas]

Reset only the valid bit to keep info ring data notably for dumping.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/baseband/acc/rte_vrb_pmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/baseband/acc/rte_vrb_pmd.c b/drivers/baseband/acc/rte_vrb_pmd.c
index f7a120688f5a..42414823541e 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -412,7 +412,7 @@ vrb_check_ir(struct acc_device *acc_dev)
 			rte_bbdev_log(WARNING, "InfoRing: ITR:%d Info:0x%x",
 					int_nb, ring_data->detailed_info);
 			/* Initialize Info Ring entry and move forward. */
-			ring_data->val = 0;
+			ring_data->valid = 0;
 		}
 		info_ring_head++;
 		ring_data = acc_dev->info_ring + (info_ring_head & ACC_INFO_RING_MASK);
-- 
2.37.1

[PATCH v3 10/12] baseband/acc: cosmetic changes

[Hernan Vargas]

Cosmetic code changes.
No functional impact.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c |  2 +-
 drivers/baseband/acc/rte_vrb_pmd.c    | 54 +++++++++++++++++----------
 2 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index c690d1492ba3..c82a0b6cc174 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -4200,7 +4200,7 @@ poweron_cleanup(struct rte_bbdev *bbdev, struct acc_device *d,
 		acc_reg_write(d, HWPfQmgrIngressAq + 0x100, enq_req.val);
 		usleep(ACC_LONG_WAIT * 100);
 		if (desc->req.word0 != 2)
-			rte_bbdev_log(WARNING, "DMA Response %#"PRIx32, desc->req.word0);
+			rte_bbdev_log(WARNING, "DMA Response %#"PRIx32"", desc->req.word0);
 	}
 
 	/* Reset LDPC Cores */
diff --git a/drivers/baseband/acc/rte_vrb_pmd.c b/drivers/baseband/acc/rte_vrb_pmd.c
index 42414823541e..c0464d20c641 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -957,6 +957,9 @@ vrb_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
 	struct acc_queue *q;
 	int32_t q_idx;
 	int ret;
+	union acc_dma_desc *desc = NULL;
+	unsigned int desc_idx, b_idx;
+	int fcw_len;
 
 	if (d == NULL) {
 		rte_bbdev_log(ERR, "Undefined device");
@@ -983,16 +986,33 @@ vrb_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
 	}
 
 	/* Prepare the Ring with default descriptor format. */
-	union acc_dma_desc *desc = NULL;
-	unsigned int desc_idx, b_idx;
-	int fcw_len = (conf->op_type == RTE_BBDEV_OP_LDPC_ENC ?
-			ACC_FCW_LE_BLEN : (conf->op_type == RTE_BBDEV_OP_TURBO_DEC ?
-			ACC_FCW_TD_BLEN : (conf->op_type == RTE_BBDEV_OP_LDPC_DEC ?
-			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;
+	switch (conf->op_type) {
+	case RTE_BBDEV_OP_LDPC_ENC:
+		fcw_len = ACC_FCW_LE_BLEN;
+		break;
+	case RTE_BBDEV_OP_LDPC_DEC:
+		fcw_len = ACC_FCW_LD_BLEN;
+		break;
+	case RTE_BBDEV_OP_TURBO_DEC:
+		fcw_len = ACC_FCW_TD_BLEN;
+		break;
+	case RTE_BBDEV_OP_TURBO_ENC:
+		fcw_len = ACC_FCW_TE_BLEN;
+		break;
+	case RTE_BBDEV_OP_FFT:
+		fcw_len = ACC_FCW_FFT_BLEN;
+		if (q->d->device_variant == VRB2_VARIANT)
+			fcw_len = ACC_FCW_FFT_BLEN_3;
+		break;
+	case RTE_BBDEV_OP_MLDTS:
+		fcw_len = ACC_FCW_MLDTS_BLEN;
+		break;
+	default:
+		/* NOT REACHED. */
+		fcw_len = 0;
+		rte_bbdev_log(ERR, "Unexpected error in %s using type %d", __func__, conf->op_type);
+		break;
+	}
 
 	for (desc_idx = 0; desc_idx < d->sw_ring_max_depth; desc_idx++) {
 		desc = q->ring_addr + desc_idx;
@@ -1758,8 +1778,7 @@ vrb_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
 	if (fcw->hcout_en > 0) {
 		parity_offset = (op->ldpc_dec.basegraph == 1 ? 20 : 8)
 				* op->ldpc_dec.z_c - op->ldpc_dec.n_filler;
-		k0_p = (fcw->k0 > parity_offset) ?
-				fcw->k0 - op->ldpc_dec.n_filler : fcw->k0;
+		k0_p = (fcw->k0 > parity_offset) ? fcw->k0 - op->ldpc_dec.n_filler : fcw->k0;
 		ncb_p = fcw->ncb - op->ldpc_dec.n_filler;
 		l = k0_p + fcw->rm_e;
 		harq_out_length = (uint16_t) fcw->hcin_size0;
@@ -2001,16 +2020,15 @@ vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
 		next_triplet++;
 	}
 
-	if (check_bit(op->ldpc_dec.op_flags,
-				RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE)) {
+	if (check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE)) {
 		if (op->ldpc_dec.harq_combined_output.data == 0) {
 			rte_bbdev_log(ERR, "HARQ output is not defined");
 			return -1;
 		}
 
-		/* Pruned size of the HARQ */
+		/* Pruned size of the HARQ. */
 		h_p_size = fcw->hcout_size0 + fcw->hcout_size1;
-		/* Non-Pruned size of the HARQ */
+		/* Non-Pruned size of the HARQ. */
 		h_np_size = fcw->hcout_offset > 0 ?
 				fcw->hcout_offset + fcw->hcout_size1 :
 				h_p_size;
@@ -2584,7 +2602,6 @@ vrb_enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 			seg_total_left = rte_pktmbuf_data_len(input) - in_offset;
 		else
 			seg_total_left = fcw->rm_e;
-
 		ret = vrb_dma_desc_ld_fill(op, &desc->req, &input, h_output,
 				&in_offset, &h_out_offset,
 				&h_out_length, &mbuf_total_left,
@@ -2646,7 +2663,6 @@ vrb_enqueue_ldpc_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 	desc_first = desc;
 	fcw_offset = (desc_idx << 8) + ACC_DESC_FCW_OFFSET;
 	harq_layout = q->d->harq_layout;
-
 	vrb_fcw_ld_fill(op, &desc->req.fcw_ld, harq_layout, q->d->device_variant);
 
 	input = op->ldpc_dec.input.data;
@@ -3274,7 +3290,7 @@ vrb2_dequeue_ldpc_enc_one_op_tb(struct acc_queue *q, struct rte_bbdev_enc_op **r
 	return 1;
 }
 
-/* Dequeue one LDPC encode operations from device in TB mode.
+/* Dequeue one encode operations from device in TB mode.
  * That operation may cover multiple descriptors.
  */
 static inline int
-- 
2.37.1

Re: [PATCH v3 10/12] baseband/acc: cosmetic changes

[Maxime Coquelin]

On 10/9/24 23:13, Hernan Vargas wrote:
> Cosmetic code changes.
> No functional impact.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   drivers/baseband/acc/rte_acc100_pmd.c |  2 +-
>   drivers/baseband/acc/rte_vrb_pmd.c    | 54 +++++++++++++++++----------
>   2 files changed, 36 insertions(+), 20 deletions(-)
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

[PATCH v3 11/12] baseband/acc: rte free refactor

[Hernan Vargas]

Refactor to explicitly set pointer to NULL after free to avoid double
free.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 23 +++++++------
 drivers/baseband/acc/rte_vrb_pmd.c    | 48 +++++++++++++++------------
 2 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index c82a0b6cc174..d33e42c8070b 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -564,6 +564,7 @@ acc100_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
 	d->tail_ptrs = NULL;
 free_sw_rings:
 	rte_free(d->sw_rings_base);
+	d->sw_rings_base = NULL;
 	d->sw_rings = NULL;
 
 	return ret;
@@ -593,6 +594,7 @@ acc100_intr_enable(struct rte_bbdev *dev)
 					"Couldn't enable interrupts for device: %s",
 					dev->data->name);
 			rte_free(d->info_ring);
+			d->info_ring = NULL;
 			return ret;
 		}
 		ret = rte_intr_callback_register(dev->intr_handle,
@@ -602,6 +604,7 @@ acc100_intr_enable(struct rte_bbdev *dev)
 					"Couldn't register interrupt callback for device: %s",
 					dev->data->name);
 			rte_free(d->info_ring);
+			d->info_ring = NULL;
 			return ret;
 		}
 
@@ -619,16 +622,15 @@ acc100_dev_close(struct rte_bbdev *dev)
 {
 	struct acc_device *d = dev->data->dev_private;
 	acc100_check_ir(d);
-	if (d->sw_rings_base != NULL) {
-		rte_free(d->tail_ptrs);
-		rte_free(d->info_ring);
-		rte_free(d->sw_rings_base);
-		rte_free(d->harq_layout);
-		d->sw_rings_base = NULL;
-		d->tail_ptrs = NULL;
-		d->info_ring = NULL;
-		d->harq_layout = NULL;
-	}
+	rte_free(d->tail_ptrs);
+	rte_free(d->info_ring);
+	rte_free(d->sw_rings_base);
+	rte_free(d->harq_layout);
+	d->tail_ptrs = NULL;
+	d->info_ring = NULL;
+	d->sw_rings_base = NULL;
+	d->sw_rings = NULL;
+	d->harq_layout = NULL;
 	/* Ensure all in flight HW transactions are completed */
 	usleep(ACC_LONG_WAIT);
 	return 0;
@@ -4235,6 +4237,7 @@ poweron_cleanup(struct rte_bbdev *bbdev, struct acc_device *d,
 	rte_bbdev_log(INFO, "Number of 5GUL engines %d", numEngines);
 
 	rte_free(d->sw_rings_base);
+	d->sw_rings_base = NULL;
 	usleep(ACC_LONG_WAIT);
 }
 
diff --git a/drivers/baseband/acc/rte_vrb_pmd.c b/drivers/baseband/acc/rte_vrb_pmd.c
index c0464d20c641..03df270af1cf 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -745,10 +745,13 @@ vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
 free_sw_rings:
 	if (d->device_variant == VRB1_VARIANT) {
 		rte_free(d->sw_rings_base);
+		d->sw_rings_base = NULL;
 		d->sw_rings = NULL;
 	} else if (d->device_variant == VRB2_VARIANT) {
-		for (i = 0; i <= RTE_BBDEV_OP_MLDTS; i++)
+		for (i = 0; i <= RTE_BBDEV_OP_MLDTS; i++) {
 			rte_free(d->sw_rings_array[i]);
+			d->sw_rings_array[i] = 0;
+		}
 	}
 
 	return ret;
@@ -786,6 +789,7 @@ vrb_intr_enable(struct rte_bbdev *dev)
 					"Couldn't enable interrupts for device: %s",
 					dev->data->name);
 			rte_free(d->info_ring);
+			d->info_ring = NULL;
 			return ret;
 		}
 		ret = rte_intr_callback_register(dev->intr_handle,
@@ -795,6 +799,7 @@ vrb_intr_enable(struct rte_bbdev *dev)
 					"Couldn't register interrupt callback for device: %s",
 					dev->data->name);
 			rte_free(d->info_ring);
+			d->info_ring = NULL;
 			return ret;
 		}
 
@@ -849,6 +854,7 @@ vrb_intr_enable(struct rte_bbdev *dev)
 					"Couldn't enable interrupts for device: %s",
 					dev->data->name);
 			rte_free(d->info_ring);
+			d->info_ring = NULL;
 			return ret;
 		}
 		ret = rte_intr_callback_register(dev->intr_handle,
@@ -858,6 +864,7 @@ vrb_intr_enable(struct rte_bbdev *dev)
 					"Couldn't register interrupt callback for device: %s",
 					dev->data->name);
 			rte_free(d->info_ring);
+			d->info_ring = NULL;
 			return ret;
 		}
 
@@ -878,28 +885,25 @@ vrb_dev_close(struct rte_bbdev *dev)
 
 	vrb_check_ir(d);
 	if (d->device_variant == VRB1_VARIANT) {
-		if (d->sw_rings_base != NULL) {
-			rte_free(d->tail_ptrs);
-			rte_free(d->info_ring);
-			rte_free(d->sw_rings_base);
-			rte_free(d->harq_layout);
-			d->tail_ptrs = NULL;
-			d->info_ring = NULL;
-			d->sw_rings_base = NULL;
-			d->harq_layout = NULL;
-		}
+		rte_free(d->tail_ptrs);
+		rte_free(d->info_ring);
+		rte_free(d->sw_rings_base);
+		rte_free(d->harq_layout);
+		d->tail_ptrs = NULL;
+		d->info_ring = NULL;
+		d->sw_rings_base = NULL;
+		d->sw_rings = NULL;
+		d->harq_layout = NULL;
 	} else if (d->device_variant == VRB2_VARIANT) {
-		if (d->sw_rings_array[1] != NULL) {
-			rte_free(d->tail_ptrs);
-			rte_free(d->info_ring);
-			rte_free(d->harq_layout);
-			d->tail_ptrs = NULL;
-			d->info_ring = NULL;
-			d->harq_layout = NULL;
-			for (i = 0; i <= RTE_BBDEV_OP_MLDTS; i++) {
-				rte_free(d->sw_rings_array[i]);
-				d->sw_rings_array[i] = NULL;
-			}
+		rte_free(d->tail_ptrs);
+		rte_free(d->info_ring);
+		rte_free(d->harq_layout);
+		d->tail_ptrs = NULL;
+		d->info_ring = NULL;
+		d->harq_layout = NULL;
+		for (i = 0; i <= RTE_BBDEV_OP_MLDTS; i++) {
+			rte_free(d->sw_rings_array[i]);
+			d->sw_rings_array[i] = NULL;
 		}
 	}
 	/* Ensure all in flight HW transactions are completed. */
-- 
2.37.1

Re: [PATCH v3 11/12] baseband/acc: rte free refactor

[Maxime Coquelin]

I would rename the title to:

"baseband/acc: refactor resources freeing"

I can fix while applying.

On 10/9/24 23:13, Hernan Vargas wrote:
> Refactor to explicitly set pointer to NULL after free to avoid double
> free.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   drivers/baseband/acc/rte_acc100_pmd.c | 23 +++++++------
>   drivers/baseband/acc/rte_vrb_pmd.c    | 48 +++++++++++++++------------
>   2 files changed, 39 insertions(+), 32 deletions(-)
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

[PATCH v3 12/12] baseband/acc: clean up of VRB1 capabilities

[Hernan Vargas]

The interrupt support was defeatured on the VRB1 device.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 doc/guides/bbdevs/vrb1.rst         | 3 ---
 drivers/baseband/acc/rte_vrb_pmd.c | 8 ++------
 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/doc/guides/bbdevs/vrb1.rst b/doc/guides/bbdevs/vrb1.rst
index 36c1ac04b6d3..e450e0061fcb 100644
--- a/doc/guides/bbdevs/vrb1.rst
+++ b/doc/guides/bbdevs/vrb1.rst
@@ -33,7 +33,6 @@ These hardware blocks provide the following features exposed by the PMD:
 - FFT processing
 - Single Root I/O Virtualization (SR-IOV) with 16 Virtual Functions (VFs) per Physical Function (PF)
 - Maximum of 256 queues per VF
-- Message Signaled Interrupts (MSIs)
 
 The Intel vRAN Boost v1.0 PMD supports the following bbdev capabilities:
 
@@ -57,14 +56,12 @@ The Intel vRAN Boost v1.0 PMD supports the following bbdev capabilities:
 * 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.
diff --git a/drivers/baseband/acc/rte_vrb_pmd.c b/drivers/baseband/acc/rte_vrb_pmd.c
index 03df270af1cf..0455320c2a50 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -1222,7 +1222,6 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct rte_bbdev_driver_info *dev_info)
 					RTE_BBDEV_TURBO_HALF_ITERATION_EVEN |
 					RTE_BBDEV_TURBO_CONTINUE_CRC_MATCH |
 					RTE_BBDEV_TURBO_EARLY_TERMINATION |
-					RTE_BBDEV_TURBO_DEC_INTERRUPTS |
 					RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN |
 					RTE_BBDEV_TURBO_MAP_DEC |
 					RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP |
@@ -1243,7 +1242,6 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct rte_bbdev_driver_info *dev_info)
 					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,
@@ -1257,8 +1255,7 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct rte_bbdev_driver_info *dev_info)
 				.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_INTERLEAVER_BYPASS,
 				.num_buffers_src =
 						RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
 				.num_buffers_dst =
@@ -1279,8 +1276,7 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct rte_bbdev_driver_info *dev_info)
 				RTE_BBDEV_LDPC_DEINTERLEAVER_BYPASS |
 				RTE_BBDEV_LDPC_DEC_SCATTER_GATHER |
 				RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION |
-				RTE_BBDEV_LDPC_LLR_COMPRESSION |
-				RTE_BBDEV_LDPC_DEC_INTERRUPTS,
+				RTE_BBDEV_LDPC_LLR_COMPRESSION,
 			.llr_size = 8,
 			.llr_decimals = 1,
 			.num_buffers_src =
-- 
2.37.1

Re: [PATCH v3 12/12] baseband/acc: clean up of VRB1 capabilities

[Maxime Coquelin]

On 10/9/24 23:13, Hernan Vargas wrote:
> The interrupt support was defeatured on the VRB1 device.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   doc/guides/bbdevs/vrb1.rst         | 3 ---
>   drivers/baseband/acc/rte_vrb_pmd.c | 8 ++------
>   2 files changed, 2 insertions(+), 9 deletions(-)
> 


Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Maxime

Re: [PATCH v3 00/12] acc baseband PMD fix and updates for 24.11

[Maxime Coquelin]

On 10/9/24 23:12, Hernan Vargas wrote:
> This series targets 24.11.
> v3: Updated code with community recommendations. Added 2 commits for rte_free refactor and clean up of VRB1 capabilities.
> v2: Rebased to the latest next-baseband-for-main which includes needed rte_bbdev lib updates.
> v1: It includes a memory access fix, refactoring of queue allocation and general improvements.
> 
> Hernan Vargas (12):
>    baseband/acc: fix access to deallocated mem
>    baseband/acc: fix soft output bypass RM
>    baseband/acc: queue allocation refactor
>    baseband/acc: configure max queues per device
>    baseband/acc: future proof structure comparison
>    baseband/acc: enhance SW ring alignment
>    baseband/acc: algorithm tuning for LDPC decoder
>    baseband/acc: remove check on HARQ memory
>    baseband/acc: reset ring data valid bit
>    baseband/acc: cosmetic changes
>    baseband/acc: rte free refactor
>    baseband/acc: clean up of VRB1 capabilities
> 
>   doc/guides/bbdevs/vrb1.rst            |   3 -
>   drivers/baseband/acc/acc_common.h     |  16 +-
>   drivers/baseband/acc/rte_acc100_pmd.c |  61 ++---
>   drivers/baseband/acc/rte_vrb_pmd.c    | 335 ++++++++++++++++----------
>   4 files changed, 227 insertions(+), 188 deletions(-)
> 

Series applied to next-baseband/for-main.

Thanks,
Maxime