diff mbox

[3/3] modetest: add atomic page flip support

Message ID 1440570108-17354-3-git-send-email-human.hwang@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hyungwon Hwang Aug. 26, 2015, 6:21 a.m. UTC
This patch adds support for atomic page flip. User can specify -V option
with the plane id for testing atomic page flipping.
---
 tests/modetest/modetest.c | 153 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 149 insertions(+), 4 deletions(-)

Comments

Emil Velikov Sept. 2, 2015, 12:43 a.m. UTC | #1
On 26 August 2015 at 07:21, Hyungwon Hwang <human.hwang@samsung.com> wrote:
> This patch adds support for atomic page flip. User can specify -V option
> with the plane id for testing atomic page flipping.
> ---
>  tests/modetest/modetest.c | 153 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 149 insertions(+), 4 deletions(-)
>
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index 753a559..9bffa98 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
> @@ -719,6 +719,10 @@ struct pipe_arg {
>         struct timeval start;
>
>         int swap_count;
> +
> +       /* for atomic modeset */
> +       uint32_t plane_id;
> +       uint32_t fb_obj_id;
>  };
>
>  struct plane_arg {
> @@ -1444,7 +1448,7 @@ static int parse_property(struct property_arg *p, const char *arg)
>
>  static void usage(char *name)
>  {
> -       fprintf(stderr, "usage: %s [-acDdefMPpsCvw]\n", name);
> +       fprintf(stderr, "usage: %s [-acDdefMPpsCvVw]\n", name);
>         fprintf(stderr, "\tA: supported in atomic modeset\n");
>         fprintf(stderr, "\tL: supported in legacy modeset\n");
>
> @@ -1459,6 +1463,7 @@ static void usage(char *name)
>
>         fprintf(stderr, "\n Atomic Test options: [A]\n\n");
>         fprintf(stderr, "\t-a\tuse atomic modeset\n");
> +       fprintf(stderr, "\t-V <flipping_plane_id>\ttest vsynced page flipping\n");
>
>         fprintf(stderr, "\n Legacy test options: [L]\n\n");
>         fprintf(stderr, "\t-P <crtc_id>:<w>x<h>[+<x>+<y>][*<scale>][@<format>]\tset a plane\n");
> @@ -1627,7 +1632,138 @@ static void atomic_modeset(struct device *dev, drmModeAtomicReqPtr req,
>         drmModeAtomicCommit(dev->fd, req, flags, NULL);
>  }
>
> -static char optstr[] = "acdD:efM:P:ps:Cvw:";
> +static void
> +atomic_page_flip_handler(int fd, unsigned int frame,
> +               unsigned int sec, unsigned int usec, void *data)
> +{
> +       static drmModeAtomicReqPtr req = NULL;
> +       unsigned int new_fb_id;
> +       struct timeval end;
> +       struct pipe_arg *pipe;
> +       double t;
> +       uint32_t flags = 0;
> +
> +       pipe = (struct pipe_arg *)(unsigned long)data;
> +
> +       if (pipe->current_fb_id == pipe->fb_id[0])
> +               new_fb_id = pipe->fb_id[1];
> +       else
> +               new_fb_id = pipe->fb_id[0];
> +
> +       pipe->current_fb_id = new_fb_id;
> +       pipe->swap_count++;
> +
> +       req = drmModeAtomicAlloc();
> +
> +       drmModeAtomicAddProperty(req, pipe->plane_id, pipe->fb_obj_id, new_fb_id);
> +
We'll crash badly if req is NULL here. I guess we can smoke test the
API, but that is no excuse for the missing null check.

> +       flags = DRM_MODE_PAGE_FLIP_EVENT;
> +       drmModeAtomicCommit(fd, req, flags, pipe);
> +
> +       if (pipe->swap_count == 60) {
> +               gettimeofday(&end, NULL);
> +               t = end.tv_sec + end.tv_usec * 1e-6 -
> +                       (pipe->start.tv_sec + pipe->start.tv_usec * 1e-6);
> +               fprintf(stderr, "freq: %.02fHz\n", pipe->swap_count / t);
> +               pipe->swap_count = 0;
> +               pipe->start = end;
> +       }
> +
> +       drmModeAtomicFree(req);
> +}
> +
> +static int atomic_test_page_flip(struct device *dev, drmModeAtomicReqPtr req,
> +                       struct resources *res, struct property_arg *prop_args,
> +                       unsigned int prop_count,unsigned int plane_id)
> +{
> +       struct bo *other_bo;
> +       unsigned int other_fb_id;
> +       struct pipe_arg *pipe = NULL;
> +       drmEventContext evctx;
> +       uint32_t flags = 0, fb_obj_id = 0, pixel_format;
> +       uint64_t width, height;
> +       int ret;
> +
> +       if (!is_obj_id_in_prop_args(prop_args, prop_count, plane_id))
> +               return -1;
> +
> +       fb_obj_id = get_atomic_plane_prop_id(res, plane_id, "FB_ID");
> +       if (!fb_obj_id)
> +               return -1;
> +
> +       width = get_value_in_prop_args(prop_args, prop_count, plane_id,
> +                       "SRC_W");
> +       height = get_value_in_prop_args(prop_args, prop_count, plane_id,
> +                       "SRC_H");
Here and in the previous patch you assume that always succeeds.
Perhaps one should check it otherwise all sort of crazy stuff will
happen in allocate_fb() ?

> +       pixel_format = DRM_FORMAT_XRGB8888;
> +
For the future we might consider making this user configurable,
although as is it looks like a good default.

> +       ret = allocate_fb(dev, req, res, width, height, pixel_format,
> +                       PATTERN_TILES, &other_bo, &other_fb_id);
> +       if (ret < 0)
> +               return ret;
> +
> +       ret = drmModeAtomicAddProperty(req, plane_id, fb_obj_id,
> +                       other_fb_id);
> +       if (ret < 0)
Why do you return 'success' here ?

> +               goto err;
> +
> +       if (!fb_obj_id) {
Here (and in previous patch) we're missing RmFB()
Leaking other_bo.

> +               fprintf(stderr, "page flipping requires at least one plane setting.\n");
> +               return -1;
> +       }
> +
> +       pipe = drmMalloc(sizeof *pipe);
Please use calloc.

> +
> +       gettimeofday(&pipe->start, NULL);
> +       pipe->swap_count = 0;
> +       pipe->plane_id = plane_id;
> +       pipe->fb_obj_id = fb_obj_id;
> +       pipe->fb_id[0] = dev->mode.fb_id;
> +       pipe->fb_id[1] = other_fb_id;
> +       pipe->current_fb_id = other_fb_id;
> +
> +       flags = DRM_MODE_PAGE_FLIP_EVENT;
> +
> +       ret = drmModeAtomicCommit(dev->fd, req, flags, pipe);
> +       if (ret < 0)
Should you return the error code rather than 0 ?

> +               goto err_rmfb;
> +
> +       memset(&evctx, 0, sizeof evctx);
> +       evctx.version = DRM_EVENT_CONTEXT_VERSION;
This is very bad, please don't do it.

DRM_EVENT_CONTEXT_VERSION defines the version currently present not
the one you implement/use. They might match now, but as one bumps the
define modetest build against it will end up very badly.

Seems issue is present in the 'legacy' modeset path and vbltest :'(

> +       evctx.vblank_handler = NULL;
> +       evctx.page_flip_handler = atomic_page_flip_handler;
> +
> +       while (1) {
> +               struct timeval timeout = { .tv_sec = 3, .tv_usec = 0 };
> +               fd_set fds;
> +               int ret;
> +
> +               FD_ZERO(&fds);
> +               FD_SET(0, &fds);
> +               FD_SET(dev->fd, &fds);
> +               ret = select(dev->fd + 1, &fds, NULL, NULL, &timeout);
> +
> +               if (ret <= 0) {
> +                       fprintf(stderr, "select timed out or error (ret %d)\n",
> +                                       ret);
> +                       continue;
> +               } else if (FD_ISSET(0, &fds)) {
> +                       break;
> +               }
> +
> +               drmHandleEvent(dev->fd, &evctx);
> +       }
> +
> +err_rmfb:
> +       drmFree(pipe);
> +       drmModeRmFB(dev->fd, other_fb_id);
> +err:
> +       bo_destroy(other_bo);
> +
> +       return 0;
> +}
> +
> +static char optstr[] = "acdD:efM:P:ps:CvV:w:";
>
>  int main(int argc, char **argv)
>  {
> @@ -1641,7 +1777,7 @@ int main(int argc, char **argv)
>         const char *modules[] = { "i915", "radeon", "nouveau", "vmwgfx", "omapdrm", "exynos", "tilcdc", "msm", "sti", "tegra", "imx-drm", "rockchip", "atmel-hlcdc" };
>         char *device = NULL;
>         char *module = NULL;
> -       unsigned int i;
> +       unsigned int i, flip_plane_id = 0;
>         int count = 0, plane_count = 0;
>         unsigned int prop_count = 0;
>         struct pipe_arg *pipe_args = NULL;
> @@ -1723,6 +1859,11 @@ int main(int argc, char **argv)
>                 case 'v':
>                         test_vsync = 1;
>                         break;
> +               case 'V':
> +                       if (sscanf(optarg, "%u", &flip_plane_id) != 1)
> +                               usage(argv[0]);
> +                       test_vsync = 1;
> +                       break;
>                 case 'w':
>                         prop_args = realloc(prop_args,
>                                            (prop_count + 1) * sizeof *prop_args);
> @@ -1775,7 +1916,7 @@ int main(int argc, char **argv)
>                 return -1;
>         }
>
> -       if (test_vsync && !count) {
> +       if (test_vsync && (!count && !req)) {
>                 fprintf(stderr, "page flipping requires at least one -s option.\n");
Something doesn't match here - the top level (help) description, the
conditional and/or the error printf.

Cheers,
Emil
Hyungwon Hwang Sept. 23, 2015, 2:40 a.m. UTC | #2
Dear Emil,

On Wed, 02 Sep 2015 01:43:41 +0100
Emil Velikov <emil.l.velikov@gmail.com> wrote:

> On 26 August 2015 at 07:21, Hyungwon Hwang <human.hwang@samsung.com>
> wrote:
> > This patch adds support for atomic page flip. User can specify -V
> > option with the plane id for testing atomic page flipping.
> > ---
> >  tests/modetest/modetest.c | 153
> > ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 149
> > insertions(+), 4 deletions(-)
> >
> > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> > index 753a559..9bffa98 100644
> > --- a/tests/modetest/modetest.c
> > +++ b/tests/modetest/modetest.c
> > @@ -719,6 +719,10 @@ struct pipe_arg {
> >         struct timeval start;
> >
> >         int swap_count;
> > +
> > +       /* for atomic modeset */
> > +       uint32_t plane_id;
> > +       uint32_t fb_obj_id;
> >  };
> >
> >  struct plane_arg {
> > @@ -1444,7 +1448,7 @@ static int parse_property(struct property_arg
> > *p, const char *arg)
> >
> >  static void usage(char *name)
> >  {
> > -       fprintf(stderr, "usage: %s [-acDdefMPpsCvw]\n", name);
> > +       fprintf(stderr, "usage: %s [-acDdefMPpsCvVw]\n", name);
> >         fprintf(stderr, "\tA: supported in atomic modeset\n");
> >         fprintf(stderr, "\tL: supported in legacy modeset\n");
> >
> > @@ -1459,6 +1463,7 @@ static void usage(char *name)
> >
> >         fprintf(stderr, "\n Atomic Test options: [A]\n\n");
> >         fprintf(stderr, "\t-a\tuse atomic modeset\n");
> > +       fprintf(stderr, "\t-V <flipping_plane_id>\ttest vsynced
> > page flipping\n");
> >
> >         fprintf(stderr, "\n Legacy test options: [L]\n\n");
> >         fprintf(stderr, "\t-P
> > <crtc_id>:<w>x<h>[+<x>+<y>][*<scale>][@<format>]\tset a plane\n");
> > @@ -1627,7 +1632,138 @@ static void atomic_modeset(struct device
> > *dev, drmModeAtomicReqPtr req, drmModeAtomicCommit(dev->fd, req,
> > flags, NULL); }
> >
> > -static char optstr[] = "acdD:efM:P:ps:Cvw:";
> > +static void
> > +atomic_page_flip_handler(int fd, unsigned int frame,
> > +               unsigned int sec, unsigned int usec, void *data)
> > +{
> > +       static drmModeAtomicReqPtr req = NULL;
> > +       unsigned int new_fb_id;
> > +       struct timeval end;
> > +       struct pipe_arg *pipe;
> > +       double t;
> > +       uint32_t flags = 0;
> > +
> > +       pipe = (struct pipe_arg *)(unsigned long)data;
> > +
> > +       if (pipe->current_fb_id == pipe->fb_id[0])
> > +               new_fb_id = pipe->fb_id[1];
> > +       else
> > +               new_fb_id = pipe->fb_id[0];
> > +
> > +       pipe->current_fb_id = new_fb_id;
> > +       pipe->swap_count++;
> > +
> > +       req = drmModeAtomicAlloc();
> > +
> > +       drmModeAtomicAddProperty(req, pipe->plane_id,
> > pipe->fb_obj_id, new_fb_id); +
> We'll crash badly if req is NULL here. I guess we can smoke test the
> API, but that is no excuse for the missing null check.
> 
> > +       flags = DRM_MODE_PAGE_FLIP_EVENT;
> > +       drmModeAtomicCommit(fd, req, flags, pipe);
> > +
> > +       if (pipe->swap_count == 60) {
> > +               gettimeofday(&end, NULL);
> > +               t = end.tv_sec + end.tv_usec * 1e-6 -
> > +                       (pipe->start.tv_sec + pipe->start.tv_usec *
> > 1e-6);
> > +               fprintf(stderr, "freq: %.02fHz\n",
> > pipe->swap_count / t);
> > +               pipe->swap_count = 0;
> > +               pipe->start = end;
> > +       }
> > +
> > +       drmModeAtomicFree(req);
> > +}
> > +
> > +static int atomic_test_page_flip(struct device *dev,
> > drmModeAtomicReqPtr req,
> > +                       struct resources *res, struct property_arg
> > *prop_args,
> > +                       unsigned int prop_count,unsigned int
> > plane_id) +{
> > +       struct bo *other_bo;
> > +       unsigned int other_fb_id;
> > +       struct pipe_arg *pipe = NULL;
> > +       drmEventContext evctx;
> > +       uint32_t flags = 0, fb_obj_id = 0, pixel_format;
> > +       uint64_t width, height;
> > +       int ret;
> > +
> > +       if (!is_obj_id_in_prop_args(prop_args, prop_count,
> > plane_id))
> > +               return -1;
> > +
> > +       fb_obj_id = get_atomic_plane_prop_id(res, plane_id,
> > "FB_ID");
> > +       if (!fb_obj_id)
> > +               return -1;
> > +
> > +       width = get_value_in_prop_args(prop_args, prop_count,
> > plane_id,
> > +                       "SRC_W");
> > +       height = get_value_in_prop_args(prop_args, prop_count,
> > plane_id,
> > +                       "SRC_H");
> Here and in the previous patch you assume that always succeeds.
> Perhaps one should check it otherwise all sort of crazy stuff will
> happen in allocate_fb() ?
> 
> > +       pixel_format = DRM_FORMAT_XRGB8888;
> > +
> For the future we might consider making this user configurable,
> although as is it looks like a good default.
> 
> > +       ret = allocate_fb(dev, req, res, width, height,
> > pixel_format,
> > +                       PATTERN_TILES, &other_bo, &other_fb_id);
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       ret = drmModeAtomicAddProperty(req, plane_id, fb_obj_id,
> > +                       other_fb_id);
> > +       if (ret < 0)
> Why do you return 'success' here ?
> 
> > +               goto err;
> > +
> > +       if (!fb_obj_id) {
> Here (and in previous patch) we're missing RmFB()
> Leaking other_bo.
> 
> > +               fprintf(stderr, "page flipping requires at least
> > one plane setting.\n");
> > +               return -1;
> > +       }
> > +
> > +       pipe = drmMalloc(sizeof *pipe);
> Please use calloc.
> 
> > +
> > +       gettimeofday(&pipe->start, NULL);
> > +       pipe->swap_count = 0;
> > +       pipe->plane_id = plane_id;
> > +       pipe->fb_obj_id = fb_obj_id;
> > +       pipe->fb_id[0] = dev->mode.fb_id;
> > +       pipe->fb_id[1] = other_fb_id;
> > +       pipe->current_fb_id = other_fb_id;
> > +
> > +       flags = DRM_MODE_PAGE_FLIP_EVENT;
> > +
> > +       ret = drmModeAtomicCommit(dev->fd, req, flags, pipe);
> > +       if (ret < 0)
> Should you return the error code rather than 0 ?
> 
> > +               goto err_rmfb;
> > +
> > +       memset(&evctx, 0, sizeof evctx);
> > +       evctx.version = DRM_EVENT_CONTEXT_VERSION;
> This is very bad, please don't do it.
> 
> DRM_EVENT_CONTEXT_VERSION defines the version currently present not
> the one you implement/use. They might match now, but as one bumps the
> define modetest build against it will end up very badly.
> 
> Seems issue is present in the 'legacy' modeset path and vbltest :'(

Yes. It could be a problem in future. Then do you think new defines
must be set for the versions? or will it be OK just use a constant in
the code?

Thank you for your review. 

Best regards,
Hyungwon Hwang

> 
> > +       evctx.vblank_handler = NULL;
> > +       evctx.page_flip_handler = atomic_page_flip_handler;
> > +
> > +       while (1) {
> > +               struct timeval timeout = { .tv_sec = 3, .tv_usec =
> > 0 };
> > +               fd_set fds;
> > +               int ret;
> > +
> > +               FD_ZERO(&fds);
> > +               FD_SET(0, &fds);
> > +               FD_SET(dev->fd, &fds);
> > +               ret = select(dev->fd + 1, &fds, NULL, NULL,
> > &timeout); +
> > +               if (ret <= 0) {
> > +                       fprintf(stderr, "select timed out or error
> > (ret %d)\n",
> > +                                       ret);
> > +                       continue;
> > +               } else if (FD_ISSET(0, &fds)) {
> > +                       break;
> > +               }
> > +
> > +               drmHandleEvent(dev->fd, &evctx);
> > +       }
> > +
> > +err_rmfb:
> > +       drmFree(pipe);
> > +       drmModeRmFB(dev->fd, other_fb_id);
> > +err:
> > +       bo_destroy(other_bo);
> > +
> > +       return 0;
> > +}
> > +
> > +static char optstr[] = "acdD:efM:P:ps:CvV:w:";
> >
> >  int main(int argc, char **argv)
> >  {
> > @@ -1641,7 +1777,7 @@ int main(int argc, char **argv)
> >         const char *modules[] = { "i915", "radeon", "nouveau",
> > "vmwgfx", "omapdrm", "exynos", "tilcdc", "msm", "sti", "tegra",
> > "imx-drm", "rockchip", "atmel-hlcdc" }; char *device = NULL; char
> > *module = NULL;
> > -       unsigned int i;
> > +       unsigned int i, flip_plane_id = 0;
> >         int count = 0, plane_count = 0;
> >         unsigned int prop_count = 0;
> >         struct pipe_arg *pipe_args = NULL;
> > @@ -1723,6 +1859,11 @@ int main(int argc, char **argv)
> >                 case 'v':
> >                         test_vsync = 1;
> >                         break;
> > +               case 'V':
> > +                       if (sscanf(optarg, "%u", &flip_plane_id) !=
> > 1)
> > +                               usage(argv[0]);
> > +                       test_vsync = 1;
> > +                       break;
> >                 case 'w':
> >                         prop_args = realloc(prop_args,
> >                                            (prop_count + 1) *
> > sizeof *prop_args); @@ -1775,7 +1916,7 @@ int main(int argc, char
> > **argv) return -1;
> >         }
> >
> > -       if (test_vsync && !count) {
> > +       if (test_vsync && (!count && !req)) {
> >                 fprintf(stderr, "page flipping requires at least
> > one -s option.\n");
> Something doesn't match here - the top level (help) description, the
> conditional and/or the error printf.
> 
> Cheers,
> Emil
diff mbox

Patch

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 753a559..9bffa98 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -719,6 +719,10 @@  struct pipe_arg {
 	struct timeval start;
 
 	int swap_count;
+
+	/* for atomic modeset */
+	uint32_t plane_id;
+	uint32_t fb_obj_id;
 };
 
 struct plane_arg {
@@ -1444,7 +1448,7 @@  static int parse_property(struct property_arg *p, const char *arg)
 
 static void usage(char *name)
 {
-	fprintf(stderr, "usage: %s [-acDdefMPpsCvw]\n", name);
+	fprintf(stderr, "usage: %s [-acDdefMPpsCvVw]\n", name);
 	fprintf(stderr, "\tA: supported in atomic modeset\n");
 	fprintf(stderr, "\tL: supported in legacy modeset\n");
 
@@ -1459,6 +1463,7 @@  static void usage(char *name)
 
 	fprintf(stderr, "\n Atomic Test options: [A]\n\n");
 	fprintf(stderr, "\t-a\tuse atomic modeset\n");
+	fprintf(stderr, "\t-V <flipping_plane_id>\ttest vsynced page flipping\n");
 
 	fprintf(stderr, "\n Legacy test options: [L]\n\n");
 	fprintf(stderr, "\t-P <crtc_id>:<w>x<h>[+<x>+<y>][*<scale>][@<format>]\tset a plane\n");
@@ -1627,7 +1632,138 @@  static void atomic_modeset(struct device *dev, drmModeAtomicReqPtr req,
 	drmModeAtomicCommit(dev->fd, req, flags, NULL);
 }
 
-static char optstr[] = "acdD:efM:P:ps:Cvw:";
+static void
+atomic_page_flip_handler(int fd, unsigned int frame,
+		unsigned int sec, unsigned int usec, void *data)
+{
+	static drmModeAtomicReqPtr req = NULL;
+	unsigned int new_fb_id;
+	struct timeval end;
+	struct pipe_arg *pipe;
+	double t;
+	uint32_t flags = 0;
+
+	pipe = (struct pipe_arg *)(unsigned long)data;
+
+	if (pipe->current_fb_id == pipe->fb_id[0])
+		new_fb_id = pipe->fb_id[1];
+	else
+		new_fb_id = pipe->fb_id[0];
+
+	pipe->current_fb_id = new_fb_id;
+	pipe->swap_count++;
+
+	req = drmModeAtomicAlloc();
+
+	drmModeAtomicAddProperty(req, pipe->plane_id, pipe->fb_obj_id, new_fb_id);
+
+	flags = DRM_MODE_PAGE_FLIP_EVENT;
+	drmModeAtomicCommit(fd, req, flags, pipe);
+
+	if (pipe->swap_count == 60) {
+		gettimeofday(&end, NULL);
+		t = end.tv_sec + end.tv_usec * 1e-6 -
+			(pipe->start.tv_sec + pipe->start.tv_usec * 1e-6);
+		fprintf(stderr, "freq: %.02fHz\n", pipe->swap_count / t);
+		pipe->swap_count = 0;
+		pipe->start = end;
+	}
+
+	drmModeAtomicFree(req);
+}
+
+static int atomic_test_page_flip(struct device *dev, drmModeAtomicReqPtr req,
+			struct resources *res, struct property_arg *prop_args,
+			unsigned int prop_count,unsigned int plane_id)
+{
+	struct bo *other_bo;
+	unsigned int other_fb_id;
+	struct pipe_arg *pipe = NULL;
+	drmEventContext evctx;
+	uint32_t flags = 0, fb_obj_id = 0, pixel_format;
+	uint64_t width, height;
+	int ret;
+
+	if (!is_obj_id_in_prop_args(prop_args, prop_count, plane_id))
+		return -1;
+
+	fb_obj_id = get_atomic_plane_prop_id(res, plane_id, "FB_ID");
+	if (!fb_obj_id)
+		return -1;
+
+	width = get_value_in_prop_args(prop_args, prop_count, plane_id,
+			"SRC_W");
+	height = get_value_in_prop_args(prop_args, prop_count, plane_id,
+			"SRC_H");
+	pixel_format = DRM_FORMAT_XRGB8888;
+
+	ret = allocate_fb(dev, req, res, width, height, pixel_format,
+			PATTERN_TILES, &other_bo, &other_fb_id);
+	if (ret < 0)
+		return ret;
+
+	ret = drmModeAtomicAddProperty(req, plane_id, fb_obj_id,
+			other_fb_id);
+	if (ret < 0)
+		goto err;
+
+	if (!fb_obj_id) {
+		fprintf(stderr, "page flipping requires at least one plane setting.\n");
+		return -1;
+	}
+
+	pipe = drmMalloc(sizeof *pipe);
+
+	gettimeofday(&pipe->start, NULL);
+	pipe->swap_count = 0;
+	pipe->plane_id = plane_id;
+	pipe->fb_obj_id = fb_obj_id;
+	pipe->fb_id[0] = dev->mode.fb_id;
+	pipe->fb_id[1] = other_fb_id;
+	pipe->current_fb_id = other_fb_id;
+
+	flags = DRM_MODE_PAGE_FLIP_EVENT;
+
+	ret = drmModeAtomicCommit(dev->fd, req, flags, pipe);
+	if (ret < 0)
+		goto err_rmfb;
+
+	memset(&evctx, 0, sizeof evctx);
+	evctx.version = DRM_EVENT_CONTEXT_VERSION;
+	evctx.vblank_handler = NULL;
+	evctx.page_flip_handler = atomic_page_flip_handler;
+
+	while (1) {
+		struct timeval timeout = { .tv_sec = 3, .tv_usec = 0 };
+		fd_set fds;
+		int ret;
+
+		FD_ZERO(&fds);
+		FD_SET(0, &fds);
+		FD_SET(dev->fd, &fds);
+		ret = select(dev->fd + 1, &fds, NULL, NULL, &timeout);
+
+		if (ret <= 0) {
+			fprintf(stderr, "select timed out or error (ret %d)\n",
+					ret);
+			continue;
+		} else if (FD_ISSET(0, &fds)) {
+			break;
+		}
+
+		drmHandleEvent(dev->fd, &evctx);
+	}
+
+err_rmfb:
+	drmFree(pipe);
+	drmModeRmFB(dev->fd, other_fb_id);
+err:
+	bo_destroy(other_bo);
+
+	return 0;
+}
+
+static char optstr[] = "acdD:efM:P:ps:CvV:w:";
 
 int main(int argc, char **argv)
 {
@@ -1641,7 +1777,7 @@  int main(int argc, char **argv)
 	const char *modules[] = { "i915", "radeon", "nouveau", "vmwgfx", "omapdrm", "exynos", "tilcdc", "msm", "sti", "tegra", "imx-drm", "rockchip", "atmel-hlcdc" };
 	char *device = NULL;
 	char *module = NULL;
-	unsigned int i;
+	unsigned int i, flip_plane_id = 0;
 	int count = 0, plane_count = 0;
 	unsigned int prop_count = 0;
 	struct pipe_arg *pipe_args = NULL;
@@ -1723,6 +1859,11 @@  int main(int argc, char **argv)
 		case 'v':
 			test_vsync = 1;
 			break;
+		case 'V':
+			if (sscanf(optarg, "%u", &flip_plane_id) != 1)
+				usage(argv[0]);
+			test_vsync = 1;
+			break;
 		case 'w':
 			prop_args = realloc(prop_args,
 					   (prop_count + 1) * sizeof *prop_args);
@@ -1775,7 +1916,7 @@  int main(int argc, char **argv)
 		return -1;
 	}
 
-	if (test_vsync && !count) {
+	if (test_vsync && (!count && !req)) {
 		fprintf(stderr, "page flipping requires at least one -s option.\n");
 		return -1;
 	}
@@ -1811,6 +1952,10 @@  int main(int argc, char **argv)
 
 		atomic_modeset(&dev, req, dev.resources, prop_args, prop_count);
 
+		if (test_vsync)
+			atomic_test_page_flip(&dev, req, dev.resources,
+					prop_args, prop_count, flip_plane_id);
+
 		getchar();
 	} else {
 		for (i = 0; i < prop_count; ++i)