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

Message ID 20210308204543.2903723-2-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 8, 2021, 8: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 | 33 +++++++++++++++++++
 lib/librte_eventdev/rte_event_timer_adapter.h |  6 ++++
 lib/librte_eventdev/rte_eventdev.h            |  3 ++
 3 files changed, 42 insertions(+)
  

Comments

Carrillo, Erik G March 9, 2021, 8:04 p.m. UTC | #1
Hi Shijith,

Please see a question in-line:

> -----Original Message-----
> From: Shijith Thotton <sthotton@marvell.com>
> Sent: Monday, March 8, 2021 2:46 PM
> 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 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(...);

It looks like periodic support is an operating mode of the adapter itself, and that all timers created with a periodic adapter instance will be periodic timers.

Is it possible to instead have "periodic/single-shot" be an attribute of an event timer itself, such that a single adapter instance could support either type of timer?

Thanks,
Erik

> 
> 	/* 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>
> ---
  
Shijith Thotton March 10, 2021, 9:14 a.m. UTC | #2
Hi Erik,

On Tue, Mar 09, 2021 at 08:04:32PM +0000, Carrillo, Erik G wrote:
> > 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(...);
> 
> It looks like periodic support is an operating mode of the adapter itself, and
> that all timers created with a periodic adapter instance will be periodic
> timers.
> 
> Is it possible to instead have "periodic/single-shot" be an attribute of an
> event timer itself, such that a single adapter instance could support either
> type of timer?
> 

With single type of timer per adapter, application can decide to create multiple
adapters of required type/mode and use as needed.

For an adapter to support both type of timers, driver ops implementation has to
follow different paths based on timer type and new capability flag should be
introduced to expose this feature. Our HW only supports single type of timer per
adapter.

Please let me know the approach you are aligned with.

Thanks,
Shijith
  
Carrillo, Erik G March 11, 2021, 8:47 p.m. UTC | #3
Hi Shijith,

> -----Original Message-----
> From: Shijith Thotton <shijith.thotton@gmail.com>
> Sent: Wednesday, March 10, 2021 3:15 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: Re: [dpdk-dev] [PATCH 1/3] eventdev: introduce adapter flags for
> periodic mode
> 
> Hi Erik,
> 
> On Tue, Mar 09, 2021 at 08:04:32PM +0000, Carrillo, Erik G wrote:
> > > 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(...);
> >
> > It looks like periodic support is an operating mode of the adapter
> > itself, and that all timers created with a periodic adapter instance
> > will be periodic timers.
> >
> > Is it possible to instead have "periodic/single-shot" be an attribute
> > of an event timer itself, such that a single adapter instance could
> > support either type of timer?
> >
> 
> With single type of timer per adapter, application can decide to create
> multiple adapters of required type/mode and use as needed.
> 
> For an adapter to support both type of timers, driver ops implementation has
> to follow different paths based on timer type and new capability flag should
> be introduced to expose this feature. Our HW only supports single type of
> timer per adapter.
> 
> Please let me know the approach you are aligned with.

Having a single type of timer per adapter is surprising initially, in my opinion,
but I think it does make using periodic timers simple.  Unless there are any
other comments to the contrary, I think we can proceed with this approach.

I'll respond in a separate post with comments on the patches.

Thanks,
Erik

> 
> Thanks,
> Shijith
  
Carrillo, Erik G March 12, 2021, 3:30 p.m. UTC | #4
Hi Shijith,

Some comments in-line:

> -----Original Message-----
> From: Shijith Thotton <sthotton@marvell.com>
> Sent: Monday, March 8, 2021 2:46 PM
> 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 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 | 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.

> +
> +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.

>  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.

Thanks,
Erik

> @@ -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
>   */
  
Shijith Thotton March 14, 2021, 4:49 p.m. UTC | #5
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 <sthotton@marvell.com>
> > Sent: Monday, March 8, 2021 2:46 PM
> > 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 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 | 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
  

Patch

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.
+
+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``.
+
 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
@@ -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
  */
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.
  *