[v2,1/3] eventdev: introduce adapter flags for periodic mode

Message ID 3d202f9526dcb90ffbcd55fc4b5888989321b797.1615738894.git.sthotton@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: Jerin Jacob
Headers
Series periodic mode for event timer adapter |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Shijith Thotton March 14, 2021, 4:45 p.m. UTC
  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 <sthotton@marvell.com>
---
 doc/guides/prog_guide/event_timer_adapter.rst | 15 ++++++++++++++-
 lib/librte_eventdev/rte_event_timer_adapter.h | 11 +++++++++++
 lib/librte_eventdev/rte_eventdev.h            |  3 +++
 3 files changed, 28 insertions(+), 1 deletion(-)
  

Comments

Carrillo, Erik G March 16, 2021, 7:04 p.m. UTC | #1
Thanks, Shijith.  I'm posting a couple of follow-up comments in-line:

> -----Original Message-----
> From: Shijith Thotton <sthotton@marvell.com>
> Sent: Sunday, March 14, 2021 11:46 AM
> To: Carrillo, Erik G <erik.g.carrillo@intel.com>
> Cc: Shijith Thotton <sthotton@marvell.com>; Pavan Nikhilesh
> <pbhagavatula@marvell.com>; Jerin Jacob <jerinj@marvell.com>;
> dev@dpdk.org
> Subject: [PATCH v2 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 <sthotton@marvell.com>
> ---
>  doc/guides/prog_guide/event_timer_adapter.rst | 15 ++++++++++++++-
> lib/librte_eventdev/rte_event_timer_adapter.h | 11 +++++++++++
>  lib/librte_eventdev/rte_eventdev.h            |  3 +++
>  3 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/guides/prog_guide/event_timer_adapter.rst
> b/doc/guides/prog_guide/event_timer_adapter.rst
> index a95efbe0d..0b2add6f7 100644
> --- a/doc/guides/prog_guide/event_timer_adapter.rst
> +++ b/doc/guides/prog_guide/event_timer_adapter.rst
> @@ -138,6 +138,17 @@ This function is passed a callback function that will be
> invoked if the  adapter needs to create an event port, giving the application
> the opportunity  to control how it is done.
> 
> +Adapter modes
> +^^^^^^^^^^^^^
> +An event timer adapter can be configured in either periodic or
> +non-periodic mode to support timers of respective type. A periodic

"...timers of the respective type..."

> +timer expires at a fixed time interval repeatedly till it is cancelled.
> +A non-periodic timer expires only once. Periodic capability flag
> +``RTE_EVENT_TIMER_ADAPTER_CAP_PERIODIC`` is set for adapters which
> +support periodic mode. To configure an adapter in periodic mode,

"The periodic capability flag, ``RTE_EVENT_TIMER_ADAPTER_CAP_PERIODIC``, can be set for implementations that support periodic mode if desired."

> +``timer_adapter_flags`` of ``rte_event_timer_adapter_conf`` is set to
> +include periodic flag ``RTE_EVENT_TIMER_ADAPTER_F_PERIODIC``.

"...include the periodic flag..."

> Maximum
> +timeout
> +(``max_tmo_nsec``) does not apply for periodic mode.
> +
>  Retrieve Event Timer Adapter Contextual Information
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  The event timer adapter implementation may have constraints on tick
> resolution @@ -229,7 +240,9 @@ Now we can arm the event timer with
> ``rte_event_timer_arm_burst()``:
> 
>  Once an event timer expires, the application may free it or rearm it as
> necessary.  If the application will rearm the timer, the state should be reset -
> to RTE_EVENT_TIMER_NOT_ARMED by the application before rearming it.
> +to RTE_EVENT_TIMER_NOT_ARMED by the application before rearming it.
> +Timer expiry events will be generated once or periodically until the
> +timer is cancelled based on adapter mode.
> 
>  Multiple Event Timers with Same Expiry Value
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> diff --git a/lib/librte_eventdev/rte_event_timer_adapter.h
> b/lib/librte_eventdev/rte_event_timer_adapter.h
> index d2ebcb090..b34dc1385 100644
> --- a/lib/librte_eventdev/rte_event_timer_adapter.h
> +++ b/lib/librte_eventdev/rte_event_timer_adapter.h
> @@ -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.

I think this could be elaborated on, perhaps like:

"Flag to configure an event timer adapter in periodic mode; non-periodic mode is the default.  A timer will fire once or periodically until the timer is cancelled based on the adapter mode."

> + *
> + * @see struct rte_event_timer_adapter_conf::flags
> + */
> +
>  /**
>   * Timer adapter configuration structure
>   */

<... snipped ...>

You should also be able to mark old versions of the series as superseded.  Also, there were a couple of coding style warnings on the second patch in the series regarding block comment syntax.

Thanks,
Erik
  
Shijith Thotton March 17, 2021, 6:10 a.m. UTC | #2
On Tue, Mar 16, 2021 at 07:04:16PM +0000, Carrillo, Erik G wrote:
> Thanks, Shijith.  I'm posting a couple of follow-up comments in-line:
> 
> > -----Original Message-----
> > From: Shijith Thotton <sthotton@marvell.com>
> > Sent: Sunday, March 14, 2021 11:46 AM
> > To: Carrillo, Erik G <erik.g.carrillo@intel.com>
> > Cc: Shijith Thotton <sthotton@marvell.com>; Pavan Nikhilesh
> > <pbhagavatula@marvell.com>; Jerin Jacob <jerinj@marvell.com>;
> > dev@dpdk.org
> > Subject: [PATCH v2 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 <sthotton@marvell.com>
> > ---
> >  doc/guides/prog_guide/event_timer_adapter.rst | 15 ++++++++++++++-
> > lib/librte_eventdev/rte_event_timer_adapter.h | 11 +++++++++++
> >  lib/librte_eventdev/rte_eventdev.h            |  3 +++
> >  3 files changed, 28 insertions(+), 1 deletion(-)
> > 
> > diff --git a/doc/guides/prog_guide/event_timer_adapter.rst
> > b/doc/guides/prog_guide/event_timer_adapter.rst
> > index a95efbe0d..0b2add6f7 100644
> > --- a/doc/guides/prog_guide/event_timer_adapter.rst
> > +++ b/doc/guides/prog_guide/event_timer_adapter.rst
> > @@ -138,6 +138,17 @@ This function is passed a callback function that will be
> > invoked if the  adapter needs to create an event port, giving the application
> > the opportunity  to control how it is done.
> > 
> > +Adapter modes
> > +^^^^^^^^^^^^^
> > +An event timer adapter can be configured in either periodic or
> > +non-periodic mode to support timers of respective type. A periodic
> 
> "...timers of the respective type..."
> 

Will change in v3.

> > +timer expires at a fixed time interval repeatedly till it is cancelled.
> > +A non-periodic timer expires only once. Periodic capability flag
> > +``RTE_EVENT_TIMER_ADAPTER_CAP_PERIODIC`` is set for adapters which
> > +support periodic mode. To configure an adapter in periodic mode,
> 
> "The periodic capability flag, ``RTE_EVENT_TIMER_ADAPTER_CAP_PERIODIC``, can be set for implementations that support periodic mode if desired."

Will change in v3.

> 
> > +``timer_adapter_flags`` of ``rte_event_timer_adapter_conf`` is set to
> > +include periodic flag ``RTE_EVENT_TIMER_ADAPTER_F_PERIODIC``.
> 
> "...include the periodic flag..."
> 

Will change in v3.

> > Maximum
> > +timeout
> > +(``max_tmo_nsec``) does not apply for periodic mode.
> > +
> >  Retrieve Event Timer Adapter Contextual Information
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >  The event timer adapter implementation may have constraints on tick
> > resolution @@ -229,7 +240,9 @@ Now we can arm the event timer with
> > ``rte_event_timer_arm_burst()``:
> > 
> >  Once an event timer expires, the application may free it or rearm it as
> > necessary.  If the application will rearm the timer, the state should be reset -
> > to RTE_EVENT_TIMER_NOT_ARMED by the application before rearming it.
> > +to RTE_EVENT_TIMER_NOT_ARMED by the application before rearming it.
> > +Timer expiry events will be generated once or periodically until the
> > +timer is cancelled based on adapter mode.
> > 
> >  Multiple Event Timers with Same Expiry Value
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > diff --git a/lib/librte_eventdev/rte_event_timer_adapter.h
> > b/lib/librte_eventdev/rte_event_timer_adapter.h
> > index d2ebcb090..b34dc1385 100644
> > --- a/lib/librte_eventdev/rte_event_timer_adapter.h
> > +++ b/lib/librte_eventdev/rte_event_timer_adapter.h
> > @@ -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.
> 
> I think this could be elaborated on, perhaps like:
> 
> "Flag to configure an event timer adapter in periodic mode; non-periodic mode is the default.  A timer will fire once or periodically until the timer is cancelled based on the adapter mode."
> 

Sounds good. Will change in v3.

> > + *
> > + * @see struct rte_event_timer_adapter_conf::flags
> > + */
> > +
> >  /**
> >   * Timer adapter configuration structure
> >   */
> 
> <... snipped ...>
> 
> You should also be able to mark old versions of the series as superseded.  Also, there were a couple of coding style warnings on the second patch in the series regarding block comment syntax.
> 

Will do.

Thanks,
Shijith
  

Patch

diff --git a/doc/guides/prog_guide/event_timer_adapter.rst b/doc/guides/prog_guide/event_timer_adapter.rst
index a95efbe0d..0b2add6f7 100644
--- a/doc/guides/prog_guide/event_timer_adapter.rst
+++ b/doc/guides/prog_guide/event_timer_adapter.rst
@@ -138,6 +138,17 @@  This function is passed a callback function that will be invoked if the
 adapter needs to create an event port, giving the application the opportunity
 to control how it is done.
 
+Adapter modes
+^^^^^^^^^^^^^
+An event timer adapter can be configured in either periodic or non-periodic mode
+to support timers of respective type. A periodic timer expires at a fixed time
+interval repeatedly till it is cancelled. A non-periodic timer expires only
+once. Periodic capability flag ``RTE_EVENT_TIMER_ADAPTER_CAP_PERIODIC`` is set
+for adapters which support periodic mode. To configure an adapter in periodic
+mode, ``timer_adapter_flags`` of ``rte_event_timer_adapter_conf`` is set to
+include periodic flag ``RTE_EVENT_TIMER_ADAPTER_F_PERIODIC``. Maximum timeout
+(``max_tmo_nsec``) does not apply for periodic mode.
+
 Retrieve Event Timer Adapter Contextual Information
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 The event timer adapter implementation may have constraints on tick resolution
@@ -229,7 +240,9 @@  Now we can arm the event timer with ``rte_event_timer_arm_burst()``:
 
 Once an event timer expires, the application may free it or rearm it as
 necessary.  If the application will rearm the timer, the state should be reset
-to RTE_EVENT_TIMER_NOT_ARMED by the application before rearming it.
+to RTE_EVENT_TIMER_NOT_ARMED by the application before rearming it. Timer expiry
+events will be generated once or periodically until the timer is cancelled based
+on adapter mode.
 
 Multiple Event Timers with Same Expiry Value
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/lib/librte_eventdev/rte_event_timer_adapter.h b/lib/librte_eventdev/rte_event_timer_adapter.h
index d2ebcb090..b34dc1385 100644
--- a/lib/librte_eventdev/rte_event_timer_adapter.h
+++ b/lib/librte_eventdev/rte_event_timer_adapter.h
@@ -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
  */
@@ -551,6 +557,8 @@  struct rte_event_timer_adapter {
  * expiry event attributes, timeout ticks from now.
  * This function submits the event timer arm requests to the event timer adapter
  * and on expiry, the events will be injected to designated event queue.
+ * Timer expiry events will be generated once or periodically until cancellation
+ * based on the adapter mode.
  *
  * @param adapter
  *   A pointer to an event timer adapter structure.
@@ -570,6 +578,9 @@  struct rte_event_timer_adapter {
  *   destination event queue.
  *   - EAGAIN Specified timer adapter is not running
  *   - EALREADY A timer was encountered that was already armed
+ *
+ * @see RTE_EVENT_TIMER_ADAPTER_F_PERIODIC
+ *
  */
 static inline uint16_t
 rte_event_timer_arm_burst(const struct rte_event_timer_adapter *adapter,
diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
index ce1fc2ce0..9fc39e9ca 100644
--- a/lib/librte_eventdev/rte_eventdev.h
+++ b/lib/librte_eventdev/rte_eventdev.h
@@ -1154,6 +1154,9 @@  rte_event_eth_rx_adapter_caps_get(uint8_t dev_id, uint16_t eth_port_id,
 #define RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT (1ULL << 0)
 /**< This flag is set when the timer mechanism is in HW. */
 
+#define RTE_EVENT_TIMER_ADAPTER_CAP_PERIODIC      (1ULL << 1)
+/**< This flag is set if periodic mode is supported. */
+
 /**
  * Retrieve the event device's timer adapter capabilities.
  *