From: John Miller <john.miller@atomicrules.com>
To: "Chautru, Nicolas" <nicolas.chautru@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"ed.czeck@atomicrules.com" <ed.czeck@atomicrules.com>,
Shepard Siegel <shepard.siegel@atomicrules.com>,
Maxime Coquelin <maxime.coquelin@redhat.com>
Subject: Re: [PATCH 11/14] baseband/ark: introduce ark baseband driver custom functions
Date: Fri, 4 Nov 2022 07:35:05 -0400 [thread overview]
Message-ID: <6A2FEB4C-95FC-43D8-88C1-3CDBFA4E3CA7@atomicrules.com> (raw)
In-Reply-To: <BY5PR11MB445192CE9AB99004B33938BAF8309@BY5PR11MB4451.namprd11.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 10020 bytes --]
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 <nicolas.chautru@intel.com> wrote:
>
> Hi John,
>
>> -----Original Message-----
>> From: John Miller <john.miller@atomicrules.com <mailto:john.miller@atomicrules.com>>
>> Sent: Wednesday, October 26, 2022 12:46 PM
>> To: Chautru, Nicolas <nicolas.chautru@intel.com <mailto:nicolas.chautru@intel.com>>
>> Cc: dev@dpdk.org <mailto:dev@dpdk.org>; ed.czeck@atomicrules.com <mailto:ed.czeck@atomicrules.com>; Shepard Siegel
>> <shepard.siegel@atomicrules.com <mailto:shepard.siegel@atomicrules.com>>; John Miller
>> <john.miller@atomicrules.com <mailto:john.miller@atomicrules.com>>
>> 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 <john.miller@atomicrules.com>
>> ---
>> 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 <rte_bbdev.h>
>> +#include <rte_bbdev_pmd.h>
>> +
>> +#include <rte_mbuf.h>
>> +#include <rte_hexdump.h> /* 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 <stdint.h>
>> +
>> +/* 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
[-- Attachment #2: Type: text/html, Size: 51598 bytes --]
next prev parent reply other threads:[~2022-11-04 11:35 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-26 19:46 [PATCH 01/14] doc/guides/bbdevs: add ark baseband device documentation John Miller
2022-10-26 19:46 ` [PATCH 02/14] common/ark: create common subdirectory for baseband support John Miller
2022-10-26 19:46 ` [PATCH 03/14] common/ark: move common files to common subdirectory John Miller
2022-10-26 19:46 ` [PATCH 04/14] common/meson.build: John Miller
2022-10-26 19:46 ` [PATCH 05/14] net/ark: remove build files moved to common John Miller
2022-10-26 19:46 ` [PATCH 06/14] common/ark: update version map file John Miller
2022-10-26 19:46 ` [PATCH 07/14] common/ark: avoid exporting internal functions John Miller
2022-10-26 19:46 ` [PATCH 08/14] net/ark: add ark PMD log interface John Miller
2022-10-26 19:46 ` [PATCH 09/14] common/ark: add VF support to caps record John Miller
2022-10-26 19:46 ` [PATCH 10/14] baseband/ark: introduce ark baseband driver John Miller
2022-10-26 23:11 ` Chautru, Nicolas
2022-10-31 17:33 ` John Miller
2022-10-31 21:15 ` Chautru, Nicolas
2022-10-26 19:46 ` [PATCH 11/14] baseband/ark: introduce ark baseband driver custom functions John Miller
2022-10-26 23:22 ` Chautru, Nicolas
2022-11-04 11:35 ` John Miller [this message]
2022-10-26 19:46 ` [PATCH 12/14] baseband/ark: introduce ark baseband driver common functions John Miller
2022-10-26 19:46 ` [PATCH 13/14] baseband/ark: introduce ark baseband build files John Miller
2022-10-26 19:46 ` [PATCH 14/14] baseband/meson.build: John Miller
2022-12-15 14:18 ` [PATCH 01/14] doc/guides/bbdevs: add ark baseband device documentation Maxime Coquelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6A2FEB4C-95FC-43D8-88C1-3CDBFA4E3CA7@atomicrules.com \
--to=john.miller@atomicrules.com \
--cc=dev@dpdk.org \
--cc=ed.czeck@atomicrules.com \
--cc=maxime.coquelin@redhat.com \
--cc=nicolas.chautru@intel.com \
--cc=shepard.siegel@atomicrules.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).