diff mbox

modetest: Use threads for cursors instead of SIGALRM

Message ID 1416634632-19319-1-git-send-email-jstpierre@mecheye.net (mailing list archive)
State New, archived
Headers show

Commit Message

Jasper St. Pierre Nov. 22, 2014, 5:37 a.m. UTC 
This fixes an issue when trying to use -v and -C together. When trying
to read the page flip event, we are interrupted by the SIGALRM that
comes in, and so we think we timed out when we simply got EINTR. While
we could just loop checking for EINTR, SIGALRM is just bad idea to
begin with, so just rewrite it to use a thread.
---
 tests/modetest/Makefile.am |  3 ++-
 tests/modetest/cursor.c    | 57 +++++++++++++++++++++++-----------------------
 2 files changed, 31 insertions(+), 29 deletions(-)

Comments

Rob Clark Nov. 22, 2014, 4:47 p.m. UTC | #1 
On Sat, Nov 22, 2014 at 12:37 AM, Jasper St. Pierre
<jstpierre@mecheye.net> wrote:
> This fixes an issue when trying to use -v and -C together. When trying
> to read the page flip event, we are interrupted by the SIGALRM that
> comes in, and so we think we timed out when we simply got EINTR. While
> we could just loop checking for EINTR, SIGALRM is just bad idea to
> begin with, so just rewrite it to use a thread.

I did eventually remember why I used a signal handler.. to emulate a
bit how input is handled on a signal handler in xorg.  But really,
that is kinda pointless, since the kernel doesn't really care.  This
is more sane.  Pushed, thanks

BR,
-R

> ---
>  tests/modetest/Makefile.am |  3 ++-
>  tests/modetest/cursor.c    | 57 +++++++++++++++++++++++-----------------------
>  2 files changed, 31 insertions(+), 29 deletions(-)
>
> diff --git a/tests/modetest/Makefile.am b/tests/modetest/Makefile.am
> index 0a6af01..8fc924a 100644
> --- a/tests/modetest/Makefile.am
> +++ b/tests/modetest/Makefile.am
> @@ -19,7 +19,8 @@ modetest_SOURCES = $(MODETEST_FILES)
>
>  modetest_LDADD = \
>         $(top_builddir)/libdrm.la \
> -       $(top_builddir)/libkms/libkms.la
> +       $(top_builddir)/libkms/libkms.la \
> +       -lpthread
>
>  if HAVE_CAIRO
>  AM_CFLAGS += $(CAIRO_CFLAGS)
> diff --git a/tests/modetest/cursor.c b/tests/modetest/cursor.c
> index 60f240a..62a50ef 100644
> --- a/tests/modetest/cursor.c
> +++ b/tests/modetest/cursor.c
> @@ -34,6 +34,8 @@
>  #include <string.h>
>  #include <signal.h>
>  #include <sys/time.h>
> +#include <pthread.h>
> +#include <unistd.h>
>
>  #include "xf86drm.h"
>  #include "xf86drmMode.h"
> @@ -59,6 +61,9 @@ struct cursor {
>  static struct cursor cursors[MAX_CURSORS];
>  static int ncursors;
>
> +static pthread_t cursor_thread;
> +static int cursor_running;
> +
>  /*
>   * Timer driven program loops through these steps to move/enable/disable
>   * the cursor
> @@ -137,33 +142,29 @@ static struct cursor_step steps[] = {
>                 {  set_cursor, 10,   0,  0 },  /* disable */
>  };
>
> -/*
> - * Cursor API:
> - */
> -
> -static void run_step(int sig)
> +static void *cursor_thread_func(void *data)
>  {
> -       struct cursor_step *step = &steps[indx % ARRAY_SIZE(steps)];
> -       struct itimerval itimer = {
> -                       .it_value.tv_usec = 1000 * step->msec,
> -       };
> -       int i;
> -
> -       for (i = 0; i < ncursors; i++) {
> -               struct cursor *cursor = &cursors[i];
> -               step->run(cursor, step);
> -       }
> -
> -       /* iterate to next count/step: */
> -       if (count < step->repeat) {
> -               count++;
> -       } else {
> -               count = 0;
> -               indx++;
> +       while (cursor_running) {
> +               struct cursor_step *step = &steps[indx % ARRAY_SIZE(steps)];
> +               int i;
> +
> +               for (i = 0; i < ncursors; i++) {
> +                       struct cursor *cursor = &cursors[i];
> +                       step->run(cursor, step);
> +               }
> +
> +               /* iterate to next count/step: */
> +               if (count < step->repeat) {
> +                       count++;
> +               } else {
> +                       count = 0;
> +                       indx++;
> +               }
> +
> +               usleep(1000 * step->msec);
>         }
>
> -       /* and lastly, setup timer for next step */
> -       setitimer(ITIMER_REAL, &itimer, NULL);
> +       return NULL;
>  }
>
>  int cursor_init(int fd, uint32_t bo_handle, uint32_t crtc_id,
> @@ -194,16 +195,16 @@ int cursor_init(int fd, uint32_t bo_handle, uint32_t crtc_id,
>
>  int cursor_start(void)
>  {
> -       /* setup signal handler to update cursor: */
> -       signal(SIGALRM, run_step);
> +       cursor_running = 1;
> +       pthread_create(&cursor_thread, NULL, cursor_thread_func, NULL);
>         printf("starting cursor\n");
> -       run_step(SIGALRM);
>         return 0;
>  }
>
>  int cursor_stop(void)
>  {
> -       signal(SIGALRM, NULL);
> +       cursor_running = 0;
> +       pthread_join(cursor_thread, NULL);
>         printf("cursor stopped\n");
>         return 0;
>  }
> --
> 2.1.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

diff --git a/tests/modetest/Makefile.am b/tests/modetest/Makefile.am
index 0a6af01..8fc924a 100644
--- a/tests/modetest/Makefile.am
+++ b/tests/modetest/Makefile.am
@@ -19,7 +19,8 @@  modetest_SOURCES = $(MODETEST_FILES)
 
 modetest_LDADD = \
 	$(top_builddir)/libdrm.la \
-	$(top_builddir)/libkms/libkms.la
+	$(top_builddir)/libkms/libkms.la \
+	-lpthread
 
 if HAVE_CAIRO
 AM_CFLAGS += $(CAIRO_CFLAGS)
diff --git a/tests/modetest/cursor.c b/tests/modetest/cursor.c
index 60f240a..62a50ef 100644
--- a/tests/modetest/cursor.c
+++ b/tests/modetest/cursor.c
@@ -34,6 +34,8 @@ 
 #include <string.h>
 #include <signal.h>
 #include <sys/time.h>
+#include <pthread.h>
+#include <unistd.h>
 
 #include "xf86drm.h"
 #include "xf86drmMode.h"
@@ -59,6 +61,9 @@  struct cursor {
 static struct cursor cursors[MAX_CURSORS];
 static int ncursors;
 
+static pthread_t cursor_thread;
+static int cursor_running;
+
 /*
  * Timer driven program loops through these steps to move/enable/disable
  * the cursor
@@ -137,33 +142,29 @@  static struct cursor_step steps[] = {
 		{  set_cursor, 10,   0,  0 },  /* disable */
 };
 
-/*
- * Cursor API:
- */
-
-static void run_step(int sig)
+static void *cursor_thread_func(void *data)
 {
-	struct cursor_step *step = &steps[indx % ARRAY_SIZE(steps)];
-	struct itimerval itimer = {
-			.it_value.tv_usec = 1000 * step->msec,
-	};
-	int i;
-
-	for (i = 0; i < ncursors; i++) {
-		struct cursor *cursor = &cursors[i];
-		step->run(cursor, step);
-	}
-
-	/* iterate to next count/step: */
-	if (count < step->repeat) {
-		count++;
-	} else {
-		count = 0;
-		indx++;
+	while (cursor_running) {
+		struct cursor_step *step = &steps[indx % ARRAY_SIZE(steps)];
+		int i;
+
+		for (i = 0; i < ncursors; i++) {
+			struct cursor *cursor = &cursors[i];
+			step->run(cursor, step);
+		}
+
+		/* iterate to next count/step: */
+		if (count < step->repeat) {
+			count++;
+		} else {
+			count = 0;
+			indx++;
+		}
+
+		usleep(1000 * step->msec);
 	}
 
-	/* and lastly, setup timer for next step */
-	setitimer(ITIMER_REAL, &itimer, NULL);
+	return NULL;
 }
 
 int cursor_init(int fd, uint32_t bo_handle, uint32_t crtc_id,
@@ -194,16 +195,16 @@  int cursor_init(int fd, uint32_t bo_handle, uint32_t crtc_id,
 
 int cursor_start(void)
 {
-	/* setup signal handler to update cursor: */
-	signal(SIGALRM, run_step);
+	cursor_running = 1;
+	pthread_create(&cursor_thread, NULL, cursor_thread_func, NULL);
 	printf("starting cursor\n");
-	run_step(SIGALRM);
 	return 0;
 }
 
 int cursor_stop(void)
 {
-	signal(SIGALRM, NULL);
+	cursor_running = 0;
+	pthread_join(cursor_thread, NULL);
 	printf("cursor stopped\n");
 	return 0;
 }