On Fri, Mar 12, 2021 at 03:30:11PM +0000, Carrillo, Erik G wrote:
> Hi Shijith,
>
> Some comments in-line:
>
> > -----Original Message-----
> > From: Shijith Thotton <sthot...@marvell.com>
> > Sent: Monday, March 8, 2021 2:46 PM
> > To: Carrillo, Erik G <erik.g.carri...@intel.com>
> > Cc: Shijith Thotton <sthot...@marvell.com>; Pavan Nikhilesh
> > <pbhagavat...@marvell.com>; Jerin Jacob <jer...@marvell.com>;
> > dev@dpdk.org
> > Subject: [PATCH 1/3] eventdev: introduce adapter flags for periodic mode
> >
> > A timer adapter in periodic mode can be used to arm periodic timers.
> > This patch adds flags used to advertise capability and configure timer
> > adapter
> > in periodic mode. Capability flag should be set for adapters which support
> > periodic mode.
> >
> > Below is a programming sequence on the usage:
> > /* check for periodic mode support by reading capability. */
> > rte_event_timer_adapter_caps_get(...);
> >
> > /* create adapter in periodic mode by setting periodic flag
> > (RTE_EVENT_TIMER_ADAPTER_F_PERIODIC) and resolution. */
> > rte_event_timer_adapter_create_ext(...);
> >
> > /* arm periodic timer of configured resolution */
> > rte_event_timer_arm_burst(...);
> >
> > /* timer event will be periodically generated at configured
> > resolution till cancel is called. */
> > while (running) { rte_event_dequeue_burst(...); }
> >
> > /* cancel periodic timer which stops generating events */
> > rte_event_timer_cancel_burst(...);
> >
> > Signed-off-by: Shijith Thotton <sthot...@marvell.com>
> > ---
> > doc/guides/prog_guide/event_timer_adapter.rst | 33
> > +++++++++++++++++++ lib/librte_eventdev/rte_event_timer_adapter.h |
> > 6 ++++
> > lib/librte_eventdev/rte_eventdev.h | 3 ++
> > 3 files changed, 42 insertions(+)
> >
> > diff --git a/doc/guides/prog_guide/event_timer_adapter.rst
> > b/doc/guides/prog_guide/event_timer_adapter.rst
> > index a95efbe0d..fcdecdb3b 100644
> > --- a/doc/guides/prog_guide/event_timer_adapter.rst
> > +++ b/doc/guides/prog_guide/event_timer_adapter.rst
> > @@ -252,6 +252,39 @@ be canceled by calling
> > ``rte_event_timer_cancel_burst()``:
> > */
> > rte_event_timer_cancel_burst(adapter, &conn->timer, 1);
> >
> > +Timer adapter in periodic mode
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +A periodic timer is a timer, which expires at fixed time intervals
> > +continuously till it is cancelled. A timer adapter can be configured
> > +to arm such timers by configuring it in periodic mode. Capability flag
> > +``RTE_EVENT_TIMER_ADAPTER_CAP_PERIODIC`` can be used to check if an
> > +adapter supports periodic mode. An adapter in periodic mode can only be
> > +used to arm periodic timers.
> > +
> > +Example config to create an adapter in periodic mode with 100ms
> > resolution:
> > +
> > +.. code-block:: c
> > +
> > + #define NSECPERSEC 1E9 // No of ns in 1 sec
> > + const struct rte_event_timer_adapter_conf adapter_config = {
> > + .event_dev_id = event_dev_id,
> > + .timer_adapter_id = 0,
> > + .clk_src = RTE_EVENT_TIMER_ADAPTER_CPU_CLK,
> > + .timer_tick_ns = NSECPERSEC / 10, // 100 milliseconds
> > + .nb_timers = 100,
> > + .timer_adapter_flags = RTE_EVENT_TIMER_ADAPTER_F_PERIODIC,
> > + };
> > +
> > +``timer_adapter_flags`` is set to include periodic flag
> > +``RTE_EVENT_TIMER_ADAPTER_F_PERIODIC`` and ``timer_tick_ns`` to
> > resolution.
> > +Maximum timeout (``max_tmo_nsec``) does not apply for periodic mode.
>
> This content can be reworked slightly and moved into the "Create and
> Configure an Adapter Instance" section, perhaps as a subsection describing
> the two adapter modes.
>
Reworked as suggested in v2.
> > +
> > +After starting the adapter, event timer arm APIs can be used to arm
> > +periodic timers. Timer events will be generated at configured
> > +``timer_tick_ns`` intervals. Event generation can be stopped using
> > +cancel API ``rte_event_timer_cancel_burst``.
> > +
>
> I think this content should be worked into the "Arming Event Timers" section,
> indicating how the timer behavior depends on the mode of the adapter that
> specified in the arm call.
>
Reworked as suggested in v2.
> > Processing Timer Expiry Events
> > ------------------------------
> >
> > diff --git a/lib/librte_eventdev/rte_event_timer_adapter.h
> > b/lib/librte_eventdev/rte_event_timer_adapter.h
> > index d2ebcb090..f9fc95711 100644
> > --- a/lib/librte_eventdev/rte_event_timer_adapter.h
> > +++ b/lib/librte_eventdev/rte_event_timer_adapter.h
>
> I think we should also update the doxygen documentation for the
> rte_event_timer_arm_burst to indicate that timer expiry events will be
> generated once or periodically until the timer is stopped based on the mode
> of the adapter that is specified. We can have a "see also" for the
> RTE_EVENT_TIMER_ADAPTER_F_PERIODIC flag.
>
Updated comments in v2.
> > @@ -151,6 +151,12 @@ enum rte_event_timer_adapter_clk_src {
> > * @see struct rte_event_timer_adapter_conf::flags
> > */
> >
> > +#define RTE_EVENT_TIMER_ADAPTER_F_PERIODIC (1ULL << 2)
> > +/**< Flag to configure event timer adapter in periodic mode.
> > + *
> > + * @see struct rte_event_timer_adapter_conf::flags
> > + */
> > +
> > /**
> > * Timer adapter configuration structure
> > */
>
Thanks,
Shijith
