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 63A03A00C5; Wed, 14 Sep 2022 21:57:39 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0B9E24021D; Wed, 14 Sep 2022 21:57:39 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mails.dpdk.org (Postfix) with ESMTP id 5D3C740156 for ; Wed, 14 Sep 2022 21:57:37 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1663185457; x=1694721457; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=vOyPpS6kYX+tifmWwyVOLe5l+MdeawI399y2ntws/4M=; b=Q9EjMMehN6ZuZ7aNKyZYYfJh3oh5EM7uA1JvhwxSXxDhVPR2ck36op70 YD+oA/+5Dm1E0leFnbLWy+e4hjc3l0YgogIiKRXc5DrVl4tvNzavERzOr ctAEy9AZjt8gmccV03yuVGDFGmPK2KWtHoQUBhgjOMManJXa/pJ8+8o2y hUR5/PHfCCixtAmfOk2kK0p2RhpOHMMYQtkoBslWMsakk5ue3Yuy4I9Xg Oi2MpPJJjmXIh1dhg4QZKLVfCLjBgA/gJc2F3qZkhO4o/S3nrBiA072Ss 9v6aqpgzVO2YUv36H9Z51mlPHxU30kX1URXOlM2s4HsgAkmOS5Ni5BezE Q==; X-IronPort-AV: E=McAfee;i="6500,9779,10470"; a="362490258" X-IronPort-AV: E=Sophos;i="5.93,315,1654585200"; d="scan'208";a="362490258" Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Sep 2022 12:57:33 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.93,315,1654585200"; d="scan'208";a="568141424" Received: from fmsmsx603.amr.corp.intel.com ([10.18.126.83]) by orsmga003.jf.intel.com with ESMTP; 14 Sep 2022 12:57:29 -0700 Received: from fmsmsx609.amr.corp.intel.com (10.18.126.89) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Wed, 14 Sep 2022 12:57:29 -0700 Received: from fmsmsx612.amr.corp.intel.com (10.18.126.92) by fmsmsx609.amr.corp.intel.com (10.18.126.89) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Wed, 14 Sep 2022 12:57:28 -0700 Received: from fmsedg602.ED.cps.intel.com (10.1.192.136) by fmsmsx612.amr.corp.intel.com (10.18.126.92) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31 via Frontend Transport; Wed, 14 Sep 2022 12:57:28 -0700 Received: from NAM10-MW2-obe.outbound.protection.outlook.com (104.47.55.104) by edgegateway.intel.com (192.55.55.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2375.31; Wed, 14 Sep 2022 12:57:27 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=GBk/kLsqffQJR4LYlslZMSJD7UjTDOSTKecfc7iRnJndRnWLoyF2AUKyqtsr04hl09eOcl6q0t4soCkJjeWKIEff/AYQC0vegyEhr9Beqbw2tAE3yrXJfFN7gsIqMtXu1i7S/WGb6DFv+6iGlAF/keAAPfJ6UkONFJ1KnJtynAK97DNTov+mXMPWu6dxE5MzPIW3acFnX/eCXHSTgGN+ELOWvopWoM5xEyiUKJH3hisESHYyUm557wQMGQXn1bMxOeO631D2CjAD54duMdfv04bZ9DCbG/zLr4ie4ZGghbdhAK5NtjnNS8M4TnszLFHddw1ZCDzmXaOQaImNpu+5uA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=Ys7Akt9uGjGKlQ3UFIk/C7d5IwLGfzZrOqCu2IfvrBY=; b=Lfc9zCIdgl2DUcpNz3ZvlH8KNAIH0ET8HiwKEK5yY0l4YM1ELF3jo5Gh13nXlZcX42TXL9SvmGcFIcxmy/D8+Zi93+GTrjJ5x7SuHDGrUOUw5ahn0QWr2gw0aGp21J/TIP7fO8euLw/k0KnAm4m6ysCMAnwaHc2ZrCTucNxjLf5kgKdwYjUC2Ua4D8w6FDCbdXMHMtXzFkeW9vwg4+mv2ksYlVDO7J7RNXfnA8URbVsTFnzABQb8kbgEOWMLh6NldGFPCf5CE9sPJJryiVzfm3wJNWoQon9qQTGfkyAV+kt5JKv0uSWSE7Q/m1DfjzQvh1uQ2l8ybsAjwIqpoqPPJg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Received: from BY5PR11MB4451.namprd11.prod.outlook.com (2603:10b6:a03:1cb::30) by DM4PR11MB6167.namprd11.prod.outlook.com (2603:10b6:8:ac::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5588.15; Wed, 14 Sep 2022 19:57:26 +0000 Received: from BY5PR11MB4451.namprd11.prod.outlook.com ([fe80::e4:133a:24c6:cddf]) by BY5PR11MB4451.namprd11.prod.outlook.com ([fe80::e4:133a:24c6:cddf%5]) with mapi id 15.20.5612.022; Wed, 14 Sep 2022 19:57:26 +0000 From: "Chautru, Nicolas" To: Thomas Monjalon , "Richardson, Bruce" , Maxime Coquelin , Akhil Goyal CC: "dev@dpdk.org" , "hemant.agrawal@nxp.com" , "Vargas, Hernan" , Tom Rix , "mdr@ashroe.eu" , "david.marchand@redhat.com" , "stephen@networkplumber.org" Subject: RE: [EXT] Re: [PATCH v1 00/10] baseband/acc200 Thread-Topic: [EXT] Re: [PATCH v1 00/10] baseband/acc200 Thread-Index: AQHYkl/s22Fusf1+10mVxCX8d5jGEK16x2YAgEycnYCAAMeXQIABjqsAgAA0KjCAACA8AIAACLTQgADXEoCAAHDawIAHWsUAgAxsj4CAABTjgIAAGPwAgAAG3QCAAAr3gIAATukQ Date: Wed, 14 Sep 2022 19:57:25 +0000 Message-ID: References: <1657238503-143836-1-git-send-email-nicolas.chautru@intel.com> <2150057.1BCLMh4Saa@thomas> In-Reply-To: <2150057.1BCLMh4Saa@thomas> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.6.500.17 authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: BY5PR11MB4451:EE_|DM4PR11MB6167:EE_ x-ms-office365-filtering-correlation-id: 9d37d3f9-101a-4fcb-9081-08da968b58cb x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: PO0KZU0+Ro3Up76FsoIWP+SZk7yd9yygP3gOkTBmO7ST0PnCiTT+k5oYCBIn1vy9swi2M3nTjHh5Pp7qixwZrqH+zqvEj0f6qGLtMWZp4fMDmOMWkSEXYs69B69UU4cEJNCMPLaHDIRBZy0vMHD4R5mLEynHP1iQ+YSpIq06OWZKvLK9lF4F8jn909yi6XiofjpGlpyMNhNAcCehZweJ8aiA5kmfEFLCcyrWo58ZEloHaaNFWp1z1zTNZ5tzgq6fjrLM7a/G7wGBAGJjte6GamxXOuZxzBCz7qILHVNCpAdK0wRw8/E7RbOBAxOrUCiXuC8JAaDWv23GgLRKtw5qFuXlt1nGVqqsQswkD4iTiqsPUVYxNMNhPMIdbCTqIzwvEX7lEflPkYdm5N1YvtMpY1m5LmsJoF1AszrxP2XFE44BLg6FM9vrrkUYuNSq9xy+HMDYwJ8BaC3UTT7hiLpcXtVnCaFcgHRyWal+JeDvbKewd+7hQkw2lqJ2M3IqPLmm4nKVDxiSb2XngnVm+px2Ekp8l2OLqEKkKlmhjhLZc818+I9cVA460CTqPUSTYLNfBiCLF5oVluqJVZDZwdIwUADzW7MmOSIIf+R35zpuBPRU7OsnZlhAmCMJR23iV5pzTkZEXQy8Cvb6qR5YmvSeulKVWliyUONsI2rhL9R+eKGYRqOVX33x8GV9SBjIQ3R6bYsJy4bhz7SZOdF4D/D4iTNqtndLOBtFm79ba/wDEQauHiv/NXtri9Bp4fZy4NVRkjtW0NYlADIfVNmn7n5Srg== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BY5PR11MB4451.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230022)(39860400002)(396003)(346002)(376002)(366004)(136003)(451199015)(55016003)(2906002)(33656002)(5660300002)(66946007)(38100700002)(8676002)(8936002)(4326008)(64756008)(52536014)(54906003)(66476007)(76116006)(86362001)(110136005)(316002)(478600001)(26005)(9686003)(186003)(82960400001)(66446008)(38070700005)(66556008)(53546011)(83380400001)(122000001)(71200400001)(6506007)(7696005)(41300700001); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?/Q+/3ODEaelm+ePSS39Sg41z9hc2sfgBNG85EsLqLyuMLnAEsolMgTC61Vy/?= =?us-ascii?Q?BsJgJlCENmdpCYg1mBnvMsdclGDMUYOskCecHCRELAoeyJqKiIdgT7R5rJCZ?= =?us-ascii?Q?d/1QHpDTgSj5qAvG7j/rhr6eJK7gWVM8Uv1RMSl+ZHEpjXYepxuI4rlBze4R?= =?us-ascii?Q?2yhyaFE1G+mfJRrXuGL7B/BvreIcpAuvleKo/5NDhU7iaoMkJVSERlsaiGtk?= =?us-ascii?Q?IT7THZGzJwzkYQqPKlXcEAekK1i1cSWqLyFIA3gUFVH+AR/WmZVUsZJphPfN?= =?us-ascii?Q?1LLK8yF5OfLEXpbOEpjNLs8cvAdwGPo+M8609zM19ooRv7S7bAupCtA8pKbg?= =?us-ascii?Q?WFUjOGkNxJ8VdneTM1eCNhPtqKW2j0zb5xMNntniEvj/1GTpzE3KNBFDfvaN?= =?us-ascii?Q?Uj0vIDOU+IKbwp25HM2saxFZeiKDmSrafj7EaTWyQgX/EUN2WH02Vloinyem?= =?us-ascii?Q?UJMdr2KlEqGtYEplPnm0L30h0XrDtqFDBnp22f/Y8DssevwnLNT4iEQHTZCy?= =?us-ascii?Q?y0C39qc0GYSBc6CbhOY6mgoiS+N3LVKAgvqBp56jM6oOf9MJtCEbexOsvzhF?= =?us-ascii?Q?00aWox9DvNjoTQ9TVAZDBrjw+vkQSc0hudfSDYVeEWCj5lWe6AG13HhJQTav?= =?us-ascii?Q?lG9qAaDIHT5mQGr/mjci0aYqM9JzWCnAQqMzF0tKEVOTOZCs1ZveOroJMxTt?= =?us-ascii?Q?U4gIt9c4HKefA14QZ3qkieQHrSm+wkYkxedxbe39dzWFVw+uF7FrlH01y5b/?= =?us-ascii?Q?nN4xjG5MQtUB9oUkWlI5qqiD452d+zm7aZptYSGCL0AJMfjPc5eyg/cHxs8M?= =?us-ascii?Q?e1/6yi1do39OyYiUvFW7p9JMvBI1+FlTy+oYi28kt11hhQOv7K3bO/Fpq9v5?= =?us-ascii?Q?PEJhoZz3nqEri5pdFfzCTaOZUFiIOQVJAqYo+PBcTX8Bx9qVFKIWyGLJR/HD?= =?us-ascii?Q?++9I+HaaMIiN1afhRB83ypPkkS+oEDEKr7muL1n2kDWGL2z50xjPmLWRJ9fp?= =?us-ascii?Q?eK9z6+TEhrHLxZ8PNqKBHumlLz2nzigJHqpi0/YYxY1gpa1aSQ1fxj8/QoAf?= =?us-ascii?Q?oV1KUukyo7LNuhpvAo7Ly3LtzZ9UdUcCMDPqU21RHlo8e2hqHaZgr559z0Ft?= =?us-ascii?Q?2Px7WyZvAnk4LUsonYUpA++s+trx3z/Xx268Cgib+4UIJ9c+xD994xhMSf8x?= =?us-ascii?Q?HsNOfJHXnWHtrDP/VcQqLtw5p3MqkVZfEqhLbO3HivXBCaWQMoRfA+wmSQiN?= =?us-ascii?Q?Lsf/wbaGH6IQ/QAFB+MQdKPgn1BXCTd/05bkCudEQtTAFmU+OvXWs6X4XrJw?= =?us-ascii?Q?d+4dCde4BJqKrmzUpgEwLT0db5Nk8OLscxoV8x2vNk/jAiXlMuB0QG3XZUvR?= =?us-ascii?Q?ID0D7+7sSidm3ywoBU/O/SsNCn6VjSZYCeyXghVtFMPRlZ/RaXdrf5+ECZXA?= =?us-ascii?Q?8LMGFOwm4kcIEvxgtcWapWhU6cd7j68scvLyV/PPMlNvYijz1ctYxi63wRQc?= =?us-ascii?Q?PA0dTlZ+4pUW6T54www0XSj3zzKwKoGlS/xZ6YTbhsk209xVmslA4IoVhCjg?= =?us-ascii?Q?SiMpnjMENTSnUnnHWqDsh90BzeN7fIt3Mf4hFuJT?= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: BY5PR11MB4451.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 9d37d3f9-101a-4fcb-9081-08da968b58cb X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Sep 2022 19:57:26.0234 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: pfNQTyUMME5MeCelnkJ/zOaYTHYcYPQ3dWxlA+1m0bJM3FrI+owRjhgolLH5rSpMtNVtZM8JlYo9P18tMfzH6OoPLC0KUjhjj0iP8Pn3o0U= X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB6167 X-OriginatorOrg: intel.com 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 Hi Thomas, Akhil, Bruce, Maxime,=20 > -----Original Message----- > From: Thomas Monjalon > Sent: Wednesday, September 14, 2022 7:23 AM > To: Richardson, Bruce ; Maxime Coquelin > ; Akhil Goyal ; > Chautru, Nicolas > Cc: dev@dpdk.org; hemant.agrawal@nxp.com; Vargas, Hernan > ; Tom Rix ; mdr@ashroe.eu; > david.marchand@redhat.com; stephen@networkplumber.org > Subject: Re: [EXT] Re: [PATCH v1 00/10] baseband/acc200 >=20 > 14/09/2022 15:44, Akhil Goyal: > > > > On 9/14/22 12:35, Thomas Monjalon wrote: > > > > > 06/09/2022 14:51, Tom Rix: > > > > > > On 9/1/22 1:34 PM, Chautru, Nicolas wrote: > > > > > > > From: Tom Rix > > > > > > > > On 8/31/22 6:26 PM, Chautru, Nicolas wrote: > > > > > > > > > From: Tom Rix > > > > > > > > > > On 8/31/22 3:37 PM, Chautru, Nicolas wrote: > > > > > > > > > > > > > > With regards to the pmd.h, some structure/defines are indeed > > > > > > > common and could be moved to a common file (for instance > > > > > > > turboencoder and LDPC encoder which are more vanilla and > > > > > > > unlikely to change for future product unlike the decoders > > > > > > > which have different feature set and behaviour; or some 3GPP > > > > > > > constant that can be defined once). We can definitely > > > > > > > change these to put together shared structures/defines, but > > > > > > > not intending to try to artificially put things together > > > > > > > with spaghetti code. We would like to keep 3 parallel > > > > > > > versions of these PMD for 3 different product lines which > > > > > > > are indeed fundamentally different designs (including > > > > > > > different workaround required as can be seen on the parallel > > > > > > > ACC100 serie under review). - one version for FPGA > > > > > > > implementation (support for N3000, N6000, ...) - one version > > > > > > > for eASIC lookaside card implementation (ACC100, ACC101, > > > > > > > ...) - one version for the integrated Xeon accelerators > > > > > > > (ACC200, ACC300, ...) > > > > > > > > > > > > Some suggestions on refactoring, > > > > > > > > > > > > For the registers, have a common file. > > > > > > > > > > > > For the shared functionality, ex/ ldpc encoder, break these > > > > > > out to its own shared file. > > > > > > > > > > > > The public interface, see my earlier comments on the > > > > > > documentation, should be have the same interfaces and the few > > > > > > differences highlighted. > > > > > > > > > > +1 to have common files, and all in a single directory > > > > > drivers/baseband/acc100/ > > > > > > > > Jus to be sure we are aligned, do you mean to have both drivers in > > > > the same directory, which will share some common files? That's the > > > > way I would go. > > > > > > > > > > I think the expectation is that the two drivers will diverge in > > > future, so having separate directories should be ok, even with > > > common files placed in one directory are shared with another. With > > > meson include paths its pretty trivial to manage if it's just header > > > files, and even if there are common C files, there is always the > > > option of using drivers/common if we want to split them out. As I > > > understand it, right now it's only headers inluding functions which > > > can be static inline, so simple sharing via include paths should work= fine. > > > > > It can be ok to have 2 separate directories, but > > - is it not possible to have them in same directory say 'acc' for all = affiliated > devices. > > Similar to other vendors' devices (cnxk, i40e, mlx). > > - Can both the devices - acc100 and acc200 coexist? If not, same direct= ory > is good enough. > > - there can be multiple files or directories in 'acc' which can be > > named appropriately to denote the actual device(acc100/200). > > > > Having cross dependency across different drivers of same type looks a k= ind > of hacking the meson. > > This was a reason we moved to have a drivers/common/ for some of the > drivers. > > Also including "../acc100/abc.h" does not look appropriate to me. > > > > IMO, we should not add unnecessary directories when the code is common > and can be managed in a single one. > > > > However, technically it is also ok to have 2 separate directories. > > But, agreeing on this will set a precedence for future next generation > devices from the same vendors. It may be a topic of discussion in techboa= rd. >=20 > Let me be frank, I don't trust Intel saying the hardware will be too much > different in future. Thanks for the review and discussion.=20 Let me clarify, this PMD segregation is specific to ACC1xx vs ACC2xxx. Ther= e is a clear intent to have a common PMD to encompass the future multiple i= ntegrated solutions VRAN accelerators on Xeon (based on ACC200 and future = Xeon products in roadmap) but not for ACC1xx. Here we are splitting the ACC1xx and the ACC2xx series (eASIC process with= off-die PCIe device with on-card DDR vs a straight integrated Xeon acceler= ator) which are fundamentally different devices, and notably the ACC100 req= uiring a lot of SW workaround/mitigations/protections in the code which wou= ld not apply moving forward and would clutter the next generations which wo= uld be managed and optimized largely independently. Basically these are not= just a few registers differences truly.=20 Again future integrated Xeon will shared common driver but always distinct = from ACC1xx (only sharing some common code and structure when possible).=20 Here the refactoring effort was to gather all reusable code and structure t= ogether; which was useful indeed as there are several common functionalitie= s and structures which could be superseded to be shared relatively seamless= ly.=20 > For mlx5, we manage to handle very different devices (like DPU and changi= ng > processors) in a single driver. > So I agree with Maxime and Akhil that a single driver in a single directo= ry > should be enough. > Having different registers in different devices is not enough to split. >=20 > The worst case would be to have a common directory acc/ but it may be a b= it > disappointing. >=20 I believe that I hear 2 different options compatible with the 2 PMDs approa= ch: =20 - The one suggested by Akhil and Maxime I think, is to put both ACC100 and = ACC200 PMDs under ./baseband/acc/ similarly to what is done for cnxk for in= stance. In that case the common files are still all in same directory as th= e 2 PMDs so we don't have do the awkard "includes +=3D include_directories(= '../acc100')" in meson which was frown upon, since everything in already un= der /drivers/baseband/acc.=20 - other option suggested by Thomas to put the shared code and structures un= der ./drivers/common/acc instead of being under ./drivers/acc/acc_common.h = which also used for many drivers.=20 My preference may probably be personally for the former option at the momen= t, but happy to get some form of consensus on this.=20 Thanks and regards,=20 Nic