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 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 ; 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 ; 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" , "dev@dpdk.org" , "gakhil@marvell.com" Cc: "thomas@monjalon.net" , "Kinsella, Ray" , "Richardson, Bruce" , "hemant.agrawal@nxp.com" , "Zhang, Mingshan" , "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> From: Tom Rix Message-ID: 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: 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 5/9/22 2:36 PM, Chautru, Nicolas wrote: > >> -----Original Message----- >> From: Tom Rix >> Sent: Sunday, May 8, 2022 6:38 AM >> To: Chautru, Nicolas ; dev@dpdk.org; >> gakhil@marvell.com >> Cc: thomas@monjalon.net; Kinsella, Ray ; Richardson, >> Bruce ; hemant.agrawal@nxp.com; Zhang, >> Mingshan ; 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 >>> --- >>> 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 >>