[PATCH v1 0/6] baseband/acc: fixes on top of RC2

[PATCH v1 0/6] baseband/acc: fixes on top of RC2

[Nicolas Chautru]

A few fixes still required for the baseband acc PMDs
for ACC100 and ACC200 on top of RC2. Most of them
introduced recently including one covery warning.
All changes pushed in same serie for ease of review/apply.

Hernan Vargas (3):
  baseband/acc: fix PMON register values
  baseband/acc: fix double MSI intr in TB mode
  baseband/acc: fix redundant function definition

Nicolas Chautru (3):
  baseband/acc: fix LTE half iteration flag
  baseband/acc: fix to possible overflow
  baseband/acc: fix to acc200 access corner case

 drivers/baseband/acc/acc100_pmd.h     |   6 +-
 drivers/baseband/acc/acc_common.h     |   7 +-
 drivers/baseband/acc/rte_acc100_pmd.c | 130 ++------------------------
 drivers/baseband/acc/rte_acc200_pmd.c |  66 ++++++-------
 4 files changed, 48 insertions(+), 161 deletions(-)

-- 
2.37.1

[PATCH v1 1/6] baseband/acc: fix LTE half iteration flag

[Nicolas Chautru]

The logic for that flag was inverted.
Specific to ACC200.
When starting with even iteration it actually runs
for an additional half iteration.

Fixes: bec597b78a0 ("baseband/acc200: add LTE processing")

Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
---
 drivers/baseband/acc/rte_acc200_pmd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc200_pmd.c b/drivers/baseband/acc/rte_acc200_pmd.c
index 8ee9023451..0cbb9a946b 100644
--- a/drivers/baseband/acc/rte_acc200_pmd.c
+++ b/drivers/baseband/acc/rte_acc200_pmd.c
@@ -1238,8 +1238,7 @@ acc200_fcw_td_fill(const struct rte_bbdev_dec_op *op, struct acc_fcw_td *fcw)
 	fcw->raw_decoder_input_on = 0;
 	fcw->max_iter = RTE_MAX((uint8_t) op->turbo_dec.iter_max, 2);
 	fcw->min_iter = 2;
-	fcw->half_iter_on = !check_bit(op->turbo_dec.op_flags,
-			RTE_BBDEV_TURBO_HALF_ITERATION_EVEN);
+	fcw->half_iter_on = check_bit(op->turbo_dec.op_flags, RTE_BBDEV_TURBO_HALF_ITERATION_EVEN);
 
 	fcw->early_stop_en = check_bit(op->turbo_dec.op_flags,
 			RTE_BBDEV_TURBO_EARLY_TERMINATION) & !fcw->soft_output_en;
-- 
2.37.1

Re: [PATCH v1 1/6] baseband/acc: fix LTE half iteration flag

[Maxime Coquelin]

On 11/2/22 00:04, Nicolas Chautru wrote:
> The logic for that flag was inverted.
> Specific to ACC200.
> When starting with even iteration it actually runs
> for an additional half iteration.
> 
> Fixes: bec597b78a0 ("baseband/acc200: add LTE processing")
> 
> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> ---
>   drivers/baseband/acc/rte_acc200_pmd.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/baseband/acc/rte_acc200_pmd.c b/drivers/baseband/acc/rte_acc200_pmd.c
> index 8ee9023451..0cbb9a946b 100644
> --- a/drivers/baseband/acc/rte_acc200_pmd.c
> +++ b/drivers/baseband/acc/rte_acc200_pmd.c
> @@ -1238,8 +1238,7 @@ acc200_fcw_td_fill(const struct rte_bbdev_dec_op *op, struct acc_fcw_td *fcw)
>   	fcw->raw_decoder_input_on = 0;
>   	fcw->max_iter = RTE_MAX((uint8_t) op->turbo_dec.iter_max, 2);
>   	fcw->min_iter = 2;
> -	fcw->half_iter_on = !check_bit(op->turbo_dec.op_flags,
> -			RTE_BBDEV_TURBO_HALF_ITERATION_EVEN);
> +	fcw->half_iter_on = check_bit(op->turbo_dec.op_flags, RTE_BBDEV_TURBO_HALF_ITERATION_EVEN);
>   
>   	fcw->early_stop_en = check_bit(op->turbo_dec.op_flags,
>   			RTE_BBDEV_TURBO_EARLY_TERMINATION) & !fcw->soft_output_en;

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

Thanks,
Maxime

[PATCH v1 2/6] baseband/acc: fix to possible overflow

[Nicolas Chautru]

Potential overflow issue when casting to 64bits,
notably relevant when extending number of queues.

Coverity issue: 381665
Fixes: 32e8b7ea35 ("baseband/acc100: refactor to segregate common code")
Fixes: 40e3adbdd3 ("baseband/acc200: add queue configuration")

Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 3 +--
 drivers/baseband/acc/rte_acc200_pmd.c | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index 23bc5d25bb..b6e500c6bc 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -888,8 +888,7 @@ acc100_queue_release(struct rte_bbdev *dev, uint16_t q_id)
 
 	if (q != NULL) {
 		/* Mark the Queue as un-assigned */
-		d->q_assigned_bit_map[q->qgrp_id] &= (0xFFFFFFFFFFFFFFFF -
-				(uint64_t) (1 << q->aq_id));
+		d->q_assigned_bit_map[q->qgrp_id] &= (~0ULL - (1 << (uint64_t) q->aq_id));
 		rte_free(q->companion_ring_addr);
 		rte_free(q->lb_in);
 		rte_free(q->lb_out);
diff --git a/drivers/baseband/acc/rte_acc200_pmd.c b/drivers/baseband/acc/rte_acc200_pmd.c
index 0cbb9a946b..76a5986448 100644
--- a/drivers/baseband/acc/rte_acc200_pmd.c
+++ b/drivers/baseband/acc/rte_acc200_pmd.c
@@ -964,7 +964,7 @@ acc200_queue_release(struct rte_bbdev *dev, uint16_t q_id)
 
 	if (q != NULL) {
 		/* Mark the Queue as un-assigned. */
-		d->q_assigned_bit_map[q->qgrp_id] &= (~0ULL - (uint64_t) (1 << q->aq_id));
+		d->q_assigned_bit_map[q->qgrp_id] &= (~0ULL - (1 << (uint64_t) q->aq_id));
 		rte_free(q->companion_ring_addr);
 		rte_free(q->lb_in);
 		rte_free(q->lb_out);
-- 
2.37.1

Re: [PATCH v1 2/6] baseband/acc: fix to possible overflow

[Maxime Coquelin]

On 11/2/22 00:04, Nicolas Chautru wrote:
> Potential overflow issue when casting to 64bits,
> notably relevant when extending number of queues.
> 
> Coverity issue: 381665
> Fixes: 32e8b7ea35 ("baseband/acc100: refactor to segregate common code")
> Fixes: 40e3adbdd3 ("baseband/acc200: add queue configuration")
> 
> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> ---
>   drivers/baseband/acc/rte_acc100_pmd.c | 3 +--
>   drivers/baseband/acc/rte_acc200_pmd.c | 2 +-
>   2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
> index 23bc5d25bb..b6e500c6bc 100644
> --- a/drivers/baseband/acc/rte_acc100_pmd.c
> +++ b/drivers/baseband/acc/rte_acc100_pmd.c
> @@ -888,8 +888,7 @@ acc100_queue_release(struct rte_bbdev *dev, uint16_t q_id)
>   
>   	if (q != NULL) {
>   		/* Mark the Queue as un-assigned */
> -		d->q_assigned_bit_map[q->qgrp_id] &= (0xFFFFFFFFFFFFFFFF -
> -				(uint64_t) (1 << q->aq_id));
> +		d->q_assigned_bit_map[q->qgrp_id] &= (~0ULL - (1 << (uint64_t) q->aq_id));
>   		rte_free(q->companion_ring_addr);
>   		rte_free(q->lb_in);
>   		rte_free(q->lb_out);
> diff --git a/drivers/baseband/acc/rte_acc200_pmd.c b/drivers/baseband/acc/rte_acc200_pmd.c
> index 0cbb9a946b..76a5986448 100644
> --- a/drivers/baseband/acc/rte_acc200_pmd.c
> +++ b/drivers/baseband/acc/rte_acc200_pmd.c
> @@ -964,7 +964,7 @@ acc200_queue_release(struct rte_bbdev *dev, uint16_t q_id)
>   
>   	if (q != NULL) {
>   		/* Mark the Queue as un-assigned. */
> -		d->q_assigned_bit_map[q->qgrp_id] &= (~0ULL - (uint64_t) (1 << q->aq_id));
> +		d->q_assigned_bit_map[q->qgrp_id] &= (~0ULL - (1 << (uint64_t) q->aq_id));
>   		rte_free(q->companion_ring_addr);
>   		rte_free(q->lb_in);
>   		rte_free(q->lb_out);

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

Thanks,
Maxime

[PATCH v1 3/6] baseband/acc: fix to acc200 access corner case

[Nicolas Chautru]

To enforce safe access to the ACC200 device, the PMD requires
to explicitly check that the device is in configured and
enabled state prior to accessing queue resources.
This is done by checking the Qmgr ingress queue status.

Fixes: 40e3adbdd3 ("baseband/acc200: add queue configuration")

Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
---
 drivers/baseband/acc/rte_acc200_pmd.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/baseband/acc/rte_acc200_pmd.c b/drivers/baseband/acc/rte_acc200_pmd.c
index 76a5986448..10511d8b56 100644
--- a/drivers/baseband/acc/rte_acc200_pmd.c
+++ b/drivers/baseband/acc/rte_acc200_pmd.c
@@ -172,6 +172,21 @@ updateQtop(uint8_t acc, uint8_t qg, struct rte_acc_conf *acc_conf, struct acc_de
 	}
 }
 
+/* Check device Qmgr is enabled for protection */
+static inline bool
+acc200_check_device_enable(struct rte_bbdev *dev)
+{
+	uint32_t reg_aq, qg;
+	struct acc_device *d = dev->data->dev_private;
+
+	for (qg = 0; qg < ACC200_NUM_QGRPS; qg++) {
+		reg_aq = acc_reg_read(d, queue_offset(d->pf_device, 0, qg, 0));
+		if (reg_aq & ACC_QUEUE_ENABLE)
+			return true;
+	}
+	return false;
+}
+
 /* Fetch configuration enabled for the PF/VF using MMIO Read (slow). */
 static inline void
 fetch_acc200_config(struct rte_bbdev *dev)
@@ -190,6 +205,12 @@ fetch_acc200_config(struct rte_bbdev *dev)
 	if (d->configured)
 		return;
 
+	if (!acc200_check_device_enable(dev)) {
+		rte_bbdev_log(NOTICE, "%s has no queue enabled and can't be used.",
+				dev->data->name);
+		return;
+	}
+
 	/* Choose correct registry addresses for the device type. */
 	if (d->pf_device)
 		reg_addr = &pf_reg_addr;
@@ -454,6 +475,12 @@ acc200_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
 		return -ENODEV;
 	}
 
+	if (!acc200_check_device_enable(dev)) {
+		rte_bbdev_log(NOTICE, "%s has no queue enabled and can't be used.",
+				dev->data->name);
+		return -ENODEV;
+	}
+
 	alloc_sw_rings_min_mem(dev, d, num_queues, socket_id);
 
 	/* If minimal memory space approach failed, then allocate
-- 
2.37.1

Re: [PATCH v1 3/6] baseband/acc: fix to acc200 access corner case

[Maxime Coquelin]

On 11/2/22 00:04, Nicolas Chautru wrote:
> To enforce safe access to the ACC200 device, the PMD requires
> to explicitly check that the device is in configured and
> enabled state prior to accessing queue resources.
> This is done by checking the Qmgr ingress queue status.
> 
> Fixes: 40e3adbdd3 ("baseband/acc200: add queue configuration")
> 
> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> ---
>   drivers/baseband/acc/rte_acc200_pmd.c | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/baseband/acc/rte_acc200_pmd.c b/drivers/baseband/acc/rte_acc200_pmd.c
> index 76a5986448..10511d8b56 100644
> --- a/drivers/baseband/acc/rte_acc200_pmd.c
> +++ b/drivers/baseband/acc/rte_acc200_pmd.c
> @@ -172,6 +172,21 @@ updateQtop(uint8_t acc, uint8_t qg, struct rte_acc_conf *acc_conf, struct acc_de
>   	}
>   }
>   
> +/* Check device Qmgr is enabled for protection */
> +static inline bool
> +acc200_check_device_enable(struct rte_bbdev *dev)
> +{
> +	uint32_t reg_aq, qg;
> +	struct acc_device *d = dev->data->dev_private;
> +
> +	for (qg = 0; qg < ACC200_NUM_QGRPS; qg++) {
> +		reg_aq = acc_reg_read(d, queue_offset(d->pf_device, 0, qg, 0));
> +		if (reg_aq & ACC_QUEUE_ENABLE)
> +			return true;
> +	}
> +	return false;
> +}
> +
>   /* Fetch configuration enabled for the PF/VF using MMIO Read (slow). */
>   static inline void
>   fetch_acc200_config(struct rte_bbdev *dev)
> @@ -190,6 +205,12 @@ fetch_acc200_config(struct rte_bbdev *dev)
>   	if (d->configured)
>   		return;
>   
> +	if (!acc200_check_device_enable(dev)) {
> +		rte_bbdev_log(NOTICE, "%s has no queue enabled and can't be used.",
> +				dev->data->name);
> +		return;
> +	}
> +
>   	/* Choose correct registry addresses for the device type. */
>   	if (d->pf_device)
>   		reg_addr = &pf_reg_addr;
> @@ -454,6 +475,12 @@ acc200_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
>   		return -ENODEV;
>   	}
>   
> +	if (!acc200_check_device_enable(dev)) {
> +		rte_bbdev_log(NOTICE, "%s has no queue enabled and can't be used.",
> +				dev->data->name);
> +		return -ENODEV;
> +	}
> +
>   	alloc_sw_rings_min_mem(dev, d, num_queues, socket_id);
>   
>   	/* If minimal memory space approach failed, then allocate

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

Thanks,
Maxime

[PATCH v1 4/6] baseband/acc: fix PMON register values

[Nicolas Chautru]

From: Hernan Vargas <hernan.vargas@intel.com>

Enable properly the PMon for ACC100.
Previous commit was missing actual implementation
and using incorrect register values.

Fixes: b4bd57b74c8 ("baseband/acc100: configure PMON control registers")

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 drivers/baseband/acc/acc100_pmd.h     | 6 ++++--
 drivers/baseband/acc/rte_acc100_pmd.c | 5 +++++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/baseband/acc/acc100_pmd.h b/drivers/baseband/acc/acc100_pmd.h
index 8c0aec5ed8..a48298650c 100644
--- a/drivers/baseband/acc/acc100_pmd.h
+++ b/drivers/baseband/acc/acc100_pmd.h
@@ -146,8 +146,8 @@ static const struct acc100_registry_addr pf_reg_addr = {
 	.depth_log1_offset = HWPfQmgrGrpDepthLog21Vf,
 	.qman_group_func = HWPfQmgrGrpFunction0,
 	.ddr_range = HWPfDmaVfDdrBaseRw,
-	.pmon_ctrl_a = HWVfPmACntrlRegVf,
-	.pmon_ctrl_b = HWVfPmBCntrlRegVf,
+	.pmon_ctrl_a = HWPfPermonACntrlRegVf,
+	.pmon_ctrl_b = HWPfPermonBCntrlRegVf,
 };
 
 /* Structure holding registry addresses for VF */
@@ -177,6 +177,8 @@ static const struct acc100_registry_addr vf_reg_addr = {
 	.depth_log1_offset = HWVfQmgrGrpDepthLog21Vf,
 	.qman_group_func = HWVfQmgrGrpFunction0Vf,
 	.ddr_range = HWVfDmaDdrBaseRangeRoVf,
+	.pmon_ctrl_a = HWVfPmACntrlRegVf,
+	.pmon_ctrl_b = HWVfPmBCntrlRegVf,
 };
 
 #endif /* _RTE_ACC100_PMD_H_ */
diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index b6e500c6bc..2999a6a81a 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -479,6 +479,11 @@ acc100_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
 	/* Read the populated cfg from ACC100 registers */
 	fetch_acc100_config(dev);
 
+	for (value = 0; value <= 2; value++) {
+		acc_reg_write(d, reg_addr->pmon_ctrl_a, value);
+		acc_reg_write(d, reg_addr->pmon_ctrl_b, value);
+	}
+
 	/* Release AXI from PF */
 	if (d->pf_device)
 		acc_reg_write(d, HWPfDmaAxiControl, 1);
-- 
2.37.1

Re: [PATCH v1 4/6] baseband/acc: fix PMON register values

[Maxime Coquelin]

On 11/2/22 00:04, Nicolas Chautru wrote:
> From: Hernan Vargas <hernan.vargas@intel.com>
> 
> Enable properly the PMon for ACC100.
> Previous commit was missing actual implementation
> and using incorrect register values.
> 
> Fixes: b4bd57b74c8 ("baseband/acc100: configure PMON control registers")
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   drivers/baseband/acc/acc100_pmd.h     | 6 ++++--
>   drivers/baseband/acc/rte_acc100_pmd.c | 5 +++++
>   2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/baseband/acc/acc100_pmd.h b/drivers/baseband/acc/acc100_pmd.h
> index 8c0aec5ed8..a48298650c 100644
> --- a/drivers/baseband/acc/acc100_pmd.h
> +++ b/drivers/baseband/acc/acc100_pmd.h
> @@ -146,8 +146,8 @@ static const struct acc100_registry_addr pf_reg_addr = {
>   	.depth_log1_offset = HWPfQmgrGrpDepthLog21Vf,
>   	.qman_group_func = HWPfQmgrGrpFunction0,
>   	.ddr_range = HWPfDmaVfDdrBaseRw,
> -	.pmon_ctrl_a = HWVfPmACntrlRegVf,
> -	.pmon_ctrl_b = HWVfPmBCntrlRegVf,
> +	.pmon_ctrl_a = HWPfPermonACntrlRegVf,
> +	.pmon_ctrl_b = HWPfPermonBCntrlRegVf,
>   };
>   
>   /* Structure holding registry addresses for VF */
> @@ -177,6 +177,8 @@ static const struct acc100_registry_addr vf_reg_addr = {
>   	.depth_log1_offset = HWVfQmgrGrpDepthLog21Vf,
>   	.qman_group_func = HWVfQmgrGrpFunction0Vf,
>   	.ddr_range = HWVfDmaDdrBaseRangeRoVf,
> +	.pmon_ctrl_a = HWVfPmACntrlRegVf,
> +	.pmon_ctrl_b = HWVfPmBCntrlRegVf,
>   };
>   
>   #endif /* _RTE_ACC100_PMD_H_ */
> diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
> index b6e500c6bc..2999a6a81a 100644
> --- a/drivers/baseband/acc/rte_acc100_pmd.c
> +++ b/drivers/baseband/acc/rte_acc100_pmd.c
> @@ -479,6 +479,11 @@ acc100_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
>   	/* Read the populated cfg from ACC100 registers */
>   	fetch_acc100_config(dev);
>   
> +	for (value = 0; value <= 2; value++) {
> +		acc_reg_write(d, reg_addr->pmon_ctrl_a, value);
> +		acc_reg_write(d, reg_addr->pmon_ctrl_b, value);
> +	}
> +
>   	/* Release AXI from PF */
>   	if (d->pf_device)
>   		acc_reg_write(d, HWPfDmaAxiControl, 1);

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

Thanks,
Maxime

[PATCH v1 5/6] baseband/acc: fix double MSI intr in TB mode

[Nicolas Chautru]

From: Hernan Vargas <hernan.vargas@intel.com>

Fix logical bug in SW causing MSI to be issued twice when running in
transport block mode.

Fixes: f404dfe35cc ("baseband/acc100: support 4G processing")
Fixes: bec597b78a0 ("baseband/acc200: add LTE processing")
Cc: stable@dpdk.org

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 drivers/baseband/acc/acc_common.h     |  7 +++++-
 drivers/baseband/acc/rte_acc100_pmd.c | 29 -----------------------
 drivers/baseband/acc/rte_acc200_pmd.c | 34 ---------------------------
 3 files changed, 6 insertions(+), 64 deletions(-)

diff --git a/drivers/baseband/acc/acc_common.h b/drivers/baseband/acc/acc_common.h
index eae7eab4e9..5cfa2b64ba 100644
--- a/drivers/baseband/acc/acc_common.h
+++ b/drivers/baseband/acc/acc_common.h
@@ -908,6 +908,7 @@ acc_dma_enqueue(struct acc_queue *q, uint16_t n,
 		struct rte_bbdev_stats *queue_stats)
 {
 	union acc_enqueue_reg_fmt enq_req;
+	union acc_dma_desc *desc;
 #ifdef RTE_BBDEV_OFFLOAD_COST
 	uint64_t start_time = 0;
 	queue_stats->acc_offload_cycles = 0;
@@ -915,13 +916,17 @@ acc_dma_enqueue(struct acc_queue *q, uint16_t n,
 	RTE_SET_USED(queue_stats);
 #endif
 
+	/* Set Sdone and IRQ enable bit on last descriptor. */
+	desc = acc_desc(q, n - 1);
+	desc->req.sdone_enable = 1;
+	desc->req.irq_enable = q->irq_enable;
+
 	enq_req.val = 0;
 	/* Setting offset, 100b for 256 DMA Desc */
 	enq_req.addr_offset = ACC_DESC_OFFSET;
 
 	/* Split ops into batches */
 	do {
-		union acc_dma_desc *desc;
 		uint16_t enq_batch_size;
 		uint64_t offset;
 		rte_iova_t req_elem_addr;
diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index 2999a6a81a..727a718e9d 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -2671,7 +2671,6 @@ enqueue_enc_one_op_tb(struct acc_queue *q, struct rte_bbdev_enc_op *op,
 
 	/* Set SDone on last CB descriptor for TB mode. */
 	desc->req.sdone_enable = 1;
-	desc->req.irq_enable = q->irq_enable;
 
 	return current_enqueued_cbs;
 }
@@ -2741,7 +2740,6 @@ enqueue_ldpc_enc_one_op_tb(struct acc_queue *q, struct rte_bbdev_enc_op *op,
 	desc_idx = ((q->sw_ring_head + enq_descs - 1) & q->sw_ring_wrap_mask);
 	desc = q->ring_addr + desc_idx;
 	desc->req.sdone_enable = 1;
-	desc->req.irq_enable = q->irq_enable;
 	desc->req.op_addr = op;
 	return return_descs;
 }
@@ -3303,7 +3301,6 @@ enqueue_ldpc_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 #endif
 	/* Set SDone on last CB descriptor for TB mode */
 	desc->req.sdone_enable = 1;
-	desc->req.irq_enable = q->irq_enable;
 
 	return current_enqueued_cbs;
 }
@@ -3408,7 +3405,6 @@ enqueue_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 #endif
 	/* Set SDone on last CB descriptor for TB mode */
 	desc->req.sdone_enable = 1;
-	desc->req.irq_enable = q->irq_enable;
 
 	return current_enqueued_cbs;
 }
@@ -3421,7 +3417,6 @@ acc100_enqueue_enc_cb(struct rte_bbdev_queue_data *q_data,
 	struct acc_queue *q = q_data->queue_private;
 	int32_t avail = acc_ring_avail_enq(q);
 	uint16_t i;
-	union acc_dma_desc *desc;
 	int ret;
 
 	for (i = 0; i < num; ++i) {
@@ -3442,11 +3437,6 @@ acc100_enqueue_enc_cb(struct rte_bbdev_queue_data *q_data,
 	if (unlikely(i == 0))
 		return 0; /* Nothing to enqueue */
 
-	/* Set SDone in last CB in enqueued ops for CB mode*/
-	desc = acc_desc(q, i - 1);
-	desc->req.sdone_enable = 1;
-	desc->req.irq_enable = q->irq_enable;
-
 	acc_dma_enqueue(q, i, &q_data->queue_stats);
 
 	/* Update stats */
@@ -3463,7 +3453,6 @@ acc100_enqueue_ldpc_enc_cb(struct rte_bbdev_queue_data *q_data,
 	struct acc_queue *q = q_data->queue_private;
 	int32_t avail = acc_ring_avail_enq(q);
 	uint16_t i = 0;
-	union acc_dma_desc *desc;
 	int ret, desc_idx = 0;
 	int16_t enq, left = num;
 
@@ -3497,11 +3486,6 @@ acc100_enqueue_ldpc_enc_cb(struct rte_bbdev_queue_data *q_data,
 	if (unlikely(i == 0))
 		return 0; /* Nothing to enqueue */
 
-	/* Set SDone in last CB in enqueued ops for CB mode*/
-	desc = acc_desc(q, desc_idx - 1);
-	desc->req.sdone_enable = 1;
-	desc->req.irq_enable = q->irq_enable;
-
 	acc_dma_enqueue(q, desc_idx, &q_data->queue_stats);
 
 	/* Update stats */
@@ -3625,7 +3609,6 @@ acc100_enqueue_dec_cb(struct rte_bbdev_queue_data *q_data,
 	struct acc_queue *q = q_data->queue_private;
 	int32_t avail = acc_ring_avail_enq(q);
 	uint16_t i;
-	union acc_dma_desc *desc;
 	int ret;
 
 	for (i = 0; i < num; ++i) {
@@ -3646,11 +3629,6 @@ acc100_enqueue_dec_cb(struct rte_bbdev_queue_data *q_data,
 	if (unlikely(i == 0))
 		return 0; /* Nothing to enqueue */
 
-	/* Set SDone in last CB in enqueued ops for CB mode*/
-	desc = acc_desc(q, i - 1);
-	desc->req.sdone_enable = 1;
-	desc->req.irq_enable = q->irq_enable;
-
 	acc_dma_enqueue(q, i, &q_data->queue_stats);
 
 	/* Update stats */
@@ -3705,7 +3683,6 @@ acc100_enqueue_ldpc_dec_cb(struct rte_bbdev_queue_data *q_data,
 	struct acc_queue *q = q_data->queue_private;
 	int32_t avail = acc_ring_avail_enq(q);
 	uint16_t i;
-	union acc_dma_desc *desc;
 	int ret;
 	bool same_op = false;
 	for (i = 0; i < num; ++i) {
@@ -3735,12 +3712,6 @@ acc100_enqueue_ldpc_dec_cb(struct rte_bbdev_queue_data *q_data,
 	if (unlikely(i == 0))
 		return 0; /* Nothing to enqueue */
 
-	/* Set SDone in last CB in enqueued ops for CB mode*/
-	desc = acc_desc(q, i - 1);
-
-	desc->req.sdone_enable = 1;
-	desc->req.irq_enable = q->irq_enable;
-
 	acc_dma_enqueue(q, i, &q_data->queue_stats);
 
 	/* Update stats */
diff --git a/drivers/baseband/acc/rte_acc200_pmd.c b/drivers/baseband/acc/rte_acc200_pmd.c
index 10511d8b56..fd725e996e 100644
--- a/drivers/baseband/acc/rte_acc200_pmd.c
+++ b/drivers/baseband/acc/rte_acc200_pmd.c
@@ -1884,7 +1884,6 @@ enqueue_enc_one_op_tb(struct acc_queue *q, struct rte_bbdev_enc_op *op,
 
 	/* Set SDone on last CB descriptor for TB mode. */
 	desc->req.sdone_enable = 1;
-	desc->req.irq_enable = q->irq_enable;
 
 	return current_enqueued_cbs;
 }
@@ -1945,7 +1944,6 @@ enqueue_ldpc_enc_one_op_tb(struct acc_queue *q, struct rte_bbdev_enc_op *op,
 	/* Set SDone on last CB descriptor for TB mode. */
 	desc = acc_desc(q, enq_descs - 1);
 	desc->req.sdone_enable = 1;
-	desc->req.irq_enable = q->irq_enable;
 	desc->req.op_addr = op;
 	return return_descs;
 }
@@ -2181,7 +2179,6 @@ enqueue_ldpc_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 #endif
 	/* Set SDone on last CB descriptor for TB mode. */
 	desc->req.sdone_enable = 1;
-	desc->req.irq_enable = q->irq_enable;
 
 	return current_enqueued_cbs;
 }
@@ -2270,7 +2267,6 @@ enqueue_dec_one_op_tb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
 
 	/* Set SDone on last CB descriptor for TB mode */
 	desc->req.sdone_enable = 1;
-	desc->req.irq_enable = q->irq_enable;
 
 	return current_enqueued_cbs;
 }
@@ -2283,7 +2279,6 @@ acc200_enqueue_enc_cb(struct rte_bbdev_queue_data *q_data,
 	struct acc_queue *q = q_data->queue_private;
 	int32_t avail = acc_ring_avail_enq(q);
 	uint16_t i;
-	union acc_dma_desc *desc;
 	int ret;
 
 	for (i = 0; i < num; ++i) {
@@ -2304,11 +2299,6 @@ acc200_enqueue_enc_cb(struct rte_bbdev_queue_data *q_data,
 	if (unlikely(i == 0))
 		return 0; /* Nothing to enqueue */
 
-	/* Set SDone in last CB in enqueued ops for CB mode*/
-	desc = acc_desc(q, i - 1);
-	desc->req.sdone_enable = 1;
-	desc->req.irq_enable = q->irq_enable;
-
 	acc_dma_enqueue(q, i, &q_data->queue_stats);
 
 	/* Update stats */
@@ -2325,7 +2315,6 @@ acc200_enqueue_ldpc_enc_cb(struct rte_bbdev_queue_data *q_data,
 	struct acc_queue *q = q_data->queue_private;
 	int32_t avail = acc_ring_avail_enq(q);
 	uint16_t i = 0;
-	union acc_dma_desc *desc;
 	int ret, desc_idx = 0;
 	int16_t enq, left = num;
 
@@ -2350,11 +2339,6 @@ acc200_enqueue_ldpc_enc_cb(struct rte_bbdev_queue_data *q_data,
 	if (unlikely(i == 0))
 		return 0; /* Nothing to enqueue. */
 
-	/* Set SDone in last CB in enqueued ops for CB mode. */
-	desc = acc_desc(q, desc_idx - 1);
-	desc->req.sdone_enable = 1;
-	desc->req.irq_enable = q->irq_enable;
-
 	acc_dma_enqueue(q, desc_idx, &q_data->queue_stats);
 
 	/* Update stats. */
@@ -2479,7 +2463,6 @@ acc200_enqueue_dec_cb(struct rte_bbdev_queue_data *q_data,
 	struct acc_queue *q = q_data->queue_private;
 	int32_t avail = acc_ring_avail_enq(q);
 	uint16_t i;
-	union acc_dma_desc *desc;
 	int ret;
 
 	for (i = 0; i < num; ++i) {
@@ -2496,11 +2479,6 @@ acc200_enqueue_dec_cb(struct rte_bbdev_queue_data *q_data,
 	if (unlikely(i == 0))
 		return 0; /* Nothing to enqueue. */
 
-	/* Set SDone in last CB in enqueued ops for CB mode. */
-	desc = acc_desc(q, i - 1);
-	desc->req.sdone_enable = 1;
-	desc->req.irq_enable = q->irq_enable;
-
 	acc_dma_enqueue(q, i, &q_data->queue_stats);
 
 	/* Update stats. */
@@ -2552,7 +2530,6 @@ acc200_enqueue_ldpc_dec_cb(struct rte_bbdev_queue_data *q_data,
 	struct acc_queue *q = q_data->queue_private;
 	int32_t avail = acc_ring_avail_enq(q);
 	uint16_t i;
-	union acc_dma_desc *desc;
 	int ret;
 	bool same_op = false;
 
@@ -2581,11 +2558,6 @@ acc200_enqueue_ldpc_dec_cb(struct rte_bbdev_queue_data *q_data,
 	if (unlikely(i == 0))
 		return 0; /* Nothing to enqueue. */
 
-	/* Set SDone in last CB in enqueued ops for CB mode. */
-	desc = acc_desc(q, i - 1);
-	desc->req.sdone_enable = 1;
-	desc->req.irq_enable = q->irq_enable;
-
 	acc_dma_enqueue(q, i, &q_data->queue_stats);
 
 	/* Update stats. */
@@ -3234,7 +3206,6 @@ acc200_enqueue_fft(struct rte_bbdev_queue_data *q_data,
 	struct acc_queue *q;
 	int32_t aq_avail, avail;
 	uint16_t i;
-	union acc_dma_desc *desc;
 	int ret;
 
 	aq_avail = acc_aq_avail(q_data, num);
@@ -3256,11 +3227,6 @@ acc200_enqueue_fft(struct rte_bbdev_queue_data *q_data,
 	if (unlikely(i == 0))
 		return 0; /* Nothing to enqueue. */
 
-	/* Set SDone in last CB in enqueued ops for CB mode. */
-	desc = acc_desc(q, i - 1);
-
-	desc->req.sdone_enable = 1;
-	desc->req.irq_enable = q->irq_enable;
 	acc_dma_enqueue(q, i, &q_data->queue_stats);
 
 	/* Update stats */
-- 
2.37.1

Re: [PATCH v1 5/6] baseband/acc: fix double MSI intr in TB mode

[Maxime Coquelin]

On 11/2/22 00:04, Nicolas Chautru wrote:
> From: Hernan Vargas <hernan.vargas@intel.com>
> 
> Fix logical bug in SW causing MSI to be issued twice when running in
> transport block mode.
> 
> Fixes: f404dfe35cc ("baseband/acc100: support 4G processing")
> Fixes: bec597b78a0 ("baseband/acc200: add LTE processing")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   drivers/baseband/acc/acc_common.h     |  7 +++++-
>   drivers/baseband/acc/rte_acc100_pmd.c | 29 -----------------------
>   drivers/baseband/acc/rte_acc200_pmd.c | 34 ---------------------------
>   3 files changed, 6 insertions(+), 64 deletions(-)
> 

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

Thanks,
Maxime

[PATCH v1 6/6] baseband/acc: fix redundant function definition

[Nicolas Chautru]

From: Hernan Vargas <hernan.vargas@intel.com>

Remove acc100_dma_fill_blk_type_in which already exists in acc_common.h

Fixes: 32e8b7ea35d ("baseband/acc100: refactor to segregate common code")

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

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index 727a718e9d..31076d382f 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -1435,93 +1435,6 @@ acc101_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
 	}
 }
 
-/**
- * Fills descriptor with data pointers of one block type.
- *
- * @param desc
- *   Pointer to DMA descriptor.
- * @param input
- *   Pointer to pointer to input data which will be encoded. It can be changed
- *   and points to next segment in scatter-gather case.
- * @param offset
- *   Input offset in rte_mbuf structure. It is used for calculating the point
- *   where data is starting.
- * @param cb_len
- *   Length of currently processed Code Block
- * @param seg_total_left
- *   It indicates how many bytes still left in segment (mbuf) for further
- *   processing.
- * @param op_flags
- *   Store information about device capabilities
- * @param next_triplet
- *   Index for ACC100 DMA Descriptor triplet
- * @param scattergather
- *   Flag to support scatter-gather for the mbuf
- *
- * @return
- *   Returns index of next triplet on success, other value if lengths of
- *   pkt and processed cb do not match.
- *
- */
-static inline int
-acc100_dma_fill_blk_type_in(struct acc_dma_req_desc *desc,
-		struct rte_mbuf **input, uint32_t *offset, uint32_t cb_len,
-		uint32_t *seg_total_left, int next_triplet,
-		bool scattergather)
-{
-	uint32_t part_len;
-	struct rte_mbuf *m = *input;
-
-	if (scattergather)
-		part_len = (*seg_total_left < cb_len) ? *seg_total_left : cb_len;
-	else
-		part_len = cb_len;
-	cb_len -= part_len;
-	*seg_total_left -= part_len;
-
-	desc->data_ptrs[next_triplet].address =
-			rte_pktmbuf_iova_offset(m, *offset);
-	desc->data_ptrs[next_triplet].blen = part_len;
-	desc->data_ptrs[next_triplet].blkid = ACC_DMA_BLKID_IN;
-	desc->data_ptrs[next_triplet].last = 0;
-	desc->data_ptrs[next_triplet].dma_ext = 0;
-	*offset += part_len;
-	next_triplet++;
-
-	while (cb_len > 0) {
-		if (next_triplet < ACC_DMA_MAX_NUM_POINTERS_IN && m->next != NULL) {
-
-			m = m->next;
-			*seg_total_left = rte_pktmbuf_data_len(m);
-			part_len = (*seg_total_left < cb_len) ?
-					*seg_total_left :
-					cb_len;
-			desc->data_ptrs[next_triplet].address =
-					rte_pktmbuf_iova_offset(m, 0);
-			desc->data_ptrs[next_triplet].blen = part_len;
-			desc->data_ptrs[next_triplet].blkid =
-					ACC_DMA_BLKID_IN;
-			desc->data_ptrs[next_triplet].last = 0;
-			desc->data_ptrs[next_triplet].dma_ext = 0;
-			cb_len -= part_len;
-			*seg_total_left -= part_len;
-			/* Initializing offset for next segment (mbuf) */
-			*offset = part_len;
-			next_triplet++;
-		} else {
-			rte_bbdev_log(ERR,
-				"Some data still left for processing: "
-				"data_left: %u, next_triplet: %u, next_mbuf: %p",
-				cb_len, next_triplet, m->next);
-			return -EINVAL;
-		}
-	}
-	/* Storing new mbuf as it could be changed in scatter-gather case*/
-	*input = m;
-
-	return next_triplet;
-}
-
 /* May need to pad LDPC Encoder input to avoid small beat for ACC100. */
 static inline uint16_t
 pad_le_in(uint16_t blen, struct acc_queue *q)
@@ -1565,7 +1478,7 @@ acc100_dma_desc_le_fill(struct rte_bbdev_enc_op *op,
 		return -1;
 	}
 
-	next_triplet = acc100_dma_fill_blk_type_in(desc, input, in_offset,
+	next_triplet = acc_dma_fill_blk_type_in(desc, input, in_offset,
 			pad_le_in(in_length_in_bytes, q), seg_total_left, next_triplet, false);
 	if (unlikely(next_triplet < 0)) {
 		rte_bbdev_log(ERR,
@@ -1653,7 +1566,7 @@ acc100_dma_desc_td_fill(struct rte_bbdev_dec_op *op,
 		return -1;
 	}
 
-	next_triplet = acc100_dma_fill_blk_type_in(desc, input, in_offset, kw,
+	next_triplet = acc_dma_fill_blk_type_in(desc, input, in_offset, kw,
 			seg_total_left, next_triplet,
 			check_bit(op->turbo_dec.op_flags,
 			RTE_BBDEV_TURBO_DEC_SCATTER_GATHER));
@@ -1756,7 +1669,7 @@ acc100_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
 		return -1;
 	}
 
-	next_triplet = acc100_dma_fill_blk_type_in(desc, input,
+	next_triplet = acc_dma_fill_blk_type_in(desc, input,
 			in_offset, input_length,
 			seg_total_left, next_triplet,
 			check_bit(op->ldpc_dec.op_flags,
-- 
2.37.1

Re: [PATCH v1 6/6] baseband/acc: fix redundant function definition

[Maxime Coquelin]

On 11/2/22 00:04, Nicolas Chautru wrote:
> From: Hernan Vargas <hernan.vargas@intel.com>
> 
> Remove acc100_dma_fill_blk_type_in which already exists in acc_common.h
> 
> Fixes: 32e8b7ea35d ("baseband/acc100: refactor to segregate common code")
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   drivers/baseband/acc/rte_acc100_pmd.c | 93 +--------------------------
>   1 file changed, 3 insertions(+), 90 deletions(-)
> 
> diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
> index 727a718e9d..31076d382f 100644
> --- a/drivers/baseband/acc/rte_acc100_pmd.c
> +++ b/drivers/baseband/acc/rte_acc100_pmd.c
> @@ -1435,93 +1435,6 @@ acc101_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw,
>   	}
>   }
>   
> -/**
> - * Fills descriptor with data pointers of one block type.
> - *
> - * @param desc
> - *   Pointer to DMA descriptor.
> - * @param input
> - *   Pointer to pointer to input data which will be encoded. It can be changed
> - *   and points to next segment in scatter-gather case.
> - * @param offset
> - *   Input offset in rte_mbuf structure. It is used for calculating the point
> - *   where data is starting.
> - * @param cb_len
> - *   Length of currently processed Code Block
> - * @param seg_total_left
> - *   It indicates how many bytes still left in segment (mbuf) for further
> - *   processing.
> - * @param op_flags
> - *   Store information about device capabilities
> - * @param next_triplet
> - *   Index for ACC100 DMA Descriptor triplet
> - * @param scattergather
> - *   Flag to support scatter-gather for the mbuf
> - *
> - * @return
> - *   Returns index of next triplet on success, other value if lengths of
> - *   pkt and processed cb do not match.
> - *
> - */
> -static inline int
> -acc100_dma_fill_blk_type_in(struct acc_dma_req_desc *desc,
> -		struct rte_mbuf **input, uint32_t *offset, uint32_t cb_len,
> -		uint32_t *seg_total_left, int next_triplet,
> -		bool scattergather)
> -{
> -	uint32_t part_len;
> -	struct rte_mbuf *m = *input;
> -
> -	if (scattergather)
> -		part_len = (*seg_total_left < cb_len) ? *seg_total_left : cb_len;
> -	else
> -		part_len = cb_len;
> -	cb_len -= part_len;
> -	*seg_total_left -= part_len;
> -
> -	desc->data_ptrs[next_triplet].address =
> -			rte_pktmbuf_iova_offset(m, *offset);
> -	desc->data_ptrs[next_triplet].blen = part_len;
> -	desc->data_ptrs[next_triplet].blkid = ACC_DMA_BLKID_IN;
> -	desc->data_ptrs[next_triplet].last = 0;
> -	desc->data_ptrs[next_triplet].dma_ext = 0;
> -	*offset += part_len;
> -	next_triplet++;
> -
> -	while (cb_len > 0) {
> -		if (next_triplet < ACC_DMA_MAX_NUM_POINTERS_IN && m->next != NULL) {
> -
> -			m = m->next;
> -			*seg_total_left = rte_pktmbuf_data_len(m);
> -			part_len = (*seg_total_left < cb_len) ?
> -					*seg_total_left :
> -					cb_len;
> -			desc->data_ptrs[next_triplet].address =
> -					rte_pktmbuf_iova_offset(m, 0);
> -			desc->data_ptrs[next_triplet].blen = part_len;
> -			desc->data_ptrs[next_triplet].blkid =
> -					ACC_DMA_BLKID_IN;
> -			desc->data_ptrs[next_triplet].last = 0;
> -			desc->data_ptrs[next_triplet].dma_ext = 0;
> -			cb_len -= part_len;
> -			*seg_total_left -= part_len;
> -			/* Initializing offset for next segment (mbuf) */
> -			*offset = part_len;
> -			next_triplet++;
> -		} else {
> -			rte_bbdev_log(ERR,
> -				"Some data still left for processing: "
> -				"data_left: %u, next_triplet: %u, next_mbuf: %p",
> -				cb_len, next_triplet, m->next);
> -			return -EINVAL;
> -		}
> -	}
> -	/* Storing new mbuf as it could be changed in scatter-gather case*/
> -	*input = m;
> -
> -	return next_triplet;
> -}
> -
>   /* May need to pad LDPC Encoder input to avoid small beat for ACC100. */
>   static inline uint16_t
>   pad_le_in(uint16_t blen, struct acc_queue *q)
> @@ -1565,7 +1478,7 @@ acc100_dma_desc_le_fill(struct rte_bbdev_enc_op *op,
>   		return -1;
>   	}
>   
> -	next_triplet = acc100_dma_fill_blk_type_in(desc, input, in_offset,
> +	next_triplet = acc_dma_fill_blk_type_in(desc, input, in_offset,
>   			pad_le_in(in_length_in_bytes, q), seg_total_left, next_triplet, false);
>   	if (unlikely(next_triplet < 0)) {
>   		rte_bbdev_log(ERR,
> @@ -1653,7 +1566,7 @@ acc100_dma_desc_td_fill(struct rte_bbdev_dec_op *op,
>   		return -1;
>   	}
>   
> -	next_triplet = acc100_dma_fill_blk_type_in(desc, input, in_offset, kw,
> +	next_triplet = acc_dma_fill_blk_type_in(desc, input, in_offset, kw,
>   			seg_total_left, next_triplet,
>   			check_bit(op->turbo_dec.op_flags,
>   			RTE_BBDEV_TURBO_DEC_SCATTER_GATHER));
> @@ -1756,7 +1669,7 @@ acc100_dma_desc_ld_fill(struct rte_bbdev_dec_op *op,
>   		return -1;
>   	}
>   
> -	next_triplet = acc100_dma_fill_blk_type_in(desc, input,
> +	next_triplet = acc_dma_fill_blk_type_in(desc, input,
>   			in_offset, input_length,
>   			seg_total_left, next_triplet,
>   			check_bit(op->ldpc_dec.op_flags,

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

Thanks,
Maxime

RE: [PATCH v1 6/6] baseband/acc: fix redundant function definition

[Chautru, Nicolas]

Thanks Maxime for the quick reviews. 

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Wednesday, November 2, 2022 12:19 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> gakhil@marvell.com; Vargas, Hernan <hernan.vargas@intel.com>
> Subject: Re: [PATCH v1 6/6] baseband/acc: fix redundant function definition
> 
> 
> 
> On 11/2/22 00:04, Nicolas Chautru wrote:
> > From: Hernan Vargas <hernan.vargas@intel.com>
> >
> > Remove acc100_dma_fill_blk_type_in which already exists in
> > acc_common.h
> >
> > Fixes: 32e8b7ea35d ("baseband/acc100: refactor to segregate common
> > code")
> >
> > Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> > ---
> >   drivers/baseband/acc/rte_acc100_pmd.c | 93 +--------------------------
> >   1 file changed, 3 insertions(+), 90 deletions(-)
> >
> > diff --git a/drivers/baseband/acc/rte_acc100_pmd.c
> > b/drivers/baseband/acc/rte_acc100_pmd.c
> > index 727a718e9d..31076d382f 100644
> > --- a/drivers/baseband/acc/rte_acc100_pmd.c
> > +++ b/drivers/baseband/acc/rte_acc100_pmd.c
> > @@ -1435,93 +1435,6 @@ acc101_fcw_ld_fill(struct rte_bbdev_dec_op *op,
> struct acc_fcw_ld *fcw,
> >   	}
> >   }
> >
> > -/**
> > - * Fills descriptor with data pointers of one block type.
> > - *
> > - * @param desc
> > - *   Pointer to DMA descriptor.
> > - * @param input
> > - *   Pointer to pointer to input data which will be encoded. It can be changed
> > - *   and points to next segment in scatter-gather case.
> > - * @param offset
> > - *   Input offset in rte_mbuf structure. It is used for calculating the point
> > - *   where data is starting.
> > - * @param cb_len
> > - *   Length of currently processed Code Block
> > - * @param seg_total_left
> > - *   It indicates how many bytes still left in segment (mbuf) for further
> > - *   processing.
> > - * @param op_flags
> > - *   Store information about device capabilities
> > - * @param next_triplet
> > - *   Index for ACC100 DMA Descriptor triplet
> > - * @param scattergather
> > - *   Flag to support scatter-gather for the mbuf
> > - *
> > - * @return
> > - *   Returns index of next triplet on success, other value if lengths of
> > - *   pkt and processed cb do not match.
> > - *
> > - */
> > -static inline int
> > -acc100_dma_fill_blk_type_in(struct acc_dma_req_desc *desc,
> > -		struct rte_mbuf **input, uint32_t *offset, uint32_t cb_len,
> > -		uint32_t *seg_total_left, int next_triplet,
> > -		bool scattergather)
> > -{
> > -	uint32_t part_len;
> > -	struct rte_mbuf *m = *input;
> > -
> > -	if (scattergather)
> > -		part_len = (*seg_total_left < cb_len) ? *seg_total_left : cb_len;
> > -	else
> > -		part_len = cb_len;
> > -	cb_len -= part_len;
> > -	*seg_total_left -= part_len;
> > -
> > -	desc->data_ptrs[next_triplet].address =
> > -			rte_pktmbuf_iova_offset(m, *offset);
> > -	desc->data_ptrs[next_triplet].blen = part_len;
> > -	desc->data_ptrs[next_triplet].blkid = ACC_DMA_BLKID_IN;
> > -	desc->data_ptrs[next_triplet].last = 0;
> > -	desc->data_ptrs[next_triplet].dma_ext = 0;
> > -	*offset += part_len;
> > -	next_triplet++;
> > -
> > -	while (cb_len > 0) {
> > -		if (next_triplet < ACC_DMA_MAX_NUM_POINTERS_IN && m-
> >next != NULL) {
> > -
> > -			m = m->next;
> > -			*seg_total_left = rte_pktmbuf_data_len(m);
> > -			part_len = (*seg_total_left < cb_len) ?
> > -					*seg_total_left :
> > -					cb_len;
> > -			desc->data_ptrs[next_triplet].address =
> > -					rte_pktmbuf_iova_offset(m, 0);
> > -			desc->data_ptrs[next_triplet].blen = part_len;
> > -			desc->data_ptrs[next_triplet].blkid =
> > -					ACC_DMA_BLKID_IN;
> > -			desc->data_ptrs[next_triplet].last = 0;
> > -			desc->data_ptrs[next_triplet].dma_ext = 0;
> > -			cb_len -= part_len;
> > -			*seg_total_left -= part_len;
> > -			/* Initializing offset for next segment (mbuf) */
> > -			*offset = part_len;
> > -			next_triplet++;
> > -		} else {
> > -			rte_bbdev_log(ERR,
> > -				"Some data still left for processing: "
> > -				"data_left: %u, next_triplet: %u, next_mbuf:
> %p",
> > -				cb_len, next_triplet, m->next);
> > -			return -EINVAL;
> > -		}
> > -	}
> > -	/* Storing new mbuf as it could be changed in scatter-gather case*/
> > -	*input = m;
> > -
> > -	return next_triplet;
> > -}
> > -
> >   /* May need to pad LDPC Encoder input to avoid small beat for ACC100. */
> >   static inline uint16_t
> >   pad_le_in(uint16_t blen, struct acc_queue *q) @@ -1565,7 +1478,7 @@
> > acc100_dma_desc_le_fill(struct rte_bbdev_enc_op *op,
> >   		return -1;
> >   	}
> >
> > -	next_triplet = acc100_dma_fill_blk_type_in(desc, input, in_offset,
> > +	next_triplet = acc_dma_fill_blk_type_in(desc, input, in_offset,
> >   			pad_le_in(in_length_in_bytes, q), seg_total_left,
> next_triplet, false);
> >   	if (unlikely(next_triplet < 0)) {
> >   		rte_bbdev_log(ERR,
> > @@ -1653,7 +1566,7 @@ acc100_dma_desc_td_fill(struct rte_bbdev_dec_op
> *op,
> >   		return -1;
> >   	}
> >
> > -	next_triplet = acc100_dma_fill_blk_type_in(desc, input, in_offset, kw,
> > +	next_triplet = acc_dma_fill_blk_type_in(desc, input, in_offset, kw,
> >   			seg_total_left, next_triplet,
> >   			check_bit(op->turbo_dec.op_flags,
> >   			RTE_BBDEV_TURBO_DEC_SCATTER_GATHER));
> > @@ -1756,7 +1669,7 @@ acc100_dma_desc_ld_fill(struct rte_bbdev_dec_op
> *op,
> >   		return -1;
> >   	}
> >
> > -	next_triplet = acc100_dma_fill_blk_type_in(desc, input,
> > +	next_triplet = acc_dma_fill_blk_type_in(desc, input,
> >   			in_offset, input_length,
> >   			seg_total_left, next_triplet,
> >   			check_bit(op->ldpc_dec.op_flags,
> 
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> Thanks,
> Maxime

RE: [EXT] [PATCH v1 0/6] baseband/acc: fixes on top of RC2

[Akhil Goyal]

> A few fixes still required for the baseband acc PMDs
> for ACC100 and ACC200 on top of RC2. Most of them
> introduced recently including one covery warning.
> All changes pushed in same serie for ease of review/apply.
> 
> Hernan Vargas (3):
>   baseband/acc: fix PMON register values
>   baseband/acc: fix double MSI intr in TB mode
>   baseband/acc: fix redundant function definition
> 
> Nicolas Chautru (3):
>   baseband/acc: fix LTE half iteration flag
>   baseband/acc: fix to possible overflow
>   baseband/acc: fix to acc200 access corner case
> 
>  drivers/baseband/acc/acc100_pmd.h     |   6 +-
>  drivers/baseband/acc/acc_common.h     |   7 +-
>  drivers/baseband/acc/rte_acc100_pmd.c | 130 ++------------------------
>  drivers/baseband/acc/rte_acc200_pmd.c |  66 ++++++-------
>  4 files changed, 48 insertions(+), 161 deletions(-)
> 
Series Applied to dpdk-next-crypto

Thanks.