diff mbox series

[v2,4/6] baseband/acc100: add support for 4G CRC drop

Message ID 1629407410-28822-5-git-send-email-nicolas.chautru@intel.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series bbdev update related to CRC usage |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Chautru, Nicolas Aug. 19, 2021, 9:10 p.m. UTC 
  This implements in PMD the option to drop the CB CRC
after 4G decoding to help transport block concatenation.

Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
---
 doc/guides/bbdevs/acc100.rst             |  1 +
 doc/guides/rel_notes/release_21_11.rst   |  4 ++++
 drivers/baseband/acc100/rte_acc100_pmd.c | 12 +++++++++---
 3 files changed, 14 insertions(+), 3 deletions(-)

Comments

Tom Rix Sept. 1, 2021, 2:20 p.m. UTC | #1 
On 8/19/21 2:10 PM, Nicolas Chautru wrote:
> This implements in PMD the option to drop the CB CRC
> after 4G decoding to help transport block concatenation.
>
> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> ---
>   doc/guides/bbdevs/acc100.rst             |  1 +
>   doc/guides/rel_notes/release_21_11.rst   |  4 ++++
>   drivers/baseband/acc100/rte_acc100_pmd.c | 12 +++++++++---
>   3 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/doc/guides/bbdevs/acc100.rst b/doc/guides/bbdevs/acc100.rst
> index ff0fa4b..9fff6ab 100644
> --- a/doc/guides/bbdevs/acc100.rst
> +++ b/doc/guides/bbdevs/acc100.rst
> @@ -58,6 +58,7 @@ ACC100 5G/4G FEC PMD supports the following BBDEV capabilities:
>      - ``RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN`` :  set if negative LLR encoder i/p is supported
>      - ``RTE_BBDEV_TURBO_POS_LLR_1_BIT_IN`` :  set if positive LLR encoder i/p is supported
>      - ``RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP`` :  keep CRC24B bits appended while decoding
> +   - ``RTE_BBDEV_TURBO_DEC_CRC_24B_DROP`` : option to drop the code block CRC after decoding
>      - ``RTE_BBDEV_TURBO_EARLY_TERMINATION`` :  set early termination feature
>      - ``RTE_BBDEV_TURBO_DEC_SCATTER_GATHER`` :  supports scatter-gather for input/output data
>      - ``RTE_BBDEV_TURBO_HALF_ITERATION_EVEN`` :  set half iteration granularity
> diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
> index 8ca59b7..f7843bc 100644
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -59,6 +59,10 @@ New Features
>   
>     Added support for more comprehensive CRC options.
>   
> +* **Updated the ACC100 bbdev PMD.**
> +
> +  Added support for more comprehensive CRC options.
> +
>   Removed Items
>   -------------
>   
> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c b/drivers/baseband/acc100/rte_acc100_pmd.c
> index 68ba523..891be81 100644
> --- a/drivers/baseband/acc100/rte_acc100_pmd.c
> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
> @@ -980,6 +980,7 @@
>   					RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN |
>   					RTE_BBDEV_TURBO_MAP_DEC |
>   					RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP |
> +					RTE_BBDEV_TURBO_DEC_CRC_24B_DROP |
>   					RTE_BBDEV_TURBO_DEC_SCATTER_GATHER,
>   				.max_llr_modulus = INT8_MAX,
>   				.num_buffers_src =
> @@ -1708,8 +1709,12 @@
>   	}
>   
>   	if ((op->turbo_dec.code_block_mode == RTE_BBDEV_TRANSPORT_BLOCK)
> -		&& !check_bit(op->turbo_dec.op_flags,
> -		RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP))
> +			&& !check_bit(op->turbo_dec.op_flags,
> +			RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP))
> +		crc24_overlap = 24;
> +	if ((op->turbo_dec.code_block_mode == RTE_BBDEV_CODE_BLOCK)
These two if-statements can be combined.
> +			&& check_bit(op->turbo_dec.op_flags,
> +			RTE_BBDEV_TURBO_DEC_CRC_24B_DROP))
>   		crc24_overlap = 24;
>   
>   	/* Calculates circular buffer size.
> @@ -1744,7 +1749,8 @@
>   
>   	next_triplet = acc100_dma_fill_blk_type_out(
>   			desc, h_output, *h_out_offset,
> -			k >> 3, next_triplet, ACC100_DMA_BLKID_OUT_HARD);
> +			(k - crc24_overlap)  >> 3, next_triplet,

crc24_overlap had been set before this patch in the above if-statement 
for crc_24b_keep.

so this looks like a bug.

If it is a bug, it should be separated out as its own patch.

Tom

> +			ACC100_DMA_BLKID_OUT_HARD);
>   	if (unlikely(next_triplet < 0)) {
>   		rte_bbdev_log(ERR,
>   				"Mismatch between data to process and mbuf data length in bbdev_op: %p",
Chautru, Nicolas Sept. 8, 2021, 1:04 a.m. UTC | #2 
> -----Original Message-----
> From: Tom Rix <trix@redhat.com>
> Sent: Wednesday, September 1, 2021 7:20 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> gakhil@marvell.com
> Cc: thomas@monjalon.net; hemant.agrawal@nxp.com; Zhang, Mingshan
> <mingshan.zhang@intel.com>; Joshi, Arun <arun.joshi@intel.com>
> Subject: Re: [PATCH v2 4/6] baseband/acc100: add support for 4G CRC drop
> 
> 
> On 8/19/21 2:10 PM, Nicolas Chautru wrote:
> > This implements in PMD the option to drop the CB CRC after 4G decoding
> > to help transport block concatenation.
> >
> > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> > ---
> >   doc/guides/bbdevs/acc100.rst             |  1 +
> >   doc/guides/rel_notes/release_21_11.rst   |  4 ++++
> >   drivers/baseband/acc100/rte_acc100_pmd.c | 12 +++++++++---
> >   3 files changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/doc/guides/bbdevs/acc100.rst
> > b/doc/guides/bbdevs/acc100.rst index ff0fa4b..9fff6ab 100644
> > --- a/doc/guides/bbdevs/acc100.rst
> > +++ b/doc/guides/bbdevs/acc100.rst
> > @@ -58,6 +58,7 @@ ACC100 5G/4G FEC PMD supports the following BBDEV
> capabilities:
> >      - ``RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN`` :  set if negative LLR
> encoder i/p is supported
> >      - ``RTE_BBDEV_TURBO_POS_LLR_1_BIT_IN`` :  set if positive LLR encoder
> i/p is supported
> >      - ``RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP`` :  keep CRC24B bits
> > appended while decoding
> > +   - ``RTE_BBDEV_TURBO_DEC_CRC_24B_DROP`` : option to drop the code
> > + block CRC after decoding
> >      - ``RTE_BBDEV_TURBO_EARLY_TERMINATION`` :  set early termination
> feature
> >      - ``RTE_BBDEV_TURBO_DEC_SCATTER_GATHER`` :  supports scatter-
> gather for input/output data
> >      - ``RTE_BBDEV_TURBO_HALF_ITERATION_EVEN`` :  set half iteration
> > granularity diff --git a/doc/guides/rel_notes/release_21_11.rst
> > b/doc/guides/rel_notes/release_21_11.rst
> > index 8ca59b7..f7843bc 100644
> > --- a/doc/guides/rel_notes/release_21_11.rst
> > +++ b/doc/guides/rel_notes/release_21_11.rst
> > @@ -59,6 +59,10 @@ New Features
> >
> >     Added support for more comprehensive CRC options.
> >
> > +* **Updated the ACC100 bbdev PMD.**
> > +
> > +  Added support for more comprehensive CRC options.
> > +
> >   Removed Items
> >   -------------
> >
> > diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
> > b/drivers/baseband/acc100/rte_acc100_pmd.c
> > index 68ba523..891be81 100644
> > --- a/drivers/baseband/acc100/rte_acc100_pmd.c
> > +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
> > @@ -980,6 +980,7 @@
> >
> 	RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN |
> >   					RTE_BBDEV_TURBO_MAP_DEC |
> >
> 	RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP |
> > +
> 	RTE_BBDEV_TURBO_DEC_CRC_24B_DROP |
> >
> 	RTE_BBDEV_TURBO_DEC_SCATTER_GATHER,
> >   				.max_llr_modulus = INT8_MAX,
> >   				.num_buffers_src =
> > @@ -1708,8 +1709,12 @@
> >   	}
> >
> >   	if ((op->turbo_dec.code_block_mode ==
> RTE_BBDEV_TRANSPORT_BLOCK)
> > -		&& !check_bit(op->turbo_dec.op_flags,
> > -		RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP))
> > +			&& !check_bit(op->turbo_dec.op_flags,
> > +			RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP))
> > +		crc24_overlap = 24;
> > +	if ((op->turbo_dec.code_block_mode == RTE_BBDEV_CODE_BLOCK)
> These two if-statements can be combined.

They could but really when I tried  that became arguably unreadable, I thought it was still better this way. 

> > +			&& check_bit(op->turbo_dec.op_flags,
> > +			RTE_BBDEV_TURBO_DEC_CRC_24B_DROP))
> >   		crc24_overlap = 24;
> >
> >   	/* Calculates circular buffer size.
> > @@ -1744,7 +1749,8 @@
> >
> >   	next_triplet = acc100_dma_fill_blk_type_out(
> >   			desc, h_output, *h_out_offset,
> > -			k >> 3, next_triplet,
> ACC100_DMA_BLKID_OUT_HARD);
> > +			(k - crc24_overlap)  >> 3, next_triplet,
> 
> crc24_overlap had been set before this patch in the above if-statement for
> crc_24b_keep.
> 
> so this looks like a bug.
> 
> If it is a bug, it should be separated out as its own patch.

Ok fair enough, will do. Thanks

> 
> Tom
> 
> > +			ACC100_DMA_BLKID_OUT_HARD);
> >   	if (unlikely(next_triplet < 0)) {
> >   		rte_bbdev_log(ERR,
> >   				"Mismatch between data to process and
> mbuf data length in
> > bbdev_op: %p",
Tom Rix Sept. 11, 2021, 7:13 p.m. UTC | #3 
On 9/7/21 6:04 PM, Chautru, Nicolas wrote:
>
>> -----Original Message-----
>> From: Tom Rix <trix@redhat.com>
>> Sent: Wednesday, September 1, 2021 7:20 AM
>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
>> gakhil@marvell.com
>> Cc: thomas@monjalon.net; hemant.agrawal@nxp.com; Zhang, Mingshan
>> <mingshan.zhang@intel.com>; Joshi, Arun <arun.joshi@intel.com>
>> Subject: Re: [PATCH v2 4/6] baseband/acc100: add support for 4G CRC drop
>>
>>
>> On 8/19/21 2:10 PM, Nicolas Chautru wrote:
>>> This implements in PMD the option to drop the CB CRC after 4G decoding
>>> to help transport block concatenation.
>>>
>>> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
>>> ---
>>>    doc/guides/bbdevs/acc100.rst             |  1 +
>>>    doc/guides/rel_notes/release_21_11.rst   |  4 ++++
>>>    drivers/baseband/acc100/rte_acc100_pmd.c | 12 +++++++++---
>>>    3 files changed, 14 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/doc/guides/bbdevs/acc100.rst
>>> b/doc/guides/bbdevs/acc100.rst index ff0fa4b..9fff6ab 100644
>>> --- a/doc/guides/bbdevs/acc100.rst
>>> +++ b/doc/guides/bbdevs/acc100.rst
>>> @@ -58,6 +58,7 @@ ACC100 5G/4G FEC PMD supports the following BBDEV
>> capabilities:
>>>       - ``RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN`` :  set if negative LLR
>> encoder i/p is supported
>>>       - ``RTE_BBDEV_TURBO_POS_LLR_1_BIT_IN`` :  set if positive LLR encoder
>> i/p is supported
>>>       - ``RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP`` :  keep CRC24B bits
>>> appended while decoding
>>> +   - ``RTE_BBDEV_TURBO_DEC_CRC_24B_DROP`` : option to drop the code
>>> + block CRC after decoding
>>>       - ``RTE_BBDEV_TURBO_EARLY_TERMINATION`` :  set early termination
>> feature
>>>       - ``RTE_BBDEV_TURBO_DEC_SCATTER_GATHER`` :  supports scatter-
>> gather for input/output data
>>>       - ``RTE_BBDEV_TURBO_HALF_ITERATION_EVEN`` :  set half iteration
>>> granularity diff --git a/doc/guides/rel_notes/release_21_11.rst
>>> b/doc/guides/rel_notes/release_21_11.rst
>>> index 8ca59b7..f7843bc 100644
>>> --- a/doc/guides/rel_notes/release_21_11.rst
>>> +++ b/doc/guides/rel_notes/release_21_11.rst
>>> @@ -59,6 +59,10 @@ New Features
>>>
>>>      Added support for more comprehensive CRC options.
>>>
>>> +* **Updated the ACC100 bbdev PMD.**
>>> +
>>> +  Added support for more comprehensive CRC options.
>>> +
>>>    Removed Items
>>>    -------------
>>>
>>> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
>>> b/drivers/baseband/acc100/rte_acc100_pmd.c
>>> index 68ba523..891be81 100644
>>> --- a/drivers/baseband/acc100/rte_acc100_pmd.c
>>> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
>>> @@ -980,6 +980,7 @@
>>>
>> 	RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN |
>>>    					RTE_BBDEV_TURBO_MAP_DEC |
>>>
>> 	RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP |
>>> +
>> 	RTE_BBDEV_TURBO_DEC_CRC_24B_DROP |
>> 	RTE_BBDEV_TURBO_DEC_SCATTER_GATHER,
>>>    				.max_llr_modulus = INT8_MAX,
>>>    				.num_buffers_src =
>>> @@ -1708,8 +1709,12 @@
>>>    	}
>>>
>>>    	if ((op->turbo_dec.code_block_mode ==
>> RTE_BBDEV_TRANSPORT_BLOCK)
>>> -		&& !check_bit(op->turbo_dec.op_flags,
>>> -		RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP))
>>> +			&& !check_bit(op->turbo_dec.op_flags,
>>> +			RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP))
>>> +		crc24_overlap = 24;
>>> +	if ((op->turbo_dec.code_block_mode == RTE_BBDEV_CODE_BLOCK)
>> These two if-statements can be combined.
> They could but really when I tried  that became arguably unreadable, I thought it was still better this way.

Ok

Tom

>
>>> +			&& check_bit(op->turbo_dec.op_flags,
>>> +			RTE_BBDEV_TURBO_DEC_CRC_24B_DROP))
>>>    		crc24_overlap = 24;
>>>
>>>    	/* Calculates circular buffer size.
>>> @@ -1744,7 +1749,8 @@
>>>
>>>    	next_triplet = acc100_dma_fill_blk_type_out(
>>>    			desc, h_output, *h_out_offset,
>>> -			k >> 3, next_triplet,
>> ACC100_DMA_BLKID_OUT_HARD);
>>> +			(k - crc24_overlap)  >> 3, next_triplet,
>> crc24_overlap had been set before this patch in the above if-statement for
>> crc_24b_keep.
>>
>> so this looks like a bug.
>>
>> If it is a bug, it should be separated out as its own patch.
> Ok fair enough, will do. Thanks
>
>> Tom
>>
>>> +			ACC100_DMA_BLKID_OUT_HARD);
>>>    	if (unlikely(next_triplet < 0)) {
>>>    		rte_bbdev_log(ERR,
>>>    				"Mismatch between data to process and
>> mbuf data length in
>>> bbdev_op: %p",
diff mbox series

Patch

diff --git a/doc/guides/bbdevs/acc100.rst b/doc/guides/bbdevs/acc100.rst
index ff0fa4b..9fff6ab 100644
--- a/doc/guides/bbdevs/acc100.rst
+++ b/doc/guides/bbdevs/acc100.rst
@@ -58,6 +58,7 @@  ACC100 5G/4G FEC PMD supports the following BBDEV capabilities:
    - ``RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN`` :  set if negative LLR encoder i/p is supported
    - ``RTE_BBDEV_TURBO_POS_LLR_1_BIT_IN`` :  set if positive LLR encoder i/p is supported
    - ``RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP`` :  keep CRC24B bits appended while decoding
+   - ``RTE_BBDEV_TURBO_DEC_CRC_24B_DROP`` : option to drop the code block CRC after decoding
    - ``RTE_BBDEV_TURBO_EARLY_TERMINATION`` :  set early termination feature
    - ``RTE_BBDEV_TURBO_DEC_SCATTER_GATHER`` :  supports scatter-gather for input/output data
    - ``RTE_BBDEV_TURBO_HALF_ITERATION_EVEN`` :  set half iteration granularity
diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index 8ca59b7..f7843bc 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -59,6 +59,10 @@  New Features
 
   Added support for more comprehensive CRC options.
 
+* **Updated the ACC100 bbdev PMD.**
+
+  Added support for more comprehensive CRC options.
+
 Removed Items
 -------------
 
diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c b/drivers/baseband/acc100/rte_acc100_pmd.c
index 68ba523..891be81 100644
--- a/drivers/baseband/acc100/rte_acc100_pmd.c
+++ b/drivers/baseband/acc100/rte_acc100_pmd.c
@@ -980,6 +980,7 @@ 
 					RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN |
 					RTE_BBDEV_TURBO_MAP_DEC |
 					RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP |
+					RTE_BBDEV_TURBO_DEC_CRC_24B_DROP |
 					RTE_BBDEV_TURBO_DEC_SCATTER_GATHER,
 				.max_llr_modulus = INT8_MAX,
 				.num_buffers_src =
@@ -1708,8 +1709,12 @@ 
 	}
 
 	if ((op->turbo_dec.code_block_mode == RTE_BBDEV_TRANSPORT_BLOCK)
-		&& !check_bit(op->turbo_dec.op_flags,
-		RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP))
+			&& !check_bit(op->turbo_dec.op_flags,
+			RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP))
+		crc24_overlap = 24;
+	if ((op->turbo_dec.code_block_mode == RTE_BBDEV_CODE_BLOCK)
+			&& check_bit(op->turbo_dec.op_flags,
+			RTE_BBDEV_TURBO_DEC_CRC_24B_DROP))
 		crc24_overlap = 24;
 
 	/* Calculates circular buffer size.
@@ -1744,7 +1749,8 @@ 
 
 	next_triplet = acc100_dma_fill_blk_type_out(
 			desc, h_output, *h_out_offset,
-			k >> 3, next_triplet, ACC100_DMA_BLKID_OUT_HARD);
+			(k - crc24_overlap)  >> 3, next_triplet,
+			ACC100_DMA_BLKID_OUT_HARD);
 	if (unlikely(next_triplet < 0)) {
 		rte_bbdev_log(ERR,
 				"Mismatch between data to process and mbuf data length in bbdev_op: %p",