From: "Chautru, Nicolas" <nicolas.chautru@intel.com>
To: Akhil Goyal <gakhil@marvell.com>,
"Vargas, Hernan" <hernan.vargas@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>,
"trix@redhat.com" <trix@redhat.com>
Cc: "Zhang, Qi Z" <qi.z.zhang@intel.com>
Subject: RE: [EXT] [PATCH v1 2/5] baseband/fpga_5gnr_fec: add FPGA Mutex
Date: Fri, 20 May 2022 01:30:06 +0000 [thread overview]
Message-ID: <BY5PR11MB44518C8097E6E45B4106451CF8D39@BY5PR11MB4451.namprd11.prod.outlook.com> (raw)
In-Reply-To: <CO6PR18MB448495FC1B822FC8745EA208D8CF9@CO6PR18MB4484.namprd18.prod.outlook.com>
Hi Hernan,
Let's be more explicit on the commit message.
An explicit mutex is added when using the register interface for HARQ memory preloading (used for unit test and/or troubleshoot) so that to prevent multiple threads to use the same interface in parallel.
This feature is implemented through MMIO exposed per VF and common to all queues unlike the default enqueue operation where each queues are isolated by design.
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Monday, May 16, 2022 11:09 AM
> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
> trix@redhat.com
> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Subject: RE: [EXT] [PATCH v1 2/5] baseband/fpga_5gnr_fec: add FPGA Mutex
>
> > FPGA mutex acquisition and mutex free implemented.
>
> Please provide better description to the patches.
>
> >
> > Signed-off-by: Hernan <hernan.vargas@intel.com>
> > ---
> > .../baseband/fpga_5gnr_fec/fpga_5gnr_fec.h | 6 +-
> > .../fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 77 ++++++++++++++-----
> > 2 files changed, 62 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h
> > b/drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h
> > index ed8ce26eaa..993cf61974 100644
> > --- a/drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h
> > +++ b/drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h
> > @@ -82,7 +82,9 @@ enum {
> > FPGA_5GNR_FEC_DDR4_RD_DATA_REGS = 0x00000A30, /* len: 8B
> */
> > FPGA_5GNR_FEC_DDR4_ADDR_RDY_REGS = 0x00000A38, /* len: 1B
> */
> > FPGA_5GNR_FEC_HARQ_BUF_SIZE_RDY_REGS = 0x00000A40, /* len:
> 1B */
> > - FPGA_5GNR_FEC_HARQ_BUF_SIZE_REGS = 0x00000A48 /* len: 4B */
> > + FPGA_5GNR_FEC_HARQ_BUF_SIZE_REGS = 0x00000A48, /* len: 4B */
> > + FPGA_5GNR_FEC_MUTEX = 0x00000A60, /* len: 4B */
> > + FPGA_5GNR_FEC_MUTEX_RESET = 0x00000A68 /* len: 4B */
> > };
> >
> > /* FPGA 5GNR FEC Ring Control Registers */ @@ -264,6 +266,8 @@ struct
> > __rte_cache_aligned fpga_queue {
> > uint32_t sw_ring_wrap_mask;
> > uint32_t irq_enable; /* Enable ops dequeue interrupts if set to 1 */
> > uint8_t q_idx; /* Queue index */
> > + /** uuid used for MUTEX acquision for DDR */
> > + uint16_t ddr_mutex_uuid;
> > struct fpga_5gnr_fec_device *d;
> > /* MMIO register of shadow_tail used to enqueue descriptors */
> > void *shadow_tail_addr;
> > diff --git a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> > b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> > index 6737b74901..435b4d90d8 100644
> > --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> > +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> > @@ -1194,11 +1194,45 @@ validate_dec_op(struct rte_bbdev_dec_op
> *op
> > __rte_unused)
> > }
> > #endif
> >
> > +static inline void
> > +fpga_mutex_acquisition(struct fpga_queue *q) {
> > + uint32_t mutex_ctrl, mutex_read, cnt = 0;
> > + /* Assign a unique id for the duration of the DDR access */
> > + q->ddr_mutex_uuid = rand();
> > + /* Request and wait for acquisition of the mutex */
> > + mutex_ctrl = (q->ddr_mutex_uuid << 16) + 1;
> > + do {
> > + if (cnt > 0)
> > + usleep(FPGA_TIMEOUT_CHECK_INTERVAL);
> > + rte_bbdev_log_debug("Acquiring Mutex for %x\n",
> > + q->ddr_mutex_uuid);
> > + fpga_reg_write_32(q->d->mmio_base,
> > + FPGA_5GNR_FEC_MUTEX,
> > + mutex_ctrl);
> > + mutex_read = fpga_reg_read_32(q->d->mmio_base,
> > + FPGA_5GNR_FEC_MUTEX);
> > + rte_bbdev_log_debug("Mutex %x cnt %d owner %x\n",
> > + mutex_read, cnt, q->ddr_mutex_uuid);
> > + cnt++;
> > + } while ((mutex_read >> 16) != q->ddr_mutex_uuid); }
> > +
> > +static inline void
> > +fpga_mutex_free(struct fpga_queue *q) {
> > + uint32_t mutex_ctrl = q->ddr_mutex_uuid << 16;
> > + fpga_reg_write_32(q->d->mmio_base,
> > + FPGA_5GNR_FEC_MUTEX,
> > + mutex_ctrl);
> > +}
> > +
> > static inline int
> > -fpga_harq_write_loopback(struct fpga_5gnr_fec_device *fpga_dev,
> > +fpga_harq_write_loopback(struct fpga_queue *q,
> > struct rte_mbuf *harq_input, uint16_t harq_in_length,
> > uint32_t harq_in_offset, uint32_t harq_out_offset) {
> > + fpga_mutex_acquisition(q);
> > uint32_t out_offset = harq_out_offset;
> > uint32_t in_offset = harq_in_offset;
> > uint32_t left_length = harq_in_length; @@ -1215,7 +1249,7 @@
> > fpga_harq_write_loopback(struct fpga_5gnr_fec_device *fpga_dev,
> > * Get HARQ buffer size for each VF/PF: When 0x00, there is no
> > * available DDR space for the corresponding VF/PF.
> > */
> > - reg_32 = fpga_reg_read_32(fpga_dev->mmio_base,
> > + reg_32 = fpga_reg_read_32(q->d->mmio_base,
> > FPGA_5GNR_FEC_HARQ_BUF_SIZE_REGS);
> > if (reg_32 < harq_in_length) {
> > left_length = reg_32;
> > @@ -1226,46 +1260,48 @@ fpga_harq_write_loopback(struct
> > fpga_5gnr_fec_device *fpga_dev,
> > uint8_t *, in_offset);
> >
> > while (left_length > 0) {
> > - if (fpga_reg_read_8(fpga_dev->mmio_base,
> > + if (fpga_reg_read_8(q->d->mmio_base,
> > FPGA_5GNR_FEC_DDR4_ADDR_RDY_REGS)
> ==
> > 1) {
> > - fpga_reg_write_32(fpga_dev->mmio_base,
> > + fpga_reg_write_32(q->d->mmio_base,
> >
> > FPGA_5GNR_FEC_DDR4_WR_ADDR_REGS,
> > out_offset);
> > - fpga_reg_write_64(fpga_dev->mmio_base,
> > + fpga_reg_write_64(q->d->mmio_base,
> >
> > FPGA_5GNR_FEC_DDR4_WR_DATA_REGS,
> > input[increment]);
> > left_length -=
> > FPGA_5GNR_FEC_DDR_WR_DATA_LEN_IN_BYTES;
> > out_offset +=
> > FPGA_5GNR_FEC_DDR_WR_DATA_LEN_IN_BYTES;
> > increment++;
> > - fpga_reg_write_8(fpga_dev->mmio_base,
> > + fpga_reg_write_8(q->d->mmio_base,
> >
> > FPGA_5GNR_FEC_DDR4_WR_DONE_REGS, 1);
> > }
> > }
> > while (last_transaction > 0) {
> > - if (fpga_reg_read_8(fpga_dev->mmio_base,
> > + if (fpga_reg_read_8(q->d->mmio_base,
> > FPGA_5GNR_FEC_DDR4_ADDR_RDY_REGS)
> ==
> > 1) {
> > - fpga_reg_write_32(fpga_dev->mmio_base,
> > + fpga_reg_write_32(q->d->mmio_base,
> >
> > FPGA_5GNR_FEC_DDR4_WR_ADDR_REGS,
> > out_offset);
> > last_word = input[increment];
> > last_word &= (uint64_t)(1 << (last_transaction * 4))
> > - 1;
> > - fpga_reg_write_64(fpga_dev->mmio_base,
> > + fpga_reg_write_64(q->d->mmio_base,
> >
> > FPGA_5GNR_FEC_DDR4_WR_DATA_REGS,
> > last_word);
> > - fpga_reg_write_8(fpga_dev->mmio_base,
> > + fpga_reg_write_8(q->d->mmio_base,
> >
> > FPGA_5GNR_FEC_DDR4_WR_DONE_REGS, 1);
> > last_transaction = 0;
> > }
> > }
> > + fpga_mutex_free(q);
> > return 1;
> > }
> >
> > static inline int
> > -fpga_harq_read_loopback(struct fpga_5gnr_fec_device *fpga_dev,
> > +fpga_harq_read_loopback(struct fpga_queue *q,
> > struct rte_mbuf *harq_output, uint16_t harq_in_length,
> > uint32_t harq_in_offset, uint32_t harq_out_offset) {
> > + fpga_mutex_acquisition(q);
> > uint32_t left_length, in_offset = harq_in_offset;
> > uint64_t reg;
> > uint32_t increment = 0;
> > @@ -1276,7 +1312,7 @@ fpga_harq_read_loopback(struct
> > fpga_5gnr_fec_device *fpga_dev,
> > if (last_transaction > 0)
> > harq_in_length += (8 - last_transaction);
> >
> > - reg = fpga_reg_read_32(fpga_dev->mmio_base,
> > + reg = fpga_reg_read_32(q->d->mmio_base,
> > FPGA_5GNR_FEC_HARQ_BUF_SIZE_REGS);
> > if (reg < harq_in_length) {
> > harq_in_length = reg;
> > @@ -1302,14 +1338,14 @@ fpga_harq_read_loopback(struct
> > fpga_5gnr_fec_device *fpga_dev,
> > uint8_t *, harq_out_offset);
> >
> > while (left_length > 0) {
> > - fpga_reg_write_32(fpga_dev->mmio_base,
> > + fpga_reg_write_32(q->d->mmio_base,
> > FPGA_5GNR_FEC_DDR4_RD_ADDR_REGS, in_offset);
> > - fpga_reg_write_8(fpga_dev->mmio_base,
> > + fpga_reg_write_8(q->d->mmio_base,
> > FPGA_5GNR_FEC_DDR4_RD_DONE_REGS, 1);
> > - reg = fpga_reg_read_8(fpga_dev->mmio_base,
> > + reg = fpga_reg_read_8(q->d->mmio_base,
> > FPGA_5GNR_FEC_DDR4_RD_RDY_REGS);
> > while (reg != 1) {
> > - reg = fpga_reg_read_8(fpga_dev->mmio_base,
> > + reg = fpga_reg_read_8(q->d->mmio_base,
> > FPGA_5GNR_FEC_DDR4_RD_RDY_REGS);
> > if (reg == FPGA_DDR_OVERFLOW) {
> > rte_bbdev_log(ERR,
> > @@ -1317,14 +1353,15 @@ fpga_harq_read_loopback(struct
> > fpga_5gnr_fec_device *fpga_dev,
> > return -1;
> > }
> > }
> > - input[increment] = fpga_reg_read_64(fpga_dev-
> >mmio_base,
> > + input[increment] = fpga_reg_read_64(q->d->mmio_base,
> > FPGA_5GNR_FEC_DDR4_RD_DATA_REGS);
> > left_length -=
> > FPGA_5GNR_FEC_DDR_RD_DATA_LEN_IN_BYTES;
> > in_offset +=
> FPGA_5GNR_FEC_DDR_WR_DATA_LEN_IN_BYTES;
> > increment++;
> > - fpga_reg_write_8(fpga_dev->mmio_base,
> > + fpga_reg_write_8(q->d->mmio_base,
> > FPGA_5GNR_FEC_DDR4_RD_DONE_REGS, 0);
> > }
> > + fpga_mutex_free(q);
> > return 1;
> > }
> >
> > @@ -1467,13 +1504,13 @@ enqueue_ldpc_dec_one_op_cb(struct
> fpga_queue
> > *q, struct rte_bbdev_dec_op *op,
> > if (check_bit(dec->op_flags,
> >
> > RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_OUT_ENABLE
> > )) {
> > - ret = fpga_harq_write_loopback(q->d, harq_in,
> > + ret = fpga_harq_write_loopback(q, harq_in,
> > harq_in_length, harq_in_offset,
> > harq_out_offset);
> > } else if (check_bit(dec->op_flags,
> >
> > RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_IN_ENABLE
> > )) {
> > - ret = fpga_harq_read_loopback(q->d, harq_out,
> > + ret = fpga_harq_read_loopback(q, harq_out,
> > harq_in_length, harq_in_offset,
> > harq_out_offset);
> > dec->harq_combined_output.length =
> harq_in_length;
> > --
> > 2.25.1
next prev parent reply other threads:[~2022-05-20 1:30 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-09 20:17 [PATCH v1 0/5] baseband/fpga_5gnr: maintenance changes to fpga_5gnr PMD Hernan
2022-05-09 20:17 ` [PATCH v1 1/5] baseband/fpga_5gnr_fec: remove FLR timeout Hernan
2022-05-09 20:17 ` [PATCH v1 2/5] baseband/fpga_5gnr_fec: add FPGA Mutex Hernan
2022-05-16 18:08 ` [EXT] " Akhil Goyal
2022-05-20 1:30 ` Chautru, Nicolas [this message]
2022-05-09 20:17 ` [PATCH v1 3/5] baseband/fpga_5gnr_fec: add check for HARQ input length Hernan
2022-05-09 20:17 ` [PATCH v1 4/5] baseband/fpga_5gnr_fec: enable validate LDPC enc/dec Hernan
2022-05-09 21:04 ` Chautru, Nicolas
2022-05-09 20:17 ` [PATCH v1 5/5] baseband/fpga_5gnr_fec: remove filler from HARQ Hernan
2022-05-16 18:07 ` [EXT] " Akhil Goyal
2022-05-20 1:25 ` Chautru, Nicolas
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=BY5PR11MB44518C8097E6E45B4106451CF8D39@BY5PR11MB4451.namprd11.prod.outlook.com \
--to=nicolas.chautru@intel.com \
--cc=dev@dpdk.org \
--cc=gakhil@marvell.com \
--cc=hernan.vargas@intel.com \
--cc=qi.z.zhang@intel.com \
--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).