Re: [PATCH v3 02/12] baseband/acc: add FFT window width in the VRB PMD

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 9/29/23 18:35, Nicolas Chautru wrote:
> This allows to expose the FFT window width being introduced in
> previous commit based on what is configured dynamically on the
> device platform.
> 
> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> ---
>   drivers/baseband/acc/acc_common.h  |  3 +++
>   drivers/baseband/acc/rte_vrb_pmd.c | 29 +++++++++++++++++++++++++++++
>   2 files changed, 32 insertions(+)
> 
> diff --git a/drivers/baseband/acc/acc_common.h b/drivers/baseband/acc/acc_common.h
> index 5bb00746c3..7d24c644c0 100644
> --- a/drivers/baseband/acc/acc_common.h
> +++ b/drivers/baseband/acc/acc_common.h
> @@ -512,6 +512,8 @@ struct acc_deq_intr_details {
>   enum {
>   	ACC_VF2PF_STATUS_REQUEST = 1,
>   	ACC_VF2PF_USING_VF = 2,
> +	ACC_VF2PF_LUT_VER_REQUEST = 3,
> +	ACC_VF2PF_FFT_WIN_REQUEST = 4,
>   };
>   
>   
> @@ -558,6 +560,7 @@ struct acc_device {
>   	queue_offset_fun_t queue_offset;  /* Device specific queue offset */
>   	uint16_t num_qgroups;
>   	uint16_t num_aqs;
> +	uint16_t fft_window_width[RTE_BBDEV_MAX_FFT_WIN]; /* FFT windowing width. */
>   };
>   
>   /* Structure associated with each queue. */
> diff --git a/drivers/baseband/acc/rte_vrb_pmd.c b/drivers/baseband/acc/rte_vrb_pmd.c
> index 9e5a73c9c7..c5a74bae11 100644
> --- a/drivers/baseband/acc/rte_vrb_pmd.c
> +++ b/drivers/baseband/acc/rte_vrb_pmd.c
> @@ -298,6 +298,34 @@ vrb_device_status(struct rte_bbdev *dev)
>   	return reg;
>   }
>   
> +/* Request device FFT windowing information. */
> +static inline void
> +vrb_device_fft_win(struct rte_bbdev *dev, struct rte_bbdev_driver_info *dev_info)
> +{
> +	struct acc_device *d = dev->data->dev_private;
> +	uint32_t reg, time_out = 0, win;
> +
> +	if (d->pf_device)
> +		return;
> +
> +	/* Check from the device the first time. */
> +	if (d->fft_window_width[0] == 0) {

O is not a possible value? Might not be a big deal as it would just
perform reading of 16 x 2 registers every time .info_get() is called,
just wondering.

> +		for (win = 0; win < RTE_BBDEV_MAX_FFT_WIN; win++) {
> +			vrb_vf2pf(d, ACC_VF2PF_FFT_WIN_REQUEST | win);

That looks broken, as extending RTE_BBDEV_MAX_FFT_WIN to support other
devices may break this implementation.

To me, it tends to show how this fft_window_width array is more device
specific than generic.

> +			reg = acc_reg_read(d, d->reg_addr->pf2vf_doorbell);
> +			while ((time_out < ACC_STATUS_TO) && (reg == RTE_BBDEV_DEV_NOSTATUS)) {
> +				usleep(ACC_STATUS_WAIT); /*< Wait or VF->PF->VF Comms */
> +				reg = acc_reg_read(d, d->reg_addr->pf2vf_doorbell);
> +				time_out++;
> +			}
> +			d->fft_window_width[win] = reg;
> +		}
> +	}
> +
> +	for (win = 0; win < RTE_BBDEV_MAX_FFT_WIN; win++)
> +		dev_info->fft_window_width[win] = d->fft_window_width[win];
> +}
> +
>   /* Checks PF Info Ring to find the interrupt cause and handles it accordingly. */
>   static inline void
>   vrb_check_ir(struct acc_device *acc_dev)
> @@ -1100,6 +1128,7 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct rte_bbdev_driver_info *dev_info)
>   	fetch_acc_config(dev);
>   	/* Check the status of device. */
>   	dev_info->device_status = vrb_device_status(dev);
> +	vrb_device_fft_win(dev, dev_info);
>   
>   	/* Exposed number of queues. */
>   	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;