* [PATCH v7 0/1] baseband/acc100: changes for 22.11 @ 2022-11-01 4:13 Hernan Vargas 2022-11-01 4:13 ` [PATCH v7 1/1] baseband/acc100: add detection for deRM corner cases Hernan Vargas 0 siblings, 1 reply; 8+ messages in thread From: Hernan Vargas @ 2022-11-01 4:13 UTC (permalink / raw) To: dev, gakhil, trix, maxime.coquelin Cc: nicolas.chautru, qi.z.zhang, Hernan Vargas v7: Not including dependency on SDK introduced in previous v3 for now due to lack of consensus yet. Still detecting the known corner case and flagging it. v6: Fix commit message typo. v5: Fix compilation error and squash documentation changes. v4: Rebased code to use the latest ACC common API and implemented review comment changes. v3: Code refactor based on comments and grouping fixes at beginning of series. v2: Rebased code to use ACC common API. v1: Upstreaming ACC100 changes for 22.11. This patch series is dependant on series: https://patches.dpdk.org/project/dpdk/list/?series=25191 Hernan Vargas (1): baseband/acc100: add detection for deRM corner cases drivers/baseband/acc/acc_common.h | 8 ++++ drivers/baseband/acc/rte_acc100_pmd.c | 55 +++++++++++++++++++++++++-- 2 files changed, 60 insertions(+), 3 deletions(-) -- 2.37.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v7 1/1] baseband/acc100: add detection for deRM corner cases 2022-11-01 4:13 [PATCH v7 0/1] baseband/acc100: changes for 22.11 Hernan Vargas @ 2022-11-01 4:13 ` Hernan Vargas 2022-11-03 13:00 ` [EXT] " Akhil Goyal 2022-11-03 14:10 ` Maxime Coquelin 0 siblings, 2 replies; 8+ messages in thread From: Hernan Vargas @ 2022-11-01 4:13 UTC (permalink / raw) To: dev, gakhil, trix, maxime.coquelin Cc: nicolas.chautru, qi.z.zhang, Hernan Vargas Add function to detect if de-ratematch pre-processing is recommended for SW corner cases. Some specific 5GUL FEC corner cases may cause unintended back pressure and in some cases a potential stability issue on the ACC100. The PMD can detect such code block configuration and issue an info message to the user. Signed-off-by: Hernan Vargas <hernan.vargas@intel.com> --- drivers/baseband/acc/acc_common.h | 8 ++++ drivers/baseband/acc/rte_acc100_pmd.c | 55 +++++++++++++++++++++++++-- 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/drivers/baseband/acc/acc_common.h b/drivers/baseband/acc/acc_common.h index eae7eab4e9..6213b0b61e 100644 --- a/drivers/baseband/acc/acc_common.h +++ b/drivers/baseband/acc/acc_common.h @@ -123,6 +123,14 @@ #define ACC_HARQ_ALIGN_64B 64 #define ACC_MAX_ZC 384 +/* De-ratematch code rate limitation for recommended operation */ +#define ACC_LIM_03 2 /* 0.03 */ +#define ACC_LIM_09 6 /* 0.09 */ +#define ACC_LIM_14 9 /* 0.14 */ +#define ACC_LIM_21 14 /* 0.21 */ +#define ACC_LIM_31 20 /* 0.31 */ +#define ACC_MAX_E (128 * 1024 - 2) + /* Helper macro for logging */ #define rte_acc_log(level, fmt, ...) \ rte_log(RTE_LOG_ ## level, RTE_LOG_NOTICE, fmt "\n", \ diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c index 23bc5d25bb..47609f95b7 100644 --- a/drivers/baseband/acc/rte_acc100_pmd.c +++ b/drivers/baseband/acc/rte_acc100_pmd.c @@ -756,6 +756,14 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id, ret = -ENOMEM; goto free_lb_out; } + q->derm_buffer = rte_zmalloc_socket(dev->device->driver->name, + RTE_BBDEV_TURBO_MAX_CB_SIZE * 10, + RTE_CACHE_LINE_SIZE, conf->socket); + if (q->derm_buffer == NULL) { + rte_bbdev_log(ERR, "Failed to allocate derm_buffer memory"); + ret = -ENOMEM; + goto free_companion_ring_addr; + } /* * Software queue ring wraps synchronously with the HW when it reaches @@ -776,7 +784,7 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id, q_idx = acc100_find_free_queue_idx(dev, conf); if (q_idx == -1) { ret = -EINVAL; - goto free_companion_ring_addr; + goto free_derm_buffer; } q->qgrp_id = (q_idx >> ACC100_GRP_ID_SHIFT) & 0xF; @@ -804,6 +812,9 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id, dev->data->queues[queue_id].queue_private = q; return 0; +free_derm_buffer: + rte_free(q->derm_buffer); + q->derm_buffer = NULL; free_companion_ring_addr: rte_free(q->companion_ring_addr); q->companion_ring_addr = NULL; @@ -890,6 +901,7 @@ acc100_queue_release(struct rte_bbdev *dev, uint16_t q_id) /* Mark the Queue as un-assigned */ d->q_assigned_bit_map[q->qgrp_id] &= (0xFFFFFFFFFFFFFFFF - (uint64_t) (1 << q->aq_id)); + rte_free(q->derm_buffer); rte_free(q->companion_ring_addr); rte_free(q->lb_in); rte_free(q->lb_out); @@ -3111,10 +3123,41 @@ harq_loopback(struct acc_queue *q, struct rte_bbdev_dec_op *op, return 1; } +/* Assess whether a work around is recommended for the deRM corner cases */ +static inline bool +derm_workaround_recommended(struct rte_bbdev_op_ldpc_dec *ldpc_dec, struct acc_queue *q) +{ + if (!is_acc100(q)) + return false; + int32_t e = ldpc_dec->cb_params.e; + int q_m = ldpc_dec->q_m; + int z_c = ldpc_dec->z_c; + int K = (ldpc_dec->basegraph == 1 ? ACC_K_ZC_1 : ACC_K_ZC_2) * z_c; + bool recommended = false; + + if (ldpc_dec->basegraph == 1) { + if ((q_m == 4) && (z_c >= 320) && (e * ACC_LIM_31 > K * 64)) + recommended = true; + else if ((e * ACC_LIM_21 > K * 64)) + recommended = true; + } else { + if (q_m <= 2) { + if ((z_c >= 208) && (e * ACC_LIM_09 > K * 64)) + recommended = true; + else if ((z_c < 208) && (e * ACC_LIM_03 > K * 64)) + recommended = true; + } else if (e * ACC_LIM_14 > K * 64) + recommended = true; + } + + return recommended; +} + /** Enqueue one decode operations for ACC100 device in CB mode */ static inline int enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op, - uint16_t total_enqueued_cbs, bool same_op) + uint16_t total_enqueued_cbs, bool same_op, + struct rte_bbdev_queue_data *q_data) { int ret; if (unlikely(check_bit(op->ldpc_dec.op_flags, @@ -3168,6 +3211,12 @@ enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op, } else { struct acc_fcw_ld *fcw; uint32_t seg_total_left; + + if (derm_workaround_recommended(&op->ldpc_dec, q)) { + RTE_SET_USED(q_data); + rte_bbdev_log(INFO, "Corner case may require deRM pre-processing"); + } + fcw = &desc->req.fcw_ld; q->d->fcw_ld_fill(op, fcw, harq_layout); @@ -3721,7 +3770,7 @@ acc100_enqueue_ldpc_dec_cb(struct rte_bbdev_queue_data *q_data, ops[i]->ldpc_dec.n_cb, ops[i]->ldpc_dec.q_m, ops[i]->ldpc_dec.n_filler, ops[i]->ldpc_dec.cb_params.e, same_op); - ret = enqueue_ldpc_dec_one_op_cb(q, ops[i], i, same_op); + ret = enqueue_ldpc_dec_one_op_cb(q, ops[i], i, same_op, q_data); if (ret < 0) { acc_enqueue_invalid(q_data); break; -- 2.37.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [EXT] [PATCH v7 1/1] baseband/acc100: add detection for deRM corner cases 2022-11-01 4:13 ` [PATCH v7 1/1] baseband/acc100: add detection for deRM corner cases Hernan Vargas @ 2022-11-03 13:00 ` Akhil Goyal 2022-11-03 14:10 ` Maxime Coquelin 1 sibling, 0 replies; 8+ messages in thread From: Akhil Goyal @ 2022-11-03 13:00 UTC (permalink / raw) To: Hernan Vargas, dev, trix, maxime.coquelin; +Cc: nicolas.chautru, qi.z.zhang > Add function to detect if de-ratematch pre-processing is recommended for > SW corner cases. > Some specific 5GUL FEC corner cases may cause unintended back pressure > and in some cases a potential stability issue on the ACC100. > The PMD can detect such code block configuration and issue an info > message to the user. > > Signed-off-by: Hernan Vargas <hernan.vargas@intel.com> Hi Tom/Maxime, Any further comments? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v7 1/1] baseband/acc100: add detection for deRM corner cases 2022-11-01 4:13 ` [PATCH v7 1/1] baseband/acc100: add detection for deRM corner cases Hernan Vargas 2022-11-03 13:00 ` [EXT] " Akhil Goyal @ 2022-11-03 14:10 ` Maxime Coquelin 2022-11-03 14:59 ` Chautru, Nicolas 1 sibling, 1 reply; 8+ messages in thread From: Maxime Coquelin @ 2022-11-03 14:10 UTC (permalink / raw) To: Hernan Vargas, dev, gakhil, trix; +Cc: nicolas.chautru, qi.z.zhang On 11/1/22 05:13, Hernan Vargas wrote: > Add function to detect if de-ratematch pre-processing is recommended for > SW corner cases. > Some specific 5GUL FEC corner cases may cause unintended back pressure > and in some cases a potential stability issue on the ACC100. > The PMD can detect such code block configuration and issue an info > message to the user. > > Signed-off-by: Hernan Vargas <hernan.vargas@intel.com> > --- > drivers/baseband/acc/acc_common.h | 8 ++++ > drivers/baseband/acc/rte_acc100_pmd.c | 55 +++++++++++++++++++++++++-- > 2 files changed, 60 insertions(+), 3 deletions(-) > > diff --git a/drivers/baseband/acc/acc_common.h b/drivers/baseband/acc/acc_common.h > index eae7eab4e9..6213b0b61e 100644 > --- a/drivers/baseband/acc/acc_common.h > +++ b/drivers/baseband/acc/acc_common.h > @@ -123,6 +123,14 @@ > #define ACC_HARQ_ALIGN_64B 64 > #define ACC_MAX_ZC 384 > > +/* De-ratematch code rate limitation for recommended operation */ > +#define ACC_LIM_03 2 /* 0.03 */ > +#define ACC_LIM_09 6 /* 0.09 */ > +#define ACC_LIM_14 9 /* 0.14 */ > +#define ACC_LIM_21 14 /* 0.21 */ > +#define ACC_LIM_31 20 /* 0.31 */ > +#define ACC_MAX_E (128 * 1024 - 2) > + > /* Helper macro for logging */ > #define rte_acc_log(level, fmt, ...) \ > rte_log(RTE_LOG_ ## level, RTE_LOG_NOTICE, fmt "\n", \ > diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c > index 23bc5d25bb..47609f95b7 100644 > --- a/drivers/baseband/acc/rte_acc100_pmd.c > +++ b/drivers/baseband/acc/rte_acc100_pmd.c > @@ -756,6 +756,14 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id, > ret = -ENOMEM; > goto free_lb_out; > } > + q->derm_buffer = rte_zmalloc_socket(dev->device->driver->name, > + RTE_BBDEV_TURBO_MAX_CB_SIZE * 10, > + RTE_CACHE_LINE_SIZE, conf->socket); > + if (q->derm_buffer == NULL) { > + rte_bbdev_log(ERR, "Failed to allocate derm_buffer memory"); > + ret = -ENOMEM; > + goto free_companion_ring_addr; > + } > > /* > * Software queue ring wraps synchronously with the HW when it reaches > @@ -776,7 +784,7 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id, > q_idx = acc100_find_free_queue_idx(dev, conf); > if (q_idx == -1) { > ret = -EINVAL; > - goto free_companion_ring_addr; > + goto free_derm_buffer; > } > > q->qgrp_id = (q_idx >> ACC100_GRP_ID_SHIFT) & 0xF; > @@ -804,6 +812,9 @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id, > dev->data->queues[queue_id].queue_private = q; > return 0; > > +free_derm_buffer: > + rte_free(q->derm_buffer); > + q->derm_buffer = NULL; > free_companion_ring_addr: > rte_free(q->companion_ring_addr); > q->companion_ring_addr = NULL; > @@ -890,6 +901,7 @@ acc100_queue_release(struct rte_bbdev *dev, uint16_t q_id) > /* Mark the Queue as un-assigned */ > d->q_assigned_bit_map[q->qgrp_id] &= (0xFFFFFFFFFFFFFFFF - > (uint64_t) (1 << q->aq_id)); > + rte_free(q->derm_buffer); > rte_free(q->companion_ring_addr); > rte_free(q->lb_in); > rte_free(q->lb_out); > @@ -3111,10 +3123,41 @@ harq_loopback(struct acc_queue *q, struct rte_bbdev_dec_op *op, > return 1; > } > > +/* Assess whether a work around is recommended for the deRM corner cases */ > +static inline bool > +derm_workaround_recommended(struct rte_bbdev_op_ldpc_dec *ldpc_dec, struct acc_queue *q) > +{ > + if (!is_acc100(q)) > + return false; > + int32_t e = ldpc_dec->cb_params.e; > + int q_m = ldpc_dec->q_m; > + int z_c = ldpc_dec->z_c; > + int K = (ldpc_dec->basegraph == 1 ? ACC_K_ZC_1 : ACC_K_ZC_2) * z_c; > + bool recommended = false; > + > + if (ldpc_dec->basegraph == 1) { > + if ((q_m == 4) && (z_c >= 320) && (e * ACC_LIM_31 > K * 64)) > + recommended = true; > + else if ((e * ACC_LIM_21 > K * 64)) > + recommended = true; > + } else { > + if (q_m <= 2) { > + if ((z_c >= 208) && (e * ACC_LIM_09 > K * 64)) > + recommended = true; > + else if ((z_c < 208) && (e * ACC_LIM_03 > K * 64)) > + recommended = true; > + } else if (e * ACC_LIM_14 > K * 64) > + recommended = true; > + } > + > + return recommended; > +} > + > /** Enqueue one decode operations for ACC100 device in CB mode */ > static inline int > enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op, > - uint16_t total_enqueued_cbs, bool same_op) > + uint16_t total_enqueued_cbs, bool same_op, > + struct rte_bbdev_queue_data *q_data) > { > int ret; > if (unlikely(check_bit(op->ldpc_dec.op_flags, > @@ -3168,6 +3211,12 @@ enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct rte_bbdev_dec_op *op, > } else { > struct acc_fcw_ld *fcw; > uint32_t seg_total_left; > + > + if (derm_workaround_recommended(&op->ldpc_dec, q)) { > + RTE_SET_USED(q_data); Why do we need q_data if it is not being used? > + rte_bbdev_log(INFO, "Corner case may require deRM pre-processing"); > + } > + > fcw = &desc->req.fcw_ld; > q->d->fcw_ld_fill(op, fcw, harq_layout); > > @@ -3721,7 +3770,7 @@ acc100_enqueue_ldpc_dec_cb(struct rte_bbdev_queue_data *q_data, > ops[i]->ldpc_dec.n_cb, ops[i]->ldpc_dec.q_m, > ops[i]->ldpc_dec.n_filler, ops[i]->ldpc_dec.cb_params.e, > same_op); > - ret = enqueue_ldpc_dec_one_op_cb(q, ops[i], i, same_op); > + ret = enqueue_ldpc_dec_one_op_cb(q, ops[i], i, same_op, q_data); > if (ret < 0) { > acc_enqueue_invalid(q_data); > break; ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v7 1/1] baseband/acc100: add detection for deRM corner cases 2022-11-03 14:10 ` Maxime Coquelin @ 2022-11-03 14:59 ` Chautru, Nicolas 2022-11-03 15:07 ` Maxime Coquelin 0 siblings, 1 reply; 8+ messages in thread From: Chautru, Nicolas @ 2022-11-03 14:59 UTC (permalink / raw) To: Maxime Coquelin, Vargas, Hernan, dev, gakhil, trix; +Cc: Zhang, Qi Z Hi Maxime, > -----Original Message----- > From: Maxime Coquelin <maxime.coquelin@redhat.com> > Sent: Thursday, November 3, 2022 7:10 AM > To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org; > gakhil@marvell.com; trix@redhat.com > Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z > <qi.z.zhang@intel.com> > Subject: Re: [PATCH v7 1/1] baseband/acc100: add detection for deRM corner > cases > > > > On 11/1/22 05:13, Hernan Vargas wrote: > > Add function to detect if de-ratematch pre-processing is recommended > > for SW corner cases. > > Some specific 5GUL FEC corner cases may cause unintended back pressure > > and in some cases a potential stability issue on the ACC100. > > The PMD can detect such code block configuration and issue an info > > message to the user. > > > > Signed-off-by: Hernan Vargas <hernan.vargas@intel.com> > > --- > > drivers/baseband/acc/acc_common.h | 8 ++++ > > drivers/baseband/acc/rte_acc100_pmd.c | 55 > +++++++++++++++++++++++++-- > > 2 files changed, 60 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/baseband/acc/acc_common.h > > b/drivers/baseband/acc/acc_common.h > > index eae7eab4e9..6213b0b61e 100644 > > --- a/drivers/baseband/acc/acc_common.h > > +++ b/drivers/baseband/acc/acc_common.h > > @@ -123,6 +123,14 @@ > > #define ACC_HARQ_ALIGN_64B 64 > > #define ACC_MAX_ZC 384 > > > > +/* De-ratematch code rate limitation for recommended operation */ > > +#define ACC_LIM_03 2 /* 0.03 */ #define ACC_LIM_09 6 /* 0.09 */ > > +#define ACC_LIM_14 9 /* 0.14 */ #define ACC_LIM_21 14 /* 0.21 */ > > +#define ACC_LIM_31 20 /* 0.31 */ #define ACC_MAX_E (128 * 1024 - 2) > > + > > /* Helper macro for logging */ > > #define rte_acc_log(level, fmt, ...) \ > > rte_log(RTE_LOG_ ## level, RTE_LOG_NOTICE, fmt "\n", \ diff --git > > a/drivers/baseband/acc/rte_acc100_pmd.c > > b/drivers/baseband/acc/rte_acc100_pmd.c > > index 23bc5d25bb..47609f95b7 100644 > > --- a/drivers/baseband/acc/rte_acc100_pmd.c > > +++ b/drivers/baseband/acc/rte_acc100_pmd.c > > @@ -756,6 +756,14 @@ acc100_queue_setup(struct rte_bbdev *dev, > uint16_t queue_id, > > ret = -ENOMEM; > > goto free_lb_out; > > } > > + q->derm_buffer = rte_zmalloc_socket(dev->device->driver->name, > > + RTE_BBDEV_TURBO_MAX_CB_SIZE * 10, > > + RTE_CACHE_LINE_SIZE, conf->socket); > > + if (q->derm_buffer == NULL) { > > + rte_bbdev_log(ERR, "Failed to allocate derm_buffer memory"); > > + ret = -ENOMEM; > > + goto free_companion_ring_addr; > > + } > > > > /* > > * Software queue ring wraps synchronously with the HW when it > > reaches @@ -776,7 +784,7 @@ acc100_queue_setup(struct rte_bbdev *dev, > uint16_t queue_id, > > q_idx = acc100_find_free_queue_idx(dev, conf); > > if (q_idx == -1) { > > ret = -EINVAL; > > - goto free_companion_ring_addr; > > + goto free_derm_buffer; > > } > > > > q->qgrp_id = (q_idx >> ACC100_GRP_ID_SHIFT) & 0xF; @@ -804,6 > +812,9 > > @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id, > > dev->data->queues[queue_id].queue_private = q; > > return 0; > > > > +free_derm_buffer: > > + rte_free(q->derm_buffer); > > + q->derm_buffer = NULL; > > free_companion_ring_addr: > > rte_free(q->companion_ring_addr); > > q->companion_ring_addr = NULL; > > @@ -890,6 +901,7 @@ acc100_queue_release(struct rte_bbdev *dev, > uint16_t q_id) > > /* Mark the Queue as un-assigned */ > > d->q_assigned_bit_map[q->qgrp_id] &= (0xFFFFFFFFFFFFFFFF - > > (uint64_t) (1 << q->aq_id)); > > + rte_free(q->derm_buffer); > > rte_free(q->companion_ring_addr); > > rte_free(q->lb_in); > > rte_free(q->lb_out); > > @@ -3111,10 +3123,41 @@ harq_loopback(struct acc_queue *q, struct > rte_bbdev_dec_op *op, > > return 1; > > } > > > > +/* Assess whether a work around is recommended for the deRM corner > > +cases */ static inline bool derm_workaround_recommended(struct > > +rte_bbdev_op_ldpc_dec *ldpc_dec, struct acc_queue *q) { > > + if (!is_acc100(q)) > > + return false; > > + int32_t e = ldpc_dec->cb_params.e; > > + int q_m = ldpc_dec->q_m; > > + int z_c = ldpc_dec->z_c; > > + int K = (ldpc_dec->basegraph == 1 ? ACC_K_ZC_1 : ACC_K_ZC_2) * z_c; > > + bool recommended = false; > > + > > + if (ldpc_dec->basegraph == 1) { > > + if ((q_m == 4) && (z_c >= 320) && (e * ACC_LIM_31 > K * 64)) > > + recommended = true; > > + else if ((e * ACC_LIM_21 > K * 64)) > > + recommended = true; > > + } else { > > + if (q_m <= 2) { > > + if ((z_c >= 208) && (e * ACC_LIM_09 > K * 64)) > > + recommended = true; > > + else if ((z_c < 208) && (e * ACC_LIM_03 > K * 64)) > > + recommended = true; > > + } else if (e * ACC_LIM_14 > K * 64) > > + recommended = true; > > + } > > + > > + return recommended; > > +} > > + > > /** Enqueue one decode operations for ACC100 device in CB mode */ > > static inline int > > enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct > rte_bbdev_dec_op *op, > > - uint16_t total_enqueued_cbs, bool same_op) > > + uint16_t total_enqueued_cbs, bool same_op, > > + struct rte_bbdev_queue_data *q_data) > > { > > int ret; > > if (unlikely(check_bit(op->ldpc_dec.op_flags, > > @@ -3168,6 +3211,12 @@ enqueue_ldpc_dec_one_op_cb(struct acc_queue > *q, struct rte_bbdev_dec_op *op, > > } else { > > struct acc_fcw_ld *fcw; > > uint32_t seg_total_left; > > + > > + if (derm_workaround_recommended(&op->ldpc_dec, q)) { > > + RTE_SET_USED(q_data); > Why do we need q_data if it is not being used? As you can guess this notably allows customer do consider running deRM from here as a local patch hence keeping the q_data accessible. Basically we keep the prototype of the function enqueue_ldpc_dec_one_op_cb() compatible with such usage. I believe this is a good practice here. Let us know if you are not convinced. Thanks Nic > > > + rte_bbdev_log(INFO, "Corner case may require deRM > pre-processing"); > > + } > > + > > fcw = &desc->req.fcw_ld; > > q->d->fcw_ld_fill(op, fcw, harq_layout); > > > > @@ -3721,7 +3770,7 @@ acc100_enqueue_ldpc_dec_cb(struct > rte_bbdev_queue_data *q_data, > > ops[i]->ldpc_dec.n_cb, ops[i]->ldpc_dec.q_m, > > ops[i]->ldpc_dec.n_filler, ops[i]- > >ldpc_dec.cb_params.e, > > same_op); > > - ret = enqueue_ldpc_dec_one_op_cb(q, ops[i], i, same_op); > > + ret = enqueue_ldpc_dec_one_op_cb(q, ops[i], i, same_op, > q_data); > > if (ret < 0) { > > acc_enqueue_invalid(q_data); > > break; ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v7 1/1] baseband/acc100: add detection for deRM corner cases 2022-11-03 14:59 ` Chautru, Nicolas @ 2022-11-03 15:07 ` Maxime Coquelin 2022-11-03 16:20 ` Chautru, Nicolas 0 siblings, 1 reply; 8+ messages in thread From: Maxime Coquelin @ 2022-11-03 15:07 UTC (permalink / raw) To: Chautru, Nicolas, Vargas, Hernan, dev, gakhil, trix; +Cc: Zhang, Qi Z Hi Nicolas, On 11/3/22 15:59, Chautru, Nicolas wrote: > Hi Maxime, > >> -----Original Message----- >> From: Maxime Coquelin <maxime.coquelin@redhat.com> >> Sent: Thursday, November 3, 2022 7:10 AM >> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org; >> gakhil@marvell.com; trix@redhat.com >> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z >> <qi.z.zhang@intel.com> >> Subject: Re: [PATCH v7 1/1] baseband/acc100: add detection for deRM corner >> cases >> >> >> >> On 11/1/22 05:13, Hernan Vargas wrote: >>> Add function to detect if de-ratematch pre-processing is recommended >>> for SW corner cases. >>> Some specific 5GUL FEC corner cases may cause unintended back pressure >>> and in some cases a potential stability issue on the ACC100. >>> The PMD can detect such code block configuration and issue an info >>> message to the user. >>> >>> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com> >>> --- >>> drivers/baseband/acc/acc_common.h | 8 ++++ >>> drivers/baseband/acc/rte_acc100_pmd.c | 55 >> +++++++++++++++++++++++++-- >>> 2 files changed, 60 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/baseband/acc/acc_common.h >>> b/drivers/baseband/acc/acc_common.h >>> index eae7eab4e9..6213b0b61e 100644 >>> --- a/drivers/baseband/acc/acc_common.h >>> +++ b/drivers/baseband/acc/acc_common.h >>> @@ -123,6 +123,14 @@ >>> #define ACC_HARQ_ALIGN_64B 64 >>> #define ACC_MAX_ZC 384 >>> >>> +/* De-ratematch code rate limitation for recommended operation */ >>> +#define ACC_LIM_03 2 /* 0.03 */ #define ACC_LIM_09 6 /* 0.09 */ >>> +#define ACC_LIM_14 9 /* 0.14 */ #define ACC_LIM_21 14 /* 0.21 */ >>> +#define ACC_LIM_31 20 /* 0.31 */ #define ACC_MAX_E (128 * 1024 - 2) >>> + >>> /* Helper macro for logging */ >>> #define rte_acc_log(level, fmt, ...) \ >>> rte_log(RTE_LOG_ ## level, RTE_LOG_NOTICE, fmt "\n", \ diff --git >>> a/drivers/baseband/acc/rte_acc100_pmd.c >>> b/drivers/baseband/acc/rte_acc100_pmd.c >>> index 23bc5d25bb..47609f95b7 100644 >>> --- a/drivers/baseband/acc/rte_acc100_pmd.c >>> +++ b/drivers/baseband/acc/rte_acc100_pmd.c >>> @@ -756,6 +756,14 @@ acc100_queue_setup(struct rte_bbdev *dev, >> uint16_t queue_id, >>> ret = -ENOMEM; >>> goto free_lb_out; >>> } >>> + q->derm_buffer = rte_zmalloc_socket(dev->device->driver->name, >>> + RTE_BBDEV_TURBO_MAX_CB_SIZE * 10, >>> + RTE_CACHE_LINE_SIZE, conf->socket); >>> + if (q->derm_buffer == NULL) { >>> + rte_bbdev_log(ERR, "Failed to allocate derm_buffer memory"); >>> + ret = -ENOMEM; >>> + goto free_companion_ring_addr; >>> + } >>> >>> /* >>> * Software queue ring wraps synchronously with the HW when it >>> reaches @@ -776,7 +784,7 @@ acc100_queue_setup(struct rte_bbdev *dev, >> uint16_t queue_id, >>> q_idx = acc100_find_free_queue_idx(dev, conf); >>> if (q_idx == -1) { >>> ret = -EINVAL; >>> - goto free_companion_ring_addr; >>> + goto free_derm_buffer; >>> } >>> >>> q->qgrp_id = (q_idx >> ACC100_GRP_ID_SHIFT) & 0xF; @@ -804,6 >> +812,9 >>> @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id, >>> dev->data->queues[queue_id].queue_private = q; >>> return 0; >>> >>> +free_derm_buffer: >>> + rte_free(q->derm_buffer); >>> + q->derm_buffer = NULL; >>> free_companion_ring_addr: >>> rte_free(q->companion_ring_addr); >>> q->companion_ring_addr = NULL; >>> @@ -890,6 +901,7 @@ acc100_queue_release(struct rte_bbdev *dev, >> uint16_t q_id) >>> /* Mark the Queue as un-assigned */ >>> d->q_assigned_bit_map[q->qgrp_id] &= (0xFFFFFFFFFFFFFFFF - >>> (uint64_t) (1 << q->aq_id)); >>> + rte_free(q->derm_buffer); >>> rte_free(q->companion_ring_addr); >>> rte_free(q->lb_in); >>> rte_free(q->lb_out); >>> @@ -3111,10 +3123,41 @@ harq_loopback(struct acc_queue *q, struct >> rte_bbdev_dec_op *op, >>> return 1; >>> } >>> >>> +/* Assess whether a work around is recommended for the deRM corner >>> +cases */ static inline bool derm_workaround_recommended(struct >>> +rte_bbdev_op_ldpc_dec *ldpc_dec, struct acc_queue *q) { >>> + if (!is_acc100(q)) >>> + return false; >>> + int32_t e = ldpc_dec->cb_params.e; >>> + int q_m = ldpc_dec->q_m; >>> + int z_c = ldpc_dec->z_c; >>> + int K = (ldpc_dec->basegraph == 1 ? ACC_K_ZC_1 : ACC_K_ZC_2) * z_c; >>> + bool recommended = false; >>> + >>> + if (ldpc_dec->basegraph == 1) { >>> + if ((q_m == 4) && (z_c >= 320) && (e * ACC_LIM_31 > K * 64)) >>> + recommended = true; >>> + else if ((e * ACC_LIM_21 > K * 64)) >>> + recommended = true; >>> + } else { >>> + if (q_m <= 2) { >>> + if ((z_c >= 208) && (e * ACC_LIM_09 > K * 64)) >>> + recommended = true; >>> + else if ((z_c < 208) && (e * ACC_LIM_03 > K * 64)) >>> + recommended = true; >>> + } else if (e * ACC_LIM_14 > K * 64) >>> + recommended = true; >>> + } >>> + >>> + return recommended; >>> +} >>> + >>> /** Enqueue one decode operations for ACC100 device in CB mode */ >>> static inline int >>> enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct >> rte_bbdev_dec_op *op, >>> - uint16_t total_enqueued_cbs, bool same_op) >>> + uint16_t total_enqueued_cbs, bool same_op, >>> + struct rte_bbdev_queue_data *q_data) >>> { >>> int ret; >>> if (unlikely(check_bit(op->ldpc_dec.op_flags, >>> @@ -3168,6 +3211,12 @@ enqueue_ldpc_dec_one_op_cb(struct acc_queue >> *q, struct rte_bbdev_dec_op *op, >>> } else { >>> struct acc_fcw_ld *fcw; >>> uint32_t seg_total_left; >>> + >>> + if (derm_workaround_recommended(&op->ldpc_dec, q)) { >>> + RTE_SET_USED(q_data); >> Why do we need q_data if it is not being used? > > As you can guess this notably allows customer do consider running deRM from here as a local patch hence keeping the q_data accessible. > Basically we keep the prototype of the function enqueue_ldpc_dec_one_op_cb() compatible with such usage. > I believe this is a good practice here. Let us know if you are not convinced. I think it is better to just warn the user, without adding unused parameters. Thanks, Maxime > Thanks > Nic > >> >>> + rte_bbdev_log(INFO, "Corner case may require deRM >> pre-processing"); >>> + } >>> + >>> fcw = &desc->req.fcw_ld; >>> q->d->fcw_ld_fill(op, fcw, harq_layout); >>> >>> @@ -3721,7 +3770,7 @@ acc100_enqueue_ldpc_dec_cb(struct >> rte_bbdev_queue_data *q_data, >>> ops[i]->ldpc_dec.n_cb, ops[i]->ldpc_dec.q_m, >>> ops[i]->ldpc_dec.n_filler, ops[i]- >>> ldpc_dec.cb_params.e, >>> same_op); >>> - ret = enqueue_ldpc_dec_one_op_cb(q, ops[i], i, same_op); >>> + ret = enqueue_ldpc_dec_one_op_cb(q, ops[i], i, same_op, >> q_data); >>> if (ret < 0) { >>> acc_enqueue_invalid(q_data); >>> break; > ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v7 1/1] baseband/acc100: add detection for deRM corner cases 2022-11-03 15:07 ` Maxime Coquelin @ 2022-11-03 16:20 ` Chautru, Nicolas 2022-11-03 19:24 ` Chautru, Nicolas 0 siblings, 1 reply; 8+ messages in thread From: Chautru, Nicolas @ 2022-11-03 16:20 UTC (permalink / raw) To: Maxime Coquelin, Vargas, Hernan, dev, gakhil, trix; +Cc: Zhang, Qi Z Hi Maxime, Akhil, > -----Original Message----- > From: Maxime Coquelin <maxime.coquelin@redhat.com> > Sent: Thursday, November 3, 2022 8:08 AM > To: Chautru, Nicolas <nicolas.chautru@intel.com>; Vargas, Hernan > <hernan.vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; > trix@redhat.com > Cc: Zhang, Qi Z <qi.z.zhang@intel.com> > Subject: Re: [PATCH v7 1/1] baseband/acc100: add detection for deRM corner > cases > > Hi Nicolas, > > On 11/3/22 15:59, Chautru, Nicolas wrote: > > Hi Maxime, > > > >> -----Original Message----- > >> From: Maxime Coquelin <maxime.coquelin@redhat.com> > >> Sent: Thursday, November 3, 2022 7:10 AM > >> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org; > >> gakhil@marvell.com; trix@redhat.com > >> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z > >> <qi.z.zhang@intel.com> > >> Subject: Re: [PATCH v7 1/1] baseband/acc100: add detection for deRM > >> corner cases > >> > >> > >> > >> On 11/1/22 05:13, Hernan Vargas wrote: > >>> Add function to detect if de-ratematch pre-processing is recommended > >>> for SW corner cases. > >>> Some specific 5GUL FEC corner cases may cause unintended back > >>> pressure and in some cases a potential stability issue on the ACC100. > >>> The PMD can detect such code block configuration and issue an info > >>> message to the user. > >>> > >>> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com> > >>> --- > >>> drivers/baseband/acc/acc_common.h | 8 ++++ > >>> drivers/baseband/acc/rte_acc100_pmd.c | 55 > >> +++++++++++++++++++++++++-- > >>> 2 files changed, 60 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/drivers/baseband/acc/acc_common.h > >>> b/drivers/baseband/acc/acc_common.h > >>> index eae7eab4e9..6213b0b61e 100644 > >>> --- a/drivers/baseband/acc/acc_common.h > >>> +++ b/drivers/baseband/acc/acc_common.h > >>> @@ -123,6 +123,14 @@ > >>> #define ACC_HARQ_ALIGN_64B 64 > >>> #define ACC_MAX_ZC 384 > >>> > >>> +/* De-ratematch code rate limitation for recommended operation */ > >>> +#define ACC_LIM_03 2 /* 0.03 */ #define ACC_LIM_09 6 /* 0.09 */ > >>> +#define ACC_LIM_14 9 /* 0.14 */ #define ACC_LIM_21 14 /* 0.21 */ > >>> +#define ACC_LIM_31 20 /* 0.31 */ #define ACC_MAX_E (128 * 1024 - 2) > >>> + > >>> /* Helper macro for logging */ > >>> #define rte_acc_log(level, fmt, ...) \ > >>> rte_log(RTE_LOG_ ## level, RTE_LOG_NOTICE, fmt "\n", \ diff > >>> --git a/drivers/baseband/acc/rte_acc100_pmd.c > >>> b/drivers/baseband/acc/rte_acc100_pmd.c > >>> index 23bc5d25bb..47609f95b7 100644 > >>> --- a/drivers/baseband/acc/rte_acc100_pmd.c > >>> +++ b/drivers/baseband/acc/rte_acc100_pmd.c > >>> @@ -756,6 +756,14 @@ acc100_queue_setup(struct rte_bbdev *dev, > >> uint16_t queue_id, > >>> ret = -ENOMEM; > >>> goto free_lb_out; > >>> } > >>> + q->derm_buffer = rte_zmalloc_socket(dev->device->driver->name, > >>> + RTE_BBDEV_TURBO_MAX_CB_SIZE * 10, > >>> + RTE_CACHE_LINE_SIZE, conf->socket); > >>> + if (q->derm_buffer == NULL) { > >>> + rte_bbdev_log(ERR, "Failed to allocate derm_buffer memory"); > >>> + ret = -ENOMEM; > >>> + goto free_companion_ring_addr; > >>> + } > >>> > >>> /* > >>> * Software queue ring wraps synchronously with the HW when it > >>> reaches @@ -776,7 +784,7 @@ acc100_queue_setup(struct rte_bbdev > >>> *dev, > >> uint16_t queue_id, > >>> q_idx = acc100_find_free_queue_idx(dev, conf); > >>> if (q_idx == -1) { > >>> ret = -EINVAL; > >>> - goto free_companion_ring_addr; > >>> + goto free_derm_buffer; > >>> } > >>> > >>> q->qgrp_id = (q_idx >> ACC100_GRP_ID_SHIFT) & 0xF; @@ -804,6 > >> +812,9 > >>> @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id, > >>> dev->data->queues[queue_id].queue_private = q; > >>> return 0; > >>> > >>> +free_derm_buffer: > >>> + rte_free(q->derm_buffer); > >>> + q->derm_buffer = NULL; > >>> free_companion_ring_addr: > >>> rte_free(q->companion_ring_addr); > >>> q->companion_ring_addr = NULL; > >>> @@ -890,6 +901,7 @@ acc100_queue_release(struct rte_bbdev *dev, > >> uint16_t q_id) > >>> /* Mark the Queue as un-assigned */ > >>> d->q_assigned_bit_map[q->qgrp_id] &= (0xFFFFFFFFFFFFFFFF - > >>> (uint64_t) (1 << q->aq_id)); > >>> + rte_free(q->derm_buffer); > >>> rte_free(q->companion_ring_addr); > >>> rte_free(q->lb_in); > >>> rte_free(q->lb_out); > >>> @@ -3111,10 +3123,41 @@ harq_loopback(struct acc_queue *q, struct > >> rte_bbdev_dec_op *op, > >>> return 1; > >>> } > >>> > >>> +/* Assess whether a work around is recommended for the deRM corner > >>> +cases */ static inline bool derm_workaround_recommended(struct > >>> +rte_bbdev_op_ldpc_dec *ldpc_dec, struct acc_queue *q) { > >>> + if (!is_acc100(q)) > >>> + return false; > >>> + int32_t e = ldpc_dec->cb_params.e; > >>> + int q_m = ldpc_dec->q_m; > >>> + int z_c = ldpc_dec->z_c; > >>> + int K = (ldpc_dec->basegraph == 1 ? ACC_K_ZC_1 : ACC_K_ZC_2) * z_c; > >>> + bool recommended = false; > >>> + > >>> + if (ldpc_dec->basegraph == 1) { > >>> + if ((q_m == 4) && (z_c >= 320) && (e * ACC_LIM_31 > K * 64)) > >>> + recommended = true; > >>> + else if ((e * ACC_LIM_21 > K * 64)) > >>> + recommended = true; > >>> + } else { > >>> + if (q_m <= 2) { > >>> + if ((z_c >= 208) && (e * ACC_LIM_09 > K * 64)) > >>> + recommended = true; > >>> + else if ((z_c < 208) && (e * ACC_LIM_03 > K * 64)) > >>> + recommended = true; > >>> + } else if (e * ACC_LIM_14 > K * 64) > >>> + recommended = true; > >>> + } > >>> + > >>> + return recommended; > >>> +} > >>> + > >>> /** Enqueue one decode operations for ACC100 device in CB mode */ > >>> static inline int > >>> enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct > >> rte_bbdev_dec_op *op, > >>> - uint16_t total_enqueued_cbs, bool same_op) > >>> + uint16_t total_enqueued_cbs, bool same_op, > >>> + struct rte_bbdev_queue_data *q_data) > >>> { > >>> int ret; > >>> if (unlikely(check_bit(op->ldpc_dec.op_flags, > >>> @@ -3168,6 +3211,12 @@ enqueue_ldpc_dec_one_op_cb(struct > acc_queue > >> *q, struct rte_bbdev_dec_op *op, > >>> } else { > >>> struct acc_fcw_ld *fcw; > >>> uint32_t seg_total_left; > >>> + > >>> + if (derm_workaround_recommended(&op->ldpc_dec, q)) { > >>> + RTE_SET_USED(q_data); > >> Why do we need q_data if it is not being used? > > > > As you can guess this notably allows customer do consider running deRM > from here as a local patch hence keeping the q_data accessible. > > Basically we keep the prototype of the function > enqueue_ldpc_dec_one_op_cb() compatible with such usage. > > I believe this is a good practice here. Let us know if you are not convinced. > > I think it is better to just warn the user, without adding unused parameters. Thanks Maxime. Akhil, any additional view on this? For the PMD usage and support this makes more sense to keep to prototype with q_data even with RTE_SET_USED(). I don't see any meaningful drawback justifying to push back. Still let us know. Thanks Nic ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v7 1/1] baseband/acc100: add detection for deRM corner cases 2022-11-03 16:20 ` Chautru, Nicolas @ 2022-11-03 19:24 ` Chautru, Nicolas 0 siblings, 0 replies; 8+ messages in thread From: Chautru, Nicolas @ 2022-11-03 19:24 UTC (permalink / raw) To: Maxime Coquelin, Vargas, Hernan, dev, gakhil, trix; +Cc: Zhang, Qi Z Given how close we are to 22.11, let's not argue over this. Hernan sending a new patch based on Maxime recommendation. > -----Original Message----- > From: Chautru, Nicolas > Sent: Thursday, November 3, 2022 9:21 AM > To: Maxime Coquelin <maxime.coquelin@redhat.com>; Vargas, Hernan > <Hernan.Vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; > trix@redhat.com > Cc: Zhang, Qi Z <qi.z.zhang@intel.com> > Subject: RE: [PATCH v7 1/1] baseband/acc100: add detection for deRM corner > cases > > Hi Maxime, Akhil, > > > > -----Original Message----- > > From: Maxime Coquelin <maxime.coquelin@redhat.com> > > Sent: Thursday, November 3, 2022 8:08 AM > > To: Chautru, Nicolas <nicolas.chautru@intel.com>; Vargas, Hernan > > <hernan.vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; > > trix@redhat.com > > Cc: Zhang, Qi Z <qi.z.zhang@intel.com> > > Subject: Re: [PATCH v7 1/1] baseband/acc100: add detection for deRM > > corner cases > > > > Hi Nicolas, > > > > On 11/3/22 15:59, Chautru, Nicolas wrote: > > > Hi Maxime, > > > > > >> -----Original Message----- > > >> From: Maxime Coquelin <maxime.coquelin@redhat.com> > > >> Sent: Thursday, November 3, 2022 7:10 AM > > >> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org; > > >> gakhil@marvell.com; trix@redhat.com > > >> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z > > >> <qi.z.zhang@intel.com> > > >> Subject: Re: [PATCH v7 1/1] baseband/acc100: add detection for deRM > > >> corner cases > > >> > > >> > > >> > > >> On 11/1/22 05:13, Hernan Vargas wrote: > > >>> Add function to detect if de-ratematch pre-processing is > > >>> recommended for SW corner cases. > > >>> Some specific 5GUL FEC corner cases may cause unintended back > > >>> pressure and in some cases a potential stability issue on the ACC100. > > >>> The PMD can detect such code block configuration and issue an info > > >>> message to the user. > > >>> > > >>> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com> > > >>> --- > > >>> drivers/baseband/acc/acc_common.h | 8 ++++ > > >>> drivers/baseband/acc/rte_acc100_pmd.c | 55 > > >> +++++++++++++++++++++++++-- > > >>> 2 files changed, 60 insertions(+), 3 deletions(-) > > >>> > > >>> diff --git a/drivers/baseband/acc/acc_common.h > > >>> b/drivers/baseband/acc/acc_common.h > > >>> index eae7eab4e9..6213b0b61e 100644 > > >>> --- a/drivers/baseband/acc/acc_common.h > > >>> +++ b/drivers/baseband/acc/acc_common.h > > >>> @@ -123,6 +123,14 @@ > > >>> #define ACC_HARQ_ALIGN_64B 64 > > >>> #define ACC_MAX_ZC 384 > > >>> > > >>> +/* De-ratematch code rate limitation for recommended operation */ > > >>> +#define ACC_LIM_03 2 /* 0.03 */ #define ACC_LIM_09 6 /* 0.09 */ > > >>> +#define ACC_LIM_14 9 /* 0.14 */ #define ACC_LIM_21 14 /* 0.21 */ > > >>> +#define ACC_LIM_31 20 /* 0.31 */ #define ACC_MAX_E (128 * 1024 - > > >>> +2) > > >>> + > > >>> /* Helper macro for logging */ > > >>> #define rte_acc_log(level, fmt, ...) \ > > >>> rte_log(RTE_LOG_ ## level, RTE_LOG_NOTICE, fmt "\n", \ diff > > >>> --git a/drivers/baseband/acc/rte_acc100_pmd.c > > >>> b/drivers/baseband/acc/rte_acc100_pmd.c > > >>> index 23bc5d25bb..47609f95b7 100644 > > >>> --- a/drivers/baseband/acc/rte_acc100_pmd.c > > >>> +++ b/drivers/baseband/acc/rte_acc100_pmd.c > > >>> @@ -756,6 +756,14 @@ acc100_queue_setup(struct rte_bbdev *dev, > > >> uint16_t queue_id, > > >>> ret = -ENOMEM; > > >>> goto free_lb_out; > > >>> } > > >>> + q->derm_buffer = rte_zmalloc_socket(dev->device->driver->name, > > >>> + RTE_BBDEV_TURBO_MAX_CB_SIZE * 10, > > >>> + RTE_CACHE_LINE_SIZE, conf->socket); > > >>> + if (q->derm_buffer == NULL) { > > >>> + rte_bbdev_log(ERR, "Failed to allocate derm_buffer > memory"); > > >>> + ret = -ENOMEM; > > >>> + goto free_companion_ring_addr; > > >>> + } > > >>> > > >>> /* > > >>> * Software queue ring wraps synchronously with the HW when it > > >>> reaches @@ -776,7 +784,7 @@ acc100_queue_setup(struct rte_bbdev > > >>> *dev, > > >> uint16_t queue_id, > > >>> q_idx = acc100_find_free_queue_idx(dev, conf); > > >>> if (q_idx == -1) { > > >>> ret = -EINVAL; > > >>> - goto free_companion_ring_addr; > > >>> + goto free_derm_buffer; > > >>> } > > >>> > > >>> q->qgrp_id = (q_idx >> ACC100_GRP_ID_SHIFT) & 0xF; @@ -804,6 > > >> +812,9 > > >>> @@ acc100_queue_setup(struct rte_bbdev *dev, uint16_t queue_id, > > >>> dev->data->queues[queue_id].queue_private = q; > > >>> return 0; > > >>> > > >>> +free_derm_buffer: > > >>> + rte_free(q->derm_buffer); > > >>> + q->derm_buffer = NULL; > > >>> free_companion_ring_addr: > > >>> rte_free(q->companion_ring_addr); > > >>> q->companion_ring_addr = NULL; @@ -890,6 +901,7 @@ > > >>> acc100_queue_release(struct rte_bbdev *dev, > > >> uint16_t q_id) > > >>> /* Mark the Queue as un-assigned */ > > >>> d->q_assigned_bit_map[q->qgrp_id] &= (0xFFFFFFFFFFFFFFFF - > > >>> (uint64_t) (1 << q->aq_id)); > > >>> + rte_free(q->derm_buffer); > > >>> rte_free(q->companion_ring_addr); > > >>> rte_free(q->lb_in); > > >>> rte_free(q->lb_out); > > >>> @@ -3111,10 +3123,41 @@ harq_loopback(struct acc_queue *q, struct > > >> rte_bbdev_dec_op *op, > > >>> return 1; > > >>> } > > >>> > > >>> +/* Assess whether a work around is recommended for the deRM > > >>> +corner cases */ static inline bool > > >>> +derm_workaround_recommended(struct > > >>> +rte_bbdev_op_ldpc_dec *ldpc_dec, struct acc_queue *q) { > > >>> + if (!is_acc100(q)) > > >>> + return false; > > >>> + int32_t e = ldpc_dec->cb_params.e; > > >>> + int q_m = ldpc_dec->q_m; > > >>> + int z_c = ldpc_dec->z_c; > > >>> + int K = (ldpc_dec->basegraph == 1 ? ACC_K_ZC_1 : ACC_K_ZC_2) * z_c; > > >>> + bool recommended = false; > > >>> + > > >>> + if (ldpc_dec->basegraph == 1) { > > >>> + if ((q_m == 4) && (z_c >= 320) && (e * ACC_LIM_31 > K * 64)) > > >>> + recommended = true; > > >>> + else if ((e * ACC_LIM_21 > K * 64)) > > >>> + recommended = true; > > >>> + } else { > > >>> + if (q_m <= 2) { > > >>> + if ((z_c >= 208) && (e * ACC_LIM_09 > K * 64)) > > >>> + recommended = true; > > >>> + else if ((z_c < 208) && (e * ACC_LIM_03 > K * 64)) > > >>> + recommended = true; > > >>> + } else if (e * ACC_LIM_14 > K * 64) > > >>> + recommended = true; > > >>> + } > > >>> + > > >>> + return recommended; > > >>> +} > > >>> + > > >>> /** Enqueue one decode operations for ACC100 device in CB mode */ > > >>> static inline int > > >>> enqueue_ldpc_dec_one_op_cb(struct acc_queue *q, struct > > >> rte_bbdev_dec_op *op, > > >>> - uint16_t total_enqueued_cbs, bool same_op) > > >>> + uint16_t total_enqueued_cbs, bool same_op, > > >>> + struct rte_bbdev_queue_data *q_data) > > >>> { > > >>> int ret; > > >>> if (unlikely(check_bit(op->ldpc_dec.op_flags, > > >>> @@ -3168,6 +3211,12 @@ enqueue_ldpc_dec_one_op_cb(struct > > acc_queue > > >> *q, struct rte_bbdev_dec_op *op, > > >>> } else { > > >>> struct acc_fcw_ld *fcw; > > >>> uint32_t seg_total_left; > > >>> + > > >>> + if (derm_workaround_recommended(&op->ldpc_dec, q)) { > > >>> + RTE_SET_USED(q_data); > > >> Why do we need q_data if it is not being used? > > > > > > As you can guess this notably allows customer do consider running > > > deRM > > from here as a local patch hence keeping the q_data accessible. > > > Basically we keep the prototype of the function > > enqueue_ldpc_dec_one_op_cb() compatible with such usage. > > > I believe this is a good practice here. Let us know if you are not convinced. > > > > I think it is better to just warn the user, without adding unused parameters. > > Thanks Maxime. > Akhil, any additional view on this? For the PMD usage and support this makes > more sense to keep to prototype with q_data even with RTE_SET_USED(). > I don't see any meaningful drawback justifying to push back. Still let us know. > > Thanks > Nic > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-11-03 19:24 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-11-01 4:13 [PATCH v7 0/1] baseband/acc100: changes for 22.11 Hernan Vargas 2022-11-01 4:13 ` [PATCH v7 1/1] baseband/acc100: add detection for deRM corner cases Hernan Vargas 2022-11-03 13:00 ` [EXT] " Akhil Goyal 2022-11-03 14:10 ` Maxime Coquelin 2022-11-03 14:59 ` Chautru, Nicolas 2022-11-03 15:07 ` Maxime Coquelin 2022-11-03 16:20 ` Chautru, Nicolas 2022-11-03 19:24 ` Chautru, Nicolas
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).