From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6CAC7C433EF for ; Thu, 19 May 2022 20:13:57 +0000 (UTC) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4A6D140223; Thu, 19 May 2022 22:13:56 +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 6302D40156 for ; Thu, 19 May 2022 22:13:55 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1652991234; 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=2WkWDMqnkW0AUHeu57Fy6zGaq5vqkaxfwnlmBddr/XY=; b=Bp120FjQckI1jZ5hEwL6u5A8t8oqzQE69FIykkqzt0SbgyPhZ8jh+6f6eURX69rH9OYx+q h0Jq1dRuf0x4q43jBGjt2fCR4Rxo+XEmKrVAilJcJiuSOhKeDLVGa3Kr2tCZrRQXA0T3YV Y2HmRi/hBlIbc0KCs0JUMQatwh5+Ir4= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-664-8Cku93PKOeqOgYuvLBL8sw-1; Thu, 19 May 2022 16:13:49 -0400 X-MC-Unique: 8Cku93PKOeqOgYuvLBL8sw-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 3251F29AA2F0; Thu, 19 May 2022 20:13:49 +0000 (UTC) Received: from [10.39.208.34] (unknown [10.39.208.34]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 27B10492C14; Thu, 19 May 2022 20:13:47 +0000 (UTC) Message-ID: Date: Thu, 19 May 2022 22:13:45 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [PATCH v3 3/4] baseband/acc100: configuration of ACC101 from PF To: Nicolas Chautru , dev@dpdk.org, gakhil@marvell.com, trix@redhat.com Cc: thomas@monjalon.net, ray.kinsella@intel.com, bruce.richardson@intel.com, hemant.agrawal@nxp.com, hernan.vargas@intel.com, david.marchand@redhat.com References: <1651083423-33202-1-git-send-email-nicolas.chautru@intel.com> <1652734113-124047-1-git-send-email-nicolas.chautru@intel.com> <1652734113-124047-4-git-send-email-nicolas.chautru@intel.com> From: Maxime Coquelin In-Reply-To: <1652734113-124047-4-git-send-email-nicolas.chautru@intel.com> X-Scanned-By: MIMEDefang 2.85 on 10.11.54.10 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=maxime.coquelin@redhat.com 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Hi Nicolas, On 5/16/22 22:48, 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 | 22 ++- > 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, 336 insertions(+), 7 deletions(-) > > diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c > index 0fa119a..b10b93d 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") > #endif > > #define OPS_CACHE_SIZE 256U > @@ -711,11 +713,12 @@ typedef int (test_case_function)(struct active_device *ad, > #endif > #ifdef RTE_BASEBAND_ACC100 > if ((get_init_device() == true) && > - (!strcmp(info->drv.driver_name, ACC100PF_DRIVER_NAME))) { > + ((!strcmp(info->drv.driver_name, ACC100PF_DRIVER_NAME)) || > + (!strcmp(info->drv.driver_name, ACC101PF_DRIVER_NAME)))) { > struct rte_acc100_conf conf; > unsigned int i; > > - printf("Configure ACC100 FEC Driver %s with default values\n", > + printf("Configure ACC10X FEC Driver %s with default values\n", > info->drv.driver_name); > > /* clear default configuration before initialization */ > @@ -760,10 +763,17 @@ typedef int (test_case_function)(struct active_device *ad, > conf.q_dl_5g.aq_depth_log2 = ACC100_QMGR_AQ_DEPTH; > > /* setup PF with configuration information */ > - ret = rte_acc100_configure(info->dev_name, &conf); > - TEST_ASSERT_SUCCESS(ret, > - "Failed to configure ACC100 PF for bbdev %s", > - info->dev_name); > + if (!strcmp(info->drv.driver_name, ACC100PF_DRIVER_NAME)) { > + ret = rte_acc100_configure(info->dev_name, &conf); > + TEST_ASSERT_SUCCESS(ret, > + "Failed to configure ACC100 PF for bbdev %s", > + info->dev_name); > + } else { > + 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 */ > 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 > @@ -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); > + > #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) > +{ Please don't introduce new API for every new variant. You should have a single API, and within it call specific configuration function based on the device ID. It will be easier for the application which calls it not to have to know which variant it is for. > + rte_bbdev_log(INFO, "rte_acc101_configure"); > + uint32_t value, address, status; > + int qg_idx, template_idx, vf_idx, acc, i; > + struct rte_bbdev *bbdev = rte_bbdev_get_named_dev(dev_name); > + > + /* Compile time checks */ > + RTE_BUILD_BUG_ON(sizeof(struct acc100_dma_req_desc) != 256); > + RTE_BUILD_BUG_ON(sizeof(union acc100_dma_desc) != 256); > + RTE_BUILD_BUG_ON(sizeof(struct acc100_fcw_td) != 24); > + RTE_BUILD_BUG_ON(sizeof(struct acc100_fcw_te) != 32); > + > + if (bbdev == NULL) { > + rte_bbdev_log(ERR, > + "Invalid dev_name (%s), or device is not yet initialised", > + dev_name); > + return -ENODEV; > + } > + struct acc100_device *d = bbdev->data->dev_private; > + > + /* Store configuration */ > + rte_memcpy(&d->acc100_conf, conf, sizeof(d->acc100_conf)); > + > + /* PCIe Bridge configuration */ > + acc100_reg_write(d, HwPfPcieGpexBridgeControl, ACC101_CFG_PCI_BRIDGE); > + for (i = 1; i < ACC101_GPEX_AXIMAP_NUM; i++) > + acc100_reg_write(d, HwPfPcieGpexAxiAddrMappingWindowPexBaseHigh + i * 16, 0); > + > + /* Prevent blocking AXI read on BRESP for AXI Write */ > + address = HwPfPcieGpexAxiPioControl; > + value = ACC101_CFG_PCI_AXI; > + acc100_reg_write(d, address, value); > + > + /* Explicitly releasing AXI as this may be stopped after PF FLR/BME */ > + usleep(2000); This sleep looks weird, especially when ACC100 does not require it. Isn't there a way to know it is ready to release the AXI? Or at least document why it is necessary. > + acc100_reg_write(d, HWPfDmaAxiControl, 1); > + > + /* Set the default 5GDL DMA configuration */ > + acc100_reg_write(d, HWPfDmaInboundDrainDataSize, ACC101_DMA_INBOUND); > + > + /* Enable granular dynamic clock gating */ > + address = HWPfHiClkGateHystReg; > + value = ACC101_CLOCK_GATING_EN; > + acc100_reg_write(d, address, value); > + > + /* Set default descriptor signature */ > + address = HWPfDmaDescriptorSignatuture; > + value = 0; > + acc100_reg_write(d, address, value); > + > + /* Enable the Error Detection in DMA */ > + value = ACC101_CFG_DMA_ERROR; > + address = HWPfDmaErrorDetectionEn; > + acc100_reg_write(d, address, value); > + > + /* AXI Cache configuration */ > + value = ACC101_CFG_AXI_CACHE; > + address = HWPfDmaAxcacheReg; > + acc100_reg_write(d, address, value); > + > + /* Default DMA Configuration (Qmgr Enabled) */ > + address = HWPfDmaConfig0Reg; > + value = 0; > + acc100_reg_write(d, address, value); > + address = HWPfDmaQmanen; > + value = 0; > + acc100_reg_write(d, address, value); > + > + /* Default RLIM/ALEN configuration */ > + address = HWPfDmaConfig1Reg; > + int alen_r = 0xF; > + int alen_w = 0x7; > + value = (1 << 31) + (alen_w << 20) + (1 << 6) + alen_r; > + acc100_reg_write(d, address, value); > + > + /* Configure DMA Qmanager addresses */ > + address = HWPfDmaQmgrAddrReg; > + value = HWPfQmgrEgressQueuesTemplate; > + acc100_reg_write(d, address, value); > + > + /* ===== Qmgr Configuration ===== */ > + /* Configuration of the AQueue Depth QMGR_GRP_0_DEPTH_LOG2 for UL */ > + int totalQgs = conf->q_ul_4g.num_qgroups + > + conf->q_ul_5g.num_qgroups + > + conf->q_dl_4g.num_qgroups + > + conf->q_dl_5g.num_qgroups; > + for (qg_idx = 0; qg_idx < totalQgs; qg_idx++) { > + address = HWPfQmgrDepthLog2Grp + > + ACC101_BYTES_IN_WORD * qg_idx; > + value = aqDepth(qg_idx, conf); > + acc100_reg_write(d, address, value); > + address = HWPfQmgrTholdGrp + > + ACC101_BYTES_IN_WORD * qg_idx; > + value = (1 << 16) + (1 << (aqDepth(qg_idx, conf) - 1)); > + acc100_reg_write(d, address, value); > + } > + > + /* Template Priority in incremental order */ > + for (template_idx = 0; template_idx < ACC101_NUM_TMPL; > + template_idx++) { > + address = HWPfQmgrGrpTmplateReg0Indx + ACC101_BYTES_IN_WORD * template_idx; > + value = ACC101_TMPL_PRI_0; > + acc100_reg_write(d, address, value); > + address = HWPfQmgrGrpTmplateReg1Indx + ACC101_BYTES_IN_WORD * template_idx; > + value = ACC101_TMPL_PRI_1; > + acc100_reg_write(d, address, value); > + address = HWPfQmgrGrpTmplateReg2indx + ACC101_BYTES_IN_WORD * template_idx; > + value = ACC101_TMPL_PRI_2; > + acc100_reg_write(d, address, value); > + address = HWPfQmgrGrpTmplateReg3Indx + ACC101_BYTES_IN_WORD * template_idx; > + value = ACC101_TMPL_PRI_3; > + acc100_reg_write(d, address, value); For both ACC100 and ACC101, ACC10x_NUM_TMPL is 32. Isn't there a bug here or in ACC100? ACC100 performs a modulo 8 on template_idx, but ACC101 does not. But the registers mapping is the same, so I guess this is the same HW block. I think ACC100 is the buggy one, because it seems weird to write the same register with the same value multiple times. In case it is ACC100 that is buggy, it needs to be fixed in a dedicated patch, with proper fixes tag so that it is backported. > + } > + > + address = HWPfQmgrGrpPriority; > + value = ACC101_CFG_QMGR_HI_P; > + acc100_reg_write(d, address, value); > + > + /* Template Configuration */ > + for (template_idx = 0; template_idx < ACC101_NUM_TMPL; > + template_idx++) { > + value = 0; > + address = HWPfQmgrGrpTmplateReg4Indx > + + ACC101_BYTES_IN_WORD * template_idx; > + acc100_reg_write(d, address, value); > + } > + /* 4GUL */ > + int numQgs = conf->q_ul_4g.num_qgroups; > + int numQqsAcc = 0; > + value = 0; > + for (qg_idx = numQqsAcc; qg_idx < (numQgs + numQqsAcc); qg_idx++) > + value |= (1 << qg_idx); > + for (template_idx = ACC101_SIG_UL_4G; > + template_idx <= ACC101_SIG_UL_4G_LAST; > + template_idx++) { > + address = HWPfQmgrGrpTmplateReg4Indx > + + ACC101_BYTES_IN_WORD * template_idx; > + acc100_reg_write(d, address, value); > + } > + /* 5GUL */ > + numQqsAcc += numQgs; > + numQgs = conf->q_ul_5g.num_qgroups; > + value = 0; > + int numEngines = 0; > + for (qg_idx = numQqsAcc; qg_idx < (numQgs + numQqsAcc); qg_idx++) > + value |= (1 << qg_idx); > + for (template_idx = ACC101_SIG_UL_5G; > + template_idx <= ACC101_SIG_UL_5G_LAST; > + template_idx++) { > + /* Check engine power-on status */ > + address = HwPfFecUl5gIbDebugReg + > + ACC101_ENGINE_OFFSET * template_idx; > + status = (acc100_reg_read(d, address) >> 4) & 0xF; > + address = HWPfQmgrGrpTmplateReg4Indx > + + ACC101_BYTES_IN_WORD * template_idx; > + if (status == 1) { > + acc100_reg_write(d, address, value); > + numEngines++; > + } else > + acc100_reg_write(d, address, 0); > +#if RTE_ACC101_SINGLE_FEC == 1 > + value = 0; > +#endif > + } > + printf("Number of 5GUL engines %d\n", numEngines); > + /* 4GDL */ > + numQqsAcc += numQgs; > + numQgs = conf->q_dl_4g.num_qgroups; > + value = 0; > + for (qg_idx = numQqsAcc; qg_idx < (numQgs + numQqsAcc); qg_idx++) > + value |= (1 << qg_idx); > + for (template_idx = ACC101_SIG_DL_4G; > + template_idx <= ACC101_SIG_DL_4G_LAST; > + template_idx++) { > + address = HWPfQmgrGrpTmplateReg4Indx > + + ACC101_BYTES_IN_WORD * template_idx; > + acc100_reg_write(d, address, value); > +#if RTE_ACC101_SINGLE_FEC == 1 > + value = 0; > +#endif > + } > + /* 5GDL */ > + numQqsAcc += numQgs; > + numQgs = conf->q_dl_5g.num_qgroups; > + value = 0; > + for (qg_idx = numQqsAcc; qg_idx < (numQgs + numQqsAcc); qg_idx++) > + value |= (1 << qg_idx); > + for (template_idx = ACC101_SIG_DL_5G; > + template_idx <= ACC101_SIG_DL_5G_LAST; > + template_idx++) { > + address = HWPfQmgrGrpTmplateReg4Indx > + + ACC101_BYTES_IN_WORD * template_idx; > + acc100_reg_write(d, address, value); > +#if RTE_ACC101_SINGLE_FEC == 1 > + value = 0; > +#endif > + } > + > + /* Queue Group Function mapping */ > + int qman_func_id[8] = {0, 2, 1, 3, 4, 0, 0, 0}; > + address = HWPfQmgrGrpFunction0; > + value = 0; > + for (qg_idx = 0; qg_idx < 8; qg_idx++) { > + acc = accFromQgid(qg_idx, conf); > + value |= qman_func_id[acc]<<(qg_idx * 4); > + } > + acc100_reg_write(d, address, value); > + > + /* Configuration of the Arbitration QGroup depth to 1 */ > + for (qg_idx = 0; qg_idx < totalQgs; qg_idx++) { > + address = HWPfQmgrArbQDepthGrp + > + ACC101_BYTES_IN_WORD * qg_idx; > + value = 0; > + acc100_reg_write(d, address, value); > + } > + > + /* Enabling AQueues through the Queue hierarchy*/ > + for (vf_idx = 0; vf_idx < ACC101_NUM_VFS; vf_idx++) { > + for (qg_idx = 0; qg_idx < ACC101_NUM_QGRPS; qg_idx++) { > + value = 0; > + if (vf_idx < conf->num_vf_bundles && > + qg_idx < totalQgs) > + value = (1 << aqNum(qg_idx, conf)) - 1; > + address = HWPfQmgrAqEnableVf > + + vf_idx * ACC101_BYTES_IN_WORD; > + value += (qg_idx << 16); > + acc100_reg_write(d, address, value); > + } > + } > + > + /* This pointer to ARAM (128kB) is shifted by 2 (4B per register) */ > + uint32_t aram_address = 0; > + for (qg_idx = 0; qg_idx < totalQgs; qg_idx++) { > + for (vf_idx = 0; vf_idx < conf->num_vf_bundles; vf_idx++) { > + address = HWPfQmgrVfBaseAddr + vf_idx > + * ACC101_BYTES_IN_WORD + qg_idx > + * ACC101_BYTES_IN_WORD * 64; > + value = aram_address; > + acc100_reg_write(d, address, value); > + /* Offset ARAM Address for next memory bank > + * - increment of 4B > + */ > + aram_address += aqNum(qg_idx, conf) * > + (1 << aqDepth(qg_idx, conf)); > + } > + } > + > + if (aram_address > ACC101_WORDS_IN_ARAM_SIZE) { > + rte_bbdev_log(ERR, "ARAM Configuration not fitting %d %d\n", > + aram_address, ACC101_WORDS_IN_ARAM_SIZE); > + return -EINVAL; > + } > + > + /* ==== HI Configuration ==== */ > + > + /* No Info Ring/MSI by default */ > + acc100_reg_write(d, HWPfHiInfoRingIntWrEnRegPf, 0); > + acc100_reg_write(d, HWPfHiInfoRingVf2pfLoWrEnReg, 0); > + acc100_reg_write(d, HWPfHiCfgMsiIntWrEnRegPf, 0xFFFFFFFF); > + acc100_reg_write(d, HWPfHiCfgMsiVf2pfLoWrEnReg, 0xFFFFFFFF); > + /* Prevent Block on Transmit Error */ > + address = HWPfHiBlockTransmitOnErrorEn; > + value = 0; > + acc100_reg_write(d, address, value); > + /* Prevents to drop MSI */ > + address = HWPfHiMsiDropEnableReg; > + value = 0; > + acc100_reg_write(d, address, value); > + /* Set the PF Mode register */ > + address = HWPfHiPfMode; > + value = (conf->pf_mode_en) ? ACC101_PF_VAL : 0; > + acc100_reg_write(d, address, value); > + /* Explicitly releasing AXI after PF Mode and 2 ms */ > + usleep(2000); > + acc100_reg_write(d, HWPfDmaAxiControl, 1); > + > + /* QoS overflow init */ > + value = 1; > + address = HWPfQosmonAEvalOverflow0; > + acc100_reg_write(d, address, value); > + address = HWPfQosmonBEvalOverflow0; > + acc100_reg_write(d, address, value); > + > + /* HARQ DDR Configuration */ > + unsigned int ddrSizeInMb = ACC101_HARQ_DDR; > + for (vf_idx = 0; vf_idx < conf->num_vf_bundles; vf_idx++) { > + address = HWPfDmaVfDdrBaseRw + vf_idx > + * 0x10; > + value = ((vf_idx * (ddrSizeInMb / 64)) << 16) + > + (ddrSizeInMb - 1); > + acc100_reg_write(d, address, value); > + } > + usleep(ACC101_LONG_WAIT); > + > + rte_bbdev_log_debug("PF TIP configuration complete for %s", dev_name); > + return 0; > +} > diff --git a/drivers/baseband/acc100/version.map b/drivers/baseband/acc100/version.map > index 40604c7..37b850f 100644 > --- a/drivers/baseband/acc100/version.map > +++ b/drivers/baseband/acc100/version.map > @@ -6,5 +6,5 @@ EXPERIMENTAL { > global: > > rte_acc100_configure; > - > + rte_acc101_configure; > };