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 EE6B1A0032; Thu, 1 Sep 2022 15:49:12 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C977540C35; Thu, 1 Sep 2022 15:49:12 +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 41F3C40695 for ; Thu, 1 Sep 2022 15:49:11 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1662040150; 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=IhmSWd/aybDlgctbV6tH1ae5MQDDRqXuyB/GzvtKJmE=; b=axve0v8CDMFu7PpThMZQVVSSo5flmkqZxeACsTaaGI664KOtuzhHbkrd60y/RAkTHUmIej lkNcGgMJ9Rn2nBloY0VL1IZfm7vWuvDk97ElB0mGoSkJ4HMeAtIiKiJ+0VFXBOBQVRsnq9 FPHM+gaouehKzXIKYujGyo2LZgpvUSU= Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-645-1uzmLq_rOvGVgVeSsg5oBg-1; Thu, 01 Sep 2022 09:49:07 -0400 X-MC-Unique: 1uzmLq_rOvGVgVeSsg5oBg-1 Received: by mail-qk1-f200.google.com with SMTP id bj2-20020a05620a190200b006bba055ab6eso14302880qkb.12 for ; Thu, 01 Sep 2022 06:49:07 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-language:content-transfer-encoding:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject :x-gm-message-state:from:to:cc:subject:date; bh=IhmSWd/aybDlgctbV6tH1ae5MQDDRqXuyB/GzvtKJmE=; b=QYcsZjhJrq4UmeDQefbAcKYBh2adatFJewQUVyTrEinx4qavHBU3c7vndZ0c5dYnKM U0D0HfqTD8R6DYl2z6uxhR3tvTj36d3oX7iifvwYc6ChIee/1TGnkcXz1iQN/e2PCUfG 867bVd4vuoEH0kr/zxwsN9ADPRvs1JqPelVUtV7x8u2dsUKErXRNVSc8HUfBZhLxnkil lkJfEFrww4/2nT7qtYKhSDTd+6U1u+A5T3q9rY8ZcxICVKLls248/tId2SlaDUeaBMcR IPsmSl7Cq7vRhSZFnveWlVEt61okBdMF9++gez0TZ/a3Uh5vHLnFuC5xUl7wYkmSQdEq ZxZA== X-Gm-Message-State: ACgBeo1OiJiGwmqvu9znjgdD1dyXgSEz0KkO4dRcVAeeO0jtfEMKs7jg w70J53YG2iG+4SLAb9d+RPT4i8uCdKEmAXlqUC8JEnQIQS3I6QYeAelpWMa9C98+j67JNYkU9vc rcGE= X-Received: by 2002:a05:622a:507:b0:344:6fcf:78f6 with SMTP id l7-20020a05622a050700b003446fcf78f6mr23703418qtx.415.1662040147290; Thu, 01 Sep 2022 06:49:07 -0700 (PDT) X-Google-Smtp-Source: AA6agR7ifn+fWujXsnfDIlJoxSLaKtpOhObt3i7qHY7WBNIdLBFtEFky0HxUZYcNQONAjkX6RG5knA== X-Received: by 2002:a05:622a:507:b0:344:6fcf:78f6 with SMTP id l7-20020a05622a050700b003446fcf78f6mr23703378qtx.415.1662040146804; Thu, 01 Sep 2022 06:49:06 -0700 (PDT) Received: from localhost.localdomain (024-205-208-113.res.spectrum.com. [24.205.208.113]) by smtp.gmail.com with ESMTPSA id bj15-20020a05620a190f00b006bbc3724affsm11232578qkb.45.2022.09.01.06.49.04 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 01 Sep 2022 06:49:06 -0700 (PDT) Subject: Re: [PATCH v1 00/10] baseband/acc200 To: "Chautru, Nicolas" , Maxime Coquelin , "dev@dpdk.org" , "thomas@monjalon.net" , "gakhil@marvell.com" , "hemant.agrawal@nxp.com" , "Vargas, Hernan" Cc: "mdr@ashroe.eu" , "Richardson, Bruce" , "david.marchand@redhat.com" , "stephen@networkplumber.org" References: <1657238503-143836-1-git-send-email-nicolas.chautru@intel.com> <9087aa5a-6ba8-5df2-8a68-63926843ff7e@redhat.com> <5144a909-7e19-bcda-7bae-89b42fff100c@redhat.com> <85c41e33-ae4b-2b58-ccea-f951eb2bfd69@redhat.com> <5c44b739-883f-0b8c-9fe4-038a665593c1@redhat.com> From: Tom Rix Message-ID: Date: Thu, 1 Sep 2022 06:49:03 -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: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit 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 8/31/22 6:26 PM, Chautru, Nicolas wrote: > Hi Tom, > >> -----Original Message----- >> From: Tom Rix >> Sent: Wednesday, August 31, 2022 5:28 PM >> To: Chautru, Nicolas ; Maxime Coquelin >> ; dev@dpdk.org; thomas@monjalon.net; >> gakhil@marvell.com; hemant.agrawal@nxp.com; Vargas, Hernan >> >> Cc: mdr@ashroe.eu; Richardson, Bruce ; >> david.marchand@redhat.com; stephen@networkplumber.org >> Subject: Re: [PATCH v1 00/10] baseband/acc200 >> >> >> On 8/31/22 3:37 PM, Chautru, Nicolas wrote: >>> Hi Thomas, Tom, >>> >>>> -----Original Message----- >>>> From: Tom Rix >>>> Sent: Wednesday, August 31, 2022 12:26 PM >>>> To: Chautru, Nicolas ; Maxime Coquelin >>>> ; dev@dpdk.org; thomas@monjalon.net; >>>> gakhil@marvell.com; hemant.agrawal@nxp.com; Vargas, Hernan >>>> >>>> Cc: mdr@ashroe.eu; Richardson, Bruce ; >>>> david.marchand@redhat.com; stephen@networkplumber.org >>>> Subject: Re: [PATCH v1 00/10] baseband/acc200 >>>> >>>> >>>> On 8/30/22 12:45 PM, Chautru, Nicolas wrote: >>>>> Hi Maxime, >>>>> >>>>>> -----Original Message----- >>>>>> From: Maxime Coquelin >>>>>> Sent: Tuesday, August 30, 2022 12:45 AM >>>>>> To: Chautru, Nicolas ; dev@dpdk.org; >>>>>> thomas@monjalon.net; gakhil@marvell.com; hemant.agrawal@nxp.com; >>>>>> trix@redhat.com; Vargas, Hernan >>>>>> Cc: mdr@ashroe.eu; Richardson, Bruce ; >>>>>> david.marchand@redhat.com; stephen@networkplumber.org >>>>>> Subject: Re: [PATCH v1 00/10] baseband/acc200 >>>>>> >>>>>> Hi Nicolas, >>>>>> >>>>>> On 7/12/22 15:48, Maxime Coquelin wrote: >>>>>>> Hi Nicolas, Hernan, >>>>>>> >>>>>>> (Adding Hernan in the recipients list) >>>>>>> >>>>>>> On 7/8/22 02:01, Nicolas Chautru wrote: >>>>>>>> This is targeting 22.11 and includes the PMD for the integrated >>>>>>>> accelerator on Intel Xeon SPR-EEC. >>>>>>>> There is a dependency on that parallel serie still in-flight >>>>>>>> which extends the bbdev api >>>>>>>> https://patches.dpdk.org/project/dpdk/list/?series=23894 >>>>>>>> >>>>>>>> I will be offline for a few weeks for the summer break but Hernan >>>>>>>> will cover for me during that time if required. >>>>>>>> >>>>>>>> Thanks >>>>>>>> Nic >>>>>>>> >>>>>>>> Nicolas Chautru (10): >>>>>>>>    baseband/acc200: introduce PMD for ACC200 >>>>>>>>    baseband/acc200: add HW register definitions >>>>>>>>    baseband/acc200: add info get function >>>>>>>>    baseband/acc200: add queue configuration >>>>>>>>    baseband/acc200: add LDPC processing functions >>>>>>>>    baseband/acc200: add LTE processing functions >>>>>>>>    baseband/acc200: add support for FFT operations >>>>>>>>    baseband/acc200: support interrupt >>>>>>>>    baseband/acc200: add device status and vf2pf comms >>>>>>>>    baseband/acc200: add PF configure companion function >>>>>>>> >>>>>>>>   MAINTAINERS                              |    3 + >>>>>>>>   app/test-bbdev/meson.build               |    3 + >>>>>>>>   app/test-bbdev/test_bbdev_perf.c         |   76 + >>>>>>>>   doc/guides/bbdevs/acc200.rst             |  244 ++ >>>>>>>>   doc/guides/bbdevs/index.rst              |    1 + >>>>>>>>   drivers/baseband/acc200/acc200_pf_enum.h |  468 +++ >>>>>>>>   drivers/baseband/acc200/acc200_pmd.h     |  690 ++++ >>>>>>>>   drivers/baseband/acc200/acc200_vf_enum.h |   89 + >>>>>>>>   drivers/baseband/acc200/meson.build      |    8 + >>>>>>>>   drivers/baseband/acc200/rte_acc200_cfg.h |  115 + >>>>>>>>   drivers/baseband/acc200/rte_acc200_pmd.c | 5403 >>>>>>>> ++++++++++++++++++++++++++++++ >>>>>>>>   drivers/baseband/acc200/version.map      |   10 + >>>>>>>>   drivers/baseband/meson.build             |    1 + >>>>>>>>   13 files changed, 7111 insertions(+) >>>>>>>>   create mode 100644 doc/guides/bbdevs/acc200.rst >>>>>>>>   create mode 100644 drivers/baseband/acc200/acc200_pf_enum.h >>>>>>>>   create mode 100644 drivers/baseband/acc200/acc200_pmd.h >>>>>>>>   create mode 100644 drivers/baseband/acc200/acc200_vf_enum.h >>>>>>>>   create mode 100644 drivers/baseband/acc200/meson.build >>>>>>>>   create mode 100644 drivers/baseband/acc200/rte_acc200_cfg.h >>>>>>>>   create mode 100644 drivers/baseband/acc200/rte_acc200_pmd.c >>>>>>>>   create mode 100644 drivers/baseband/acc200/version.map >>>>>>>> >>>>>>> Comparing ACC200 & ACC100 header files, I understand ACC200 is an >>>>>>> evolution of the ACC10x family. The FEC bits are really close, >>>>>>> ACC200 main addition seems to be FFT acceleration which could be >>>>>>> handled in ACC10x driver based on device ID. >>>>>>> >>>>>>> I think both drivers have to be merged in order to avoid code >>>>>>> duplication. That's how other families of devices (e.g. i40e) are >>>>>>> handled. >>>>>> I haven't seen your reply on this point. >>>>>> Do you confirm you are working on a single driver for ACC family in >>>>>> order to avoid code duplication? >>>>>> >>>>> The implementation is based on distinct ACC100 and ACC200 drivers. >>>>> The 2 >>>> devices are fundamentally different generation, processes and IP. >>>>> MountBryce is an eASIC device over PCIe while ACC200 is an >>>>> integrated >>>> accelerator on Xeon CPU. >>>>> The actual implementation are not the same, underlying IP are all >>>>> distinct >>>> even if many of the descriptor format have similarities. >>>>> The actual capabilities of the acceleration are different and/or new. >>>>> The workaround and silicon errata are also different causing >>>>> different >>>> limitation and implementation in the driver (see the serie with >>>> ongoing changes for ACC100 in parallel). >>>>> This is fundamentally distinct from ACC101 which was a derivative >>>>> product >>>> from ACC100 and where it made sense to share implementation between >>>> ACC100 and ACC101. >>>>> So in a nutshell these 2 devices and drivers are 2 different beasts >>>>> and the >>>> intention is to keep them intentionally separate as in the serie. >>>>> Let me know if unclear, thanks! >>>> Nic, >>>> >>>> I used a similarity checker to compare acc100 and acc200 >>>> >>>> https://dickgrune.com/Programs/similarity_tester/ >>>> >>>> l=simum.log >>>> if [ -f $l ]; then >>>>     rm $l >>>> fi >>>> >>>> sim_c -s -R -o$l -R -p -P -a . >>>> >>>> There results are >>>> >>>> ./acc200/acc200_pf_enum.h consists for 100 % of >>>> ./acc100/acc100_pf_enum.h material ./acc100/acc100_pf_enum.h consists >>>> for 98 % of ./acc200/acc200_pf_enum.h material >>>> ./acc100/rte_acc100_pmd.h consists for >>>> 98 % of ./acc200/acc200_pmd.h material ./acc200/acc200_vf_enum.h >>>> consists for 95 % of ./acc100/acc100_pf_enum.h material >>>> ./acc200/acc200_pmd.h consists for 92 % of ./acc100/rte_acc100_pmd.h >>>> material ./acc200/rte_acc200_cfg.h consists for 92 % of >>>> ./acc100/rte_acc100_cfg.h material ./acc100/rte_acc100_pmd.c consists >>>> for 87 % of ./acc200/rte_acc200_pmd.c material >>>> ./acc100/acc100_vf_enum.h consists for >>>> 80 % of ./acc200/acc200_pf_enum.h material ./acc200/rte_acc200_pmd.c >>>> consists for 78 % of ./acc100/rte_acc100_pmd.c material >>>> ./acc100/rte_acc100_cfg.h consists for 75 % of >>>> ./acc200/rte_acc200_cfg.h material >>>> >>>> Spot checking the first *pf_enum.h at 100%, these are the devices' >>>> registers, they are the same. >>>> >>>> I raised this similarity issue with 100 vs 101. >>>> >>>> Having multiple copies is difficult to support and should be avoided. >>>> >>>> For the end user, they should have to use only one driver. >>>> >>> There are really different IP and do not have the same interface (PCIe/DDR vs >> integrated) and there is big serie of changes which are specific to ACC100 >> coming in parallel. Any workaround, optimization would be different. >>> I agree that for the coming serie of integrated accelerator we will use a >> unified driver approach but for that very case that would be quite messy to >> artificially put them within the same PMD. >> >> How is the IP different when 100% of the registers are the same ? >> > These are 2 different HW aspects. The base toplevel configuration registers are kept similar on purpose but the underlying IP are totally different design and implementation. > Even the registers have differences but not visible here, the actual RDL file would define more specifically these registers bitfields and implementation including which ones are not implemented (but that is proprietary information), and at bbdev level the interface is not some much register based than processing based on data from DMA. > Basically even if there was a common driver, all these would be duplicated and they are indeed different IP (including different vendors).. > But I agree with the general intent and to have a common driver for the integrated driver serie (ACC200, ACC300...) now that we are moving away from PCIe/DDR lookaside acceleration and eASIC/FPGA implementation (ACC100/AC101). Looking a little deeper, at how the driver is lays out some of its bitfields and private data by reviewing the ./acc200/acc200_pmd.h consists for 92 % of ./acc100/rte_acc100_pmd.h There are some minor changes to existing reserved bitfields. A new structure for fft. The acc200_device, the private data for the driver, is an exact copy of acc100_device. acc200_pmd.h is the superset and could be used with little changes as a common acc_pmd.h. acc200 is doing everything the acc100 did in a very similar if not exact way, adding the fft feature. Can you point to some portion of this patchset that is so unique that it could not be abstracted to an if-check or function and so requiring this separate, nearly identical driver ? Tom >> Tom >> >>>