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?
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
