Hi Nicolas,
On 5/26/22 00:15, Chautru, Nicolas wrote:
-----Original Message-----
From: Maxime Coquelin <maxime.coque...@redhat.com>
Sent: Wednesday, May 25, 2022 7:33 AM
To: Chautru, Nicolas <nicolas.chau...@intel.com>; dev@dpdk.org;
gak...@marvell.com; t...@redhat.com
Cc: tho...@monjalon.net; Kinsella, Ray <ray.kinse...@intel.com>;
Richardson, Bruce <bruce.richard...@intel.com>;
hemant.agra...@nxp.com; Vargas, Hernan <hernan.var...@intel.com>;
david.march...@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.chau...@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
