Re: [PATCH v5 01/12] bbdev: add FFT window width member in driver info

[Maxime Coquelin]

On 10/6/23 22:08, Chautru, Nicolas wrote:

Hi Maxime,

-----Original Message-----
From: Maxime Coquelin <maxime.coque...@redhat.com>
Sent: Friday, October 6, 2023 12:54 AM
To: Chautru, Nicolas <nicolas.chau...@intel.com>; dev@dpdk.org
Cc: hemant.agra...@nxp.com; david.march...@redhat.com; Vargas, Hernan
<hernan.var...@intel.com>
Subject: Re: [PATCH v5 01/12] bbdev: add FFT window width member in driver
info



On 10/5/23 21:48, Nicolas Chautru wrote:

This exposes the width of each windowing shape being configured on the
device. This allows to distinguish different version of the flexible
pointwise windowing applied to the FFT and expose this platform
configuration to the application.
This also add the total number of windows supported in the capability.

The SRS processing chain
(https://doc.dpdk.org/guides/prog_guide/bbdev.html#bbdev-fft-operation
) includes a pointwise multiplication by time window whose shape width
needs to be exposed, notably for accurate SNR estimate.
Using that mechanism user application can retrieve information related
to what has been dynamically programmed on any bbdev device supporting
FFT windowing operation.

Signed-off-by: Nicolas Chautru <nicolas.chau...@intel.com>
---
   lib/bbdev/rte_bbdev.h    | 2 ++
   lib/bbdev/rte_bbdev_op.h | 2 ++
   2 files changed, 4 insertions(+)

diff --git a/lib/bbdev/rte_bbdev.h b/lib/bbdev/rte_bbdev.h index
2985c9f42b..d12e2e7fbc 100644
--- a/lib/bbdev/rte_bbdev.h
+++ b/lib/bbdev/rte_bbdev.h
@@ -349,6 +349,8 @@ struct rte_bbdev_driver_info {
        const struct rte_bbdev_op_cap *capabilities;
        /** Device cpu_flag requirements */
        const enum rte_cpu_flag_t *cpu_flag_reqs;
+       /** FFT windowing width for 2048 FFT - size defined in capability. */
+       uint16_t *fft_window_width;
   };

   /** Macro used at end of bbdev PMD list */ diff --git
a/lib/bbdev/rte_bbdev_op.h b/lib/bbdev/rte_bbdev_op.h index
693baa8386..369ac331bf 100644
--- a/lib/bbdev/rte_bbdev_op.h
+++ b/lib/bbdev/rte_bbdev_op.h
@@ -905,6 +905,8 @@ struct rte_bbdev_op_cap_fft {
        uint16_t num_buffers_src;
        /** Num output code block buffers. */
        uint16_t num_buffers_dst;
+       /** Number of FFT windows supported. */
+       uint16_t fft_windows_num;
   };

   /** List of the capabilities for the MLD */

I am Ok to move forwards with this, even if I would have liked feedback from
Hemant.

However, I think you need to add some documentation around it, as what is in
the commit message is important and won't be visible to the end- user.

Can we extend the documentation in incremental serie? There other things that 
could arguably be exposed more clearly, not just this one but more generally on 
device enumeration which is currently limited (pretty much everything from 
info_get).

Ok, as you take the action to do it in a further series:

Reviewed-by: Maxime Coquelin <maxime.coque...@redhat.com>

Thanks,
Maxime

Thanks,
Maxime