On Thu, Sep 2, 2021 at 1:36 PM Bruce Richardson <[email protected]> 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 > > <[email protected]> wrote: > > > > > > For each dmadev instance, perform some basic copy tests to validate that > > > functionality. > > > > > > Signed-off-by: Bruce Richardson <[email protected]> > > > --- > > > 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 <unistd.h> > > > #include <inttypes.h> > > > > > > #include <rte_common.h> > > > #include <rte_dev.h> > > > #include <rte_dmadev.h> > > > #include <rte_bus_vdev.h> > > > +#include <rte_mbuf.h> > > > +#include <rte_random.h> > > > > > > #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
