Message ID | 20221012025346.204394-1-hernan.vargas@intel.com (mailing list archive) |
---|---|
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id B7760A0548; Tue, 11 Oct 2022 20:57:22 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A65374282D; Tue, 11 Oct 2022 20:57:22 +0200 (CEST) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by mails.dpdk.org (Postfix) with ESMTP id BDB7740146 for <dev@dpdk.org>; Tue, 11 Oct 2022 20:57:20 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1665514640; x=1697050640; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=h8HxRgohWPeHHfqJuJ7BOCcCNmaXGKLLupCoqYQPdV0=; b=Lv6WG4EXUAlfs00B5iHN+u/JBJBrFuW26ThtKzP00S1ZUsHY0D5Ov0mc rT0WCwHAdoPTZUdxjJnzy9GSdCK7zP0vdezV/svsA/z55aadYlxN8Ikod B5zYvhW+Xf5kGMttPhCU0nOGjFta6DSXelBG0YK1wpv3LgDl3aZEIaAnJ 7gdUCtoU6nhonWH/9q1ihfmC8lwZ1iHxcyhiIdWccwLctUgm+pfiJZxmY 9cXCkdMba3JWCl4SVYDjs+rQKOdhnYPw3YvCxSm8nM1eDCUUKE93mHYoT uIB6NjApF2pntRIm296HZ0u8qo2IVScRei4VIYCIWmozVNkGMngWNNB19 g==; X-IronPort-AV: E=McAfee;i="6500,9779,10497"; a="284981606" X-IronPort-AV: E=Sophos;i="5.95,177,1661842800"; d="scan'208";a="284981606" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Oct 2022 11:57:19 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10497"; a="604261506" X-IronPort-AV: E=Sophos;i="5.95,177,1661842800"; d="scan'208";a="604261506" Received: from unknown (HELO csl-npg-qt0.la.intel.com) ([10.233.181.103]) by orsmga006.jf.intel.com with ESMTP; 11 Oct 2022 11:57:19 -0700 From: Hernan Vargas <hernan.vargas@intel.com> To: dev@dpdk.org, gakhil@marvell.com, trix@redhat.com, maxime.coquelin@redhat.com Cc: nicolas.chautru@intel.com, qi.z.zhang@intel.com, Hernan Vargas <hernan.vargas@intel.com> Subject: [PATCH v3 00/30] baseband/acc100: changes for 22.11 Date: Tue, 11 Oct 2022 19:53:16 -0700 Message-Id: <20221012025346.204394-1-hernan.vargas@intel.com> X-Mailer: git-send-email 2.37.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org |
Series |
baseband/acc100: changes for 22.11
|
|
Message
Hernan Vargas
Oct. 12, 2022, 2:53 a.m. UTC
Upstreaming ACC100 changes for 22.11. This patch series is dependant on series: https://patches.dpdk.org/project/dpdk/list/?series=25041 Hernan Vargas (30): baseband/acc100: fix ring availability calculation baseband/acc100: add function to check AQ availability baseband/acc100: memory leak fix baseband/acc100: add LDPC encoder padding function baseband/acc100: check turbo dec/enc input baseband/acc100: check for unlikely operation vals baseband/acc100: enforce additional check on FCW baseband/acc100: allocate ring/queue mem when NULL baseband/acc100: reduce input length for CRC24B baseband/acc100: fix clearing PF IR outside handler baseband/acc100: set device min alignment to 1 baseband/acc100: add protection for NULL HARQ input baseband/acc100: reset pointer after rte_free baseband/acc100: fix debug print for LDPC FCW baseband/acc100: add enqueue status baseband/acc100: add scatter-gather support baseband/acc100: add HARQ index helper function baseband/acc100: enable input validation by default baseband/acc100: added LDPC transport block support baseband/acc100: update validate LDPC enc/dec baseband/acc100: implement configurable queue depth baseband/acc100: add queue stop operation baseband/acc100: update uplink CB input length baseband/acc100: rename ldpc encode function arg baseband/acc100: update log messages baseband/acc100: store FCW from first CB descriptor baseband/acc100: update device info baseband/acc100: add ring companion address baseband/acc100: add workaround for deRM corner cases baseband/acc100: configure PMON control registers drivers/baseband/acc/acc100_pmd.h | 5 + drivers/baseband/acc/acc_common.h | 10 + drivers/baseband/acc/meson.build | 21 + drivers/baseband/acc/rte_acc100_pmd.c | 1197 ++++++++++++++++++++----- 4 files changed, 1010 insertions(+), 223 deletions(-)
Comments
> Hernan Vargas (30): > baseband/acc100: fix ring availability calculation > baseband/acc100: add function to check AQ availability > baseband/acc100: memory leak fix > baseband/acc100: add LDPC encoder padding function > baseband/acc100: check turbo dec/enc input > baseband/acc100: check for unlikely operation vals > baseband/acc100: enforce additional check on FCW > baseband/acc100: allocate ring/queue mem when NULL > baseband/acc100: reduce input length for CRC24B > baseband/acc100: fix clearing PF IR outside handler > baseband/acc100: set device min alignment to 1 > baseband/acc100: add protection for NULL HARQ input > baseband/acc100: reset pointer after rte_free > baseband/acc100: fix debug print for LDPC FCW > baseband/acc100: add enqueue status > baseband/acc100: add scatter-gather support > baseband/acc100: add HARQ index helper function > baseband/acc100: enable input validation by default > baseband/acc100: added LDPC transport block support > baseband/acc100: update validate LDPC enc/dec > baseband/acc100: implement configurable queue depth > baseband/acc100: add queue stop operation > baseband/acc100: update uplink CB input length > baseband/acc100: rename ldpc encode function arg > baseband/acc100: update log messages > baseband/acc100: store FCW from first CB descriptor > baseband/acc100: update device info > baseband/acc100: add ring companion address > baseband/acc100: add workaround for deRM corner cases > baseband/acc100: configure PMON control registers > > drivers/baseband/acc/acc100_pmd.h | 5 + > drivers/baseband/acc/acc_common.h | 10 + > drivers/baseband/acc/meson.build | 21 + > drivers/baseband/acc/rte_acc100_pmd.c | 1197 ++++++++++++++++++++----- > 4 files changed, 1010 insertions(+), 223 deletions(-) > Please rebase over dpdk-next-crypto TOT
> Hernan Vargas (30): > baseband/acc100: fix ring availability calculation > baseband/acc100: add function to check AQ availability > baseband/acc100: memory leak fix > baseband/acc100: add LDPC encoder padding function > baseband/acc100: check turbo dec/enc input > baseband/acc100: check for unlikely operation vals > baseband/acc100: enforce additional check on FCW > baseband/acc100: allocate ring/queue mem when NULL > baseband/acc100: reduce input length for CRC24B > baseband/acc100: fix clearing PF IR outside handler > baseband/acc100: set device min alignment to 1 > baseband/acc100: add protection for NULL HARQ input > baseband/acc100: reset pointer after rte_free > baseband/acc100: fix debug print for LDPC FCW > baseband/acc100: add enqueue status > baseband/acc100: add scatter-gather support > baseband/acc100: add HARQ index helper function > baseband/acc100: enable input validation by default > baseband/acc100: added LDPC transport block support > baseband/acc100: update validate LDPC enc/dec > baseband/acc100: implement configurable queue depth > baseband/acc100: add queue stop operation > baseband/acc100: update uplink CB input length > baseband/acc100: rename ldpc encode function arg > baseband/acc100: update log messages > baseband/acc100: store FCW from first CB descriptor > baseband/acc100: update device info > baseband/acc100: add ring companion address > baseband/acc100: add workaround for deRM corner cases > baseband/acc100: configure PMON control registers > > drivers/baseband/acc/acc100_pmd.h | 5 + > drivers/baseband/acc/acc_common.h | 10 + > drivers/baseband/acc/meson.build | 21 + > drivers/baseband/acc/rte_acc100_pmd.c | 1197 ++++++++++++++++++++----- > 4 files changed, 1010 insertions(+), 223 deletions(-) > Hi Hernan/Nicolas, I see some ifdefs being used in the code and there is no documentation for them On when and how to enable/disable them. It would be much like a dead code which is not compiled at all, if any of the build target does not enable them. Is it possible to replace them with runtime devargs instead of compile time ifdefs?
Hi Akhil, > -----Original Message----- > From: Akhil Goyal <gakhil@marvell.com> > Sent: Thursday, October 13, 2022 6:02 AM > To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org; > trix@redhat.com; maxime.coquelin@redhat.com > Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z > <qi.z.zhang@intel.com> > Subject: RE: [EXT] [PATCH v3 00/30] baseband/acc100: changes for 22.11 > > > Hernan Vargas (30): > > baseband/acc100: fix ring availability calculation > > baseband/acc100: add function to check AQ availability > > baseband/acc100: memory leak fix > > baseband/acc100: add LDPC encoder padding function > > baseband/acc100: check turbo dec/enc input > > baseband/acc100: check for unlikely operation vals > > baseband/acc100: enforce additional check on FCW > > baseband/acc100: allocate ring/queue mem when NULL > > baseband/acc100: reduce input length for CRC24B > > baseband/acc100: fix clearing PF IR outside handler > > baseband/acc100: set device min alignment to 1 > > baseband/acc100: add protection for NULL HARQ input > > baseband/acc100: reset pointer after rte_free > > baseband/acc100: fix debug print for LDPC FCW > > baseband/acc100: add enqueue status > > baseband/acc100: add scatter-gather support > > baseband/acc100: add HARQ index helper function > > baseband/acc100: enable input validation by default > > baseband/acc100: added LDPC transport block support > > baseband/acc100: update validate LDPC enc/dec > > baseband/acc100: implement configurable queue depth > > baseband/acc100: add queue stop operation > > baseband/acc100: update uplink CB input length > > baseband/acc100: rename ldpc encode function arg > > baseband/acc100: update log messages > > baseband/acc100: store FCW from first CB descriptor > > baseband/acc100: update device info > > baseband/acc100: add ring companion address > > baseband/acc100: add workaround for deRM corner cases > > baseband/acc100: configure PMON control registers > > > > drivers/baseband/acc/acc100_pmd.h | 5 + > > drivers/baseband/acc/acc_common.h | 10 + > > drivers/baseband/acc/meson.build | 21 + > > drivers/baseband/acc/rte_acc100_pmd.c | 1197 > > ++++++++++++++++++++----- > > 4 files changed, 1010 insertions(+), 223 deletions(-) > > > Hi Hernan/Nicolas, > > I see some ifdefs being used in the code and there is no documentation for > them On when and how to enable/disable them. > It would be much like a dead code which is not compiled at all, if any of the > build target does not enable them. > > Is it possible to replace them with runtime devargs instead of compile time > ifdefs? In term of documentation gap, that is a fair point: - I believe the build time variable ACC100_EXT_MEM can be valuable for very specific troubleshoot (not for production) to shortcut the DDR on the card, but good point to document in the acc100.rst. - The RTE_BBDEV_OFFLOAD_COST is used across PMDs for a while. That could also be added to each PMD .rst. - The #ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE is to skip validation of user API which protect from negative scenario and hence save a few cycles by not recommended by default. Can be added to acc100.rst. In term of runtime devargs usage, can we decorrelate this from the series? If we introduce such dev args and expose them more explicitly we would need more updated validation in place, are you okay if we defer such change to next year? If really this is a big concern for you for 22.11 (this is indeed not ideal), we could limit/strip the upstream of the series to the default build configuration only (hence no new #ifdef) but still these options can be genuinely valuable to users (until moved to devargs) so I would prefer to keep as is but with updated documentation in acc100.rst. What do you think? Thanks Nic
> > > Hernan Vargas (30): > > > baseband/acc100: fix ring availability calculation > > > baseband/acc100: add function to check AQ availability > > > baseband/acc100: memory leak fix > > > baseband/acc100: add LDPC encoder padding function > > > baseband/acc100: check turbo dec/enc input > > > baseband/acc100: check for unlikely operation vals > > > baseband/acc100: enforce additional check on FCW > > > baseband/acc100: allocate ring/queue mem when NULL > > > baseband/acc100: reduce input length for CRC24B > > > baseband/acc100: fix clearing PF IR outside handler > > > baseband/acc100: set device min alignment to 1 > > > baseband/acc100: add protection for NULL HARQ input > > > baseband/acc100: reset pointer after rte_free > > > baseband/acc100: fix debug print for LDPC FCW > > > baseband/acc100: add enqueue status > > > baseband/acc100: add scatter-gather support > > > baseband/acc100: add HARQ index helper function > > > baseband/acc100: enable input validation by default > > > baseband/acc100: added LDPC transport block support > > > baseband/acc100: update validate LDPC enc/dec > > > baseband/acc100: implement configurable queue depth > > > baseband/acc100: add queue stop operation > > > baseband/acc100: update uplink CB input length > > > baseband/acc100: rename ldpc encode function arg > > > baseband/acc100: update log messages > > > baseband/acc100: store FCW from first CB descriptor > > > baseband/acc100: update device info > > > baseband/acc100: add ring companion address > > > baseband/acc100: add workaround for deRM corner cases > > > baseband/acc100: configure PMON control registers > > > > > > drivers/baseband/acc/acc100_pmd.h | 5 + > > > drivers/baseband/acc/acc_common.h | 10 + > > > drivers/baseband/acc/meson.build | 21 + > > > drivers/baseband/acc/rte_acc100_pmd.c | 1197 > > > ++++++++++++++++++++----- > > > 4 files changed, 1010 insertions(+), 223 deletions(-) > > > > > Hi Hernan/Nicolas, > > > > I see some ifdefs being used in the code and there is no documentation for > > them On when and how to enable/disable them. > > It would be much like a dead code which is not compiled at all, if any of the > > build target does not enable them. > > > > Is it possible to replace them with runtime devargs instead of compile time > > ifdefs? > > In term of documentation gap, that is a fair point: > - I believe the build time variable ACC100_EXT_MEM can be valuable for very > specific troubleshoot (not for production) to shortcut the DDR on the card, but > good point to document in the acc100.rst. > - The RTE_BBDEV_OFFLOAD_COST is used across PMDs for a while. That could > also be added to each PMD .rst. > - The #ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE is to skip validation of user > API which protect from negative scenario and hence save a few cycles by not > recommended by default. Can be added to acc100.rst. > > In term of runtime devargs usage, can we decorrelate this from the series? If we > introduce such dev args and expose them more explicitly we would need more > updated validation in place, are you okay if we defer such change to next year? > If really this is a big concern for you for 22.11 (this is indeed not ideal), we could > limit/strip the upstream of the series to the default build configuration only > (hence no new #ifdef) but still these options can be genuinely valuable to users > (until moved to devargs) so I would prefer to keep as is but with updated > documentation in acc100.rst. > > What do you think? I am ok to skip devargs for now. Can be added in next cycle. Please update the documentation as needed for compile time flags.