From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id A336BA00C5; Wed, 14 Sep 2022 21:35:39 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 592F44021D; Wed, 14 Sep 2022 21:35:39 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 060E340156 for ; Wed, 14 Sep 2022 21:35:37 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1663184137; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ffhs2Rd19PkAkHkjaXTf1hDc9xaM1tqwczvqkn00WoA=; b=YObHbSgKkIJdY2gHWZt2N+RmZiUpHdPW+nQ8sURluqP90Q9aEONduyvKNjloDQ/QmG5gwI DiFx0xwiQIug0YeXRWDmrL+NrjuG4934QRTZ1FtUXLV6YqgmFpGWepI1+lH37WWrdroRWS HXXsMikB790GVOE1/++BIQ3HxdRfnts= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-612-oEWyGwFkOjW0Km3v5KxO2Q-1; Wed, 14 Sep 2022 15:35:36 -0400 X-MC-Unique: oEWyGwFkOjW0Km3v5KxO2Q-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id ED40F3C02590; Wed, 14 Sep 2022 19:35:35 +0000 (UTC) Received: from [10.39.208.12] (unknown [10.39.208.12]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 80EAC1121314; Wed, 14 Sep 2022 19:35:34 +0000 (UTC) Message-ID: <7b356859-afb9-74ce-121e-b661a6b6f4d9@redhat.com> Date: Wed, 14 Sep 2022 21:35:33 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 To: Hernan Vargas , dev@dpdk.org, gakhil@marvell.com, trix@redhat.com Cc: nicolas.chautru@intel.com, qi.z.zhang@intel.com References: <20220820023157.189047-1-hernan.vargas@intel.com> <20220820023157.189047-8-hernan.vargas@intel.com> From: Maxime Coquelin Subject: Re: [PATCH v2 07/37] baseband/acc100: add LDPC encoder padding function In-Reply-To: <20220820023157.189047-8-hernan.vargas@intel.com> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 8/20/22 04:31, Hernan Vargas wrote: > LDPC Encoder input may need to be padded to avoid small beat for ACC100. > Padding 5GDL input buffer length (BLEN) to avoid case (BLEN % 64) <= 8. > Adding protection for corner case to avoid for 5GDL occurrence of last > beat within the ACC100 fabric with <= 8B which might trigger a fabric > corner case hang issue. Again, it looks like a bug fix. > Signed-off-by: Hernan Vargas > --- > drivers/baseband/acc100/rte_acc100_pmd.c | 28 ++++++++++++++---------- > 1 file changed, 16 insertions(+), 12 deletions(-) > > diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c b/drivers/baseband/acc100/rte_acc100_pmd.c > index cc7d146e74..4849d822d1 100644 > --- a/drivers/baseband/acc100/rte_acc100_pmd.c > +++ b/drivers/baseband/acc100/rte_acc100_pmd.c > @@ -1304,7 +1304,6 @@ acc100_fcw_td_fill(const struct rte_bbdev_dec_op *op, struct acc100_fcw_td *fcw) > RTE_BBDEV_TURBO_HALF_ITERATION_EVEN); > } > > -#ifdef RTE_LIBRTE_BBDEV_DEBUG > > static inline bool > is_acc100(struct acc100_queue *q) > @@ -1317,7 +1316,6 @@ validate_op_required(struct acc100_queue *q) > { > return is_acc100(q); > } > -#endif > > /* Fill in a frame control word for LDPC decoding. */ > static inline void > @@ -1773,12 +1771,24 @@ acc100_dma_desc_te_fill(struct rte_bbdev_enc_op *op, > return 0; > } > > +/* May need to pad LDPC Encoder input to avoid small beat for ACC100 */ > +static inline uint16_t > +pad_le_in(uint16_t blen, struct acc100_queue *q) > +{ > + if (!is_acc100(q)) > + return blen; > + uint16_t last_beat = blen % 64; Do not mix variables declaration & code. > + if ((last_beat > 0) && (last_beat <= 8)) > + blen += 8; > + return blen; > +} > + > static inline int > acc100_dma_desc_le_fill(struct rte_bbdev_enc_op *op, > struct acc100_dma_req_desc *desc, struct rte_mbuf **input, > struct rte_mbuf *output, uint32_t *in_offset, > uint32_t *out_offset, uint32_t *out_length, > - uint32_t *mbuf_total_left, uint32_t *seg_total_left) > + uint32_t *mbuf_total_left, uint32_t *seg_total_left, struct acc100_queue *q) > { > int next_triplet = 1; /* FCW already done */ > uint16_t K, in_length_in_bits, in_length_in_bytes; > @@ -1802,8 +1812,7 @@ acc100_dma_desc_le_fill(struct rte_bbdev_enc_op *op, > } > > next_triplet = acc100_dma_fill_blk_type_in(desc, input, in_offset, > - in_length_in_bytes, > - seg_total_left, next_triplet); > + pad_le_in(in_length_in_bytes, q), seg_total_left, next_triplet); > if (unlikely(next_triplet < 0)) { > rte_bbdev_log(ERR, > "Mismatch between data to process and mbuf data length in bbdev_op: %p", > @@ -2531,7 +2540,7 @@ enqueue_ldpc_enc_n_op_cb(struct acc100_queue *q, struct rte_bbdev_enc_op **ops, > acc100_header_init(&desc->req); > desc->req.numCBs = num; > > - in_length_in_bytes = ops[0]->ldpc_enc.input.data->data_len; > + in_length_in_bytes = pad_le_in(ops[0]->ldpc_enc.input.data->data_len, q); > out_length = (enc->cb_params.e + 7) >> 3; > desc->req.m2dlen = 1 + num; > desc->req.d2mlen = num; > @@ -2600,7 +2609,7 @@ enqueue_ldpc_enc_one_op_cb(struct acc100_queue *q, struct rte_bbdev_enc_op *op, > > ret = acc100_dma_desc_le_fill(op, &desc->req, &input, output, > &in_offset, &out_offset, &out_length, &mbuf_total_left, > - &seg_total_left); > + &seg_total_left, q); > > if (unlikely(ret < 0)) > return ret; > @@ -3921,9 +3930,6 @@ dequeue_enc_one_op_cb(struct acc100_queue *q, struct rte_bbdev_enc_op **ref_op, > > /* Clearing status, it will be set based on response */ > op->status = 0; > - > - op->status |= ((rsp.input_err) > - ? (1 << RTE_BBDEV_DATA_ERROR) : 0); Is this really related to the purpose of the patch? > op->status |= ((rsp.dma_err) ? (1 << RTE_BBDEV_DRV_ERROR) : 0); > op->status |= ((rsp.fcw_err) ? (1 << RTE_BBDEV_DRV_ERROR) : 0); > > @@ -3994,8 +4000,6 @@ dequeue_enc_one_op_tb(struct acc100_queue *q, struct rte_bbdev_enc_op **ref_op, > rte_bbdev_log_debug("Resp. desc %p: %x", desc, > rsp.val); > > - op->status |= ((rsp.input_err) > - ? (1 << RTE_BBDEV_DATA_ERROR) : 0); Ditto. > op->status |= ((rsp.dma_err) ? (1 << RTE_BBDEV_DRV_ERROR) : 0); > op->status |= ((rsp.fcw_err) ? (1 << RTE_BBDEV_DRV_ERROR) : 0); >