From: David Marchand <david.marchand@redhat.com>
To: "Chautru, Nicolas" <nicolas.chautru@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"maxime.coquelin@redhat.com" <maxime.coquelin@redhat.com>,
"hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>,
"Vargas, Hernan" <hernan.vargas@intel.com>,
Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [PATCH v1 3/7] baseband/acc: remove the 4G SO capability for VRB1
Date: Thu, 21 Sep 2023 09:13:11 +0200 [thread overview]
Message-ID: <CAJFAV8y0XjsU=ZzXphgBng_9-RmHDc08rv8GyrHF-RP5B-UAHw@mail.gmail.com> (raw)
In-Reply-To: <BY5PR11MB4451BB28FFDB9F5A4957B43DF8FAA@BY5PR11MB4451.namprd11.prod.outlook.com>
On Tue, Sep 19, 2023 at 10:32 PM Chautru, Nicolas
<nicolas.chautru@intel.com> wrote:
>
> Hi David,
>
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Tuesday, September 19, 2023 8:20 AM
> > To: Chautru, Nicolas <nicolas.chautru@intel.com>
> > Cc: dev@dpdk.org; maxime.coquelin@redhat.com;
> > hemant.agrawal@nxp.com; Vargas, Hernan <hernan.vargas@intel.com>
> > Subject: Re: [PATCH v1 3/7] baseband/acc: remove the 4G SO capability for
> > VRB1
> >
> > On Tue, Sep 19, 2023 at 3:25 AM Nicolas Chautru
> > <nicolas.chautru@intel.com> wrote:
> > >
> > > This removes the specific capability and support of LTE Decoder Soft
> > > Output option on the VRB1 PMD.
> >
> > Please explain why such support is removed for this hw.
>
> The decision is made to defeature this optional capability as under certain race conditions enabling this may potentially cause reliability issues which would not be acceptable.
> Note that this is an optional additional output information (soft output information) independent of the actual decoding operation.
> More details below next to your other comments.
This must be explained in the commitlog.
>
> >
> >
> > >
> > > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> > > ---
> > > drivers/baseband/acc/rte_vrb_pmd.c | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/baseband/acc/rte_vrb_pmd.c
> > > b/drivers/baseband/acc/rte_vrb_pmd.c
> > > index 3c8f3409ed..e0f50460bd 100644
> > > --- a/drivers/baseband/acc/rte_vrb_pmd.c
> > > +++ b/drivers/baseband/acc/rte_vrb_pmd.c
> > > @@ -1019,14 +1019,11 @@ vrb_dev_info_get(struct rte_bbdev *dev, struct
> > rte_bbdev_driver_info *dev_info)
> > > RTE_BBDEV_TURBO_CRC_TYPE_24B |
> > > RTE_BBDEV_TURBO_DEC_CRC_24B_DROP |
> > > RTE_BBDEV_TURBO_EQUALIZER |
> > > - RTE_BBDEV_TURBO_SOFT_OUT_SATURATE |
> > > RTE_BBDEV_TURBO_HALF_ITERATION_EVEN |
> > > RTE_BBDEV_TURBO_CONTINUE_CRC_MATCH |
> > > - RTE_BBDEV_TURBO_SOFT_OUTPUT |
> > > RTE_BBDEV_TURBO_EARLY_TERMINATION |
> > > RTE_BBDEV_TURBO_DEC_INTERRUPTS |
> > > RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN |
> > > - RTE_BBDEV_TURBO_NEG_LLR_1_BIT_SOFT_OUT |
> > > RTE_BBDEV_TURBO_MAP_DEC |
> > > RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP |
> > >
> > > RTE_BBDEV_TURBO_DEC_SCATTER_GATHER,
> > > @@ -1975,6 +1972,9 @@ enqueue_dec_one_op_cb(struct acc_queue *q,
> > struct rte_bbdev_dec_op *op,
> > > struct rte_mbuf *input, *h_output_head, *h_output,
> > > *s_output_head, *s_output;
> > >
> > > + /* Disable explictly SO for VRB 1. */
> > > + op->turbo_dec.op_flags &= ~RTE_BBDEV_TURBO_SOFT_OUTPUT;
> >
> > Can you explain why it is needed to filter this out?
> >
> > I did not find a clear description in the bbdev API.
> > It would help if there were explicits references in doxygen of which capability
> > is necessary for using flags/API.
> >
> >
> > I was expecting that asking for RTE_BBDEV_TURBO_SOFT_OUTPUT to a driver
> > is only allowed if rte_bbdev_op_cap contains it.
> > With this assumption, it would be invalid for an application to request
> > RTE_BBDEV_TURBO_SOFT_OUTPUT through rte_bbdev_enqueue_dec_ops.
>
> You may arguably expect this from a well behaved user application but still there is nothing that enforces it explicitly, ie. notably under negative scenario conditions which we still need to manage gracefully.
If your application is buggy (not reading / complying with the device
capabilities), fix it.
> Here we want to make sure that in case the optional operational flag is included, we fall back to default mode when using the VRB1 variant.
> Keep in mind that the unified driver can support multiple HW variant (see rest of the serie) and may support this option for other variants using same code.
Whatever the HW variant, the API should be respected: exposing
capabilities is done on a per device basis.
>
> In term of documentation, I believe that capability/flag (ie. note that the enum maps to a capability when retrieved from info_get, and to an operation flag when provided to the bbdev api) is already captured explicitly for many generations. Basically this an optional output of the LTE decoding processing, to provide APP LLR which can be potentially be useful for the user application (separate optional mbuf). It may or may not be supported by a bb device, and it may or may not be requested to be provided through the API. Typically this is not enabled.
Being optional does not mean that a driver can ignore it.
Otherwise, there is no point in exposing a capability.
Thanks.
--
David Marchand
next prev parent reply other threads:[~2023-09-21 7:13 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-19 1:21 [PATCH v1 0/7] VRB2 BBDEV PMD introduction Nicolas Chautru
2023-09-19 1:21 ` [PATCH v1 1/7] bbdev: add FFT version member in driver info Nicolas Chautru
2023-09-19 9:55 ` Maxime Coquelin
2023-09-19 20:51 ` Chautru, Nicolas
2023-09-22 8:14 ` Maxime Coquelin
2023-09-22 16:41 ` Chautru, Nicolas
2023-09-26 10:00 ` Maxime Coquelin
2023-09-27 23:50 ` Chautru, Nicolas
2023-09-28 8:27 ` Maxime Coquelin
2023-09-28 16:33 ` Chautru, Nicolas
2023-10-03 11:46 ` Maxime Coquelin
2023-09-19 1:21 ` [PATCH v1 2/7] baseband/acc: add FFT version in the VRM PMD Nicolas Chautru
2023-09-19 1:21 ` [PATCH v1 3/7] baseband/acc: remove the 4G SO capability for VRB1 Nicolas Chautru
2023-09-19 15:20 ` David Marchand
2023-09-19 20:32 ` Chautru, Nicolas
2023-09-21 7:13 ` David Marchand [this message]
2023-09-21 17:18 ` Chautru, Nicolas
2023-09-27 7:08 ` Maxime Coquelin
2023-09-27 7:32 ` Maxime Coquelin
2023-09-19 1:21 ` [PATCH v1 4/7] baseband/acc: allocate FCW memory separately Nicolas Chautru
2023-09-19 1:21 ` [PATCH v1 5/7] baseband/acc: add support for MLD operation Nicolas Chautru
2023-09-19 1:21 ` [PATCH v1 6/7] baseband/acc: introduce the new VRB2 variant Nicolas Chautru
2023-09-19 1:21 ` [PATCH v1 7/7] baseband/acc: add configure helper for VRB2 Nicolas Chautru
2023-09-21 7:25 ` [PATCH v1 0/7] VRB2 BBDEV PMD introduction David Marchand
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='CAJFAV8y0XjsU=ZzXphgBng_9-RmHDc08rv8GyrHF-RP5B-UAHw@mail.gmail.com' \
--to=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=hemant.agrawal@nxp.com \
--cc=hernan.vargas@intel.com \
--cc=maxime.coquelin@redhat.com \
--cc=nicolas.chautru@intel.com \
--cc=thomas@monjalon.net \
/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).