On Fri, Jan 15, 2016 at 11:06:24AM +0200, Marius Vlad wrote:
> So far, we had only COMMIT_UNIVERSAL and COMMIT_LEGACY, using
> drmModeSetPlane()/drmSetCrtc(). This patch adds COMMIT_ATOMIC
> to igt_display_commit2() that makes use of drmModeAtomicCommit().
>
> Signed-off-by: Marius Vlad <marius.c.vlad@xxxxxxxxx>
Hmm, this looks pretty similar to the IGT infrastructure I added for
nuclear/atomic about a year ago:
https://github.com/mattrope/intel-gpu-tools/commits/nuclear_pageflip
Specifically the commit "lib/kms: Add nuclear pageflip commit style"
At the time atomic was still too new and the various pieces like libdrm
weren't readily available so we didn't actually merge it. Then I got
too busy and never updated/merged it once the API finalized.
One of the VPG validation teams was in the process of resurrecting my
old IGT work and updating it to match the small changes that snuck in as
the API finalized; it may make more sense for them to leverage your work
here instead since it's already up-to-date. +Cc Avinash.
Also, as noted by Maarten, you're calling drmModeAtomicCommit() at the
wrong layer and submitting every single plane update as its own
transaction (and missing the CRTC-level updates?). You either want to
call commit once per-top level update (for true atomic) or once per CRTC
update (for the "nuclear pageflip" subset). I don't think I had it in
my github repo, but I had some patches that made these two separate
commit styles, COMMIT_ATOMIC and COMMIT_NUCLEAR that would allow either
of these styles to be used.
Matt
> ---
> lib/igt_kms.c | 190 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> lib/igt_kms.h | 33 +++++++++-
> 2 files changed, 221 insertions(+), 2 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 497118a..61f7a39 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1306,6 +1306,191 @@ static uint32_t igt_plane_get_fb_gem_handle(igt_plane_t *plane)
> igt_assert(r == 0); \
> }
>
> +static const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
> + "SRC_X",
> + "SRC_Y",
> + "SRC_W",
> + "SRC_H",
> + "CRTC_X",
> + "CRTC_Y",
> + "CRTC_W",
> + "CRTC_H",
> + "FB_ID",
> + "CRTC_ID",
> + "type"
> +};
> +
> +/*
> + * Retrieve all the properies specified in props_name and store them into
> + * plane->atomic_props_plane.
> + */
> +static void
> +igt_atomic_fill_plane_props(igt_display_t *display, igt_plane_t *plane,
> + int type, int num_props, const char **prop_names)
> +{
> + drmModeObjectPropertiesPtr props;
> + int i, j, fd;
> +
> + fd = display->drm_fd;
> +
> + props = drmModeObjectGetProperties(fd, plane->drm_plane->plane_id, type);
> + igt_assert(props);
> +
> + for (i = 0; i < props->count_props; i++) {
> + drmModePropertyPtr prop =
> + drmModeGetProperty(fd, props->props[i]);
> +
> + for (j = 0; j < num_props; j++) {
> + if (strcmp(prop->name, prop_names[j]) != 0)
> + continue;
> + plane->atomic_props_plane[j] = props->props[i];
> + break;
> + }
> +
> + drmModeFreeProperty(prop);
> + }
> +
> + drmModeFreeObjectProperties(props);
> +}
> +
> +/*
> + * Commit position and fb changes to a DRM plane via the AtomicCommit()
> + * ioctl; if the DRM call to program the plane fails, we'll either fail
> + * immediately (for tests that expect the commit to succeed) or return the
> + * failure code (for tests that expect a specific error code).
> + */
> +static int
> +igt_atomic_plane_commit(igt_plane_t *plane, igt_output_t *output,
> + bool fail_on_error)
> +{
> + igt_display_t *display = output->display;
> +
> + uint32_t fb_id, crtc_id;
> + int ret;
> + uint32_t src_x;
> + uint32_t src_y;
> + uint32_t src_w;
> + uint32_t src_h;
> + int32_t crtc_x;
> + int32_t crtc_y;
> + uint32_t crtc_w;
> + uint32_t crtc_h;
> + drmModeAtomicReq *req;
> +
> + igt_assert(plane->drm_plane);
> +
> + do_or_die(drmSetClientCap(display->drm_fd, DRM_CLIENT_CAP_ATOMIC, 1));
> +
> + /* it's an error to try an unsupported feature */
> + igt_assert(igt_plane_supports_rotation(plane) ||
> + !plane->rotation_changed);
> +
> + fb_id = igt_plane_get_fb_id(plane);
> + crtc_id = output->config.crtc->crtc_id;
> +
> + if ((plane->fb_changed || plane->size_changed) && fb_id == 0) {
> +
> + LOG(display,
> + "%s: drmModeAtomicCommit pipe %s, plane %d, disabling\n",
> + igt_output_name(output),
> + kmstest_pipe_name(output->config.pipe),
> + plane->index);
> +
> + req = drmModeAtomicAlloc();
> + igt_atomic_fill_plane_props(display, plane,
> + DRM_MODE_OBJECT_PLANE,
> + IGT_NUM_PLANE_PROPS,
> + igt_plane_prop_names);
> +
> + drmModeAtomicSetCursor(req, 0);
> +
> + /* populate plane req */
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, crtc_id);
> +
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_X, IGT_FIXED(0, 0));
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_Y, IGT_FIXED(0, 0));
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_W, IGT_FIXED(0, 0));
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_H, IGT_FIXED(0, 0));
> +
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_X, 0);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_Y, 0);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_W, 0);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_H, 0);
> +
> + ret = drmModeAtomicCommit(display->drm_fd, req, 0, NULL);
> + drmModeAtomicFree(req);
> +
> + CHECK_RETURN(ret, fail_on_error);
> +
> + } else if (plane->fb_changed || plane->position_changed ||
> + plane->size_changed) {
> +
> + src_x = IGT_FIXED(plane->fb->src_x,0); /* src_x */
> + src_y = IGT_FIXED(plane->fb->src_y,0); /* src_y */
> + src_w = IGT_FIXED(plane->fb->src_w,0); /* src_w */
> + src_h = IGT_FIXED(plane->fb->src_h,0); /* src_h */
> + crtc_x = plane->crtc_x;
> + crtc_y = plane->crtc_y;
> + crtc_w = plane->crtc_w;
> + crtc_h = plane->crtc_h;
> +
> + LOG(display,
> + "%s: drmModeAtomicCommit %s.%d, fb %u, src = (%d, %d) "
> + "%ux%u dst = (%u, %u) %ux%u\n",
> + igt_output_name(output),
> + kmstest_pipe_name(output->config.pipe),
> + plane->index,
> + fb_id,
> + src_x >> 16, src_y >> 16, src_w >> 16, src_h >> 16,
> + crtc_x, crtc_y, crtc_w, crtc_h);
> +
> +
> + req = drmModeAtomicAlloc();
> + igt_atomic_fill_plane_props(display, plane,
> + DRM_MODE_OBJECT_PLANE,
> + IGT_NUM_PLANE_PROPS,
> + igt_plane_prop_names);
> +
> + drmModeAtomicSetCursor(req, 0);
> +
> + /* populate plane req */
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, crtc_id);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, fb_id);
> +
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_X, src_x);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_Y, src_y);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_W, src_w);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_H, src_h);
> +
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_X, crtc_x);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_Y, crtc_y);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_W, crtc_w);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_H, crtc_h);
> +
> + ret = drmModeAtomicCommit(display->drm_fd, req, 0, NULL);
> + drmModeAtomicFree(req);
> +
> + CHECK_RETURN(ret, fail_on_error);
> + }
> +
> + plane->fb_changed = false;
> + plane->position_changed = false;
> + plane->size_changed = false;
> +
> +
> + if (plane->rotation_changed) {
> + ret = igt_plane_set_property(plane, plane->rotation_property,
> + plane->rotation);
> +
> + plane->rotation_changed = false;
> + CHECK_RETURN(ret, fail_on_error);
> + }
> +
> + return 0;
> +}
> +
> +
> +
> /*
> * Commit position and fb changes to a DRM plane via the SetPlane ioctl; if the
> * DRM call to program the plane fails, we'll either fail immediately (for
> @@ -1558,7 +1743,10 @@ static int igt_plane_commit(igt_plane_t *plane,
> return igt_primary_plane_commit_legacy(plane, output,
> fail_on_error);
> } else {
> - return igt_drm_plane_commit(plane, output, fail_on_error);
> + if (s == COMMIT_ATOMIC)
> + return igt_atomic_plane_commit(plane, output, fail_on_error);
> + else
> + return igt_drm_plane_commit(plane, output, fail_on_error);
> }
> }
>
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 94f315f..81d8dd7 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -156,9 +156,27 @@ void kmstest_unset_all_crtcs(int drm_fd, drmModeResPtr resources);
> enum igt_commit_style {
> COMMIT_LEGACY = 0,
> COMMIT_UNIVERSAL,
> - /* We'll add atomic here eventually. */
> + COMMIT_ATOMIC,
> };
>
> +enum igt_atomic_plane_properties {
> + IGT_PLANE_SRC_X = 0,
> + IGT_PLANE_SRC_Y,
> + IGT_PLANE_SRC_W,
> + IGT_PLANE_SRC_H,
> +
> + IGT_PLANE_CRTC_X,
> + IGT_PLANE_CRTC_Y,
> + IGT_PLANE_CRTC_W,
> + IGT_PLANE_CRTC_H,
> +
> + IGT_PLANE_FB_ID,
> + IGT_PLANE_CRTC_ID,
> + IGT_PLANE_TYPE,
> + IGT_NUM_PLANE_PROPS
> +};
> +
> +
> typedef struct igt_display igt_display_t;
> typedef struct igt_pipe igt_pipe_t;
> typedef uint32_t igt_fixed_t; /* 16.16 fixed point */
> @@ -200,6 +218,7 @@ typedef struct {
> /* panning offset within the fb */
> unsigned int pan_x, pan_y;
> igt_rotation_t rotation;
> + uint32_t atomic_props_plane[IGT_NUM_PLANE_PROPS];
> } igt_plane_t;
>
> struct igt_pipe {
> @@ -281,6 +300,18 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>
> #define IGT_FIXED(i,f) ((i) << 16 | (f))
>
> +/**
> + * igt_atomic_populate_plane_req:
> + * @req: A pointer to drmModeAtomicReq
> + * @plane: A pointer igt_plane_t
> + * @prop: one of igt_atomic_plane_properties
> + * @value: the value to add
> + */
> +#define igt_atomic_populate_plane_req(req, plane, prop, value) \
> + igt_assert_lt(0, drmModeAtomicAddProperty(req, plane->drm_plane->plane_id,\
> + plane->atomic_props_plane[prop], value))
> +
> +
> void igt_enable_connectors(void);
> void igt_reset_connectors(void);
>
> --
> 2.6.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
