Re: [PATCH i-g-t v2] tests/kms_atomic: Convert/rewrite tests to use igt_kms framework, v2.

Mika Kahola
2017-10-20T10:16:09Z

On Fri, 2017-10-20 at 12:08 +0200, Maarten Lankhorst wrote:
> Op 20-10-17 om 12:02 schreef Mika Kahola:
> >
> > On Thu, 2017-10-12 at 17:33 +0200, Maarten Lankhorst wrote:
> > >
> > > Now that we can set individual properties through the igt_kms
> > > api,
> > > we no longer need to duplicate functionality from igt_kms. Set
> > > invalid
> > > properties directly, and rewrite kms_atomic.c to use igt_display.
> > > This will allow us to remove a lot of code in kms_atomic.c,
> > > and benefit from how igt_kms can set up a valid configuration,
> > > instead of having to inherit it from fbcon.
> > >
> > > Changes since v1:
> > > - Fix test failure when atomic_invalid_params is run standalone.
> > >
> > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.c
> > > om>
> > > Cc: Daniel Stone <daniels@collabora.com>
> > > ---
> > >  tests/kms_atomic.c | 1668 ++++++++++++++++--------------------
> > > ----
> > > ------------
> > >  1 file changed, 512 insertions(+), 1156 deletions(-)
> > >
> > > diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
> > > index 042a7c263aab..e0fc324eab61 100644
> > > --- a/tests/kms_atomic.c
> > > +++ b/tests/kms_atomic.c
> > > @@ -46,10 +46,6 @@
> > >  #include "igt_aux.h"
> > >  #include "sw_sync.h"
> > >
> > > -#ifndef DRM_CLIENT_CAP_ATOMIC
> > > -#define DRM_CLIENT_CAP_ATOMIC 3
> > > -#endif
> > > -
> > >  #ifndef DRM_CAP_CURSOR_WIDTH
> > >  #define DRM_CAP_CURSOR_WIDTH 0x8
> > >  #endif
> > > @@ -58,23 +54,6 @@
> > >  #define DRM_CAP_CURSOR_HEIGHT 0x9
> > >  #endif
> > >
> > > -#ifndef DRM_MODE_ATOMIC_TEST_ONLY
> > > -#define DRM_MODE_ATOMIC_TEST_ONLY 0x0100
> > > -#define DRM_MODE_ATOMIC_NONBLOCK 0x0200
> > > -#define DRM_MODE_ATOMIC_ALLOW_MODESET 0x0400
> > > -
> > > -struct drm_mode_atomic {
> > > - __u32 flags;
> > > - __u32 count_objs;
> > > - __u64 objs_ptr;
> > > - __u64 count_props_ptr;
> > > - __u64 props_ptr;
> > > - __u64 prop_values_ptr;
> > > - __u64 reserved;
> > > - __u64 user_data;
> > > -};
> > > -#endif
> > > -
> > >  IGT_TEST_DESCRIPTION("Test atomic modesetting API");
> > >
> > >  enum kms_atomic_check_relax {
> > > @@ -83,1259 +62,652 @@ enum kms_atomic_check_relax {
> > >   PLANE_RELAX_FB = (1 << 1)
> > >  };
> > >
> > > -/**
> > > - * KMS plane type enum
> > > - *
> > > - * KMS plane types are represented by enums, which do not have
> > > stable numeric
> > > - * values, but must be looked up by their string value each
> > > time.
> > > - *
> > > - * To make the code more simple, we define a plane_type enum
> > > which
> > > maps to
> > > - * each KMS enum value. These values must be looked up through
> > > the
> > > map, and
> > > - * cannot be passed directly to KMS functions.
> > > - */
> > > -enum plane_type {
> > > - PLANE_TYPE_PRIMARY = 0,
> > > - PLANE_TYPE_OVERLAY,
> > > - PLANE_TYPE_CURSOR,
> > > - NUM_PLANE_TYPE_PROPS
> > > -};
> > > -
> > > -static const char *plane_type_prop_names[NUM_PLANE_TYPE_PROPS] =
> > > {
> > > - "Primary",
> > > - "Overlay",
> > > - "Cursor"
> > > -};
> > > -
> > > -struct kms_atomic_blob {
> > > - uint32_t id; /* 0 if not already allocated */
> > > - size_t len;
> > > - void *data;
> > > -};
> > > -
> > > -struct kms_atomic_connector_state {
> > > - struct kms_atomic_state *state;
> > > - uint32_t obj;
> > > - uint32_t crtc_id;
> > > -};
> > > -
> > > -struct kms_atomic_plane_state {
> > > - struct kms_atomic_state *state;
> > > - uint32_t obj;
> > > - enum plane_type type;
> > > - uint32_t crtc_mask;
> > > - uint32_t crtc_id; /* 0 to disable */
> > > - uint32_t fb_id; /* 0 to disable */
> > > - uint32_t src_x, src_y, src_w, src_h; /* 16.16 fixed-
> > > point */
> > > - uint32_t crtc_x, crtc_y, crtc_w, crtc_h; /* normal
> > > integers
> > > */
> > > - int32_t fence_fd;
> > > -};
> > > -
> > > -struct kms_atomic_crtc_state {
> > > - struct kms_atomic_state *state;
> > > - uint32_t obj;
> > > - int idx;
> > > - bool active;
> > > - int32_t *out_fence_ptr;
> > > - struct kms_atomic_blob mode;
> > > -};
> > > -
> > > -struct kms_atomic_state {
> > > - struct kms_atomic_connector_state *connectors;
> > > - int num_connectors;
> > > - struct kms_atomic_crtc_state *crtcs;
> > > - int num_crtcs;
> > > - struct kms_atomic_plane_state *planes;
> > > - int num_planes;
> > > - struct kms_atomic_desc *desc;
> > > -};
> > > -
> > > -struct kms_atomic_desc {
> > > - int fd;
> > > - uint32_t props_connector[IGT_NUM_CONNECTOR_PROPS];
> > > - uint32_t props_crtc[IGT_NUM_CRTC_PROPS];
> > > - uint32_t props_plane[IGT_NUM_PLANE_PROPS];
> > > - uint64_t props_plane_type[NUM_PLANE_TYPE_PROPS];
> > > -};
> > > -
> > > -static uint32_t blob_duplicate(int fd, uint32_t id_orig)
> > > +static bool plane_filter(enum igt_atomic_plane_properties prop)
> > >  {
> > > - drmModePropertyBlobPtr orig = drmModeGetPropertyBlob(fd,
> > > id_orig);
> > > - uint32_t id_new;
> > > -
> > > - igt_assert(orig);
> > > - do_or_die(drmModeCreatePropertyBlob(fd, orig->data,
> > > orig-
> > > >
> > > > length,
> > > -     &id_new));
> > > - drmModeFreePropertyBlob(orig);
> > > + if ((1 << prop) & IGT_PLANE_COORD_CHANGED_MASK)
> > > + return false;
> > >
> > > - return id_new;
> > > -}
> > > -
> > > -#define crtc_set_prop(req, crtc, prop, value) \
> > > - igt_assert_lt(0, drmModeAtomicAddProperty(req, crtc-
> > > >obj, \
> > > -   crtc->state-
> > > >desc-
> > > >
> > > > props_crtc[prop], \
> > > -   value));
> > > -
> > > -#define plane_set_prop(req, plane, prop, value) \
> > > - igt_assert_lt(0, drmModeAtomicAddProperty(req, plane-
> > > >obj, \
> > > -   plane->state-
> > > >
> > > > desc->props_plane[prop], \
> > > -   value));
> > > -
> > > -#define do_atomic_commit(fd, req, flags) \
> > > - do_or_die(drmModeAtomicCommit(fd, req, flags, NULL));
> > > + if (prop == IGT_PLANE_CRTC_ID || prop ==
> > > IGT_PLANE_FB_ID)
> > > + return false;
> > >
> > > -#define do_atomic_commit_err(fd, req, flags, err) { \
> > > - igt_assert_neq(drmModeAtomicCommit(fd, req, flags,
> > > NULL),
> > > 0); \
> > > - igt_assert_eq(errno, err); \
> > > -}
> > > -
> > > -#define crtc_commit_atomic(crtc, plane, req, relax, flags) { \
> > > - drmModeAtomicSetCursor(req, 0); \
> > > - crtc_populate_req(crtc, req); \
> > > - plane_populate_req(plane, req); \
> > > - do_atomic_commit((crtc)->state->desc->fd, req, flags); \
> > > - if (!(flags & DRM_MODE_ATOMIC_TEST_ONLY)) { \
> > > - crtc_check_current_state(crtc, plane, relax); \
> > > - plane_check_current_state(plane, relax); \
> > > - } \
> > > -}
> > > + if (prop == IGT_PLANE_IN_FENCE_FD)
> > > + return false;
> > >
> > > -#define crtc_commit_atomic_err(crtc, plane, crtc_old, plane_old,
> > > req, flags, relax, e) { \
> > > - drmModeAtomicSetCursor(req, 0); \
> > > - crtc_populate_req(crtc, req); \
> > > - plane_populate_req(plane, req); \
> > > - do_atomic_commit_err((crtc)->state->desc->fd, req,
> > > flags,
> > > e); \
> > > - crtc_check_current_state(crtc_old, plane_old, relax); \
> > > - plane_check_current_state(plane_old, relax); \
> > > + /* Don't care about other properties */
> > > + return true;
> > >  }
> > >
> > > -#define plane_commit_atomic(plane, req, relax) { \
> > > - drmModeAtomicSetCursor(req, 0); \
> > > - plane_populate_req(plane, req); \
> > > - do_atomic_commit((plane)->state->desc->fd, req, 0); \
> > > - plane_check_current_state(plane, relax); \
> > > -}
> > > -
> > > -#define plane_commit_atomic_err(plane, plane_old, req, relax, e)
> > > { \
> > > - drmModeAtomicSetCursor(req, 0); \
> > > - plane_populate_req(plane, req); \
> > > - do_atomic_commit_err((plane)->state->desc->fd, req, 0,
> > > e); \
> > > - plane_check_current_state(plane_old, relax); \
> > > -}
> > > -
> > > -static void
> > > -connector_get_current_state(struct kms_atomic_connector_state
> > > *connector)
> > > -{
> > > - drmModeObjectPropertiesPtr props;
> > > - int i;
> > > -
> > > - props = drmModeObjectGetProperties(connector->state-
> > > >desc-
> > > >
> > > > fd,
> > > -    connector->obj,
> > > -    DRM_MODE_OBJECT_CONNE
> > > CTOR
> > > );
> > > - igt_assert(props);
> > > -
> > > - for (i = 0; i < props->count_props; i++) {
> > > - uint32_t *prop_ids = connector->state->desc-
> > > >
> > > > props_connector;
> > > -
> > > - if (props->props[i] ==
> > > prop_ids[IGT_CONNECTOR_CRTC_ID])
> > > - connector->crtc_id = props-
> > > >prop_values[i];
> > > - }
> > > - drmModeFreeObjectProperties(props);
> > > -}
> > > -
> > > -#if 0
> > > -/* XXX: Checking this repeatedly actually hangs the GPU. I have
> > > literally no
> > > - *      idea why. */
> > > -static void
> > > -connector_check_current_state(struct kms_atomic_connector_state
> > > *connector)
> > > -{
> > > - struct kms_atomic_connector_state connector_kernel;
> > > - drmModeConnectorPtr legacy;
> > > - uint32_t crtc_id;
> > > -
> > > - legacy = drmModeGetConnectorCurrent(connector->state-
> > > >desc-
> > > >
> > > > fd,
> > > -     connector->obj);
> > > - igt_assert(legacy);
> > > -
> > > - if (legacy->encoder_id) {
> > > - drmModeEncoderPtr legacy_enc;
> > > -
> > > - legacy_enc = drmModeGetEncoder(connector->state-
> > > >
> > > > desc->fd,
> > > -        legacy-
> > > >encoder_id);
> > > - igt_assert(legacy_enc);
> > > -
> > > - crtc_id = legacy_enc->crtc_id;
> > > - drmModeFreeEncoder(legacy_enc);
> > > - } else {
> > > - crtc_id = 0;
> > > - }
> > > -
> > > - igt_assert_eq_u32(crtc_id, connector->crtc_id);
> > > -
> > > - memcpy(&connector_kernel, connector,
> > > sizeof(connector_kernel));
> > > - connector_get_current_state(&connector_kernel);
> > > - do_or_die(memcmp(&connector_kernel, connector,
> > > -  sizeof(connector_kernel)));
> > > -
> > > - drmModeFreeConnector(legacy);
> > > -}
> > > -#endif
> > > -
> > > -static struct kms_atomic_connector_state *
> > > -find_connector(struct kms_atomic_state *state,
> > > -        struct kms_atomic_crtc_state *crtc)
> > > +static void plane_get_current_state(igt_plane_t *plane, uint64_t
> > > *values)
> > >  {
> > >   int i;
> > >
> > > - for (i = 0; i < state->num_connectors; i++) {
> > > - struct kms_atomic_connector_state *connector =
> > > - &state->connectors[i];
> > > -
> > > - if (!connector->obj)
> > > + for (i = 0; i < IGT_NUM_PLANE_PROPS; i++) {
> > > + if (plane_filter(i)) {
> > > + values[i] = 0;
> > >   continue;
> > > - if (crtc && connector->crtc_id != crtc->obj)
> > > - continue;
> > > -
> > > - return connector;
> > > - }
> > > -
> > > - return NULL;
> > > -}
> > > -
> > > -static void plane_populate_req(struct kms_atomic_plane_state
> > > *plane,
> > > -        drmModeAtomicReq *req)
> > > -{
> > > - if (plane->fence_fd)
> > > - plane_set_prop(req, plane,
> > > IGT_PLANE_IN_FENCE_FD,
> > > plane->fence_fd);
> > > -
> > > - plane_set_prop(req, plane, IGT_PLANE_CRTC_ID, plane-
> > > >
> > > > crtc_id);
> > > - plane_set_prop(req, plane, IGT_PLANE_FB_ID, plane-
> > > >fb_id);
> > > - plane_set_prop(req, plane, IGT_PLANE_SRC_X, plane-
> > > >src_x);
> > > - plane_set_prop(req, plane, IGT_PLANE_SRC_Y, plane-
> > > >src_y);
> > > - plane_set_prop(req, plane, IGT_PLANE_SRC_W, plane-
> > > >src_w);
> > > - plane_set_prop(req, plane, IGT_PLANE_SRC_H, plane-
> > > >src_h);
> > > - plane_set_prop(req, plane, IGT_PLANE_CRTC_X, plane-
> > > >crtc_x);
> > > - plane_set_prop(req, plane, IGT_PLANE_CRTC_Y, plane-
> > > >crtc_y);
> > > - plane_set_prop(req, plane, IGT_PLANE_CRTC_W, plane-
> > > >crtc_w);
> > > - plane_set_prop(req, plane, IGT_PLANE_CRTC_H, plane-
> > > >crtc_h);
> > > -}
> > > -
> > > -static void plane_get_current_state(struct
> > > kms_atomic_plane_state
> > > *plane)
> > > -{
> > > - struct kms_atomic_desc *desc = plane->state->desc;
> > > - drmModeObjectPropertiesPtr props;
> > > - int i;
> > > -
> > > - props = drmModeObjectGetProperties(desc->fd, plane->obj,
> > > -    DRM_MODE_OBJECT_PLANE
> > > );
> > > - igt_assert(props);
> > > -
> > > - for (i = 0; i < props->count_props; i++) {
> > > - uint32_t *prop_ids = desc->props_plane;
> > > -
> > > - if (props->props[i] ==
> > > prop_ids[IGT_PLANE_CRTC_ID])
> > > - plane->crtc_id = props->prop_values[i];
> > > - else if (props->props[i] ==
> > > prop_ids[IGT_PLANE_FB_ID])
> > > - plane->fb_id = props->prop_values[i];
> > > - else if (props->props[i] ==
> > > prop_ids[IGT_PLANE_CRTC_X])
> > > - plane->crtc_x = props->prop_values[i];
> > > - else if (props->props[i] ==
> > > prop_ids[IGT_PLANE_CRTC_Y])
> > > - plane->crtc_y = props->prop_values[i];
> > > - else if (props->props[i] ==
> > > prop_ids[IGT_PLANE_CRTC_W])
> > > - plane->crtc_w = props->prop_values[i];
> > > - else if (props->props[i] ==
> > > prop_ids[IGT_PLANE_CRTC_H])
> > > - plane->crtc_h = props->prop_values[i];
> > > - else if (props->props[i] ==
> > > prop_ids[IGT_PLANE_SRC_X])
> > > - plane->src_x = props->prop_values[i];
> > > - else if (props->props[i] ==
> > > prop_ids[IGT_PLANE_SRC_Y])
> > > - plane->src_y = props->prop_values[i];
> > > - else if (props->props[i] ==
> > > prop_ids[IGT_PLANE_SRC_W])
> > > - plane->src_w = props->prop_values[i];
> > > - else if (props->props[i] ==
> > > prop_ids[IGT_PLANE_SRC_H])
> > > - plane->src_h = props->prop_values[i];
> > > - else if (props->props[i] ==
> > > prop_ids[IGT_PLANE_TYPE]) {
> > > - int j;
> > > -
> > > - for (j = 0; j < ARRAY_SIZE(desc-
> > > >
> > > > props_plane_type); j++) {
> > > - if (props->prop_values[i] ==
> > > desc-
> > > >
> > > > props_plane_type[j]) {
> > > - plane->type = j;
> > > - break;
> > > - }
> > > - }
> > >   }
> > > - }
> > >
> > > - drmModeFreeObjectProperties(props);
> > > + values[i] = igt_plane_get_prop(plane, i);
> > > + }
> > >  }
> > >
> > > -static void plane_check_current_state(struct
> > > kms_atomic_plane_state
> > > *plane,
> > > +static void plane_check_current_state(igt_plane_t *plane, const
> > > uint64_t *values,
> > >         enum
> > > kms_atomic_check_relax
> > > relax)
> > >  {
> > >   drmModePlanePtr legacy;
> > > - struct kms_atomic_plane_state plane_kernel;
> > > + uint64_t current_values[IGT_NUM_PLANE_PROPS];
> > > + int i;
> > >
> > > - legacy = drmModeGetPlane(plane->state->desc->fd, plane-
> > > >
> > > > obj);
> > > + legacy = drmModeGetPlane(plane->pipe->display->drm_fd,
> > > plane->drm_plane->plane_id);
> > >   igt_assert(legacy);
> > >
> > > - igt_assert_eq_u32(legacy->crtc_id, plane->crtc_id);
> > > + igt_assert_eq_u32(legacy->crtc_id,
> > > values[IGT_PLANE_CRTC_ID]);
> > >
> > >   if (!(relax & PLANE_RELAX_FB))
> > > - igt_assert_eq_u32(legacy->fb_id, plane->fb_id);
> > > + igt_assert_eq_u32(legacy->fb_id,
> > > values[IGT_PLANE_FB_ID]);
> > >
> > > - memcpy(&plane_kernel, plane, sizeof(plane_kernel));
> > > - plane_get_current_state(&plane_kernel);
> > > + plane_get_current_state(plane, current_values);
> > >
> > >   /* Legacy cursor ioctls create their own, unknowable,
> > > internal
> > >    * framebuffer which we can't reason about. */
> > >   if (relax & PLANE_RELAX_FB)
> > > - plane_kernel.fb_id = plane->fb_id;
> > > - do_or_die(memcmp(&plane_kernel, plane,
> > > sizeof(plane_kernel)));
> > > + current_values[IGT_PLANE_FB_ID] =
> > > values[IGT_PLANE_FB_ID];
> > > +
> > > + for (i = 0; i < IGT_NUM_PLANE_PROPS; i++)
> > > + if (!plane_filter(i))
> > > + igt_assert_eq_u64(current_values[i],
> > > values[i]);
> > >
> > >   drmModeFreePlane(legacy);
> > >  }
> > >
> > > -static void plane_commit_legacy(struct kms_atomic_plane_state
> > > *plane,
> > > +static void plane_commit(igt_plane_t *plane, enum
> > > igt_commit_style
> > > s,
> > >                                  enum kms_atomic_check_relax
> > > relax)
> > >  {
> > > - do_or_die(drmModeSetPlane(plane->state->desc->fd, plane-
> > > >
> > > > obj,
> > > -   plane->crtc_id,
> > > -   plane->fb_id, 0,
> > > -   plane->crtc_x, plane->crtc_y,
> > > -   plane->crtc_w, plane->crtc_h,
> > > -   plane->src_x, plane->src_y,
> > > -   plane->src_w, plane->src_h));
> > > - plane_check_current_state(plane, relax);
> > > + igt_display_commit2(plane->pipe->display, s);
> > > + plane_check_current_state(plane, plane->values, relax);
> > >  }
> > >
> > > -static struct kms_atomic_plane_state *
> > > -find_plane(struct kms_atomic_state *state, enum plane_type type,
> > > -    struct kms_atomic_crtc_state *crtc)
> > > +static void plane_commit_atomic_err(igt_plane_t *plane,
> > > +     enum kms_atomic_check_relax
> > > relax,
> > > +     int err)
> > >  {
> > > - struct kms_atomic_plane_state *ret = NULL;
> > > - int i;
> > > -
> > > - for (i = 0; i < state->num_planes; i++) {
> > > - struct kms_atomic_plane_state *plane = &state-
> > > >
> > > > planes[i];
> > > -
> > > - if (!plane->obj)
> > > - continue;
> > > - if (type != NUM_PLANE_TYPE_PROPS && plane->type
> > > !=
> > > type)
> > > - continue;
> > > - if (crtc && !(plane->crtc_mask & (1 << crtc-
> > > >idx)))
> > > - continue;
> > > + uint64_t current_values[IGT_NUM_PLANE_PROPS];
> > >
> > > - plane_get_current_state(plane);
> > > + plane_get_current_state(plane, current_values);
> > >
> > > - /* Try to find a plane that's already on this
> > > CRTC.
> > > In
> > > -  * particular, this ensures that for special
> > > (primary/cursor)
> > > -  * planes that can be on multiple CRTCs, we find
> > > the
> > > same
> > > -  * one that the legacy ioctls will. */
> > > - if (!crtc || plane->crtc_id == crtc->obj)
> > > - return plane;
> > > -
> > > - ret = plane;
> > > - }
> > > + igt_assert_eq(-err, igt_display_try_commit2(plane->pipe-
> > > >
> > > > display, COMMIT_ATOMIC));
> > >
> > > - return ret;
> > > + plane_check_current_state(plane, current_values, relax);
> > >  }
> > >
> > > -static void crtc_populate_req(struct kms_atomic_crtc_state
> > > *crtc,
> > > -       drmModeAtomicReq *req)
> > > +static bool crtc_filter(enum igt_atomic_crtc_properties prop)
> > >  {
> > > - if (crtc->out_fence_ptr)
> > > - crtc_set_prop(req, crtc, IGT_CRTC_OUT_FENCE_PTR,
> > > -       to_user_pointer(crtc-
> > > >out_fence_ptr));
> > > + if (prop == IGT_CRTC_MODE_ID || prop == IGT_CRTC_ACTIVE)
> > > + return false;
> > >
> > > - crtc_set_prop(req, crtc, IGT_CRTC_MODE_ID, crtc-
> > > >mode.id);
> > > - crtc_set_prop(req, crtc, IGT_CRTC_ACTIVE, crtc->active);
> > > + return true;
> > >  }
> > >
> > > -static void crtc_get_current_state(struct kms_atomic_crtc_state
> > > *crtc)
> > > +static void crtc_get_current_state(igt_pipe_t *pipe, uint64_t
> > > *values)
> > >  {
> > > - drmModeObjectPropertiesPtr props;
> > >   int i;
> > >
> > > - props = drmModeObjectGetProperties(crtc->state->desc-
> > > >fd,
> > > crtc->obj,
> > > -    DRM_MODE_OBJECT_CRTC)
> > > ;
> > > - igt_assert(props);
> > > -
> > > - for (i = 0; i < props->count_props; i++) {
> > > - uint32_t *prop_ids = crtc->state->desc-
> > > >props_crtc;
> > > -
> > > - if (props->props[i] ==
> > > prop_ids[IGT_CRTC_MODE_ID]) {
> > > - drmModePropertyBlobPtr blob;
> > > -
> > > - crtc->mode.id = props->prop_values[i];
> > > - if (!crtc->mode.id) {
> > > - crtc->mode.len = 0;
> > > - continue;
> > > - }
> > > -
> > > - blob = drmModeGetPropertyBlob(crtc-
> > > >state-
> > > >
> > > > desc->fd,
> > > -       crtc-
> > > >
> > > > mode.id);
> > > - igt_assert(blob);
> > > - igt_assert_eq_u32(blob->length,
> > > -   sizeof(struct
> > > drm_mode_modeinfo));
> > > -
> > > - if (!crtc->mode.data ||
> > > -     memcmp(crtc->mode.data, blob->data,
> > > blob->length) != 0)
> > > - crtc->mode.data = blob->data;
> > > - crtc->mode.len = blob->length;
> > > - }
> > > - else if (props->props[i] ==
> > > prop_ids[IGT_CRTC_ACTIVE]) {
> > > - crtc->active = props->prop_values[i];
> > > + for (i = 0; i < IGT_NUM_CRTC_PROPS; i++) {
> > > + if (crtc_filter(i)) {
> > > + values[i] = 0;
> > > + continue;
> > >   }
> > > - }
> > >
> > > - drmModeFreeObjectProperties(props);
> > > + values[i] = igt_pipe_obj_get_prop(pipe, i);
> > > + }
> > >  }
> > >
> > > -static void crtc_check_current_state(struct
> > > kms_atomic_crtc_state
> > > *crtc,
> > > -      struct
> > > kms_atomic_plane_state
> > > *primary,
> > > +static void crtc_check_current_state(igt_pipe_t *pipe,
> > > +      const uint64_t
> > > *pipe_values,
> > > +      const uint64_t
> > > *primary_values,
> > >        enum kms_atomic_check_relax
> > > relax)
> > >  {
> > > - struct kms_atomic_crtc_state crtc_kernel;
> > > + uint64_t current_pipe_values[IGT_NUM_CRTC_PROPS];
> > >   drmModeCrtcPtr legacy;
> > > + drmModePropertyBlobRes *mode_prop = NULL;
> > > + struct drm_mode_modeinfo *mode = NULL;
> > >
> > > - legacy = drmModeGetCrtc(crtc->state->desc->fd, crtc-
> > > >obj);
> > > - igt_assert(legacy);
> > > -
> > > - igt_assert_eq_u32(legacy->crtc_id, crtc->obj);
> > > - igt_assert_eq_u32(legacy->x, primary->src_x >> 16);
> > > - igt_assert_eq_u32(legacy->y, primary->src_y >> 16);
> > > -
> > > - if (crtc->active)
> > > - igt_assert_eq_u32(legacy->buffer_id, primary-
> > > >
> > > > fb_id);
> > > - else
> > > - igt_assert_eq_u32(legacy->buffer_id, 0);
> > > + if (pipe_values[IGT_CRTC_MODE_ID]) {
> > > + mode_prop = drmModeGetPropertyBlob(pipe-
> > > >display-
> > > >
> > > > drm_fd,
> > > +    pipe_values[I
> > > GT_C
> > > RTC_MODE_ID]);
> > >
> > > - if (legacy->mode_valid) {
> > > - igt_assert_neq(legacy->mode_valid, 0);
> > > - igt_assert_eq(crtc->mode.len,
> > > -               sizeof(struct drm_mode_modeinfo));
> > > - do_or_die(memcmp(&legacy->mode, crtc->mode.data,
> > > -                  crtc->mode.len));
> > > - igt_assert_eq(legacy->width, legacy-
> > > >mode.hdisplay);
> > > - igt_assert_eq(legacy->height, legacy-
> > > >
> > > > mode.vdisplay);
> > > - } else {
> > > - igt_assert_eq(legacy->mode_valid, 0);
> > > - }
> > > + igt_assert(mode_prop);
> > >
> > > - memcpy(&crtc_kernel, crtc, sizeof(crtc_kernel));
> > > - crtc_get_current_state(&crtc_kernel);
> > > -
> > > - if (crtc_kernel.mode.id != 0)
> > > - igt_assert_eq(crtc_kernel.mode.len,
> > > + igt_assert_eq(mode_prop->length,
> > >                 sizeof(struct drm_mode_modeinfo));
> > > -
> > > - /* Optionally relax the check for MODE_ID: using the
> > > legacy
> > > SetCrtc
> > > -  * API can potentially change MODE_ID even if the mode
> > > itself remains
> > > -  * unchanged. */
> > > - if (((relax & CRTC_RELAX_MODE) &&
> > > -     (crtc_kernel.mode.id != crtc->mode.id &&
> > > -      crtc_kernel.mode.id != 0 && crtc->mode.id != 0)) &&
> > > -     memcmp(crtc_kernel.mode.data, crtc->mode.data,
> > > -    sizeof(struct drm_mode_modeinfo)) == 0) {
> > > - crtc_kernel.mode.id = crtc->mode.id;
> > > - crtc_kernel.mode.data = crtc->mode.data;
> > > - }
> > > -
> > > - do_or_die(memcmp(&crtc_kernel, crtc,
> > > sizeof(crtc_kernel)));
> > > -
> > > - drmModeFreeCrtc(legacy);
> > > -}
> > > -
> > > -static void crtc_commit_legacy(struct kms_atomic_crtc_state
> > > *crtc,
> > > -        struct kms_atomic_plane_state
> > > *plane,
> > > -        enum kms_atomic_check_relax
> > > relax)
> > > -{
> > > - drmModeObjectPropertiesPtr props;
> > > - uint32_t *connectors;
> > > - int num_connectors = 0;
> > > - int i;
> > > -
> > > - if (!crtc->active) {
> > > - do_or_die(drmModeSetCrtc(crtc->state->desc->fd,
> > > -  crtc->obj, 0, 0, 0,
> > > NULL,
> > > 0, NULL));
> > > - return;
> > > - }
> > > -
> > > - connectors = calloc(crtc->state->num_connectors,
> > > -     sizeof(*connectors));
> > > - igt_assert(connectors);
> > > -
> > > - igt_assert_neq_u32(crtc->mode.id, 0);
> > > -
> > > - for (i = 0; i < crtc->state->num_connectors; i++) {
> > > - struct kms_atomic_connector_state *connector =
> > > - &crtc->state->connectors[i];
> > > -
> > > - if (connector->crtc_id != crtc->obj)
> > > - continue;
> > > -
> > > - connectors[num_connectors++] = connector->obj;
> > > + mode = mode_prop->data;
> > >   }
> > >
> > > - do_or_die(drmModeSetCrtc(crtc->state->desc->fd, crtc-
> > > >obj,
> > > -                          plane->fb_id,
> > > -  plane->src_x >> 16, plane-
> > > >src_y >>
> > > 16,
> > > -  (num_connectors) ? connectors :
> > > NULL,
> > > -  num_connectors,
> > > -  crtc->mode.data));
> > > - /* When doing a legacy commit, the core may update
> > > MODE_ID
> > > to be a new
> > > -  * blob implicitly created by the legacy request. Hence
> > > we
> > > backfill
> > > -  * the value in the state object to ensure they match.
> > > */
> > > - props = drmModeObjectGetProperties(crtc->state->desc-
> > > >fd,
> > > crtc->obj,
> > > -    DRM_MODE_OBJECT_CRTC)
> > > ;
> > > - igt_assert(props);
> > > -
> > > - for (i = 0; i < props->count_props; i++) {
> > > - if (props->props[i] !=
> > > -     crtc->state->desc-
> > > >props_crtc[IGT_CRTC_MODE_ID])
> > > - continue;
> > > - crtc->mode.id = props->prop_values[i];
> > > - break;
> > > - }
> > > + legacy = drmModeGetCrtc(pipe->display->drm_fd, pipe-
> > > >
> > > > crtc_id);
> > > + igt_assert(legacy);
> > >
> > > - drmModeFreeObjectProperties(props);
> > > + igt_assert_eq_u32(legacy->crtc_id, pipe->crtc_id);
> > > + igt_assert_eq_u32(legacy->x,
> > > primary_values[IGT_PLANE_SRC_X]
> > > >
> > > > >
> > > > > 16);
> > > + igt_assert_eq_u32(legacy->y,
> > > primary_values[IGT_PLANE_SRC_Y]
> > > >
> > > > >
> > > > > 16);
> > >
> > > - crtc_check_current_state(crtc, plane, relax);
> > > - plane_check_current_state(plane, relax);
> > > -}
> > > + igt_assert_eq_u32(legacy->buffer_id,
> > > primary_values[IGT_PLANE_FB_ID]);
> > >
> > > -static struct kms_atomic_crtc_state *find_crtc(struct
> > > kms_atomic_state *state,
> > > -        bool
> > > must_be_enabled)
> > > -{
> > > - int i;
> > > + if (legacy->mode_valid) {
> > > + igt_assert(mode_prop);
> > >
> > > - for (i = 0; i < state->num_crtcs; i++) {
> > > - struct kms_atomic_crtc_state *crtc = &state-
> > > >
> > > > crtcs[i];
> > > + do_or_die(memcmp(&legacy->mode, mode,
> > > sizeof(*mode)));
> > >
> > > - if (!crtc->obj)
> > > - continue;
> > > - if (must_be_enabled && !crtc->active)
> > > - continue;
> > > + igt_assert_eq(legacy->width, legacy-
> > > >mode.hdisplay);
> > > + igt_assert_eq(legacy->height, legacy-
> > > >
> > > > mode.vdisplay);
> > >
> > > - crtc_get_current_state(crtc);
> > > - return crtc;
> > > + igt_assert_neq(pipe_values[IGT_CRTC_MODE_ID],
> > > 0);
> > > + } else {
> > > + igt_assert(!mode_prop);
> > >   }
> > >
> > > - return NULL;
> > > -}
> > > + crtc_get_current_state(pipe, current_pipe_values);
> > >
> > > -static void fill_obj_props(int fd, uint32_t id, int type, int
> > > num_props,
> > > -    const char **prop_names, uint32_t
> > > *prop_ids)
> > > -{
> > > - drmModeObjectPropertiesPtr props;
> > > - int i, j;
> > > -
> > > - props = drmModeObjectGetProperties(fd, id, type);
> > > - igt_assert(props);
> > > + /* Optionally relax the check for MODE_ID: using the
> > > legacy
> > > SetCrtc
> > > +  * API can potentially change MODE_ID even if the mode
> > > itself remains
> > > +  * unchanged. */
> > > + if (relax & CRTC_RELAX_MODE && mode &&
> > > current_pipe_values[IGT_CRTC_MODE_ID] &&
> > > +     current_pipe_values[IGT_CRTC_MODE_ID] !=
> > > pipe_values[IGT_CRTC_MODE_ID]) {
> > > + drmModePropertyBlobRes *cur_prop =
> > > + drmModeGetPropertyBlob(pipe->display-
> > > >
> > > > drm_fd,
> > > +        current_pipe_valu
> > > es[I
> > > GT_CRTC_MODE_ID]);
> > >
> > > - for (i = 0; i < props->count_props; i++) {
> > > - drmModePropertyPtr prop =
> > > - drmModeGetProperty(fd, props->props[i]);
> > > + igt_assert(cur_prop);
> > > + igt_assert_eq(cur_prop->length, sizeof(struct
> > > drm_mode_modeinfo));
> > >
> > > - for (j = 0; j < num_props; j++) {
> > > - if (strcmp(prop->name, prop_names[j]) !=
> > > 0)
> > > - continue;
> > > - prop_ids[j] = props->props[i];
> > > - break;
> > > - }
> > > + if (!memcmp(cur_prop->data, mode,
> > > sizeof(*mode)))
> > > + current_pipe_values[IGT_CRTC_MODE_ID] =
> > > pipe_values[IGT_CRTC_MODE_ID];
> > >
> > > - drmModeFreeProperty(prop);
> > > + drmModeFreePropertyBlob(cur_prop);
> > >   }
> > >
> > > - drmModeFreeObjectProperties(props);
> > > -}
> > > + do_or_die(memcmp(pipe_values, current_pipe_values,
> > > sizeof(current_pipe_values)));
> > >
> > > -static void fill_obj_prop_map(int fd, uint32_t id, int type,
> > > const
> > > char *name,
> > > -       int num_enums, const char
> > > **enum_names,
> > > -       uint64_t *enum_ids)
> > > -{
> > > - drmModeObjectPropertiesPtr props;
> > > - int i, j, k;
> > > -
> > > - props = drmModeObjectGetProperties(fd, id, type);
> > > - igt_assert(props);
> > > -
> > > - for (i = 0; i < props->count_props; i++) {
> > > - drmModePropertyPtr prop =
> > > - drmModeGetProperty(fd, props->props[i]);
> > > -
> > > - igt_assert(prop);
> > > -
> > > - if (strcmp(prop->name, name) != 0) {
> > > - drmModeFreeProperty(prop);
> > > - continue;
> > > - }
> > > -
> > > - for (j = 0; j < prop->count_enums; j++) {
> > > - struct drm_mode_property_enum *e =
> > > &prop-
> > > >
> > > > enums[j];
> > > -
> > > - for (k = 0; k < num_enums; k++) {
> > > - if (strcmp(e->name,
> > > enum_names[k])
> > > != 0)
> > > - continue;
> > > -
> > > - enum_ids[k] = e->value;
> > > - break;
> > > - }
> > > - }
> > > -
> > > - drmModeFreeProperty(prop);
> > > - }
> > > + drmModeFreeCrtc(legacy);
> > > + drmModeFreePropertyBlob(mode_prop);
> > >  }
> > >
> > > -static void atomic_setup(struct kms_atomic_state *state)
> > > +static void crtc_commit(igt_pipe_t *pipe, igt_plane_t *plane,
> > > + enum igt_commit_style s,
> > > + enum kms_atomic_check_relax relax)
> > >  {
> > > - struct kms_atomic_desc *desc = state->desc;
> > > - drmModeResPtr res;
> > > - drmModePlaneResPtr res_plane;
> > > - int i;
> > > -
> > > - desc->fd = drm_open_driver_master(DRIVER_ANY);
> > > - igt_assert_fd(desc->fd);
> > > -
> > > - igt_skip_on(drmSetClientCap(desc->fd,
> > > DRM_CLIENT_CAP_ATOMIC,
> > > 1));
> > > -
> > > - res = drmModeGetResources(desc->fd);
> > > - res_plane = drmModeGetPlaneResources(desc->fd);
> > > - igt_assert(res);
> > > - igt_assert(res_plane);
> > > -
> > > - igt_assert_lt(0, res->count_crtcs);
> > > - state->num_crtcs = res->count_crtcs;
> > > - state->crtcs = calloc(state->num_crtcs, sizeof(*state-
> > > >
> > > > crtcs));
> > > - igt_assert(state->crtcs);
> > > -
> > > - igt_assert_lt(0, res_plane->count_planes);
> > > - state->num_planes = res_plane->count_planes;
> > > - state->planes = calloc(state->num_planes, sizeof(*state-
> > > >
> > > > planes));
> > > - igt_assert(state->planes);
> > > -
> > > - igt_assert_lt(0, res->count_connectors);
> > > - state->num_connectors = res->count_connectors;
> > > - state->connectors = calloc(state->num_connectors,
> > > -    sizeof(*state->connectors));
> > > - igt_assert(state->connectors);
> > > -
> > > - fill_obj_props(desc->fd, res->crtcs[0],
> > > -        DRM_MODE_OBJECT_CRTC, IGT_NUM_CRTC_PROPS,
> > > -        igt_crtc_prop_names, desc->props_crtc);
> > > -
> > > - fill_obj_props(desc->fd, res_plane->planes[0],
> > > -        DRM_MODE_OBJECT_PLANE,
> > > IGT_NUM_PLANE_PROPS,
> > > -        igt_plane_prop_names, desc->props_plane);
> > > - fill_obj_prop_map(desc->fd, res_plane->planes[0],
> > > -   DRM_MODE_OBJECT_PLANE, "type",
> > > -   NUM_PLANE_TYPE_PROPS,
> > > plane_type_prop_names,
> > > -   desc->props_plane_type);
> > > -
> > > - fill_obj_props(desc->fd, res->connectors[0],
> > > -        DRM_MODE_OBJECT_CONNECTOR,
> > > IGT_NUM_CONNECTOR_PROPS,
> > > -        igt_connector_prop_names, desc-
> > > >
> > > > props_connector);
> > > -
> > > - for (i = 0; i < state->num_crtcs; i++) {
> > > - struct kms_atomic_crtc_state *crtc = &state-
> > > >
> > > > crtcs[i];
> > > -
> > > - crtc->state = state;
> > > - crtc->obj = res->crtcs[i];
> > > - crtc->idx = i;
> > > - crtc_get_current_state(crtc);
> > > -
> > > - /* The blob pointed to by MODE_ID could well be
> > > transient,
> > > -  * and lose its last reference as we switch away
> > > from it.
> > > -  * Duplicate the blob here so we have a
> > > reference we
> > > know we
> > > -  * own. */
> > > - if (crtc->mode.id != 0)
> > > -     crtc->mode.id = blob_duplicate(desc->fd,
> > > crtc-
> > > >
> > > > mode.id);
> > > - }
> > > + igt_display_commit2(pipe->display, s);
> > >
> > > - for (i = 0; i < state->num_planes; i++) {
> > > - drmModePlanePtr plane =
> > > - drmModeGetPlane(desc->fd, res_plane-
> > > >
> > > > planes[i]);
> > > - igt_assert(plane);
> > > -
> > > - state->planes[i].state = state;
> > > - state->planes[i].obj = res_plane->planes[i];
> > > - state->planes[i].crtc_mask = plane-
> > > >possible_crtcs;
> > > - plane_get_current_state(&state->planes[i]);
> > > - }
> > > -
> > > - for (i = 0; i < state->num_connectors; i++) {
> > > - state->connectors[i].state = state;
> > > - state->connectors[i].obj = res->connectors[i];
> > > - connector_get_current_state(&state-
> > > >connectors[i]);
> > > - }
> > > -
> > > - drmModeFreePlaneResources(res_plane);
> > > - drmModeFreeResources(res);
> > > + crtc_check_current_state(pipe, pipe->values, plane-
> > > >values,
> > > relax);
> > > + plane_check_current_state(plane, plane->values, relax);
> > >  }
> > >
> > > -static struct kms_atomic_state *
> > > -atomic_state_dup(const struct kms_atomic_state *state)
> > > +static void crtc_commit_atomic_flags_err(igt_pipe_t *pipe,
> > > igt_plane_t *plane,
> > > +  unsigned flags,
> > > +  enum
> > > kms_atomic_check_relax
> > > relax,
> > > +  int err)
> > >  {
> > > - struct kms_atomic_state *ret = calloc(1, sizeof(*ret));
> > > -
> > > - igt_assert(ret);
> > > - *ret = *state;
> > > + uint64_t current_pipe_values[IGT_NUM_CRTC_PROPS];
> > > + uint64_t current_plane_values[IGT_NUM_PLANE_PROPS];
> > >
> > > - ret->crtcs = calloc(ret->num_crtcs, sizeof(*ret-
> > > >crtcs));
> > > - igt_assert(ret->crtcs);
> > > - memcpy(ret->crtcs, state->crtcs, ret->num_crtcs *
> > > sizeof(*ret->crtcs));
> > > + crtc_get_current_state(pipe, current_pipe_values);
> > > + plane_get_current_state(plane, current_plane_values);
> > >
> > > - ret->planes = calloc(ret->num_planes, sizeof(*ret-
> > > >planes));
> > > - igt_assert(ret->planes);
> > > - memcpy(ret->planes, state->planes,
> > > -        ret->num_planes * sizeof(*ret->planes));
> > > + igt_assert_eq(-err, igt_display_try_commit_atomic(pipe-
> > > >
> > > > display, flags, NULL));
> > >
> > > - ret->connectors = calloc(ret->num_connectors,
> > > sizeof(*ret-
> > > >
> > > > connectors));
> > > - igt_assert(ret->connectors);
> > > - memcpy(ret->connectors, state->connectors,
> > > -        ret->num_connectors * sizeof(*ret->connectors));
> > > -
> > > - return ret;
> > > + crtc_check_current_state(pipe, current_pipe_values,
> > > current_plane_values, relax);
> > > + plane_check_current_state(plane, current_plane_values,
> > > relax);
> > >  }
> > >
> > > -static void atomic_state_free(struct kms_atomic_state *state)
> > > -{
> > > - free(state->crtcs);
> > > - free(state->planes);
> > > - free(state->connectors);
> > > - free(state);
> > > -}
> > > +#define crtc_commit_atomic_err(pipe, plane, relax, err) \
> > > + crtc_commit_atomic_flags_err(pipe, plane,
> > > DRM_MODE_ATOMIC_ALLOW_MODESET, relax, err)
> > >
> > > -static uint32_t plane_get_igt_format(struct
> > > kms_atomic_plane_state
> > > *plane)
> > > +static uint32_t plane_get_igt_format(igt_plane_t *plane)
> > >  {
> > >   drmModePlanePtr plane_kms;
> > >   const uint32_t *igt_formats;
> > > - uint32_t ret = 0;
> > >   int num_igt_formats;
> > >   int i;
> > >
> > > - plane_kms = drmModeGetPlane(plane->state->desc->fd,
> > > plane-
> > > >
> > > > obj);
> > > - igt_assert(plane_kms);
> > > + plane_kms = plane->drm_plane;
> > >
> > >   igt_get_all_cairo_formats(&igt_formats,
> > > &num_igt_formats);
> > >   for (i = 0; i < num_igt_formats; i++) {
> > >   int j;
> > >
> > >   for (j = 0; j < plane_kms->count_formats; j++) {
> > > - if (plane_kms->formats[j] ==
> > > igt_formats[i])
> > > {
> > > - ret = plane_kms->formats[j];
> > > - break;
> > > - }
> > > + if (plane_kms->formats[j] ==
> > > igt_formats[i])
> > > + return plane_kms->formats[j];
> > >   }
> > >   }
> > >
> > > - drmModeFreePlane(plane_kms);
> > > - return ret;
> > > + return 0;
> > >  }
> > >
> > >  static void
> > > -set_dpms(int fd, int mode)
> > > +set_dpms(igt_output_t *output, int mode)
> > >  {
> > > - int i;
> > > - drmModeConnector *connector;
> > > - uint32_t id;
> > > - drmModeRes *resources = drmModeGetResources(fd);
> > > -
> > > - for (i = 0; i < resources->count_connectors; i++) {
> > > - id = resources->connectors[i];
> > > -
> > > - connector = drmModeGetConnectorCurrent(fd, id);
> > > -
> > > - kmstest_set_connector_dpms(fd, connector, mode);
> > > -
> > > - drmModeFreeConnector(connector);
> > > - }
> > > + do_or_die(drmModeConnectorSetProperty(output->display-
> > > >
> > > > drm_fd, output->id,
> > > +       output-
> > > >
> > > > props[IGT_CONNECTOR_DPMS], mode));
> > >  }
> > >
> > > -static void plane_overlay(struct kms_atomic_crtc_state *crtc,
> > > -   struct kms_atomic_plane_state
> > > *plane_old)
> > > +static void plane_overlay(igt_pipe_t *pipe, igt_output_t
> > > *output,
> > > igt_plane_t *plane)
> > >  {
> > > - struct drm_mode_modeinfo *mode = crtc->mode.data;
> > > - struct kms_atomic_plane_state plane = *plane_old;
> > > - uint32_t format = plane_get_igt_format(&plane);
> > > - drmModeAtomicReq *req = drmModeAtomicAlloc();
> > > + drmModeModeInfo *mode = igt_output_get_mode(output);
> > > + uint32_t format = plane_get_igt_format(plane);
> > >   struct igt_fb fb;
> > > + uint32_t w = mode->hdisplay / 2;
> > > + uint32_t h = mode->vdisplay / 2;
> > >
> > > - igt_require(req);
> > >   igt_require(format != 0);
> > >
> > > - plane.src_x = 0;
> > > - plane.src_y = 0;
> > > - plane.src_w = (mode->hdisplay / 2) << 16;
> > > - plane.src_h = (mode->vdisplay / 2) << 16;
> > > - plane.crtc_x = mode->hdisplay / 4;
> > > - plane.crtc_y = mode->vdisplay / 4;
> > > - plane.crtc_w = mode->hdisplay / 2;
> > > - plane.crtc_h = mode->vdisplay / 2;
> > > - plane.crtc_id = crtc->obj;
> > > - plane.fb_id = igt_create_pattern_fb(plane.state->desc-
> > > >fd,
> > > -     plane.crtc_w,
> > > plane.crtc_h,
> > > -     format,
> > > I915_TILING_NONE, &fb);
> > > + igt_create_pattern_fb(pipe->display->drm_fd, w, h,
> > > +       format, I915_TILING_NONE, &fb);
> > > +
> > > + igt_plane_set_fb(plane, &fb);
> > > + igt_plane_set_position(plane, w/2, h/2);
> > >
> > >   /* Enable the overlay plane using the atomic API, and
> > > double-check
> > >    * state is what we think it should be. */
> > > - plane_commit_atomic(&plane, req, ATOMIC_RELAX_NONE);
> > > + plane_commit(plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
> > >
> > >   /* Disable the plane and check the state matches the
> > > old. */
> > > - plane_commit_atomic(plane_old, req, ATOMIC_RELAX_NONE);
> > > + igt_plane_set_fb(plane, NULL);
> > > + igt_plane_set_position(plane, 0, 0);
> > > + plane_commit(plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
> > >
> > >   /* Re-enable the plane through the legacy plane API, and
> > > verify through
> > >    * atomic. */
> > > - plane_commit_legacy(&plane, ATOMIC_RELAX_NONE);
> > > + igt_plane_set_fb(plane, &fb);
> > > + igt_plane_set_position(plane, w/2, h/2);
> > > + plane_commit(plane, COMMIT_LEGACY, ATOMIC_RELAX_NONE);
> > >
> > >   /* Restore the plane to its original settings through
> > > the
> > > legacy plane
> > >    * API, and verify through atomic. */
> > > - plane_commit_legacy(plane_old, ATOMIC_RELAX_NONE);
> > > + igt_plane_set_fb(plane, NULL);
> > > + igt_plane_set_position(plane, 0, 0);
> > > + plane_commit(plane, COMMIT_LEGACY, ATOMIC_RELAX_NONE);
> > >
> > > - drmModeAtomicFree(req);
> > > + igt_remove_fb(pipe->display->drm_fd, &fb);
> > >  }
> > >
> > > -static void plane_primary(struct kms_atomic_crtc_state *crtc,
> > > -   struct kms_atomic_plane_state
> > > *plane_old)
> > > +static void plane_primary(igt_pipe_t *pipe, igt_plane_t *plane,
> > > struct igt_fb *fb)
> > >  {
> > > - struct drm_mode_modeinfo *mode = crtc->mode.data;
> > > - struct kms_atomic_plane_state plane = *plane_old;
> > > - uint32_t format = plane_get_igt_format(&plane);
> > > - drmModeAtomicReq *req = drmModeAtomicAlloc();
> > > - struct igt_fb fb;
> > > - uint32_t flags = 0;
> > > - int ret;
> > > -
> > > - igt_require(format != 0);
> > > + struct igt_fb fb2;
> > >
> > > - plane.src_x = 0;
> > > - plane.src_y = 0;
> > > - plane.src_w = mode->hdisplay << 16;
> > > - plane.src_h = mode->vdisplay << 16;
> > > - plane.crtc_x = 0;
> > > - plane.crtc_y = 0;
> > > - plane.crtc_w = mode->hdisplay;
> > > - plane.crtc_h = mode->vdisplay;
> > > - plane.crtc_id = crtc->obj;
> > > - plane.fb_id = igt_create_pattern_fb(plane.state->desc-
> > > >fd,
> > > -     plane.crtc_w,
> > > plane.crtc_h,
> > > -     format,
> > > I915_TILING_NONE, &fb);
> > > -
> > > - drmModeAtomicSetCursor(req, 0);
> > > - crtc_populate_req(crtc, req);
> > > - plane_populate_req(&plane, req);
> > > - ret = drmModeAtomicCommit(crtc->state->desc->fd, req,
> > > -   DRM_MODE_ATOMIC_TEST_ONLY,
> > > NULL);
> > > - /* Try harder in case the failure is caused by
> > > disallowing
> > > modeset. */
> > > - if (ret == -EINVAL)
> > > - flags |= DRM_MODE_ATOMIC_ALLOW_MODESET;
> > > + igt_create_color_pattern_fb(pipe->display->drm_fd,
> > > +     fb->width, fb->height,
> > > +     fb->drm_format,
> > > I915_TILING_NONE,
> > > +     0.2, 0.2, 0.2, &fb2);
> > >
> > >   /* Flip the primary plane using the atomic API, and
> > > double-
> > > check
> > >    * state is what we think it should be. */
> > > - crtc_commit_atomic(crtc, &plane, req, ATOMIC_RELAX_NONE,
> > > flags);
> > > + igt_plane_set_fb(plane, &fb2);
> > > + crtc_commit(pipe, plane, COMMIT_ATOMIC,
> > > ATOMIC_RELAX_NONE);
> > >
> > >   /* Restore the primary plane and check the state matches
> > > the
> > > old. */
> > > - crtc_commit_atomic(crtc, plane_old, req,
> > > ATOMIC_RELAX_NONE,
> > > flags);
> > > + igt_plane_set_fb(plane, fb);
> > > + crtc_commit(pipe, plane, COMMIT_ATOMIC,
> > > ATOMIC_RELAX_NONE);
> > >
> > > - /* Re-enable the plane through the legacy CRTC/primary-
> > > plane
> > > API, and
> > > + /* Set the plane through the legacy CRTC/primary-plane
> > > API,
> > > and
> > >    * verify through atomic. */
> > > - crtc_commit_legacy(crtc, &plane, CRTC_RELAX_MODE);
> > > + igt_plane_set_fb(plane, &fb2);
> > > + crtc_commit(pipe, plane, COMMIT_LEGACY,
> > > CRTC_RELAX_MODE);
> > >
> > >   /* Restore the plane to its original settings through
> > > the
> > > legacy CRTC
> > >    * API, and verify through atomic. */
> > > - crtc_commit_legacy(crtc, plane_old, CRTC_RELAX_MODE);
> > > -
> > > - /* Finally, restore to the original state. */
> > > - crtc_commit_atomic(crtc, plane_old, req,
> > > ATOMIC_RELAX_NONE,
> > > flags);
> > > + igt_plane_set_fb(plane, fb);
> > > + crtc_commit(pipe, plane, COMMIT_LEGACY,
> > > CRTC_RELAX_MODE);
> > >
> > > - drmModeAtomicFree(req);
> > > + /* Set the plane through the universal setplane API, and
> > > +  * verify through atomic. */
> > > + igt_plane_set_fb(plane, &fb2);
> > > + plane_commit(plane, COMMIT_UNIVERSAL,
> > > ATOMIC_RELAX_NONE);
> > >  }
> > >
> > >  /* test to ensure that DRM_MODE_ATOMIC_TEST_ONLY really only
> > > touches
> > > the
> > >   * free-standing state objects and nothing else.
> > >   */
> > > -static void test_only(struct kms_atomic_crtc_state *crtc,
> > > -       struct kms_atomic_plane_state *plane_old)
> > > +static void test_only(igt_pipe_t *pipe_obj,
> > > +       igt_plane_t *primary,
> > > +       igt_output_t *output)
> > >  {
> > > - struct drm_mode_modeinfo *mode = crtc->mode.data;
> > > - struct kms_atomic_plane_state plane = *plane_old;
> > > - uint32_t format = plane_get_igt_format(&plane);
> > > - drmModeAtomicReq *req = drmModeAtomicAlloc();
> > > + drmModeModeInfo *mode = igt_output_get_mode(output);
> > > + uint32_t format = plane_get_igt_format(primary);
> > >   struct igt_fb fb;
> > > - int ret;
> > > + uint64_t old_plane_values[IGT_NUM_PLANE_PROPS],
> > > old_crtc_values[IGT_NUM_CRTC_PROPS];
> > >
> > >   igt_require(format != 0);
> > >
> > > - plane.src_x = 0;
> > > - plane.src_y = 0;
> > > - plane.src_w = mode->hdisplay << 16;
> > > - plane.src_h = mode->vdisplay << 16;
> > > - plane.crtc_x = 0;
> > > - plane.crtc_y = 0;
> > > - plane.crtc_w = mode->hdisplay;
> > > - plane.crtc_h = mode->vdisplay;
> > > - plane.crtc_id = crtc->obj;
> > > - plane.fb_id = igt_create_pattern_fb(plane.state->desc-
> > > >fd,
> > > -     plane.crtc_w,
> > > plane.crtc_h,
> > > -     format,
> > > I915_TILING_NONE, &fb);
> > > -
> > > - drmModeAtomicSetCursor(req, 0);
> > > - crtc_populate_req(crtc, req);
> > > - plane_populate_req(&plane, req);
> > > - ret = drmModeAtomicCommit(crtc->state->desc->fd, req,
> > > -   DRM_MODE_ATOMIC_TEST_ONLY,
> > > NULL);
> > > -
> > > - igt_assert_eq(ret, 0);
> > > -
> > > - /* go through dpms off/on cycle */
> > > - set_dpms(crtc->state->desc->fd, DRM_MODE_DPMS_OFF);
> > > - set_dpms(crtc->state->desc->fd, DRM_MODE_DPMS_ON);
> > > -
> > > - /* check the state */
> > > - crtc_check_current_state(crtc, plane_old,
> > > ATOMIC_RELAX_NONE);
> > > - plane_check_current_state(plane_old, ATOMIC_RELAX_NONE);
> > > -
> > > - /* Re-enable the plane through the legacy CRTC/primary-
> > > plane
> > > API, and
> > > -  * verify through atomic. */
> > > - crtc_commit_legacy(crtc, plane_old, CRTC_RELAX_MODE);
> > > + plane_get_current_state(primary, old_plane_values);
> > > + crtc_get_current_state(pipe_obj, old_crtc_values);
> > > +
> > > + igt_assert(!old_crtc_values[IGT_CRTC_MODE_ID]);
> > > +
> > > + igt_create_pattern_fb(pipe_obj->display->drm_fd,
> > > +      mode->hdisplay, mode->vdisplay,
> > > +      format, I915_TILING_NONE, &fb);
> > > + igt_plane_set_fb(primary, &fb);
> > > + igt_output_set_pipe(output, pipe_obj->pipe);
> > > +
> > > + igt_display_commit_atomic(pipe_obj->display,
> > > DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> > > +
> > > + /* check the state, should still be old state */
> > > + crtc_check_current_state(pipe_obj, old_crtc_values,
> > > old_plane_values, ATOMIC_RELAX_NONE);
> > > + plane_check_current_state(primary, old_plane_values,
> > > ATOMIC_RELAX_NONE);
> > > +
> > > + /*
> > > +  * Enable the plane through the legacy CRTC/primary-
> > > plane
> > > API, and
> > > +  * verify through atomic.
> > > +  */
> > > + crtc_commit(pipe_obj, primary, COMMIT_LEGACY,
> > > CRTC_RELAX_MODE);
> > > +
> > > + /* Same for disable.. */
> > > + plane_get_current_state(primary, old_plane_values);
> > > + crtc_get_current_state(pipe_obj, old_crtc_values);
> > > +
> > > + igt_plane_set_fb(primary, NULL);
> > > + igt_output_set_pipe(output, PIPE_NONE);
> > > +
> > > + igt_display_commit_atomic(pipe_obj->display,
> > > DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> > >
> > > - drmModeAtomicFree(req);
> > > + /* for extra stress, go through dpms off/on cycle */
> > > + set_dpms(output, DRM_MODE_DPMS_OFF);
> > > + set_dpms(output, DRM_MODE_DPMS_ON);
> > There is this library function 'kmstest_set_connector_dpms()'.
> > Could we
> > utilize this function here instead?
> Sure, I never understood what the dpms call was supposed to expose
> though..
>
> Want me to resend or is replacing the call with
> kmstest_set_connector_dpms(s, output->config.connector,
> DRM_MODE_DPMS_OFF); enough?
Well, my point of view is that we should use library functions as much
as possible. I guess that's the reason why we have the libraries in the
first place. So a vote for a replacement.

--
Mika Kahola - Intel OTC