DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Chautru, Nicolas" <nicolas.chautru@intel.com>
To: Maxime Coquelin <maxime.coquelin@redhat.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"thomas@monjalon.net" <thomas@monjalon.net>
Cc: "trix@redhat.com" <trix@redhat.com>,
	"mdr@ashroe.eu" <mdr@ashroe.eu>,
	"Richardson, Bruce" <bruce.richardson@intel.com>,
	"hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>,
	"david.marchand@redhat.com" <david.marchand@redhat.com>,
	"stephen@networkplumber.org" <stephen@networkplumber.org>,
	"Vargas, Hernan" <hernan.vargas@intel.com>
Subject: RE: [PATCH v4 10/14] baseband/acc: add support for FFT operations
Date: Fri, 23 Sep 2022 22:31:46 +0000	[thread overview]
Message-ID: <BY5PR11MB4451329F778E5CE7FB8E9DF4F8519@BY5PR11MB4451.namprd11.prod.outlook.com> (raw)
In-Reply-To: <3ec99dcc-898d-a916-0fe5-c1fbf7e77a37@redhat.com>

Hi Maxime, 

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Friday, September 23, 2022 2:06 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> thomas@monjalon.net
> Cc: trix@redhat.com; mdr@ashroe.eu; Richardson, Bruce
> <bruce.richardson@intel.com>; hemant.agrawal@nxp.com;
> david.marchand@redhat.com; stephen@networkplumber.org; Vargas,
> Hernan <hernan.vargas@intel.com>
> Subject: Re: [PATCH v4 10/14] baseband/acc: add support for FFT operations
> 
> 
> 
> On 9/22/22 02:27, Nic Chautru wrote:
> > Add functions and capability for FFT processing
> >
> > Signed-off-by: Nic Chautru <nicolas.chautru@intel.com>
> > ---
> >   drivers/baseband/acc/rte_acc200_pmd.c | 251
> +++++++++++++++++++++++++++++++++-
> >   1 file changed, 249 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/baseband/acc/rte_acc200_pmd.c
> > b/drivers/baseband/acc/rte_acc200_pmd.c
> > index 35ea0fe..fa23b6e 100644
> > --- a/drivers/baseband/acc/rte_acc200_pmd.c
> > +++ b/drivers/baseband/acc/rte_acc200_pmd.c
> > @@ -717,6 +717,21 @@
> >   			.num_buffers_soft_out = 0,
> >   			}
> >   		},
> > +		{
> > +			.type	= RTE_BBDEV_OP_FFT,
> > +			.cap.fft = {
> > +				.capability_flags =
> > +
> 	RTE_BBDEV_FFT_WINDOWING |
> > +
> 	RTE_BBDEV_FFT_CS_ADJUSTMENT |
> > +
> 	RTE_BBDEV_FFT_DFT_BYPASS |
> > +
> 	RTE_BBDEV_FFT_IDFT_BYPASS |
> > +
> 	RTE_BBDEV_FFT_WINDOWING_BYPASS,
> > +				.num_buffers_src =
> > +
> 	RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
> > +				.num_buffers_dst =
> > +
> 	RTE_BBDEV_LDPC_MAX_CODE_BLOCKS,
> > +			}
> > +		},
> >   		RTE_BBDEV_END_OF_CAPABILITIES_LIST()
> >   	};
> >
> > @@ -739,12 +754,13 @@
> >   			d->acc_conf.q_ul_5g.num_qgroups;
> >   	dev_info->num_queues[RTE_BBDEV_OP_LDPC_ENC] = d-
> >acc_conf.q_dl_5g.num_aqs_per_groups *
> >   			d->acc_conf.q_dl_5g.num_qgroups;
> > -	dev_info->num_queues[RTE_BBDEV_OP_FFT] = 0;
> > +	dev_info->num_queues[RTE_BBDEV_OP_FFT] = d-
> >acc_conf.q_fft.num_aqs_per_groups *
> > +			d->acc_conf.q_fft.num_qgroups;
> >   	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_DEC] = d-
> >acc_conf.q_ul_4g.num_qgroups;
> >   	dev_info->queue_priority[RTE_BBDEV_OP_TURBO_ENC] = d-
> >acc_conf.q_dl_4g.num_qgroups;
> >   	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_DEC] = d-
> >acc_conf.q_ul_5g.num_qgroups;
> >   	dev_info->queue_priority[RTE_BBDEV_OP_LDPC_ENC] = d-
> >acc_conf.q_dl_5g.num_qgroups;
> > -	dev_info->queue_priority[RTE_BBDEV_OP_FFT] = 0;
> > +	dev_info->queue_priority[RTE_BBDEV_OP_FFT] =
> > +d->acc_conf.q_fft.num_qgroups;
> >   	dev_info->max_num_queues = 0;
> >   	for (i = RTE_BBDEV_OP_NONE; i <= RTE_BBDEV_OP_FFT; i++)
> >   		dev_info->max_num_queues += dev_info->num_queues[i];
> @@ -3034,6
> > +3050,235 @@
> >   	return i;
> >   }
> >
> > +/* Fill in a frame control word for FFT processing. */ static inline
> > +void acc200_fcw_fft_fill(struct rte_bbdev_fft_op *op, struct
> > +acc_fcw_fft *fcw) {
> > +	fcw->in_frame_size = op->fft.input_sequence_size;
> > +	fcw->leading_pad_size = op->fft.input_leading_padding;
> > +	fcw->out_frame_size = op->fft.output_sequence_size;
> > +	fcw->leading_depad_size = op->fft.output_leading_depadding;
> > +	fcw->cs_window_sel = op->fft.window_index[0] +
> > +			(op->fft.window_index[1] << 8) +
> > +			(op->fft.window_index[2] << 16) +
> > +			(op->fft.window_index[3] << 24);
> > +	fcw->cs_window_sel2 = op->fft.window_index[4] +
> > +			(op->fft.window_index[5] << 8);
> > +	fcw->cs_enable_bmap = op->fft.cs_bitmap;
> > +	fcw->num_antennas = op->fft.num_antennas_log2;
> > +	fcw->idft_size = op->fft.idft_log2;
> > +	fcw->dft_size = op->fft.dft_log2;
> > +	fcw->cs_offset = op->fft.cs_time_adjustment;
> > +	fcw->idft_shift = op->fft.idft_shift;
> > +	fcw->dft_shift = op->fft.dft_shift;
> > +	fcw->cs_multiplier = op->fft.ncs_reciprocal;
> > +	if (check_bit(op->fft.op_flags,
> > +			RTE_BBDEV_FFT_IDFT_BYPASS)) {
> > +		if (check_bit(op->fft.op_flags,
> > +				RTE_BBDEV_FFT_WINDOWING_BYPASS))
> > +			fcw->bypass = 2;
> > +		else
> > +			fcw->bypass = 1;
> > +	} else if (check_bit(op->fft.op_flags,
> > +			RTE_BBDEV_FFT_DFT_BYPASS))
> > +		fcw->bypass = 3;
> > +	else
> > +		fcw->bypass = 0;
> > +}
> > +
> > +static inline int
> > +acc200_dma_desc_fft_fill(struct rte_bbdev_fft_op *op,
> > +		struct acc_dma_req_desc *desc,
> > +		struct rte_mbuf *input, struct rte_mbuf *output,
> > +		uint32_t *in_offset, uint32_t *out_offset) {
> > +	/* FCW already done */
> > +	acc_header_init(desc);
> > +	desc->data_ptrs[1].address =
> > +			rte_pktmbuf_iova_offset(input, *in_offset);
> > +	desc->data_ptrs[1].blen = op->fft.input_sequence_size * 4;
> > +	desc->data_ptrs[1].blkid = ACC_DMA_BLKID_IN;
> > +	desc->data_ptrs[1].last = 1;
> > +	desc->data_ptrs[1].dma_ext = 0;
> > +	desc->data_ptrs[2].address =
> > +			rte_pktmbuf_iova_offset(output, *out_offset);
> > +	desc->data_ptrs[2].blen = op->fft.output_sequence_size * 4;
> > +	desc->data_ptrs[2].blkid = ACC_DMA_BLKID_OUT_HARD;
> > +	desc->data_ptrs[2].last = 1;
> > +	desc->data_ptrs[2].dma_ext = 0;
> > +	desc->m2dlen = 2;
> > +	desc->d2mlen = 1;
> > +	desc->ib_ant_offset = op->fft.input_sequence_size;
> > +	desc->num_ant = op->fft.num_antennas_log2 - 3;
> > +	int num_cs = 0, i;
> > +	for (i = 0; i < 12; i++)
> > +		if (check_bit(op->fft.cs_bitmap, 1 << i))
> > +			num_cs++;
> > +	desc->num_cs = num_cs;
> > +	desc->ob_cyc_offset = op->fft.output_sequence_size;
> > +	desc->ob_ant_offset = op->fft.output_sequence_size * num_cs;
> > +	desc->op_addr = op;
> > +	return 0;
> > +}
> > +
> > +
> > +/** Enqueue one FFT operation for ACC200 device*/ static inline int
> > +enqueue_fft_one_op(struct acc_queue *q, struct rte_bbdev_fft_op *op,
> > +		uint16_t total_enqueued_cbs)
> > +{
> > +	union acc_dma_desc *desc;
> > +	uint16_t desc_idx = ((q->sw_ring_head + total_enqueued_cbs)
> > +			& q->sw_ring_wrap_mask);
> > +	desc = q->ring_addr + desc_idx;
> > +	struct rte_mbuf *input, *output;
> > +	uint32_t in_offset, out_offset;
> > +	input = op->fft.base_input.data;
> > +	output = op->fft.base_output.data;
> > +	in_offset = op->fft.base_input.offset;
> > +	out_offset = op->fft.base_output.offset; #ifdef
> > +RTE_LIBRTE_BBDEV_DEBUG
> > +	if (unlikely(input == NULL)) {
> > +		rte_bbdev_log(ERR, "Invalid mbuf pointer");
> > +		return -EFAULT;
> > +	}
> > +#endif
> 
> Same comment as on previous patch, if needed just have this check by
> default, otherwise drop it.

OK

> 
> > +	struct acc_fcw_fft *fcw;
> 
> Don't mix declarations & code.

OK

> 
> > +	fcw = &desc->req.fcw_fft;
> > +	acc200_fcw_fft_fill(op, fcw);
> > +	acc200_dma_desc_fft_fill(op, &desc->req, input, output,
> > +			&in_offset, &out_offset);
> > +#ifdef RTE_LIBRTE_BBDEV_DEBUG
> > +	rte_memdump(stderr, "FCW", &desc->req.fcw_fft,
> > +			sizeof(desc->req.fcw_fft));
> > +	rte_memdump(stderr, "Req Desc.", desc, sizeof(*desc)); #endif
> > +	return 1;
> > +}
> > +
> > +/* Enqueue decode operations for ACC200 device. */ static uint16_t
> > +acc200_enqueue_fft(struct rte_bbdev_queue_data *q_data,
> > +		struct rte_bbdev_fft_op **ops, uint16_t num) {
> > +	int32_t aq_avail = acc_aq_avail(q_data, num);
> > +	if (unlikely((aq_avail <= 0) || (num == 0)))
> > +		return 0;
> 
> Don't mix declarations & code.

OK

> 
> > +	struct acc_queue *q = q_data->queue_private;
> > +	int32_t avail = acc_ring_avail_enq(q);
> > +	uint16_t i;
> > +	union acc_dma_desc *desc;
> > +	int ret;
> > +	for (i = 0; i < num; ++i) {
> > +		/* Check if there are available space for further processing */
> > +		if (unlikely(avail < 1))
> > +			break;
> > +		avail -= 1;
> > +		ret = enqueue_fft_one_op(q, ops[i], i);
> > +		if (ret < 0)
> > +			break;
> > +	}
> > +
> > +	if (unlikely(i == 0))
> > +		return 0; /* Nothing to enqueue */
> > +
> > +	/* Set SDone in last CB in enqueued ops for CB mode*/
> > +	desc = q->ring_addr + ((q->sw_ring_head + i - 1)
> > +			& q->sw_ring_wrap_mask);
> > +
> > +	desc->req.sdone_enable = 1;
> > +	desc->req.irq_enable = q->irq_enable;
> > +	acc_dma_enqueue(q, i, &q_data->queue_stats);
> > +
> > +	/* Update stats */
> > +	q_data->queue_stats.enqueued_count += i;
> > +	q_data->queue_stats.enqueue_err_count += num - i;
> > +	return i;
> > +}
> > +
> > +
> > +/* Dequeue one FFT operations from ACC200 device */ static inline int
> > +dequeue_fft_one_op(struct rte_bbdev_queue_data *q_data,
> > +		struct acc_queue *q, struct rte_bbdev_fft_op **ref_op,
> > +		uint16_t dequeued_cbs, uint32_t *aq_dequeued) {
> > +	union acc_dma_desc *desc, atom_desc;
> > +	union acc_dma_rsp_desc rsp;
> > +	struct rte_bbdev_fft_op *op;
> > +
> > +	desc = q->ring_addr + ((q->sw_ring_tail + dequeued_cbs)
> > +			& q->sw_ring_wrap_mask);
> > +	atom_desc.atom_hdr = __atomic_load_n((uint64_t *)desc,
> > +			__ATOMIC_RELAXED);
> > +
> > +	/* Check fdone bit */
> > +	if (!(atom_desc.rsp.val & ACC_FDONE))
> > +		return -1;
> > +
> > +	rsp.val = atom_desc.rsp.val;
> > +#ifdef RTE_LIBRTE_BBDEV_DEBUG
> > +	rte_memdump(stderr, "Resp", &desc->rsp.val,
> > +			sizeof(desc->rsp.val));
> > +#endif
> > +	/* Dequeue */
> > +	op = desc->req.op_addr;
> > +
> > +	/* Clearing status, it will be set based on response */
> > +	op->status = 0;
> > +	op->status |= rsp.input_err << RTE_BBDEV_DATA_ERROR;
> > +	op->status |= rsp.dma_err << RTE_BBDEV_DRV_ERROR;
> > +	op->status |= rsp.fcw_err << RTE_BBDEV_DRV_ERROR;
> > +	if (op->status != 0)
> > +		q_data->queue_stats.dequeue_err_count++;
> > +
> > +	/* Check if this is the last desc in batch (Atomic Queue) */
> > +	if (desc->req.last_desc_in_batch) {
> > +		(*aq_dequeued)++;
> > +		desc->req.last_desc_in_batch = 0;
> > +	}
> > +	desc->rsp.val = ACC_DMA_DESC_TYPE;
> > +	desc->rsp.add_info_0 = 0;
> > +	*ref_op = op;
> > +	/* One CB (op) was successfully dequeued */
> > +	return 1;
> > +}
> > +
> > +
> > +/* Dequeue FFT operations from ACC200 device. */ static uint16_t
> > +acc200_dequeue_fft(struct rte_bbdev_queue_data *q_data,
> > +		struct rte_bbdev_fft_op **ops, uint16_t num) {
> > +	struct acc_queue *q = q_data->queue_private;
> > +	uint16_t dequeue_num, i, dequeued_cbs = 0;
> > +	uint32_t avail = acc_ring_avail_deq(q);
> > +	uint32_t aq_dequeued = 0;
> > +	int ret;
> > +
> > +#ifdef RTE_LIBRTE_BBDEV_DEBUG
> > +	if (unlikely(ops == 0 && q == NULL))
> > +		return 0;
> > +#endif
> > +
> > +	dequeue_num = RTE_MIN(avail, num);
> 
> num can be reused, no need to dequeue_num

I would keep as is notably for consistency for other similar functions
Thanks

> 
> > +
> > +	for (i = 0; i < dequeue_num; ++i) {
> > +		ret = dequeue_fft_one_op(
> > +				q_data, q, &ops[i], dequeued_cbs,
> > +				&aq_dequeued);
> > +		if (ret <= 0)
> > +			break;
> > +		dequeued_cbs += ret;
> > +	}
> > +
> > +	q->aq_dequeued += aq_dequeued;
> > +	q->sw_ring_tail += dequeued_cbs;
> > +	/* Update enqueue stats */
> > +	q_data->queue_stats.dequeued_count += i;
> > +	return i;
> > +}
> > +
> >   /* Initialization Function */
> >   static void
> >   acc200_bbdev_init(struct rte_bbdev *dev, struct rte_pci_driver *drv)
> > @@ -3049,6 +3294,8 @@
> >   	dev->enqueue_ldpc_dec_ops = acc200_enqueue_ldpc_dec;
> >   	dev->dequeue_ldpc_enc_ops = acc200_dequeue_ldpc_enc;
> >   	dev->dequeue_ldpc_dec_ops = acc200_dequeue_ldpc_dec;
> > +	dev->enqueue_fft_ops = acc200_enqueue_fft;
> > +	dev->dequeue_fft_ops = acc200_dequeue_fft;
> >
> >   	((struct acc_device *) dev->data->dev_private)->pf_device =
> >   			!strcmp(drv->driver.name,


  reply	other threads:[~2022-09-23 22:31 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-22  0:27 From: Nic Chautru <nicolas.chautru@intel.com> Nic Chautru
2022-09-22  0:27 ` [PATCH v4 01/14] baseband/acc100: remove unused registers Nic Chautru
2022-09-22  0:37   ` Chautru, Nicolas
2022-09-22 13:08     ` Maxime Coquelin
2022-09-22 13:09   ` Maxime Coquelin
2022-09-22  0:27 ` [PATCH v4 02/14] baseband/acc100: refactor to segregate common code Nic Chautru
2022-09-22 13:15   ` Maxime Coquelin
2022-09-22  0:27 ` [PATCH v4 03/14] baseband/acc: rename directory from acc100 to acc Nic Chautru
2022-09-22 13:17   ` Maxime Coquelin
2022-09-22 18:06     ` Chautru, Nicolas
2022-09-22  0:27 ` [PATCH v4 04/14] baseband/acc: introduce PMD for ACC200 Nic Chautru
2022-09-22 14:01   ` Maxime Coquelin
2022-09-22  0:27 ` [PATCH v4 05/14] baseband/acc: add HW register definitions " Nic Chautru
2022-09-22 14:04   ` Maxime Coquelin
2022-09-22  0:27 ` [PATCH v4 06/14] baseband/acc: add info get function " Nic Chautru
2022-09-22 14:11   ` Maxime Coquelin
2022-09-22 18:20     ` Chautru, Nicolas
2022-09-22  0:27 ` [PATCH v4 07/14] baseband/acc: add queue configuration " Nic Chautru
2022-09-22 14:30   ` Maxime Coquelin
2022-09-22 18:51     ` Chautru, Nicolas
2022-09-22  0:27 ` [PATCH v4 08/14] baseband/acc: add LDPC processing functions Nic Chautru
2022-09-23  8:29   ` Maxime Coquelin
2022-09-23 21:55     ` Chautru, Nicolas
2022-09-27 13:12       ` Maxime Coquelin
2022-09-22  0:27 ` [PATCH v4 09/14] baseband/acc: add LTE " Nic Chautru
2022-09-23  8:59   ` Maxime Coquelin
2022-09-23 22:21     ` Chautru, Nicolas
2022-09-27 13:33       ` Maxime Coquelin
2022-09-22  0:27 ` [PATCH v4 10/14] baseband/acc: add support for FFT operations Nic Chautru
2022-09-23  9:05   ` Maxime Coquelin
2022-09-23 22:31     ` Chautru, Nicolas [this message]
2022-09-22  0:27 ` [PATCH v4 11/14] baseband/acc: support interrupt Nic Chautru
2022-09-23  9:17   ` Maxime Coquelin
2022-09-23 22:58     ` Chautru, Nicolas
2022-09-22  0:27 ` [PATCH v4 12/14] baseband/acc: add device status and vf2pf comms Nic Chautru
2022-09-23  9:23   ` Maxime Coquelin
2022-09-24  0:04     ` Chautru, Nicolas
2022-09-22  0:27 ` [PATCH v4 13/14] baseband/acc: add PF configure companion function Nic Chautru
2022-09-23  9:26   ` Maxime Coquelin
2022-09-24  0:20     ` Chautru, Nicolas
2022-09-27 13:56       ` Maxime Coquelin
2022-09-22  0:27 ` [PATCH v4 14/14] baseband/acc: simplify meson dependency Nic Chautru
2022-09-23 11:41 ` From: Nic Chautru <nicolas.chautru@intel.com> Maxime Coquelin

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=BY5PR11MB4451329F778E5CE7FB8E9DF4F8519@BY5PR11MB4451.namprd11.prod.outlook.com \
    --to=nicolas.chautru@intel.com \
    --cc=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=hemant.agrawal@nxp.com \
    --cc=hernan.vargas@intel.com \
    --cc=maxime.coquelin@redhat.com \
    --cc=mdr@ashroe.eu \
    --cc=stephen@networkplumber.org \
    --cc=thomas@monjalon.net \
    --cc=trix@redhat.com \
    /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).