Hi,
> -----Original Message-----
> From: Chautru, Nicolas <nicolas.chau...@intel.com>
> Sent: Sunday, August 30, 2020 2:01
> To: Xu, Rosen <rosen...@intel.com>; dev@dpdk.org; akhil.go...@nxp.com
> Cc: Richardson, Bruce <bruce.richard...@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v3 05/11] baseband/acc100: add LDPC
> processing functions
>
> Hi Rosen,
>
> > From: Xu, Rosen <rosen...@intel.com>
> >
> > Hi,
> >
> > > -----Original Message-----
> > > From: dev <dev-boun...@dpdk.org> On Behalf Of Nicolas Chautru
> > > Sent: Wednesday, August 19, 2020 8:25
> > > To: dev@dpdk.org; akhil.go...@nxp.com
> > > Cc: Richardson, Bruce <bruce.richard...@intel.com>; Chautru, Nicolas
> > > <nicolas.chau...@intel.com>
> > > Subject: [dpdk-dev] [PATCH v3 05/11] baseband/acc100: add LDPC
> > > processing functions
> > >
> > > Adding LDPC decode and encode processing operations
> > >
> > > Signed-off-by: Nicolas Chautru <nicolas.chau...@intel.com>
> > > ---
> > > drivers/baseband/acc100/rte_acc100_pmd.c | 1625
> > > +++++++++++++++++++++++++++++-
> > > drivers/baseband/acc100/rte_acc100_pmd.h | 3 +
> > > 2 files changed, 1626 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
> > > b/drivers/baseband/acc100/rte_acc100_pmd.c
> > > index 7a21c57..5f32813 100644
> > > --- a/drivers/baseband/acc100/rte_acc100_pmd.c
> > > +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
> > > @@ -15,6 +15,9 @@
> > > #include <rte_hexdump.h>
> > > #include <rte_pci.h>
> > > #include <rte_bus_pci.h>
> > > +#ifdef RTE_BBDEV_OFFLOAD_COST
> > > +#include <rte_cycles.h>
> > > +#endif
> > >
> > > #include <rte_bbdev.h>
> > > #include <rte_bbdev_pmd.h>
> > > @@ -449,7 +452,6 @@
> > > return 0;
> > > }
> > >
> > > -
> > > /**
> > > * Report a ACC100 queue index which is free
> > > * Return 0 to 16k for a valid queue_idx or -1 when no queue is
> > > available @@ -634,6 +636,46 @@
> > > struct acc100_device *d = dev->data->dev_private;
> > >
> > > static const struct rte_bbdev_op_cap bbdev_capabilities[] = {
> > > + {
> > > + .type = RTE_BBDEV_OP_LDPC_ENC,
> > > + .cap.ldpc_enc = {
> > > + .capability_flags =
> > > + RTE_BBDEV_LDPC_RATE_MATCH |
> > > + RTE_BBDEV_LDPC_CRC_24B_ATTACH
> > > |
> > > +
> > > RTE_BBDEV_LDPC_INTERLEAVER_BYPASS,
> > > + .num_buffers_src =
> > > +
> > > RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
> > > + .num_buffers_dst =
> > > +
> > > RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
> > > + }
> > > + },
> > > + {
> > > + .type = RTE_BBDEV_OP_LDPC_DEC,
> > > + .cap.ldpc_dec = {
> > > + .capability_flags =
> > > + RTE_BBDEV_LDPC_CRC_TYPE_24B_CHECK |
> > > + RTE_BBDEV_LDPC_CRC_TYPE_24B_DROP |
> > > +
> > > RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE |
> > > +
> > > RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE |
> > > +#ifdef ACC100_EXT_MEM
> > > +
> > > RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_IN_ENABLE |
> > > +
> > > RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_OUT_ENABLE |
> > > +#endif
> > > +
> > > RTE_BBDEV_LDPC_ITERATION_STOP_ENABLE |
> > > + RTE_BBDEV_LDPC_DEINTERLEAVER_BYPASS
> > > |
> > > + RTE_BBDEV_LDPC_DECODE_BYPASS |
> > > + RTE_BBDEV_LDPC_DEC_SCATTER_GATHER |
> > > +
> > > RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION |
> > > + RTE_BBDEV_LDPC_LLR_COMPRESSION,
> > > + .llr_size = 8,
> > > + .llr_decimals = 1,
> > > + .num_buffers_src =
> > > +
> > > RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
> > > + .num_buffers_hard_out =
> > > +
> > > RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
> > > + .num_buffers_soft_out = 0,
> > > + }
> > > + },
> > > RTE_BBDEV_END_OF_CAPABILITIES_LIST()
> > > };
> > >
> > > @@ -669,9 +711,14 @@
> > > dev_info->cpu_flag_reqs = NULL;
> > > dev_info->min_alignment = 64;
> > > dev_info->capabilities = bbdev_capabilities;
> > > +#ifdef ACC100_EXT_MEM
> > > dev_info->harq_buffer_size = d->ddr_size;
> > > +#else
> > > + dev_info->harq_buffer_size = 0;
> > > +#endif
> > > }
> > >
> > > +
> > > static const struct rte_bbdev_ops acc100_bbdev_ops = {
> > > .setup_queues = acc100_setup_queues,
> > > .close = acc100_dev_close,
> > > @@ -696,6 +743,1577 @@
> > > {.device_id = 0},
> > > };
> > >
> > > +/* Read flag value 0/1 from bitmap */ static inline bool
> > > +check_bit(uint32_t bitmap, uint32_t bitmask) {
> > > + return bitmap & bitmask;
> > > +}
> > > +
> > > +static inline char *
> > > +mbuf_append(struct rte_mbuf *m_head, struct rte_mbuf *m, uint16_t
> > > +len) {
> > > + if (unlikely(len > rte_pktmbuf_tailroom(m)))
> > > + return NULL;
> > > +
> > > + char *tail = (char *)m->buf_addr + m->data_off + m->data_len;
> > > + m->data_len = (uint16_t)(m->data_len + len);
> > > + m_head->pkt_len = (m_head->pkt_len + len);
> > > + return tail;
> > > +}
> >
> > Is it reasonable to direct add data_len of rte_mbuf?
> >
>
> Do you suggest to add directly without checking there is enough room in the
> mbuf? We cannot rely on the application providing mbuf with enough
> tailroom.
What I mentioned is this changes about mbuf should move to librte_mbuf.
And it's better to align Olivier Matz.
> In case you ask about the 2 mbufs, this is because this function is used to
> also
> support segmented memory made of multiple mbufs segments.
> Note that this function is also used in other existing bbdev PMDs. In case you
> believe there is a better way to do this, we can certainly discuss and change
> these in several PMDs through another serie.
>
> Thanks for all the reviews and useful comments.
> Nic
