Hi Maxime, 

> -----Original Message-----
> From: Maxime Coquelin <maxime.coque...@redhat.com>
> Sent: Friday, February 10, 2023 6:01 AM
> To: Chautru, Nicolas <nicolas.chau...@intel.com>; Vargas, Hernan
> <hernan.var...@intel.com>; dev@dpdk.org; gak...@marvell.com; Rix, Tom
> <t...@redhat.com>
> Cc: Zhang, Qi Z <qi.z.zh...@intel.com>
> Subject: Re: [PATCH v1 13/13] test/bbdev: remove iteration count check
> 
> Hi Nicolas,
> 
> On 2/9/23 17:59, Chautru, Nicolas wrote:
> > Hi Maxime,
> >
> >> -----Original Message-----
> >> From: Maxime Coquelin <maxime.coque...@redhat.com>
> >> Sent: Thursday, February 9, 2023 1:11 AM
> >> To: Vargas, Hernan <hernan.var...@intel.com>; dev@dpdk.org;
> >> gak...@marvell.com; Rix, Tom <t...@redhat.com>
> >> Cc: Chautru, Nicolas <nicolas.chau...@intel.com>; Zhang, Qi Z
> >> <qi.z.zh...@intel.com>
> >> Subject: Re: [PATCH v1 13/13] test/bbdev: remove iteration count
> >> check
> >>
> >>
> >>
> >> On 2/8/23 21:38, Vargas, Hernan wrote:
> >>> Hi Maxime,
> >>>
> >>> We would like to keep the same signature for validate_dec_op because
> >>> there
> >> are functions such as latency_test_dec that have vector_mask on their
> >> signatures and they pass it to validate_dec_op.
> >>> Let me know if you'd like to discuss more.
> >>
> >> I think this is not a valid reason, just simplify latency_test_dec too.
> >
> > The principle is that all these functions may or may not use that
> > generic operation masks, but we still use a stable (future proof) and
> consistent prototype for these test functions.
> 
> I would agree that it would be necessary if these were callbacks, but that's 
> not
> the case.
> 
> > I believe this is valid and better practice for the test functions, but 
> > again if you
> really want to push back, this could be changed.
> 
> I prefer we do not bloat the code with things that could be useful in an
> hypothetical future.

OK, no big deal. Thanks. 

> 
> Thanks,
> Maxime
> 
> > Thanks!!
> > Nic
> >
> >>
> >> Thanks,
> >> Maxime
> >>
> >>> Thanks,
> >>> Hernan
> >>>
> >>> -----Original Message-----
> >>> From: Maxime Coquelin <maxime.coque...@redhat.com>
> >>> Sent: Tuesday, January 31, 2023 6:36 AM
> >>> To: Vargas, Hernan <hernan.var...@intel.com>; dev@dpdk.org;
> >>> gak...@marvell.com; Rix, Tom <t...@redhat.com>
> >>> Cc: Chautru, Nicolas <nicolas.chau...@intel.com>; Zhang, Qi Z
> >>> <qi.z.zh...@intel.com>
> >>> Subject: Re: [PATCH v1 13/13] test/bbdev: remove iteration count
> >>> check
> >>>
> >>>
> >>>
> >>> On 1/17/23 17:50, Hernan Vargas wrote:
> >>>> To make the test compatible with devices that do not support early
> >>>> termination, the iteration count assert can be removed.
> >>>>
> >>>> Signed-off-by: Hernan Vargas <hernan.var...@intel.com>
> >>>> ---
> >>>>     app/test-bbdev/test_bbdev_perf.c | 6 +-----
> >>>>     1 file changed, 1 insertion(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/app/test-bbdev/test_bbdev_perf.c
> >>>> b/app/test-bbdev/test_bbdev_perf.c
> >>>> index 81bf2c8b60..c68d79cf29 100644
> >>>> --- a/app/test-bbdev/test_bbdev_perf.c
> >>>> +++ b/app/test-bbdev/test_bbdev_perf.c
> >>>> @@ -2290,6 +2290,7 @@ static int
> >>>>     validate_dec_op(struct rte_bbdev_dec_op **ops, const uint16_t n,
> >>>>                  struct rte_bbdev_dec_op *ref_op, const int vector_mask)
> >>>>     {
> >>>> +        RTE_SET_USED(vector_mask);
> >>>
> >>> Why not just remove vector_mask if it isn't of any use instead of
> >>> hiding the
> >> warning?
> >>>
> >>>>          unsigned int i;
> >>>>          int ret;
> >>>>          struct op_data_entries *hard_data_orig = @@ -2299,17 +2300,12
> >> @@
> >>>> validate_dec_op(struct rte_bbdev_dec_op **ops, const uint16_t n,
> >>>>          struct rte_bbdev_op_turbo_dec *ops_td;
> >>>>          struct rte_bbdev_op_data *hard_output;
> >>>>          struct rte_bbdev_op_data *soft_output;
> >>>> -        struct rte_bbdev_op_turbo_dec *ref_td = &ref_op->turbo_dec;
> >>>>
> >>>>          for (i = 0; i < n; ++i) {
> >>>>                  ops_td = &ops[i]->turbo_dec;
> >>>>                  hard_output = &ops_td->hard_output;
> >>>>                  soft_output = &ops_td->soft_output;
> >>>>
> >>>> -                if (vector_mask & TEST_BBDEV_VF_EXPECTED_ITER_COUNT)
> >>>> -                        TEST_ASSERT(ops_td->iter_count <= ref_td-
> >>> iter_count,
> >>>> -                                        "Returned iter_count (%d) > 
> >>>> expected
> >> iter_count (%d)",
> >>>> -                                        ops_td->iter_count, ref_td-
> >>> iter_count);
> >>>>                  ret = check_dec_status_and_ordering(ops[i], i, ref_op-
> >>> status);
> >>>>                  TEST_ASSERT_SUCCESS(ret,
> >>>>                                  "Checking status and ordering for 
> >>>> decoder
> >> failed");
> >>>
> >>> Maxime
> >>>
> >

Reply via email to