From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id 5C7DBA00C2;
Tue, 27 Sep 2022 15:56:47 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id EE2EA41133;
Tue, 27 Sep 2022 15:56:46 +0200 (CEST)
Received: from us-smtp-delivery-124.mimecast.com
(us-smtp-delivery-124.mimecast.com [170.10.129.124])
by mails.dpdk.org (Postfix) with ESMTP id 55BA440694
for <dev@dpdk.org>; Tue, 27 Sep 2022 15:56:45 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
s=mimecast20190719; t=1664287004;
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=Ler0guTKEzVD/hsbyuJ8zfXV6+UKO46MZrMLOjasXWc=;
b=hzrycHlYk3XDzti/8UO/s/lRD24/hmWRK8Gd1g97NaGEeAm0pC4McZc7kmVOtNEVS9PDjX
VwwqIywXg1ICSttWxynGF7njOqRW4zBEhBUOQqBwY9rguSmVSk39yRV/cEl/86rtMWvi4q
8dRKZmH6rHWbYSJTarMzknyvW104rMM=
Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com
[66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS
(version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
us-mta-319-6kPya_CvOgurnnnkmcOQKg-1; Tue, 27 Sep 2022 09:56:41 -0400
X-MC-Unique: 6kPya_CvOgurnnnkmcOQKg-1
Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com
[10.11.54.9])
(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
(No client certificate requested)
by mimecast-mx02.redhat.com (Postfix) with ESMTPS id C90EE8027FD;
Tue, 27 Sep 2022 13:56:40 +0000 (UTC)
Received: from [10.39.208.26] (unknown [10.39.208.26])
by smtp.corp.redhat.com (Postfix) with ESMTPS id B83A4492CA2;
Tue, 27 Sep 2022 13:56:38 +0000 (UTC)
Message-ID: <0643ebb8-44c6-06df-5bda-08e4142b1693@redhat.com>
Date: Tue, 27 Sep 2022 15:56:37 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
Thunderbird/102.3.0
Subject: Re: [PATCH v4 13/14] baseband/acc: add PF configure companion function
To: "Chautru, Nicolas" <nicolas.chautru@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>, "thomas@monjalon.net" <thomas@monjalon.net>
Cc: "trix@redhat.com" <trix@redhat.com>, "mdr@ashroe.eu" <mdr@ashroe.eu>,
"Richardson, Bruce" <bruce.richardson@intel.com>,
"hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>,
"david.marchand@redhat.com" <david.marchand@redhat.com>,
"stephen@networkplumber.org" <stephen@networkplumber.org>,
"Vargas, Hernan" <hernan.vargas@intel.com>
References: <1663806460-45162-1-git-send-email-nicolas.chautru@intel.com>
<1663806460-45162-14-git-send-email-nicolas.chautru@intel.com>
<b0c9a7b1-db07-1257-f50f-4f34a9c42c09@redhat.com>
<BY5PR11MB44510F260597D32CCAC279D0F8509@BY5PR11MB4451.namprd11.prod.outlook.com>
From: Maxime Coquelin <maxime.coquelin@redhat.com>
In-Reply-To: <BY5PR11MB44510F260597D32CCAC279D0F8509@BY5PR11MB4451.namprd11.prod.outlook.com>
X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9
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 <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
On 9/24/22 02:20, Chautru, Nicolas wrote:
> Hi Maxime,
> Thanks will send an updated serie now. Much appreciated.
>
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Friday, September 23, 2022 2:26 AM
>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
>> thomas@monjalon.net
>> Cc: trix@redhat.com; mdr@ashroe.eu; Richardson, Bruce
>> <bruce.richardson@intel.com>; hemant.agrawal@nxp.com;
>> david.marchand@redhat.com; stephen@networkplumber.org; Vargas,
>> Hernan <hernan.vargas@intel.com>
>> Subject: Re: [PATCH v4 13/14] baseband/acc: add PF configure companion
>> function
>>
>>
>>
>> On 9/22/22 02:27, Nic Chautru wrote:
>>> Add configure function notably to configure the device from the PF
>>> within DPDK and bbdev-test (without external dependency).
>>>
>>> Signed-off-by: Nic Chautru <nicolas.chautru@intel.com>
>>> ---
>>> app/test-bbdev/test_bbdev_perf.c | 71 ++++++
>>> drivers/baseband/acc/meson.build | 2 +-
>>> drivers/baseband/acc/rte_acc200_cfg.h | 48 ++++
>>> drivers/baseband/acc/rte_acc200_pmd.c | 453
>> ++++++++++++++++++++++++++++++++++
>>> drivers/baseband/acc/version.map | 1 +
>>> 5 files changed, 574 insertions(+), 1 deletion(-)
>>> create mode 100644 drivers/baseband/acc/rte_acc200_cfg.h
>>>
>>> diff --git a/app/test-bbdev/test_bbdev_perf.c
>>> b/app/test-bbdev/test_bbdev_perf.c
>>> index 41c78de..ec6ee2a 100644
>>> --- a/app/test-bbdev/test_bbdev_perf.c
>>> +++ b/app/test-bbdev/test_bbdev_perf.c
>>> @@ -62,6 +62,15 @@
>>> #define ACC100_QMGR_INVALID_IDX -1
>>> #define ACC100_QMGR_RR 1
>>> #define ACC100_QOS_GBR 0
>>> +#include <rte_acc200_cfg.h>
>>> +#define ACC200PF_DRIVER_NAME ("intel_acc200_pf")
>>> +#define ACC200VF_DRIVER_NAME ("intel_acc200_vf")
>>> +#define ACC200_QMGR_NUM_AQS 16
>>> +#define ACC200_QMGR_NUM_QGS 2
>>> +#define ACC200_QMGR_AQ_DEPTH 5
>>> +#define ACC200_QMGR_INVALID_IDX -1
>>> +#define ACC200_QMGR_RR 1
>>> +#define ACC200_QOS_GBR 0
>>> #endif
>>>
>>> #define OPS_CACHE_SIZE 256U
>>> @@ -761,6 +770,68 @@ typedef int (test_case_function)(struct
>> active_device *ad,
>>> "Failed to configure ACC100 PF for bbdev
>> %s",
>>> info->dev_name);
>>> }
>>> + if ((get_init_device() == true) &&
>>> + (!strcmp(info->drv.driver_name, ACC200PF_DRIVER_NAME)))
>> {
>>> + struct rte_acc_conf conf;
>>> + unsigned int i;
>>> +
>>> + printf("Configure ACC200 FEC Driver %s with default
>> values\n",
>>> + info->drv.driver_name);
>>> +
>>> + /* clear default configuration before initialization */
>>> + memset(&conf, 0, sizeof(struct rte_acc_conf));
>>> +
>>> + /* Always set in PF mode for built-in configuration */
>>> + conf.pf_mode_en = true;
>>> + for (i = 0; i < RTE_ACC_NUM_VFS; ++i) {
>>> + conf.arb_dl_4g[i].gbr_threshold1 =
>> ACC200_QOS_GBR;
>>> + conf.arb_dl_4g[i].gbr_threshold1 =
>> ACC200_QOS_GBR;
>>> + conf.arb_dl_4g[i].round_robin_weight =
>> ACC200_QMGR_RR;
>>> + conf.arb_ul_4g[i].gbr_threshold1 =
>> ACC200_QOS_GBR;
>>> + conf.arb_ul_4g[i].gbr_threshold1 =
>> ACC200_QOS_GBR;
>>> + conf.arb_ul_4g[i].round_robin_weight =
>> ACC200_QMGR_RR;
>>> + conf.arb_dl_5g[i].gbr_threshold1 =
>> ACC200_QOS_GBR;
>>> + conf.arb_dl_5g[i].gbr_threshold1 =
>> ACC200_QOS_GBR;
>>> + conf.arb_dl_5g[i].round_robin_weight =
>> ACC200_QMGR_RR;
>>> + conf.arb_ul_5g[i].gbr_threshold1 =
>> ACC200_QOS_GBR;
>>> + conf.arb_ul_5g[i].gbr_threshold1 =
>> ACC200_QOS_GBR;
>>> + conf.arb_ul_5g[i].round_robin_weight =
>> ACC200_QMGR_RR;
>>> + conf.arb_fft[i].gbr_threshold1 = ACC200_QOS_GBR;
>>> + conf.arb_fft[i].gbr_threshold1 = ACC200_QOS_GBR;
>>> + conf.arb_fft[i].round_robin_weight =
>> ACC200_QMGR_RR;
>>> + }
>>> +
>>> + conf.input_pos_llr_1_bit = true;
>>> + conf.output_pos_llr_1_bit = true;
>>> + conf.num_vf_bundles = 1; /**< Number of VF bundles to
>> setup */
>>> +
>>> + conf.q_ul_4g.num_qgroups = ACC200_QMGR_NUM_QGS;
>>> + conf.q_ul_4g.first_qgroup_index =
>> ACC200_QMGR_INVALID_IDX;
>>> + conf.q_ul_4g.num_aqs_per_groups =
>> ACC200_QMGR_NUM_AQS;
>>> + conf.q_ul_4g.aq_depth_log2 = ACC200_QMGR_AQ_DEPTH;
>>> + conf.q_dl_4g.num_qgroups = ACC200_QMGR_NUM_QGS;
>>> + conf.q_dl_4g.first_qgroup_index =
>> ACC200_QMGR_INVALID_IDX;
>>> + conf.q_dl_4g.num_aqs_per_groups =
>> ACC200_QMGR_NUM_AQS;
>>> + conf.q_dl_4g.aq_depth_log2 = ACC200_QMGR_AQ_DEPTH;
>>> + conf.q_ul_5g.num_qgroups = ACC200_QMGR_NUM_QGS;
>>> + conf.q_ul_5g.first_qgroup_index =
>> ACC200_QMGR_INVALID_IDX;
>>> + conf.q_ul_5g.num_aqs_per_groups =
>> ACC200_QMGR_NUM_AQS;
>>> + conf.q_ul_5g.aq_depth_log2 = ACC200_QMGR_AQ_DEPTH;
>>> + conf.q_dl_5g.num_qgroups = ACC200_QMGR_NUM_QGS;
>>> + conf.q_dl_5g.first_qgroup_index =
>> ACC200_QMGR_INVALID_IDX;
>>> + conf.q_dl_5g.num_aqs_per_groups =
>> ACC200_QMGR_NUM_AQS;
>>> + conf.q_dl_5g.aq_depth_log2 = ACC200_QMGR_AQ_DEPTH;
>>> + conf.q_fft.num_qgroups = ACC200_QMGR_NUM_QGS;
>>> + conf.q_fft.first_qgroup_index =
>> ACC200_QMGR_INVALID_IDX;
>>> + conf.q_fft.num_aqs_per_groups =
>> ACC200_QMGR_NUM_AQS;
>>> + conf.q_fft.aq_depth_log2 = ACC200_QMGR_AQ_DEPTH;
>>> +
>>> + /* setup PF with configuration information */
>>> + ret = rte_acc200_configure(info->dev_name, &conf);
>>> + TEST_ASSERT_SUCCESS(ret,
>>> + "Failed to configure ACC200 PF for bbdev
>> %s",
>>> + info->dev_name);
>>> + }
>>> #endif
>>> /* Let's refresh this now this is configured */
>>> rte_bbdev_info_get(dev_id, info);
>>> diff --git a/drivers/baseband/acc/meson.build
>>> b/drivers/baseband/acc/meson.build
>>> index 63912f0..7ae162a 100644
>>> --- a/drivers/baseband/acc/meson.build
>>> +++ b/drivers/baseband/acc/meson.build
>>> @@ -5,4 +5,4 @@ deps += ['bbdev', 'bus_vdev', 'ring', 'pci',
>>> 'bus_pci']
>>>
>>> sources = files('rte_acc100_pmd.c', 'rte_acc200_pmd.c')
>>>
>>> -headers = files('rte_acc100_cfg.h')
>>> +headers = files('rte_acc100_cfg.h', 'rte_acc200_cfg.h')
>>> diff --git a/drivers/baseband/acc/rte_acc200_cfg.h
>>> b/drivers/baseband/acc/rte_acc200_cfg.h
>>> new file mode 100644
>>> index 0000000..8bd9b20
>>> --- /dev/null
>>> +++ b/drivers/baseband/acc/rte_acc200_cfg.h
>>> @@ -0,0 +1,48 @@
>>> +/* SPDX-License-Identifier: BSD-3-Clause
>>> + * Copyright(c) 2022 Intel Corporation */
>>> +
>>> +#ifndef _RTE_ACC200_CFG_H_
>>> +#define _RTE_ACC200_CFG_H_
>>> +
>>> +/**
>>> + * @file rte_acc200_cfg.h
>>> + *
>>> + * Functions for configuring ACC200 HW, exposed directly to applications.
>>> + * Configuration related to encoding/decoding is done through the
>>> + * librte_bbdev library.
>>> + *
>>> + * @warning
>>> + * @b EXPERIMENTAL: this API may change without prior notice */
>>> +
>>> +#include <stdint.h>
>>> +#include <stdbool.h>
>>> +#include "rte_acc_common_cfg.h"
>>> +
>>> +#ifdef __cplusplus
>>> +extern "C" {
>>> +#endif
>>> +
>>> +/**
>>> + * Configure a ACC200 device
>>> + *
>>> + * @param dev_name
>>> + * The name of the device. This is the short form of PCI BDF, e.g. 00:01.0.
>>> + * It can also be retrieved for a bbdev device from the dev_name field in
>> the
>>> + * rte_bbdev_info structure returned by rte_bbdev_info_get().
>>> + * @param conf
>>> + * Configuration to apply to ACC200 HW.
>>> + *
>>> + * @return
>>> + * Zero on success, negative value on failure.
>>> + */
>>> +__rte_experimental
>>> +int
>>> +rte_acc200_configure(const char *dev_name, struct rte_acc_conf
>>> +*conf);
>>> +
>>> +#ifdef __cplusplus
>>> +}
>>> +#endif
>>> +
>>> +#endif /* _RTE_ACC200_CFG_H_ */
>>> diff --git a/drivers/baseband/acc/rte_acc200_pmd.c
>>> b/drivers/baseband/acc/rte_acc200_pmd.c
>>> index 3c2931b..21d614d 100644
>>> --- a/drivers/baseband/acc/rte_acc200_pmd.c
>>> +++ b/drivers/baseband/acc/rte_acc200_pmd.c
>>> @@ -43,6 +43,27 @@
>>>
>>> enum {UL_4G = 0, UL_5G, DL_4G, DL_5G, FFT, NUM_ACC};
>>>
>>> +/* Return the accelerator enum for a Queue Group Index */ static
>>> +inline int accFromQgid(int qg_idx, const struct rte_acc_conf
>>> +*acc_conf) {
>>> + int accQg[ACC200_NUM_QGRPS];
>>> + int NumQGroupsPerFn[NUM_ACC];
>>> + int acc, qgIdx, qgIndex = 0;
>>> + for (qgIdx = 0; qgIdx < ACC200_NUM_QGRPS; qgIdx++)
>>> + accQg[qgIdx] = 0;
>>> + NumQGroupsPerFn[UL_4G] = acc_conf->q_ul_4g.num_qgroups;
>>> + NumQGroupsPerFn[UL_5G] = acc_conf->q_ul_5g.num_qgroups;
>>> + NumQGroupsPerFn[DL_4G] = acc_conf->q_dl_4g.num_qgroups;
>>> + NumQGroupsPerFn[DL_5G] = acc_conf->q_dl_5g.num_qgroups;
>>> + NumQGroupsPerFn[FFT] = acc_conf->q_fft.num_qgroups;
>>> + for (acc = UL_4G; acc < NUM_ACC; acc++)
>>> + for (qgIdx = 0; qgIdx < NumQGroupsPerFn[acc]; qgIdx++)
>>> + accQg[qgIndex++] = acc;
>>> + acc = accQg[qg_idx];
>>> + return acc;
>>> +}
>>> +
>>> /* Return the queue topology for a Queue Group Index */
>>> static inline void
>>> qtopFromAcc(struct rte_acc_queue_topology **qtop, int acc_enum, @@
>>> -75,6 +96,30 @@
>>> *qtop = p_qtop;
>>> }
>>>
>>> +/* Return the AQ depth for a Queue Group Index */ static inline int
>>> +aqDepth(int qg_idx, struct rte_acc_conf *acc_conf) {
>>> + struct rte_acc_queue_topology *q_top = NULL;
>>> + int acc_enum = accFromQgid(qg_idx, acc_conf);
>>> + qtopFromAcc(&q_top, acc_enum, acc_conf);
>>> + if (unlikely(q_top == NULL))
>>> + return 0;
>>> + return q_top->aq_depth_log2;
>>> +}
>>> +
>>> +/* Return the AQ depth for a Queue Group Index */ static inline int
>>> +aqNum(int qg_idx, struct rte_acc_conf *acc_conf) {
>>> + struct rte_acc_queue_topology *q_top = NULL;
>>> + int acc_enum = accFromQgid(qg_idx, acc_conf);
>>> + qtopFromAcc(&q_top, acc_enum, acc_conf);
>>> + if (unlikely(q_top == NULL))
>>> + return 0;
>>> + return q_top->num_aqs_per_groups;
>>> +}
>>
>>
>> For the 3 functions above, please add new lines for clarity.
>
> OK, guidelines would be good, as I am not fully convinced. But updating in new series.
>
I have not checked whether it is in the contribution guidelines, but
having new lines between the variables declarations and code is
a common rules that makes the code more readable. So is having new lines
before the returns.