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

Maarten Lankhorst
2016-01-20T14:52:11Z

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