diff mbox

[1/2] Use timer resolution

Message ID 1409235313-20445-1-git-send-email-petri.savolainen@linaro.org
State New
Headers show

Commit Message

Petri Savolainen Aug. 28, 2014, 2:15 p.m. UTC 
Use resolution and min/max tmo values from timer create call.

Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>
---
 platform/linux-generic/odp_timer.c | 68 ++++++++++++++++++++++++++++----------
 1 file changed, 51 insertions(+), 17 deletions(-)

Comments

Anders Roxell Sept. 2, 2014, 7:34 a.m. UTC | #1 
On 2014-08-28 17:15, Petri Savolainen wrote:
> Use resolution and min/max tmo values from timer create call.
> 
> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>
> ---
>  platform/linux-generic/odp_timer.c | 68 ++++++++++++++++++++++++++++----------
>  1 file changed, 51 insertions(+), 17 deletions(-)
> 
> diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-generic/odp_timer.c
> index 1bf37f9..07b1029 100644
> --- a/platform/linux-generic/odp_timer.c
> +++ b/platform/linux-generic/odp_timer.c
> @@ -18,9 +18,16 @@
>  
>  #include <string.h>
>  
> +
> +/* Nanoseconds */
> +#define USEC          1000UL
> +#define MSEC          1000000UL
> +#define SEC           1000000000UL

Should this be moved to a common odp_header file?
This is defined in example/timer/odp_timer_test.c as well...
This got introduced in commit c51c0cbbdaa0f4668c966a9f50c7866d6ac489ce

Cheers,
Anders

> +
>  #define NUM_TIMERS    1
>  #define MAX_TICKS     1024
> -#define RESOLUTION_NS 1000000
> +#define MAX_RES       SEC
> +#define MIN_RES       (100*USEC)
>  
>  
>  typedef struct {
> @@ -112,13 +119,13 @@ static int find_and_del_tmo(timeout_t **tmo, odp_timer_tmo_t handle)
>  int odp_timer_cancel_tmo(odp_timer_t timer_hdl, odp_timer_tmo_t tmo)
>  {
>  	int id;
> -	uint64_t tick_idx;
> +	int tick_idx;
>  	timeout_t *cancel_tmo;
>  	odp_timeout_hdr_t *tmo_hdr;
>  	tick_t *tick;
>  
>  	/* get id */
> -	id = timer_hdl - 1;
> +	id = (int)timer_hdl - 1;
>  
>  	tmo_hdr = odp_timeout_hdr((odp_timeout_t) tmo);
>  	/* get tmo_buf to cancel */
> @@ -179,6 +186,7 @@ static void timer_start(timer_ring_t *timer)
>  {
>  	struct sigevent   sigev;
>  	struct itimerspec ispec;
> +	uint64_t res, sec, nsec;
>  
>  	ODP_DBG("\nTimer (%u) starts\n", timer->timer_hdl);
>  
> @@ -194,10 +202,14 @@ static void timer_start(timer_ring_t *timer)
>  		return;
>  	}
>  
> -	ispec.it_interval.tv_sec  = 0;
> -	ispec.it_interval.tv_nsec = RESOLUTION_NS;
> -	ispec.it_value.tv_sec     = 0;
> -	ispec.it_value.tv_nsec    = RESOLUTION_NS;
> +	res  = timer->resolution_ns;
> +	sec  = res / SEC;
> +	nsec = res - sec*SEC;
> +
> +	ispec.it_interval.tv_sec  = (time_t)sec;
> +	ispec.it_interval.tv_nsec = (long)nsec;
> +	ispec.it_value.tv_sec     = (time_t)sec;
> +	ispec.it_value.tv_nsec    = (long)nsec;
>  
>  	if (timer_settime(timer->timerid, 0, &ispec, NULL)) {
>  		ODP_DBG("Timer set failed\n");
> @@ -250,19 +262,41 @@ int odp_timer_disarm_all(void)
>  }
>  
>  odp_timer_t odp_timer_create(const char *name, odp_buffer_pool_t pool,
> -			     uint64_t resolution, uint64_t min_tmo,
> -			     uint64_t max_tmo)
> +			     uint64_t resolution_ns, uint64_t min_ns,
> +			     uint64_t max_ns)
>  {
>  	uint32_t id;
>  	timer_ring_t *timer;
>  	odp_timer_t timer_hdl;
>  	int i;
> -	(void) name; (void) resolution; (void) min_tmo; (void) max_tmo;
> +	uint64_t max_ticks;
> +	(void) name;
> +
> +	if (resolution_ns < MIN_RES)
> +		resolution_ns = MIN_RES;
> +
> +	if (resolution_ns > MAX_RES)
> +		resolution_ns = MAX_RES;
> +
> +	max_ticks = max_ns / resolution_ns;
> +
> +	if (max_ticks > MAX_TICKS) {
> +		ODP_DBG("Maximum timeout too long: %"PRIu64" ticks\n",
> +			max_ticks);
> +		return ODP_TIMER_INVALID;
> +	}
> +
> +	if (min_ns < resolution_ns) {
> +		ODP_DBG("Min timeout %"PRIu64" ns < resolution %"PRIu64" ns\n",
> +			min_ns, resolution_ns);
> +		return ODP_TIMER_INVALID;
> +	}
>  
>  	odp_spinlock_lock(&odp_timer.lock);
>  
>  	if (odp_timer.num_timers >= NUM_TIMERS) {
>  		odp_spinlock_unlock(&odp_timer.lock);
> +		ODP_DBG("All timers allocated\n");
>  		return ODP_TIMER_INVALID;
>  	}
>  
> @@ -281,7 +315,7 @@ odp_timer_t odp_timer_create(const char *name, odp_buffer_pool_t pool,
>  
>  	timer->timer_hdl     = timer_hdl;
>  	timer->pool          = pool;
> -	timer->resolution_ns = RESOLUTION_NS;
> +	timer->resolution_ns = resolution_ns;
>  	timer->max_ticks     = MAX_TICKS;
>  
>  	for (i = 0; i < MAX_TICKS; i++) {
> @@ -308,7 +342,7 @@ odp_timer_tmo_t odp_timer_absolute_tmo(odp_timer_t timer_hdl, uint64_t tmo_tick,
>  	odp_timeout_hdr_t *tmo_hdr;
>  	timer_ring_t *timer;
>  
> -	id = timer_hdl - 1;
> +	id = (int)timer_hdl - 1;
>  	timer = &odp_timer.timer[id];
>  
>  	cur_tick = timer->cur_tick;
> @@ -317,17 +351,17 @@ odp_timer_tmo_t odp_timer_absolute_tmo(odp_timer_t timer_hdl, uint64_t tmo_tick,
>  		return ODP_TIMER_TMO_INVALID;
>  	}
>  
> -	tick = tmo_tick - cur_tick;
> -	if (tick > MAX_TICKS) {
> -		ODP_DBG("timeout too far\n");
> +	if ((tmo_tick - cur_tick) > MAX_TICKS) {
> +		ODP_DBG("timeout too far: cur %"PRIu64" tmo %"PRIu64"\n",
> +			cur_tick, tmo_tick);
>  		return ODP_TIMER_TMO_INVALID;
>  	}
>  
> -	tick = (cur_tick + tick) % MAX_TICKS;
> +	tick = tmo_tick % MAX_TICKS;
>  
>  	tmo_buf = odp_buffer_alloc(timer->pool);
>  	if (tmo_buf == ODP_BUFFER_INVALID) {
> -		ODP_DBG("alloc failed\n");
> +		ODP_DBG("tmo buffer alloc failed\n");
>  		return ODP_TIMER_TMO_INVALID;
>  	}
>  
> -- 
> 2.1.0
> 
> 
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
Mike Holmes Sept. 2, 2014, 11:38 a.m. UTC | #2 
On 2 September 2014 03:34, Anders Roxell <anders.roxell@linaro.org> wrote:

> On 2014-08-28 17:15, Petri Savolainen wrote:
> > Use resolution and min/max tmo values from timer create call.
> >
> > Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>
> > ---
> >  platform/linux-generic/odp_timer.c | 68
> ++++++++++++++++++++++++++++----------
> >  1 file changed, 51 insertions(+), 17 deletions(-)
> >
> > diff --git a/platform/linux-generic/odp_timer.c
> b/platform/linux-generic/odp_timer.c
> > index 1bf37f9..07b1029 100644
> > --- a/platform/linux-generic/odp_timer.c
> > +++ b/platform/linux-generic/odp_timer.c
> > @@ -18,9 +18,16 @@
> >
> >  #include <string.h>
> >
> > +
> > +/* Nanoseconds */
> > +#define USEC          1000UL
> > +#define MSEC          1000000UL
> > +#define SEC           1000000000UL
>
> Should this be moved to a common odp_header file?
> This is defined in example/timer/odp_timer_test.c as well...
> This got introduced in commit c51c0cbbdaa0f4668c966a9f50c7866d6ac489ce
>

The ones in odp_timer_test.c are generating doxygen warnings.
https://ci.linaro.org/view/odp-ci/job/odp-api-doxygen-generation/379/warnings12Result/NORMAL/


>
> Cheers,
> Anders
>
> > +
> >  #define NUM_TIMERS    1
> >  #define MAX_TICKS     1024
> > -#define RESOLUTION_NS 1000000
> > +#define MAX_RES       SEC
> > +#define MIN_RES       (100*USEC)
> >
> >
> >  typedef struct {
> > @@ -112,13 +119,13 @@ static int find_and_del_tmo(timeout_t **tmo,
> odp_timer_tmo_t handle)
> >  int odp_timer_cancel_tmo(odp_timer_t timer_hdl, odp_timer_tmo_t tmo)
> >  {
> >       int id;
> > -     uint64_t tick_idx;
> > +     int tick_idx;
> >       timeout_t *cancel_tmo;
> >       odp_timeout_hdr_t *tmo_hdr;
> >       tick_t *tick;
> >
> >       /* get id */
> > -     id = timer_hdl - 1;
> > +     id = (int)timer_hdl - 1;
> >
> >       tmo_hdr = odp_timeout_hdr((odp_timeout_t) tmo);
> >       /* get tmo_buf to cancel */
> > @@ -179,6 +186,7 @@ static void timer_start(timer_ring_t *timer)
> >  {
> >       struct sigevent   sigev;
> >       struct itimerspec ispec;
> > +     uint64_t res, sec, nsec;
> >
> >       ODP_DBG("\nTimer (%u) starts\n", timer->timer_hdl);
> >
> > @@ -194,10 +202,14 @@ static void timer_start(timer_ring_t *timer)
> >               return;
> >       }
> >
> > -     ispec.it_interval.tv_sec  = 0;
> > -     ispec.it_interval.tv_nsec = RESOLUTION_NS;
> > -     ispec.it_value.tv_sec     = 0;
> > -     ispec.it_value.tv_nsec    = RESOLUTION_NS;
> > +     res  = timer->resolution_ns;
> > +     sec  = res / SEC;
> > +     nsec = res - sec*SEC;
> > +
> > +     ispec.it_interval.tv_sec  = (time_t)sec;
> > +     ispec.it_interval.tv_nsec = (long)nsec;
> > +     ispec.it_value.tv_sec     = (time_t)sec;
> > +     ispec.it_value.tv_nsec    = (long)nsec;
> >
> >       if (timer_settime(timer->timerid, 0, &ispec, NULL)) {
> >               ODP_DBG("Timer set failed\n");
> > @@ -250,19 +262,41 @@ int odp_timer_disarm_all(void)
> >  }
> >
> >  odp_timer_t odp_timer_create(const char *name, odp_buffer_pool_t pool,
> > -                          uint64_t resolution, uint64_t min_tmo,
> > -                          uint64_t max_tmo)
> > +                          uint64_t resolution_ns, uint64_t min_ns,
> > +                          uint64_t max_ns)
> >  {
> >       uint32_t id;
> >       timer_ring_t *timer;
> >       odp_timer_t timer_hdl;
> >       int i;
> > -     (void) name; (void) resolution; (void) min_tmo; (void) max_tmo;
> > +     uint64_t max_ticks;
> > +     (void) name;
> > +
> > +     if (resolution_ns < MIN_RES)
> > +             resolution_ns = MIN_RES;
> > +
> > +     if (resolution_ns > MAX_RES)
> > +             resolution_ns = MAX_RES;
> > +
> > +     max_ticks = max_ns / resolution_ns;
> > +
> > +     if (max_ticks > MAX_TICKS) {
> > +             ODP_DBG("Maximum timeout too long: %"PRIu64" ticks\n",
> > +                     max_ticks);
> > +             return ODP_TIMER_INVALID;
> > +     }
> > +
> > +     if (min_ns < resolution_ns) {
> > +             ODP_DBG("Min timeout %"PRIu64" ns < resolution %"PRIu64"
> ns\n",
> > +                     min_ns, resolution_ns);
> > +             return ODP_TIMER_INVALID;
> > +     }
> >
> >       odp_spinlock_lock(&odp_timer.lock);
> >
> >       if (odp_timer.num_timers >= NUM_TIMERS) {
> >               odp_spinlock_unlock(&odp_timer.lock);
> > +             ODP_DBG("All timers allocated\n");
> >               return ODP_TIMER_INVALID;
> >       }
> >
> > @@ -281,7 +315,7 @@ odp_timer_t odp_timer_create(const char *name,
> odp_buffer_pool_t pool,
> >
> >       timer->timer_hdl     = timer_hdl;
> >       timer->pool          = pool;
> > -     timer->resolution_ns = RESOLUTION_NS;
> > +     timer->resolution_ns = resolution_ns;
> >       timer->max_ticks     = MAX_TICKS;
> >
> >       for (i = 0; i < MAX_TICKS; i++) {
> > @@ -308,7 +342,7 @@ odp_timer_tmo_t odp_timer_absolute_tmo(odp_timer_t
> timer_hdl, uint64_t tmo_tick,
> >       odp_timeout_hdr_t *tmo_hdr;
> >       timer_ring_t *timer;
> >
> > -     id = timer_hdl - 1;
> > +     id = (int)timer_hdl - 1;
> >       timer = &odp_timer.timer[id];
> >
> >       cur_tick = timer->cur_tick;
> > @@ -317,17 +351,17 @@ odp_timer_tmo_t odp_timer_absolute_tmo(odp_timer_t
> timer_hdl, uint64_t tmo_tick,
> >               return ODP_TIMER_TMO_INVALID;
> >       }
> >
> > -     tick = tmo_tick - cur_tick;
> > -     if (tick > MAX_TICKS) {
> > -             ODP_DBG("timeout too far\n");
> > +     if ((tmo_tick - cur_tick) > MAX_TICKS) {
> > +             ODP_DBG("timeout too far: cur %"PRIu64" tmo %"PRIu64"\n",
> > +                     cur_tick, tmo_tick);
> >               return ODP_TIMER_TMO_INVALID;
> >       }
> >
> > -     tick = (cur_tick + tick) % MAX_TICKS;
> > +     tick = tmo_tick % MAX_TICKS;
> >
> >       tmo_buf = odp_buffer_alloc(timer->pool);
> >       if (tmo_buf == ODP_BUFFER_INVALID) {
> > -             ODP_DBG("alloc failed\n");
> > +             ODP_DBG("tmo buffer alloc failed\n");
> >               return ODP_TIMER_TMO_INVALID;
> >       }
> >
> > --
> > 2.1.0
> >
> >
> > _______________________________________________
> > lng-odp mailing list
> > lng-odp@lists.linaro.org
> > http://lists.linaro.org/mailman/listinfo/lng-odp
>
> --
> Anders Roxell
> anders.roxell@linaro.org
> M: +46 709 71 42 85 | IRC: roxell
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
diff mbox

Patch

diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-generic/odp_timer.c
index 1bf37f9..07b1029 100644
--- a/platform/linux-generic/odp_timer.c
+++ b/platform/linux-generic/odp_timer.c
@@ -18,9 +18,16 @@ 
 
 #include <string.h>
 
+
+/* Nanoseconds */
+#define USEC          1000UL
+#define MSEC          1000000UL
+#define SEC           1000000000UL
+
 #define NUM_TIMERS    1
 #define MAX_TICKS     1024
-#define RESOLUTION_NS 1000000
+#define MAX_RES       SEC
+#define MIN_RES       (100*USEC)
 
 
 typedef struct {
@@ -112,13 +119,13 @@  static int find_and_del_tmo(timeout_t **tmo, odp_timer_tmo_t handle)
 int odp_timer_cancel_tmo(odp_timer_t timer_hdl, odp_timer_tmo_t tmo)
 {
 	int id;
-	uint64_t tick_idx;
+	int tick_idx;
 	timeout_t *cancel_tmo;
 	odp_timeout_hdr_t *tmo_hdr;
 	tick_t *tick;
 
 	/* get id */
-	id = timer_hdl - 1;
+	id = (int)timer_hdl - 1;
 
 	tmo_hdr = odp_timeout_hdr((odp_timeout_t) tmo);
 	/* get tmo_buf to cancel */
@@ -179,6 +186,7 @@  static void timer_start(timer_ring_t *timer)
 {
 	struct sigevent   sigev;
 	struct itimerspec ispec;
+	uint64_t res, sec, nsec;
 
 	ODP_DBG("\nTimer (%u) starts\n", timer->timer_hdl);
 
@@ -194,10 +202,14 @@  static void timer_start(timer_ring_t *timer)
 		return;
 	}
 
-	ispec.it_interval.tv_sec  = 0;
-	ispec.it_interval.tv_nsec = RESOLUTION_NS;
-	ispec.it_value.tv_sec     = 0;
-	ispec.it_value.tv_nsec    = RESOLUTION_NS;
+	res  = timer->resolution_ns;
+	sec  = res / SEC;
+	nsec = res - sec*SEC;
+
+	ispec.it_interval.tv_sec  = (time_t)sec;
+	ispec.it_interval.tv_nsec = (long)nsec;
+	ispec.it_value.tv_sec     = (time_t)sec;
+	ispec.it_value.tv_nsec    = (long)nsec;
 
 	if (timer_settime(timer->timerid, 0, &ispec, NULL)) {
 		ODP_DBG("Timer set failed\n");
@@ -250,19 +262,41 @@  int odp_timer_disarm_all(void)
 }
 
 odp_timer_t odp_timer_create(const char *name, odp_buffer_pool_t pool,
-			     uint64_t resolution, uint64_t min_tmo,
-			     uint64_t max_tmo)
+			     uint64_t resolution_ns, uint64_t min_ns,
+			     uint64_t max_ns)
 {
 	uint32_t id;
 	timer_ring_t *timer;
 	odp_timer_t timer_hdl;
 	int i;
-	(void) name; (void) resolution; (void) min_tmo; (void) max_tmo;
+	uint64_t max_ticks;
+	(void) name;
+
+	if (resolution_ns < MIN_RES)
+		resolution_ns = MIN_RES;
+
+	if (resolution_ns > MAX_RES)
+		resolution_ns = MAX_RES;
+
+	max_ticks = max_ns / resolution_ns;
+
+	if (max_ticks > MAX_TICKS) {
+		ODP_DBG("Maximum timeout too long: %"PRIu64" ticks\n",
+			max_ticks);
+		return ODP_TIMER_INVALID;
+	}
+
+	if (min_ns < resolution_ns) {
+		ODP_DBG("Min timeout %"PRIu64" ns < resolution %"PRIu64" ns\n",
+			min_ns, resolution_ns);
+		return ODP_TIMER_INVALID;
+	}
 
 	odp_spinlock_lock(&odp_timer.lock);
 
 	if (odp_timer.num_timers >= NUM_TIMERS) {
 		odp_spinlock_unlock(&odp_timer.lock);
+		ODP_DBG("All timers allocated\n");
 		return ODP_TIMER_INVALID;
 	}
 
@@ -281,7 +315,7 @@  odp_timer_t odp_timer_create(const char *name, odp_buffer_pool_t pool,
 
 	timer->timer_hdl     = timer_hdl;
 	timer->pool          = pool;
-	timer->resolution_ns = RESOLUTION_NS;
+	timer->resolution_ns = resolution_ns;
 	timer->max_ticks     = MAX_TICKS;
 
 	for (i = 0; i < MAX_TICKS; i++) {
@@ -308,7 +342,7 @@  odp_timer_tmo_t odp_timer_absolute_tmo(odp_timer_t timer_hdl, uint64_t tmo_tick,
 	odp_timeout_hdr_t *tmo_hdr;
 	timer_ring_t *timer;
 
-	id = timer_hdl - 1;
+	id = (int)timer_hdl - 1;
 	timer = &odp_timer.timer[id];
 
 	cur_tick = timer->cur_tick;
@@ -317,17 +351,17 @@  odp_timer_tmo_t odp_timer_absolute_tmo(odp_timer_t timer_hdl, uint64_t tmo_tick,
 		return ODP_TIMER_TMO_INVALID;
 	}
 
-	tick = tmo_tick - cur_tick;
-	if (tick > MAX_TICKS) {
-		ODP_DBG("timeout too far\n");
+	if ((tmo_tick - cur_tick) > MAX_TICKS) {
+		ODP_DBG("timeout too far: cur %"PRIu64" tmo %"PRIu64"\n",
+			cur_tick, tmo_tick);
 		return ODP_TIMER_TMO_INVALID;
 	}
 
-	tick = (cur_tick + tick) % MAX_TICKS;
+	tick = tmo_tick % MAX_TICKS;
 
 	tmo_buf = odp_buffer_alloc(timer->pool);
 	if (tmo_buf == ODP_BUFFER_INVALID) {
-		ODP_DBG("alloc failed\n");
+		ODP_DBG("tmo buffer alloc failed\n");
 		return ODP_TIMER_TMO_INVALID;
 	}