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

[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