[v4,4/7] drivers/baseband: update PMDs to expose queue per operation

Message ID 1657067022-54373-5-git-send-email-nicolas.chautru@intel.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series bbdev changes for 22.11 |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Chautru, Nicolas July 6, 2022, 12:23 a.m. UTC
  Add support in existing bbdev PMDs for the explicit number of queue
and priority for each operation type configured on the device.

Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
---
 drivers/baseband/acc100/rte_acc100_pmd.c           | 29 +++++++++++++---------
 drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c |  8 ++++++
 drivers/baseband/fpga_lte_fec/fpga_lte_fec.c       |  8 ++++++
 drivers/baseband/la12xx/bbdev_la12xx.c             |  7 ++++++
 drivers/baseband/turbo_sw/bbdev_turbo_software.c   | 11 ++++++++
 5 files changed, 51 insertions(+), 12 deletions(-)
  

Comments

Tom Rix July 6, 2022, 4:15 p.m. UTC | #1
On 7/5/22 5:23 PM, Nicolas Chautru wrote:
> Add support in existing bbdev PMDs for the explicit number of queue
> and priority for each operation type configured on the device.
>
> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> ---
>   drivers/baseband/acc100/rte_acc100_pmd.c           | 29 +++++++++++++---------
>   drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c |  8 ++++++
>   drivers/baseband/fpga_lte_fec/fpga_lte_fec.c       |  8 ++++++
>   drivers/baseband/la12xx/bbdev_la12xx.c             |  7 ++++++
>   drivers/baseband/turbo_sw/bbdev_turbo_software.c   | 11 ++++++++
>   5 files changed, 51 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c b/drivers/baseband/acc100/rte_acc100_pmd.c
> index 17ba798..d568d0d 100644
> --- a/drivers/baseband/acc100/rte_acc100_pmd.c
> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
> @@ -966,6 +966,7 @@
>   		struct rte_bbdev_driver_info *dev_info)
>   {
>   	struct acc100_device *d = dev->data->dev_private;
> +	int i;
>   
>   	static const struct rte_bbdev_op_cap bbdev_capabilities[] = {
>   		{
> @@ -1062,19 +1063,23 @@
>   	fetch_acc100_config(dev);
>   	dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>   
> -	/* This isn't ideal because it reports the maximum number of queues but
> -	 * does not provide info on how many can be uplink/downlink or different
> -	 * priorities
> -	 */
> -	dev_info->max_num_queues =
> -			d->acc100_conf.q_dl_5g.num_aqs_per_groups *
> -			d->acc100_conf.q_dl_5g.num_qgroups +
> -			d->acc100_conf.q_ul_5g.num_aqs_per_groups *
> -			d->acc100_conf.q_ul_5g.num_qgroups +
> -			d->acc100_conf.q_dl_4g.num_aqs_per_groups *
> -			d->acc100_conf.q_dl_4g.num_qgroups +
> -			d->acc100_conf.q_ul_4g.num_aqs_per_groups *
> +	/* Expose number of queues */
> +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = d->acc100_conf.q_ul_4g.num_aqs_per_groups *
>   			d->acc100_conf.q_ul_4g.num_qgroups;
> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = d->acc100_conf.q_dl_4g.num_aqs_per_groups *
> +			d->acc100_conf.q_dl_4g.num_qgroups;
> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = d->acc100_conf.q_ul_5g.num_aqs_per_groups *
> +			d->acc100_conf.q_ul_5g.num_qgroups;
> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = d->acc100_conf.q_dl_5g.num_aqs_per_groups *
> +			d->acc100_conf.q_dl_5g.num_qgroups;
> +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_DEC] = d->acc100_conf.q_ul_4g.num_qgroups;
> +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_ENC] = d->acc100_conf.q_dl_4g.num_qgroups;
> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = d->acc100_conf.q_ul_5g.num_qgroups;
> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = d->acc100_conf.q_dl_5g.num_qgroups;
> +	dev_info->max_num_queues = 0;
> +	for (i = RTE_BBDEV_OP_TURBO_DEC; i < RTE_BBDEV_OP_LDPC_ENC; i++)

should this be i <=  ?


> +		dev_info->max_num_queues += dev_info->num_queues[i];
>   	dev_info->queue_size_lim = ACC100_MAX_QUEUE_DEPTH;
>   	dev_info->hardware_accelerated = true;
>   	dev_info->max_dl_queue_priority =
> diff --git a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> index 57b12af..b4982af 100644
> --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> @@ -379,6 +379,14 @@
>   		if (hw_q_id != FPGA_INVALID_HW_QUEUE_ID)
>   			dev_info->max_num_queues++;
>   	}
> +	/* Expose number of queue per operation type */
> +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = 0;
> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = 0;
> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = dev_info->max_num_queues / 2;
> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = dev_info->max_num_queues / 2;
> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = 1;
> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = 1;
>   }
>   
>   /**
> diff --git a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> index 2a330c4..dc7f479 100644
> --- a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> +++ b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> @@ -655,6 +655,14 @@ struct __rte_cache_aligned fpga_queue {
>   		if (hw_q_id != FPGA_INVALID_HW_QUEUE_ID)
>   			dev_info->max_num_queues++;
>   	}
> +	/* Expose number of queue per operation type */
> +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = dev_info->max_num_queues / 2;
> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = dev_info->max_num_queues / 2;
> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = 0;
> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = 0;
> +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_DEC] = 1;
> +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_ENC] = 1;
>   }
>   
>   /**
> diff --git a/drivers/baseband/la12xx/bbdev_la12xx.c b/drivers/baseband/la12xx/bbdev_la12xx.c
> index c1f88c6..e99ea9a 100644
> --- a/drivers/baseband/la12xx/bbdev_la12xx.c
> +++ b/drivers/baseband/la12xx/bbdev_la12xx.c
> @@ -102,6 +102,13 @@ struct bbdev_la12xx_params {
>   	dev_info->min_alignment = 64;
>   	dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>   
> +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = 0;
> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = 0;
> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = LA12XX_MAX_QUEUES / 2;
> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = LA12XX_MAX_QUEUES / 2;
> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = 1;
> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = 1;
>   	rte_bbdev_log_debug("got device info from %u", dev->data->dev_id);
>   }
>   
> diff --git a/drivers/baseband/turbo_sw/bbdev_turbo_software.c b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> index dbc5524..647e706 100644
> --- a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> +++ b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> @@ -256,6 +256,17 @@ struct turbo_sw_queue {
>   	dev_info->data_endianness = RTE_LITTLE_ENDIAN;
>   	dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>   
> +	const struct rte_bbdev_op_cap *op_cap = bbdev_capabilities;

Should this be done through dev instead of assigning directly ?

Tom

> +	int num_op_type = 0;
> +	for (; op_cap->type != RTE_BBDEV_OP_NONE; ++op_cap)
> +		num_op_type++;
> +	op_cap = bbdev_capabilities;
> +	if (num_op_type > 0) {
> +		int num_queue_per_type = dev_info->max_num_queues / num_op_type;
> +		for (; op_cap->type != RTE_BBDEV_OP_NONE; ++op_cap)
> +			dev_info->num_queues[op_cap->type] = num_queue_per_type;
> +	}
> +
>   	rte_bbdev_log_debug("got device info from %u\n", dev->data->dev_id);
>   }
>
  
Chautru, Nicolas July 6, 2022, 9:10 p.m. UTC | #2
Hi Tom, 

> -----Original Message-----
> From: Tom Rix <trix@redhat.com>
> Sent: Wednesday, July 6, 2022 9:15 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> thomas@monjalon.net; gakhil@marvell.com; hemant.agrawal@nxp.com
> Cc: maxime.coquelin@redhat.com; mdr@ashroe.eu; Richardson, Bruce
> <bruce.richardson@intel.com>; david.marchand@redhat.com;
> stephen@networkplumber.org
> Subject: Re: [PATCH v4 4/7] drivers/baseband: update PMDs to expose queue
> per operation
> 
> 
> On 7/5/22 5:23 PM, Nicolas Chautru wrote:
> > Add support in existing bbdev PMDs for the explicit number of queue
> > and priority for each operation type configured on the device.
> >
> > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> > ---
> >   drivers/baseband/acc100/rte_acc100_pmd.c           | 29 +++++++++++++------
> ---
> >   drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c |  8 ++++++
> >   drivers/baseband/fpga_lte_fec/fpga_lte_fec.c       |  8 ++++++
> >   drivers/baseband/la12xx/bbdev_la12xx.c             |  7 ++++++
> >   drivers/baseband/turbo_sw/bbdev_turbo_software.c   | 11 ++++++++
> >   5 files changed, 51 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
> > b/drivers/baseband/acc100/rte_acc100_pmd.c
> > index 17ba798..d568d0d 100644
> > --- a/drivers/baseband/acc100/rte_acc100_pmd.c
> > +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
> > @@ -966,6 +966,7 @@
> >   		struct rte_bbdev_driver_info *dev_info)
> >   {
> >   	struct acc100_device *d = dev->data->dev_private;
> > +	int i;
> >
> >   	static const struct rte_bbdev_op_cap bbdev_capabilities[] = {
> >   		{
> > @@ -1062,19 +1063,23 @@
> >   	fetch_acc100_config(dev);
> >   	dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
> >
> > -	/* This isn't ideal because it reports the maximum number of queues
> but
> > -	 * does not provide info on how many can be uplink/downlink or
> different
> > -	 * priorities
> > -	 */
> > -	dev_info->max_num_queues =
> > -			d->acc100_conf.q_dl_5g.num_aqs_per_groups *
> > -			d->acc100_conf.q_dl_5g.num_qgroups +
> > -			d->acc100_conf.q_ul_5g.num_aqs_per_groups *
> > -			d->acc100_conf.q_ul_5g.num_qgroups +
> > -			d->acc100_conf.q_dl_4g.num_aqs_per_groups *
> > -			d->acc100_conf.q_dl_4g.num_qgroups +
> > -			d->acc100_conf.q_ul_4g.num_aqs_per_groups *
> > +	/* Expose number of queues */
> > +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
> > +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] =
> > +d->acc100_conf.q_ul_4g.num_aqs_per_groups *
> >   			d->acc100_conf.q_ul_4g.num_qgroups;
> > +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = d-
> >acc100_conf.q_dl_4g.num_aqs_per_groups *
> > +			d->acc100_conf.q_dl_4g.num_qgroups;
> > +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = d-
> >acc100_conf.q_ul_5g.num_aqs_per_groups *
> > +			d->acc100_conf.q_ul_5g.num_qgroups;
> > +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = d-
> >acc100_conf.q_dl_5g.num_aqs_per_groups *
> > +			d->acc100_conf.q_dl_5g.num_qgroups;
> > +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_DEC] = d-
> >acc100_conf.q_ul_4g.num_qgroups;
> > +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_ENC] = d-
> >acc100_conf.q_dl_4g.num_qgroups;
> > +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = d-
> >acc100_conf.q_ul_5g.num_qgroups;
> > +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = d-
> >acc100_conf.q_dl_5g.num_qgroups;
> > +	dev_info->max_num_queues = 0;
> > +	for (i = RTE_BBDEV_OP_TURBO_DEC; i < RTE_BBDEV_OP_LDPC_ENC;
> i++)
> 
> should this be i <=  ?
>

Thanks

> 
> > +		dev_info->max_num_queues += dev_info->num_queues[i];
> >   	dev_info->queue_size_lim = ACC100_MAX_QUEUE_DEPTH;
> >   	dev_info->hardware_accelerated = true;
> >   	dev_info->max_dl_queue_priority =
> > diff --git a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> > b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> > index 57b12af..b4982af 100644
> > --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> > +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> > @@ -379,6 +379,14 @@
> >   		if (hw_q_id != FPGA_INVALID_HW_QUEUE_ID)
> >   			dev_info->max_num_queues++;
> >   	}
> > +	/* Expose number of queue per operation type */
> > +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
> > +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = 0;
> > +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = 0;
> > +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = dev_info-
> >max_num_queues / 2;
> > +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = dev_info-
> >max_num_queues / 2;
> > +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = 1;
> > +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = 1;
> >   }
> >
> >   /**
> > diff --git a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> > b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> > index 2a330c4..dc7f479 100644
> > --- a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> > +++ b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> > @@ -655,6 +655,14 @@ struct __rte_cache_aligned fpga_queue {
> >   		if (hw_q_id != FPGA_INVALID_HW_QUEUE_ID)
> >   			dev_info->max_num_queues++;
> >   	}
> > +	/* Expose number of queue per operation type */
> > +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
> > +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = dev_info-
> >max_num_queues / 2;
> > +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = dev_info-
> >max_num_queues / 2;
> > +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = 0;
> > +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = 0;
> > +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_DEC] = 1;
> > +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_ENC] = 1;
> >   }
> >
> >   /**
> > diff --git a/drivers/baseband/la12xx/bbdev_la12xx.c
> > b/drivers/baseband/la12xx/bbdev_la12xx.c
> > index c1f88c6..e99ea9a 100644
> > --- a/drivers/baseband/la12xx/bbdev_la12xx.c
> > +++ b/drivers/baseband/la12xx/bbdev_la12xx.c
> > @@ -102,6 +102,13 @@ struct bbdev_la12xx_params {
> >   	dev_info->min_alignment = 64;
> >   	dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
> >
> > +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
> > +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = 0;
> > +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = 0;
> > +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] =
> LA12XX_MAX_QUEUES / 2;
> > +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] =
> LA12XX_MAX_QUEUES / 2;
> > +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = 1;
> > +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = 1;
> >   	rte_bbdev_log_debug("got device info from %u", dev->data->dev_id);
> >   }
> >
> > diff --git a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> > b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> > index dbc5524..647e706 100644
> > --- a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> > +++ b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> > @@ -256,6 +256,17 @@ struct turbo_sw_queue {
> >   	dev_info->data_endianness = RTE_LITTLE_ENDIAN;
> >   	dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
> >
> > +	const struct rte_bbdev_op_cap *op_cap = bbdev_capabilities;
> 
> Should this be done through dev instead of assigning directly ?

I am not sure I follow your suggestion. Do you mind clarifying?

> 
> Tom
> 
> > +	int num_op_type = 0;
> > +	for (; op_cap->type != RTE_BBDEV_OP_NONE; ++op_cap)
> > +		num_op_type++;
> > +	op_cap = bbdev_capabilities;
> > +	if (num_op_type > 0) {
> > +		int num_queue_per_type = dev_info->max_num_queues /
> num_op_type;
> > +		for (; op_cap->type != RTE_BBDEV_OP_NONE; ++op_cap)
> > +			dev_info->num_queues[op_cap->type] =
> num_queue_per_type;
> > +	}
> > +
> >   	rte_bbdev_log_debug("got device info from %u\n", dev->data-
> >dev_id);
> >   }
> >
  
Tom Rix July 7, 2022, 1:20 p.m. UTC | #3
On 7/6/22 2:10 PM, Chautru, Nicolas wrote:
> Hi Tom,
>
>> -----Original Message-----
>> From: Tom Rix <trix@redhat.com>
>> Sent: Wednesday, July 6, 2022 9:15 AM
>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
>> thomas@monjalon.net; gakhil@marvell.com; hemant.agrawal@nxp.com
>> Cc: maxime.coquelin@redhat.com; mdr@ashroe.eu; Richardson, Bruce
>> <bruce.richardson@intel.com>; david.marchand@redhat.com;
>> stephen@networkplumber.org
>> Subject: Re: [PATCH v4 4/7] drivers/baseband: update PMDs to expose queue
>> per operation
>>
>>
>> On 7/5/22 5:23 PM, Nicolas Chautru wrote:
>>> Add support in existing bbdev PMDs for the explicit number of queue
>>> and priority for each operation type configured on the device.
>>>
>>> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
>>> ---
>>>    drivers/baseband/acc100/rte_acc100_pmd.c           | 29 +++++++++++++------
>> ---
>>>    drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c |  8 ++++++
>>>    drivers/baseband/fpga_lte_fec/fpga_lte_fec.c       |  8 ++++++
>>>    drivers/baseband/la12xx/bbdev_la12xx.c             |  7 ++++++
>>>    drivers/baseband/turbo_sw/bbdev_turbo_software.c   | 11 ++++++++
>>>    5 files changed, 51 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
>>> b/drivers/baseband/acc100/rte_acc100_pmd.c
>>> index 17ba798..d568d0d 100644
>>> --- a/drivers/baseband/acc100/rte_acc100_pmd.c
>>> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
>>> @@ -966,6 +966,7 @@
>>>    		struct rte_bbdev_driver_info *dev_info)
>>>    {
>>>    	struct acc100_device *d = dev->data->dev_private;
>>> +	int i;
>>>
>>>    	static const struct rte_bbdev_op_cap bbdev_capabilities[] = {
>>>    		{
>>> @@ -1062,19 +1063,23 @@
>>>    	fetch_acc100_config(dev);
>>>    	dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>>>
>>> -	/* This isn't ideal because it reports the maximum number of queues
>> but
>>> -	 * does not provide info on how many can be uplink/downlink or
>> different
>>> -	 * priorities
>>> -	 */
>>> -	dev_info->max_num_queues =
>>> -			d->acc100_conf.q_dl_5g.num_aqs_per_groups *
>>> -			d->acc100_conf.q_dl_5g.num_qgroups +
>>> -			d->acc100_conf.q_ul_5g.num_aqs_per_groups *
>>> -			d->acc100_conf.q_ul_5g.num_qgroups +
>>> -			d->acc100_conf.q_dl_4g.num_aqs_per_groups *
>>> -			d->acc100_conf.q_dl_4g.num_qgroups +
>>> -			d->acc100_conf.q_ul_4g.num_aqs_per_groups *
>>> +	/* Expose number of queues */
>>> +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] =
>>> +d->acc100_conf.q_ul_4g.num_aqs_per_groups *
>>>    			d->acc100_conf.q_ul_4g.num_qgroups;
>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = d-
>>> acc100_conf.q_dl_4g.num_aqs_per_groups *
>>> +			d->acc100_conf.q_dl_4g.num_qgroups;
>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = d-
>>> acc100_conf.q_ul_5g.num_aqs_per_groups *
>>> +			d->acc100_conf.q_ul_5g.num_qgroups;
>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = d-
>>> acc100_conf.q_dl_5g.num_aqs_per_groups *
>>> +			d->acc100_conf.q_dl_5g.num_qgroups;
>>> +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_DEC] = d-
>>> acc100_conf.q_ul_4g.num_qgroups;
>>> +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_ENC] = d-
>>> acc100_conf.q_dl_4g.num_qgroups;
>>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = d-
>>> acc100_conf.q_ul_5g.num_qgroups;
>>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = d-
>>> acc100_conf.q_dl_5g.num_qgroups;
>>> +	dev_info->max_num_queues = 0;
>>> +	for (i = RTE_BBDEV_OP_TURBO_DEC; i < RTE_BBDEV_OP_LDPC_ENC;
>> i++)
>>
>> should this be i <=  ?
>>
> Thanks
>
>>> +		dev_info->max_num_queues += dev_info->num_queues[i];
>>>    	dev_info->queue_size_lim = ACC100_MAX_QUEUE_DEPTH;
>>>    	dev_info->hardware_accelerated = true;
>>>    	dev_info->max_dl_queue_priority =
>>> diff --git a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>> b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>> index 57b12af..b4982af 100644
>>> --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>> +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>> @@ -379,6 +379,14 @@
>>>    		if (hw_q_id != FPGA_INVALID_HW_QUEUE_ID)
>>>    			dev_info->max_num_queues++;
>>>    	}
>>> +	/* Expose number of queue per operation type */
>>> +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = 0;
>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = 0;
>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = dev_info-
>>> max_num_queues / 2;
>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = dev_info-
>>> max_num_queues / 2;
>>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = 1;
>>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = 1;
>>>    }
>>>
>>>    /**
>>> diff --git a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
>>> b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
>>> index 2a330c4..dc7f479 100644
>>> --- a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
>>> +++ b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
>>> @@ -655,6 +655,14 @@ struct __rte_cache_aligned fpga_queue {
>>>    		if (hw_q_id != FPGA_INVALID_HW_QUEUE_ID)
>>>    			dev_info->max_num_queues++;
>>>    	}
>>> +	/* Expose number of queue per operation type */
>>> +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = dev_info-
>>> max_num_queues / 2;
>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = dev_info-
>>> max_num_queues / 2;
>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = 0;
>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = 0;
>>> +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_DEC] = 1;
>>> +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_ENC] = 1;
>>>    }
>>>
>>>    /**
>>> diff --git a/drivers/baseband/la12xx/bbdev_la12xx.c
>>> b/drivers/baseband/la12xx/bbdev_la12xx.c
>>> index c1f88c6..e99ea9a 100644
>>> --- a/drivers/baseband/la12xx/bbdev_la12xx.c
>>> +++ b/drivers/baseband/la12xx/bbdev_la12xx.c
>>> @@ -102,6 +102,13 @@ struct bbdev_la12xx_params {
>>>    	dev_info->min_alignment = 64;
>>>    	dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>>>
>>> +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = 0;
>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = 0;
>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] =
>> LA12XX_MAX_QUEUES / 2;
>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] =
>> LA12XX_MAX_QUEUES / 2;
>>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = 1;
>>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = 1;
>>>    	rte_bbdev_log_debug("got device info from %u", dev->data->dev_id);
>>>    }
>>>
>>> diff --git a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
>>> b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
>>> index dbc5524..647e706 100644
>>> --- a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
>>> +++ b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
>>> @@ -256,6 +256,17 @@ struct turbo_sw_queue {
>>>    	dev_info->data_endianness = RTE_LITTLE_ENDIAN;
>>>    	dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>>>
>>> +	const struct rte_bbdev_op_cap *op_cap = bbdev_capabilities;
>> Should this be done through dev instead of assigning directly ?
> I am not sure I follow your suggestion. Do you mind clarifying?

bbdev_capabilites is a const defined in this function, do you really 
need to loop over it to find information that is constant ?

Tom

>
>> Tom
>>
>>> +	int num_op_type = 0;
>>> +	for (; op_cap->type != RTE_BBDEV_OP_NONE; ++op_cap)
>>> +		num_op_type++;
>>> +	op_cap = bbdev_capabilities;
>>> +	if (num_op_type > 0) {
>>> +		int num_queue_per_type = dev_info->max_num_queues /
>> num_op_type;
>>> +		for (; op_cap->type != RTE_BBDEV_OP_NONE; ++op_cap)
>>> +			dev_info->num_queues[op_cap->type] =
>> num_queue_per_type;
>>> +	}
>>> +
>>>    	rte_bbdev_log_debug("got device info from %u\n", dev->data-
>>> dev_id);
>>>    }
>>>
  
Chautru, Nicolas July 7, 2022, 5:19 p.m. UTC | #4
Hi Tom, 

> -----Original Message-----
> From: Tom Rix <trix@redhat.com>
> Sent: Thursday, July 7, 2022 6:21 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> thomas@monjalon.net; gakhil@marvell.com; hemant.agrawal@nxp.com
> Cc: maxime.coquelin@redhat.com; mdr@ashroe.eu; Richardson, Bruce
> <bruce.richardson@intel.com>; david.marchand@redhat.com;
> stephen@networkplumber.org
> Subject: Re: [PATCH v4 4/7] drivers/baseband: update PMDs to expose queue
> per operation
> 
> 
> On 7/6/22 2:10 PM, Chautru, Nicolas wrote:
> > Hi Tom,
> >
> >> -----Original Message-----
> >> From: Tom Rix <trix@redhat.com>
> >> Sent: Wednesday, July 6, 2022 9:15 AM
> >> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> >> thomas@monjalon.net; gakhil@marvell.com; hemant.agrawal@nxp.com
> >> Cc: maxime.coquelin@redhat.com; mdr@ashroe.eu; Richardson, Bruce
> >> <bruce.richardson@intel.com>; david.marchand@redhat.com;
> >> stephen@networkplumber.org
> >> Subject: Re: [PATCH v4 4/7] drivers/baseband: update PMDs to expose
> >> queue per operation
> >>
> >>
> >> On 7/5/22 5:23 PM, Nicolas Chautru wrote:
> >>> Add support in existing bbdev PMDs for the explicit number of queue
> >>> and priority for each operation type configured on the device.
> >>>
> >>> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> >>> ---
> >>>    drivers/baseband/acc100/rte_acc100_pmd.c           | 29 +++++++++++++--
> ----
> >> ---
> >>>    drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c |  8 ++++++
> >>>    drivers/baseband/fpga_lte_fec/fpga_lte_fec.c       |  8 ++++++
> >>>    drivers/baseband/la12xx/bbdev_la12xx.c             |  7 ++++++
> >>>    drivers/baseband/turbo_sw/bbdev_turbo_software.c   | 11 ++++++++
> >>>    5 files changed, 51 insertions(+), 12 deletions(-)
> >>>
> >>> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
> >>> b/drivers/baseband/acc100/rte_acc100_pmd.c
> >>> index 17ba798..d568d0d 100644
> >>> --- a/drivers/baseband/acc100/rte_acc100_pmd.c
> >>> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
> >>> @@ -966,6 +966,7 @@
> >>>    		struct rte_bbdev_driver_info *dev_info)
> >>>    {
> >>>    	struct acc100_device *d = dev->data->dev_private;
> >>> +	int i;
> >>>
> >>>    	static const struct rte_bbdev_op_cap bbdev_capabilities[] = {
> >>>    		{
> >>> @@ -1062,19 +1063,23 @@
> >>>    	fetch_acc100_config(dev);
> >>>    	dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
> >>>
> >>> -	/* This isn't ideal because it reports the maximum number of queues
> >> but
> >>> -	 * does not provide info on how many can be uplink/downlink or
> >> different
> >>> -	 * priorities
> >>> -	 */
> >>> -	dev_info->max_num_queues =
> >>> -			d->acc100_conf.q_dl_5g.num_aqs_per_groups *
> >>> -			d->acc100_conf.q_dl_5g.num_qgroups +
> >>> -			d->acc100_conf.q_ul_5g.num_aqs_per_groups *
> >>> -			d->acc100_conf.q_ul_5g.num_qgroups +
> >>> -			d->acc100_conf.q_dl_4g.num_aqs_per_groups *
> >>> -			d->acc100_conf.q_dl_4g.num_qgroups +
> >>> -			d->acc100_conf.q_ul_4g.num_aqs_per_groups *
> >>> +	/* Expose number of queues */
> >>> +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
> >>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] =
> >>> +d->acc100_conf.q_ul_4g.num_aqs_per_groups *
> >>>    			d->acc100_conf.q_ul_4g.num_qgroups;
> >>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = d-
> >>> acc100_conf.q_dl_4g.num_aqs_per_groups *
> >>> +			d->acc100_conf.q_dl_4g.num_qgroups;
> >>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = d-
> >>> acc100_conf.q_ul_5g.num_aqs_per_groups *
> >>> +			d->acc100_conf.q_ul_5g.num_qgroups;
> >>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = d-
> >>> acc100_conf.q_dl_5g.num_aqs_per_groups *
> >>> +			d->acc100_conf.q_dl_5g.num_qgroups;
> >>> +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_DEC] = d-
> >>> acc100_conf.q_ul_4g.num_qgroups;
> >>> +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_ENC] = d-
> >>> acc100_conf.q_dl_4g.num_qgroups;
> >>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = d-
> >>> acc100_conf.q_ul_5g.num_qgroups;
> >>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = d-
> >>> acc100_conf.q_dl_5g.num_qgroups;
> >>> +	dev_info->max_num_queues = 0;
> >>> +	for (i = RTE_BBDEV_OP_TURBO_DEC; i < RTE_BBDEV_OP_LDPC_ENC;
> >> i++)
> >>
> >> should this be i <=  ?
> >>
> > Thanks
> >
> >>> +		dev_info->max_num_queues += dev_info->num_queues[i];
> >>>    	dev_info->queue_size_lim = ACC100_MAX_QUEUE_DEPTH;
> >>>    	dev_info->hardware_accelerated = true;
> >>>    	dev_info->max_dl_queue_priority = diff --git
> >>> a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> >>> b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> >>> index 57b12af..b4982af 100644
> >>> --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> >>> +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> >>> @@ -379,6 +379,14 @@
> >>>    		if (hw_q_id != FPGA_INVALID_HW_QUEUE_ID)
> >>>    			dev_info->max_num_queues++;
> >>>    	}
> >>> +	/* Expose number of queue per operation type */
> >>> +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
> >>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = 0;
> >>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = 0;
> >>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = dev_info-
> >>> max_num_queues / 2;
> >>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = dev_info-
> >>> max_num_queues / 2;
> >>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = 1;
> >>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = 1;
> >>>    }
> >>>
> >>>    /**
> >>> diff --git a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> >>> b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> >>> index 2a330c4..dc7f479 100644
> >>> --- a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> >>> +++ b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> >>> @@ -655,6 +655,14 @@ struct __rte_cache_aligned fpga_queue {
> >>>    		if (hw_q_id != FPGA_INVALID_HW_QUEUE_ID)
> >>>    			dev_info->max_num_queues++;
> >>>    	}
> >>> +	/* Expose number of queue per operation type */
> >>> +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
> >>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = dev_info-
> >>> max_num_queues / 2;
> >>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = dev_info-
> >>> max_num_queues / 2;
> >>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = 0;
> >>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = 0;
> >>> +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_DEC] = 1;
> >>> +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_ENC] = 1;
> >>>    }
> >>>
> >>>    /**
> >>> diff --git a/drivers/baseband/la12xx/bbdev_la12xx.c
> >>> b/drivers/baseband/la12xx/bbdev_la12xx.c
> >>> index c1f88c6..e99ea9a 100644
> >>> --- a/drivers/baseband/la12xx/bbdev_la12xx.c
> >>> +++ b/drivers/baseband/la12xx/bbdev_la12xx.c
> >>> @@ -102,6 +102,13 @@ struct bbdev_la12xx_params {
> >>>    	dev_info->min_alignment = 64;
> >>>    	dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
> >>>
> >>> +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
> >>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = 0;
> >>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = 0;
> >>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] =
> >> LA12XX_MAX_QUEUES / 2;
> >>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] =
> >> LA12XX_MAX_QUEUES / 2;
> >>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = 1;
> >>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = 1;
> >>>    	rte_bbdev_log_debug("got device info from %u", dev->data->dev_id);
> >>>    }
> >>>
> >>> diff --git a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> >>> b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> >>> index dbc5524..647e706 100644
> >>> --- a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> >>> +++ b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> >>> @@ -256,6 +256,17 @@ struct turbo_sw_queue {
> >>>    	dev_info->data_endianness = RTE_LITTLE_ENDIAN;
> >>>    	dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
> >>>
> >>> +	const struct rte_bbdev_op_cap *op_cap = bbdev_capabilities;
> >> Should this be done through dev instead of assigning directly ?
> > I am not sure I follow your suggestion. Do you mind clarifying?
> 
> bbdev_capabilites is a const defined in this function, do you really need to loop
> over it to find information that is constant ?

I still miss your point. Note that this constant is not always the same at build time (based on what SDK it can links to).
What would suggest?

Thanks
Nic


> 
> Tom
> 
> >
> >> Tom
> >>
> >>> +	int num_op_type = 0;
> >>> +	for (; op_cap->type != RTE_BBDEV_OP_NONE; ++op_cap)
> >>> +		num_op_type++;
> >>> +	op_cap = bbdev_capabilities;
> >>> +	if (num_op_type > 0) {
> >>> +		int num_queue_per_type = dev_info->max_num_queues /
> >> num_op_type;
> >>> +		for (; op_cap->type != RTE_BBDEV_OP_NONE; ++op_cap)
> >>> +			dev_info->num_queues[op_cap->type] =
> >> num_queue_per_type;
> >>> +	}
> >>> +
> >>>    	rte_bbdev_log_debug("got device info from %u\n", dev->data-
> >>> dev_id);
> >>>    }
> >>>
  
Tom Rix July 18, 2022, 1:21 p.m. UTC | #5
On 7/7/22 10:19 AM, Chautru, Nicolas wrote:
> Hi Tom,
>
>> -----Original Message-----
>> From: Tom Rix <trix@redhat.com>
>> Sent: Thursday, July 7, 2022 6:21 AM
>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
>> thomas@monjalon.net; gakhil@marvell.com; hemant.agrawal@nxp.com
>> Cc: maxime.coquelin@redhat.com; mdr@ashroe.eu; Richardson, Bruce
>> <bruce.richardson@intel.com>; david.marchand@redhat.com;
>> stephen@networkplumber.org
>> Subject: Re: [PATCH v4 4/7] drivers/baseband: update PMDs to expose queue
>> per operation
>>
>>
>> On 7/6/22 2:10 PM, Chautru, Nicolas wrote:
>>> Hi Tom,
>>>
>>>> -----Original Message-----
>>>> From: Tom Rix <trix@redhat.com>
>>>> Sent: Wednesday, July 6, 2022 9:15 AM
>>>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
>>>> thomas@monjalon.net; gakhil@marvell.com; hemant.agrawal@nxp.com
>>>> Cc: maxime.coquelin@redhat.com; mdr@ashroe.eu; Richardson, Bruce
>>>> <bruce.richardson@intel.com>; david.marchand@redhat.com;
>>>> stephen@networkplumber.org
>>>> Subject: Re: [PATCH v4 4/7] drivers/baseband: update PMDs to expose
>>>> queue per operation
>>>>
>>>>
>>>> On 7/5/22 5:23 PM, Nicolas Chautru wrote:
>>>>> Add support in existing bbdev PMDs for the explicit number of queue
>>>>> and priority for each operation type configured on the device.
>>>>>
>>>>> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
>>>>> ---
>>>>>     drivers/baseband/acc100/rte_acc100_pmd.c           | 29 +++++++++++++--
>> ----
>>>> ---
>>>>>     drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c |  8 ++++++
>>>>>     drivers/baseband/fpga_lte_fec/fpga_lte_fec.c       |  8 ++++++
>>>>>     drivers/baseband/la12xx/bbdev_la12xx.c             |  7 ++++++
>>>>>     drivers/baseband/turbo_sw/bbdev_turbo_software.c   | 11 ++++++++
>>>>>     5 files changed, 51 insertions(+), 12 deletions(-)
>>>>>
>>>>> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
>>>>> b/drivers/baseband/acc100/rte_acc100_pmd.c
>>>>> index 17ba798..d568d0d 100644
>>>>> --- a/drivers/baseband/acc100/rte_acc100_pmd.c
>>>>> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
>>>>> @@ -966,6 +966,7 @@
>>>>>     		struct rte_bbdev_driver_info *dev_info)
>>>>>     {
>>>>>     	struct acc100_device *d = dev->data->dev_private;
>>>>> +	int i;
>>>>>
>>>>>     	static const struct rte_bbdev_op_cap bbdev_capabilities[] = {
>>>>>     		{
>>>>> @@ -1062,19 +1063,23 @@
>>>>>     	fetch_acc100_config(dev);
>>>>>     	dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>>>>>
>>>>> -	/* This isn't ideal because it reports the maximum number of queues
>>>> but
>>>>> -	 * does not provide info on how many can be uplink/downlink or
>>>> different
>>>>> -	 * priorities
>>>>> -	 */
>>>>> -	dev_info->max_num_queues =
>>>>> -			d->acc100_conf.q_dl_5g.num_aqs_per_groups *
>>>>> -			d->acc100_conf.q_dl_5g.num_qgroups +
>>>>> -			d->acc100_conf.q_ul_5g.num_aqs_per_groups *
>>>>> -			d->acc100_conf.q_ul_5g.num_qgroups +
>>>>> -			d->acc100_conf.q_dl_4g.num_aqs_per_groups *
>>>>> -			d->acc100_conf.q_dl_4g.num_qgroups +
>>>>> -			d->acc100_conf.q_ul_4g.num_aqs_per_groups *
>>>>> +	/* Expose number of queues */
>>>>> +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
>>>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] =
>>>>> +d->acc100_conf.q_ul_4g.num_aqs_per_groups *
>>>>>     			d->acc100_conf.q_ul_4g.num_qgroups;
>>>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = d-
>>>>> acc100_conf.q_dl_4g.num_aqs_per_groups *
>>>>> +			d->acc100_conf.q_dl_4g.num_qgroups;
>>>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = d-
>>>>> acc100_conf.q_ul_5g.num_aqs_per_groups *
>>>>> +			d->acc100_conf.q_ul_5g.num_qgroups;
>>>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = d-
>>>>> acc100_conf.q_dl_5g.num_aqs_per_groups *
>>>>> +			d->acc100_conf.q_dl_5g.num_qgroups;
>>>>> +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_DEC] = d-
>>>>> acc100_conf.q_ul_4g.num_qgroups;
>>>>> +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_ENC] = d-
>>>>> acc100_conf.q_dl_4g.num_qgroups;
>>>>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = d-
>>>>> acc100_conf.q_ul_5g.num_qgroups;
>>>>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = d-
>>>>> acc100_conf.q_dl_5g.num_qgroups;
>>>>> +	dev_info->max_num_queues = 0;
>>>>> +	for (i = RTE_BBDEV_OP_TURBO_DEC; i < RTE_BBDEV_OP_LDPC_ENC;
>>>> i++)
>>>>
>>>> should this be i <=  ?
>>>>
>>> Thanks
>>>
>>>>> +		dev_info->max_num_queues += dev_info->num_queues[i];
>>>>>     	dev_info->queue_size_lim = ACC100_MAX_QUEUE_DEPTH;
>>>>>     	dev_info->hardware_accelerated = true;
>>>>>     	dev_info->max_dl_queue_priority = diff --git
>>>>> a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>>>> b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>>>> index 57b12af..b4982af 100644
>>>>> --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>>>> +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>>>> @@ -379,6 +379,14 @@
>>>>>     		if (hw_q_id != FPGA_INVALID_HW_QUEUE_ID)
>>>>>     			dev_info->max_num_queues++;
>>>>>     	}
>>>>> +	/* Expose number of queue per operation type */
>>>>> +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
>>>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = 0;
>>>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = 0;
>>>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = dev_info-
>>>>> max_num_queues / 2;
>>>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = dev_info-
>>>>> max_num_queues / 2;
>>>>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = 1;
>>>>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = 1;
>>>>>     }
>>>>>
>>>>>     /**
>>>>> diff --git a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
>>>>> b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
>>>>> index 2a330c4..dc7f479 100644
>>>>> --- a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
>>>>> +++ b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
>>>>> @@ -655,6 +655,14 @@ struct __rte_cache_aligned fpga_queue {
>>>>>     		if (hw_q_id != FPGA_INVALID_HW_QUEUE_ID)
>>>>>     			dev_info->max_num_queues++;
>>>>>     	}
>>>>> +	/* Expose number of queue per operation type */
>>>>> +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
>>>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = dev_info-
>>>>> max_num_queues / 2;
>>>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = dev_info-
>>>>> max_num_queues / 2;
>>>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = 0;
>>>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = 0;
>>>>> +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_DEC] = 1;
>>>>> +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_ENC] = 1;
>>>>>     }
>>>>>
>>>>>     /**
>>>>> diff --git a/drivers/baseband/la12xx/bbdev_la12xx.c
>>>>> b/drivers/baseband/la12xx/bbdev_la12xx.c
>>>>> index c1f88c6..e99ea9a 100644
>>>>> --- a/drivers/baseband/la12xx/bbdev_la12xx.c
>>>>> +++ b/drivers/baseband/la12xx/bbdev_la12xx.c
>>>>> @@ -102,6 +102,13 @@ struct bbdev_la12xx_params {
>>>>>     	dev_info->min_alignment = 64;
>>>>>     	dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>>>>>
>>>>> +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
>>>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = 0;
>>>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = 0;
>>>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] =
>>>> LA12XX_MAX_QUEUES / 2;
>>>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] =
>>>> LA12XX_MAX_QUEUES / 2;
>>>>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = 1;
>>>>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = 1;
>>>>>     	rte_bbdev_log_debug("got device info from %u", dev->data->dev_id);
>>>>>     }
>>>>>
>>>>> diff --git a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
>>>>> b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
>>>>> index dbc5524..647e706 100644
>>>>> --- a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
>>>>> +++ b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
>>>>> @@ -256,6 +256,17 @@ struct turbo_sw_queue {
>>>>>     	dev_info->data_endianness = RTE_LITTLE_ENDIAN;
>>>>>     	dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
>>>>>
>>>>> +	const struct rte_bbdev_op_cap *op_cap = bbdev_capabilities;
>>>> Should this be done through dev instead of assigning directly ?
>>> I am not sure I follow your suggestion. Do you mind clarifying?
>> bbdev_capabilites is a const defined in this function, do you really need to loop
>> over it to find information that is constant ?
> I still miss your point. Note that this constant is not always the same at build time (based on what SDK it can links to).
> What would suggest?

Operations that can be done at compile time, should be.  Useless there 
is a good reason.

You need to provide a good reason or make the change.

Tom

>
> Thanks
> Nic
>
>
>> Tom
>>
>>>> Tom
>>>>
>>>>> +	int num_op_type = 0;
>>>>> +	for (; op_cap->type != RTE_BBDEV_OP_NONE; ++op_cap)
>>>>> +		num_op_type++;
>>>>> +	op_cap = bbdev_capabilities;
>>>>> +	if (num_op_type > 0) {
>>>>> +		int num_queue_per_type = dev_info->max_num_queues /
>>>> num_op_type;
>>>>> +		for (; op_cap->type != RTE_BBDEV_OP_NONE; ++op_cap)
>>>>> +			dev_info->num_queues[op_cap->type] =
>>>> num_queue_per_type;
>>>>> +	}
>>>>> +
>>>>>     	rte_bbdev_log_debug("got device info from %u\n", dev->data-
>>>>> dev_id);
>>>>>     }
>>>>>
  
Chautru, Nicolas Aug. 15, 2022, 5:28 p.m. UTC | #6
Hi Tom, 

Back from time off, replying to that previous email.

> -----Original Message-----
> From: Tom Rix <trix@redhat.com>
> Sent: Monday, July 18, 2022 6:21 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> thomas@monjalon.net; gakhil@marvell.com; hemant.agrawal@nxp.com
> Cc: maxime.coquelin@redhat.com; mdr@ashroe.eu; Richardson, Bruce
> <bruce.richardson@intel.com>; david.marchand@redhat.com;
> stephen@networkplumber.org
> Subject: Re: [PATCH v4 4/7] drivers/baseband: update PMDs to expose queue
> per operation
> 
> 
> On 7/7/22 10:19 AM, Chautru, Nicolas wrote:
> > Hi Tom,
> >
> >> -----Original Message-----
> >> From: Tom Rix <trix@redhat.com>
> >> Sent: Thursday, July 7, 2022 6:21 AM
> >> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> >> thomas@monjalon.net; gakhil@marvell.com; hemant.agrawal@nxp.com
> >> Cc: maxime.coquelin@redhat.com; mdr@ashroe.eu; Richardson, Bruce
> >> <bruce.richardson@intel.com>; david.marchand@redhat.com;
> >> stephen@networkplumber.org
> >> Subject: Re: [PATCH v4 4/7] drivers/baseband: update PMDs to expose
> >> queue per operation
> >>
> >>
> >> On 7/6/22 2:10 PM, Chautru, Nicolas wrote:
> >>> Hi Tom,
> >>>
> >>>> -----Original Message-----
> >>>> From: Tom Rix <trix@redhat.com>
> >>>> Sent: Wednesday, July 6, 2022 9:15 AM
> >>>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> >>>> thomas@monjalon.net; gakhil@marvell.com;
> hemant.agrawal@nxp.com
> >>>> Cc: maxime.coquelin@redhat.com; mdr@ashroe.eu; Richardson, Bruce
> >>>> <bruce.richardson@intel.com>; david.marchand@redhat.com;
> >>>> stephen@networkplumber.org
> >>>> Subject: Re: [PATCH v4 4/7] drivers/baseband: update PMDs to expose
> >>>> queue per operation
> >>>>
> >>>>
> >>>> On 7/5/22 5:23 PM, Nicolas Chautru wrote:
> >>>>> Add support in existing bbdev PMDs for the explicit number of
> >>>>> queue and priority for each operation type configured on the device.
> >>>>>
> >>>>> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> >>>>> ---
> >>>>>     drivers/baseband/acc100/rte_acc100_pmd.c           | 29
> +++++++++++++--
> >> ----
> >>>> ---
> >>>>>     drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c |  8 ++++++
> >>>>>     drivers/baseband/fpga_lte_fec/fpga_lte_fec.c       |  8 ++++++
> >>>>>     drivers/baseband/la12xx/bbdev_la12xx.c             |  7 ++++++
> >>>>>     drivers/baseband/turbo_sw/bbdev_turbo_software.c   | 11
> ++++++++
> >>>>>     5 files changed, 51 insertions(+), 12 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
> >>>>> b/drivers/baseband/acc100/rte_acc100_pmd.c
> >>>>> index 17ba798..d568d0d 100644
> >>>>> --- a/drivers/baseband/acc100/rte_acc100_pmd.c
> >>>>> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
> >>>>> @@ -966,6 +966,7 @@
> >>>>>     		struct rte_bbdev_driver_info *dev_info)
> >>>>>     {
> >>>>>     	struct acc100_device *d = dev->data->dev_private;
> >>>>> +	int i;
> >>>>>
> >>>>>     	static const struct rte_bbdev_op_cap bbdev_capabilities[] = {
> >>>>>     		{
> >>>>> @@ -1062,19 +1063,23 @@
> >>>>>     	fetch_acc100_config(dev);
> >>>>>     	dev_info->device_status =
> RTE_BBDEV_DEV_NOT_SUPPORTED;
> >>>>>
> >>>>> -	/* This isn't ideal because it reports the maximum number of queues
> >>>> but
> >>>>> -	 * does not provide info on how many can be uplink/downlink or
> >>>> different
> >>>>> -	 * priorities
> >>>>> -	 */
> >>>>> -	dev_info->max_num_queues =
> >>>>> -			d->acc100_conf.q_dl_5g.num_aqs_per_groups *
> >>>>> -			d->acc100_conf.q_dl_5g.num_qgroups +
> >>>>> -			d->acc100_conf.q_ul_5g.num_aqs_per_groups *
> >>>>> -			d->acc100_conf.q_ul_5g.num_qgroups +
> >>>>> -			d->acc100_conf.q_dl_4g.num_aqs_per_groups *
> >>>>> -			d->acc100_conf.q_dl_4g.num_qgroups +
> >>>>> -			d->acc100_conf.q_ul_4g.num_aqs_per_groups *
> >>>>> +	/* Expose number of queues */
> >>>>> +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
> >>>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] =
> >>>>> +d->acc100_conf.q_ul_4g.num_aqs_per_groups *
> >>>>>     			d->acc100_conf.q_ul_4g.num_qgroups;
> >>>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = d-
> >>>>> acc100_conf.q_dl_4g.num_aqs_per_groups *
> >>>>> +			d->acc100_conf.q_dl_4g.num_qgroups;
> >>>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = d-
> >>>>> acc100_conf.q_ul_5g.num_aqs_per_groups *
> >>>>> +			d->acc100_conf.q_ul_5g.num_qgroups;
> >>>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = d-
> >>>>> acc100_conf.q_dl_5g.num_aqs_per_groups *
> >>>>> +			d->acc100_conf.q_dl_5g.num_qgroups;
> >>>>> +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_DEC] =
> d-
> >>>>> acc100_conf.q_ul_4g.num_qgroups;
> >>>>> +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_ENC] =
> d-
> >>>>> acc100_conf.q_dl_4g.num_qgroups;
> >>>>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = d-
> >>>>> acc100_conf.q_ul_5g.num_qgroups;
> >>>>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = d-
> >>>>> acc100_conf.q_dl_5g.num_qgroups;
> >>>>> +	dev_info->max_num_queues = 0;
> >>>>> +	for (i = RTE_BBDEV_OP_TURBO_DEC; i <
> RTE_BBDEV_OP_LDPC_ENC;
> >>>> i++)
> >>>>
> >>>> should this be i <=  ?
> >>>>
> >>> Thanks
> >>>
> >>>>> +		dev_info->max_num_queues += dev_info-
> >num_queues[i];
> >>>>>     	dev_info->queue_size_lim = ACC100_MAX_QUEUE_DEPTH;
> >>>>>     	dev_info->hardware_accelerated = true;
> >>>>>     	dev_info->max_dl_queue_priority = diff --git
> >>>>> a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> >>>>> b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> >>>>> index 57b12af..b4982af 100644
> >>>>> --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> >>>>> +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> >>>>> @@ -379,6 +379,14 @@
> >>>>>     		if (hw_q_id != FPGA_INVALID_HW_QUEUE_ID)
> >>>>>     			dev_info->max_num_queues++;
> >>>>>     	}
> >>>>> +	/* Expose number of queue per operation type */
> >>>>> +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
> >>>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = 0;
> >>>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = 0;
> >>>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] =
> dev_info-
> >>>>> max_num_queues / 2;
> >>>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] =
> dev_info-
> >>>>> max_num_queues / 2;
> >>>>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = 1;
> >>>>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = 1;
> >>>>>     }
> >>>>>
> >>>>>     /**
> >>>>> diff --git a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> >>>>> b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> >>>>> index 2a330c4..dc7f479 100644
> >>>>> --- a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> >>>>> +++ b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
> >>>>> @@ -655,6 +655,14 @@ struct __rte_cache_aligned fpga_queue {
> >>>>>     		if (hw_q_id != FPGA_INVALID_HW_QUEUE_ID)
> >>>>>     			dev_info->max_num_queues++;
> >>>>>     	}
> >>>>> +	/* Expose number of queue per operation type */
> >>>>> +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
> >>>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] =
> dev_info-
> >>>>> max_num_queues / 2;
> >>>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] =
> dev_info-
> >>>>> max_num_queues / 2;
> >>>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = 0;
> >>>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = 0;
> >>>>> +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_DEC] = 1;
> >>>>> +	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_ENC] =
> 1;
> >>>>>     }
> >>>>>
> >>>>>     /**
> >>>>> diff --git a/drivers/baseband/la12xx/bbdev_la12xx.c
> >>>>> b/drivers/baseband/la12xx/bbdev_la12xx.c
> >>>>> index c1f88c6..e99ea9a 100644
> >>>>> --- a/drivers/baseband/la12xx/bbdev_la12xx.c
> >>>>> +++ b/drivers/baseband/la12xx/bbdev_la12xx.c
> >>>>> @@ -102,6 +102,13 @@ struct bbdev_la12xx_params {
> >>>>>     	dev_info->min_alignment = 64;
> >>>>>     	dev_info->device_status =
> RTE_BBDEV_DEV_NOT_SUPPORTED;
> >>>>>
> >>>>> +	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
> >>>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = 0;
> >>>>> +	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = 0;
> >>>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] =
> >>>> LA12XX_MAX_QUEUES / 2;
> >>>>> +	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] =
> >>>> LA12XX_MAX_QUEUES / 2;
> >>>>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = 1;
> >>>>> +	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = 1;
> >>>>>     	rte_bbdev_log_debug("got device info from %u", dev->data-
> >dev_id);
> >>>>>     }
> >>>>>
> >>>>> diff --git a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> >>>>> b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> >>>>> index dbc5524..647e706 100644
> >>>>> --- a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> >>>>> +++ b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
> >>>>> @@ -256,6 +256,17 @@ struct turbo_sw_queue {
> >>>>>     	dev_info->data_endianness = RTE_LITTLE_ENDIAN;
> >>>>>     	dev_info->device_status =
> RTE_BBDEV_DEV_NOT_SUPPORTED;
> >>>>>
> >>>>> +	const struct rte_bbdev_op_cap *op_cap =
> bbdev_capabilities;
> >>>> Should this be done through dev instead of assigning directly ?
> >>> I am not sure I follow your suggestion. Do you mind clarifying?
> >> bbdev_capabilites is a const defined in this function, do you really
> >> need to loop over it to find information that is constant ?
> > I still miss your point. Note that this constant is not always the same at
> build time (based on what SDK it can links to).
> > What would suggest?
> 
> Operations that can be done at compile time, should be.  Useless there is a
> good reason.
> 
> You need to provide a good reason or make the change.

I believe this is more graceful, scalable and maintainable this way. 
At build time we already define the list of capability then we just process that information without risk of disconnect between two pieces of code. 
The drawback is execution time but this function is not time sensititive (enumeration).
I could change it but the code would be poorer with risk of breaking stuff in the future (redundant information in the code hence bug prone).
Ie. defining the number of operations and queues using another serie of  #define. 
From my point of view that would be something that we would not do internally for the reasons above, but if you insist I will just change accordingly so that to move on. 
Not a big deal, let us know. 

Thanks
Nic

> 
> Tom
> 
> >
> > Thanks
> > Nic
> >
> >
> >> Tom
> >>
> >>>> Tom
> >>>>
> >>>>> +	int num_op_type = 0;
> >>>>> +	for (; op_cap->type != RTE_BBDEV_OP_NONE; ++op_cap)
> >>>>> +		num_op_type++;
> >>>>> +	op_cap = bbdev_capabilities;
> >>>>> +	if (num_op_type > 0) {
> >>>>> +		int num_queue_per_type = dev_info-
> >max_num_queues /
> >>>> num_op_type;
> >>>>> +		for (; op_cap->type != RTE_BBDEV_OP_NONE;
> ++op_cap)
> >>>>> +			dev_info->num_queues[op_cap->type] =
> >>>> num_queue_per_type;
> >>>>> +	}
> >>>>> +
> >>>>>     	rte_bbdev_log_debug("got device info from %u\n", dev-
> >data-
> >>>>> dev_id);
> >>>>>     }
> >>>>>
  

Patch

diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c b/drivers/baseband/acc100/rte_acc100_pmd.c
index 17ba798..d568d0d 100644
--- a/drivers/baseband/acc100/rte_acc100_pmd.c
+++ b/drivers/baseband/acc100/rte_acc100_pmd.c
@@ -966,6 +966,7 @@ 
 		struct rte_bbdev_driver_info *dev_info)
 {
 	struct acc100_device *d = dev->data->dev_private;
+	int i;
 
 	static const struct rte_bbdev_op_cap bbdev_capabilities[] = {
 		{
@@ -1062,19 +1063,23 @@ 
 	fetch_acc100_config(dev);
 	dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
 
-	/* This isn't ideal because it reports the maximum number of queues but
-	 * does not provide info on how many can be uplink/downlink or different
-	 * priorities
-	 */
-	dev_info->max_num_queues =
-			d->acc100_conf.q_dl_5g.num_aqs_per_groups *
-			d->acc100_conf.q_dl_5g.num_qgroups +
-			d->acc100_conf.q_ul_5g.num_aqs_per_groups *
-			d->acc100_conf.q_ul_5g.num_qgroups +
-			d->acc100_conf.q_dl_4g.num_aqs_per_groups *
-			d->acc100_conf.q_dl_4g.num_qgroups +
-			d->acc100_conf.q_ul_4g.num_aqs_per_groups *
+	/* Expose number of queues */
+	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
+	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = d->acc100_conf.q_ul_4g.num_aqs_per_groups *
 			d->acc100_conf.q_ul_4g.num_qgroups;
+	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = d->acc100_conf.q_dl_4g.num_aqs_per_groups *
+			d->acc100_conf.q_dl_4g.num_qgroups;
+	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = d->acc100_conf.q_ul_5g.num_aqs_per_groups *
+			d->acc100_conf.q_ul_5g.num_qgroups;
+	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = d->acc100_conf.q_dl_5g.num_aqs_per_groups *
+			d->acc100_conf.q_dl_5g.num_qgroups;
+	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_DEC] = d->acc100_conf.q_ul_4g.num_qgroups;
+	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_ENC] = d->acc100_conf.q_dl_4g.num_qgroups;
+	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = d->acc100_conf.q_ul_5g.num_qgroups;
+	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = d->acc100_conf.q_dl_5g.num_qgroups;
+	dev_info->max_num_queues = 0;
+	for (i = RTE_BBDEV_OP_TURBO_DEC; i < RTE_BBDEV_OP_LDPC_ENC; i++)
+		dev_info->max_num_queues += dev_info->num_queues[i];
 	dev_info->queue_size_lim = ACC100_MAX_QUEUE_DEPTH;
 	dev_info->hardware_accelerated = true;
 	dev_info->max_dl_queue_priority =
diff --git a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
index 57b12af..b4982af 100644
--- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
+++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
@@ -379,6 +379,14 @@ 
 		if (hw_q_id != FPGA_INVALID_HW_QUEUE_ID)
 			dev_info->max_num_queues++;
 	}
+	/* Expose number of queue per operation type */
+	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
+	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = 0;
+	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = 0;
+	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = dev_info->max_num_queues / 2;
+	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = dev_info->max_num_queues / 2;
+	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = 1;
+	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = 1;
 }
 
 /**
diff --git a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
index 2a330c4..dc7f479 100644
--- a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
+++ b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
@@ -655,6 +655,14 @@  struct __rte_cache_aligned fpga_queue {
 		if (hw_q_id != FPGA_INVALID_HW_QUEUE_ID)
 			dev_info->max_num_queues++;
 	}
+	/* Expose number of queue per operation type */
+	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
+	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = dev_info->max_num_queues / 2;
+	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = dev_info->max_num_queues / 2;
+	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = 0;
+	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = 0;
+	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_DEC] = 1;
+	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_ENC] = 1;
 }
 
 /**
diff --git a/drivers/baseband/la12xx/bbdev_la12xx.c b/drivers/baseband/la12xx/bbdev_la12xx.c
index c1f88c6..e99ea9a 100644
--- a/drivers/baseband/la12xx/bbdev_la12xx.c
+++ b/drivers/baseband/la12xx/bbdev_la12xx.c
@@ -102,6 +102,13 @@  struct bbdev_la12xx_params {
 	dev_info->min_alignment = 64;
 	dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
 
+	dev_info->num_queues[RTE_BBDEV_OP_NONE] = 0;
+	dev_info->num_queues[RTE_BBDEV_OP_TURBO_DEC] = 0;
+	dev_info->num_queues[RTE_BBDEV_OP_TURBO_ENC] = 0;
+	dev_info->num_queues[RTE_BBDEV_OP_LDPC_DEC] = LA12XX_MAX_QUEUES / 2;
+	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = LA12XX_MAX_QUEUES / 2;
+	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = 1;
+	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = 1;
 	rte_bbdev_log_debug("got device info from %u", dev->data->dev_id);
 }
 
diff --git a/drivers/baseband/turbo_sw/bbdev_turbo_software.c b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
index dbc5524..647e706 100644
--- a/drivers/baseband/turbo_sw/bbdev_turbo_software.c
+++ b/drivers/baseband/turbo_sw/bbdev_turbo_software.c
@@ -256,6 +256,17 @@  struct turbo_sw_queue {
 	dev_info->data_endianness = RTE_LITTLE_ENDIAN;
 	dev_info->device_status = RTE_BBDEV_DEV_NOT_SUPPORTED;
 
+	const struct rte_bbdev_op_cap *op_cap = bbdev_capabilities;
+	int num_op_type = 0;
+	for (; op_cap->type != RTE_BBDEV_OP_NONE; ++op_cap)
+		num_op_type++;
+	op_cap = bbdev_capabilities;
+	if (num_op_type > 0) {
+		int num_queue_per_type = dev_info->max_num_queues / num_op_type;
+		for (; op_cap->type != RTE_BBDEV_OP_NONE; ++op_cap)
+			dev_info->num_queues[op_cap->type] = num_queue_per_type;
+	}
+
 	rte_bbdev_log_debug("got device info from %u\n", dev->data->dev_id);
 }