acc baseband PMD fix and updates for 24.11
v2 cover acc baseband PMD fix and updates for 24.11
This series targets 24.11.
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 (10):
baseband/acc: fix access to deallocated mem
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: remove soft output bypass
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
drivers/baseband/acc/acc_common.h | 16 +- drivers/baseband/acc/rte_acc100_pmd.c | 38 +-- drivers/baseband/acc/rte_vrb_pmd.c | 347 ++++++++++++++++---------- 3 files changed, 224 insertions(+), 177 deletions(-)
Signature · 1 line
2.37.1
PATCH v2 1/10 baseband/acc: fix access to deallocated mem
Prevent op_addr access during queue_stop operation, as this memory may have been deallocated.
--- drivers/baseband/acc/rte_acc100_pmd.c | 36 ---------------------- drivers/baseband/acc/rte_vrb_pmd.c | 44 +-------------------------- 2 files changed, 1 insertion(+), 79 deletions(-)
drivers/baseband/acc/rte_acc100_pmd.c +0 -36
@@ -837,51 +837,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 ;
drivers/baseband/acc/rte_vrb_pmd.c +1 -43
59 lines not on this page · raw message
Signature · 1 line
2.37.1
PATCH v2 2/10 baseband/acc: queue allocation refactor
In reply to Hernan VargasRefactor to manage queue memory per operation more flexibly for VRB devices.
--- drivers/baseband/acc/acc_common.h | 5 + drivers/baseband/acc/rte_vrb_pmd.c | 214 ++++++++++++++++++++--------- 2 files changed, 157 insertions(+), 62 deletions(-)
drivers/baseband/acc/acc_common.h +5 -0
@@ -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 256extern int 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
drivers/baseband/acc/rte_vrb_pmd.c +152 -62
300 lines not on this page · raw message
Signature · 1 line
2.37.1
Re: [PATCH v2 02/10] baseband/acc: queue allocation refactor
Quoted 64 lines · Hernan Vargas
Refactor to manage queue memory per operation more flexibly for VRB devices.
Signed-off-by Hernan Vargas <[email protected]>--- drivers/baseband/acc/acc_common.h | 5 + drivers/baseband/acc/rte_vrb_pmd.c | 214 ++++++++++++++++++++--------- 2 files changed, 157 insertions(+), 62 deletions(-)drivers/baseband/acc/acc_common.h +5
-0@@ -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 256extern int 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 drivers/baseband/acc/rte_vrb_pmd.c +3
-3@@ -281,7 +281,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 =@@ -290,7 +290,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 );@@ -308,7 +308,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); }
drivers/baseband/acc/rte_vrb_pmd.c fetch_acc_config(struct rte_bbdev *dev) · line 327
Quoted 8 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-1@@ -321,7 +321,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 %
This function could be much heavily refactored.
If we look at was is actuallt performed, VRB1 and VRB2 logic is the same, just a couple of value differs (they could be set at probe time).
I might propose something in the future.
Quoted 173 lines
drivers/baseband/acc/rte_vrb_pmd.c +96
-43@@ -543,6 +543,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) {@@ -564,27 +565,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);@@ -599,20 +610,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). @@ -636,19 +687,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);
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id) · line 742
Quoted 8 lines
drivers/baseband/acc/rte_vrb_pmd.c +3
-2@@ -684,8 +737,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 ;
It was not caught initially, but it looks akward to free sw_rings_base, and then set sw_rings to NULL. sw_rings_based should also be set to NULL.
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id) · line 745
Quoted 3 lines
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ }else if (d->device_variant ==VRB2_VARIANT ) { +for (i =0 ; i <=RTE_BBDEV_OP_MLDTS ; i++) +rte_free (d->sw_rings_array[i]);
Same here, you should set sw_rings_array[i] to NULL to avoid double free later.
Quoted 22 lines
drivers/baseband/acc/rte_vrb_pmd.c +4
-9+ }return ret; }@@ -809,17 +867,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 ) {
drivers/baseband/acc/rte_vrb_pmd.c vrb_intr_enable(struct rte_bbdev *dev) · line 884
Quoted 10 lines
drivers/baseband/acc/rte_vrb_pmd.c +10
-0+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 ; + }
Actually, it would be cleanner to perform the free operations systematically, no need to check whether sw_rings_base is NULL.
drivers/baseband/acc/rte_vrb_pmd.c vrb_intr_enable(struct rte_bbdev *dev) · line 886
Quoted 2 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ }else if (d->device_variant ==VRB2_VARIANT ) { +if (d->sw_rings_array[1 ] !=NULL ) {
Why 1 and not 0? And as mentionned above this can be done unconditionnally.
Quoted 59 lines
drivers/baseband/acc/rte_vrb_pmd.c +30
-4+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 );@@ -890,8 +965,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 ;
RE: [PATCH v2 02/10] baseband/acc: queue allocation refactor
Hi Maxime,
Quoted 322 lines
-----Original Message-----
From: Maxime Coquelin <[email protected]>
Sent: Friday, October 4, 2024 5:08 AM
To: Vargas, Hernan <[email protected]>; [email protected]; [email protected]; [email protected]
Cc: Chautru, Nicolas <[email protected]>; Zhang, Qi Z <[email protected]>
Subject: Re: [PATCH v2 02/10] baseband/acc: queue allocation refactorQuoted 24 lines · Hernan Vargas
Refactor to manage queue memory per operation more flexibly for VRB devices.
Signed-off-by Hernan Vargas <[email protected]>--- drivers/baseband/acc/acc_common.h | 5 + drivers/baseband/acc/rte_vrb_pmd.c | 214 ++++++++++++++++++++--------- 2 files changed, 157 insertions(+), 62 deletions(-)drivers/baseband/acc/acc_common.h +2
-0@@ -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 256extern int acc_common_logtype;drivers/baseband/acc/acc_common.h struct acc_device { · line 583
Quoted 1 line
drivers/baseband/acc/acc_common.h +0
-0@@ -581,6 +583,9 @@ struct acc_device {void *sw_rings_base;/* Base addr of un-aligned memory for sw rings */
drivers/baseband/acc/acc_common.h struct acc_device {
Quoted 3 lines
drivers/baseband/acc/acc_common.h +1
-0void *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. */
drivers/baseband/acc/acc_common.h struct acc_device { · line 587
Quoted 1 line
drivers/baseband/acc/acc_common.h +1
-0+rte_iova_t sw_rings_iova_array[ACC_MAX_RING_BUFFER ];/* Array of
sw_ringsIOVA. */drivers/baseband/acc/acc_common.h struct acc_device { · line 588
Quoted 1 line
drivers/baseband/acc/acc_common.h +1
-0+uint32_t queue_index[ACC_MAX_RING_BUFFER ];/* Tracking queue index
Quoted 10 lines
drivers/baseband/acc/acc_common.h +1
-1+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
indexbae01e563826..2c62a5b3e329100644drivers/baseband/acc/rte_vrb_pmd.c fetch_acc_config(struct rte_bbdev *dev) · line 286
Quoted 7 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-1@@ -281,7 +281,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)Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c +0
-0q_top->aq_depth_log2 =@@ -290,7 +290,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;drivers/baseband/acc/rte_vrb_pmd.c fetch_acc_config(struct rte_bbdev *dev) · line 296
Quoted 6 lines
}drivers/baseband/acc/rte_vrb_pmd.c +1
-1- }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 );@@
Quoted 2 lines
-308,7 +308,7 @@
fetch_acc_config(struct rte_bbdev *dev)
idx = (reg2 >> ((qg %
ACC_NUM_QGRPS_PER_WORD) * 4)) &0x7;Quoted 2 lines
else
idx = (reg3 >> ((qg %
ACC_NUM_QGRPS_PER_WORD) * 4)) &0x7;drivers/baseband/acc/rte_vrb_pmd.c fetch_acc_config(struct rte_bbdev *dev) · line 321
Quoted 7 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-1-if (idx <VRB_NUM_ACCS ) { +if (idx <VRB2_NUM_ACCS ) { acc = qman_func_id[idx];updateQtop (acc, qg, acc_conf, d); }@@ -321,7 +321,7 @@ fetch_acc_config(struct rte_bbdev *dev) reg_len2 = acc_reg_read(d, d->reg_addr->depth_log0_offset +8);
drivers/baseband/acc/rte_vrb_pmd.c fetch_acc_config(struct rte_bbdev *dev) · line 322
Quoted 1 line
drivers/baseband/acc/rte_vrb_pmd.c +0
-0reg_len3 = acc_reg_read(d, d->reg_addr->depth_log0_offset +12);
drivers/baseband/acc/rte_vrb_pmd.c fetch_acc_config(struct rte_bbdev *dev)
Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-1-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)Quoted 3 lines
drivers/baseband/acc/rte_vrb_pmd.c +0
-0q_top->aq_depth_log2 = (reg_len0 >> ((q_top-
first_qgroup_index
%This function could be much heavily refactored.
If we look at was is actuallt performed, VRB1 and VRB2 logic is the same, just a couple of value differs (they could be set at probe time).I might propose something in the future.
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t
Quoted 1 line
drivers/baseband/acc/rte_vrb_pmd.c +0
-0@@ -543,6 +543,7 @@ vrb_setup_queues(struct rte_bbdev *dev, uint16_t
num_queues, intsocket_id)Quoted 8 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-0{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) { @@ -564,27 +565,37@@ vrb_setup_queues(struct
rte_bbdev*dev,uint16_tnum_queues, int
socket_id)Quoted 14 lines
return -ENODEV; }drivers/baseband/acc/rte_vrb_pmd.c +4
-6-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. drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 10 lines
drivers/baseband/acc/rte_vrb_pmd.c +5
-4+*/ +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_ringsmemory");
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 4 lines
drivers/baseband/acc/rte_vrb_pmd.c +4
-0+return -ENOMEM ; + } + }else if (d->device_variant ==VRB2_VARIANT ) { + queues_per_op = RTE_MIN(VRB2_MAX_Q_PER_OP,
num_queues);drivers/baseband/acc/rte_vrb_pmd.c
Quoted 2 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+for (i =0 ; i <=RTE_BBDEV_OP_MLDTS ; i++) { + alloc_sw_rings_min_mem(dev, d, queues_per_op,
socket_id);drivers/baseband/acc/rte_vrb_pmd.c
Quoted 2 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+if (d->sw_rings ==NULL ) { + rte_bbdev_log(NOTICE, "Failure allocating
sw_ringsmemory %d", i);drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev,
Quoted 23 lines
drivers/baseband/acc/rte_vrb_pmd.c +10
-7+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);@@ -599,20 +610,60 @@ vrb_setup_queues(struct rte_bbdev *dev,
uint16_tnum_queues, intsocket_id)Quoted 8 lines
drivers/baseband/acc/rte_vrb_pmd.c +0
-5if (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);drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 10 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-8-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.
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c +5
-0+* 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));
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 12 lines
drivers/baseband/acc/rte_vrb_pmd.c +12
-0+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.
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev,
Quoted 51 lines
drivers/baseband/acc/rte_vrb_pmd.c +7
-0+* 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]
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ & ~(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));
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ acc_reg_write(d, d->reg_addr->dma_ring_dl5g_lo, + (uint32_t)(d-sw_rings_iova_array[RTE_BBDEV_OP_LDPC_ENC]
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ & ~(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));
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ acc_reg_write(d, d->reg_addr->dma_ring_ul4g_lo, + (uint32_t)(d-sw_rings_iova_array[RTE_BBDEV_OP_TURBO_DEC]
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ & ~(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));
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ acc_reg_write(d, d->reg_addr->dma_ring_dl4g_lo, + (uint32_t)(d-sw_rings_iova_array[RTE_BBDEV_OP_TURBO_ENC]
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ & ~(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));
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ acc_reg_write(d, d->reg_addr->dma_ring_fft_lo, + (uint32_t)(d-sw_rings_iova_array[RTE_BBDEV_OP_FFT]
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ & ~(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));
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ acc_reg_write(d, d->reg_addr->dma_ring_mld_lo, + (uint32_t)(d-sw_rings_iova_array[RTE_BBDEV_OP_MLDTS]
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ & ~(ACC_SIZE_64MBYTE - 1))); } +/* * Configure Ring Size to the max queue ring size * (used for wrapping purpose). @@ -636,19 +687,21 @@ vrb_setup_queues(struct rte_bbdev *dev,
uint16_tQuoted 20 lines
drivers/baseband/acc/rte_vrb_pmd.c +3
-13num_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);drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 10 lines
drivers/baseband/acc/rte_vrb_pmd.c +10
-0+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);drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, · line 704
Quoted 1 line
drivers/baseband/acc/rte_vrb_pmd.c +1
-0+ acc_reg_write(d, d->reg_addr->tail_ptrs_mld_lo,
phys_low);drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t
Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-0+ } }ret = allocate_info_ring(dev);drivers/baseband/acc/rte_vrb_pmd.c +0
-0@@ -684,8 +737,13 @@ vrb_setup_queues(struct rte_bbdev *dev, uint16_t
num_queues, intsocket_id)drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id) · line 742
Quoted 8 lines
drivers/baseband/acc/rte_vrb_pmd.c +3
-2rte_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 ;It was not caught initially, but it looks akward to free
sw_rings_base, and then setsw_ringsto NULL.sw_rings_basedshould also be set to NULL.
Quoted 6 lines
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id) · line 745
Quoted 3 lines
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ }else if (d->device_variant ==VRB2_VARIANT ) { +for (i =0 ; i <=RTE_BBDEV_OP_MLDTS ; i++) +rte_free (d->sw_rings_array[i]);Same here, you should set sw_rings_array[i] to NULL to avoid double free later.
I believe this is more historical and orthogonal to that commit.
I would prefer to clean that through another serie and apply same BKM consistently, and would not want to rush it. OK with you?
Quoted 28 lines
Quoted 28 lines
drivers/baseband/acc/rte_vrb_pmd.c +9
-9+ }return ret; }@@ -809,17 +867,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);
Quoted 8 lines
drivers/baseband/acc/rte_vrb_pmd.c vrb_intr_enable(struct rte_bbdev *dev) · line 884
Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c +5
-0+ d->tail_ptrs =NULL ; + d->info_ring =NULL ; + d->sw_rings_base =NULL ; + d->harq_layout =NULL ; + }Actually, it would be cleanner to perform the free operations systematically, no need to check whether
sw_rings_baseis NULL.
Noted as above.
Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c vrb_intr_enable(struct rte_bbdev *dev) · line 886
Quoted 3 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ }else if (d->device_variant ==VRB2_VARIANT ) { +if (d->sw_rings_array[1 ] !=NULL ) {Why 1 and not 0? And as mentionned above this can be done unconditionnally.
Thanks. We can check to start from index zero indeed to match the allocation. Arguably as an aside the queue zero is a special one but we can optimize this a bit in the future. Thanks
Quoted 73 lines
drivers/baseband/acc/rte_vrb_pmd.c vrb_queue_setup(struct rte_bbdev *dev, uint16_t
Quoted 16 lines
drivers/baseband/acc/rte_vrb_pmd.c +11
-0+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 );@@ -890,8 +965,16 @@ vrb_queue_setup(struct rte_bbdev *dev, uint16_t
queue_id,drivers/baseband/acc/rte_vrb_pmd.c vrb_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
Quoted 4 lines
drivers/baseband/acc/rte_vrb_pmd.c +0
-1} q->d = d; - q->ring_addr = RTE_PTR_ADD(d->sw_rings, (d->sw_ring_size *
queue_id));drivers/baseband/acc/rte_vrb_pmd.c vrb_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
Quoted 3 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-1- 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));drivers/baseband/acc/rte_vrb_pmd.c vrb_queue_setup(struct rte_bbdev *dev, uint16_t · line 970
Quoted 1 line
drivers/baseband/acc/rte_vrb_pmd.c +1
-0+ q->ring_addr_iova = d->sw_rings_iova + (d->sw_ring_size *
queue_id);drivers/baseband/acc/rte_vrb_pmd.c vrb_dev_info_get(struct rte_bbdev *dev, struct
Quoted 14 lines
drivers/baseband/acc/rte_vrb_pmd.c +7
-0+ }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;drivers/baseband/acc/rte_vrb_pmd.c +0
-0@@ -1347,8 +1430,14 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct
rte_bbdev_driver_info*dev_info)drivers/baseband/acc/rte_vrb_pmd.c vrb_dev_info_get(struct rte_bbdev *dev, struct · line 1436
Quoted 8 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-1dev_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)) {drivers/baseband/acc/rte_vrb_pmd.c vrb_dev_info_get(struct rte_bbdev *dev, struct · line 1437
Quoted 1 line
drivers/baseband/acc/rte_vrb_pmd.c +1
-0+ rte_bbdev_log(ERR, "Unexpected number of queues%d exposed for op %d",
drivers/baseband/acc/rte_vrb_pmd.c vrb_bbdev_init(struct rte_bbdev *dev, struct
Quoted 9 lines
drivers/baseband/acc/rte_vrb_pmd.c +4
-0+ 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 =drivers/baseband/acc/rte_vrb_pmd.c +0
-0@@ -4239,7 +4328,8 @@ vrb_bbdev_init(struct rte_bbdev *dev, struct
rte_pci_driver*drv)Quoted 9 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-1d->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 ;
Re: [PATCH v2 02/10] baseband/acc: queue allocation refactor
Hi Nicolas,
Quoted 331 lines · Nicolas
Hi Maxime,
Quoted 322 lines
-----Original Message-----
From: Maxime Coquelin <[email protected]>
Sent: Friday, October 4, 2024 5:08 AM
To: Vargas, Hernan <[email protected]>; [email protected]; [email protected]; [email protected]
Cc: Chautru, Nicolas <[email protected]>; Zhang, Qi Z <[email protected]>
Subject: Re: [PATCH v2 02/10] baseband/acc: queue allocation refactorQuoted 24 lines · Hernan Vargas
Refactor to manage queue memory per operation more flexibly for VRB devices.
Signed-off-by Hernan Vargas <[email protected]>--- drivers/baseband/acc/acc_common.h | 5 + drivers/baseband/acc/rte_vrb_pmd.c | 214 ++++++++++++++++++++--------- 2 files changed, 157 insertions(+), 62 deletions(-)drivers/baseband/acc/acc_common.h +2
-0@@ -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 256extern int acc_common_logtype;drivers/baseband/acc/acc_common.h struct acc_device { · line 583
Quoted 1 line
drivers/baseband/acc/acc_common.h +0
-0@@ -581,6 +583,9 @@ struct acc_device {void *sw_rings_base;/* Base addr of un-aligned memory for sw rings */
drivers/baseband/acc/acc_common.h struct acc_device {
Quoted 3 lines
drivers/baseband/acc/acc_common.h +1
-0void *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. */
drivers/baseband/acc/acc_common.h struct acc_device { · line 587
Quoted 1 line
drivers/baseband/acc/acc_common.h +1
-0+rte_iova_t sw_rings_iova_array[ACC_MAX_RING_BUFFER ];/* Array of
sw_ringsIOVA. */drivers/baseband/acc/acc_common.h struct acc_device { · line 588
Quoted 1 line
drivers/baseband/acc/acc_common.h +1
-0+uint32_t queue_index[ACC_MAX_RING_BUFFER ];/* Tracking queue index
Quoted 10 lines
drivers/baseband/acc/acc_common.h +1
-1+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
indexbae01e563826..2c62a5b3e329100644drivers/baseband/acc/rte_vrb_pmd.c fetch_acc_config(struct rte_bbdev *dev) · line 286
Quoted 7 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-1@@ -281,7 +281,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)Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c +0
-0q_top->aq_depth_log2 =@@ -290,7 +290,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;drivers/baseband/acc/rte_vrb_pmd.c fetch_acc_config(struct rte_bbdev *dev) · line 296
Quoted 6 lines
}drivers/baseband/acc/rte_vrb_pmd.c +1
-1- }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 );@@
Quoted 2 lines
-308,7 +308,7 @@
fetch_acc_config(struct rte_bbdev *dev)
idx = (reg2 >> ((qg %
ACC_NUM_QGRPS_PER_WORD) * 4)) &0x7;Quoted 2 lines
else idx = (reg3 >> ((qg %
ACC_NUM_QGRPS_PER_WORD) * 4)) &0x7;drivers/baseband/acc/rte_vrb_pmd.c fetch_acc_config(struct rte_bbdev *dev) · line 321
Quoted 7 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-1-if (idx <VRB_NUM_ACCS ) { +if (idx <VRB2_NUM_ACCS ) { acc = qman_func_id[idx];updateQtop (acc, qg, acc_conf, d); }@@ -321,7 +321,7 @@ fetch_acc_config(struct rte_bbdev *dev) reg_len2 = acc_reg_read(d, d->reg_addr->depth_log0_offset +8);
drivers/baseband/acc/rte_vrb_pmd.c fetch_acc_config(struct rte_bbdev *dev) · line 322
Quoted 1 line
drivers/baseband/acc/rte_vrb_pmd.c +0
-0reg_len3 = acc_reg_read(d, d->reg_addr->depth_log0_offset +12);
drivers/baseband/acc/rte_vrb_pmd.c fetch_acc_config(struct rte_bbdev *dev)
Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-1-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)Quoted 3 lines
drivers/baseband/acc/rte_vrb_pmd.c +0
-0q_top->aq_depth_log2 = (reg_len0 >> ((q_top-
first_qgroup_index
%This function could be much heavily refactored.
If we look at was is actuallt performed, VRB1 and VRB2 logic is the same, just a couple of value differs (they could be set at probe time).I might propose something in the future.
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t
Quoted 1 line
drivers/baseband/acc/rte_vrb_pmd.c +0
-0@@ -543,6 +543,7 @@ vrb_setup_queues(struct rte_bbdev *dev, uint16_t
num_queues, intsocket_id)Quoted 8 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-0{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) { @@ -564,27 +565,37@@ vrb_setup_queues(struct
rte_bbdev*dev,uint16_tnum_queues, int
socket_id)Quoted 14 lines
return -ENODEV; }drivers/baseband/acc/rte_vrb_pmd.c +4
-6-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. drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 10 lines
drivers/baseband/acc/rte_vrb_pmd.c +5
-4+*/ +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_ringsmemory");
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 4 lines
drivers/baseband/acc/rte_vrb_pmd.c +4
-0+return -ENOMEM ; + } + }else if (d->device_variant ==VRB2_VARIANT ) { + queues_per_op = RTE_MIN(VRB2_MAX_Q_PER_OP,
num_queues);drivers/baseband/acc/rte_vrb_pmd.c
Quoted 2 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+for (i =0 ; i <=RTE_BBDEV_OP_MLDTS ; i++) { + alloc_sw_rings_min_mem(dev, d, queues_per_op,
socket_id);drivers/baseband/acc/rte_vrb_pmd.c
Quoted 2 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+if (d->sw_rings ==NULL ) { + rte_bbdev_log(NOTICE, "Failure allocating
sw_ringsmemory %d", i);drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev,
Quoted 23 lines
drivers/baseband/acc/rte_vrb_pmd.c +10
-7+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);@@ -599,20 +610,60 @@ vrb_setup_queues(struct rte_bbdev *dev,
uint16_tnum_queues, intsocket_id)Quoted 8 lines
drivers/baseband/acc/rte_vrb_pmd.c +0
-5if (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);drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 10 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-8-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.
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c +5
-0+* 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));
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 12 lines
drivers/baseband/acc/rte_vrb_pmd.c +12
-0+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.
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev,
Quoted 51 lines
drivers/baseband/acc/rte_vrb_pmd.c +7
-0+* 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]
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ & ~(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));
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ acc_reg_write(d, d->reg_addr->dma_ring_dl5g_lo, + (uint32_t)(d-sw_rings_iova_array[RTE_BBDEV_OP_LDPC_ENC]
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ & ~(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));
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ acc_reg_write(d, d->reg_addr->dma_ring_ul4g_lo, + (uint32_t)(d-sw_rings_iova_array[RTE_BBDEV_OP_TURBO_DEC]
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ & ~(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));
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ acc_reg_write(d, d->reg_addr->dma_ring_dl4g_lo, + (uint32_t)(d-sw_rings_iova_array[RTE_BBDEV_OP_TURBO_ENC]
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ & ~(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));
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ acc_reg_write(d, d->reg_addr->dma_ring_fft_lo, + (uint32_t)(d-sw_rings_iova_array[RTE_BBDEV_OP_FFT]
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ & ~(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));
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ acc_reg_write(d, d->reg_addr->dma_ring_mld_lo, + (uint32_t)(d-sw_rings_iova_array[RTE_BBDEV_OP_MLDTS]
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ & ~(ACC_SIZE_64MBYTE - 1))); } +/* * Configure Ring Size to the max queue ring size * (used for wrapping purpose). @@ -636,19 +687,21 @@ vrb_setup_queues(struct rte_bbdev *dev,
uint16_tQuoted 20 lines
drivers/baseband/acc/rte_vrb_pmd.c +3
-13num_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);drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 10 lines
drivers/baseband/acc/rte_vrb_pmd.c +10
-0+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);drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, · line 704
Quoted 1 line
drivers/baseband/acc/rte_vrb_pmd.c +1
-0+ acc_reg_write(d, d->reg_addr->tail_ptrs_mld_lo,
phys_low);drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t
Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-0+ } }ret = allocate_info_ring(dev);drivers/baseband/acc/rte_vrb_pmd.c +0
-0@@ -684,8 +737,13 @@ vrb_setup_queues(struct rte_bbdev *dev, uint16_t
num_queues, intsocket_id)drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id) · line 742
Quoted 8 lines
drivers/baseband/acc/rte_vrb_pmd.c +3
-2rte_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 ;It was not caught initially, but it looks akward to free
sw_rings_base, and then setsw_ringsto NULL.sw_rings_basedshould also be set to NULL.Quoted 6 lines
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id) · line 745
Quoted 3 lines
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ }else if (d->device_variant ==VRB2_VARIANT ) { +for (i =0 ; i <=RTE_BBDEV_OP_MLDTS ; i++) +rte_free (d->sw_rings_array[i]);Same here, you should set sw_rings_array[i] to NULL to avoid double free later.
Quoted 3 lines
I believe this is more historical and orthogonal to that commit.
I would prefer to clean that through another serie and apply same BKM consistently, and would not want to rush it. OK with you?
Sure, please send another series, and then rebase this one on top of it. Or it can be done in some patches at the begining of this series.
Thanks,
Maxime
Quoted 124 lines
Quoted 29 lines
Quoted 29 lines
drivers/baseband/acc/rte_vrb_pmd.c +9
-9+ }return ret; }@@ -809,17 +867,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);Quoted 8 lines
drivers/baseband/acc/rte_vrb_pmd.c vrb_intr_enable(struct rte_bbdev *dev) · line 884
Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c +5
-0+ d->tail_ptrs =NULL ; + d->info_ring =NULL ; + d->sw_rings_base =NULL ; + d->harq_layout =NULL ; + }Actually, it would be cleanner to perform the free operations systematically, no need to check whether
sw_rings_baseis NULL.Noted as above.
Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c vrb_intr_enable(struct rte_bbdev *dev) · line 886
Quoted 3 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ }else if (d->device_variant ==VRB2_VARIANT ) { +if (d->sw_rings_array[1 ] !=NULL ) {Why 1 and not 0? And as mentionned above this can be done unconditionnally.
Thanks. We can check to start from index zero indeed to match the allocation. Arguably as an aside the queue zero is a special one but we can optimize this a bit in the future. Thanks
Quoted 73 lines
drivers/baseband/acc/rte_vrb_pmd.c vrb_queue_setup(struct rte_bbdev *dev, uint16_t
Quoted 16 lines
drivers/baseband/acc/rte_vrb_pmd.c +11
-0+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 );@@ -890,8 +965,16 @@ vrb_queue_setup(struct rte_bbdev *dev, uint16_t
queue_id,drivers/baseband/acc/rte_vrb_pmd.c vrb_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
Quoted 4 lines
drivers/baseband/acc/rte_vrb_pmd.c +0
-1} q->d = d; - q->ring_addr = RTE_PTR_ADD(d->sw_rings, (d->sw_ring_size *
queue_id));drivers/baseband/acc/rte_vrb_pmd.c vrb_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
Quoted 3 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-1- 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));drivers/baseband/acc/rte_vrb_pmd.c vrb_queue_setup(struct rte_bbdev *dev, uint16_t · line 970
Quoted 1 line
drivers/baseband/acc/rte_vrb_pmd.c +1
-0+ q->ring_addr_iova = d->sw_rings_iova + (d->sw_ring_size *
queue_id);drivers/baseband/acc/rte_vrb_pmd.c vrb_dev_info_get(struct rte_bbdev *dev, struct
Quoted 14 lines
drivers/baseband/acc/rte_vrb_pmd.c +7
-0+ }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;drivers/baseband/acc/rte_vrb_pmd.c +0
-0@@ -1347,8 +1430,14 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct
rte_bbdev_driver_info*dev_info)drivers/baseband/acc/rte_vrb_pmd.c vrb_dev_info_get(struct rte_bbdev *dev, struct · line 1436
Quoted 8 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-1dev_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)) {drivers/baseband/acc/rte_vrb_pmd.c vrb_dev_info_get(struct rte_bbdev *dev, struct · line 1437
Quoted 1 line
drivers/baseband/acc/rte_vrb_pmd.c +1
-0+ rte_bbdev_log(ERR, "Unexpected number of queues%d exposed for op %d",
drivers/baseband/acc/rte_vrb_pmd.c vrb_bbdev_init(struct rte_bbdev *dev, struct
Quoted 9 lines
drivers/baseband/acc/rte_vrb_pmd.c +4
-0+ 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 =drivers/baseband/acc/rte_vrb_pmd.c +0
-0@@ -4239,7 +4328,8 @@ vrb_bbdev_init(struct rte_bbdev *dev, struct
rte_pci_driver*drv)Quoted 9 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-1d->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 ;
RE: [PATCH v2 02/10] baseband/acc: queue allocation refactor
Hi Maxime,
Quoted 349 lines
-----Original Message-----
From: Maxime Coquelin <[email protected]>
Sent: Monday, October 7, 2024 2:31 AM
To: Chautru, Nicolas <[email protected]>; Vargas, Hernan <[email protected]>; [email protected]; [email protected]; [email protected]
Cc: Zhang, Qi Z <[email protected]>
Subject: Re: [PATCH v2 02/10] baseband/acc: queue allocation refactorHi Nicolas,
Quoted 21 lines · Nicolas
Hi Maxime,
Quoted 19 lines
-----Original Message-----
From: Maxime Coquelin <[email protected]>
Sent: Friday, October 4, 2024 5:08 AM
To: Vargas, Hernan <[email protected]>; [email protected]; [email protected]; [email protected]
Cc: Chautru, Nicolas <[email protected]>; Zhang, Qi Z <[email protected]>
Subject: Re: [PATCH v2 02/10] baseband/acc: queue allocation refactorQuoted 7 lines · Hernan Vargas
Refactor to manage queue memory per operation more flexibly for VRB devices.
Signed-off-by Hernan Vargas <[email protected]>--- drivers/baseband/acc/acc_common.h | 5 + drivers/baseband/acc/rte_vrb_pmd.c | 214 ++++++++++++++++++++---------
Quoted 84 lines
Quoted 84 lines
Quoted 18 lines
2 files changed, 157 insertions(+), 62 deletions(-)drivers/baseband/acc/acc_common.h +2
-0@@ -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 256extern int acc_common_logtype;@@ -581,6 +583,9 @@ struct acc_device {void *sw_rings_base;/* Base addr of un-aligned memory for sw Quoted 1 line
rings
*/
drivers/baseband/acc/acc_common.h
Quoted 3 lines
void
*sw_rings; /* 64MBs of 64MB aligned memory for sw rings */rte_iova_tsw_rings_iova; /* IOVA address ofsw_rings*/drivers/baseband/acc/acc_common.h +1
-0+void *sw_rings_array[ACC_MAX_RING_BUFFER ];/* Array of aligned memory for sw rings. */
drivers/baseband/acc/acc_common.h
Quoted 1 line
drivers/baseband/acc/acc_common.h +1
-0+rte_iova_t sw_rings_iova_array[ACC_MAX_RING_BUFFER ];/* Array of
sw_ringsIOVA. */drivers/baseband/acc/acc_common.h
Quoted 1 line
drivers/baseband/acc/acc_common.h +1
-0+uint32_t queue_index[ACC_MAX_RING_BUFFER ];/* Tracking queue index
Quoted 10 lines
drivers/baseband/acc/acc_common.h +1
-0+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 drivers/baseband/acc/rte_vrb_pmd.c +0
-0drivers/baseband/acc/rte_vrb_pmd.c fetch_acc_config(struct rte_bbdev *dev) · line 286
Quoted 7 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-1@@ -281,7 +281,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)Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c +0
-0q_top->aq_depth_log2 =@@ -290,7 +290,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;drivers/baseband/acc/rte_vrb_pmd.c fetch_acc_config(struct rte_bbdev *dev) · line 296
Quoted 6 lines
}drivers/baseband/acc/rte_vrb_pmd.c +1
-1- }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 );@@
Quoted 2 lines
-308,7 +308,7 @@
fetch_acc_config(struct rte_bbdev *dev)
idx = (reg2 >> ((qg %
ACC_NUM_QGRPS_PER_WORD) * 4)) &0x7;Quoted 2 lines
else idx = (reg3 >> ((qg %
ACC_NUM_QGRPS_PER_WORD) * 4)) &0x7;drivers/baseband/acc/rte_vrb_pmd.c fetch_acc_config(struct rte_bbdev *dev) · line 321
Quoted 7 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-1-if (idx <VRB_NUM_ACCS ) { +if (idx <VRB2_NUM_ACCS ) { acc = qman_func_id[idx];updateQtop (acc, qg, acc_conf, d); }@@ -321,7 +321,7 @@ fetch_acc_config(struct rte_bbdev *dev) reg_len2 = acc_reg_read(d, d->reg_addr->depth_log0_offset +8);
drivers/baseband/acc/rte_vrb_pmd.c fetch_acc_config(struct rte_bbdev *dev) · line 322
Quoted 1 line
drivers/baseband/acc/rte_vrb_pmd.c +0
-0reg_len3 = acc_reg_read(d, d->reg_addr->depth_log0_offset +12);
drivers/baseband/acc/rte_vrb_pmd.c fetch_acc_config(struct rte_bbdev *dev)
Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-1-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)Quoted 2 lines
drivers/baseband/acc/rte_vrb_pmd.c +0
-0q_top->aq_depth_log2 = (reg_len0 >> ((q_top-
first_qgroup_index%Quoted 7 lines
Quoted 7 lines
This function could be much heavily refactored.
If we look at was is actuallt performed, VRB1 and VRB2 logic is the same, just a couple of value differs (they could be set at probe time).I might propose something in the future.
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev,
Quoted 1 line
drivers/baseband/acc/rte_vrb_pmd.c +0
-0@@ -543,6 +543,7 @@ vrb_setup_queues(struct rte_bbdev *dev,
uint16_tQuoted 221 lines
Quoted 214 lines
num_queues, intsocket_id)Quoted 9 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-0{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) { @@ -564,27 +565,37 @@ vrb_setup_queues(struct rte_bbdev *dev, uint16_t
num_queues, int
socket_id)Quoted 14 lines
return -ENODEV; }drivers/baseband/acc/rte_vrb_pmd.c +4
-6-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. drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 10 lines
drivers/baseband/acc/rte_vrb_pmd.c +5
-4+*/ +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_ringsmemory");
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 4 lines
drivers/baseband/acc/rte_vrb_pmd.c +4
-0+return -ENOMEM ; + } + }else if (d->device_variant ==VRB2_VARIANT ) { + queues_per_op = RTE_MIN(VRB2_MAX_Q_PER_OP,
num_queues);drivers/baseband/acc/rte_vrb_pmd.c
Quoted 2 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+for (i =0 ; i <=RTE_BBDEV_OP_MLDTS ; i++) { + alloc_sw_rings_min_mem(dev, d, queues_per_op,
socket_id);drivers/baseband/acc/rte_vrb_pmd.c
Quoted 2 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+if (d->sw_rings ==NULL ) { + rte_bbdev_log(NOTICE, "Failure allocating
sw_ringsmemory %d", i);drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev,
Quoted 23 lines
drivers/baseband/acc/rte_vrb_pmd.c +10
-7+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);@@ -599,20 +610,60 @@ vrb_setup_queues(struct rte_bbdev *dev,
uint16_tnum_queues, intsocket_id)Quoted 8 lines
drivers/baseband/acc/rte_vrb_pmd.c +0
-5if (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);drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 10 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-8-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.
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c +5
-0+* 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));
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 12 lines
drivers/baseband/acc/rte_vrb_pmd.c +12
-0+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.
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev,
Quoted 51 lines
drivers/baseband/acc/rte_vrb_pmd.c +7
-0+* 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]
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ & ~(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));
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ acc_reg_write(d, d->reg_addr->dma_ring_dl5g_lo, + (uint32_t)(d-sw_rings_iova_array[RTE_BBDEV_OP_LDPC_ENC]
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ & ~(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));
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ acc_reg_write(d, d->reg_addr->dma_ring_ul4g_lo, + (uint32_t)(d-sw_rings_iova_array[RTE_BBDEV_OP_TURBO_DEC]
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ & ~(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));
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ acc_reg_write(d, d->reg_addr->dma_ring_dl4g_lo, + (uint32_t)(d-sw_rings_iova_array[RTE_BBDEV_OP_TURBO_ENC]
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ & ~(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));
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ acc_reg_write(d, d->reg_addr->dma_ring_fft_lo, + (uint32_t)(d-sw_rings_iova_array[RTE_BBDEV_OP_FFT]
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ & ~(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));
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ acc_reg_write(d, d->reg_addr->dma_ring_mld_lo, + (uint32_t)(d-sw_rings_iova_array[RTE_BBDEV_OP_MLDTS]
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ & ~(ACC_SIZE_64MBYTE - 1))); } +/* * Configure Ring Size to the max queue ring size * (used for wrapping purpose). @@ -636,19 +687,21 @@ vrb_setup_queues(struct rte_bbdev *dev,
uint16_tQuoted 20 lines
drivers/baseband/acc/rte_vrb_pmd.c +3
-13num_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);drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 10 lines
drivers/baseband/acc/rte_vrb_pmd.c +10
-0+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);drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, · line 704
Quoted 1 line
drivers/baseband/acc/rte_vrb_pmd.c +1
-0+ acc_reg_write(d, d->reg_addr->tail_ptrs_mld_lo,
phys_low);Quoted 6 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-0+ } }ret = allocate_info_ring(dev);drivers/baseband/acc/rte_vrb_pmd.c +0
-0@@ -684,8 +737,13 @@ vrb_setup_queues(struct rte_bbdev *dev,
uint16_t
num_queues, intsocket_id)drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id) · line 742
Quoted 8 lines
rte_free(d->tail_ptrs); d->tail_ptrs = NULL; free_sw_rings:drivers/baseband/acc/rte_vrb_pmd.c +3
-2-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 ;It was not caught initially, but it looks akward to free
sw_rings_base, and then setsw_ringsto NULL.sw_rings_basedshould also be set to NULL.Quoted 6 lines
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id) · line 745
Quoted 3 lines
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ }else if (d->device_variant ==VRB2_VARIANT ) { +for (i =0 ; i <=RTE_BBDEV_OP_MLDTS ; i++) +rte_free (d->sw_rings_array[i]);Same here, you should set sw_rings_array[i] to NULL to avoid double free later.
Quoted 2 lines
I believe this is more historical and orthogonal to that commit. I would prefer to clean that through another serie and apply same BKM
Quoted 8 lines
consistently, and would not want to rush it.
Quoted 1 line
OK with you?
Sure, please send another series, and then rebase this one on top of it. Or it can be done in some patches at the begining of this series.
Thanks,
Maxime
I believe this will take more time to get this done.
I would rather we do the opposite and get this current series applied first.
Then the historical improvement that you spotted in parallel of this review can be managed separately no its own merit, and possibly be applied in 24.03 realistically. I would rather not introduce new changes in which would have not been through thorough validation.
Quoted 129 lines
Quoted 39 lines
Quoted 30 lines
Quoted 30 lines
drivers/baseband/acc/rte_vrb_pmd.c +9
-9+ }return ret; }@@ -809,17 +867,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);Quoted 8 lines
drivers/baseband/acc/rte_vrb_pmd.c vrb_intr_enable(struct rte_bbdev *dev) · line 884
Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c +5
-0+ d->tail_ptrs =NULL ; + d->info_ring =NULL ; + d->sw_rings_base =NULL ; + d->harq_layout =NULL ; + }Actually, it would be cleanner to perform the free operations systematically, no need to check whether
sw_rings_baseis NULL.Quoted 7 lines
Noted as above.
Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c vrb_intr_enable(struct rte_bbdev *dev) · line 886
Quoted 3 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ }else if (d->device_variant ==VRB2_VARIANT ) { +if (d->sw_rings_array[1 ] !=NULL ) {Why 1 and not 0? And as mentionned above this can be done
unconditionnally.
Quoted 2 lines
Thanks. We can check to start from index zero indeed to match the
allocation.
Quoted 1 line
Arguably as an aside the queue zero is a special one but we can optimize this
a bit in the future.
Quoted 18 lines
Thanks
Quoted 16 lines
drivers/baseband/acc/rte_vrb_pmd.c vrb_queue_setup(struct rte_bbdev *dev,
Quoted 16 lines
drivers/baseband/acc/rte_vrb_pmd.c +11
-0+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 );@@ -890,8 +965,16 @@ vrb_queue_setup(struct rte_bbdev *dev,
uint16_tQuoted 58 lines
Quoted 57 lines
queue_id,drivers/baseband/acc/rte_vrb_pmd.c vrb_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
Quoted 4 lines
drivers/baseband/acc/rte_vrb_pmd.c +0
-1} q->d = d; - q->ring_addr = RTE_PTR_ADD(d->sw_rings, (d->sw_ring_size *
queue_id));drivers/baseband/acc/rte_vrb_pmd.c vrb_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
Quoted 3 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-1- 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));drivers/baseband/acc/rte_vrb_pmd.c vrb_queue_setup(struct rte_bbdev *dev, · line 970
Quoted 1 line
drivers/baseband/acc/rte_vrb_pmd.c +1
-0+ q->ring_addr_iova = d->sw_rings_iova + (d->sw_ring_size *
queue_id);Quoted 11 lines
drivers/baseband/acc/rte_vrb_pmd.c +7
-0+ }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]++; + }Quoted 3 lines
/* 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)drivers/baseband/acc/rte_vrb_pmd.c vrb_dev_info_get(struct rte_bbdev *dev, struct rte_bbdev_driver_info *dev_info)
Quoted 8 lines
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;drivers/baseband/acc/rte_vrb_pmd.c +2
-1-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)) {drivers/baseband/acc/rte_vrb_pmd.c
Quoted 1 line
drivers/baseband/acc/rte_vrb_pmd.c +1
-0+ rte_bbdev_log(ERR, "Unexpected number of queues%d exposed for op %d",
Quoted 9 lines
drivers/baseband/acc/rte_vrb_pmd.c +4
-0+ 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)Quoted 9 lines
d->reg_addr = &vrb1_pf_reg_addr; else d->reg_addr = &vrb1_vf_reg_addr;drivers/baseband/acc/rte_vrb_pmd.c +2
-1- }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 ;
Re: [PATCH v2 02/10] baseband/acc: queue allocation refactor
Hi Nicolas,
Quoted 357 lines · Nicolas
Hi Maxime,
Quoted 355 lines
-----Original Message-----
From: Maxime Coquelin <[email protected]>
Sent: Monday, October 7, 2024 2:31 AM
To: Chautru, Nicolas <[email protected]>; Vargas, Hernan <[email protected]>; [email protected]; [email protected]; [email protected]
Cc: Zhang, Qi Z <[email protected]>
Subject: Re: [PATCH v2 02/10] baseband/acc: queue allocation refactorHi Nicolas,
Quoted 21 lines · Nicolas
Hi Maxime,
Quoted 19 lines
-----Original Message-----
From: Maxime Coquelin <[email protected]>
Sent: Friday, October 4, 2024 5:08 AM
To: Vargas, Hernan <[email protected]>; [email protected]; [email protected]; [email protected]
Cc: Chautru, Nicolas <[email protected]>; Zhang, Qi Z <[email protected]>
Subject: Re: [PATCH v2 02/10] baseband/acc: queue allocation refactorQuoted 7 lines · Hernan Vargas
Refactor to manage queue memory per operation more flexibly for VRB devices.
Signed-off-by Hernan Vargas <[email protected]>--- drivers/baseband/acc/acc_common.h | 5 + drivers/baseband/acc/rte_vrb_pmd.c | 214 ++++++++++++++++++++---------
Quoted 84 lines
Quoted 84 lines
Quoted 18 lines
2 files changed, 157 insertions(+), 62 deletions(-)drivers/baseband/acc/acc_common.h +2
-0@@ -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 256extern int acc_common_logtype;@@ -581,6 +583,9 @@ struct acc_device {void *sw_rings_base;/* Base addr of un-aligned memory for sw Quoted 1 line
rings
*/
drivers/baseband/acc/acc_common.h
Quoted 3 lines
void
*sw_rings; /* 64MBs of 64MB aligned memory for sw rings */rte_iova_tsw_rings_iova; /* IOVA address ofsw_rings*/drivers/baseband/acc/acc_common.h +1
-0+void *sw_rings_array[ACC_MAX_RING_BUFFER ];/* Array of aligned memory for sw rings. */
drivers/baseband/acc/acc_common.h
Quoted 1 line
drivers/baseband/acc/acc_common.h +1
-0+rte_iova_t sw_rings_iova_array[ACC_MAX_RING_BUFFER ];/* Array of
sw_ringsIOVA. */drivers/baseband/acc/acc_common.h
Quoted 1 line
drivers/baseband/acc/acc_common.h +1
-0+uint32_t queue_index[ACC_MAX_RING_BUFFER ];/* Tracking queue index
Quoted 10 lines
drivers/baseband/acc/acc_common.h +1
-0+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 drivers/baseband/acc/rte_vrb_pmd.c +0
-0drivers/baseband/acc/rte_vrb_pmd.c fetch_acc_config(struct rte_bbdev *dev) · line 286
Quoted 7 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-1@@ -281,7 +281,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)Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c +0
-0q_top->aq_depth_log2 =@@ -290,7 +290,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;drivers/baseband/acc/rte_vrb_pmd.c fetch_acc_config(struct rte_bbdev *dev) · line 296
Quoted 6 lines
}drivers/baseband/acc/rte_vrb_pmd.c +1
-1- }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 );@@
Quoted 2 lines
-308,7 +308,7 @@
fetch_acc_config(struct rte_bbdev *dev)idx = (reg2 >> ((qg %
ACC_NUM_QGRPS_PER_WORD) * 4)) &0x7;Quoted 2 lines
else idx = (reg3 >> ((qg %
ACC_NUM_QGRPS_PER_WORD) * 4)) &0x7;drivers/baseband/acc/rte_vrb_pmd.c fetch_acc_config(struct rte_bbdev *dev) · line 321
Quoted 7 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-1-if (idx <VRB_NUM_ACCS ) { +if (idx <VRB2_NUM_ACCS ) { acc = qman_func_id[idx];updateQtop (acc, qg, acc_conf, d); }@@ -321,7 +321,7 @@ fetch_acc_config(struct rte_bbdev *dev) reg_len2 = acc_reg_read(d, d->reg_addr->depth_log0_offset +8);
drivers/baseband/acc/rte_vrb_pmd.c fetch_acc_config(struct rte_bbdev *dev) · line 322
Quoted 1 line
drivers/baseband/acc/rte_vrb_pmd.c +0
-0reg_len3 = acc_reg_read(d, d->reg_addr->depth_log0_offset +12);
drivers/baseband/acc/rte_vrb_pmd.c fetch_acc_config(struct rte_bbdev *dev)
Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-1-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)Quoted 2 lines
drivers/baseband/acc/rte_vrb_pmd.c +0
-0q_top->aq_depth_log2 = (reg_len0 >> ((q_top-
first_qgroup_index%Quoted 7 lines
Quoted 7 lines
This function could be much heavily refactored.
If we look at was is actuallt performed, VRB1 and VRB2 logic is the same, just a couple of value differs (they could be set at probe time).I might propose something in the future.
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev,
Quoted 1 line
drivers/baseband/acc/rte_vrb_pmd.c +0
-0@@ -543,6 +543,7 @@ vrb_setup_queues(struct rte_bbdev *dev,
uint16_tQuoted 221 lines
Quoted 214 lines
num_queues, intsocket_id)Quoted 9 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-0{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) { @@ -564,27 +565,37 @@ vrb_setup_queues(struct rte_bbdev *dev, uint16_t
num_queues, int
socket_id)Quoted 14 lines
return -ENODEV; }drivers/baseband/acc/rte_vrb_pmd.c +4
-6-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. drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 10 lines
drivers/baseband/acc/rte_vrb_pmd.c +5
-4+*/ +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_ringsmemory");
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 4 lines
drivers/baseband/acc/rte_vrb_pmd.c +4
-0+return -ENOMEM ; + } + }else if (d->device_variant ==VRB2_VARIANT ) { + queues_per_op = RTE_MIN(VRB2_MAX_Q_PER_OP,
num_queues);drivers/baseband/acc/rte_vrb_pmd.c
Quoted 2 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+for (i =0 ; i <=RTE_BBDEV_OP_MLDTS ; i++) { + alloc_sw_rings_min_mem(dev, d, queues_per_op,
socket_id);drivers/baseband/acc/rte_vrb_pmd.c
Quoted 2 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+if (d->sw_rings ==NULL ) { + rte_bbdev_log(NOTICE, "Failure allocating
sw_ringsmemory %d", i);drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev,
Quoted 23 lines
drivers/baseband/acc/rte_vrb_pmd.c +10
-7+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);@@ -599,20 +610,60 @@ vrb_setup_queues(struct rte_bbdev *dev,
uint16_tnum_queues, intsocket_id)Quoted 8 lines
drivers/baseband/acc/rte_vrb_pmd.c +0
-5if (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);drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 10 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-8-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.
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c +5
-0+* 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));
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 12 lines
drivers/baseband/acc/rte_vrb_pmd.c +12
-0+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.
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev,
Quoted 51 lines
drivers/baseband/acc/rte_vrb_pmd.c +7
-0+* 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]
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ & ~(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));
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ acc_reg_write(d, d->reg_addr->dma_ring_dl5g_lo, + (uint32_t)(d-sw_rings_iova_array[RTE_BBDEV_OP_LDPC_ENC]
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ & ~(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));
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ acc_reg_write(d, d->reg_addr->dma_ring_ul4g_lo, + (uint32_t)(d-sw_rings_iova_array[RTE_BBDEV_OP_TURBO_DEC]
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ & ~(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));
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ acc_reg_write(d, d->reg_addr->dma_ring_dl4g_lo, + (uint32_t)(d-sw_rings_iova_array[RTE_BBDEV_OP_TURBO_ENC]
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ & ~(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));
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ acc_reg_write(d, d->reg_addr->dma_ring_fft_lo, + (uint32_t)(d-sw_rings_iova_array[RTE_BBDEV_OP_FFT]
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ & ~(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));
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ acc_reg_write(d, d->reg_addr->dma_ring_mld_lo, + (uint32_t)(d-sw_rings_iova_array[RTE_BBDEV_OP_MLDTS]
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ & ~(ACC_SIZE_64MBYTE - 1))); } +/* * Configure Ring Size to the max queue ring size * (used for wrapping purpose). @@ -636,19 +687,21 @@ vrb_setup_queues(struct rte_bbdev *dev,
uint16_tQuoted 20 lines
drivers/baseband/acc/rte_vrb_pmd.c +3
-13num_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);drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
Quoted 10 lines
drivers/baseband/acc/rte_vrb_pmd.c +10
-0+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);drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, · line 704
Quoted 1 line
drivers/baseband/acc/rte_vrb_pmd.c +1
-0+ acc_reg_write(d, d->reg_addr->tail_ptrs_mld_lo,
phys_low);Quoted 6 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-0+ } }ret = allocate_info_ring(dev);drivers/baseband/acc/rte_vrb_pmd.c +0
-0@@ -684,8 +737,13 @@ vrb_setup_queues(struct rte_bbdev *dev,
uint16_t
num_queues, intsocket_id)drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id) · line 742
Quoted 8 lines
rte_free(d->tail_ptrs); d->tail_ptrs = NULL; free_sw_rings:drivers/baseband/acc/rte_vrb_pmd.c +3
-2-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 ;It was not caught initially, but it looks akward to free
sw_rings_base, and then setsw_ringsto NULL.sw_rings_basedshould also be set to NULL.Quoted 6 lines
drivers/baseband/acc/rte_vrb_pmd.c vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id) · line 745
Quoted 3 lines
drivers/baseband/acc/rte_vrb_pmd.c +3
-0+ }else if (d->device_variant ==VRB2_VARIANT ) { +for (i =0 ; i <=RTE_BBDEV_OP_MLDTS ; i++) +rte_free (d->sw_rings_array[i]);Same here, you should set sw_rings_array[i] to NULL to avoid double free later.
Quoted 2 lines
I believe this is more historical and orthogonal to that commit. I would prefer to clean that through another serie and apply same BKM
consistently, and would not want to rush it.
Quoted 1 line
OK with you?
Sure, please send another series, and then rebase this one on top of it. Or it can be done in some patches at the begining of this series.
Quoted 7 lines
Quoted 2 lines
Thanks,
MaximeI believe this will take more time to get this done.
I would rather we do the opposite and get this current series applied first.
Then the historical improvement that you spotted in parallel of this review can be managed separately no its own merit, and possibly be applied in 24.03 realistically. I would rather not introduce new changes in which would have not been through thorough validation.
Validation for v24.11 has not started yet, so we have time to make changes. Especially here, as the change I request is only to ensure we avoid potential double frees.
I'll continue the review of the series, but I won't merge it until this is fixed.
Thanks,
Maxime
Quoted 130 lines
Quoted 129 lines
Quoted 39 lines
Quoted 30 lines
Quoted 30 lines
drivers/baseband/acc/rte_vrb_pmd.c +9
-9+ }return ret; }@@ -809,17 +867,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);Quoted 8 lines
drivers/baseband/acc/rte_vrb_pmd.c vrb_intr_enable(struct rte_bbdev *dev) · line 884
Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c +5
-0+ d->tail_ptrs =NULL ; + d->info_ring =NULL ; + d->sw_rings_base =NULL ; + d->harq_layout =NULL ; + }Actually, it would be cleanner to perform the free operations systematically, no need to check whether
sw_rings_baseis NULL.Quoted 7 lines
Noted as above.
Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c vrb_intr_enable(struct rte_bbdev *dev) · line 886
Quoted 3 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+ }else if (d->device_variant ==VRB2_VARIANT ) { +if (d->sw_rings_array[1 ] !=NULL ) {Why 1 and not 0? And as mentionned above this can be done
unconditionnally.
Quoted 2 lines
Thanks. We can check to start from index zero indeed to match the
allocation.
Quoted 1 line
Arguably as an aside the queue zero is a special one but we can optimize this
a bit in the future.
Quoted 18 lines
Thanks
Quoted 16 lines
drivers/baseband/acc/rte_vrb_pmd.c vrb_queue_setup(struct rte_bbdev *dev,
Quoted 16 lines
drivers/baseband/acc/rte_vrb_pmd.c +11
-0+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 );@@ -890,8 +965,16 @@ vrb_queue_setup(struct rte_bbdev *dev,
uint16_tQuoted 58 lines
Quoted 57 lines
queue_id,drivers/baseband/acc/rte_vrb_pmd.c vrb_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
Quoted 4 lines
drivers/baseband/acc/rte_vrb_pmd.c +0
-1} q->d = d; - q->ring_addr = RTE_PTR_ADD(d->sw_rings, (d->sw_ring_size *
queue_id));drivers/baseband/acc/rte_vrb_pmd.c vrb_queue_setup(struct rte_bbdev *dev, uint16_t queue_id,
Quoted 3 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-1- 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));drivers/baseband/acc/rte_vrb_pmd.c vrb_queue_setup(struct rte_bbdev *dev, · line 970
Quoted 1 line
drivers/baseband/acc/rte_vrb_pmd.c +1
-0+ q->ring_addr_iova = d->sw_rings_iova + (d->sw_ring_size *
queue_id);Quoted 11 lines
drivers/baseband/acc/rte_vrb_pmd.c +7
-0+ }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]++; + }Quoted 3 lines
/* 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)drivers/baseband/acc/rte_vrb_pmd.c vrb_dev_info_get(struct rte_bbdev *dev, struct rte_bbdev_driver_info *dev_info)
Quoted 8 lines
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;drivers/baseband/acc/rte_vrb_pmd.c +2
-1-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)) {drivers/baseband/acc/rte_vrb_pmd.c
Quoted 1 line
drivers/baseband/acc/rte_vrb_pmd.c +1
-0+ rte_bbdev_log(ERR, "Unexpected number of queues%d exposed for op %d",
Quoted 9 lines
drivers/baseband/acc/rte_vrb_pmd.c +4
-0+ 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)Quoted 9 lines
d->reg_addr = &vrb1_pf_reg_addr; else d->reg_addr = &vrb1_vf_reg_addr;drivers/baseband/acc/rte_vrb_pmd.c +2
-1- }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 ;
PATCH v2 3/10 baseband/acc: configure max queues per device
In reply to Hernan VargasConfigure max_queues based on the number of queue groups and numbers of AQS per device variant.
--- drivers/baseband/acc/rte_vrb_pmd.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/baseband/acc/rte_vrb_pmd.c +7 -2
@@ -544,7 +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; +int ret, max_queues =0 ;if (d->pf_device && !d->acc_conf.pf_mode_en) { rte_bbdev_log(NOTICE,
@@ -671,10 +671,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",
Signature · 1 line
2.37.1
Re: [PATCH v2 03/10] baseband/acc: configure max queues per device
Quoted 8 lines · Hernan Vargas
Configure
max_queuesbased on the number of queue groups and numbers of AQS per device variant.Signed-off-by Hernan Vargas <[email protected]>--- drivers/baseband/acc/rte_vrb_pmd.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
Thanks,
Maxime
PATCH v2 4/10 baseband/acc: future proof structure comparison
In reply to Hernan VargasSome 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.
--- drivers/baseband/acc/acc_common.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/baseband/acc/acc_common.h +2 -2
@@ -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
Signature · 1 line
2.37.1
Re: [PATCH v2 04/10] baseband/acc: future proof structure comparison
Quoted 9 lines · 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 <[email protected]>--- drivers/baseband/acc/acc_common.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Thanks,
Maxime
PATCH v2 5/10 baseband/acc: enhance SW ring alignment
In reply to Hernan VargasCalculate the aligned total size required for queue rings, ensuring that the size is a power of two for proper memory allocation.
--- drivers/baseband/acc/acc_common.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/baseband/acc/acc_common.h +4 -3
@@ -767,19 +767,20 @@ 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; -/* Free first in case this is a reconfiguration */ +uint32_t alignment = q_sw_ring_size *rte_align32pow2 (num_queues); +/* Free first in case this is dev_sw_ring_size, q_sw_ring_size, socket); reconfiguration */ rte_free (d->sw_rings_base);/* Find an aligned block of memory to store sw rings */ 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 +* 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,
Signature · 1 line
2.37.1
Re: [PATCH v2 05/10] baseband/acc: enhance SW ring alignment
Quoted 13 lines · 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 <[email protected]>--- drivers/baseband/acc/acc_common.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)drivers/baseband/acc/acc_common.h +0
-0
drivers/baseband/acc/acc_common.h alloc_sw_rings_min_mem(struct rte_bbdev *dev, struct acc_device *d, · line 771
Quoted 6 lines
drivers/baseband/acc/acc_common.h +2
-1@@ -767,19 +767,20 @@ 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; -/* Free first in case this is a reconfiguration */ +uint32_t alignment = q_sw_ring_size *rte_align32pow2 (num_queues); +/* Free first in case this is dev_sw_ring_size, q_sw_ring_size, socket); reconfiguration */
There is a copy/paste mistake in the comment?
drivers/baseband/acc/acc_common.h alloc_sw_rings_min_mem(struct rte_bbdev *dev, struct acc_device *d, · line 778
Quoted 8 lines
drivers/baseband/acc/acc_common.h +1
-1rte_free (d->sw_rings_base);/* Find an aligned block of memory to store sw rings */ 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 +* alignment at the condition that the requested size is
This comment is really unclear "aligned to alignment"
Quoted 9 lines
drivers/baseband/acc/acc_common.h +1
-1* 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,
RE: [PATCH v2 05/10] baseband/acc: enhance SW ring alignment
Hi Maxime,
Quoted 30 lines
-----Original Message-----
From: Maxime Coquelin <[email protected]>
Sent: Tuesday, October 8, 2024 12:52 AM
To: Vargas, Hernan <[email protected]>; [email protected]; [email protected]; [email protected]
Cc: Chautru, Nicolas <[email protected]>; Zhang, Qi Z <[email protected]>
Subject: Re: [PATCH v2 05/10] baseband/acc: enhance SW ring alignmentdrivers/baseband/acc/acc_common.h alloc_sw_rings_min_mem(struct rte_bbdev
Quoted 14 lines · 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 <[email protected]>--- drivers/baseband/acc/acc_common.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)drivers/baseband/acc/acc_common.h +0
-0@@ -767,19 +767,20 @@ alloc_sw_rings_min_mem(struct rte_bbdev*dev, struct
acc_device*d,drivers/baseband/acc/acc_common.h alloc_sw_rings_min_mem(struct rte_bbdev · line 768
Quoted 2 lines
drivers/baseband/acc/acc_common.h +0
-0int i =0 ; uint32_t q_sw_ring_size = ACC_MAX_QUEUE_DEPTH *
get_desc_len();
Quoted 8 lines
drivers/baseband/acc/acc_common.h alloc_sw_rings_min_mem(struct rte_bbdev *dev, struct acc_device *d,
Quoted 3 lines
drivers/baseband/acc/acc_common.h +1
-1uint32_t dev_sw_ring_size = q_sw_ring_size * num_queues; -/* Free first in case this is a reconfiguration */ + uint32_t alignment = q_sw_ring_size *
rte_align32pow2(num_queues);drivers/baseband/acc/acc_common.h alloc_sw_rings_min_mem(struct rte_bbdev · line 772
Quoted 2 lines
drivers/baseband/acc/acc_common.h +2
-0+/* Free first in case this is dev_sw_ring_size, q_sw_ring_size, +socket); reconfiguration */ There is a copy/paste mistake in the comment?
Thanks, yes. We missed it in the rebase somehow.
Quoted 11 lines
drivers/baseband/acc/acc_common.h alloc_sw_rings_min_mem(struct rte_bbdev *dev, struct acc_device *d, · line 778
Quoted 9 lines
drivers/baseband/acc/acc_common.h +1
-1rte_free (d->sw_rings_base);/* Find an aligned block of memory to store sw rings */ 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 +* alignment at the condition that the requested size is This comment is really unclear "aligned to alignment"
Unclear indeed when reading it again. Should be "aligned to the variable alignment ..." Ie the change is purely to use now the new variable alignment instead of queue_ring_size Thanks
Quoted 10 lines
Quoted 10 lines
drivers/baseband/acc/acc_common.h +1
-1* 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,
PATCH v2 6/10 baseband/acc: remove soft output bypass
In reply to Hernan VargasRemoving soft output bypass capability due to device limitations.
--- drivers/baseband/acc/rte_vrb_pmd.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/baseband/acc/rte_vrb_pmd.c +3 -4
@@ -1359,7 +1359,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,
@@ -1736,18 +1735,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;
Signature · 1 line
2.37.1
Re: [PATCH v2 06/10] baseband/acc: remove soft output bypass
Quoted 1 line · Hernan Vargas
Removing soft output bypass capability due to device limitations.
It should be specified this is for VRB2 device variant.
And this should be backported, so pass Fixes tag and cc stable as it was introduced in v23.11 LTS.
Thanks,
Maxime
Quoted 40 lines
Signed-off-by Hernan Vargas <[email protected]>--- drivers/baseband/acc/rte_vrb_pmd.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)drivers/baseband/acc/rte_vrb_pmd.c +3
-4@@ -1359,7 +1359,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,@@ -1736,18 +1735,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;
RE: [PATCH v2 06/10] baseband/acc: remove soft output bypass
Hi Maxime,
There is no formal support for VRB2 yet, this is ongoing and we will formally enable this only in DPDK 24.11.
So we don’t believe this commit below is truly a fix required to be applied on stable branch.
Ie. DPDK 23.11 will not support VRB2 anyway.
Still Hernan will put the fix tag that you requested in the v3 for now, but feel free to remove that tag if you agree this is actually not required. Up to you, best probably best to only apply true fixes on LTS release.
Thanks
Nic
Quoted 72 lines
-----Original Message-----
From: Maxime Coquelin <[email protected]>
Sent: Tuesday, October 8, 2024 1:00 AM
To: Vargas, Hernan <[email protected]>; [email protected]; [email protected]; [email protected]
Cc: Chautru, Nicolas <[email protected]>; Zhang, Qi Z <[email protected]>
Subject: Re: [PATCH v2 06/10] baseband/acc: remove soft output bypassQuoted 1 line · Hernan Vargas
Removing soft output bypass capability due to device limitations.
It should be specified this is for VRB2 device variant.
And this should be backported, so pass Fixes tag and cc stable as it was introduced in v23.11 LTS.
Fixes b49fe052f9cd ("baseband/acc: add FEC capabilities for VRB2 variant")Thanks,
Maximedrivers/baseband/acc/rte_vrb_pmd.c vrb_dev_info_get(struct rte_bbdev *dev, struct
Quoted 12 lines
Signed-off-by Hernan Vargas <[email protected]>--- drivers/baseband/acc/rte_vrb_pmd.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)drivers/baseband/acc/rte_vrb_pmd.c +0
-0@@ -1359,7 +1359,6 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct
rte_bbdev_driver_info*dev_info)Quoted 1 line
RTE_BBDEV_LDPC_HARQ_4BIT_COMPRESSION|drivers/baseband/acc/rte_vrb_pmd.c vrb_dev_info_get(struct rte_bbdev *dev, struct rte_bbdev_driver_info *dev_info) · line 1362
Quoted 4 lines
RTE_BBDEV_LDPC_LLR_COMPRESSION|RTE_BBDEV_LDPC_SOFT_OUT_ENABLE|drivers/baseband/acc/rte_vrb_pmd.c +0
-1- RTE_BBDEV_LDPC_SOFT_OUT_RM_BYPASS |
RTE_BBDEV_LDPC_SOFT_OUT_DEINTERLEAVER_BYPASS|drivers/baseband/acc/rte_vrb_pmd.c vrb_fcw_ld_fill(struct rte_bbdev_dec_op *op,
Quoted 3 lines
RTE_BBDEV_LDPC_DEC_INTERRUPTS,
.llr_size = 8,drivers/baseband/acc/rte_vrb_pmd.c +0
-0@@ -1736,18 +1735,18 @@ vrb_fcw_ld_fill(struct rte_bbdev_dec_op *op,struct
acc_fcw_ld*fcw,drivers/baseband/acc/rte_vrb_pmd.c vrb_fcw_ld_fill(struct rte_bbdev_dec_op *op, · line 1735
Quoted 1 line
drivers/baseband/acc/rte_vrb_pmd.c +0
-0fcw->so_en = check_bit(op->ldpc_dec.op_flags,
RTE_BBDEV_LDPC_SOFT_OUT_ENABLE);drivers/baseband/acc/rte_vrb_pmd.c vrb_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc_fcw_ld *fcw, · line 1742
Quoted 2 lines
drivers/baseband/acc/rte_vrb_pmd.c +0
-0fcw->so_bypass_intlv = check_bit(op->ldpc_dec.op_flags,RTE_BBDEV_LDPC_SOFT_OUT_DEINTERLEAVER_BYPASS);Quoted 18 lines
drivers/baseband/acc/rte_vrb_pmd.c +3
-3- 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;
PATCH v2 7/10 baseband/acc: algorithm tuning for LDPC decoder
In reply to Hernan VargasReverting to MS1 version of the algorithm to improve MU1 fading conditions.
--- drivers/baseband/acc/rte_vrb_pmd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/baseband/acc/rte_vrb_pmd.c +3 -3
@@ -1362,7 +1362,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 =
@@ -1736,8 +1736,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 ; }/*
Signature · 1 line
2.37.1
Re: [PATCH v2 07/10] baseband/acc: algorithm tuning for LDPC decoder
Quoted 8 lines · Hernan Vargas
Reverting to MS1 version of the algorithm to improve MU1 fading conditions.
Signed-off-by Hernan Vargas <[email protected]>--- drivers/baseband/acc/rte_vrb_pmd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Thanks,
Maxime
PATCH v2 8/10 baseband/acc: remove check on HARQ memory
In reply to Hernan VargasAutomatically 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.
--- drivers/baseband/acc/rte_vrb_pmd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/baseband/acc/rte_vrb_pmd.c +2 -1
@@ -2595,8 +2595,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;
Signature · 1 line
2.37.1
Re: [PATCH v2 08/10] baseband/acc: remove check on HARQ memory
Quoted 4 lines · 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.
Should it be backported?
Quoted 10 lines
Signed-off-by Hernan Vargas <[email protected]>--- drivers/baseband/acc/rte_vrb_pmd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/baseband/acc/rte_vrb_pmd.c +0
-0
drivers/baseband/acc/rte_vrb_pmd.c vrb_enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op, · line 2603
Quoted 10 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-1@@ -2595,8 +2595,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;
Thanks,
Maxime
RE: [PATCH v2 08/10] baseband/acc: remove check on HARQ memory
Hi Maxime,
It doesn’t need to be backported.
Thanks
Nic
Quoted 49 lines
-----Original Message-----
From: Maxime Coquelin <[email protected]>
Sent: Tuesday, October 8, 2024 1:08 AM
To: Vargas, Hernan <[email protected]>; [email protected]; [email protected]; [email protected]
Cc: Chautru, Nicolas <[email protected]>; Zhang, Qi Z <[email protected]>
Subject: Re: [PATCH v2 08/10] baseband/acc: remove check on HARQ memoryQuoted 4 lines · 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.
Should it be backported?
drivers/baseband/acc/rte_vrb_pmd.c vrb_enqueue_ldpc_dec_one_op_cb(struct
Quoted 11 lines
Signed-off-by Hernan Vargas <[email protected]>--- drivers/baseband/acc/rte_vrb_pmd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/baseband/acc/rte_vrb_pmd.c +0
-0@@ -2595,8 +2595,9 @@ vrb_enqueue_ldpc_dec_one_op_cb(struct
acc_queue*q, structrte_bbdev_dec_op*op,drivers/baseband/acc/rte_vrb_pmd.c vrb_enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op,
Quoted 5 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-1/* 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.
drivers/baseband/acc/rte_vrb_pmd.c vrb_enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op, · line 2603
Quoted 6 lines
drivers/baseband/acc/rte_vrb_pmd.c +2
-0+*/ 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;Reviewed-by Maxime Coquelin <[email protected]>Thanks,
Maxime
PATCH v2 9/10 baseband/acc: reset ring data valid bit
In reply to Hernan VargasReset only the valid bit to keep info ring data notably for dumping.
--- drivers/baseband/acc/rte_vrb_pmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/baseband/acc/rte_vrb_pmd.c +1 -1
@@ -411,7 +411,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 );
Signature · 1 line
2.37.1
Re: [PATCH v2 09/10] baseband/acc: reset ring data valid bit
Quoted 12 lines · Hernan Vargas
Reset only the valid bit to keep info ring data notably for dumping.
Signed-off-by Hernan Vargas <[email protected]>--- drivers/baseband/acc/rte_vrb_pmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/baseband/acc/rte_vrb_pmd.c +0
-0
drivers/baseband/acc/rte_vrb_pmd.c vrb_check_ir(struct acc_device *acc_dev) · line 417
Quoted 8 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-1@@ -411,7 +411,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 );
Thanks,
Maxime
PATCH v2 10/10 baseband/acc: cosmetic changes
In reply to Hernan VargasCosmetic code changes.
No functional impact.
--- drivers/baseband/acc/rte_acc100_pmd.c | 2 +- drivers/baseband/acc/rte_vrb_pmd.c | 62 +++++++++++++++++++-------- 2 files changed, 44 insertions(+), 20 deletions(-)
drivers/baseband/acc/rte_acc100_pmd.c +1 -1
@@ -4199,7 +4199,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 */
drivers/baseband/acc/rte_vrb_pmd.c +43 -19
153 lines not on this page · raw message
Signature · 1 line
2.37.1
Re: [PATCH v2 10/10] baseband/acc: cosmetic changes
Quoted 67 lines · Hernan Vargas
Cosmetic code changes.
No functional impact.Signed-off-by Hernan Vargas <[email protected]>--- drivers/baseband/acc/rte_acc100_pmd.c | 2 +- drivers/baseband/acc/rte_vrb_pmd.c | 62 +++++++++++++++++++-------- 2 files changed, 44 insertions(+), 20 deletions(-)drivers/baseband/acc/rte_acc100_pmd.c +1
-1@@ -4199,7 +4199,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 */ drivers/baseband/acc/rte_vrb_pmd.c +20
-10@@ -956,6 +956,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" );@@ -982,16 +985,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 ;
drivers/baseband/acc/rte_vrb_pmd.c vrb_queue_setup(struct rte_bbdev *dev, uint16_t queue_id, · line 1014
Quoted 10 lines
drivers/baseband/acc/rte_vrb_pmd.c +10
-0+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 ; + }
This part is useful as it makes the code clearer.
Quoted 108 lines
drivers/baseband/acc/rte_vrb_pmd.c +12
-9for (desc_idx =0 ; desc_idx < d->sw_ring_max_depth; desc_idx++) { desc = q->ring_addr + desc_idx;@@ -1757,8 +1777,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;@@ -2000,16 +2019,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;@@ -2583,7 +2601,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,@@ -2645,7 +2662,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;@@ -2849,6 +2865,7 @@ vrb_enqueue_enc_cb(struct rte_bbdev_queue_data *q_data,acc_dma_enqueue (q, i, &q_data->queue_stats);acc_update_qstat_enqueue (q_data, i, num - i); +return i; }@@ -2956,6 +2973,7 @@ vrb_enqueue_ldpc_enc_tb(struct rte_bbdev_queue_data *q_data, } descs_used =vrb2_enqueue_ldpc_enc_one_op_tb (q, ops[i], enqueued_descs); } +if (descs_used <0 ) {acc_enqueue_invalid (q_data);break ;@@ -3062,6 +3080,7 @@ vrb_enqueue_ldpc_dec_tb(struct rte_bbdev_queue_data *q_data,acc_dma_enqueue (q, enqueued_cbs, &q_data->queue_stats);acc_update_qstat_enqueue (q_data, i, num - i); +return i; }@@ -3103,6 +3122,7 @@ vrb_enqueue_ldpc_dec_cb(struct rte_bbdev_queue_data *q_data,acc_dma_enqueue (q, i, &q_data->queue_stats);acc_update_qstat_enqueue (q_data, i, num - i); +return i; }@@ -3273,7 +3293,7 @@ vrb2_dequeue_ldpc_enc_one_op_tb(struct acc_queue *q, struct rte_bbdev_enc_op **rreturn 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@@ -3873,6 +3893,7 @@ vrb_enqueue_fft(struct rte_bbdev_queue_data *q_data,acc_dma_enqueue (q, i, &q_data->queue_stats);acc_update_qstat_enqueue (q_data, i, num - i); +return i; }@@ -3939,7 +3960,9 @@ vrb_dequeue_fft(struct rte_bbdev_queue_data *q_data, q->aq_dequeued += aq_dequeued; q->sw_ring_tail += dequeued_cbs; +acc_update_qstat_dequeue (q_data, i); +return i; }
drivers/baseband/acc/rte_vrb_pmd.c vrb2_enqueue_mldts(struct rte_bbdev_queue_data *q_data, · line 4223
Quoted 7 lines
drivers/baseband/acc/rte_vrb_pmd.c +1
-0@@ -4194,6 +4217,7 @@ vrb2_enqueue_mldts(struct rte_bbdev_queue_data *q_data,acc_dma_enqueue (q, enqueued_descs, &q_data->queue_stats);acc_update_qstat_enqueue (q_data, i, num - i); +return i; }
All the new lines removal does not bring clarity, they could be skipped.
Regards,
Maxime