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 38769A0093;
Tue, 10 May 2022 14:02:14 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id CDB85415D7;
Tue, 10 May 2022 14:02:13 +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 93ED44069D
for <dev@dpdk.org>; Tue, 10 May 2022 14:02:12 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
s=mimecast20190719; t=1652184132;
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=KVol0ooXAc8S6AYCOckcBg2R+CAm+/Sx/3rCmVRiwB4=;
b=ebSOwO8bibNb479cGrpgylHyteXo+a5UJzNLlFiUhp69e2GfAnZv6IBf/2w0+BXdmImXYX
qkdK28KI26r9UOJZVo8QdxAdIMo/lshHo089jxvkThIul+glj/3cf0HJDwiMi3usLBIpGa
ZbTdtjU95PBMaf4ePOv3yUszmdbhtEI=
Received: from mail-qk1-f197.google.com (mail-qk1-f197.google.com
[209.85.222.197]) by relay.mimecast.com with ESMTP with STARTTLS
(version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
us-mta-642--zmw4ApOMkaq3L_ncw9khA-1; Tue, 10 May 2022 08:02:09 -0400
X-MC-Unique: -zmw4ApOMkaq3L_ncw9khA-1
Received: by mail-qk1-f197.google.com with SMTP id
j12-20020ae9c20c000000b0069e8ac6b244so10542059qkg.1
for <dev@dpdk.org>; Tue, 10 May 2022 05:02:09 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20210112;
h=x-gm-message-state:subject:to:cc:references:from:message-id:date
:user-agent:mime-version:in-reply-to:content-transfer-encoding
:content-language;
bh=KVol0ooXAc8S6AYCOckcBg2R+CAm+/Sx/3rCmVRiwB4=;
b=crEadJKkCnnYJJq7Q9mCQn12zBBBuSov1aBa0jTbwg+dDWARTDmH0FBGEMXJZfuvDi
RBP8cXjxks7GSVdMm4tEMPJH1Pq1aULIsqFFrs8F5vrzu341Bf3PNujhML/3aBywrThm
b6a9UTZAe1QMOT42Mtsq3VqTJOBOWXbNg23Ma0FPbu3Ivit2YAE+kLsFCoHravGjGdZ0
x1mOzNgCwKG++C82ubgCL3anpG/b1X1HzdlYWGtmpHtqXLfLW3gmW/u2t+2HoBKqfKDg
SfFPVR/1EA340q1Q7x42HSp9+5gOm64t9Yq6FmD1YV3XT+0qBk25fUkXrb3XJ6HmYP/2
VW3A==
X-Gm-Message-State: AOAM530FapAkrNmUTunmmUIv/MRLtqEvEWpQ6Q39OwVbtkakOAEGDULT
bp1cmYOGTjuEydW8XMvCqZjpmsEe6Ly/JkNKJSoasYf6jA7GItoNh0Y4Ckf4SafmZ86lnfn4uze
DU7E=
X-Received: by 2002:a05:622a:284:b0:2f3:c6aa:6c96 with SMTP id
z4-20020a05622a028400b002f3c6aa6c96mr18842881qtw.512.1652184128462;
Tue, 10 May 2022 05:02:08 -0700 (PDT)
X-Google-Smtp-Source: ABdhPJyYcr8/p5p2EwDhJuixmcnXEbKgrVVVNHuxRQEg7V4w0k+85F7KBjbyLqk5O3SINOhC40Vy7Q==
X-Received: by 2002:a05:622a:284:b0:2f3:c6aa:6c96 with SMTP id
z4-20020a05622a028400b002f3c6aa6c96mr18842835qtw.512.1652184128047;
Tue, 10 May 2022 05:02:08 -0700 (PDT)
Received: from localhost.localdomain (024-205-208-113.res.spectrum.com.
[24.205.208.113]) by smtp.gmail.com with ESMTPSA id
a123-20020ae9e881000000b0069fc13ce227sm8395060qkg.88.2022.05.10.05.02.06
(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);
Tue, 10 May 2022 05:02:07 -0700 (PDT)
Subject: Re: [PATCH v2 3/5] baseband/acc100: configuration of ACC101 from PF
To: "Chautru, Nicolas" <nicolas.chautru@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>, "gakhil@marvell.com" <gakhil@marvell.com>
Cc: "thomas@monjalon.net" <thomas@monjalon.net>,
"Kinsella, Ray" <ray.kinsella@intel.com>,
"Richardson, Bruce" <bruce.richardson@intel.com>,
"hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>,
"Zhang, Mingshan" <mingshan.zhang@intel.com>,
"david.marchand@redhat.com" <david.marchand@redhat.com>
References: <1651083423-33202-1-git-send-email-nicolas.chautru@intel.com>
<1651083423-33202-4-git-send-email-nicolas.chautru@intel.com>
<8384fb18-ce2d-a6e3-2930-c519f3b2139b@redhat.com>
<BY5PR11MB44511BB19E72FB53C2838F64F8C69@BY5PR11MB4451.namprd11.prod.outlook.com>
From: Tom Rix <trix@redhat.com>
Message-ID: <b11a2b28-7242-5a6a-9825-336550034d04@redhat.com>
Date: Tue, 10 May 2022 05:02:04 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
Thunderbird/78.10.1
MIME-Version: 1.0
In-Reply-To: <BY5PR11MB44511BB19E72FB53C2838F64F8C69@BY5PR11MB4451.namprd11.prod.outlook.com>
Authentication-Results: relay.mimecast.com;
auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=trix@redhat.com
X-Mimecast-Spam-Score: 0
X-Mimecast-Originator: redhat.com
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
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 5/9/22 2:36 PM, Chautru, Nicolas wrote:
>
>> -----Original Message-----
>> From: Tom Rix <trix@redhat.com>
>> Sent: Sunday, May 8, 2022 6:38 AM
>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
>> gakhil@marvell.com
>> Cc: thomas@monjalon.net; Kinsella, Ray <ray.kinsella@intel.com>; Richardson,
>> Bruce <bruce.richardson@intel.com>; hemant.agrawal@nxp.com; Zhang,
>> Mingshan <mingshan.zhang@intel.com>; david.marchand@redhat.com
>> Subject: Re: [PATCH v2 3/5] baseband/acc100: configuration of ACC101 from
>> PF
>>
>>
>> On 4/27/22 11:17 AM, Nicolas Chautru wrote:
>>> Adding companion function specific to ACC100 and it can be called from
>>> bbdev-test when running from PF.
>>>
>>> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
>>> ---
>>> app/test-bbdev/test_bbdev_perf.c | 57 ++++++
>>> drivers/baseband/acc100/rte_acc100_cfg.h | 17 ++
>>> drivers/baseband/acc100/rte_acc100_pmd.c | 302
>> +++++++++++++++++++++++++++++++
>>> drivers/baseband/acc100/version.map | 2 +-
>>> 4 files changed, 377 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/app/test-bbdev/test_bbdev_perf.c
>>> b/app/test-bbdev/test_bbdev_perf.c
>>> index 0fa119a..baf5f6d 100644
>>> --- a/app/test-bbdev/test_bbdev_perf.c
>>> +++ b/app/test-bbdev/test_bbdev_perf.c
>>> @@ -63,6 +63,8 @@
>>> #define ACC100_QMGR_INVALID_IDX -1
>>> #define ACC100_QMGR_RR 1
>>> #define ACC100_QOS_GBR 0
>>> +#define ACC101PF_DRIVER_NAME ("intel_acc101_pf")
>>> +#define ACC101VF_DRIVER_NAME ("intel_acc101_vf")
>> A dup from patch 1
>>> #endif
>>>
>>> #define OPS_CACHE_SIZE 256U
>>> @@ -765,6 +767,61 @@ 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, ACC101PF_DRIVER_NAME)))
>> {
>>> + struct rte_acc100_conf conf;
>> Mixing up acc100 and acc101 ?
>>
>> If this actually works, combine the two.
> The configuration file template is the same but not the configuration file. I can combine a bit more that part.
>
>>> + unsigned int i;
>>> +
>>> + printf("Configure ACC101 FEC Driver %s with default values\n",
>>> + info->drv.driver_name);
>>> +
>>> + /* clear default configuration before initialization */
>>> + memset(&conf, 0, sizeof(struct rte_acc100_conf));
>>> +
>>> + /* Always set in PF mode for built-in configuration */
>>> + conf.pf_mode_en = true;
>>> + for (i = 0; i < RTE_ACC100_NUM_VFS; ++i) {
>>> + conf.arb_dl_4g[i].gbr_threshold1 =
>> ACC100_QOS_GBR;
>>> + conf.arb_dl_4g[i].gbr_threshold1 =
>> ACC100_QOS_GBR;
>>> + conf.arb_dl_4g[i].round_robin_weight =
>> ACC100_QMGR_RR;
>>> + conf.arb_ul_4g[i].gbr_threshold1 =
>> ACC100_QOS_GBR;
>>> + conf.arb_ul_4g[i].gbr_threshold1 =
>> ACC100_QOS_GBR;
>>> + conf.arb_ul_4g[i].round_robin_weight =
>> ACC100_QMGR_RR;
>>> + conf.arb_dl_5g[i].gbr_threshold1 =
>> ACC100_QOS_GBR;
>>> + conf.arb_dl_5g[i].gbr_threshold1 =
>> ACC100_QOS_GBR;
>>> + conf.arb_dl_5g[i].round_robin_weight =
>> ACC100_QMGR_RR;
>>> + conf.arb_ul_5g[i].gbr_threshold1 =
>> ACC100_QOS_GBR;
>>> + conf.arb_ul_5g[i].gbr_threshold1 =
>> ACC100_QOS_GBR;
>>> + conf.arb_ul_5g[i].round_robin_weight =
>> ACC100_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 = ACC100_QMGR_NUM_QGS;
>>> + conf.q_ul_4g.first_qgroup_index =
>> ACC100_QMGR_INVALID_IDX;
>>> + conf.q_ul_4g.num_aqs_per_groups =
>> ACC100_QMGR_NUM_AQS;
>>> + conf.q_ul_4g.aq_depth_log2 = ACC100_QMGR_AQ_DEPTH;
>>> + conf.q_dl_4g.num_qgroups = ACC100_QMGR_NUM_QGS;
>>> + conf.q_dl_4g.first_qgroup_index =
>> ACC100_QMGR_INVALID_IDX;
>>> + conf.q_dl_4g.num_aqs_per_groups =
>> ACC100_QMGR_NUM_AQS;
>>> + conf.q_dl_4g.aq_depth_log2 = ACC100_QMGR_AQ_DEPTH;
>>> + conf.q_ul_5g.num_qgroups = ACC100_QMGR_NUM_QGS;
>>> + conf.q_ul_5g.first_qgroup_index =
>> ACC100_QMGR_INVALID_IDX;
>>> + conf.q_ul_5g.num_aqs_per_groups =
>> ACC100_QMGR_NUM_AQS;
>>> + conf.q_ul_5g.aq_depth_log2 = ACC100_QMGR_AQ_DEPTH;
>>> + conf.q_dl_5g.num_qgroups = ACC100_QMGR_NUM_QGS;
>>> + conf.q_dl_5g.first_qgroup_index =
>> ACC100_QMGR_INVALID_IDX;
>>> + conf.q_dl_5g.num_aqs_per_groups =
>> ACC100_QMGR_NUM_AQS;
>>> + conf.q_dl_5g.aq_depth_log2 = ACC100_QMGR_AQ_DEPTH;
>>> +
>>> + /* setup PF with configuration information */
>>> + ret = rte_acc101_configure(info->dev_name, &conf);
>>> + TEST_ASSERT_SUCCESS(ret,
>>> + "Failed to configure ACC101 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/acc100/rte_acc100_cfg.h
>>> b/drivers/baseband/acc100/rte_acc100_cfg.h
>>> index d233e42..2e3c43f 100644
>>> --- a/drivers/baseband/acc100/rte_acc100_cfg.h
>>> +++ b/drivers/baseband/acc100/rte_acc100_cfg.h
>> This file marks its API as experimental though the acc100 has been used in
>> production for some time.
>>
>> It is important that the API be stable.
>>
>> Is this an oversight ?
>>
>> Or what is needed to stabilize the API ?
> This is not part of the BBDEV-API, this is companion function to configure the device notably for bbdev-test. ie. would not be used in live production (ie. we would not run from the PF).
> It could be made non experimental through another patch if desired.
> With regards to the ACC101, this is the new function hence starting as experimental.
If this is not part of the bbdev-api at all and is just test code,
please remove all of the experimental tags.
Such tags are reviewed and would be misinterpreted that the source is
not production ready.
>
>>> @@ -106,6 +106,23 @@ struct rte_acc100_conf {
>>> int
>>> rte_acc100_configure(const char *dev_name, struct rte_acc100_conf
>>> *conf);
>>>
>>> +/**
>>> + * Configure a ACC101 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 ACC101 HW.
>>> + *
>>> + * @return
>>> + * Zero on success, negative value on failure.
>>> + */
>>> +__rte_experimental
>>> +int
>>> +rte_acc101_configure(const char *dev_name, struct rte_acc100_conf
>>> +*conf);
>> I am finding seeing acc100* structs in acc101 function parameters confusing.
>>
>> Maybe a general renaming of acc100 -> acc10x for the common parts.
> Again this is just a companion function to configure the device.
>
>> Will we have this problem on acc120 or acc200 ?
> There is a plan for ACC200 but that is a complete different product and distinct PMD.
>
>> Maybe shorten everything now to acc
>>
>>> +
>>> #ifdef __cplusplus
>>> }
>>> #endif
>>> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
>>> b/drivers/baseband/acc100/rte_acc100_pmd.c
>>> index daf2ce0..b03cedc 100644
>>> --- a/drivers/baseband/acc100/rte_acc100_pmd.c
>>> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
>>> @@ -4921,3 +4921,305 @@ static int acc100_pci_remove(struct
>> rte_pci_device *pci_dev)
>>> rte_bbdev_log_debug("PF Tip configuration complete for %s",
>> dev_name);
>>> return 0;
>>> }
>>> +
>>> +
>>> +/* Initial configuration of a ACC101 device prior to running
>>> +configure() */ int rte_acc101_configure(const char *dev_name, struct
>>> +rte_acc100_conf *conf) {
>> This is very similar to the acc100 configure function.
>>
>> It would be good if these could be combined.
> These should not be combined. The device configuration is distinct and would be artificial to make that function support non compatible register interface.
> Note that this functional is again is not part of PMD.
ok
Tom
>
>> Tom
>>