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

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

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.com>
>> 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_CONNECTOR
>> );
>> -	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[IGT_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_values[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?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx