diff mbox series

[v5,4/5] baseband/acc100: modify validation code for ACC101

Message ID 1653350912-53876-5-git-send-email-nicolas.chautru@intel.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series drivers/baseband: PMD to support ACC100/ACC101 devices |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Chautru, Nicolas May 24, 2022, 12:08 a.m. UTC 
  The validation requirement is different for the two
devices.

Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
---
 drivers/baseband/acc100/rte_acc100_pmd.c | 47 ++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 12 deletions(-)

Comments

Maxime Coquelin May 25, 2022, 2:33 p.m. UTC | #1 
On 5/24/22 02:08, Nicolas Chautru wrote:
> The validation requirement is different for the two
> devices.
> 
> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> ---
>   drivers/baseband/acc100/rte_acc100_pmd.c | 47 ++++++++++++++++++++++++--------
>   1 file changed, 35 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c b/drivers/baseband/acc100/rte_acc100_pmd.c
> index 41475b2..e3706e0 100644
> --- a/drivers/baseband/acc100/rte_acc100_pmd.c
> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
> @@ -1289,6 +1289,21 @@
>   			RTE_BBDEV_TURBO_HALF_ITERATION_EVEN);
>   }
>   
> +#ifdef RTE_LIBRTE_BBDEV_DEBUG
> +
> +static inline bool
> +is_acc100(struct acc100_queue *q)
> +{
> +	return (q->d->device_variant == ACC100_VARIANT);

I keep insisting, but please rely on the PCI device ID, there is no need
to introduce a new field.

> +}
> +
> +static inline bool
> +validate_op_required(struct acc100_queue *q)
> +{
> +	return is_acc100(q);
> +}
> +#endif
> +
>   /* Fill in a frame control word for LDPC decoding. */
>   static inline void
>   acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc100_fcw_ld *fcw,
> @@ -2176,8 +2191,10 @@ static inline uint32_t hq_index(uint32_t offset)
>   #ifdef RTE_LIBRTE_BBDEV_DEBUG
>   /* Validates turbo encoder parameters */
>   static inline int
> -validate_enc_op(struct rte_bbdev_enc_op *op)
> +validate_enc_op(struct rte_bbdev_enc_op *op, struct acc100_queue *q)
>   {
> +	if (!validate_op_required(q))
> +		return 0;
>   	struct rte_bbdev_op_turbo_enc *turbo_enc = &op->turbo_enc;
>   	struct rte_bbdev_op_enc_turbo_cb_params *cb = NULL;
>   	struct rte_bbdev_op_enc_turbo_tb_params *tb = NULL;
> @@ -2314,8 +2331,10 @@ static inline uint32_t hq_index(uint32_t offset)
>   }
>   /* Validates LDPC encoder parameters */
>   static inline int
> -validate_ldpc_enc_op(struct rte_bbdev_enc_op *op)
> +validate_ldpc_enc_op(struct rte_bbdev_enc_op *op, struct acc100_queue *q)
>   {
> +	if (!validate_op_required(q))
> +		return 0;
>   	struct rte_bbdev_op_ldpc_enc *ldpc_enc = &op->ldpc_enc;
>   
>   	if (op->mempool == NULL) {
> @@ -2367,8 +2386,10 @@ static inline uint32_t hq_index(uint32_t offset)
>   
>   /* Validates LDPC decoder parameters */
>   static inline int
> -validate_ldpc_dec_op(struct rte_bbdev_dec_op *op)
> +validate_ldpc_dec_op(struct rte_bbdev_dec_op *op, struct acc100_queue *q)
>   {
> +	if (!validate_op_required(q))
> +		return 0;
>   	struct rte_bbdev_op_ldpc_dec *ldpc_dec = &op->ldpc_dec;
>   
>   	if (op->mempool == NULL) {
> @@ -2423,7 +2444,7 @@ static inline uint32_t hq_index(uint32_t offset)
>   
>   #ifdef RTE_LIBRTE_BBDEV_DEBUG
>   	/* Validate op structure */
> -	if (validate_enc_op(op) == -1) {
> +	if (validate_enc_op(op, q) == -1) {
>   		rte_bbdev_log(ERR, "Turbo encoder validation failed");
>   		return -EINVAL;
>   	}
> @@ -2477,7 +2498,7 @@ static inline uint32_t hq_index(uint32_t offset)
>   
>   #ifdef RTE_LIBRTE_BBDEV_DEBUG
>   	/* Validate op structure */
> -	if (validate_ldpc_enc_op(ops[0]) == -1) {
> +	if (validate_ldpc_enc_op(ops[0], q) == -1) {
>   		rte_bbdev_log(ERR, "LDPC encoder validation failed");
>   		return -EINVAL;
>   	}
> @@ -2539,7 +2560,7 @@ static inline uint32_t hq_index(uint32_t offset)
>   
>   #ifdef RTE_LIBRTE_BBDEV_DEBUG
>   	/* Validate op structure */
> -	if (validate_ldpc_enc_op(op) == -1) {
> +	if (validate_ldpc_enc_op(op, q) == -1) {
>   		rte_bbdev_log(ERR, "LDPC encoder validation failed");
>   		return -EINVAL;
>   	}
> @@ -2596,7 +2617,7 @@ static inline uint32_t hq_index(uint32_t offset)
>   
>   #ifdef RTE_LIBRTE_BBDEV_DEBUG
>   	/* Validate op structure */
> -	if (validate_enc_op(op) == -1) {
> +	if (validate_enc_op(op, q) == -1) {
>   		rte_bbdev_log(ERR, "Turbo encoder validation failed");
>   		return -EINVAL;
>   	}
> @@ -2669,8 +2690,10 @@ static inline uint32_t hq_index(uint32_t offset)
>   #ifdef RTE_LIBRTE_BBDEV_DEBUG
>   /* Validates turbo decoder parameters */
>   static inline int
> -validate_dec_op(struct rte_bbdev_dec_op *op)
> +validate_dec_op(struct rte_bbdev_dec_op *op, struct acc100_queue *q)
>   {
> +	if (!validate_op_required(q))
> +		return 0;
>   	struct rte_bbdev_op_turbo_dec *turbo_dec = &op->turbo_dec;
>   	struct rte_bbdev_op_dec_turbo_cb_params *cb = NULL;
>   	struct rte_bbdev_op_dec_turbo_tb_params *tb = NULL;
> @@ -2816,7 +2839,7 @@ static inline uint32_t hq_index(uint32_t offset)
>   
>   #ifdef RTE_LIBRTE_BBDEV_DEBUG
>   	/* Validate op structure */
> -	if (validate_dec_op(op) == -1) {
> +	if (validate_dec_op(op, q) == -1) {
>   		rte_bbdev_log(ERR, "Turbo decoder validation failed");
>   		return -EINVAL;
>   	}
> @@ -3041,7 +3064,7 @@ static inline uint32_t hq_index(uint32_t offset)
>   
>   #ifdef RTE_LIBRTE_BBDEV_DEBUG
>   	/* Validate op structure */
> -	if (validate_ldpc_dec_op(op) == -1) {
> +	if (validate_ldpc_dec_op(op, q) == -1) {
>   		rte_bbdev_log(ERR, "LDPC decoder validation failed");
>   		return -EINVAL;
>   	}
> @@ -3145,7 +3168,7 @@ static inline uint32_t hq_index(uint32_t offset)
>   
>   #ifdef RTE_LIBRTE_BBDEV_DEBUG
>   	/* Validate op structure */
> -	if (validate_ldpc_dec_op(op) == -1) {
> +	if (validate_ldpc_dec_op(op, q) == -1) {
>   		rte_bbdev_log(ERR, "LDPC decoder validation failed");
>   		return -EINVAL;
>   	}
> @@ -3235,7 +3258,7 @@ static inline uint32_t hq_index(uint32_t offset)
>   
>   #ifdef RTE_LIBRTE_BBDEV_DEBUG
>   	/* Validate op structure */
> -	if (validate_dec_op(op) == -1) {
> +	if (validate_dec_op(op, q) == -1) {
>   		rte_bbdev_log(ERR, "Turbo decoder validation failed");
>   		return -EINVAL;
>   	}
Chautru, Nicolas May 25, 2022, 10:15 p.m. UTC | #2 
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Wednesday, May 25, 2022 7:33 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> gakhil@marvell.com; trix@redhat.com
> Cc: thomas@monjalon.net; Kinsella, Ray <ray.kinsella@intel.com>;
> Richardson, Bruce <bruce.richardson@intel.com>;
> hemant.agrawal@nxp.com; Vargas, Hernan <hernan.vargas@intel.com>;
> david.marchand@redhat.com
> Subject: Re: [PATCH v5 4/5] baseband/acc100: modify validation code for
> ACC101
> 
> 
> 
> On 5/24/22 02:08, Nicolas Chautru wrote:
> > The validation requirement is different for the two devices.
> >
> > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> > ---
> >   drivers/baseband/acc100/rte_acc100_pmd.c | 47
> ++++++++++++++++++++++++--------
> >   1 file changed, 35 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
> > b/drivers/baseband/acc100/rte_acc100_pmd.c
> > index 41475b2..e3706e0 100644
> > --- a/drivers/baseband/acc100/rte_acc100_pmd.c
> > +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
> > @@ -1289,6 +1289,21 @@
> >   			RTE_BBDEV_TURBO_HALF_ITERATION_EVEN);
> >   }
> >
> > +#ifdef RTE_LIBRTE_BBDEV_DEBUG
> > +
> > +static inline bool
> > +is_acc100(struct acc100_queue *q)
> > +{
> > +	return (q->d->device_variant == ACC100_VARIANT);
> 
> I keep insisting, but please rely on the PCI device ID, there is no need to
> introduce a new field.

Thanks. I have a couple of concerns changing this:
1) the device id is not accessible from the structures currently passed by the functions which would rely on this. Ie. device_id is accessible from rte_bbdev/rte_device but not from acc100_device or acc100_queue. Would be convoluted to have to carry forward this structure when needed instead of using directly acc100_device structure.
2) These call would be done as part of the workload operation where performance matters, best to keep the check as trivial as possible within the PMD. 

Will aim for new patch in next few days based on the other refactory suggestions and unified driver. 

Thanks
Nic

> 
> > +}
> > +
> > +static inline bool
> > +validate_op_required(struct acc100_queue *q) {
> > +	return is_acc100(q);
> > +}
> > +#endif
> > +
> >   /* Fill in a frame control word for LDPC decoding. */
> >   static inline void
> >   acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc100_fcw_ld
> > *fcw, @@ -2176,8 +2191,10 @@ static inline uint32_t hq_index(uint32_t
> offset)
> >   #ifdef RTE_LIBRTE_BBDEV_DEBUG
> >   /* Validates turbo encoder parameters */
> >   static inline int
> > -validate_enc_op(struct rte_bbdev_enc_op *op)
> > +validate_enc_op(struct rte_bbdev_enc_op *op, struct acc100_queue *q)
> >   {
> > +	if (!validate_op_required(q))
> > +		return 0;
> >   	struct rte_bbdev_op_turbo_enc *turbo_enc = &op->turbo_enc;
> >   	struct rte_bbdev_op_enc_turbo_cb_params *cb = NULL;
> >   	struct rte_bbdev_op_enc_turbo_tb_params *tb = NULL; @@ -2314,8
> > +2331,10 @@ static inline uint32_t hq_index(uint32_t offset)
> >   }
> >   /* Validates LDPC encoder parameters */
> >   static inline int
> > -validate_ldpc_enc_op(struct rte_bbdev_enc_op *op)
> > +validate_ldpc_enc_op(struct rte_bbdev_enc_op *op, struct acc100_queue
> > +*q)
> >   {
> > +	if (!validate_op_required(q))
> > +		return 0;
> >   	struct rte_bbdev_op_ldpc_enc *ldpc_enc = &op->ldpc_enc;
> >
> >   	if (op->mempool == NULL) {
> > @@ -2367,8 +2386,10 @@ static inline uint32_t hq_index(uint32_t
> > offset)
> >
> >   /* Validates LDPC decoder parameters */
> >   static inline int
> > -validate_ldpc_dec_op(struct rte_bbdev_dec_op *op)
> > +validate_ldpc_dec_op(struct rte_bbdev_dec_op *op, struct acc100_queue
> > +*q)
> >   {
> > +	if (!validate_op_required(q))
> > +		return 0;
> >   	struct rte_bbdev_op_ldpc_dec *ldpc_dec = &op->ldpc_dec;
> >
> >   	if (op->mempool == NULL) {
> > @@ -2423,7 +2444,7 @@ static inline uint32_t hq_index(uint32_t offset)
> >
> >   #ifdef RTE_LIBRTE_BBDEV_DEBUG
> >   	/* Validate op structure */
> > -	if (validate_enc_op(op) == -1) {
> > +	if (validate_enc_op(op, q) == -1) {
> >   		rte_bbdev_log(ERR, "Turbo encoder validation failed");
> >   		return -EINVAL;
> >   	}
> > @@ -2477,7 +2498,7 @@ static inline uint32_t hq_index(uint32_t offset)
> >
> >   #ifdef RTE_LIBRTE_BBDEV_DEBUG
> >   	/* Validate op structure */
> > -	if (validate_ldpc_enc_op(ops[0]) == -1) {
> > +	if (validate_ldpc_enc_op(ops[0], q) == -1) {
> >   		rte_bbdev_log(ERR, "LDPC encoder validation failed");
> >   		return -EINVAL;
> >   	}
> > @@ -2539,7 +2560,7 @@ static inline uint32_t hq_index(uint32_t offset)
> >
> >   #ifdef RTE_LIBRTE_BBDEV_DEBUG
> >   	/* Validate op structure */
> > -	if (validate_ldpc_enc_op(op) == -1) {
> > +	if (validate_ldpc_enc_op(op, q) == -1) {
> >   		rte_bbdev_log(ERR, "LDPC encoder validation failed");
> >   		return -EINVAL;
> >   	}
> > @@ -2596,7 +2617,7 @@ static inline uint32_t hq_index(uint32_t offset)
> >
> >   #ifdef RTE_LIBRTE_BBDEV_DEBUG
> >   	/* Validate op structure */
> > -	if (validate_enc_op(op) == -1) {
> > +	if (validate_enc_op(op, q) == -1) {
> >   		rte_bbdev_log(ERR, "Turbo encoder validation failed");
> >   		return -EINVAL;
> >   	}
> > @@ -2669,8 +2690,10 @@ static inline uint32_t hq_index(uint32_t offset)
> >   #ifdef RTE_LIBRTE_BBDEV_DEBUG
> >   /* Validates turbo decoder parameters */
> >   static inline int
> > -validate_dec_op(struct rte_bbdev_dec_op *op)
> > +validate_dec_op(struct rte_bbdev_dec_op *op, struct acc100_queue *q)
> >   {
> > +	if (!validate_op_required(q))
> > +		return 0;
> >   	struct rte_bbdev_op_turbo_dec *turbo_dec = &op->turbo_dec;
> >   	struct rte_bbdev_op_dec_turbo_cb_params *cb = NULL;
> >   	struct rte_bbdev_op_dec_turbo_tb_params *tb = NULL; @@ -2816,7
> > +2839,7 @@ static inline uint32_t hq_index(uint32_t offset)
> >
> >   #ifdef RTE_LIBRTE_BBDEV_DEBUG
> >   	/* Validate op structure */
> > -	if (validate_dec_op(op) == -1) {
> > +	if (validate_dec_op(op, q) == -1) {
> >   		rte_bbdev_log(ERR, "Turbo decoder validation failed");
> >   		return -EINVAL;
> >   	}
> > @@ -3041,7 +3064,7 @@ static inline uint32_t hq_index(uint32_t offset)
> >
> >   #ifdef RTE_LIBRTE_BBDEV_DEBUG
> >   	/* Validate op structure */
> > -	if (validate_ldpc_dec_op(op) == -1) {
> > +	if (validate_ldpc_dec_op(op, q) == -1) {
> >   		rte_bbdev_log(ERR, "LDPC decoder validation failed");
> >   		return -EINVAL;
> >   	}
> > @@ -3145,7 +3168,7 @@ static inline uint32_t hq_index(uint32_t offset)
> >
> >   #ifdef RTE_LIBRTE_BBDEV_DEBUG
> >   	/* Validate op structure */
> > -	if (validate_ldpc_dec_op(op) == -1) {
> > +	if (validate_ldpc_dec_op(op, q) == -1) {
> >   		rte_bbdev_log(ERR, "LDPC decoder validation failed");
> >   		return -EINVAL;
> >   	}
> > @@ -3235,7 +3258,7 @@ static inline uint32_t hq_index(uint32_t offset)
> >
> >   #ifdef RTE_LIBRTE_BBDEV_DEBUG
> >   	/* Validate op structure */
> > -	if (validate_dec_op(op) == -1) {
> > +	if (validate_dec_op(op, q) == -1) {
> >   		rte_bbdev_log(ERR, "Turbo decoder validation failed");
> >   		return -EINVAL;
> >   	}
Maxime Coquelin May 31, 2022, 7:59 a.m. UTC | #3 
Hi Nicolas,

On 5/26/22 00:15, Chautru, Nicolas wrote:
> 
> 
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Wednesday, May 25, 2022 7:33 AM
>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
>> gakhil@marvell.com; trix@redhat.com
>> Cc: thomas@monjalon.net; Kinsella, Ray <ray.kinsella@intel.com>;
>> Richardson, Bruce <bruce.richardson@intel.com>;
>> hemant.agrawal@nxp.com; Vargas, Hernan <hernan.vargas@intel.com>;
>> david.marchand@redhat.com
>> Subject: Re: [PATCH v5 4/5] baseband/acc100: modify validation code for
>> ACC101
>>
>>
>>
>> On 5/24/22 02:08, Nicolas Chautru wrote:
>>> The validation requirement is different for the two devices.
>>>
>>> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
>>> ---
>>>    drivers/baseband/acc100/rte_acc100_pmd.c | 47
>> ++++++++++++++++++++++++--------
>>>    1 file changed, 35 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
>>> b/drivers/baseband/acc100/rte_acc100_pmd.c
>>> index 41475b2..e3706e0 100644
>>> --- a/drivers/baseband/acc100/rte_acc100_pmd.c
>>> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
>>> @@ -1289,6 +1289,21 @@
>>>    			RTE_BBDEV_TURBO_HALF_ITERATION_EVEN);
>>>    }
>>>
>>> +#ifdef RTE_LIBRTE_BBDEV_DEBUG
>>> +
>>> +static inline bool
>>> +is_acc100(struct acc100_queue *q)
>>> +{
>>> +	return (q->d->device_variant == ACC100_VARIANT);
>>
>> I keep insisting, but please rely on the PCI device ID, there is no need to
>> introduce a new field.
> 
> Thanks. I have a couple of concerns changing this:
> 1) the device id is not accessible from the structures currently passed by the functions which would rely on this. Ie. device_id is accessible from rte_bbdev/rte_device but not from acc100_device or acc100_queue. Would be convoluted to have to carry forward this structure when needed instead of using directly acc100_device structure.
> 2) These call would be done as part of the workload operation where performance matters, best to keep the check as trivial as possible within the PMD.

I think it is better to have a pointer to the rte_bbdev/rte_device than
introducing a new ID.

Regarding performance, is_acc100 is only defined in
RTE_LIBRTE_BBDEV_DEBUG, so it should not be that critical.

Regarding that, I have hard time to understand why we need to validate
encoder/decoder parameters in ACC100 case, but not in ACC101 one. For
example, I guess having a valid mempool pointer is required in both
ACC100 and ACC101 cases.

> Will aim for new patch in next few days based on the other refactory suggestions and unified driver.
> 
> Thanks
> Nic

Thanks,
Maxime
Chautru, Nicolas May 31, 2022, 6:19 p.m. UTC | #4 
Hi Maxime, 

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, May 31, 2022 1:00 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> gakhil@marvell.com; trix@redhat.com
> Cc: thomas@monjalon.net; Kinsella, Ray <ray.kinsella@intel.com>;
> Richardson, Bruce <bruce.richardson@intel.com>;
> hemant.agrawal@nxp.com; Vargas, Hernan <hernan.vargas@intel.com>;
> david.marchand@redhat.com
> Subject: Re: [PATCH v5 4/5] baseband/acc100: modify validation code for
> ACC101
> 
> Hi Nicolas,
> 
> On 5/26/22 00:15, Chautru, Nicolas wrote:
> >
> >
> >> -----Original Message-----
> >> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >> Sent: Wednesday, May 25, 2022 7:33 AM
> >> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> >> gakhil@marvell.com; trix@redhat.com
> >> Cc: thomas@monjalon.net; Kinsella, Ray <ray.kinsella@intel.com>;
> >> Richardson, Bruce <bruce.richardson@intel.com>;
> >> hemant.agrawal@nxp.com; Vargas, Hernan <hernan.vargas@intel.com>;
> >> david.marchand@redhat.com
> >> Subject: Re: [PATCH v5 4/5] baseband/acc100: modify validation code
> >> for
> >> ACC101
> >>
> >>
> >>
> >> On 5/24/22 02:08, Nicolas Chautru wrote:
> >>> The validation requirement is different for the two devices.
> >>>
> >>> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> >>> ---
> >>>    drivers/baseband/acc100/rte_acc100_pmd.c | 47
> >> ++++++++++++++++++++++++--------
> >>>    1 file changed, 35 insertions(+), 12 deletions(-)
> >>>
> >>> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
> >>> b/drivers/baseband/acc100/rte_acc100_pmd.c
> >>> index 41475b2..e3706e0 100644
> >>> --- a/drivers/baseband/acc100/rte_acc100_pmd.c
> >>> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
> >>> @@ -1289,6 +1289,21 @@
> >>>    			RTE_BBDEV_TURBO_HALF_ITERATION_EVEN);
> >>>    }
> >>>
> >>> +#ifdef RTE_LIBRTE_BBDEV_DEBUG
> >>> +
> >>> +static inline bool
> >>> +is_acc100(struct acc100_queue *q)
> >>> +{
> >>> +	return (q->d->device_variant == ACC100_VARIANT);
> >>
> >> I keep insisting, but please rely on the PCI device ID, there is no
> >> need to introduce a new field.
> >
> > Thanks. I have a couple of concerns changing this:
> > 1) the device id is not accessible from the structures currently passed by the
> functions which would rely on this. Ie. device_id is accessible from
> rte_bbdev/rte_device but not from acc100_device or acc100_queue. Would
> be convoluted to have to carry forward this structure when needed instead
> of using directly acc100_device structure.
> > 2) These call would be done as part of the workload operation where
> performance matters, best to keep the check as trivial as possible within the
> PMD.
> 
> I think it is better to have a pointer to the rte_bbdev/rte_device than
> introducing a new ID.
> 
> Regarding performance, is_acc100 is only defined in
> RTE_LIBRTE_BBDEV_DEBUG, so it should not be that critical.

That function is not intended to be only used for debug purpose but more generally for other handling
when the code would differ from ACC100 vs ACC101, includes future changes targeting next release. 

> 
> Regarding that, I have hard time to understand why we need to validate
> encoder/decoder parameters in ACC100 case, but not in ACC101 one. For
> example, I guess having a valid mempool pointer is required in both
> ACC100 and ACC101 cases.

This is due to difference in HW implementation in ACC100 and ACC101, ACC101 HW interface
would not require validation in driver of the input API. 

> 
> > Will aim for new patch in next few days based on the other refactory
> suggestions and unified driver.
> >
> > Thanks
> > Nic
> 
> Thanks,
> Maxime
diff mbox series

Patch

diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c b/drivers/baseband/acc100/rte_acc100_pmd.c
index 41475b2..e3706e0 100644
--- a/drivers/baseband/acc100/rte_acc100_pmd.c
+++ b/drivers/baseband/acc100/rte_acc100_pmd.c
@@ -1289,6 +1289,21 @@ 
 			RTE_BBDEV_TURBO_HALF_ITERATION_EVEN);
 }
 
+#ifdef RTE_LIBRTE_BBDEV_DEBUG
+
+static inline bool
+is_acc100(struct acc100_queue *q)
+{
+	return (q->d->device_variant == ACC100_VARIANT);
+}
+
+static inline bool
+validate_op_required(struct acc100_queue *q)
+{
+	return is_acc100(q);
+}
+#endif
+
 /* Fill in a frame control word for LDPC decoding. */
 static inline void
 acc100_fcw_ld_fill(struct rte_bbdev_dec_op *op, struct acc100_fcw_ld *fcw,
@@ -2176,8 +2191,10 @@  static inline uint32_t hq_index(uint32_t offset)
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 /* Validates turbo encoder parameters */
 static inline int
-validate_enc_op(struct rte_bbdev_enc_op *op)
+validate_enc_op(struct rte_bbdev_enc_op *op, struct acc100_queue *q)
 {
+	if (!validate_op_required(q))
+		return 0;
 	struct rte_bbdev_op_turbo_enc *turbo_enc = &op->turbo_enc;
 	struct rte_bbdev_op_enc_turbo_cb_params *cb = NULL;
 	struct rte_bbdev_op_enc_turbo_tb_params *tb = NULL;
@@ -2314,8 +2331,10 @@  static inline uint32_t hq_index(uint32_t offset)
 }
 /* Validates LDPC encoder parameters */
 static inline int
-validate_ldpc_enc_op(struct rte_bbdev_enc_op *op)
+validate_ldpc_enc_op(struct rte_bbdev_enc_op *op, struct acc100_queue *q)
 {
+	if (!validate_op_required(q))
+		return 0;
 	struct rte_bbdev_op_ldpc_enc *ldpc_enc = &op->ldpc_enc;
 
 	if (op->mempool == NULL) {
@@ -2367,8 +2386,10 @@  static inline uint32_t hq_index(uint32_t offset)
 
 /* Validates LDPC decoder parameters */
 static inline int
-validate_ldpc_dec_op(struct rte_bbdev_dec_op *op)
+validate_ldpc_dec_op(struct rte_bbdev_dec_op *op, struct acc100_queue *q)
 {
+	if (!validate_op_required(q))
+		return 0;
 	struct rte_bbdev_op_ldpc_dec *ldpc_dec = &op->ldpc_dec;
 
 	if (op->mempool == NULL) {
@@ -2423,7 +2444,7 @@  static inline uint32_t hq_index(uint32_t offset)
 
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	/* Validate op structure */
-	if (validate_enc_op(op) == -1) {
+	if (validate_enc_op(op, q) == -1) {
 		rte_bbdev_log(ERR, "Turbo encoder validation failed");
 		return -EINVAL;
 	}
@@ -2477,7 +2498,7 @@  static inline uint32_t hq_index(uint32_t offset)
 
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	/* Validate op structure */
-	if (validate_ldpc_enc_op(ops[0]) == -1) {
+	if (validate_ldpc_enc_op(ops[0], q) == -1) {
 		rte_bbdev_log(ERR, "LDPC encoder validation failed");
 		return -EINVAL;
 	}
@@ -2539,7 +2560,7 @@  static inline uint32_t hq_index(uint32_t offset)
 
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	/* Validate op structure */
-	if (validate_ldpc_enc_op(op) == -1) {
+	if (validate_ldpc_enc_op(op, q) == -1) {
 		rte_bbdev_log(ERR, "LDPC encoder validation failed");
 		return -EINVAL;
 	}
@@ -2596,7 +2617,7 @@  static inline uint32_t hq_index(uint32_t offset)
 
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	/* Validate op structure */
-	if (validate_enc_op(op) == -1) {
+	if (validate_enc_op(op, q) == -1) {
 		rte_bbdev_log(ERR, "Turbo encoder validation failed");
 		return -EINVAL;
 	}
@@ -2669,8 +2690,10 @@  static inline uint32_t hq_index(uint32_t offset)
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 /* Validates turbo decoder parameters */
 static inline int
-validate_dec_op(struct rte_bbdev_dec_op *op)
+validate_dec_op(struct rte_bbdev_dec_op *op, struct acc100_queue *q)
 {
+	if (!validate_op_required(q))
+		return 0;
 	struct rte_bbdev_op_turbo_dec *turbo_dec = &op->turbo_dec;
 	struct rte_bbdev_op_dec_turbo_cb_params *cb = NULL;
 	struct rte_bbdev_op_dec_turbo_tb_params *tb = NULL;
@@ -2816,7 +2839,7 @@  static inline uint32_t hq_index(uint32_t offset)
 
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	/* Validate op structure */
-	if (validate_dec_op(op) == -1) {
+	if (validate_dec_op(op, q) == -1) {
 		rte_bbdev_log(ERR, "Turbo decoder validation failed");
 		return -EINVAL;
 	}
@@ -3041,7 +3064,7 @@  static inline uint32_t hq_index(uint32_t offset)
 
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	/* Validate op structure */
-	if (validate_ldpc_dec_op(op) == -1) {
+	if (validate_ldpc_dec_op(op, q) == -1) {
 		rte_bbdev_log(ERR, "LDPC decoder validation failed");
 		return -EINVAL;
 	}
@@ -3145,7 +3168,7 @@  static inline uint32_t hq_index(uint32_t offset)
 
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	/* Validate op structure */
-	if (validate_ldpc_dec_op(op) == -1) {
+	if (validate_ldpc_dec_op(op, q) == -1) {
 		rte_bbdev_log(ERR, "LDPC decoder validation failed");
 		return -EINVAL;
 	}
@@ -3235,7 +3258,7 @@  static inline uint32_t hq_index(uint32_t offset)
 
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	/* Validate op structure */
-	if (validate_dec_op(op) == -1) {
+	if (validate_dec_op(op, q) == -1) {
 		rte_bbdev_log(ERR, "Turbo decoder validation failed");
 		return -EINVAL;
 	}