[Intel-gfx] [PATCH i-g-t v7] lib/igt_kms: Add COMMIT_ATOMIC to igt_display_commit2()

Matthew Auld matthew.william.auld at gmail.com
Tue Mar 8 16:58:33 UTC 2016


I think it would be more idiomatic to replace:

+ for (i = 0; i < display->n_outputs; i++) {
+               igt_output_t *output = &display->outputs[i];

with:

+ for_each_connected_output(display, output)

And:

+ for (i = 0; i < pipe->n_planes; i++) {
+             igt_plane_t *plane = &pipe->planes[i];

with:

+ for_each_plane_on_pipe(display, pipe, plane)

On 7 March 2016 at 10:18, Pratik Vishwakarma
<pratik.vishwakarma at intel.com> wrote:
> From: Mayuresh Gharpure <mayuresh.s.gharpure at intel.com>
>
> Co-Author : Marius Vlad <marius.c.vlad at intel.com>
> Co-Author : Pratik Vishwakarma <pratik.vishwakarma at intel.com>
>
> So far we have had only two commit styles, COMMIT_LEGACY
> and COMMIT_UNIVERSAL. This patch adds another commit style
> COMMIT_ATOMIC which makes use of drmModeAtomicCommit()
>
> v2: (Marius)
>         i)Set CRTC_ID to zero while disabling plane
>         ii)Modified the log message in igt_atomic_prepare_plane_commit
>         https://patchwork.freedesktop.org/patch/71945/
>
> v3: (Marius)Set FB_ID to zero while disabling plane
>         https://patchwork.freedesktop.org/patch/72179/
>
> v4: (Maarten) Corrected the typo in commit message
>         https://patchwork.freedesktop.org/patch/72598/
>
> v5: Added check for DRM_CLIENT_CAP_ATOMIC in igt_display_init
>     (Marius)
>         i)Removed unused props from igt_display_init
>         ii)Removed unused ret. Changed function to void
>         iii)Declare the variable before checking if we have
>         DRM_CLIENT_CAP_ATOMIC.
>         https://patchwork.freedesktop.org/patch/73505/
>
> v6: (Jani) Corrected typo in commit message
>         https://patchwork.freedesktop.org/patch/74468/
>
> v7: (Vlad & Maarten) Added is_atomic check for DRM_CLIENT_CAP_ATOMIC
>         in igt_display_init and igt_atomic_commit
>
> Signed-off-by: Mayuresh Gharpure <mayuresh.s.gharpure at intel.com>
> Signed-off-by: Pratik Vishwakarma <pratik.vishwakarma at intel.com>
> ---
>  lib/igt_kms.c | 321 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  lib/igt_kms.h |  72 ++++++++++++-
>  2 files changed, 391 insertions(+), 2 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 5a61def..1f738e1 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -145,6 +145,120 @@ const unsigned char* igt_kms_get_base_edid(void)
>   *
>   * Returns: an alternate edid block
>   */
> +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",
> +       "rotation"
> +};
> +
> +static const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
> +       "background_color"
> +};
> +
> +static const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
> +       "scaling mode",
> +       "DPMS"
> +};
> +
> +/*
> + * 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 num_props, const char **prop_names)
> +{
> +       drmModeObjectPropertiesPtr props;
> +       int i, j, fd;
> +
> +       fd = display->drm_fd;
> +
> +       props = drmModeObjectGetProperties(fd, plane->drm_plane->plane_id, DRM_MODE_OBJECT_PLANE);
> +       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);
> +}
> +
> +/*
> + * Retrieve all the properies specified in props_name and store them into
> + * config->atomic_props_crtc and config->atomic_props_connector.
> + */
> +static void
> +igt_atomic_fill_props(igt_display_t *display, igt_output_t *output,
> +                       int num_crtc_props, const char **crtc_prop_names,
> +                       int num_connector_props, const char **conn_prop_names)
> +{
> +       drmModeObjectPropertiesPtr props;
> +       int i, j, fd;
> +
> +       fd = display->drm_fd;
> +
> +       props = drmModeObjectGetProperties(fd, output->config.crtc->crtc_id, DRM_MODE_OBJECT_CRTC);
> +       igt_assert(props);
> +
> +       for (i = 0; i < props->count_props; i++) {
> +               drmModePropertyPtr prop =
> +                       drmModeGetProperty(fd, props->props[i]);
> +
> +               for (j = 0; j < num_crtc_props; j++) {
> +                       if (strcmp(prop->name, crtc_prop_names[j]) != 0)
> +                               continue;
> +
> +                       output->config.atomic_props_crtc[j] = props->props[i];
> +                       break;
> +               }
> +
> +               drmModeFreeProperty(prop);
> +       }
> +
> +       drmModeFreeObjectProperties(props);
> +       props = NULL;
> +       props = drmModeObjectGetProperties(fd, output->config.connector->connector_id, DRM_MODE_OBJECT_CONNECTOR);
> +       igt_assert(props);
> +
> +       for (i = 0; i < props->count_props; i++) {
> +               drmModePropertyPtr prop =
> +                       drmModeGetProperty(fd, props->props[i]);
> +
> +               for (j = 0; j < num_connector_props; j++) {
> +                       if (strcmp(prop->name, conn_prop_names[j]) != 0)
> +                               continue;
> +
> +                       output->config.atomic_props_connector[j] = props->props[i];
> +                       break;
> +               }
> +
> +               drmModeFreeProperty(prop);
> +       }
> +
> +       drmModeFreeObjectProperties(props);
> +
> +}
> +
>  const unsigned char* igt_kms_get_alt_edid(void)
>  {
>         update_edid_csum(alt_edid);
> @@ -1002,6 +1116,8 @@ static void igt_output_refresh(igt_output_t *output)
>             kmstest_pipe_name(output->config.pipe));
>
>         display->pipes_in_use |= 1 << output->config.pipe;
> +       igt_atomic_fill_props(display, output, IGT_NUM_CRTC_PROPS, igt_crtc_prop_names,
> +               IGT_NUM_CONNECTOR_PROPS, igt_connector_prop_names);
>  }
>
>  static bool
> @@ -1071,6 +1187,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
>         drmModeRes *resources;
>         drmModePlaneRes *plane_resources;
>         int i;
> +       int ret = 0;
>
>         memset(display, 0, sizeof(igt_display_t));
>
> @@ -1088,6 +1205,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
>         display->n_pipes = resources->count_crtcs;
>
>         drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> +       ret = drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1);
>         plane_resources = drmModeGetPlaneResources(display->drm_fd);
>         igt_assert(plane_resources);
>
> @@ -1144,6 +1262,10 @@ void igt_display_init(igt_display_t *display, int drm_fd)
>
>                         plane->pipe = pipe;
>                         plane->drm_plane = drm_plane;
> +                       if (ret == 0) {
> +                               display->is_atomic = true;
> +                               igt_atomic_fill_plane_props(display, plane, IGT_NUM_PLANE_PROPS, igt_plane_prop_names);
> +                       }
>
>                         get_plane_property(display->drm_fd, drm_plane->plane_id,
>                                            "rotation",
> @@ -1399,6 +1521,111 @@ static uint32_t igt_plane_get_fb_gem_handle(igt_plane_t *plane)
>         igt_assert(r == 0);     \
>  }
>
> +
> +
> +
> +/*
> + * Add position and fb changes of a plane to the atomic property set
> + */
> +static void
> +igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output,
> +       drmModeAtomicReq *req)
> +{
> +
> +       igt_display_t *display = output->display;
> +       uint32_t fb_id, crtc_id;
> +       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;
> +
> +       igt_assert(plane->drm_plane);
> +
> +       /* 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: populating plane data: pipe %s, plane %d, disabling\n",
> +                    igt_output_name(output),
> +                    kmstest_pipe_name(output->config.pipe),
> +                    plane->index);
> +
> +               /* populate plane req */
> +               igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, 0);
> +               igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, 0);
> +
> +               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);
> +
> +       } else if (plane->fb_changed || plane->position_changed ||
> +                       plane->size_changed) {
> +
> +               src_x = IGT_FIXED(plane->fb->src_x, 0); /* src_x */
> +               src_y = IGT_FIXED(plane->fb->src_y, 0); /* src_y */
> +               src_w = IGT_FIXED(plane->fb->src_w, 0); /* src_w */
> +               src_h = IGT_FIXED(plane->fb->src_h, 0); /* src_h */
> +               crtc_x = plane->crtc_x;
> +               crtc_y = plane->crtc_y;
> +               crtc_w = plane->crtc_w;
> +               crtc_h = plane->crtc_h;
> +
> +               LOG(display,
> +                   "%s: populating plane data: %s.%d, fb %u, src = (%d, %d) "
> +                   "%ux%u dst = (%u, %u) %ux%u\n",
> +                   igt_output_name(output),
> +                   kmstest_pipe_name(output->config.pipe),
> +                   plane->index,
> +                   fb_id,
> +                   src_x >> 16, src_y >> 16, src_w >> 16, src_h >> 16,
> +                   crtc_x, crtc_y, crtc_w, crtc_h);
> +
> +
> +               /* populate plane req */
> +               igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, crtc_id);
> +               igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, fb_id);
> +
> +               igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_X, src_x);
> +               igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_Y, src_y);
> +               igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_W, src_w);
> +               igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_H, src_h);
> +
> +               igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_X, crtc_x);
> +               igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_Y, crtc_y);
> +               igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_W, crtc_w);
> +               igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_H, crtc_h);
> +
> +               if (plane->rotation_changed)
> +                       igt_atomic_populate_plane_req(req, plane,
> +                               IGT_PLANE_ROTATION, plane->rotation);
> +
> +       }
> +
> +       plane->fb_changed = false;
> +       plane->position_changed = false;
> +       plane->size_changed = false;
> +
> +       return;
> +}
> +
> +
> +
>  /*
>   * Commit position and fb changes to a DRM plane via the SetPlane ioctl; if the
>   * DRM call to program the plane fails, we'll either fail immediately (for
> @@ -1651,7 +1878,7 @@ static int igt_plane_commit(igt_plane_t *plane,
>                 return igt_primary_plane_commit_legacy(plane, output,
>                                                        fail_on_error);
>         } else {
> -               return igt_drm_plane_commit(plane, output, fail_on_error);
> +                       return igt_drm_plane_commit(plane, output, fail_on_error);
>         }
>  }
>
> @@ -1705,6 +1932,90 @@ static int igt_output_commit(igt_output_t *output,
>         return 0;
>  }
>
> +
> +/*
> + * Add crtc property changes to the atomic property set
> + */
> +static void igt_atomic_prepare_crtc_commit(igt_output_t *output, drmModeAtomicReq *req)
> +{
> +
> +       igt_pipe_t *pipe_obj = igt_output_get_driving_pipe(output);
> +
> +       if (pipe_obj->background_changed)
> +               igt_atomic_populate_crtc_req(req, output, IGT_CRTC_BACKGROUND, pipe_obj->background);
> +
> +       /*
> +        *      TODO: Add all crtc level properties here
> +        */
> +
> +}
> +
> +/*
> + * Add connector property changes to the atomic property set
> + */
> +static void igt_atomic_prepare_connector_commit(igt_output_t *output, drmModeAtomicReq *req)
> +{
> +
> +       struct kmstest_connector_config *config = &output->config;
> +
> +       if (config->connector_scaling_mode_changed)
> +               igt_atomic_populate_connector_req(req, output, IGT_CONNECTOR_SCALING_MODE, config->connector_scaling_mode);
> +
> +       if (config->connector_dpms_changed)
> +               igt_atomic_populate_connector_req(req, output, IGT_CONNECTOR_DPMS, config->connector_dpms);
> +       /*
> +        *      TODO: Add all other connector level properties here
> +        */
> +
> +}
> +
> +/*
> + * Commit all the changes of all the planes,crtcs, connectors
> + * atomically using drmModeAtomicCommit()
> + */
> +static int igt_atomic_commit(igt_display_t *display)
> +{
> +
> +       int ret = 0;
> +       int i = 0;
> +       drmModeAtomicReq *req;
> +       if (display->is_atomic != true)
> +               return -EINVAL;
> +       req = drmModeAtomicAlloc();
> +       drmModeAtomicSetCursor(req, 0);
> +
> +       for (i = 0; i < display->n_outputs; i++) {
> +               igt_output_t *output = &display->outputs[i];
> +               igt_pipe_t *pipe;
> +
> +               if (!output->valid)
> +                       continue;
> +
> +               /*
> +                * Add CRTC Properties to the property set
> +                */
> +               igt_atomic_prepare_crtc_commit(output, req);
> +
> +               /*
> +                * Add Connector Properties to the property set
> +                */
> +               igt_atomic_prepare_connector_commit(output, req);
> +
> +
> +               pipe = igt_output_get_driving_pipe(output);
> +
> +               for (i = 0; i < pipe->n_planes; i++) {
> +                       igt_plane_t *plane = &pipe->planes[i];
> +                       igt_atomic_prepare_plane_commit(plane, output, req);
> +               }
> +
> +       }
> +
> +       ret = drmModeAtomicCommit(display->drm_fd, req, 0, NULL);
> +       drmModeAtomicFree(req);
> +       return ret;
> +
> +}
>  /*
>   * Commit all plane changes across all outputs of the display.
>   *
> @@ -1724,6 +2035,14 @@ static int do_display_commit(igt_display_t *display,
>
>         igt_display_refresh(display);
>
> +       if (s == COMMIT_ATOMIC) {
> +
> +               ret = igt_atomic_commit(display);
> +               CHECK_RETURN(ret, fail_on_error);
> +               return 0;
> +
> +       }
> +
>         for (i = 0; i < display->n_outputs; i++) {
>                 igt_output_t *output = &display->outputs[i];
>
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 5744ed0..d6e2f12 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -106,11 +106,28 @@ int kmstest_get_pipe_from_crtc_id(int fd, int crtc_id);
>  void kmstest_set_vt_graphics_mode(void);
>  void kmstest_restore_vt_mode(void);
>
> +enum igt_atomic_crtc_properties {
> +       IGT_CRTC_BACKGROUND = 0,
> +       IGT_NUM_CRTC_PROPS
> +};
> +
> +enum igt_atomic_connector_properties {
> +       IGT_CONNECTOR_SCALING_MODE = 0,
> +       IGT_CONNECTOR_DPMS,
> +       IGT_NUM_CONNECTOR_PROPS
> +};
> +
>  struct kmstest_connector_config {
>         drmModeCrtc *crtc;
>         drmModeConnector *connector;
>         drmModeEncoder *encoder;
>         drmModeModeInfo default_mode;
> +       uint64_t connector_scaling_mode;
> +       bool connector_scaling_mode_changed;
> +       uint64_t connector_dpms;
> +       bool connector_dpms_changed;
> +       uint32_t atomic_props_crtc[IGT_NUM_CRTC_PROPS];
> +       uint32_t atomic_props_connector[IGT_NUM_CONNECTOR_PROPS];
>         int crtc_idx;
>         int pipe;
>  };
> @@ -163,9 +180,28 @@ uint32_t kmstest_find_crtc_for_connector(int fd, drmModeRes *res,
>  enum igt_commit_style {
>         COMMIT_LEGACY = 0,
>         COMMIT_UNIVERSAL,
> -       /* We'll add atomic here eventually. */
> +       COMMIT_ATOMIC,
>  };
>
> +enum igt_atomic_plane_properties {
> +       IGT_PLANE_SRC_X = 0,
> +       IGT_PLANE_SRC_Y,
> +       IGT_PLANE_SRC_W,
> +       IGT_PLANE_SRC_H,
> +
> +       IGT_PLANE_CRTC_X,
> +       IGT_PLANE_CRTC_Y,
> +       IGT_PLANE_CRTC_W,
> +       IGT_PLANE_CRTC_H,
> +
> +       IGT_PLANE_FB_ID,
> +       IGT_PLANE_CRTC_ID,
> +       IGT_PLANE_TYPE,
> +       IGT_PLANE_ROTATION,
> +       IGT_NUM_PLANE_PROPS
> +};
> +
> +
>  typedef struct igt_display igt_display_t;
>  typedef struct igt_pipe igt_pipe_t;
>  typedef uint32_t igt_fixed_t;                  /* 16.16 fixed point */
> @@ -207,6 +243,7 @@ typedef struct {
>         /* panning offset within the fb */
>         unsigned int pan_x, pan_y;
>         igt_rotation_t rotation;
> +       uint32_t atomic_props_plane[IGT_NUM_PLANE_PROPS];
>  } igt_plane_t;
>
>  struct igt_pipe {
> @@ -242,6 +279,7 @@ struct igt_display {
>         igt_output_t *outputs;
>         igt_pipe_t pipes[I915_MAX_PIPES];
>         bool has_universal_planes;
> +       bool is_atomic;
>  };
>
>  void igt_display_init(igt_display_t *display, int drm_fd);
> @@ -288,6 +326,38 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>
>  #define IGT_FIXED(i,f) ((i) << 16 | (f))
>
> +/**
> + * igt_atomic_populate_plane_req:
> + * @req: A pointer to drmModeAtomicReq
> + * @plane: A pointer igt_plane_t
> + * @prop: one of igt_atomic_plane_properties
> + * @value: the value to add
> + */
> +#define igt_atomic_populate_plane_req(req, plane, prop, value) \
> +       igt_assert_lt(0, drmModeAtomicAddProperty(req, plane->drm_plane->plane_id,\
> +                                                 plane->atomic_props_plane[prop], value))
> +
> +/**
> + * igt_atomic_populate_crtc_req:
> + * @req: A pointer to drmModeAtomicReq
> + * @output: A pointer igt_output_t
> + * @prop: one of igt_atomic_crtc_properties
> + * @value: the value to add
> + */
> +#define igt_atomic_populate_crtc_req(req, output, prop, value) \
> +       igt_assert_lt(0, drmModeAtomicAddProperty(req, output->config.crtc->crtc_id,\
> +                                                 output->config.atomic_props_crtc[prop], value))
> +/**
> + * igt_atomic_populate_connector_req:
> + * @req: A pointer to drmModeAtomicReq
> + * @output: A pointer igt_output_t
> + * @prop: one of igt_atomic_connector_properties
> + * @value: the value to add
> + */
> +#define igt_atomic_populate_connector_req(req, output, prop, value) \
> +       igt_assert_lt(0, drmModeAtomicAddProperty(req, output->config.connector->connector_id,\
> +                                                 output->config.atomic_props_connector[prop], value))
> +
>  void igt_enable_connectors(void);
>  void igt_reset_connectors(void);
>
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


More information about the Intel-gfx mailing list