Re: [EXT] Re: [PATCH v9 13/14] baseband/acc: add PF configure companion function

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

On 10/11/22 23:18, Chautru, Nicolas wrote:
> Hi Akhil, Maxime,
> 
>> -----Original Message-----
>> From: Akhil Goyal <gakhil@marvell.com>
>> Sent: Monday, October 10, 2022 11:12 AM
>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; Maxime Coquelin
>> <maxime.coquelin@redhat.com>; dev@dpdk.org
>> Cc: trix@redhat.com; mdr@ashroe.eu; Richardson, Bruce
>> <bruce.richardson@intel.com>; hemant.agrawal@nxp.com;
>> david.marchand@redhat.com; stephen@networkplumber.org; Vargas,
>> Hernan <hernan.vargas@intel.com>
>> Subject: RE: [EXT] Re: [PATCH v9 13/14] baseband/acc: add PF configure
>> companion function
>>
>>> Hi Akhil, Maxime,
>>>
>>>> From: Akhil Goyal <gakhil@marvell.com>
>>>> Sent: Monday, October 10, 2022 3:09 AM
>>>> Subject: RE: [EXT] Re: [PATCH v9 13/14] baseband/acc: add PF
>>>> configure companion function
>>>>
>>>>>> diff --git a/drivers/baseband/acc/version.map
>>>>> b/drivers/baseband/acc/version.map
>>>>>> index b4ff13e38f..27fbbe3de5 100644
>>>>>> --- a/drivers/baseband/acc/version.map
>>>>>> +++ b/drivers/baseband/acc/version.map
>>>>>> @@ -6,4 +6,5 @@ EXPERIMENTAL {
>>>>>>    	global:
>>>>>>
>>>>>>    	rte_acc10x_configure;
>>>>>> +	rte_acc200_configure;
>>>>>>    };
>>>>>
>>>>> This is same comment as for ACC100 vs. ACC101.
>>>>> Having a single API would be the way to go, given the prototype of
>>>>> the functions are identical.
>>>>>
>>>>> Keep acc200 function, but internal only, rte_acc_configure() would
>>>>> call the acc100/acc101/acc200/accXXX based on the device ID.
>>>>>
>>>> +1 for this.
>>>>
>>>> I believe a bbdev API should be defined to be used by each of the PMD.
>>>> So that application can be agnostic of the underneath device.
>>>>
>>>> I would recommend to send a deprecation notice to remove all the pmd
>>>> APIs going forward. We can take it for now, but these need to be
>>>> replaced with generic API as soon as possible. No new such PMD API
>>>> would be accepted going forward.
>>>>
>>>
>>> OK understood, we can look into this for 23.03.
>>> Are we okay to keep that commit as is for 22.11?
>>>
>> What Maxime is suggesting is adding a wrapper in PMD to hide variant of
>> acc.
>> I believe it is better to do it now as this is long term stable release.
>> You can send a deprecation notice for removing PMD APIs and adding new
>> generic API now which can be done in next cycle.
> 
> Updated in the V10 as suggested.

Thanks.

> This rte_acc_configure() symbol is marked as experimental. I believe we can remove it without notice (this is already modified in this serie without notice).

Yes, no worries since this is experimental.

> Note that this is only used by bbdev-test so this is all self-contained and no impact to the ecosystem.

Given it is only meant to be used by the dpdk-test-bbdev application,
maybe it could be an internal API? Avoiding to export it would make our
lives easier.

Regards,
Maxime

> Thanks,
> Nic
> 
> 
> 
> 
>