[PATCH 1/6] igt_kms: don't get drmModeRes just to free it later

[PATCH 1/6] igt_kms: don't get drmModeRes just to free it later

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Stress testing malloc is not our goal :)

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 lib/igt_kms.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 740b5dd..8ab729b 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -276,21 +276,11 @@ void igt_set_vt_graphics_mode(void)
 int kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
 				      drmModeModeInfo *mode)
 {
-	drmModeRes *resources;
 	int i;
 
-	resources = drmModeGetResources(drm_fd);
-	if (!resources) {
-		perror("drmModeGetResources failed");
-
-		return -1;
-	}
-
 	if (!connector->count_modes) {
 		fprintf(stderr, "no modes for connector %d\n",
 			connector->connector_id);
-		drmModeFreeResources(resources);
-
 		return -1;
 	}
 
@@ -303,8 +293,6 @@ int kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
 		}
 	}
 
-	drmModeFreeResources(resources);
-
 	return 0;
 }
 
-- 
2.0.1

[PATCH 2/6] igt_kms: pass drm_fd instead of igt_display_t on some functions

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Since these functions only really use the drm_fd. The goal is to be
able to reuse these functions on programs that don't use the
igt_display_t structure.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 lib/igt_kms.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 8ab729b..5dd67fe 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -593,20 +593,19 @@ static void igt_output_refresh(igt_output_t *output)
 }
 
 static bool
-get_property(igt_display_t *display,
-	     uint32_t object_id, uint32_t object_type, const char *name,
-	     uint32_t *prop_id /* out */, uint64_t *value /* out */)
+get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
+	     const char *name, uint32_t *prop_id /* out */,
+	     uint64_t *value /* out */)
 {
 	drmModeObjectPropertiesPtr proplist;
 	drmModePropertyPtr prop = NULL;
 	bool found = false;
 	int i;
 
-	proplist = drmModeObjectGetProperties(display->drm_fd,
-					      object_id, object_type);
+	proplist = drmModeObjectGetProperties(drm_fd, object_id, object_type);
 	for (i = 0; i < proplist->count_props; i++) {
 		drmModeFreeProperty(prop);
-		prop = drmModeGetProperty(display->drm_fd, proplist->props[i]);
+		prop = drmModeGetProperty(drm_fd, proplist->props[i]);
 		if (!prop)
 			continue;
 
@@ -627,10 +626,10 @@ out:
 }
 
 static bool
-get_plane_property(igt_display_t *display, uint32_t plane_id, const char *name,
+get_plane_property(int drm_fd, uint32_t plane_id, const char *name,
 		   uint32_t *prop_id /* out */, uint64_t *value /* out */)
 {
-	return get_property(display, plane_id, DRM_MODE_OBJECT_PLANE,
+	return get_property(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE,
 			    name, prop_id, value);
 }
 
@@ -649,12 +648,12 @@ igt_plane_set_property(igt_plane_t *plane, uint32_t prop_id, uint64_t value)
  * find a type property, then the kernel doesn't support universal
  * planes and we know the plane is an overlay/sprite.
  */
-static int get_drm_plane_type(igt_display_t *display, uint32_t plane_id)
+static int get_drm_plane_type(int drm_fd, uint32_t plane_id)
 {
 	uint64_t value;
 	bool has_prop;
 
-	has_prop = get_plane_property(display, plane_id, "type",
+	has_prop = get_plane_property(drm_fd, plane_id, "type",
 				      NULL /* prop_id */, &value);
 	if (has_prop)
 		return (int)value;
@@ -710,7 +709,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
 				continue;
 			}
 
-			type = get_drm_plane_type(display,
+			type = get_drm_plane_type(display->drm_fd,
 						  plane_resources->planes[j]);
 			switch (type) {
 			case DRM_PLANE_TYPE_PRIMARY:
@@ -741,7 +740,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
 			plane->pipe = pipe;
 			plane->drm_plane = drm_plane;
 
-			get_plane_property(display, drm_plane->plane_id,
+			get_plane_property(display->drm_fd, drm_plane->plane_id,
 					   "rotation",
 					   &plane->rotation_property,
 					   &prop_value);
-- 
2.0.1

[PATCH 3/6] igt_kms: optionally return the property from get_property

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

So we can use this function on places that also need the property
pointer, without having to call drmModeGetProperty() again with the
returned id.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 lib/igt_kms.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 5dd67fe..664b9e8 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -595,42 +595,46 @@ static void igt_output_refresh(igt_output_t *output)
 static bool
 get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
 	     const char *name, uint32_t *prop_id /* out */,
-	     uint64_t *value /* out */)
+	     uint64_t *value /* out */, drmModePropertyPtr *prop /* out */)
 {
 	drmModeObjectPropertiesPtr proplist;
-	drmModePropertyPtr prop = NULL;
+	drmModePropertyPtr _prop;
 	bool found = false;
 	int i;
 
 	proplist = drmModeObjectGetProperties(drm_fd, object_id, object_type);
 	for (i = 0; i < proplist->count_props; i++) {
-		drmModeFreeProperty(prop);
-		prop = drmModeGetProperty(drm_fd, proplist->props[i]);
-		if (!prop)
+		_prop = drmModeGetProperty(drm_fd, proplist->props[i]);
+		if (!_prop)
 			continue;
 
-		if (strcmp(prop->name, name) == 0) {
+		if (strcmp(_prop->name, name) == 0) {
 			found = true;
 			if (prop_id)
 				*prop_id = proplist->props[i];
 			if (value)
 				*value = proplist->prop_values[i];
-			goto out;
+			if (prop)
+				*prop = _prop;
+			else
+				drmModeFreeProperty(_prop);
+
+			break;
 		}
+		drmModeFreeProperty(_prop);
 	}
 
-out:
-	drmModeFreeProperty(prop);
 	drmModeFreeObjectProperties(proplist);
 	return found;
 }
 
 static bool
 get_plane_property(int drm_fd, uint32_t plane_id, const char *name,
-		   uint32_t *prop_id /* out */, uint64_t *value /* out */)
+		   uint32_t *prop_id /* out */, uint64_t *value /* out */,
+		   drmModePropertyPtr *prop /* out */)
 {
 	return get_property(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE,
-			    name, prop_id, value);
+			    name, prop_id, value, prop);
 }
 
 static void
@@ -654,7 +658,7 @@ static int get_drm_plane_type(int drm_fd, uint32_t plane_id)
 	bool has_prop;
 
 	has_prop = get_plane_property(drm_fd, plane_id, "type",
-				      NULL /* prop_id */, &value);
+				      NULL /* prop_id */, &value, NULL);
 	if (has_prop)
 		return (int)value;
 
@@ -743,7 +747,8 @@ 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);
+					   &prop_value,
+					   NULL);
 			plane->rotation = (igt_rotation_t)prop_value;
 		}
 
-- 
2.0.1

[PATCH 4/6] igt_kms: document and export igt_get_property()

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

So we can use it on pm_rpm.c.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 lib/igt_kms.c | 27 +++++++++++++++++++++------
 lib/igt_kms.h |  4 ++++
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 664b9e8..6cca7e8 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -592,10 +592,25 @@ static void igt_output_refresh(igt_output_t *output)
 	display->pipes_in_use |= 1 << output->config.pipe;
 }
 
-static bool
-get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
-	     const char *name, uint32_t *prop_id /* out */,
-	     uint64_t *value /* out */, drmModePropertyPtr *prop /* out */)
+/**
+ * igt_get_property:
+ * @drm_fd: drm file descriptor
+ * @object_id: object whose properties we're going to get
+ * @object_type: type of obj_id (DRM_MODE_OBJECT_*)
+ * @name: name of the property we're going to get
+ * @prop_id: if not NULL, returns the property id
+ * @value: if not NULL, returns the property value
+ * @prop: if not NULL, returns the property, and the caller will have to free
+ *        it manually.
+ *
+ * Finds a property with the given name on the given object.
+ *
+ * Returns: true in case we found something.
+ */
+bool
+igt_get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
+		 const char *name, uint32_t *prop_id /* out */,
+		 uint64_t *value /* out */, drmModePropertyPtr *prop /* out */)
 {
 	drmModeObjectPropertiesPtr proplist;
 	drmModePropertyPtr _prop;
@@ -633,8 +648,8 @@ get_plane_property(int drm_fd, uint32_t plane_id, const char *name,
 		   uint32_t *prop_id /* out */, uint64_t *value /* out */,
 		   drmModePropertyPtr *prop /* out */)
 {
-	return get_property(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE,
-			    name, prop_id, value, prop);
+	return igt_get_property(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE,
+				name, prop_id, value, prop);
 }
 
 static void
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 08b46ab..f8c500e 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -255,5 +255,9 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
 void igt_enable_connectors(void);
 void igt_reset_connectors(void);
 
+bool igt_get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
+		      const char *name, uint32_t *prop_id, uint64_t *value,
+		      drmModePropertyPtr *prop);
+
 #endif /* __IGT_KMS_H__ */
 
-- 
2.0.1

[PATCH 5/6] tests/pm_rpm: use igt_get_property()

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

So we can reduce the code size.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 tests/pm_rpm.c | 97 ++++++++++++++++++++++++----------------------------------
 1 file changed, 40 insertions(+), 57 deletions(-)

diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
index 0633476..800ab4b 100644
--- a/tests/pm_rpm.c
+++ b/tests/pm_rpm.c
@@ -348,29 +348,25 @@ static void enable_one_screen(struct mode_set_data *data)
 static drmModePropertyBlobPtr get_connector_edid(drmModeConnectorPtr connector,
 						 int index)
 {
-	unsigned int i;
-	drmModeObjectPropertiesPtr props;
-	drmModePropertyBlobPtr ret = NULL;
+	bool found;
+	uint64_t prop_value;
+	drmModePropertyPtr prop;
+	drmModePropertyBlobPtr blob = NULL;
 
-	props = drmModeObjectGetProperties(drm_fd, connector->connector_id,
-					   DRM_MODE_OBJECT_CONNECTOR);
+	found = igt_get_property(drm_fd, connector->connector_id,
+				 DRM_MODE_OBJECT_CONNECTOR, "EDID",
+				 NULL, &prop_value, &prop);
 
-	for (i = 0; i < props->count_props; i++) {
-		drmModePropertyPtr prop = drmModeGetProperty(drm_fd,
-							     props->props[i]);
+	if (found) {
+		igt_assert(prop->flags & DRM_MODE_PROP_BLOB);
+		igt_assert(prop->count_blobs == 0);
 
-		if (strcmp(prop->name, "EDID") == 0) {
-			igt_assert(prop->flags & DRM_MODE_PROP_BLOB);
-			igt_assert(prop->count_blobs == 0);
-			ret = drmModeGetPropertyBlob(drm_fd,
-						     props->prop_values[i]);
-		}
+		blob = drmModeGetPropertyBlob(drm_fd, prop_value);
 
 		drmModeFreeProperty(prop);
 	}
 
-	drmModeFreeObjectProperties(props);
-	return ret;
+	return blob;
 }
 
 static void init_mode_set_data(struct mode_set_data *data)
@@ -1529,52 +1525,39 @@ static void cursor_subtest(bool dpms)
 
 static enum plane_type get_plane_type(uint32_t plane_id)
 {
-	drmModeObjectPropertiesPtr props;
-	int i, j;
+	int i;
+	bool found;
+	uint64_t prop_value;
+	drmModePropertyPtr prop;
+	const char *enum_name = NULL;
 	enum plane_type type;
-	bool found = false;
-
-	props = drmModeObjectGetProperties(drm_fd, plane_id,
-					   DRM_MODE_OBJECT_PLANE);
-	igt_assert(props);
-
-	for (i = 0; i < props->count_props && !found; i++) {
-		drmModePropertyPtr prop;
-		const char *enum_name = NULL;
-
-		prop = drmModeGetProperty(drm_fd, props->props[i]);
-		igt_assert(prop);
-
-		if (strcmp(prop->name, "type") == 0) {
-			igt_assert(prop->flags & DRM_MODE_PROP_ENUM);
-			igt_assert(props->prop_values[i] < prop->count_enums);
-
-			for (j = 0; j < prop->count_enums; j++) {
-				if (prop->enums[j].value ==
-				    props->prop_values[i]) {
-					enum_name = prop->enums[j].name;
-					break;
-				}
-			}
-			igt_assert(enum_name);
-
-			if (strcmp(enum_name, "Overlay") == 0)
-				type = PLANE_OVERLAY;
-			else if (strcmp(enum_name, "Primary") == 0)
-				type = PLANE_PRIMARY;
-			else if (strcmp(enum_name, "Cursor") == 0)
-				type = PLANE_CURSOR;
-			else
-				igt_assert(0);
 
-			found = true;
-		}
+	found = igt_get_property(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE,
+				 "type", NULL, &prop_value, &prop);
+	igt_assert(found);
 
-		drmModeFreeProperty(prop);
+	igt_assert(prop->flags & DRM_MODE_PROP_ENUM);
+	igt_assert(prop_value < prop->count_enums);
+
+	for (i = 0; i < prop->count_enums; i++) {
+		if (prop->enums[i].value == prop_value) {
+			enum_name = prop->enums[i].name;
+			break;
+		}
 	}
-	igt_assert(found);
+	igt_assert(enum_name);
+
+	if (strcmp(enum_name, "Overlay") == 0)
+		type = PLANE_OVERLAY;
+	else if (strcmp(enum_name, "Primary") == 0)
+		type = PLANE_PRIMARY;
+	else if (strcmp(enum_name, "Cursor") == 0)
+		type = PLANE_CURSOR;
+	else
+		igt_assert(0);
+
+	drmModeFreeProperty(prop);
 
-	drmModeFreeObjectProperties(props);
 	return type;
 }
 
-- 
2.0.1

[PATCH 6/6] igt_kms: add igt_unset_all_crtcs()

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Both pm_rpm.c and pm_lpsp.c call it "disable_all_screens", but let's
give it a name that better describes what the implementation does.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 lib/igt_kms.c   | 18 ++++++++++++++++++
 lib/igt_kms.h   |  1 +
 tests/pm_lpsp.c | 17 +++--------------
 tests/pm_rpm.c  |  8 +-------
 4 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 6cca7e8..678b3cd 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1560,3 +1560,21 @@ void igt_reset_connectors(void)
 
 	close(drm_fd);
 }
+
+/**
+ * igt_unset_all_crtcs:
+ * @drm_fd: the DRM fd
+ * @resources: libdrm resources pointer
+ *
+ * Disables all the screens.
+ */
+void igt_unset_all_crtcs(int drm_fd, drmModeResPtr resources)
+{
+	int i, rc;
+
+	for (i = 0; i < resources->count_crtcs; i++) {
+		rc = drmModeSetCrtc(drm_fd, resources->crtcs[i], -1, 0, 0, NULL,
+				    0, NULL);
+		igt_assert(rc == 0);
+	}
+}
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index f8c500e..43e84f6 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -258,6 +258,7 @@ void igt_reset_connectors(void);
 bool igt_get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
 		      const char *name, uint32_t *prop_id, uint64_t *value,
 		      drmModePropertyPtr *prop);
+void igt_unset_all_crtcs(int drm_fd, drmModeResPtr resources);
 
 #endif /* __IGT_KMS_H__ */
 
diff --git a/tests/pm_lpsp.c b/tests/pm_lpsp.c
index 9d3884c..b3a21ea 100644
--- a/tests/pm_lpsp.c
+++ b/tests/pm_lpsp.c
@@ -70,22 +70,11 @@ static bool lpsp_is_enabled(int drm_fd)
 	return !(val & HSW_PWR_WELL_STATE_ENABLED);
 }
 
-static void disable_all_screens(int drm_fd, drmModeResPtr drm_resources)
-{
-	int i, rc;
-
-	for (i = 0; i < drm_resources->count_crtcs; i++) {
-		rc = drmModeSetCrtc(drm_fd, drm_resources->crtcs[i], -1, 0, 0,
-				    NULL, 0, NULL);
-		igt_assert(rc == 0);
-	}
-}
-
 /* The LPSP mode is all about an enabled pipe, but we expect to also be in the
  * low power mode when no pipes are enabled, so do this check anyway. */
 static void screens_disabled_subtest(int drm_fd, drmModeResPtr drm_res)
 {
-	disable_all_screens(drm_fd, drm_res);
+	igt_unset_all_crtcs(drm_fd, drm_res);
 	igt_assert(lpsp_is_enabled(drm_fd));
 }
 
@@ -131,7 +120,7 @@ static void edp_subtest(int drm_fd, drmModeResPtr drm_res,
 		.name = "Custom 1024x768",
 	};
 
-	disable_all_screens(drm_fd, drm_res);
+	igt_unset_all_crtcs(drm_fd, drm_res);
 
 	for (i = 0; i < drm_res->count_connectors; i++) {
 		drmModeConnectorPtr c = drm_connectors[i];
@@ -193,7 +182,7 @@ static void non_edp_subtest(int drm_fd, drmModeResPtr drm_res,
 	uint32_t connector_id = 0, crtc_id = 0, buffer_id = 0;
 	drmModeModeInfoPtr mode = NULL;
 
-	disable_all_screens(drm_fd, drm_res);
+	igt_unset_all_crtcs(drm_fd, drm_res);
 
 	for (i = 0; i < drm_res->count_connectors; i++) {
 		drmModeConnectorPtr c = drm_connectors[i];
diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
index 800ab4b..84c71bd 100644
--- a/tests/pm_rpm.c
+++ b/tests/pm_rpm.c
@@ -232,13 +232,7 @@ static void disable_all_screens_dpms(struct mode_set_data *data)
 
 static void disable_all_screens(struct mode_set_data *data)
 {
-	int i, rc;
-
-	for (i = 0; i < data->res->count_crtcs; i++) {
-		rc = drmModeSetCrtc(drm_fd, data->res->crtcs[i], -1, 0, 0,
-				    NULL, 0, NULL);
-		igt_assert(rc == 0);
-	}
+	igt_unset_all_crtcs(drm_fd, data->res);
 }
 
 static struct scanout_fb *create_fb(struct mode_set_data *data, int width,
-- 
2.0.1

Re: [PATCH 4/6] igt_kms: document and export igt_get_property()

[Daniel Vetter]

On Thu, Aug 07, 2014 at 10:09:05AM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> So we can use it on pm_rpm.c.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

The idea behind the igt_kms_ and kmstest_ prefix split is that kmstests_
are the lower-level helpers wrapping around just the libdrm interfaces
(simalar to the piles of ioctl wrappers we have) and igt_kms_ provides
higher-level code on top.

So I think for this we should use kmstest_ as the prefix.
-Daniel

> ---
>  lib/igt_kms.c | 27 +++++++++++++++++++++------
>  lib/igt_kms.h |  4 ++++
>  2 files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 664b9e8..6cca7e8 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -592,10 +592,25 @@ static void igt_output_refresh(igt_output_t *output)
>  	display->pipes_in_use |= 1 << output->config.pipe;
>  }
>  
> -static bool
> -get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
> -	     const char *name, uint32_t *prop_id /* out */,
> -	     uint64_t *value /* out */, drmModePropertyPtr *prop /* out */)
> +/**
> + * igt_get_property:
> + * @drm_fd: drm file descriptor
> + * @object_id: object whose properties we're going to get
> + * @object_type: type of obj_id (DRM_MODE_OBJECT_*)
> + * @name: name of the property we're going to get
> + * @prop_id: if not NULL, returns the property id
> + * @value: if not NULL, returns the property value
> + * @prop: if not NULL, returns the property, and the caller will have to free
> + *        it manually.
> + *
> + * Finds a property with the given name on the given object.
> + *
> + * Returns: true in case we found something.
> + */
> +bool
> +igt_get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
> +		 const char *name, uint32_t *prop_id /* out */,
> +		 uint64_t *value /* out */, drmModePropertyPtr *prop /* out */)
>  {
>  	drmModeObjectPropertiesPtr proplist;
>  	drmModePropertyPtr _prop;
> @@ -633,8 +648,8 @@ get_plane_property(int drm_fd, uint32_t plane_id, const char *name,
>  		   uint32_t *prop_id /* out */, uint64_t *value /* out */,
>  		   drmModePropertyPtr *prop /* out */)
>  {
> -	return get_property(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE,
> -			    name, prop_id, value, prop);
> +	return igt_get_property(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE,
> +				name, prop_id, value, prop);
>  }
>  
>  static void
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 08b46ab..f8c500e 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -255,5 +255,9 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>  void igt_enable_connectors(void);
>  void igt_reset_connectors(void);
>  
> +bool igt_get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
> +		      const char *name, uint32_t *prop_id, uint64_t *value,
> +		      drmModePropertyPtr *prop);
> +
>  #endif /* __IGT_KMS_H__ */
>  
> -- 
> 2.0.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 6/6] igt_kms: add igt_unset_all_crtcs()

[Daniel Vetter]

On Thu, Aug 07, 2014 at 10:09:07AM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Both pm_rpm.c and pm_lpsp.c call it "disable_all_screens", but let's
> give it a name that better describes what the implementation does.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Same comment as earlier, this should have a kmstest_ prefix and needs to
be grouped together with those functions.
-Daniel

> ---
>  lib/igt_kms.c   | 18 ++++++++++++++++++
>  lib/igt_kms.h   |  1 +
>  tests/pm_lpsp.c | 17 +++--------------
>  tests/pm_rpm.c  |  8 +-------
>  4 files changed, 23 insertions(+), 21 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 6cca7e8..678b3cd 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1560,3 +1560,21 @@ void igt_reset_connectors(void)
>  
>  	close(drm_fd);
>  }
> +
> +/**
> + * igt_unset_all_crtcs:
> + * @drm_fd: the DRM fd
> + * @resources: libdrm resources pointer
> + *
> + * Disables all the screens.
> + */
> +void igt_unset_all_crtcs(int drm_fd, drmModeResPtr resources)
> +{
> +	int i, rc;
> +
> +	for (i = 0; i < resources->count_crtcs; i++) {
> +		rc = drmModeSetCrtc(drm_fd, resources->crtcs[i], -1, 0, 0, NULL,
> +				    0, NULL);
> +		igt_assert(rc == 0);
> +	}
> +}
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index f8c500e..43e84f6 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -258,6 +258,7 @@ void igt_reset_connectors(void);
>  bool igt_get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
>  		      const char *name, uint32_t *prop_id, uint64_t *value,
>  		      drmModePropertyPtr *prop);
> +void igt_unset_all_crtcs(int drm_fd, drmModeResPtr resources);
>  
>  #endif /* __IGT_KMS_H__ */
>  
> diff --git a/tests/pm_lpsp.c b/tests/pm_lpsp.c
> index 9d3884c..b3a21ea 100644
> --- a/tests/pm_lpsp.c
> +++ b/tests/pm_lpsp.c
> @@ -70,22 +70,11 @@ static bool lpsp_is_enabled(int drm_fd)
>  	return !(val & HSW_PWR_WELL_STATE_ENABLED);
>  }
>  
> -static void disable_all_screens(int drm_fd, drmModeResPtr drm_resources)
> -{
> -	int i, rc;
> -
> -	for (i = 0; i < drm_resources->count_crtcs; i++) {
> -		rc = drmModeSetCrtc(drm_fd, drm_resources->crtcs[i], -1, 0, 0,
> -				    NULL, 0, NULL);
> -		igt_assert(rc == 0);
> -	}
> -}
> -
>  /* The LPSP mode is all about an enabled pipe, but we expect to also be in the
>   * low power mode when no pipes are enabled, so do this check anyway. */
>  static void screens_disabled_subtest(int drm_fd, drmModeResPtr drm_res)
>  {
> -	disable_all_screens(drm_fd, drm_res);
> +	igt_unset_all_crtcs(drm_fd, drm_res);
>  	igt_assert(lpsp_is_enabled(drm_fd));
>  }
>  
> @@ -131,7 +120,7 @@ static void edp_subtest(int drm_fd, drmModeResPtr drm_res,
>  		.name = "Custom 1024x768",
>  	};
>  
> -	disable_all_screens(drm_fd, drm_res);
> +	igt_unset_all_crtcs(drm_fd, drm_res);
>  
>  	for (i = 0; i < drm_res->count_connectors; i++) {
>  		drmModeConnectorPtr c = drm_connectors[i];
> @@ -193,7 +182,7 @@ static void non_edp_subtest(int drm_fd, drmModeResPtr drm_res,
>  	uint32_t connector_id = 0, crtc_id = 0, buffer_id = 0;
>  	drmModeModeInfoPtr mode = NULL;
>  
> -	disable_all_screens(drm_fd, drm_res);
> +	igt_unset_all_crtcs(drm_fd, drm_res);
>  
>  	for (i = 0; i < drm_res->count_connectors; i++) {
>  		drmModeConnectorPtr c = drm_connectors[i];
> diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
> index 800ab4b..84c71bd 100644
> --- a/tests/pm_rpm.c
> +++ b/tests/pm_rpm.c
> @@ -232,13 +232,7 @@ static void disable_all_screens_dpms(struct mode_set_data *data)
>  
>  static void disable_all_screens(struct mode_set_data *data)
>  {
> -	int i, rc;
> -
> -	for (i = 0; i < data->res->count_crtcs; i++) {
> -		rc = drmModeSetCrtc(drm_fd, data->res->crtcs[i], -1, 0, 0,
> -				    NULL, 0, NULL);
> -		igt_assert(rc == 0);
> -	}
> +	igt_unset_all_crtcs(drm_fd, data->res);
>  }
>  
>  static struct scanout_fb *create_fb(struct mode_set_data *data, int width,
> -- 
> 2.0.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch