[PATCH v2 0/5] ACC100 improvements and clean up

[PATCH v2 0/5] ACC100 improvements and clean up

[Hernan Vargas]

v2: Label memory barrier change as a fix.
v1: This series targeting 24.07 removes unused/dead code, memory barrier improvements, cosmetic log changes.

Hernan Vargas (5):
  baseband/acc: fix memory barrier
  baseband/acc: remove ACC100 unused code
  baseband/acc: remove ACC100 HARQ pruning
  baseband/acc: improve error description
  baseband/acc: cosmetic log changes

 drivers/baseband/acc/acc_common.h     |   5 +-
 drivers/baseband/acc/rte_acc100_pmd.c | 267 ++++++++------------------
 2 files changed, 79 insertions(+), 193 deletions(-)

-- 
2.37.1

[PATCH v2 1/5] baseband/acc: fix memory barrier

[Hernan Vargas]

Moving memory barrier so that dequeue thread can be in sync with enqueue
thread.

Fixes: 32e8b7ea35dd ("baseband/acc100: refactor to segregate common code")
Cc: stable@dpdk.org

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

diff --git a/drivers/baseband/acc/acc_common.h b/drivers/baseband/acc/acc_common.h
index fddeb0737b8b..e249f37e38fe 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -1112,6 +1112,9 @@ acc_dma_enqueue(struct acc_queue *q, uint16_t n,
 				req_elem_addr,
 				(void *)q->mmio_reg_enqueue);
 
+		q->aq_enqueued++;
+		q->sw_ring_head += enq_batch_size;
+
 		rte_wmb();
 
 		/* Start time measurement for enqueue function offload. */
@@ -1122,8 +1125,6 @@ acc_dma_enqueue(struct acc_queue *q, uint16_t n,
 
 		queue_stats->acc_offload_cycles += rte_rdtsc_precise() - start_time;
 
-		q->aq_enqueued++;
-		q->sw_ring_head += enq_batch_size;
 		n -= enq_batch_size;
 
 	} while (n);
-- 
2.37.1

[PATCH v2 2/5] baseband/acc: remove ACC100 unused code

[Hernan Vargas]

Remove dead code and unused function in ACC100 driver.

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

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index 4f666e514b72..d6b0b9400c82 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -1640,59 +1640,6 @@ acc100_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
 	return 0;
 }
 
-static inline void
-acc100_dma_desc_ld_update(struct rte_bbdev_dec_op *op,
-		struct acc_dma_req_desc *desc,
-		struct rte_mbuf *input, struct rte_mbuf *h_output,
-		uint32_t *in_offset, uint32_t *h_out_offset,
-		uint32_t *h_out_length,
-		union acc_harq_layout_data *harq_layout)
-{
-	int next_triplet = 1; /* FCW already done */
-	desc->data_ptrs[next_triplet].address =
-			rte_pktmbuf_iova_offset(input, *in_offset);
-	next_triplet++;
-
-	if (check_bit(op->ldpc_dec.op_flags,
-				RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE)) {
-		struct rte_bbdev_op_data hi = op->ldpc_dec.harq_combined_input;
-		desc->data_ptrs[next_triplet].address = hi.offset;
-#ifndef ACC100_EXT_MEM
-		desc->data_ptrs[next_triplet].address =
-				rte_pktmbuf_iova_offset(hi.data, hi.offset);
-#endif
-		next_triplet++;
-	}
-
-	desc->data_ptrs[next_triplet].address =
-			rte_pktmbuf_iova_offset(h_output, *h_out_offset);
-	*h_out_length = desc->data_ptrs[next_triplet].blen;
-	next_triplet++;
-
-	if (check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE)) {
-		struct rte_bbdev_dec_op *prev_op;
-		uint32_t harq_idx, prev_harq_idx;
-		desc->data_ptrs[next_triplet].address = op->ldpc_dec.harq_combined_output.offset;
-		/* Adjust based on previous operation */
-		prev_op = desc->op_addr;
-		op->ldpc_dec.harq_combined_output.length =
-				prev_op->ldpc_dec.harq_combined_output.length;
-		harq_idx = hq_index(op->ldpc_dec.harq_combined_output.offset);
-		prev_harq_idx = hq_index(prev_op->ldpc_dec.harq_combined_output.offset);
-		harq_layout[harq_idx].val = harq_layout[prev_harq_idx].val;
-#ifndef ACC100_EXT_MEM
-		struct rte_bbdev_op_data ho =
-				op->ldpc_dec.harq_combined_output;
-		desc->data_ptrs[next_triplet].address =
-				rte_pktmbuf_iova_offset(ho.data, ho.offset);
-#endif
-		next_triplet++;
-	}
-
-	op->ldpc_dec.hard_output.length += *h_out_length;
-	desc->op_addr = op;
-}
-
 #ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE
 /* Validates turbo encoder parameters */
 static inline int
@@ -2935,8 +2882,7 @@ derm_workaround_recommended(struct rte_bbdev_op_ldpc_dec *ldpc_dec, struct acc_q
 /** Enqueue one decode operations for ACC100 device in CB mode */
 static inline int
 enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
-		uint16_t total_enqueued_cbs, bool same_op,
-		struct rte_bbdev_queue_data *q_data)
+		uint16_t total_enqueued_cbs, struct rte_bbdev_queue_data *q_data)
 {
 	int ret;
 	if (unlikely(check_bit(op->ldpc_dec.op_flags,
@@ -2969,93 +2915,73 @@ enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 #endif
 	union acc_harq_layout_data *harq_layout = q->d->harq_layout;
 
-	if (same_op) {
-		union acc_dma_desc *prev_desc;
-		prev_desc = acc_desc(q, total_enqueued_cbs - 1);
-		uint8_t *prev_ptr = (uint8_t *) prev_desc;
-		uint8_t *new_ptr = (uint8_t *) desc;
-		/* Copy first 4 words and BDESCs */
-		rte_memcpy(new_ptr, prev_ptr, ACC_5GUL_SIZE_0);
-		rte_memcpy(new_ptr + ACC_5GUL_OFFSET_0,
-				prev_ptr + ACC_5GUL_OFFSET_0,
-				ACC_5GUL_SIZE_1);
-		desc->req.op_addr = prev_desc->req.op_addr;
-		/* Copy FCW */
-		rte_memcpy(new_ptr + ACC_DESC_FCW_OFFSET,
-				prev_ptr + ACC_DESC_FCW_OFFSET,
-				ACC_FCW_LD_BLEN);
-		acc100_dma_desc_ld_update(op, &desc->req, input, h_output,
-				&in_offset, &h_out_offset,
-				&h_out_length, harq_layout);
-	} else {
-		struct acc_fcw_ld *fcw;
-		uint32_t seg_total_left;
-
-		if (derm_workaround_recommended(&op->ldpc_dec, q)) {
-			#ifdef RTE_BBDEV_SDK_AVX512
-			struct rte_bbdev_op_ldpc_dec *dec = &op->ldpc_dec;
-			struct bblib_rate_dematching_5gnr_request derm_req;
-			struct bblib_rate_dematching_5gnr_response derm_resp;
-			uint8_t *in;
-
-			/* Checking input size is matching with E */
-			if (dec->input.data->data_len < (dec->cb_params.e % 65536)) {
-				rte_bbdev_log(ERR, "deRM: Input size mismatch");
-				return -EFAULT;
-			}
-			/* Run first deRM processing in SW */
-			in = rte_pktmbuf_mtod_offset(dec->input.data, uint8_t *, in_offset);
-			derm_req.p_in = (int8_t *) in;
-			derm_req.p_harq = (int8_t *) q->derm_buffer;
-			derm_req.base_graph = dec->basegraph;
-			derm_req.zc = dec->z_c;
-			derm_req.ncb = dec->n_cb;
-			derm_req.e = dec->cb_params.e;
-			if (derm_req.e > ACC_MAX_E) {
-				rte_bbdev_log(WARNING,
-						"deRM: E %d > %d max",
-						derm_req.e, ACC_MAX_E);
-				derm_req.e = ACC_MAX_E;
-			}
-			derm_req.k0 = 0; /* Actual output from SDK */
-			derm_req.isretx = false;
-			derm_req.rvid = dec->rv_index;
-			derm_req.modulation_order = dec->q_m;
-			derm_req.start_null_index =
-					(dec->basegraph == 1 ? 22 : 10)
-					* dec->z_c - 2 * dec->z_c
-					- dec->n_filler;
-			derm_req.num_of_null = dec->n_filler;
-			bblib_rate_dematching_5gnr(&derm_req, &derm_resp);
-			/* Force back the HW DeRM */
-			dec->q_m = 1;
-			dec->cb_params.e = dec->n_cb - dec->n_filler;
-			dec->rv_index = 0;
-			rte_memcpy(in, q->derm_buffer, dec->cb_params.e);
-			/* Capture counter when pre-processing is used */
-			q_data->queue_stats.enqueue_warn_count++;
-			#else
-			RTE_SET_USED(q_data);
-			rte_bbdev_log(INFO, "Corner case may require deRM pre-processing in SDK");
-			#endif
+	struct acc_fcw_ld *fcw;
+	uint32_t seg_total_left;
+
+	if (derm_workaround_recommended(&op->ldpc_dec, q)) {
+		#ifdef RTE_BBDEV_SDK_AVX512
+		struct rte_bbdev_op_ldpc_dec *dec = &op->ldpc_dec;
+		struct bblib_rate_dematching_5gnr_request derm_req;
+		struct bblib_rate_dematching_5gnr_response derm_resp;
+		uint8_t *in;
+
+		/* Checking input size is matching with E */
+		if (dec->input.data->data_len < (dec->cb_params.e % 65536)) {
+			rte_bbdev_log(ERR, "deRM: Input size mismatch");
+			return -EFAULT;
 		}
+		/* Run first deRM processing in SW */
+		in = rte_pktmbuf_mtod_offset(dec->input.data, uint8_t *, in_offset);
+		derm_req.p_in = (int8_t *) in;
+		derm_req.p_harq = (int8_t *) q->derm_buffer;
+		derm_req.base_graph = dec->basegraph;
+		derm_req.zc = dec->z_c;
+		derm_req.ncb = dec->n_cb;
+		derm_req.e = dec->cb_params.e;
+		if (derm_req.e > ACC_MAX_E) {
+			rte_bbdev_log(WARNING,
+					"deRM: E %d > %d max",
+					derm_req.e, ACC_MAX_E);
+			derm_req.e = ACC_MAX_E;
+		}
+		derm_req.k0 = 0; /* Actual output from SDK */
+		derm_req.isretx = false;
+		derm_req.rvid = dec->rv_index;
+		derm_req.modulation_order = dec->q_m;
+		derm_req.start_null_index =
+				(dec->basegraph == 1 ? 22 : 10)
+				* dec->z_c - 2 * dec->z_c
+				- dec->n_filler;
+		derm_req.num_of_null = dec->n_filler;
+		bblib_rate_dematching_5gnr(&derm_req, &derm_resp);
+		/* Force back the HW DeRM */
+		dec->q_m = 1;
+		dec->cb_params.e = dec->n_cb - dec->n_filler;
+		dec->rv_index = 0;
+		rte_memcpy(in, q->derm_buffer, dec->cb_params.e);
+		/* Capture counter when pre-processing is used */
+		q_data->queue_stats.enqueue_warn_count++;
+		#else
+		RTE_SET_USED(q_data);
+		rte_bbdev_log(INFO, "Corner case may require deRM pre-processing in SDK");
+		#endif
+	}
 
-		fcw = &desc->req.fcw_ld;
-		q->d->fcw_ld_fill(op, fcw, harq_layout);
+	fcw = &desc->req.fcw_ld;
+	q->d->fcw_ld_fill(op, fcw, harq_layout);
 
-		/* Special handling when using mbuf or not */
-		if (check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_DEC_SCATTER_GATHER))
-			seg_total_left = rte_pktmbuf_data_len(input) - in_offset;
-		else
-			seg_total_left = fcw->rm_e;
+	/* Special handling when using mbuf or not */
+	if (check_bit(op->ldpc_dec.op_flags, RTE_BBDEV_LDPC_DEC_SCATTER_GATHER))
+		seg_total_left = rte_pktmbuf_data_len(input) - in_offset;
+	else
+		seg_total_left = fcw->rm_e;
 
-		ret = acc100_dma_desc_ld_fill(op, &desc->req, &input, h_output,
-				&in_offset, &h_out_offset,
-				&h_out_length, &mbuf_total_left,
-				&seg_total_left, fcw);
-		if (unlikely(ret < 0))
-			return ret;
-	}
+	ret = acc100_dma_desc_ld_fill(op, &desc->req, &input, h_output,
+			&in_offset, &h_out_offset,
+			&h_out_length, &mbuf_total_left,
+			&seg_total_left, fcw);
+	if (unlikely(ret < 0))
+		return ret;
 
 	/* Hard output */
 	mbuf_append(h_output_head, h_output, h_out_length);
@@ -3553,7 +3479,7 @@ acc100_enqueue_ldpc_dec_cb(struct rte_bbdev_queue_data *q_data,
 	int32_t avail = acc_ring_avail_enq(q);
 	uint16_t i;
 	int ret;
-	bool same_op = false;
+
 	for (i = 0; i < num; ++i) {
 		/* Check if there are available space for further processing */
 		if (unlikely(avail < 1)) {
@@ -3562,16 +3488,13 @@ acc100_enqueue_ldpc_dec_cb(struct rte_bbdev_queue_data *q_data,
 		}
 		avail -= 1;
 
-		if (i > 0)
-			same_op = cmp_ldpc_dec_op(&ops[i-1]);
-		rte_bbdev_log(INFO, "Op %d %d %d %d %d %d %d %d %d %d %d %d\n",
+		rte_bbdev_log(INFO, "Op %d %d %d %d %d %d %d %d %d %d %d\n",
 			i, ops[i]->ldpc_dec.op_flags, ops[i]->ldpc_dec.rv_index,
 			ops[i]->ldpc_dec.iter_max, ops[i]->ldpc_dec.iter_count,
 			ops[i]->ldpc_dec.basegraph, ops[i]->ldpc_dec.z_c,
 			ops[i]->ldpc_dec.n_cb, ops[i]->ldpc_dec.q_m,
-			ops[i]->ldpc_dec.n_filler, ops[i]->ldpc_dec.cb_params.e,
-			same_op);
-		ret = enqueue_ldpc_dec_one_op_cb(q, ops[i], i, same_op, q_data);
+			ops[i]->ldpc_dec.n_filler, ops[i]->ldpc_dec.cb_params.e);
+		ret = enqueue_ldpc_dec_one_op_cb(q, ops[i], i, q_data);
 		if (ret < 0) {
 			acc_enqueue_invalid(q_data);
 			break;
-- 
2.37.1

[PATCH v2 3/5] baseband/acc: remove ACC100 HARQ pruning

[Hernan Vargas]

HARQ pruning is not an ACC100 feature. Removing in effect dead code.

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

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index d6b0b9400c82..f37722879c20 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -1152,7 +1152,6 @@ acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
 	uint16_t harq_out_length, harq_in_length, ncb_p, k0_p, parity_offset;
 	uint32_t harq_index;
 	uint32_t l;
-	bool harq_prun = false;
 	uint32_t max_hc_in;
 
 	fcw->qm = op->ldpc_dec.q_m;
@@ -1199,13 +1198,6 @@ acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
 	fcw->llr_pack_mode = check_bit(op->ldpc_dec.op_flags,
 			RTE_BBDEV_LDPC_LLR_COMPRESSION);
 	harq_index = hq_index(op->ldpc_dec.harq_combined_output.offset);
-#ifdef ACC100_EXT_MEM
-	/* Limit cases when HARQ pruning is valid */
-	harq_prun = ((op->ldpc_dec.harq_combined_output.offset %
-			ACC_HARQ_OFFSET) == 0) &&
-			(op->ldpc_dec.harq_combined_output.offset <= UINT16_MAX
-			* ACC_HARQ_OFFSET);
-#endif
 	if (fcw->hcin_en > 0) {
 		harq_in_length = op->ldpc_dec.harq_combined_input.length;
 		if (fcw->hcin_decomp_mode > 0)
@@ -1221,16 +1213,9 @@ acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
 		if (fcw->hcin_decomp_mode > 0)
 			harq_in_length = RTE_ALIGN_FLOOR(harq_in_length, ACC100_HARQ_ALIGN_COMP);
 
-		if ((harq_layout[harq_index].offset > 0) && harq_prun) {
-			rte_bbdev_log_debug("HARQ IN offset unexpected for now\n");
-			fcw->hcin_size0 = harq_layout[harq_index].size0;
-			fcw->hcin_offset = harq_layout[harq_index].offset;
-			fcw->hcin_size1 = harq_in_length - harq_layout[harq_index].offset;
-		} else {
-			fcw->hcin_size0 = harq_in_length;
-			fcw->hcin_offset = 0;
-			fcw->hcin_size1 = 0;
-		}
+		fcw->hcin_size0 = harq_in_length;
+		fcw->hcin_offset = 0;
+		fcw->hcin_size1 = 0;
 	} else {
 		fcw->hcin_size0 = 0;
 		fcw->hcin_offset = 0;
@@ -1296,15 +1281,9 @@ acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
 		if (fcw->hcout_comp_mode > 0)
 			harq_out_length = RTE_ALIGN_FLOOR(harq_out_length, ACC100_HARQ_ALIGN_COMP);
 
-		if ((k0_p > fcw->hcin_size0 + ACC_HARQ_OFFSET_THRESHOLD) && harq_prun) {
-			fcw->hcout_size0 = (uint16_t) fcw->hcin_size0;
-			fcw->hcout_offset = k0_p & 0xFFC0;
-			fcw->hcout_size1 = harq_out_length - fcw->hcout_offset;
-		} else {
-			fcw->hcout_size0 = harq_out_length;
-			fcw->hcout_size1 = 0;
-			fcw->hcout_offset = 0;
-		}
+		fcw->hcout_size0 = harq_out_length;
+		fcw->hcout_size1 = 0;
+		fcw->hcout_offset = 0;
 
 		if (fcw->hcout_size0 == 0) {
 			rte_bbdev_log(ERR, " Invalid FCW : HCout %d",
-- 
2.37.1

[PATCH v2 4/5] baseband/acc: improve error description

[Hernan Vargas]

Remove dead code for error and update description of one error print.

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

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index f37722879c20..8ec521675f34 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -1152,7 +1152,6 @@ acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
 	uint16_t harq_out_length, harq_in_length, ncb_p, k0_p, parity_offset;
 	uint32_t harq_index;
 	uint32_t l;
-	uint32_t max_hc_in;
 
 	fcw->qm = op->ldpc_dec.q_m;
 	fcw->nfiller = op->ldpc_dec.n_filler;
@@ -1222,21 +1221,6 @@ acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
 		fcw->hcin_size1 = 0;
 	}
 
-	/* Enforce additional check on FCW validity */
-	max_hc_in = RTE_ALIGN_CEIL(fcw->ncb - fcw->nfiller, ACC_HARQ_ALIGN_64B);
-	if ((fcw->hcin_size0 > max_hc_in) ||
-			(fcw->hcin_size1 + fcw->hcin_offset > max_hc_in) ||
-			((fcw->hcin_size0 > fcw->hcin_offset) &&
-			(fcw->hcin_size1 != 0))) {
-		rte_bbdev_log(ERR, " Invalid FCW : HCIn %d %d %d, Ncb %d F %d",
-				fcw->hcin_size0, fcw->hcin_size1,
-				fcw->hcin_offset,
-				fcw->ncb, fcw->nfiller);
-		/* Disable HARQ input in that case to carry forward */
-		op->ldpc_dec.op_flags ^= RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE;
-		fcw->hcin_en = 0;
-	}
-
 	fcw->itmax = op->ldpc_dec.iter_max;
 	fcw->itstop = check_bit(op->ldpc_dec.op_flags,
 			RTE_BBDEV_LDPC_ITERATION_STOP_ENABLE);
@@ -1286,8 +1270,7 @@ acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
 		fcw->hcout_offset = 0;
 
 		if (fcw->hcout_size0 == 0) {
-			rte_bbdev_log(ERR, " Invalid FCW : HCout %d",
-				fcw->hcout_size0);
+			rte_bbdev_log(ERR, " Disabling HARQ output as size is zero");
 			op->ldpc_dec.op_flags ^= RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE;
 			fcw->hcout_en = 0;
 		}
-- 
2.37.1

[PATCH v2 5/5] baseband/acc: cosmetic log changes

[Hernan Vargas]

Minor cosmetic log change.
No functional impact.

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

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index 8ec521675f34..c0305e1e645a 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -4253,7 +4253,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"\n", desc->req.word0);
+			rte_bbdev_log(WARNING, "DMA Response %#"PRIx32"", desc->req.word0);
 	}
 
 	/* Reset LDPC Cores */
-- 
2.37.1

Re: [PATCH v2 1/5] baseband/acc: fix memory barrier

[Maxime Coquelin]

On 5/2/24 22:45, Hernan Vargas wrote:
> Moving memory barrier so that dequeue thread can be in sync with enqueue
> thread.
> 
> Fixes: 32e8b7ea35dd ("baseband/acc100: refactor to segregate common code")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   drivers/baseband/acc/acc_common.h | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/baseband/acc/acc_common.h b/drivers/baseband/acc/acc_common.h
> index fddeb0737b8b..e249f37e38fe 100644
> --- a/drivers/baseband/acc/acc_common.h
> +++ b/drivers/baseband/acc/acc_common.h
> @@ -1112,6 +1112,9 @@ acc_dma_enqueue(struct acc_queue *q, uint16_t n,
>   				req_elem_addr,
>   				(void *)q->mmio_reg_enqueue);
>   
> +		q->aq_enqueued++;
> +		q->sw_ring_head += enq_batch_size;
> +
>   		rte_wmb();
>   
>   		/* Start time measurement for enqueue function offload. */
> @@ -1122,8 +1125,6 @@ acc_dma_enqueue(struct acc_queue *q, uint16_t n,
>   
>   		queue_stats->acc_offload_cycles += rte_rdtsc_precise() - start_time;
>   
> -		q->aq_enqueued++;
> -		q->sw_ring_head += enq_batch_size;
>   		n -= enq_batch_size;
>   
>   	} while (n);

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

Thanks,
Maxime

Re: [PATCH v2 0/5] ACC100 improvements and clean up

[Maxime Coquelin]

On 5/2/24 22:45, Hernan Vargas wrote:
> v2: Label memory barrier change as a fix.
> v1: This series targeting 24.07 removes unused/dead code, memory barrier improvements, cosmetic log changes.
> 
> Hernan Vargas (5):
>    baseband/acc: fix memory barrier
>    baseband/acc: remove ACC100 unused code
>    baseband/acc: remove ACC100 HARQ pruning
>    baseband/acc: improve error description
>    baseband/acc: cosmetic log changes
> 
>   drivers/baseband/acc/acc_common.h     |   5 +-
>   drivers/baseband/acc/rte_acc100_pmd.c | 267 ++++++++------------------
>   2 files changed, 79 insertions(+), 193 deletions(-)
> 

Applied to next-baseband/for-main

Thanks,
Maxime

Re: [PATCH v2 5/5] baseband/acc: cosmetic log changes

[Thomas Monjalon]

02/05/2024 22:45, Hernan Vargas:
> -			rte_bbdev_log(WARNING, "DMA Response %#"PRIx32"\n", desc->req.word0);
> +			rte_bbdev_log(WARNING, "DMA Response %#"PRIx32"", desc->req.word0);

Why keeping an empty string after PRIx32? Should I remove it when pulling?
The title should be about removing superfluous empty line?

Re: [PATCH v2 5/5] baseband/acc: cosmetic log changes

[Thomas Monjalon]

14/06/2024 23:04, Thomas Monjalon:
> 02/05/2024 22:45, Hernan Vargas:
> > -			rte_bbdev_log(WARNING, "DMA Response %#"PRIx32"\n", desc->req.word0);
> > +			rte_bbdev_log(WARNING, "DMA Response %#"PRIx32"", desc->req.word0);
> 
> Why keeping an empty string after PRIx32? Should I remove it when pulling?
> The title should be about removing superfluous empty line?

I'm quite sure answers are "yes", so I move forward.

Also please rename log functions in all bbdev PMDs as they are defined inside the drivers.
For instance you should rename rte_bbdev_log to acc_log in this driver.
Same for rte_bbdev_log_debug.