Hi Nicolas, I spoke with the code author and this file was not intended to be upstreamed. It will be removed in V2. -John > On Oct 26, 2022, at 7:22 PM, Chautru, Nicolas wrote: > > Hi John, > >> -----Original Message----- >> From: John Miller > >> Sent: Wednesday, October 26, 2022 12:46 PM >> To: Chautru, Nicolas > >> Cc: dev@dpdk.org ; ed.czeck@atomicrules.com ; Shepard Siegel >> >; John Miller >> > >> Subject: [PATCH 11/14] baseband/ark: introduce ark baseband driver custom >> functions >> >> This patch introduces the Arkville baseband device driver custom functions. >> >> Signed-off-by: John Miller >> --- >> drivers/baseband/ark/ark_bbdev_custom.c | 201 ++++++++++++++++++++++++ >> drivers/baseband/ark/ark_bbdev_custom.h | 30 ++++ >> 2 files changed, 231 insertions(+) >> create mode 100644 drivers/baseband/ark/ark_bbdev_custom.c >> create mode 100644 drivers/baseband/ark/ark_bbdev_custom.h >> >> diff --git a/drivers/baseband/ark/ark_bbdev_custom.c >> b/drivers/baseband/ark/ark_bbdev_custom.c >> new file mode 100644 >> index 0000000000..6b1553abe1 >> --- /dev/null >> +++ b/drivers/baseband/ark/ark_bbdev_custom.c >> @@ -0,0 +1,201 @@ >> +/* SPDX-License-Identifier: BSD-3-Clause >> + * Copyright(c) 2016-2021 Atomic Rules LLC */ >> + >> +#include >> +#include >> + >> +#include >> +#include /* For debug */ >> + >> + >> +#include "ark_bbdev_common.h" >> +#include "ark_bbdev_custom.h" >> + >> +/* It is expected that functions in this file will be modified based on >> + * specifics of the FPGA hardware beyond the core Arkville >> + * components. >> + */ >> + >> +/* bytyes must be range of 0 to 20 */ >> +static inline >> +uint8_t ark_bb_cvt_bytes_meta_cnt(size_t bytes) { >> + return (bytes + 3) / 8; >> +} >> + >> +void >> +ark_bbdev_info_get(struct rte_bbdev *dev, >> + struct rte_bbdev_driver_info *dev_info) { >> + struct ark_bbdevice *ark_bb = dev->data->dev_private; >> + > > In your documentation in first commit you mentioned this > * Support for LDPC encode and decode operations. > * Support for Turbo encode and decode operations. > But I only see LDPC below. More generally not really matching the doc I think. Good to have the code and docs in same commits for that reason. > >> + static const struct rte_bbdev_op_cap bbdev_capabilities[] = { >> + { >> + .type = RTE_BBDEV_OP_LDPC_DEC, >> + .cap.ldpc_dec = { >> + .capability_flags = >> + RTE_BBDEV_LDPC_CRC_24B_ATTACH >> | >> + RTE_BBDEV_LDPC_RATE_MATCH, > > It doesn't look right > Basically there se flags are not for LDPC_DEC but for the encoder > There is no HARQ combine etc? > I would have expected scatter gather here as well based on your documentation > >> + .num_buffers_src = >> + >> RTE_BBDEV_LDPC_MAX_CODE_BLOCKS, >> + .num_buffers_hard_out = >> + >> RTE_BBDEV_LDPC_MAX_CODE_BLOCKS >> + } >> + }, >> + { >> + .type = RTE_BBDEV_OP_LDPC_ENC, >> + .cap.ldpc_enc = { >> + .capability_flags = >> + RTE_BBDEV_LDPC_CRC_24B_ATTACH >> | >> + RTE_BBDEV_LDPC_RATE_MATCH, >> + .num_buffers_src = >> + >> RTE_BBDEV_LDPC_MAX_CODE_BLOCKS, >> + .num_buffers_dst = >> + >> RTE_BBDEV_LDPC_MAX_CODE_BLOCKS >> + } >> + }, >> + RTE_BBDEV_END_OF_CAPABILITIES_LIST(), >> + }; >> + >> + static struct rte_bbdev_queue_conf default_queue_conf = { >> + .queue_size = RTE_BBDEV_QUEUE_SIZE_LIMIT, >> + }; >> + >> + default_queue_conf.socket = dev->data->socket_id; >> + >> + dev_info->driver_name = RTE_STR(DRIVER_NAME); >> + dev_info->max_num_queues = ark_bb->max_nb_queues; >> + dev_info->queue_size_lim = RTE_BBDEV_QUEUE_SIZE_LIMIT; >> + dev_info->hardware_accelerated = true; >> + dev_info->max_dl_queue_priority = 0; >> + dev_info->max_ul_queue_priority = 0; >> + dev_info->default_queue_conf = default_queue_conf; >> + dev_info->capabilities = bbdev_capabilities; >> + dev_info->cpu_flag_reqs = NULL; >> + dev_info->min_alignment = 4; > > Is there really a 4 Bytes alignment requirement per code blocks? That sounds extremely cumbersome, is that what you really mean? > >> + >> +} >> + >> +/* Structure defining layout of the ldpc command struct */ struct >> +ark_bb_ldpc_enc_meta { >> + uint16_t header; >> + uint8_t rv_index:2, >> + basegraph:1, >> + code_block_mode:1, >> + rfu_71_68:4; > > What is this? Just curious. > >> + >> + uint8_t q_m; >> + uint32_t e_ea; >> + uint32_t eb; >> + uint8_t c; >> + uint8_t cab; >> + uint16_t n_cb; >> + uint16_t pad; >> + uint16_t trailer; >> +} __rte_packed; >> + >> +/* The size must be less then 20 Bytes */ static_assert(sizeof(struct >> +ark_bb_ldpc_enc_meta) <= 20, "struct size"); >> + >> +/* Custom operation on equeue ldpc operation */ > > Typo enqueue > >> +/* Do these function need queue number? */ > > Who is the question for? Through bbdev api the queue index is expected, and from your documentation I believe you support multiple queues. > >> +/* Maximum of 20 bytes */ >> +int >> +ark_bb_user_enqueue_ldpc_enc(struct rte_bbdev_enc_op *enc_op, >> + uint32_t *meta, uint8_t *meta_cnt) { >> + struct rte_bbdev_op_ldpc_enc *ldpc_enc_op = &enc_op->ldpc_enc; >> + struct ark_bb_ldpc_enc_meta *src = (struct ark_bb_ldpc_enc_meta >> +*)meta; >> + >> + src->header = 0x4321; /* For testings */ >> + src->trailer = 0xFEDC; >> + >> + src->rv_index = ldpc_enc_op->rv_index; >> + src->basegraph = ldpc_enc_op->basegraph; >> + src->code_block_mode = ldpc_enc_op->code_block_mode; >> + >> + src->q_m = ldpc_enc_op->q_m; >> + src->e_ea = 0xABCD; >> + src->eb = ldpc_enc_op->tb_params.eb; >> + src->c = ldpc_enc_op->tb_params.c; >> + src->cab = ldpc_enc_op->tb_params.cab; >> + >> + src->n_cb = 0; >> + >> + meta[0] = 0x11111110; >> + meta[1] = 0x22222220; >> + meta[2] = 0x33333330; >> + meta[3] = 0x44444440; >> + meta[4] = 0x55555550; > > Should these be defined separately? > >> + >> + *meta_cnt = ark_bb_cvt_bytes_meta_cnt( >> + sizeof(struct ark_bb_ldpc_enc_meta)); >> + return 0; >> +} >> + >> +/* Custom operation on dequeue ldpc operation */ int >> +ark_bb_user_dequeue_ldpc_enc(struct rte_bbdev_enc_op *enc_op, >> + const uint32_t *usermeta) >> +{ >> + static int dump; /* = 0 */ >> + /* Just compare with what was sent? */ >> + uint32_t meta_in[5] = {0}; >> + uint8_t meta_cnt; >> + >> + ark_bb_user_enqueue_ldpc_enc(enc_op, meta_in, &meta_cnt); >> + if (memcmp(usermeta, meta_in, 3 + (meta_cnt * 8))) { >> + fprintf(stderr, >> + "------------------------------------------\n"); >> + rte_hexdump(stdout, "meta difference for lpdc_enc IN", >> + meta_in, 20); >> + rte_hexdump(stdout, "meta difference for lpdc_enc OUT", >> + usermeta, 20); >> + } else if (dump) { >> + rte_hexdump(stdout, "DUMP lpdc_enc IN", usermeta, 20); >> + dump--; >> + } >> + >> + return 0; >> +} >> + >> + >> +/* Turbo op call backs for user meta data */ int >> +ark_bb_user_enqueue_ldpc_dec(struct rte_bbdev_dec_op *enc_op, >> + uint32_t *meta, uint8_t *meta_cnt) { >> + RTE_SET_USED(enc_op); > > Is the implementation missing? > > The enc_op should be called differently. > >> + meta[0] = 0xF1111110; >> + meta[1] = 0xF2222220; >> + meta[2] = 0xF3333330; >> + meta[3] = 0xF4444440; >> + meta[4] = 0xF5555550; >> + >> + *meta_cnt = ark_bb_cvt_bytes_meta_cnt(20); >> + return 0; >> +} >> + >> +int ark_bb_user_dequeue_ldpc_dec(struct rte_bbdev_dec_op *enc_op, >> + const uint32_t *usermeta) >> +{ >> + RTE_SET_USED(enc_op); >> + static int dump; /* = 0 */ >> + /* Just compare with what was sent? */ > > That looks like still testcode isn't it? Doing some loopback. > >> + uint32_t meta_in[5] = {0}; >> + uint8_t meta_cnt; >> + >> + ark_bb_user_enqueue_ldpc_dec(enc_op, meta_in, &meta_cnt); >> + if (memcmp(usermeta, meta_in, 3 + (meta_cnt * 8))) { >> + fprintf(stderr, >> + "------------------------------------------\n"); >> + rte_hexdump(stdout, "meta difference for lpdc_enc IN", >> + meta_in, 20); >> + rte_hexdump(stdout, "meta difference for lpdc_enc OUT", >> + usermeta, 20); >> + } else if (dump) { >> + rte_hexdump(stdout, "DUMP lpdc_enc IN", usermeta, 20); >> + dump--; >> + } >> + return 0; >> +} >> diff --git a/drivers/baseband/ark/ark_bbdev_custom.h >> b/drivers/baseband/ark/ark_bbdev_custom.h >> new file mode 100644 >> index 0000000000..32a2ef6bb6 >> --- /dev/null >> +++ b/drivers/baseband/ark/ark_bbdev_custom.h >> @@ -0,0 +1,30 @@ >> +/* SPDX-License-Identifier: BSD-3-Clause >> + * Copyright(c) 2016-2021 Atomic Rules LLC */ >> + >> +#ifndef _ARK_BBDEV_CUSTOM_H_ >> +#define _ARK_BBDEV_CUSTOM_H_ >> + >> +#include >> + >> +/* Forward declarations */ >> +struct rte_bbdev; >> +struct rte_bbdev_driver_info; >> +struct rte_bbdev_enc_op; >> +struct rte_bbdev_dec_op; >> +struct rte_mbuf; >> + >> +void ark_bbdev_info_get(struct rte_bbdev *dev, >> + struct rte_bbdev_driver_info *dev_info); >> + >> +int ark_bb_user_enqueue_ldpc_dec(struct rte_bbdev_dec_op *enc_op, >> + uint32_t *meta, uint8_t *meta_cnt); int >> +ark_bb_user_dequeue_ldpc_dec(struct rte_bbdev_dec_op *enc_op, >> + const uint32_t *usermeta); >> + >> +int ark_bb_user_enqueue_ldpc_enc(struct rte_bbdev_enc_op *enc_op, >> + uint32_t *meta, uint8_t *meta_cnt); int >> +ark_bb_user_dequeue_ldpc_enc(struct rte_bbdev_enc_op *enc_op, >> + const uint32_t *usermeta); >> + >> +#endif >> -- >> 2.25.1