RE: [EXT] [PATCH v3 00/30] baseband/acc100: changes for 22.11

[Akhil Goyal <gakhil@marvell.com>]

> > > Hernan Vargas (30):
> > >   baseband/acc100: fix ring availability calculation
> > >   baseband/acc100: add function to check AQ availability
> > >   baseband/acc100: memory leak fix
> > >   baseband/acc100: add LDPC encoder padding function
> > >   baseband/acc100: check turbo dec/enc input
> > >   baseband/acc100: check for unlikely operation vals
> > >   baseband/acc100: enforce additional check on FCW
> > >   baseband/acc100: allocate ring/queue mem when NULL
> > >   baseband/acc100: reduce input length for CRC24B
> > >   baseband/acc100: fix clearing PF IR outside handler
> > >   baseband/acc100: set device min alignment to 1
> > >   baseband/acc100: add protection for NULL HARQ input
> > >   baseband/acc100: reset pointer after rte_free
> > >   baseband/acc100: fix debug print for LDPC FCW
> > >   baseband/acc100: add enqueue status
> > >   baseband/acc100: add scatter-gather support
> > >   baseband/acc100: add HARQ index helper function
> > >   baseband/acc100: enable input validation by default
> > >   baseband/acc100: added LDPC transport block support
> > >   baseband/acc100: update validate LDPC enc/dec
> > >   baseband/acc100: implement configurable queue depth
> > >   baseband/acc100: add queue stop operation
> > >   baseband/acc100: update uplink CB input length
> > >   baseband/acc100: rename ldpc encode function arg
> > >   baseband/acc100: update log messages
> > >   baseband/acc100: store FCW from first CB descriptor
> > >   baseband/acc100: update device info
> > >   baseband/acc100: add ring companion address
> > >   baseband/acc100: add workaround for deRM corner cases
> > >   baseband/acc100: configure PMON control registers
> > >
> > >  drivers/baseband/acc/acc100_pmd.h     |    5 +
> > >  drivers/baseband/acc/acc_common.h     |   10 +
> > >  drivers/baseband/acc/meson.build      |   21 +
> > >  drivers/baseband/acc/rte_acc100_pmd.c | 1197
> > > ++++++++++++++++++++-----
> > >  4 files changed, 1010 insertions(+), 223 deletions(-)
> > >
> > Hi Hernan/Nicolas,
> >
> > I see some ifdefs being used in the code and there is no documentation for
> > them On when and how to enable/disable them.
> > It would be much like a dead code which is not compiled at all, if any of the
> > build target does not enable them.
> >
> > Is it possible to replace them with runtime devargs instead of compile time
> > ifdefs?
> 
> In term of documentation gap, that is a fair point:
> - I believe the build time variable ACC100_EXT_MEM can be valuable for very
> specific troubleshoot (not for production) to shortcut the DDR on the card, but
> good point to document in the acc100.rst.
> - The RTE_BBDEV_OFFLOAD_COST is used across PMDs for a while. That could
> also be added to each PMD .rst.
> - The #ifndef RTE_LIBRTE_BBDEV_SKIP_VALIDATE is to skip validation of user
> API which protect from negative scenario and hence save a few cycles by not
> recommended by default. Can be added to acc100.rst.
> 
> In term of runtime devargs usage, can we decorrelate this from the series? If we
> introduce such dev args and expose them more explicitly we would need more
> updated validation in place, are you okay if we defer such change to next year?
> If really this is a big concern for you for 22.11 (this is indeed not ideal), we could
> limit/strip the upstream of the series to the default build configuration only
> (hence no new #ifdef) but still these options can be genuinely valuable to users
> (until moved to devargs) so I would prefer to keep as is but with updated
> documentation in acc100.rst.
> 
> What do you think?
I am ok to skip devargs for now. Can be added in next cycle.
Please update the documentation as needed for compile time flags.