On 9/16/22 03:34, Nic Chautru wrote:
Refactoring all shareable common code to be used by future PMD
(including ACC200 in this patchset as well as taking into account
following PMDs in roadmap) by gathering such structures or inline methods.
Cleaning up the enum files to remove un-used registers definitions.
No functionality change.
Signed-off-by: Nic Chautru <nicolas.chau...@intel.com>
You usually sign-off with Nicolas, but some of the patches of this
series are with Nic.
Acked-by: Bruce Richardson <bruce.richard...@intel.com>
---
app/test-bbdev/test_bbdev_perf.c | 6 +-
drivers/baseband/acc100/acc100_pf_enum.h | 939 -------------
drivers/baseband/acc100/acc100_pmd.h | 449 +------
drivers/baseband/acc100/acc101_pmd.h | 10 -
drivers/baseband/acc100/acc_common.h | 1388 +++++++++++++++++++
drivers/baseband/acc100/rte_acc100_cfg.h | 70 +-
drivers/baseband/acc100/rte_acc100_pmd.c | 1856 ++++++++------------------
drivers/baseband/acc100/rte_acc_common_cfg.h | 101 ++
8 files changed, 2069 insertions(+), 2750 deletions(-)
create mode 100644 drivers/baseband/acc100/acc_common.h
create mode 100644 drivers/baseband/acc100/rte_acc_common_cfg.h
Overall, I think the patch should be split.
For example:
- One patch for rings rework as it introduces new helpers (and this
patch should make use of them in ACC100).
- One patch for the structs renaming and move to common file.
- One patch for registers
- ...
It will make easier for the reviewer to identify discrepancies, and also
will help to identify regressions when using git bisect.
I have not done a full review of this patch, but something caught my eye
wrt to available entries calculation in rings:
diff --git a/drivers/baseband/acc100/acc100_pf_enum.h
b/drivers/baseband/acc100/acc100_pf_enum.h
index 2fba667..f4e5002 100644
--- a/drivers/baseband/acc100/acc100_pf_enum.h
+++ b/drivers/baseband/acc100/acc100_pf_enum.h
@@ -14,32 +14,6 @@
...
+
+/* Number of available descriptor in ring to enqueue */
+static inline uint32_t
+acc_ring_avail_enq(struct acc_queue *q)
+{
+ return (q->sw_ring_depth - 1 + q->sw_ring_tail - q->sw_ring_head) %
q->sw_ring_depth;
+}
+
+/* Number of available descriptor in ring to dequeue */
+static inline uint32_t
+acc_ring_avail_deq(struct acc_queue *q)
+{
+ return (q->sw_ring_depth + q->sw_ring_head - q->sw_ring_tail) %
q->sw_ring_depth;
+}
It is surprising to me that the number of available descriptors
calculation for enqueue and dequeue are différent. Could you please
explain why there a off-by-one delta between enc and dec?
If we look at other avail calculations in ACC100 enqueue, we get this:
int32_t avail = q->sw_ring_depth + q->sw_ring_tail - q->sw_ring_head;
It looks like there is indeed a off-by-one delta even for different
avail enqueue calculation.
Also, these new helpers are introduced but are not used in the patch.
