Re: [PATCH i-g-t] lib/igt_kms: Add COMIT_ATOMIC to igt_display_commit2()

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

Op 15-01-16 om 10:06 schreef Marius Vlad:
> 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@intel.com>
> ---
>  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);
Set crtc_id and fb_id to 0 when disabling plane.
> +		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);
One drmModeAtomicCommit per igt_display_commit2 is enough. :)

Patch is looking good otherwise, would be nice if we could get rid of the duplication with kms_atomic.c
That will make it a lot easier to add more tests in the future.

~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx