[PATCH i-g-t 0/7] lib/igt_kms cleanups

[PATCH i-g-t 0/7] lib/igt_kms cleanups

[Lionel Landwerlin]

Hi,

This series is a bit of cleanup I had in mind for after the atomic
commit support (from Marius, Mayuresh and Pratik) had landed.

We're using both crtc and pipe denominations, maybe we can settle for
one.

The atomic support introduced some redundancy in how we deal with
property ids, this can be simplified a bit.

And we're also missing some documentation.

Cheers,

Lionel Landwerlin (7):
  lib/igt_kms: fix clearing up connector/pipe state on atomic commit
  lib/igt_kms: refactor atomic properties load/commit
  lib/igt_kms: don't store rotation property id twice
  lib/igt_kms: don't store background property id twice
  lib/igt_kms: don't store color management properties ids twice
  lib/igt_kms: rename igt_crtc_set_background
  lib/igt_kms: add some missing documentation

 lib/igt_kms.c                     | 277 +++++++++++++++++++-------------------
 lib/igt_kms.h                     |  50 +++----
 tests/kms_crtc_background_color.c |  20 +--
 tests/kms_rotation_crc.c          |   4 +-
 4 files changed, 176 insertions(+), 175 deletions(-)

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

[PATCH i-g-t 1/7] lib/igt_kms: fix clearing up connector/pipe state on atomic commit

[Lionel Landwerlin]

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 lib/igt_kms.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 82257a6..5ef89cf 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1959,9 +1959,8 @@ static void igt_atomic_prepare_crtc_commit(igt_output_t *output, drmModeAtomicRe
 		igt_atomic_populate_crtc_req(req, output, IGT_CRTC_GAMMA_LUT, pipe_obj->gamma_blob);
 	}
 
-	/*
-	 *	TODO: Add all crtc level properties here
-	 */
+	pipe_obj->background_changed = false;
+	pipe_obj->color_mgmt_changed = false;
 }
 
 /*
@@ -1977,10 +1976,9 @@ static void igt_atomic_prepare_connector_commit(igt_output_t *output, drmModeAto
 
 	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
-	 */
 
+	config->connector_scaling_mode_changed = false;
+	config->connector_dpms_changed = false;
 }
 
 /*
-- 
2.8.0.rc3

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

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

[Lionel Landwerlin]

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@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
+ * 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);
 }
 
 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];
 };
 
 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Marius Vlad]

[-- Attachment #1.1: Type: text/plain, Size: 13775 bytes --]

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

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

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

[Lionel Landwerlin]

On 23/03/16 12:13, Marius Vlad wrote:
> 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@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.

Thanks.

>> + * 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?

I can't think of a reason not to full the properties array all the time.
It makes the initialization simpler and we use these rather than having 
rotation_property/background_property/degamma_lut_property/etc...

>
>>   }
>>   
>>   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?

Ah sorry, I should leave igt_atomic_populate_crtc_req untouched in this 
commit and move rename in the other one.
Following Daniel's comment, it might not need to be changed at all.

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

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

[PATCH i-g-t 3/7] lib/igt_kms: don't store rotation property id twice

[Lionel Landwerlin]

Now that we load all properties regardless of atomic, the rotation
property id is redundant.

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 lib/igt_kms.c            | 9 ++++-----
 lib/igt_kms.h            | 4 +---
 tests/kms_rotation_crc.c | 4 ++--
 3 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 3321738..c82ba3f 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1219,9 +1219,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
 
 			get_plane_property(display->drm_fd, drm_plane->plane_id,
 					   "rotation",
-					   &plane->rotation_property,
-					   &prop_value,
-					   NULL);
+					   NULL, &prop_value, NULL);
 			plane->rotation = (igt_rotation_t)prop_value;
 		}
 
@@ -1661,8 +1659,9 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
 	plane->size_changed = false;
 
 	if (plane->rotation_changed) {
-		ret = igt_plane_set_property(plane, plane->rotation_property,
-				       plane->rotation);
+		ret = igt_plane_set_property(plane,
+			plane->properties[IGT_PLANE_ROTATION],
+			plane->rotation);
 
 		plane->rotation_changed = false;
 		CHECK_RETURN(ret, fail_on_error);
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 9f04f72..ac2ca11 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -235,8 +235,6 @@ typedef struct {
 	drmModePlane *drm_plane;
 	struct igt_fb *fb;
 
-	uint32_t rotation_property;
-
 	/* position within pipe_src_w x pipe_src_h */
 	int crtc_x, crtc_y;
 	/* size within pipe_src_w x pipe_src_h */
@@ -314,7 +312,7 @@ bool igt_pipe_get_property(igt_pipe_t *pipe, const char *name,
 
 static inline bool igt_plane_supports_rotation(igt_plane_t *plane)
 {
-	return plane->rotation_property != 0;
+	return plane->properties[IGT_PLANE_ROTATION] != 0;
 }
 
 void igt_pipe_set_degamma_lut(igt_pipe_t *pipe, void *ptr, size_t length);
diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index f94f8f1..6a92d56 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -368,7 +368,7 @@ static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_ty
 
 	drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
 				 DRM_MODE_OBJECT_PLANE,
-				 plane->rotation_property,
+				 plane->properties[IGT_PLANE_ROTATION],
 				 plane->rotation);
 	ret = igt_display_try_commit2(display, commit);
 
@@ -457,7 +457,7 @@ static void test_plane_rotation_exhaust_fences(data_t *data, enum igt_plane plan
 
 		drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
 					 DRM_MODE_OBJECT_PLANE,
-					 plane->rotation_property,
+					 plane->properties[IGT_PLANE_ROTATION],
 					 plane->rotation);
 		ret = igt_display_try_commit2(display, commit);
 		if (ret) {
-- 
2.8.0.rc3

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

[PATCH i-g-t 4/7] lib/igt_kms: don't store background property id twice

[Lionel Landwerlin]

Now that we load all properties regardless of atomic, the background
color property id is redundant.

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 lib/igt_kms.c                     | 7 ++++---
 lib/igt_kms.h                     | 7 ++++++-
 tests/kms_crtc_background_color.c | 2 +-
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index c82ba3f..c3d389a 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1269,7 +1269,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
 
 			get_crtc_property(display->drm_fd, pipe->crtc_id,
 					  "background_color",
-					  &pipe->background_property,
+					  NULL,
 					  &prop_value,
 					  NULL);
 			pipe->background = (uint32_t)prop_value;
@@ -1849,8 +1849,9 @@ static int igt_output_commit(igt_output_t *output,
 	pipe = igt_output_get_driving_pipe(output);
 
 	if (pipe->background_changed) {
-		igt_crtc_set_property(output, pipe->background_property,
-			pipe->background);
+		igt_crtc_set_property(output,
+				      pipe->properties[IGT_CRTC_BACKGROUND],
+				      pipe->background);
 
 		pipe->background_changed = false;
 	}
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index ac2ca11..8e8b240 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -254,7 +254,6 @@ struct igt_pipe {
 	igt_plane_t planes[IGT_MAX_PLANES];
 	uint64_t background; /* Background color MSB BGR 16bpc LSB */
 	uint32_t background_changed : 1;
-	uint32_t background_property;
 
 	uint64_t degamma_blob;
 	uint32_t degamma_property;
@@ -330,6 +329,12 @@ void igt_fb_set_position(struct igt_fb *fb, igt_plane_t *plane,
 void igt_fb_set_size(struct igt_fb *fb, igt_plane_t *plane,
 	uint32_t w, uint32_t h);
 
+static inline bool igt_pipe_supports_background(igt_pipe_t *pipe)
+{
+	return pipe->properties[IGT_CRTC_BACKGROUND] != 0;
+}
+
+
 void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
 
 #define for_each_connected_output(display, output)		\
diff --git a/tests/kms_crtc_background_color.c b/tests/kms_crtc_background_color.c
index b496625..055f935 100644
--- a/tests/kms_crtc_background_color.c
+++ b/tests/kms_crtc_background_color.c
@@ -140,7 +140,7 @@ static void test_crtc_background(data_t *data)
 		igt_output_set_pipe(output, pipe);
 
 		plane = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
-		igt_require(plane->pipe->background_property);
+		igt_require(igt_pipe_supports_background (plane->pipe));
 
 		prepare_crtc(data, output, pipe, plane, 1, PURPLE, BLACK64);
 
-- 
2.8.0.rc3

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

[PATCH i-g-t 5/7] lib/igt_kms: don't store color management properties ids twice

[Lionel Landwerlin]

Now that we load all properties regardless of atomic, the color
management properties ids are redundant.

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 lib/igt_kms.c | 22 ++++------------------
 lib/igt_kms.h |  3 ---
 2 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index c3d389a..e3488f1 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1273,21 +1273,6 @@ void igt_display_init(igt_display_t *display, int drm_fd)
 					  &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);
 		}
 
 
@@ -1857,11 +1842,12 @@ static int igt_output_commit(igt_output_t *output,
 	}
 
 	if (pipe->color_mgmt_changed) {
-		igt_crtc_set_property(output, pipe->degamma_property,
+		igt_crtc_set_property(output,
+				      pipe->properties[IGT_CRTC_DEGAMMA_LUT],
 				      pipe->degamma_blob);
-		igt_crtc_set_property(output, pipe->ctm_property,
+		igt_crtc_set_property(output, pipe->properties[IGT_CRTC_CTM],
 				      pipe->ctm_blob);
-		igt_crtc_set_property(output, pipe->gamma_property,
+		igt_crtc_set_property(output, pipe->properties[IGT_CRTC_GAMMA_LUT],
 				      pipe->gamma_blob);
 
 		pipe->color_mgmt_changed = false;
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 8e8b240..4d3abf6 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -256,11 +256,8 @@ struct igt_pipe {
 	uint32_t background_changed : 1;
 
 	uint64_t degamma_blob;
-	uint32_t degamma_property;
 	uint64_t ctm_blob;
-	uint32_t ctm_property;
 	uint64_t gamma_blob;
-	uint32_t gamma_property;
 	uint32_t color_mgmt_changed : 1;
 
 	uint32_t crtc_id;
-- 
2.8.0.rc3

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

[PATCH i-g-t 6/7] lib/igt_kms: rename igt_crtc_set_background

[Lionel Landwerlin]

We're a bit inconsistent with functions named igt_crtc_* and other
named igt_pipe_* even though they apply to the same objects.

It seems most of the kernel/igt is using the pipe denomination. Let's
rename the igt_crtc_* into igt_pipe_*.

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 lib/igt_kms.c                     | 28 ++++++++++++++--------------
 lib/igt_kms.h                     | 29 ++++++++++++++---------------
 tests/kms_crtc_background_color.c | 18 +++++++++---------
 3 files changed, 37 insertions(+), 38 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index e3488f1..1e0585f 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -160,7 +160,7 @@ static const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
 	"rotation"
 };
 
-static const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
+static const char *igt_pipe_prop_names[IGT_NUM_PIPE_PROPS] = {
 	"background_color",
 	"DEGAMMA_LUT",
 	"CTM",
@@ -1262,8 +1262,8 @@ void igt_display_init(igt_display_t *display, int drm_fd)
 		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);
+				      pipe->properties, igt_pipe_prop_names,
+				      IGT_NUM_PIPE_PROPS);
 		if (pipe->crtc_id) {
 			uint64_t prop_value;
 
@@ -1835,7 +1835,7 @@ static int igt_output_commit(igt_output_t *output,
 
 	if (pipe->background_changed) {
 		igt_crtc_set_property(output,
-				      pipe->properties[IGT_CRTC_BACKGROUND],
+				      pipe->properties[IGT_PIPE_BACKGROUND],
 				      pipe->background);
 
 		pipe->background_changed = false;
@@ -1843,11 +1843,11 @@ static int igt_output_commit(igt_output_t *output,
 
 	if (pipe->color_mgmt_changed) {
 		igt_crtc_set_property(output,
-				      pipe->properties[IGT_CRTC_DEGAMMA_LUT],
+				      pipe->properties[IGT_PIPE_DEGAMMA_LUT],
 				      pipe->degamma_blob);
-		igt_crtc_set_property(output, pipe->properties[IGT_CRTC_CTM],
+		igt_crtc_set_property(output, pipe->properties[IGT_PIPE_CTM],
 				      pipe->ctm_blob);
-		igt_crtc_set_property(output, pipe->properties[IGT_CRTC_GAMMA_LUT],
+		igt_crtc_set_property(output, pipe->properties[IGT_PIPE_GAMMA_LUT],
 				      pipe->gamma_blob);
 
 		pipe->color_mgmt_changed = false;
@@ -1881,12 +1881,12 @@ static int igt_output_commit(igt_output_t *output,
 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_atomic_populate_pipe_req(req, pipe, IGT_PIPE_BACKGROUND, pipe->background);
 
 	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);
+		igt_atomic_populate_pipe_req(req, pipe, IGT_PIPE_DEGAMMA_LUT, pipe->degamma_blob);
+		igt_atomic_populate_pipe_req(req, pipe, IGT_PIPE_CTM, pipe->ctm_blob);
+		igt_atomic_populate_pipe_req(req, pipe, IGT_PIPE_GAMMA_LUT, pipe->gamma_blob);
 	}
 
 	pipe->background_changed = false;
@@ -2316,7 +2316,7 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
 }
 
 /**
- * igt_crtc_set_background:
+ * igt_pipe_set_background:
  * @pipe: pipe pointer to which background color to be set
  * @background: background color value in BGR 16bpc
  *
@@ -2325,11 +2325,11 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
  * property.
  * For example to get red as background, set background = 0x00000000FFFF.
  */
-void igt_crtc_set_background(igt_pipe_t *pipe, uint64_t background)
+void igt_pipe_set_background(igt_pipe_t *pipe, uint64_t background)
 {
 	igt_display_t *display = pipe->display;
 
-	LOG(display, "%s.%d: crtc_set_background(%"PRIx64")\n",
+	LOG(display, "%s.%d: pipe_set_background(%"PRIx64")\n",
 	    kmstest_pipe_name(pipe->pipe),
 	    pipe->pipe, background);
 
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 4d3abf6..27c87ea 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -106,12 +106,12 @@ 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_CRTC_CTM,
-       IGT_CRTC_DEGAMMA_LUT,
-       IGT_CRTC_GAMMA_LUT,
-       IGT_NUM_CRTC_PROPS
+enum igt_atomic_pipe_properties {
+       IGT_PIPE_BACKGROUND = 0,
+       IGT_PIPE_CTM,
+       IGT_PIPE_DEGAMMA_LUT,
+       IGT_PIPE_GAMMA_LUT,
+       IGT_NUM_PIPE_PROPS
 };
 
 enum igt_atomic_connector_properties {
@@ -262,7 +262,7 @@ struct igt_pipe {
 
 	uint32_t crtc_id;
 
-	uint32_t properties[IGT_NUM_CRTC_PROPS];
+	uint32_t properties[IGT_NUM_PIPE_PROPS];
 };
 
 typedef struct {
@@ -305,6 +305,12 @@ igt_plane_t *igt_output_get_plane(igt_output_t *output, enum igt_plane plane);
 bool igt_pipe_get_property(igt_pipe_t *pipe, const char *name,
 			   uint32_t *prop_id, uint64_t *value,
 			   drmModePropertyPtr *prop);
+void igt_pipe_set_background(igt_pipe_t *pipe, uint64_t background);
+
+static inline bool igt_pipe_supports_background(igt_pipe_t *pipe)
+{
+	return pipe->properties[IGT_PIPE_BACKGROUND] != 0;
+}
 
 static inline bool igt_plane_supports_rotation(igt_plane_t *plane)
 {
@@ -320,18 +326,11 @@ void igt_plane_set_position(igt_plane_t *plane, int x, int y);
 void igt_plane_set_size(igt_plane_t *plane, int w, int h);
 void igt_plane_set_panning(igt_plane_t *plane, int x, int y);
 void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation);
-void igt_crtc_set_background(igt_pipe_t *pipe, uint64_t background);
 void igt_fb_set_position(struct igt_fb *fb, igt_plane_t *plane,
 	uint32_t x, uint32_t y);
 void igt_fb_set_size(struct igt_fb *fb, igt_plane_t *plane,
 	uint32_t w, uint32_t h);
 
-static inline bool igt_pipe_supports_background(igt_pipe_t *pipe)
-{
-	return pipe->properties[IGT_CRTC_BACKGROUND] != 0;
-}
-
-
 void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
 
 #define for_each_connected_output(display, output)		\
@@ -362,7 +361,7 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
  * igt_atomic_populate_pipe_req:
  * @req: A pointer to drmModeAtomicReq
  * @pipe: A pointer igt_pipe_t
- * @prop: one of igt_atomic_crtc_properties
+ * @prop: one of igt_atomic_pipe_properties
  * @value: the value to add
  */
 #define igt_atomic_populate_pipe_req(req, pipe, prop, value) \
diff --git a/tests/kms_crtc_background_color.c b/tests/kms_crtc_background_color.c
index 055f935..0bf205e 100644
--- a/tests/kms_crtc_background_color.c
+++ b/tests/kms_crtc_background_color.c
@@ -97,7 +97,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 	igt_assert(fb_id);
 
 	/* To make FB pixel win with background color, set alpha as full opaque */
-	igt_crtc_set_background(plane->pipe, pipe_background_color);
+	igt_pipe_set_background(plane->pipe, pipe_background_color);
 	if (opaque_buffer)
 		alpha = 1.0;    /* alpha 1 is fully opque */
 	else
@@ -117,7 +117,7 @@ static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane)
 
 	igt_remove_fb(data->gfx_fd, &data->fb);
 
-	igt_crtc_set_background(plane->pipe, BLACK64);
+	igt_pipe_set_background(plane->pipe, BLACK64);
 	igt_plane_set_fb(plane, NULL);
 	igt_output_set_pipe(output, PIPE_ANY);
 
@@ -147,26 +147,26 @@ static void test_crtc_background(data_t *data)
 		/* Now set background without using a plane, i.e.,
 		 * Disable the plane to let hw background color win blend. */
 		igt_plane_set_fb(plane, NULL);
-		igt_crtc_set_background(plane->pipe, PURPLE64);
+		igt_pipe_set_background(plane->pipe, PURPLE64);
 		igt_display_commit2(display, COMMIT_UNIVERSAL);
 
 		/* Try few other background colors */
-		igt_crtc_set_background(plane->pipe, CYAN64);
+		igt_pipe_set_background(plane->pipe, CYAN64);
 		igt_display_commit2(display, COMMIT_UNIVERSAL);
 
-		igt_crtc_set_background(plane->pipe, YELLOW64);
+		igt_pipe_set_background(plane->pipe, YELLOW64);
 		igt_display_commit2(display, COMMIT_UNIVERSAL);
 
-		igt_crtc_set_background(plane->pipe, RED64);
+		igt_pipe_set_background(plane->pipe, RED64);
 		igt_display_commit2(display, COMMIT_UNIVERSAL);
 
-		igt_crtc_set_background(plane->pipe, GREEN64);
+		igt_pipe_set_background(plane->pipe, GREEN64);
 		igt_display_commit2(display, COMMIT_UNIVERSAL);
 
-		igt_crtc_set_background(plane->pipe, BLUE64);
+		igt_pipe_set_background(plane->pipe, BLUE64);
 		igt_display_commit2(display, COMMIT_UNIVERSAL);
 
-		igt_crtc_set_background(plane->pipe, WHITE64);
+		igt_pipe_set_background(plane->pipe, WHITE64);
 		igt_display_commit2(display, COMMIT_UNIVERSAL);
 
 		valid_tests++;
-- 
2.8.0.rc3

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

Re: [PATCH i-g-t 6/7] lib/igt_kms: rename igt_crtc_set_background

[Daniel Vetter]

On Wed, Mar 23, 2016 at 11:38:09AM +0000, Lionel Landwerlin wrote:
> We're a bit inconsistent with functions named igt_crtc_* and other
> named igt_pipe_* even though they apply to the same objects.
> 
> It seems most of the kernel/igt is using the pipe denomination. Let's
> rename the igt_crtc_* into igt_pipe_*.
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>

Tbh I'd go the other direction. pipe is use in Bspec, and hence why it's
used a lot in our i915 code. And also why we leaked it into igt_kms. But
the drm thing a pipe is represented by is the CRTC. With others starting
to use igt on non-intel drivers I think it'd be better to move into the
other direction, and rename all the pipe stuff to CRTC. Or well most of it
(I think we have some intel-specific concepts mixed in here).
-Daniel

> ---
>  lib/igt_kms.c                     | 28 ++++++++++++++--------------
>  lib/igt_kms.h                     | 29 ++++++++++++++---------------
>  tests/kms_crtc_background_color.c | 18 +++++++++---------
>  3 files changed, 37 insertions(+), 38 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index e3488f1..1e0585f 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -160,7 +160,7 @@ static const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
>  	"rotation"
>  };
>  
> -static const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
> +static const char *igt_pipe_prop_names[IGT_NUM_PIPE_PROPS] = {
>  	"background_color",
>  	"DEGAMMA_LUT",
>  	"CTM",
> @@ -1262,8 +1262,8 @@ void igt_display_init(igt_display_t *display, int drm_fd)
>  		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);
> +				      pipe->properties, igt_pipe_prop_names,
> +				      IGT_NUM_PIPE_PROPS);
>  		if (pipe->crtc_id) {
>  			uint64_t prop_value;
>  
> @@ -1835,7 +1835,7 @@ static int igt_output_commit(igt_output_t *output,
>  
>  	if (pipe->background_changed) {
>  		igt_crtc_set_property(output,
> -				      pipe->properties[IGT_CRTC_BACKGROUND],
> +				      pipe->properties[IGT_PIPE_BACKGROUND],
>  				      pipe->background);
>  
>  		pipe->background_changed = false;
> @@ -1843,11 +1843,11 @@ static int igt_output_commit(igt_output_t *output,
>  
>  	if (pipe->color_mgmt_changed) {
>  		igt_crtc_set_property(output,
> -				      pipe->properties[IGT_CRTC_DEGAMMA_LUT],
> +				      pipe->properties[IGT_PIPE_DEGAMMA_LUT],
>  				      pipe->degamma_blob);
> -		igt_crtc_set_property(output, pipe->properties[IGT_CRTC_CTM],
> +		igt_crtc_set_property(output, pipe->properties[IGT_PIPE_CTM],
>  				      pipe->ctm_blob);
> -		igt_crtc_set_property(output, pipe->properties[IGT_CRTC_GAMMA_LUT],
> +		igt_crtc_set_property(output, pipe->properties[IGT_PIPE_GAMMA_LUT],
>  				      pipe->gamma_blob);
>  
>  		pipe->color_mgmt_changed = false;
> @@ -1881,12 +1881,12 @@ static int igt_output_commit(igt_output_t *output,
>  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_atomic_populate_pipe_req(req, pipe, IGT_PIPE_BACKGROUND, pipe->background);
>  
>  	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);
> +		igt_atomic_populate_pipe_req(req, pipe, IGT_PIPE_DEGAMMA_LUT, pipe->degamma_blob);
> +		igt_atomic_populate_pipe_req(req, pipe, IGT_PIPE_CTM, pipe->ctm_blob);
> +		igt_atomic_populate_pipe_req(req, pipe, IGT_PIPE_GAMMA_LUT, pipe->gamma_blob);
>  	}
>  
>  	pipe->background_changed = false;
> @@ -2316,7 +2316,7 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
>  }
>  
>  /**
> - * igt_crtc_set_background:
> + * igt_pipe_set_background:
>   * @pipe: pipe pointer to which background color to be set
>   * @background: background color value in BGR 16bpc
>   *
> @@ -2325,11 +2325,11 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
>   * property.
>   * For example to get red as background, set background = 0x00000000FFFF.
>   */
> -void igt_crtc_set_background(igt_pipe_t *pipe, uint64_t background)
> +void igt_pipe_set_background(igt_pipe_t *pipe, uint64_t background)
>  {
>  	igt_display_t *display = pipe->display;
>  
> -	LOG(display, "%s.%d: crtc_set_background(%"PRIx64")\n",
> +	LOG(display, "%s.%d: pipe_set_background(%"PRIx64")\n",
>  	    kmstest_pipe_name(pipe->pipe),
>  	    pipe->pipe, background);
>  
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 4d3abf6..27c87ea 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -106,12 +106,12 @@ 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_CRTC_CTM,
> -       IGT_CRTC_DEGAMMA_LUT,
> -       IGT_CRTC_GAMMA_LUT,
> -       IGT_NUM_CRTC_PROPS
> +enum igt_atomic_pipe_properties {
> +       IGT_PIPE_BACKGROUND = 0,
> +       IGT_PIPE_CTM,
> +       IGT_PIPE_DEGAMMA_LUT,
> +       IGT_PIPE_GAMMA_LUT,
> +       IGT_NUM_PIPE_PROPS
>  };
>  
>  enum igt_atomic_connector_properties {
> @@ -262,7 +262,7 @@ struct igt_pipe {
>  
>  	uint32_t crtc_id;
>  
> -	uint32_t properties[IGT_NUM_CRTC_PROPS];
> +	uint32_t properties[IGT_NUM_PIPE_PROPS];
>  };
>  
>  typedef struct {
> @@ -305,6 +305,12 @@ igt_plane_t *igt_output_get_plane(igt_output_t *output, enum igt_plane plane);
>  bool igt_pipe_get_property(igt_pipe_t *pipe, const char *name,
>  			   uint32_t *prop_id, uint64_t *value,
>  			   drmModePropertyPtr *prop);
> +void igt_pipe_set_background(igt_pipe_t *pipe, uint64_t background);
> +
> +static inline bool igt_pipe_supports_background(igt_pipe_t *pipe)
> +{
> +	return pipe->properties[IGT_PIPE_BACKGROUND] != 0;
> +}
>  
>  static inline bool igt_plane_supports_rotation(igt_plane_t *plane)
>  {
> @@ -320,18 +326,11 @@ void igt_plane_set_position(igt_plane_t *plane, int x, int y);
>  void igt_plane_set_size(igt_plane_t *plane, int w, int h);
>  void igt_plane_set_panning(igt_plane_t *plane, int x, int y);
>  void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation);
> -void igt_crtc_set_background(igt_pipe_t *pipe, uint64_t background);
>  void igt_fb_set_position(struct igt_fb *fb, igt_plane_t *plane,
>  	uint32_t x, uint32_t y);
>  void igt_fb_set_size(struct igt_fb *fb, igt_plane_t *plane,
>  	uint32_t w, uint32_t h);
>  
> -static inline bool igt_pipe_supports_background(igt_pipe_t *pipe)
> -{
> -	return pipe->properties[IGT_CRTC_BACKGROUND] != 0;
> -}
> -
> -
>  void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>  
>  #define for_each_connected_output(display, output)		\
> @@ -362,7 +361,7 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>   * igt_atomic_populate_pipe_req:
>   * @req: A pointer to drmModeAtomicReq
>   * @pipe: A pointer igt_pipe_t
> - * @prop: one of igt_atomic_crtc_properties
> + * @prop: one of igt_atomic_pipe_properties
>   * @value: the value to add
>   */
>  #define igt_atomic_populate_pipe_req(req, pipe, prop, value) \
> diff --git a/tests/kms_crtc_background_color.c b/tests/kms_crtc_background_color.c
> index 055f935..0bf205e 100644
> --- a/tests/kms_crtc_background_color.c
> +++ b/tests/kms_crtc_background_color.c
> @@ -97,7 +97,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
>  	igt_assert(fb_id);
>  
>  	/* To make FB pixel win with background color, set alpha as full opaque */
> -	igt_crtc_set_background(plane->pipe, pipe_background_color);
> +	igt_pipe_set_background(plane->pipe, pipe_background_color);
>  	if (opaque_buffer)
>  		alpha = 1.0;    /* alpha 1 is fully opque */
>  	else
> @@ -117,7 +117,7 @@ static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane)
>  
>  	igt_remove_fb(data->gfx_fd, &data->fb);
>  
> -	igt_crtc_set_background(plane->pipe, BLACK64);
> +	igt_pipe_set_background(plane->pipe, BLACK64);
>  	igt_plane_set_fb(plane, NULL);
>  	igt_output_set_pipe(output, PIPE_ANY);
>  
> @@ -147,26 +147,26 @@ static void test_crtc_background(data_t *data)
>  		/* Now set background without using a plane, i.e.,
>  		 * Disable the plane to let hw background color win blend. */
>  		igt_plane_set_fb(plane, NULL);
> -		igt_crtc_set_background(plane->pipe, PURPLE64);
> +		igt_pipe_set_background(plane->pipe, PURPLE64);
>  		igt_display_commit2(display, COMMIT_UNIVERSAL);
>  
>  		/* Try few other background colors */
> -		igt_crtc_set_background(plane->pipe, CYAN64);
> +		igt_pipe_set_background(plane->pipe, CYAN64);
>  		igt_display_commit2(display, COMMIT_UNIVERSAL);
>  
> -		igt_crtc_set_background(plane->pipe, YELLOW64);
> +		igt_pipe_set_background(plane->pipe, YELLOW64);
>  		igt_display_commit2(display, COMMIT_UNIVERSAL);
>  
> -		igt_crtc_set_background(plane->pipe, RED64);
> +		igt_pipe_set_background(plane->pipe, RED64);
>  		igt_display_commit2(display, COMMIT_UNIVERSAL);
>  
> -		igt_crtc_set_background(plane->pipe, GREEN64);
> +		igt_pipe_set_background(plane->pipe, GREEN64);
>  		igt_display_commit2(display, COMMIT_UNIVERSAL);
>  
> -		igt_crtc_set_background(plane->pipe, BLUE64);
> +		igt_pipe_set_background(plane->pipe, BLUE64);
>  		igt_display_commit2(display, COMMIT_UNIVERSAL);
>  
> -		igt_crtc_set_background(plane->pipe, WHITE64);
> +		igt_pipe_set_background(plane->pipe, WHITE64);
>  		igt_display_commit2(display, COMMIT_UNIVERSAL);
>  
>  		valid_tests++;
> -- 
> 2.8.0.rc3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 6/7] lib/igt_kms: rename igt_crtc_set_background

[Lionel Landwerlin]

On 23/03/16 12:27, Daniel Vetter wrote:
> On Wed, Mar 23, 2016 at 11:38:09AM +0000, Lionel Landwerlin wrote:
>> We're a bit inconsistent with functions named igt_crtc_* and other
>> named igt_pipe_* even though they apply to the same objects.
>>
>> It seems most of the kernel/igt is using the pipe denomination. Let's
>> rename the igt_crtc_* into igt_pipe_*.
>>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Tbh I'd go the other direction. pipe is use in Bspec, and hence why it's
> used a lot in our i915 code. And also why we leaked it into igt_kms. But
> the drm thing a pipe is represented by is the CRTC. With others starting
> to use igt on non-intel drivers I think it'd be better to move into the
> other direction, and rename all the pipe stuff to CRTC. Or well most of it
> (I think we have some intel-specific concepts mixed in here).
> -Daniel

Thanks,

I also based my assumption on :

$ git grep igt_crtc_ | wc -l
19

$ git grep igt_pipe_ | wc -l
240

So maybe I'll submit another series for that as this would generate a 
fair amount of noise :(


>> ---
>>   lib/igt_kms.c                     | 28 ++++++++++++++--------------
>>   lib/igt_kms.h                     | 29 ++++++++++++++---------------
>>   tests/kms_crtc_background_color.c | 18 +++++++++---------
>>   3 files changed, 37 insertions(+), 38 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index e3488f1..1e0585f 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -160,7 +160,7 @@ static const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
>>   	"rotation"
>>   };
>>   
>> -static const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
>> +static const char *igt_pipe_prop_names[IGT_NUM_PIPE_PROPS] = {
>>   	"background_color",
>>   	"DEGAMMA_LUT",
>>   	"CTM",
>> @@ -1262,8 +1262,8 @@ void igt_display_init(igt_display_t *display, int drm_fd)
>>   		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);
>> +				      pipe->properties, igt_pipe_prop_names,
>> +				      IGT_NUM_PIPE_PROPS);
>>   		if (pipe->crtc_id) {
>>   			uint64_t prop_value;
>>   
>> @@ -1835,7 +1835,7 @@ static int igt_output_commit(igt_output_t *output,
>>   
>>   	if (pipe->background_changed) {
>>   		igt_crtc_set_property(output,
>> -				      pipe->properties[IGT_CRTC_BACKGROUND],
>> +				      pipe->properties[IGT_PIPE_BACKGROUND],
>>   				      pipe->background);
>>   
>>   		pipe->background_changed = false;
>> @@ -1843,11 +1843,11 @@ static int igt_output_commit(igt_output_t *output,
>>   
>>   	if (pipe->color_mgmt_changed) {
>>   		igt_crtc_set_property(output,
>> -				      pipe->properties[IGT_CRTC_DEGAMMA_LUT],
>> +				      pipe->properties[IGT_PIPE_DEGAMMA_LUT],
>>   				      pipe->degamma_blob);
>> -		igt_crtc_set_property(output, pipe->properties[IGT_CRTC_CTM],
>> +		igt_crtc_set_property(output, pipe->properties[IGT_PIPE_CTM],
>>   				      pipe->ctm_blob);
>> -		igt_crtc_set_property(output, pipe->properties[IGT_CRTC_GAMMA_LUT],
>> +		igt_crtc_set_property(output, pipe->properties[IGT_PIPE_GAMMA_LUT],
>>   				      pipe->gamma_blob);
>>   
>>   		pipe->color_mgmt_changed = false;
>> @@ -1881,12 +1881,12 @@ static int igt_output_commit(igt_output_t *output,
>>   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_atomic_populate_pipe_req(req, pipe, IGT_PIPE_BACKGROUND, pipe->background);
>>   
>>   	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);
>> +		igt_atomic_populate_pipe_req(req, pipe, IGT_PIPE_DEGAMMA_LUT, pipe->degamma_blob);
>> +		igt_atomic_populate_pipe_req(req, pipe, IGT_PIPE_CTM, pipe->ctm_blob);
>> +		igt_atomic_populate_pipe_req(req, pipe, IGT_PIPE_GAMMA_LUT, pipe->gamma_blob);
>>   	}
>>   
>>   	pipe->background_changed = false;
>> @@ -2316,7 +2316,7 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
>>   }
>>   
>>   /**
>> - * igt_crtc_set_background:
>> + * igt_pipe_set_background:
>>    * @pipe: pipe pointer to which background color to be set
>>    * @background: background color value in BGR 16bpc
>>    *
>> @@ -2325,11 +2325,11 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
>>    * property.
>>    * For example to get red as background, set background = 0x00000000FFFF.
>>    */
>> -void igt_crtc_set_background(igt_pipe_t *pipe, uint64_t background)
>> +void igt_pipe_set_background(igt_pipe_t *pipe, uint64_t background)
>>   {
>>   	igt_display_t *display = pipe->display;
>>   
>> -	LOG(display, "%s.%d: crtc_set_background(%"PRIx64")\n",
>> +	LOG(display, "%s.%d: pipe_set_background(%"PRIx64")\n",
>>   	    kmstest_pipe_name(pipe->pipe),
>>   	    pipe->pipe, background);
>>   
>> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>> index 4d3abf6..27c87ea 100644
>> --- a/lib/igt_kms.h
>> +++ b/lib/igt_kms.h
>> @@ -106,12 +106,12 @@ 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_CRTC_CTM,
>> -       IGT_CRTC_DEGAMMA_LUT,
>> -       IGT_CRTC_GAMMA_LUT,
>> -       IGT_NUM_CRTC_PROPS
>> +enum igt_atomic_pipe_properties {
>> +       IGT_PIPE_BACKGROUND = 0,
>> +       IGT_PIPE_CTM,
>> +       IGT_PIPE_DEGAMMA_LUT,
>> +       IGT_PIPE_GAMMA_LUT,
>> +       IGT_NUM_PIPE_PROPS
>>   };
>>   
>>   enum igt_atomic_connector_properties {
>> @@ -262,7 +262,7 @@ struct igt_pipe {
>>   
>>   	uint32_t crtc_id;
>>   
>> -	uint32_t properties[IGT_NUM_CRTC_PROPS];
>> +	uint32_t properties[IGT_NUM_PIPE_PROPS];
>>   };
>>   
>>   typedef struct {
>> @@ -305,6 +305,12 @@ igt_plane_t *igt_output_get_plane(igt_output_t *output, enum igt_plane plane);
>>   bool igt_pipe_get_property(igt_pipe_t *pipe, const char *name,
>>   			   uint32_t *prop_id, uint64_t *value,
>>   			   drmModePropertyPtr *prop);
>> +void igt_pipe_set_background(igt_pipe_t *pipe, uint64_t background);
>> +
>> +static inline bool igt_pipe_supports_background(igt_pipe_t *pipe)
>> +{
>> +	return pipe->properties[IGT_PIPE_BACKGROUND] != 0;
>> +}
>>   
>>   static inline bool igt_plane_supports_rotation(igt_plane_t *plane)
>>   {
>> @@ -320,18 +326,11 @@ void igt_plane_set_position(igt_plane_t *plane, int x, int y);
>>   void igt_plane_set_size(igt_plane_t *plane, int w, int h);
>>   void igt_plane_set_panning(igt_plane_t *plane, int x, int y);
>>   void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation);
>> -void igt_crtc_set_background(igt_pipe_t *pipe, uint64_t background);
>>   void igt_fb_set_position(struct igt_fb *fb, igt_plane_t *plane,
>>   	uint32_t x, uint32_t y);
>>   void igt_fb_set_size(struct igt_fb *fb, igt_plane_t *plane,
>>   	uint32_t w, uint32_t h);
>>   
>> -static inline bool igt_pipe_supports_background(igt_pipe_t *pipe)
>> -{
>> -	return pipe->properties[IGT_CRTC_BACKGROUND] != 0;
>> -}
>> -
>> -
>>   void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>>   
>>   #define for_each_connected_output(display, output)		\
>> @@ -362,7 +361,7 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>>    * igt_atomic_populate_pipe_req:
>>    * @req: A pointer to drmModeAtomicReq
>>    * @pipe: A pointer igt_pipe_t
>> - * @prop: one of igt_atomic_crtc_properties
>> + * @prop: one of igt_atomic_pipe_properties
>>    * @value: the value to add
>>    */
>>   #define igt_atomic_populate_pipe_req(req, pipe, prop, value) \
>> diff --git a/tests/kms_crtc_background_color.c b/tests/kms_crtc_background_color.c
>> index 055f935..0bf205e 100644
>> --- a/tests/kms_crtc_background_color.c
>> +++ b/tests/kms_crtc_background_color.c
>> @@ -97,7 +97,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
>>   	igt_assert(fb_id);
>>   
>>   	/* To make FB pixel win with background color, set alpha as full opaque */
>> -	igt_crtc_set_background(plane->pipe, pipe_background_color);
>> +	igt_pipe_set_background(plane->pipe, pipe_background_color);
>>   	if (opaque_buffer)
>>   		alpha = 1.0;    /* alpha 1 is fully opque */
>>   	else
>> @@ -117,7 +117,7 @@ static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane)
>>   
>>   	igt_remove_fb(data->gfx_fd, &data->fb);
>>   
>> -	igt_crtc_set_background(plane->pipe, BLACK64);
>> +	igt_pipe_set_background(plane->pipe, BLACK64);
>>   	igt_plane_set_fb(plane, NULL);
>>   	igt_output_set_pipe(output, PIPE_ANY);
>>   
>> @@ -147,26 +147,26 @@ static void test_crtc_background(data_t *data)
>>   		/* Now set background without using a plane, i.e.,
>>   		 * Disable the plane to let hw background color win blend. */
>>   		igt_plane_set_fb(plane, NULL);
>> -		igt_crtc_set_background(plane->pipe, PURPLE64);
>> +		igt_pipe_set_background(plane->pipe, PURPLE64);
>>   		igt_display_commit2(display, COMMIT_UNIVERSAL);
>>   
>>   		/* Try few other background colors */
>> -		igt_crtc_set_background(plane->pipe, CYAN64);
>> +		igt_pipe_set_background(plane->pipe, CYAN64);
>>   		igt_display_commit2(display, COMMIT_UNIVERSAL);
>>   
>> -		igt_crtc_set_background(plane->pipe, YELLOW64);
>> +		igt_pipe_set_background(plane->pipe, YELLOW64);
>>   		igt_display_commit2(display, COMMIT_UNIVERSAL);
>>   
>> -		igt_crtc_set_background(plane->pipe, RED64);
>> +		igt_pipe_set_background(plane->pipe, RED64);
>>   		igt_display_commit2(display, COMMIT_UNIVERSAL);
>>   
>> -		igt_crtc_set_background(plane->pipe, GREEN64);
>> +		igt_pipe_set_background(plane->pipe, GREEN64);
>>   		igt_display_commit2(display, COMMIT_UNIVERSAL);
>>   
>> -		igt_crtc_set_background(plane->pipe, BLUE64);
>> +		igt_pipe_set_background(plane->pipe, BLUE64);
>>   		igt_display_commit2(display, COMMIT_UNIVERSAL);
>>   
>> -		igt_crtc_set_background(plane->pipe, WHITE64);
>> +		igt_pipe_set_background(plane->pipe, WHITE64);
>>   		igt_display_commit2(display, COMMIT_UNIVERSAL);
>>   
>>   		valid_tests++;
>> -- 
>> 2.8.0.rc3
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH i-g-t 7/7] lib/igt_kms: add some missing documentation

[Lionel Landwerlin]

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 lib/igt_kms.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 1e0585f..ded614f 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -2092,6 +2092,13 @@ void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode)
 	output->use_override_mode = true;
 }
 
+/**
+ * igt_output_set_pipe:
+ * @output: Output which the pipe will be attached to
+ * @pipe: Pipe to attach to the output
+ *
+ * Attach a pipe to an output.
+ */
 void igt_output_set_pipe(igt_output_t *output, enum pipe pipe)
 {
 	igt_display_t *display = output->display;
@@ -2114,6 +2121,13 @@ igt_plane_t *igt_output_get_plane(igt_output_t *output, enum igt_plane plane)
 	return igt_pipe_get_plane(pipe, plane);
 }
 
+/**
+ * igt_plane_set_fb:
+ * @plane: Plane which the framebuffer with be attached to
+ * @fb: Framebuffer to attach to the plane
+ *
+ * Attach a framebufer to a plane.
+ */
 void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb)
 {
 	igt_pipe_t *pipe = plane->pipe;
@@ -2143,6 +2157,14 @@ void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb)
 	plane->size_changed = true;
 }
 
+/**
+ * igt_plane_set_position:
+ * @plane: A plane
+ * @x: New X position of the plane
+ * @y: New Y position of the plane
+ *
+ * Set position of a plane.
+ */
 void igt_plane_set_position(igt_plane_t *plane, int x, int y)
 {
 	igt_pipe_t *pipe = plane->pipe;
@@ -2232,6 +2254,15 @@ void igt_fb_set_size(struct igt_fb *fb, igt_plane_t *plane,
 	plane->fb_changed = true;
 }
 
+/**
+ * igt_pipe_set_rotation:
+ * @plane: A plane on which to set the panning
+ * @x: x
+ * @y: y
+ *
+ * Sets the panning of a plane. The new panning will be committed at
+ * plane commit time via drmModeSetCrtc().
+ */
 void igt_plane_set_panning(igt_plane_t *plane, int x, int y)
 {
 	igt_pipe_t *pipe = plane->pipe;
@@ -2263,6 +2294,13 @@ static const char *rotation_name(igt_rotation_t rotation)
 	}
 }
 
+/**
+ * igt_pipe_set_rotation:
+ * @plane: A plane on which to set the rotation
+ * @rotation:
+ *
+ * Sets the rotation of a plane.
+ */
 void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation)
 {
 	igt_pipe_t *pipe = plane->pipe;
@@ -2294,6 +2332,17 @@ igt_pipe_replace_blob(igt_pipe_t *pipe, uint64_t *blob, void *ptr, size_t length
 	*blob = blob_id;
 }
 
+/**
+ * igt_pipe_set_degamma_lut:
+ * @pipe: A pipe on which to set a gamma LUT
+ * @ptr: A pointer to an array of #drm_color_lut elements
+ * @size: The length of the array in bytes or 0
+ *
+ * Sets a degamma LUT onto the pipe. A new DRM blob is created to hold
+ * the degamma LUT and the data pointed by ptr is copied into it. The
+ * previous DRM blob set on the pipe is destroyed. Call this function
+ * with size = 0 to disable the degamma LUT on @pipe.
+ */
 void
 igt_pipe_set_degamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
 {
@@ -2301,6 +2350,18 @@ igt_pipe_set_degamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
 	pipe->color_mgmt_changed = 1;
 }
 
+/**
+ * igt_pipe_set_ctm_matrix:
+ * @pipe: A pipe on which to set a transformation matrix
+ * @ptr: A pointer to a #drm_color_ctm structure
+ * @size: The size of the #drm_color_ctm structure or 0
+ *
+ * Sets a transformation matrix onto the pipe. A new DRM blob is
+ * created to hold the matrix and the data pointed by ptr is copied
+ * into it. The previous DRM blob set on the pipe is destroyed. Call
+ * this function with size = 0 to disable the transformation matrix on
+ * @pipe.
+ */
 void
 igt_pipe_set_ctm_matrix(igt_pipe_t *pipe, void *ptr, size_t length)
 {
@@ -2308,6 +2369,17 @@ igt_pipe_set_ctm_matrix(igt_pipe_t *pipe, void *ptr, size_t length)
 	pipe->color_mgmt_changed = 1;
 }
 
+/**
+ * igt_pipe_set_gamma_lut:
+ * @pipe: A pipe on which to set a gamma LUT
+ * @ptr: A pointer to an array of #drm_color_lut elements
+ * @size: The length of the array in bytes or 0
+ *
+ * Sets a gamma LUT onto the pipe. A new DRM blob is created to hold
+ * the gamma LUT and the data pointed by ptr is copied into it. The
+ * previous DRM blob set on the pipe is destroyed. Call this function
+ * with size = 0 to disable the gamma LUT on @pipe.
+ */
 void
 igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
 {
-- 
2.8.0.rc3

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