Re: [dpdk-dev] [PATCH v10 03/10] baseband/acc100: add info get function

["Chautru, Nicolas" <nicolas.chautru@intel.com>]

Hi Maxime, 
Ok for all. I can rename. 

> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> On 10/1/20 5:14 AM, Nicolas Chautru wrote:
> > Add in the "info_get" function to the driver, to allow us to query the
> > device.
> > No processing capability are available yet.
> > Linking bbdev-test to support the PMD with null capability.
> >
> > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> > Acked-by: Liu Tianjiao <Tianjiao.liu@intel.com>
> > ---
> >  app/test-bbdev/meson.build               |   3 +
> >  drivers/baseband/acc100/rte_acc100_cfg.h |  96 +++++++++++++
> > drivers/baseband/acc100/rte_acc100_pmd.c | 229
> > +++++++++++++++++++++++++++++++
> > drivers/baseband/acc100/rte_acc100_pmd.h |  10 ++
> >  4 files changed, 338 insertions(+)
> >  create mode 100644 drivers/baseband/acc100/rte_acc100_cfg.h
> >
> > diff --git a/app/test-bbdev/meson.build b/app/test-bbdev/meson.build
> > index 18ab6a8..fbd8ae3 100644
> > --- a/app/test-bbdev/meson.build
> > +++ b/app/test-bbdev/meson.build
> > @@ -12,3 +12,6 @@ endif
> >  if dpdk_conf.has('RTE_LIBRTE_PMD_BBDEV_FPGA_5GNR_FEC')
> >  	deps += ['pmd_bbdev_fpga_5gnr_fec']
> >  endif
> > +if dpdk_conf.has('RTE_LIBRTE_PMD_BBDEV_ACC100')
> > +	deps += ['pmd_bbdev_acc100']
> > +endif
> > \ No newline at end of file
> > diff --git a/drivers/baseband/acc100/rte_acc100_cfg.h
> > b/drivers/baseband/acc100/rte_acc100_cfg.h
> > new file mode 100644
> > index 0000000..73bbe36
> > --- /dev/null
> > +++ b/drivers/baseband/acc100/rte_acc100_cfg.h
> > @@ -0,0 +1,96 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2020 Intel Corporation  */
> > +
> > +#ifndef _RTE_ACC100_CFG_H_
> > +#define _RTE_ACC100_CFG_H_
> > +
> > +/**
> > + * @file rte_acc100_cfg.h
> > + *
> > + * Functions for configuring ACC100 HW, exposed directly to applications.
> > + * Configuration related to encoding/decoding is done through the
> > + * librte_bbdev library.
> > + *
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice  */
> > +
> > +#include <stdint.h>
> > +#include <stdbool.h>
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +/**< Number of Virtual Functions ACC100 supports */ #define
> > +RTE_ACC100_NUM_VFS 16
> > +
> > +/**
> > + * Definition of Queue Topology for ACC100 Configuration
> > + * Some level of details is abstracted out to expose a clean
> > +interface
> > + * given that comprehensive flexibility is not required  */ struct
> > +rte_q_topology_t {
> 
> The naming is too generic, it has to contain the driver name.
> Also, it should not pe postfixed with _t, as it is not a typedef.
> 
> "struct rte_acc100_queue_topology"?
> 
> > +	/** Number of QGroups in incremental order of priority */
> > +	uint16_t num_qgroups;
> > +	/**
> > +	 * All QGroups have the same number of AQs here.
> > +	 * Note : Could be made a 16-array if more flexibility is really
> > +	 * required
> > +	 */
> > +	uint16_t num_aqs_per_groups;
> > +	/**
> > +	 * Depth of the AQs is the same of all QGroups here. Log2 Enum : 2^N
> > +	 * Note : Could be made a 16-array if more flexibility is really
> > +	 * required
> > +	 */
> > +	uint16_t aq_depth_log2;
> > +	/**
> > +	 * Index of the first Queue Group Index - assuming contiguity
> > +	 * Initialized as -1
> > +	 */
> > +	int8_t first_qgroup_index;
> > +};
> > +
> > +/**
> > + * Definition of Arbitration related parameters for ACC100
> > +Configuration  */ struct rte_arbitration_t {
> 
> Same remark here.
> 
> > +	/** Default Weight for VF Fairness Arbitration */
> > +	uint16_t round_robin_weight;
> > +	uint32_t gbr_threshold1; /**< Guaranteed Bitrate Threshold 1 */
> > +	uint32_t gbr_threshold2; /**< Guaranteed Bitrate Threshold 2 */ };
> > +
> > +/**
> > + * Structure to pass ACC100 configuration.
> > + * Note: all VF Bundles will have the same configuration.
> > + */
> > +struct acc100_conf {
> 
> "struct rte_acc100_conf"?
> 
> > +	bool pf_mode_en; /**< 1 if PF is used for dataplane, 0 for VFs */
> > +	/** 1 if input '1' bit is represented by a positive LLR value, 0 if '1'
> > +	 * bit is represented by a negative value.
> > +	 */
> > +	bool input_pos_llr_1_bit;
> > +	/** 1 if output '1' bit is represented by a positive value, 0 if '1'
> > +	 * bit is represented by a negative value.
> > +	 */
> > +	bool output_pos_llr_1_bit;
> > +	uint16_t num_vf_bundles; /**< Number of VF bundles to setup */
> > +	/** Queue topology for each operation type */
> > +	struct rte_q_topology_t q_ul_4g;
> > +	struct rte_q_topology_t q_dl_4g;
> > +	struct rte_q_topology_t q_ul_5g;
> > +	struct rte_q_topology_t q_dl_5g;
> > +	/** Arbitration configuration for each operation type */
> > +	struct rte_arbitration_t arb_ul_4g[RTE_ACC100_NUM_VFS];
> > +	struct rte_arbitration_t arb_dl_4g[RTE_ACC100_NUM_VFS];
> > +	struct rte_arbitration_t arb_ul_5g[RTE_ACC100_NUM_VFS];
> > +	struct rte_arbitration_t arb_dl_5g[RTE_ACC100_NUM_VFS]; };
> > +
> > +#ifdef __cplusplus
> > +}
> > +#endif
> > +
> > +#endif /* _RTE_ACC100_CFG_H_ */
> 
> Regards,
> Maxime