[Intel-gfx] [PATCH i-g-t 2/7] lib/igt_kms: refactor atomic properties load/commit
Marius Vlad
marius.c.vlad at intel.com
Wed Mar 23 12:13:50 UTC 2016
On Wed, Mar 23, 2016 at 11:38:05AM +0000, Lionel Landwerlin wrote:
> All properties can be listed regardless of whether we're dealing with
> an atomic enabled driver, so load all of the properties ids regardless
> at init time.
>
> Also, we can have only one function loading the property ids and reuse
> that for different DRM objects.
>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> ---
> lib/igt_kms.c | 185 +++++++++++++++++++++-------------------------------------
> lib/igt_kms.h | 21 +++----
> 2 files changed, 76 insertions(+), 130 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 5ef89cf..3321738 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -173,85 +173,31 @@ static const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
> };
>
> /*
> - * Retrieve all the properies specified in props_name and store them into
> - * plane->atomic_props_plane.
> + * Retrieve all the properies specified in props_name from object_id
typo.
> + * and store them into prop_ids.
> */
> static void
> -igt_atomic_fill_plane_props(igt_display_t *display, igt_plane_t *plane,
> - int num_props, const char **prop_names)
> +igt_atomic_fill_props(igt_display_t *display,
> + uint32_t object_id, uint32_t object_type,
> + uint32_t *prop_ids, const char **prop_names,
> + int num_props)
> {
> drmModeObjectPropertiesPtr props;
> - int i, j, fd;
> -
> - fd = display->drm_fd;
> + int i, j;
>
> - props = drmModeObjectGetProperties(fd, plane->drm_plane->plane_id, DRM_MODE_OBJECT_PLANE);
> + props = drmModeObjectGetProperties(display->drm_fd,
> + object_id, object_type);
> igt_assert(props);
>
> for (i = 0; i < props->count_props; i++) {
> drmModePropertyPtr prop =
> - drmModeGetProperty(fd, props->props[i]);
> + drmModeGetProperty(display->drm_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];
> + prop_ids[j] = props->props[i];
> break;
> }
>
> @@ -259,7 +205,6 @@ igt_atomic_fill_props(igt_display_t *display, igt_output_t *output,
> }
>
> drmModeFreeObjectProperties(props);
> -
> }
>
> const unsigned char* igt_kms_get_alt_edid(void)
> @@ -1118,8 +1063,9 @@ 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);
> + igt_atomic_fill_props(display, output->id, DRM_MODE_OBJECT_CONNECTOR,
> + output->properties, igt_connector_prop_names,
> + IGT_NUM_CONNECTOR_PROPS);
Does it make sense to call it if !display->atomic?
> }
>
> static bool
> @@ -1189,7 +1135,6 @@ void igt_display_init(igt_display_t *display, int drm_fd)
> drmModeRes *resources;
> drmModePlaneRes *plane_resources;
> int i;
> - int is_atomic = 0;
>
> memset(display, 0, sizeof(igt_display_t));
>
> @@ -1207,7 +1152,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);
> - is_atomic = drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1);
> + display->is_atomic = drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0;
> plane_resources = drmModeGetPlaneResources(display->drm_fd);
> igt_assert(plane_resources);
>
> @@ -1265,10 +1210,12 @@ void igt_display_init(igt_display_t *display, int drm_fd)
>
> plane->pipe = pipe;
> plane->drm_plane = drm_plane;
> - if (is_atomic == 0) {
> - display->is_atomic = 1;
> - igt_atomic_fill_plane_props(display, plane, IGT_NUM_PLANE_PROPS, igt_plane_prop_names);
> - }
> + igt_atomic_fill_props(display,
> + plane->drm_plane->plane_id,
> + DRM_MODE_OBJECT_PLANE,
> + plane->properties,
> + igt_plane_prop_names,
> + IGT_NUM_PLANE_PROPS);
>
> get_plane_property(display->drm_fd, drm_plane->plane_id,
> "rotation",
> @@ -1316,6 +1263,36 @@ void igt_display_init(igt_display_t *display, int drm_fd)
> /* planes = 1 primary, (p-1) sprites, 1 cursor */
> pipe->n_planes = p + 1;
>
> + igt_atomic_fill_props(display, pipe->crtc_id, DRM_MODE_OBJECT_CRTC,
> + pipe->properties, igt_crtc_prop_names,
> + IGT_NUM_CRTC_PROPS);
> + if (pipe->crtc_id) {
> + uint64_t prop_value;
> +
> + get_crtc_property(display->drm_fd, pipe->crtc_id,
> + "background_color",
> + &pipe->background_property,
> + &prop_value,
> + NULL);
> + pipe->background = (uint32_t)prop_value;
> + get_crtc_property(display->drm_fd, pipe->crtc_id,
> + "DEGAMMA_LUT",
> + &pipe->degamma_property,
> + NULL,
> + NULL);
> + get_crtc_property(display->drm_fd, pipe->crtc_id,
> + "CTM",
> + &pipe->ctm_property,
> + NULL,
> + NULL);
> + get_crtc_property(display->drm_fd, pipe->crtc_id,
> + "GAMMA_LUT",
> + &pipe->gamma_property,
> + NULL,
> + NULL);
> + }
> +
> +
> /* make sure we don't overflow the plane array */
> igt_assert(pipe->n_planes <= IGT_MAX_PLANES);
> }
> @@ -1330,7 +1307,6 @@ void igt_display_init(igt_display_t *display, int drm_fd)
> igt_assert(display->outputs);
>
> for (i = 0; i < display->n_outputs; i++) {
> - int j;
> igt_output_t *output = &display->outputs[i];
>
> /*
> @@ -1342,35 +1318,6 @@ void igt_display_init(igt_display_t *display, int drm_fd)
> output->display = display;
>
> igt_output_refresh(output);
> -
> - for (j = 0; j < display->n_pipes; j++) {
> - uint64_t prop_value;
> - igt_pipe_t *pipe = &display->pipes[j];
> -
> - if (output->config.crtc) {
> - get_crtc_property(display->drm_fd, output->config.crtc->crtc_id,
> - "background_color",
> - &pipe->background_property,
> - &prop_value,
> - NULL);
> - pipe->background = (uint32_t)prop_value;
> - get_crtc_property(display->drm_fd, output->config.crtc->crtc_id,
> - "DEGAMMA_LUT",
> - &pipe->degamma_property,
> - NULL,
> - NULL);
> - get_crtc_property(display->drm_fd, output->config.crtc->crtc_id,
> - "CTM",
> - &pipe->ctm_property,
> - NULL,
> - NULL);
> - get_crtc_property(display->drm_fd, output->config.crtc->crtc_id,
> - "GAMMA_LUT",
> - &pipe->gamma_property,
> - NULL,
> - NULL);
> - }
> - }
> }
>
> drmModeFreePlaneResources(plane_resources);
> @@ -1945,22 +1892,19 @@ static int igt_output_commit(igt_output_t *output,
> /*
> * Add crtc property changes to the atomic property set
> */
> -static void igt_atomic_prepare_crtc_commit(igt_output_t *output, drmModeAtomicReq *req)
> +static void igt_atomic_prepare_pipe_commit(igt_pipe_t *pipe, drmModeAtomicReq *req)
> {
> + if (pipe->background_changed)
> + igt_atomic_populate_pipe_req(req, pipe, IGT_CRTC_BACKGROUND, pipe->background);
>
> - 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);
> -
> - if (pipe_obj->color_mgmt_changed) {
> - igt_atomic_populate_crtc_req(req, output, IGT_CRTC_DEGAMMA_LUT, pipe_obj->degamma_blob);
> - igt_atomic_populate_crtc_req(req, output, IGT_CRTC_CTM, pipe_obj->ctm_blob);
> - igt_atomic_populate_crtc_req(req, output, IGT_CRTC_GAMMA_LUT, pipe_obj->gamma_blob);
> + if (pipe->color_mgmt_changed) {
> + igt_atomic_populate_pipe_req(req, pipe, IGT_CRTC_DEGAMMA_LUT, pipe->degamma_blob);
> + igt_atomic_populate_pipe_req(req, pipe, IGT_CRTC_CTM, pipe->ctm_blob);
> + igt_atomic_populate_pipe_req(req, pipe, IGT_CRTC_GAMMA_LUT, pipe->gamma_blob);
> }
>
> - pipe_obj->background_changed = false;
> - pipe_obj->color_mgmt_changed = false;
> + pipe->background_changed = false;
> + pipe->color_mgmt_changed = false;
> }
>
> /*
> @@ -2001,10 +1945,14 @@ static int igt_atomic_commit(igt_display_t *display)
> igt_plane_t *plane;
> enum pipe pipe;
>
> + pipe_obj = igt_output_get_driving_pipe(output);
> +
> + pipe = pipe_obj->pipe;
> +
> /*
> * Add CRTC Properties to the property set
> */
> - igt_atomic_prepare_crtc_commit(output, req);
> + igt_atomic_prepare_pipe_commit(pipe_obj, req);
>
> /*
> * Add Connector Properties to the property set
> @@ -2012,9 +1960,6 @@ static int igt_atomic_commit(igt_display_t *display)
> igt_atomic_prepare_connector_commit(output, req);
>
>
> - pipe_obj = igt_output_get_driving_pipe(output);
> -
> - pipe = pipe_obj->pipe;
>
> for_each_plane_on_pipe(display, pipe, plane) {
> igt_atomic_prepare_plane_commit(plane, output, req);
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 8ae1192..9f04f72 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -129,8 +129,6 @@ struct kmstest_connector_config {
> 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;
> };
> @@ -246,7 +244,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];
> + uint32_t properties[IGT_NUM_PLANE_PROPS];
> } igt_plane_t;
>
> struct igt_pipe {
> @@ -269,6 +267,8 @@ struct igt_pipe {
> uint32_t color_mgmt_changed : 1;
>
> uint32_t crtc_id;
> +
> + uint32_t properties[IGT_NUM_CRTC_PROPS];
> };
So the macro remain as CRTCs?
>
> typedef struct {
> @@ -281,6 +281,7 @@ typedef struct {
> unsigned long pending_crtc_idx_mask;
> bool use_override_mode;
> drmModeModeInfo override_mode;
> + uint32_t properties[IGT_NUM_CONNECTOR_PROPS];
> } igt_output_t;
>
> struct igt_display {
> @@ -355,18 +356,18 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
> */
> #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))
> + plane->properties[prop], value))
>
> /**
> - * igt_atomic_populate_crtc_req:
> + * igt_atomic_populate_pipe_req:
> * @req: A pointer to drmModeAtomicReq
> - * @output: A pointer igt_output_t
> + * @pipe: A pointer igt_pipe_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))
> +#define igt_atomic_populate_pipe_req(req, pipe, prop, value) \
> + igt_assert_lt(0, drmModeAtomicAddProperty(req, pipe->crtc_id,\
> + pipe->properties[prop], value))
> /**
> * igt_atomic_populate_connector_req:
> * @req: A pointer to drmModeAtomicReq
> @@ -376,7 +377,7 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
> */
> #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))
> + output->properties[prop], value))
>
> void igt_enable_connectors(void);
> void igt_reset_connectors(void);
> --
> 2.8.0.rc3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20160323/a0851067/attachment.sig>
More information about the Intel-gfx
mailing list