Hi Santosh,
On Sun, Feb 18, 2018 at 05:12:33PM +0530, santosh wrote:
> Hi Pavan,
>
>
> On Saturday 17 February 2018 03:06 AM, Pavan Nikhilesh wrote:
> > Add compile-time configurable option to force TIMvf to use Octeontx
> > FPAvf pool manager as its chunk pool.
> > When FPAvf is used as pool manager the TIMvf automatically frees the
> > chunks to FPAvf through gpool-id.
> >
> > Signed-off-by: Pavan Nikhilesh <pbhagavat...@caviumnetworks.com>
> > ---
> > config/common_base | 1 +
> > drivers/event/octeontx/timvf_evdev.c | 23 +++++++++++++++++++++++
> > drivers/event/octeontx/timvf_evdev.h | 3 +++
> > drivers/event/octeontx/timvf_worker.h | 35
> > +++++++++++++++++++++++++++++++++++
> > 4 files changed, 62 insertions(+)
> >
> > diff --git a/config/common_base b/config/common_base
> > index ad03cf433..00010de92 100644
> > --- a/config/common_base
> > +++ b/config/common_base
> > @@ -562,6 +562,7 @@ CONFIG_RTE_LIBRTE_PMD_SW_EVENTDEV=y
> > # Compile PMD for octeontx sso event device
> > #
> > CONFIG_RTE_LIBRTE_PMD_OCTEONTX_SSOVF=y
> > +CONFIG_RTE_PMD_OCTEONTX_TIMVF_USE_FPAVF=n
> >
>
> How about using CONFIG_RTE_LIBRTE_OCTEONTX_MEMPOOL?
I think CONFIG_RTE_LIBRTE_OCTEONTX_MEMPOOL is always enabled by default,using
something like CONFIG_RTE_LIBRTE_TIMVF_USE_OCTEONTX_MEMPOOL be better? i.e
telling TIMvf to specifically use fpa.
>
> > #
> > # Compile PMD for OPDL event device
> > diff --git a/drivers/event/octeontx/timvf_evdev.c
> > b/drivers/event/octeontx/timvf_evdev.c
> > index ffdfbb387..386eaa08f 100644
> > --- a/drivers/event/octeontx/timvf_evdev.c
> > +++ b/drivers/event/octeontx/timvf_evdev.c
> > @@ -162,10 +162,27 @@ timvf_ring_start(const struct rte_event_timer_adapter
> > *adptr)
> > 1ull << 48 |
> > 1ull << 47 |
> > 1ull << 44 |
> > +#ifndef RTE_PMD_OCTEONTX_TIMVF_USE_FPAVF
> > + 1ull << 43 |
> > +#endif
> > (timr->meta.nb_bkts - 1);
> >
> > rctrl.rctrl2 = (uint64_t)(TIM_CHUNK_SIZE / 16) << 40;
> >
> > +#ifdef RTE_PMD_OCTEONTX_TIMVF_USE_FPAVF
> > + uintptr_t pool;
> > + pool = (uintptr_t)((struct rte_mempool *)
> > + timr->meta.chunk_pool)->pool_id;
> > + ret = octeontx_fpa_bufpool_gpool(pool);
> > + if (ret < 0) {
> > + timvf_log_dbg("Unable to get gaura id");
> > + ret = -ENOMEM;
> > + goto error;
> > + }
> > + timvf_write64((uint64_t)ret,
> > + (uint8_t *)timr->vbar0 + TIM_VRING_AURA);
> > +#endif
> > +
> > timvf_write64((uint64_t)timr->meta.bkt,
> > (uint8_t *)timr->vbar0 + TIM_VRING_BASE);
> > if (timvf_ring_conf_set(&rctrl, timr->tim_ring_id)) {
> > @@ -296,9 +313,15 @@ timvf_ring_create(struct rte_event_timer_adapter
> > *adptr)
> > return -ENOMEM;
> > }
> >
> > +#ifdef RTE_PMD_OCTEONTX_TIMVF_USE_FPAVF
> > + ret = rte_mempool_set_ops_byname(timr->meta.chunk_pool,
> > + "octeontx_fpavf", NULL);
> > + timvf_log_dbg("Giving back chunks to fpa gaura : %d", ret);
> > +#else
> > ret = rte_mempool_set_ops_byname(timr->meta.chunk_pool,
> > RTE_MBUF_DEFAULT_MEMPOOL_OPS, NULL);
> > timvf_log_dbg("Not giving back chunks to fpa");
> > +#endif
> >
>
> May be use rte_mbuf_best_mempool_ops Or rte_mbuf_user_mempool_ops,
> that way avoid above ifdef.
In timvf_worker.h the logic used to arm the event timer changes with the
mempool used i.e. octeontx_fpavf has a different implementation vs ring.
We could use rte_mbuf_best_mempool_ops() but again need check if the
returned ops is fpa to enable automatic buffer recycling.
>
> Also a suggestion, Try to reduce ifdef by creating a new header file
> called timvf_fpa_evdev.h, abstract all possible ifdefs their,
> create small static inline API and call them in timvf eventdev driver.
> [...]
Agreed, will modify arm logic to reduce ifdef clutter.
>
> Thanks.
>
Thanks,
Pavan.
