Re: [PATCH i-g-t 2/7] lib/igt_kms: refactor atomic properties load/commit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Attachment: signature.asc
Description: Digital signature

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux