Hi Nicolas,

On 12/6/22 01:18, Chautru, Nicolas wrote:
Hi Maxime,

-----Original Message-----
From: Maxime Coquelin <maxime.coque...@redhat.com>
Sent: Thursday, November 24, 2022 12:09 PM
To: Chautru, Nicolas <nicolas.chau...@intel.com>; dev@dpdk.org;
tho...@monjalon.net; gak...@marvell.com
Cc: Vargas, Hernan <hernan.var...@intel.com>; vipin.vargh...@amd.com;
Mcnamara, John <john.mcnam...@intel.com>; ferruh.yi...@amd.com;
clinton.fra...@amd.com
Subject: Re: [PATCH v1 2/2] test/bbdev: fix build issue with optional build flag

Hi Nicolas,

On 11/24/22 17:06, Nicolas Chautru wrote:
Missing implementation for offload test with FFT.
Only build when the optional build flag RTE_BBDEV_OFFLOAD_COST is set.

Fixes: 0acdb9866756 ("test/bbdev: add FFT operations cases")

Signed-off-by: Nicolas Chautru <nicolas.chau...@intel.com>
---
   app/test-bbdev/test_bbdev_perf.c | 82
++++++++++++++++++++++++++++++++
   1 file changed, 82 insertions(+)

diff --git a/app/test-bbdev/test_bbdev_perf.c
b/app/test-bbdev/test_bbdev_perf.c
index 1859952901..b2e536b5e3 100644
--- a/app/test-bbdev/test_bbdev_perf.c
+++ b/app/test-bbdev/test_bbdev_perf.c
@@ -4940,6 +4940,88 @@ get_bbdev_queue_stats(uint16_t dev_id,
uint16_t queue_id,
        return 0;
   }

+static int
+offload_latency_test_fft(struct rte_mempool *mempool, struct
test_buffers *bufs,
+               struct rte_bbdev_fft_op *ref_op, uint16_t dev_id,
+               uint16_t queue_id, const uint16_t num_to_process,
+               uint16_t burst_sz, struct test_time_stats *time_st) {
+       int i, dequeued, ret;
+       struct rte_bbdev_fft_op *ops_enq[MAX_BURST],
*ops_deq[MAX_BURST];
+       uint64_t enq_start_time, deq_start_time;
+       uint64_t enq_sw_last_time, deq_last_time;
+       struct rte_bbdev_stats stats;
+
+       for (i = 0, dequeued = 0; dequeued < num_to_process; ++i) {
+               uint16_t enq = 0, deq = 0;
+
+               if (unlikely(num_to_process - dequeued < burst_sz))
+                       burst_sz = num_to_process - dequeued;
+
+               rte_bbdev_fft_op_alloc_bulk(mempool, ops_enq, burst_sz);

It might be safer to check for error.

We don’t check for error on the _op_alloc_bulk() for any of the other offload 
test functions. This could be changed through an independent patchset.
Are you okay to apply as is on the next-baseband subtree? Or you prefer a v2 
with an additional commit now for all these functions?

Both options would be fine to me as long as it is fixed for all the
similar functions.

Thanks,
Maxime


Thanks
Nic


Given how late we are in the release, and also because it is in the test
application, I'm fine if the fix is done in next release.

Other than that, it looks good to me:
Reviewed-by: Maxime Coquelin <maxime.coque...@redhat.com>

Thanks,
Maxime


Reply via email to