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

Emil Velikov emil.l.velikov at gmail.com
Thu Nov 26 15:57:34 PST 2015


On 23 September 2015 at 03:45, Hyungwon Hwang <human.hwang at 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.
>
> Signed-off-by: Hyungwon Hwang <human.hwang at samsung.com>
> ---
>  tests/modetest/modetest.c | 195 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 187 insertions(+), 8 deletions(-)
>
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index bc5a227..418acaa 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
> @@ -747,6 +747,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 {
> @@ -1477,7 +1481,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");
>
> @@ -1492,6 +1496,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");
> @@ -1641,7 +1646,8 @@ static int allocate_fb(int fd, drmModeAtomicReqPtr req, struct resources *res,
>
>  static int allocate_fbs(struct device *dev, drmModeAtomicReqPtr req,
>                         struct resources *res, struct property_arg *prop_args,
> -                       unsigned int prop_count, struct bo **bo, uint32_t *fb_id)
> +                       unsigned int prop_count, struct bo **bo, uint32_t *fb_id,
> +                       uint32_t flip_plane_id)
>  {
>         uint32_t plane_id, fb_obj_id, pixel_format;
>         uint64_t width, height;
> @@ -1652,6 +1658,9 @@ static int allocate_fbs(struct device *dev, drmModeAtomicReqPtr req,
>                 if (!is_obj_id_in_prop_args(prop_args, prop_count, plane_id))
>                         continue;
>
> +               if (flip_plane_id == plane_id)
> +                       dev->mode.fb_id = fb_id[i];
> +
>                 fb_obj_id = get_plane_prop_id(res, plane_id, "FB_ID");
>                 if (!fb_obj_id) {
>                         fprintf(stderr, "plane(%u) does not exist\n", plane_id);
> @@ -1714,8 +1723,162 @@ static void deallocate_fbs(int fd, int num_planes, uint32_t *fb_id, struct bo **
>         }
>  }
>
> +static void atomic_page_flip_handler(int fd, unsigned int frame, unsigned int sec,
> +                                       unsigned int usec, void *data)
> +{
> +       static drmModeAtomicReqPtr req = NULL;
Initialisation not needed

> +       unsigned int new_fb_id;
> +       struct timeval end;
> +       struct pipe_arg *pipe;
> +       double t;
> +       uint32_t flags = 0;
Feed 0 directly into the function ?

> +       int ret;
> +
> +       pipe = (struct pipe_arg *)(unsigned long)data;
> +
> +       req = drmModeAtomicAlloc();
> +       if (!req) {
> +               fprintf(stderr, "failed to allocate drmModeAtomicReqPtr\n");
> +               return;
> +       }
> +
> +       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++;
> +
> +       ret = drmModeAtomicAddProperty(req, pipe->plane_id, pipe->fb_obj_id, new_fb_id);
> +       if (ret < 0) {
> +               fprintf(stderr, "failed to add atomic property in pageflip handler\n");
Use a goto label ?
> +               drmModeAtomicFree(req);
> +               return;
> +       }
> +
> +       flags = DRM_MODE_PAGE_FLIP_EVENT;
> +       ret = drmModeAtomicCommit(fd, req, flags, pipe);
> +       if (ret < 0) {
> +               fprintf(stderr, "failed to commit in pageflip handler\n");
> +               drmModeAtomicFree(req);
> +               return;
> +       }
> +
> +       if (pipe->swap_count == 60) {
Use vrefresh here ?

> +               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 void atomic_test_page_flip(struct device *dev, drmModeAtomicReqPtr req,
> +                       struct resources *res, struct property_arg *prop_args,
constify the structs ?

> +                       unsigned int prop_count, unsigned int flip_plane_id)
> +{
> +       struct bo *other_bo;
> +       unsigned int other_fb_id;
> +       struct pipe_arg pipe;
> +       drmEventContext evctx;
> +       uint32_t flags = 0, fb_obj_id = 0, pixel_format;
Variable and initialisation isn't needed.

> +       uint64_t width, height;
> +       int ret;
> +
> +       fb_obj_id = get_plane_prop_id(res, flip_plane_id, "FB_ID");
> +       if (!fb_obj_id) {
> +               fprintf(stderr, "plane(%u) does not exist\n", flip_plane_id);
> +               return;
> +       }
> +
> +       if (!is_obj_id_in_prop_args(prop_args, prop_count, flip_plane_id)) {
> +               fprintf(stderr, "plane (%u) must be set for pageflip\n", flip_plane_id);
> +               return;
> +       }
> +
> +       ret = get_value_in_prop_args(prop_args, prop_count, flip_plane_id,
> +                                       "SRC_W", &width);
> +       if (ret < 0) {
> +               fprintf(stderr, "SRC_W for plane (%u) must be set\n", flip_plane_id);
> +               return;
> +       }
> +
> +       ret = get_value_in_prop_args(prop_args, prop_count, flip_plane_id,
> +                                       "SRC_H", &height);
> +       if (ret < 0) {
> +               fprintf(stderr, "SRC_H for plane (%u) must be set\n", flip_plane_id);
> +               return;
> +       }
> +
> +       pixel_format = DRM_FORMAT_XRGB8888;
> +
> +       ret = allocate_fb(dev->fd, req, res, width, height, pixel_format,
> +                       PATTERN_TILES, &other_bo, &other_fb_id);
> +       if (ret < 0)
> +               return;
> +
> +       ret = drmModeAtomicAddProperty(req, flip_plane_id, fb_obj_id,
> +                       other_fb_id);
> +       if (ret < 0) {
> +               fprintf(stderr, "failed to add atomic property for pageflip");
> +               goto err;
> +       }
> +
> +       gettimeofday(&pipe.start, NULL);
> +       pipe.swap_count = 0;
> +       pipe.plane_id = flip_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) {
> +               fprintf(stderr, "failed to commit for pageflip\n");
> +               goto err;
> +       }
> +
> +       memset(&evctx, 0, sizeof evctx);
> +       evctx.version = 2;
> +       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;
> +
> +               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:
> +       drmModeRmFB(dev->fd, other_fb_id);
> +       bo_destroy(other_bo);
> +
> +       return;
> +}
> +
>  static int atomic_modeset(struct device *dev, drmModeAtomicReqPtr req,
> -                       struct property_arg *prop_args, unsigned int prop_count)
> +                       struct property_arg *prop_args, unsigned int prop_count,
> +                       int test_vsync, int flip_plane_id)
>  {
>         unsigned int num_planes;
>         const char *obj_type = NULL;
> @@ -1753,7 +1916,8 @@ static int atomic_modeset(struct device *dev, drmModeAtomicReqPtr req,
>                 return -1;
>         }
>
> -       ret = allocate_fbs(dev, req, dev->resources, prop_args, prop_count, bo, fb_id);
> +       ret = allocate_fbs(dev, req, dev->resources, prop_args, prop_count, bo,
> +                               fb_id, flip_plane_id);
>         if (ret < 0) {
>                 free(fb_id);
>                 free(bo);
> @@ -1769,6 +1933,10 @@ static int atomic_modeset(struct device *dev, drmModeAtomicReqPtr req,
>                 return ret;
>         }
>
> +       if (test_vsync)
> +               atomic_test_page_flip(dev, req, dev->resources,
> +                               prop_args, prop_count, flip_plane_id);
> +
>         getchar();
>
>         deallocate_fbs(dev->fd, num_planes, fb_id, bo);
> @@ -1778,7 +1946,7 @@ static int atomic_modeset(struct device *dev, drmModeAtomicReqPtr req,
>         return 0;
>  }
>
> -static char optstr[] = "acdD:efM:P:ps:Cvw:";
> +static char optstr[] = "acdD:efM:P:ps:CvV:w:";
>
>  int main(int argc, char **argv)
>  {
> @@ -1792,7 +1960,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;
>         unsigned int count = 0, plane_count = 0;
>         unsigned int prop_count = 0;
>         struct pipe_arg *pipe_args = NULL;
> @@ -1873,6 +2041,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);
> @@ -1925,11 +2098,16 @@ int main(int argc, char **argv)
>                 return -1;
>         }
>
> -       if (test_vsync && !count) {
> +       if (test_vsync && (!count && !is_atomic)) {
>                 fprintf(stderr, "page flipping requires at least one -s option.\n");
>                 return -1;
>         }
>
> +       if (test_vsync && is_atomic && flip_plane_id == 0) {
> +               fprintf(stderr, "fliping plane id must be set for atomic pageflip\n");
And how does average Joe find out the ID ? Something feels quite funny
with this approach.

Hyungwon, despite my nitpicking your work looks quite good. The above
question is rather important and I take care of everything else
-either before pushing or as a follow up patch-es, depending on your
preference. Let me know how you feel about it.

Thanks
Emil


More information about the dri-devel mailing list