On 8/30/22 12:45 PM, Chautru, Nicolas wrote:
Hi Maxime,

-----Original Message-----
From: Maxime Coquelin <maxime.coque...@redhat.com>
Sent: Tuesday, August 30, 2022 12:45 AM
To: Chautru, Nicolas <nicolas.chau...@intel.com>; dev@dpdk.org;
tho...@monjalon.net; gak...@marvell.com; hemant.agra...@nxp.com;
t...@redhat.com; Vargas, Hernan <hernan.var...@intel.com>
Cc: m...@ashroe.eu; Richardson, Bruce <bruce.richard...@intel.com>;
david.march...@redhat.com; step...@networkplumber.org
Subject: Re: [PATCH v1 00/10] baseband/acc200

Hi Nicolas,

On 7/12/22 15:48, Maxime Coquelin wrote:
Hi Nicolas, Hernan,

(Adding Hernan in the recipients list)

On 7/8/22 02:01, Nicolas Chautru wrote:
This is targeting 22.11 and includes the PMD for the integrated
accelerator on Intel Xeon SPR-EEC.
There is a dependency on that parallel serie still in-flight which
extends the bbdev api
https://patches.dpdk.org/project/dpdk/list/?series=23894

I will be offline for a few weeks for the summer break but Hernan
will cover for me during that time if required.

Thanks
Nic

Nicolas Chautru (10):
    baseband/acc200: introduce PMD for ACC200
    baseband/acc200: add HW register definitions
    baseband/acc200: add info get function
    baseband/acc200: add queue configuration
    baseband/acc200: add LDPC processing functions
    baseband/acc200: add LTE processing functions
    baseband/acc200: add support for FFT operations
    baseband/acc200: support interrupt
    baseband/acc200: add device status and vf2pf comms
    baseband/acc200: add PF configure companion function

   MAINTAINERS                              |    3 +
   app/test-bbdev/meson.build               |    3 +
   app/test-bbdev/test_bbdev_perf.c         |   76 +
   doc/guides/bbdevs/acc200.rst             |  244 ++
   doc/guides/bbdevs/index.rst              |    1 +
   drivers/baseband/acc200/acc200_pf_enum.h |  468 +++
   drivers/baseband/acc200/acc200_pmd.h     |  690 ++++
   drivers/baseband/acc200/acc200_vf_enum.h |   89 +
   drivers/baseband/acc200/meson.build      |    8 +
   drivers/baseband/acc200/rte_acc200_cfg.h |  115 +
   drivers/baseband/acc200/rte_acc200_pmd.c | 5403
++++++++++++++++++++++++++++++
   drivers/baseband/acc200/version.map      |   10 +
   drivers/baseband/meson.build             |    1 +
   13 files changed, 7111 insertions(+)
   create mode 100644 doc/guides/bbdevs/acc200.rst
   create mode 100644 drivers/baseband/acc200/acc200_pf_enum.h
   create mode 100644 drivers/baseband/acc200/acc200_pmd.h
   create mode 100644 drivers/baseband/acc200/acc200_vf_enum.h
   create mode 100644 drivers/baseband/acc200/meson.build
   create mode 100644 drivers/baseband/acc200/rte_acc200_cfg.h
   create mode 100644 drivers/baseband/acc200/rte_acc200_pmd.c
   create mode 100644 drivers/baseband/acc200/version.map

Comparing ACC200 & ACC100 header files, I understand ACC200 is an
evolution of the ACC10x family. The FEC bits are really close, ACC200
main addition seems to be FFT acceleration which could be handled in
ACC10x driver based on device ID.

I think both drivers have to be merged in order to avoid code
duplication. That's how other families of devices (e.g. i40e) are
handled.
I haven't seen your reply on this point.
Do you confirm you are working on a single driver for ACC family in order to
avoid code duplication?

The implementation is based on distinct ACC100 and ACC200 drivers. The 2 
devices are fundamentally different generation, processes and IP.
MountBryce is an eASIC device over PCIe while ACC200 is an integrated 
accelerator on Xeon CPU.
The actual implementation are not the same, underlying IP are all distinct even 
if many of the descriptor format have similarities.
The actual capabilities of the acceleration are different and/or new.
The workaround and silicon errata are also different causing different 
limitation and implementation in the driver (see the serie with ongoing changes 
for ACC100 in parallel).
This is fundamentally distinct from ACC101 which was a derivative product from 
ACC100 and where it made sense to share implementation between ACC100 and 
ACC101.
So in a nutshell these 2 devices and drivers are 2 different beasts and the 
intention is to keep them intentionally separate as in the serie.
Let me know if unclear, thanks!

Nic,

I used a similarity checker to compare acc100 and acc200

https://dickgrune.com/Programs/similarity_tester/

l=simum.log
if [ -f $l ]; then
    rm $l
fi

sim_c -s -R -o$l -R -p -P -a .

There results are

./acc200/acc200_pf_enum.h consists for 100 % of ./acc100/acc100_pf_enum.h material ./acc100/acc100_pf_enum.h consists for 98 % of ./acc200/acc200_pf_enum.h material ./acc100/rte_acc100_pmd.h consists for 98 % of ./acc200/acc200_pmd.h material ./acc200/acc200_vf_enum.h consists for 95 % of ./acc100/acc100_pf_enum.h material ./acc200/acc200_pmd.h consists for 92 % of ./acc100/rte_acc100_pmd.h material ./acc200/rte_acc200_cfg.h consists for 92 % of ./acc100/rte_acc100_cfg.h material ./acc100/rte_acc100_pmd.c consists for 87 % of ./acc200/rte_acc200_pmd.c material ./acc100/acc100_vf_enum.h consists for 80 % of ./acc200/acc200_pf_enum.h material ./acc200/rte_acc200_pmd.c consists for 78 % of ./acc100/rte_acc100_pmd.c material ./acc100/rte_acc100_cfg.h consists for 75 % of ./acc200/rte_acc200_cfg.h material

Spot checking the first *pf_enum.h at 100%, these are the devices' registers, they are the same.

I raised this similarity issue with 100 vs 101.

Having multiple copies is difficult to support and should be avoided.

For the end user, they should have to use only one driver.

Tom


Thanks
Nic


Maxime

Thanks,
Maxime

Reply via email to