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 78C30A0C47; Thu, 2 Sep 2021 12:54:47 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EC5DC4003E; Thu, 2 Sep 2021 12:54:46 +0200 (CEST) Received: from mail-il1-f176.google.com (mail-il1-f176.google.com [209.85.166.176]) by mails.dpdk.org (Postfix) with ESMTP id 8D3784003C for ; Thu, 2 Sep 2021 12:54:45 +0200 (CEST) Received: by mail-il1-f176.google.com with SMTP id u7so1305605ilk.7 for ; Thu, 02 Sep 2021 03:54:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=d/M3Fkpvok+NpsQthBS+l/L0fGduZahc63DEd0jI8Jo=; b=HHL7Ssnhj8yAoKDBLkbDJ512SAXVw9rs7ESsz0LfpTIWAVk75DmX0t8M5YnBGz+0DA fYN29ItYKhjMWPXliui3KoCRxMpMySG3eppOK4xU+lJbxDXJ3DxvuOHygiB9M3lnjWZg YF5AWE68fqms+OEUKxcW1ISD7hhygxVL2QD/mLAJq6/DURsd2g95wLRYcgu5M0+g+n5o VmFXoMOmshfyuXm/5SOTw9UJrLAgIpQoilsWfvvIEmJw2DhuwGwaOdDmWRgso1hrPrz5 XnwSeIUudmnpEWRbLGauWIZdSCl8bEjyc3S4IRtL/77WWTbzd6IheiGu+BFHpMtgiSmf /OZg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=d/M3Fkpvok+NpsQthBS+l/L0fGduZahc63DEd0jI8Jo=; b=ir4RD6pPbkIEf5MflA8WXnOc+dsbZ30wwSxJVD9ek52yzJ3BNfgRGn/ntc90EwBiGl uemDie65fEkyaOv/FpML7qLfaiQx7MoJT99ptWFedOwxORlyX1IuLA0xFq6f/9D6LdPV eEGDnoonCB6k9VZCRgPHrUriFzQn8NW+LQrdxms66LBkyJ+zwt6LtzGTFwamSugtmNQB G+/Ok8k5BC1ELbl3IxnRPqsv1HbdNZ3Cp+hMJblZnrZGamDDC9aXlqOsRDvdpuwS7gS+ 35gR0V3EzeWEqousLoybEO5u58GMzMdPFBc/IG2M4+1raXjA/8MtDchB7RdMkHO1ZV7Z Cuxw== X-Gm-Message-State: AOAM530AzoakegA/kw0pH6LKg26aGye5DBkFzidg25FO/W2qolsSJktc xXjzwaAdHQT+T8MmKVtu3kLTH/uM0Ydde7+j85s= X-Google-Smtp-Source: ABdhPJy6rjzYh5jngqg9ZExLx6j07eD/1eS4Hi6TwX69LPX/0cwM5yJWCuqxk1a3LBhQyAmkYTrRLlfbha6sYD9+G1g= X-Received: by 2002:a05:6e02:160f:: with SMTP id t15mr1978330ilu.60.1630580084869; Thu, 02 Sep 2021 03:54:44 -0700 (PDT) MIME-Version: 1.0 References: <20210826183301.333442-1-bruce.richardson@intel.com> <20210901163216.120087-1-bruce.richardson@intel.com> <20210901163216.120087-4-bruce.richardson@intel.com> In-Reply-To: From: Jerin Jacob Date: Thu, 2 Sep 2021 16:24:18 +0530 Message-ID: To: Bruce Richardson Cc: dpdk-dev , "Walsh, Conor" , "Laatz, Kevin" , fengchengwen , Jerin Jacob , Satananda Burla Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH v2 3/6] app/test: add basic dmadev copy tests 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 Sender: "dev" On Thu, Sep 2, 2021 at 1:36 PM Bruce Richardson wrote: > > On Thu, Sep 02, 2021 at 01:14:38PM +0530, Jerin Jacob wrote: > > On Wed, Sep 1, 2021 at 10:02 PM Bruce Richardson > > wrote: > > > > > > For each dmadev instance, perform some basic copy tests to validate that > > > functionality. > > > > > > Signed-off-by: Bruce Richardson > > > --- > > > app/test/test_dmadev.c | 174 +++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 174 insertions(+) > > > > > > diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c > > > index 12f7c69629..261f45db71 100644 > > > --- a/app/test/test_dmadev.c > > > +++ b/app/test/test_dmadev.c > > > @@ -2,12 +2,15 @@ > > > * Copyright(c) 2021 HiSilicon Limited. > > > * Copyright(c) 2021 Intel Corporation. > > > */ > > > +#include > > > #include > > > > > > #include > > > #include > > > #include > > > #include > > > +#include > > > +#include > > > > > > #include "test.h" > > > > > > @@ -16,6 +19,11 @@ extern int test_dmadev_api(uint16_t dev_id); > > > > > > #define PRINT_ERR(...) print_err(__func__, __LINE__, __VA_ARGS__) > > > > > > +#define COPY_LEN 1024 > > > + > > > +static struct rte_mempool *pool; > > > +static uint16_t id_count; > > > + > > > static inline int > > > __rte_format_printf(3, 4) > > > print_err(const char *func, int lineno, const char *format, ...) > > > @@ -31,6 +39,134 @@ print_err(const char *func, int lineno, const char *format, ...) > > > return ret; > > > } > > > > > > +static inline void > > > +await_hw(int dev_id, uint16_t vchan) > > > +{ > > > + int idle = rte_dmadev_vchan_idle(dev_id, vchan); > > > + if (idle < 0) { > > > + /* for drivers that don't support this op, just sleep for 25 microseconds */ > > > + usleep(25); > > > + return; > > > + } > > > > Can following model eliminate the need for rte_dmadev_vchan_idle() API. Right? > > > > static inline bool > > await_hw(int dev_id, uint16_t vchan, uint16_t nb_req, uint16_t *last_idx) > > { > > const uint64_t tmo = rte_get_timer_hz(); > > bool has_error = false; > > > > const uint64_t end_cycles = rte_get_timer_cycles() + tmo; > > while (rte_get_timer_cycles() < end_cycles && nb_req > 0 > > && has_error == false) { > > rte_pause(); > > nb_req -= rte_dmadev_completed(dev_id, > > nb_req, last_idx, &has_error); > > } > > > > return has_error ; > > } > > > It would, but unfortunately it also removes the possibility of doing a > number of the tests in the set, particularly around failure handling. We > used runtime coverage tools to ensure we were hitting as many legs of code > as possible in drivers, and to cover these possibilities we need to do > various different types of completion gathering, e.g. gather multiple > bursts in one go, gathering a single burst in two halves, gathering a burst > using completion_status rather than completion, gathering completions > one-at-a-time with a call for each individually, and for error handling > gathering just the failing element alone, or gathering completions for all > remaining elements not just the failing one, etc. etc. Agree with the rationale. > > These tests are useful both for finding bugs (and they did find ones in our > drivers), but also to ensure similar behaviour across different drivers > using the API. However, they really only can be done in a consistent way if > we are able to ensure that at certain points the hardware has finished > processing before we begin gathering completions. Therefore, having a way > to poll for idle is useful. As you see, I've also left in the delay as a > fallback in case drivers choose not to implement it, thereby making it an > optional API. > > Beyond testing, I can see the API to poll for idleness being useful for the > device shutdown case. I was considering whether the "stop" API should also > use it to ensure that the hardware is idle before stopping. I decided > against it for now, but you could see applications making use of this - > waiting for the hardware to finish its work before stopping it. I think 25us will not be enough, e.s.p If is PCI-Dev to PCI-Dev kind of test cases. Since it is the functional test case, I think, we can keep it a very higher range to support all cases. Maybe 50ms is a good target. > > Regards, > /Bruce