[v3,00/30] baseband/acc100: changes for 22.11

  Upstreaming ACC100 changes for 22.11.
This patch series is dependant on series:
https://patches.dpdk.org/project/dpdk/list/?series=25041

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(-)
  

> 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(-)
> 
Please rebase over dpdk-next-crypto TOT
  

> 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?
  

Hi Akhil, 

> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Thursday, October 13, 2022 6:02 AM
> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
> trix@redhat.com; maxime.coquelin@redhat.com
> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Subject: RE: [EXT] [PATCH v3 00/30] baseband/acc100: changes for 22.11
> 
> > 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?

Thanks
Nic
  

> > > 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.