All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH libdrm 1/5] modetest: fix some compiler warnings
@ 2012-03-29 21:28 Paulo Zanoni
  2012-03-29 21:28 ` [PATCH libdrm 2/5] modetest: fix memory leak Paulo Zanoni
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Paulo Zanoni @ 2012-03-29 21:28 UTC (permalink / raw)
  To: dri-devel; +Cc: Paulo Zanoni

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

Use unsigned int instead of int:
- modetest.c:89:1: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
- modetest.c:97:1: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
- modetest.c:117:1: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
- modetest.c:772:16: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

The 'fd' variable is global, we don't need to pass it as an argument:
- modetest.c:698:40: warning: unused parameter ‘fd’ [-Wunused-parameter]

We don't use the 'modeset' variable:
- modetest.c:725:8: warning: variable ‘modeset’ set but not used [-Wunused-but-set-variable]

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 tests/modetest/modetest.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 1e4ec91..11f8d65 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -70,7 +70,7 @@ struct type_name {
 
 #define type_name_fn(res) \
 char * res##_str(int type) {			\
-	int i;						\
+	unsigned int i;					\
 	for (i = 0; i < ARRAY_SIZE(res##_names); i++) { \
 		if (res##_names[i].type == type)	\
 			return res##_names[i].name;	\
@@ -695,7 +695,7 @@ void usage(char *name)
 
 #define dump_resource(res) if (res) dump_##res()
 
-static int page_flipping_supported(int fd)
+static int page_flipping_supported(void)
 {
 	/*FIXME: generic ioctl needed? */
 	return 1;
@@ -722,8 +722,8 @@ int main(int argc, char **argv)
 	int encoders = 0, connectors = 0, crtcs = 0, framebuffers = 0;
 	int test_vsync = 0;
 	char *modules[] = { "i915", "radeon", "nouveau", "vmwgfx", "omapdrm" };
-	char *modeset = NULL;
-	int i, count = 0;
+	unsigned int i;
+	int count = 0;
 	struct connector con_args[2];
 	
 	opterr = 0;
@@ -748,7 +748,6 @@ int main(int argc, char **argv)
 			test_vsync = 1;
 			break;
 		case 's':
-			modeset = strdup(optarg);
 			con_args[count].crtc = -1;
 			if (sscanf(optarg, "%d:%64s",
 				   &con_args[count].id,
@@ -780,7 +779,7 @@ int main(int argc, char **argv)
 		}
 	}
 
-	if (test_vsync && !page_flipping_supported(fd)) {
+	if (test_vsync && !page_flipping_supported()) {
 		fprintf(stderr, "page flipping not supported by drm.\n");
 		return -1;
 	}
-- 
1.7.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH libdrm 2/5] modetest: fix memory leak
  2012-03-29 21:28 [PATCH libdrm 1/5] modetest: fix some compiler warnings Paulo Zanoni
@ 2012-03-29 21:28 ` Paulo Zanoni
  2012-03-30  2:09   ` Eugeni Dodonov
  2012-03-29 21:28 ` [PATCH libdrm 3/5] modetest: print more about our properties Paulo Zanoni
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Paulo Zanoni @ 2012-03-29 21:28 UTC (permalink / raw)
  To: dri-devel; +Cc: Paulo Zanoni

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

Don't "continue" without freeing the connector.

192 bytes in 6 blocks are indirectly lost in loss record 6 of 12
   at 0x4C2779D: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
   by 0x4E30DD8: drmMalloc (xf86drm.c:147)
   by 0x4E35024: drmAllocCpy (xf86drmMode.c:73)
   by 0x4E35D69: drmModeGetConnector (xf86drmMode.c:507)
   by 0x402F22: dump_connectors (modetest.c:181)
   by 0x40261B: main (modetest.c:801)

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 tests/modetest/modetest.c |   21 ++++++++++-----------
 1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 11f8d65..173d587 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -198,17 +198,16 @@ void dump_connectors(void)
 			printf("%s%d", j > 0 ? ", " : "", connector->encoders[j]);
 		printf("\n");
 
-		if (!connector->count_modes)
-			continue;
-
-		printf("  modes:\n");
-		printf("  name refresh (Hz) hdisp hss hse htot vdisp "
-		       "vss vse vtot)\n");
-		for (j = 0; j < connector->count_modes; j++)
-			dump_mode(&connector->modes[j]);
-
-		printf("  props:\n");
-		dump_props(connector);
+		if (connector->count_modes) {
+			printf("  modes:\n");
+			printf("  name refresh (Hz) hdisp hss hse htot vdisp "
+			       "vss vse vtot)\n");
+			for (j = 0; j < connector->count_modes; j++)
+				dump_mode(&connector->modes[j]);
+
+			printf("  props:\n");
+			dump_props(connector);
+		}
 
 		drmModeFreeConnector(connector);
 	}
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH libdrm 3/5] modetest: print more about our properties
  2012-03-29 21:28 [PATCH libdrm 1/5] modetest: fix some compiler warnings Paulo Zanoni
  2012-03-29 21:28 ` [PATCH libdrm 2/5] modetest: fix memory leak Paulo Zanoni
@ 2012-03-29 21:28 ` Paulo Zanoni
  2012-03-30  2:19   ` Eugeni Dodonov
  2012-03-29 21:28 ` [PATCH libdrm 4/5] Add support for generic object properties IOCTLs Paulo Zanoni
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Paulo Zanoni @ 2012-03-29 21:28 UTC (permalink / raw)
  To: dri-devel; +Cc: Paulo Zanoni

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

In the future we'll have more than just connector properties, so create
a dump_prop function that can handle any property (instead of the
current dump_props function that only handles connector properties).

Also, make this function print a lot more information about the existing
properties.

Also change the printed indentation of the modes to make the output more
readable.

The previous function dump_props also segfaulted when we didn't have
enought permissions. The new function does not segfault in this case (by
checking for the return value of drmModeGetProperty).

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 tests/modetest/modetest.c |   95 ++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 86 insertions(+), 9 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 173d587..02ea579 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -43,6 +43,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdint.h>
+#include <inttypes.h>
 #include <unistd.h>
 #include <string.h>
 #include <errno.h>
@@ -144,7 +145,7 @@ void dump_encoders(void)
 
 void dump_mode(drmModeModeInfo *mode)
 {
-	printf("  %s %d %d %d %d %d %d %d %d %d\n",
+	printf("\t%s %d %d %d %d %d %d %d %d %d\n",
 	       mode->name,
 	       mode->vrefresh,
 	       mode->hdisplay,
@@ -158,16 +159,90 @@ void dump_mode(drmModeModeInfo *mode)
 }
 
 static void
-dump_props(drmModeConnector *connector)
+dump_blob(uint32_t blob_id)
+{
+	uint32_t i;
+	unsigned char *blob_data;
+	drmModePropertyBlobPtr blob;
+
+	blob = drmModeGetPropertyBlob(fd, blob_id);
+	if (!blob)
+		return;
+
+	blob_data = blob->data;
+
+	for (i = 0; i < blob->length; i++) {
+		if (i % 16 == 0)
+			printf("\n\t\t\t");
+		printf("%.2hhx", blob_data[i]);
+	}
+	printf("\n");
+
+	drmModeFreePropertyBlob(blob);
+}
+
+static void
+dump_prop(uint32_t prop_id, uint64_t value)
 {
-	drmModePropertyPtr props;
 	int i;
+	drmModePropertyPtr prop;
+
+	prop = drmModeGetProperty(fd, prop_id);
+
+	printf("\t%d", prop_id);
+	if (!prop) {
+		printf("\n");
+		return;
+	}
+
+	printf(" %s:\n", prop->name);
+
+	printf("\t\tflags:");
+	if (prop->flags & DRM_MODE_PROP_PENDING)
+		printf(" pending");
+	if (prop->flags & DRM_MODE_PROP_RANGE)
+		printf(" range");
+	if (prop->flags & DRM_MODE_PROP_IMMUTABLE)
+		printf(" immutable");
+	if (prop->flags & DRM_MODE_PROP_ENUM)
+		printf(" enum");
+	if (prop->flags & DRM_MODE_PROP_BLOB)
+		printf(" blob");
+	printf("\n");
+
+	if (prop->flags & DRM_MODE_PROP_RANGE) {
+		printf("\t\tvalues:");
+		for (i = 0; i < prop->count_values; i++)
+			printf(" %"PRIu64, prop->values[i]);
+		printf("\n");
+	}
 
-	for (i = 0; i < connector->count_props; i++) {
-		props = drmModeGetProperty(fd, connector->props[i]);
-		printf("\t%s, flags %d\n", props->name, props->flags);
-		drmModeFreeProperty(props);
+	if (prop->flags & DRM_MODE_PROP_ENUM) {
+		printf("\t\tenums:");
+		for (i = 0; i < prop->count_enums; i++)
+			printf(" %s=%llu", prop->enums[i].name,
+			       prop->enums[i].value);
+		printf("\n");
+	} else {
+		assert(prop->count_enums == 0);
+	}
+
+	if (prop->flags & DRM_MODE_PROP_BLOB) {
+		printf("\t\tblobs:\n");
+		for (i = 0; i < prop->count_blobs; i++)
+			dump_blob(prop->blob_ids[i]);
+		printf("\n");
+	} else {
+		assert(prop->count_blobs == 0);
 	}
+
+	printf("\t\tvalue:");
+	if (prop->flags & DRM_MODE_PROP_BLOB)
+		dump_blob(value);
+	else
+		printf(" %"PRIu64"\n", value);
+
+	drmModeFreeProperty(prop);
 }
 
 void dump_connectors(void)
@@ -200,13 +275,15 @@ void dump_connectors(void)
 
 		if (connector->count_modes) {
 			printf("  modes:\n");
-			printf("  name refresh (Hz) hdisp hss hse htot vdisp "
+			printf("\tname refresh (Hz) hdisp hss hse htot vdisp "
 			       "vss vse vtot)\n");
 			for (j = 0; j < connector->count_modes; j++)
 				dump_mode(&connector->modes[j]);
 
 			printf("  props:\n");
-			dump_props(connector);
+			for (j = 0; j < connector->count_props; j++)
+				dump_prop(connector->props[j],
+					  connector->prop_values[j]);
 		}
 
 		drmModeFreeConnector(connector);
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH libdrm 4/5] Add support for generic object properties IOCTLs
  2012-03-29 21:28 [PATCH libdrm 1/5] modetest: fix some compiler warnings Paulo Zanoni
  2012-03-29 21:28 ` [PATCH libdrm 2/5] modetest: fix memory leak Paulo Zanoni
  2012-03-29 21:28 ` [PATCH libdrm 3/5] modetest: print more about our properties Paulo Zanoni
@ 2012-03-29 21:28 ` Paulo Zanoni
  2012-03-30  2:29   ` Eugeni Dodonov
  2012-03-29 21:28 ` [PATCH libdrm 5/5] modetest: print CRTC properties Paulo Zanoni
  2012-03-30  2:08 ` [PATCH libdrm 1/5] modetest: fix some compiler warnings Eugeni Dodonov
  4 siblings, 1 reply; 12+ messages in thread
From: Paulo Zanoni @ 2012-03-29 21:28 UTC (permalink / raw)
  To: dri-devel; +Cc: Paulo Zanoni

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

New library calls:
- drmModeObjectGetProperties
- drmModeFreeObjectProperties
- drmModeObjectSetProperties

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 include/drm/drm.h      |    2 +
 include/drm/drm_mode.h |   24 ++++++++++++++
 xf86drmMode.c          |   83 ++++++++++++++++++++++++++++++++++++++++++++++++
 xf86drmMode.h          |   14 ++++++++
 4 files changed, 123 insertions(+), 0 deletions(-)

diff --git a/include/drm/drm.h b/include/drm/drm.h
index 8adb9d5..155d25f 100644
--- a/include/drm/drm.h
+++ b/include/drm/drm.h
@@ -717,6 +717,8 @@ struct drm_get_cap {
 #define DRM_IOCTL_MODE_GETPLANE	DRM_IOWR(0xB6, struct drm_mode_get_plane)
 #define DRM_IOCTL_MODE_SETPLANE	DRM_IOWR(0xB7, struct drm_mode_set_plane)
 #define DRM_IOCTL_MODE_ADDFB2		DRM_IOWR(0xB8, struct drm_mode_fb_cmd2)
+#define DRM_IOCTL_MODE_OBJ_GETPROPERTIES	DRM_IOWR(0xB9, struct drm_mode_obj_get_properties)
+#define DRM_IOCTL_MODE_OBJ_SETPROPERTY	DRM_IOWR(0xBA, struct drm_mode_obj_set_property)
 
 /**
  * Device specific ioctls should only be in their respective headers
diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
index f36c61a..f303d94 100644
--- a/include/drm/drm_mode.h
+++ b/include/drm/drm_mode.h
@@ -250,6 +250,30 @@ struct drm_mode_connector_set_property {
 	__u32 connector_id;
 };
 
+#define DRM_MODE_OBJECT_CRTC 0xcccccccc
+#define DRM_MODE_OBJECT_CONNECTOR 0xc0c0c0c0
+#define DRM_MODE_OBJECT_ENCODER 0xe0e0e0e0
+#define DRM_MODE_OBJECT_MODE 0xdededede
+#define DRM_MODE_OBJECT_PROPERTY 0xb0b0b0b0
+#define DRM_MODE_OBJECT_FB 0xfbfbfbfb
+#define DRM_MODE_OBJECT_BLOB 0xbbbbbbbb
+#define DRM_MODE_OBJECT_PLANE 0xeeeeeeee
+
+struct drm_mode_obj_get_properties {
+	__u64 props_ptr;
+	__u64 prop_values_ptr;
+	__u32 count_props;
+	__u32 obj_id;
+	__u32 obj_type;
+};
+
+struct drm_mode_obj_set_property {
+	__u64 value;
+	__u32 prop_id;
+	__u32 obj_id;
+	__u32 obj_type;
+};
+
 struct drm_mode_get_blob {
 	__u32 blob_id;
 	__u32 length;
diff --git a/xf86drmMode.c b/xf86drmMode.c
index c809c44..a60c7cb 100644
--- a/xf86drmMode.c
+++ b/xf86drmMode.c
@@ -974,3 +974,86 @@ void drmModeFreePlaneResources(drmModePlaneResPtr ptr)
 	drmFree(ptr->planes);
 	drmFree(ptr);
 }
+
+drmModeObjectPropertiesPtr drmModeObjectGetProperties(int fd,
+						      uint32_t object_id,
+						      uint32_t object_type)
+{
+	struct drm_mode_obj_get_properties properties;
+	drmModeObjectPropertiesPtr ret = NULL;
+	uint32_t count;
+
+retry:
+	memset(&properties, 0, sizeof(struct drm_mode_obj_get_properties));
+	properties.obj_id = object_id;
+	properties.obj_type = object_type;
+
+	if (drmIoctl(fd, DRM_IOCTL_MODE_OBJ_GETPROPERTIES, &properties))
+		return 0;
+
+	count = properties.count_props;
+
+	if (count) {
+		properties.props_ptr = VOID2U64(drmMalloc(count *
+							  sizeof(uint32_t)));
+		if (!properties.props_ptr)
+			goto err_allocs;
+		properties.prop_values_ptr = VOID2U64(drmMalloc(count *
+						      sizeof(uint64_t)));
+		if (!properties.prop_values_ptr)
+			goto err_allocs;
+	}
+
+	if (drmIoctl(fd, DRM_IOCTL_MODE_OBJ_GETPROPERTIES, &properties))
+		goto err_allocs;
+
+	if (count < properties.count_props) {
+		drmFree(U642VOID(properties.props_ptr));
+		drmFree(U642VOID(properties.prop_values_ptr));
+		goto retry;
+	}
+	count = properties.count_props;
+
+	ret = drmMalloc(sizeof(*ret));
+	if (!ret)
+		goto err_allocs;
+
+	ret->count_props = count;
+	ret->props = drmAllocCpy(U642VOID(properties.props_ptr),
+				 count, sizeof(uint32_t));
+	ret->prop_values = drmAllocCpy(U642VOID(properties.prop_values_ptr),
+				       count, sizeof(uint64_t));
+	if (ret->count_props && (!ret->props || !ret->prop_values)) {
+		drmFree(ret->props);
+		drmFree(ret->prop_values);
+		drmFree(ret);
+		ret = NULL;
+	}
+
+err_allocs:
+	drmFree(U642VOID(properties.props_ptr));
+	drmFree(U642VOID(properties.prop_values_ptr));
+	return ret;
+}
+
+void drmModeFreeObjectProperties(drmModeObjectPropertiesPtr ptr)
+{
+	if (!ptr)
+		return;
+	drmFree(ptr->props);
+	drmFree(ptr->prop_values);
+	drmFree(ptr);
+}
+
+int drmModeObjectSetProperty(int fd, uint32_t object_id, uint32_t object_type,
+			     uint32_t property_id, uint64_t value)
+{
+	struct drm_mode_obj_set_property prop;
+
+	prop.value = value;
+	prop.prop_id = property_id;
+	prop.obj_id = object_id;
+	prop.obj_type = object_type;
+
+	return DRM_IOCTL(fd, DRM_IOCTL_MODE_OBJ_SETPROPERTY, &prop);
+}
diff --git a/xf86drmMode.h b/xf86drmMode.h
index 991e3f9..8e40034 100644
--- a/xf86drmMode.h
+++ b/xf86drmMode.h
@@ -281,6 +281,12 @@ typedef struct _drmModeConnector {
 	uint32_t *encoders; /**< List of encoder ids */
 } drmModeConnector, *drmModeConnectorPtr;
 
+typedef struct _drmModeObjectProperties {
+	uint32_t count_props;
+	uint32_t *props;
+	uint64_t *prop_values;
+} drmModeObjectProperties, *drmModeObjectPropertiesPtr;
+
 typedef struct _drmModePlane {
 	uint32_t count_formats;
 	uint32_t *formats;
@@ -428,6 +434,14 @@ extern int drmModeSetPlane(int fd, uint32_t plane_id, uint32_t crtc_id,
 			   uint32_t src_x, uint32_t src_y,
 			   uint32_t src_w, uint32_t src_h);
 
+extern drmModeObjectPropertiesPtr drmModeObjectGetProperties(int fd,
+							uint32_t object_id,
+							uint32_t object_type);
+extern void drmModeFreeObjectProperties(drmModeObjectPropertiesPtr ptr);
+extern int drmModeObjectSetProperty(int fd, uint32_t object_id,
+				    uint32_t object_type, uint32_t property_id,
+				    uint64_t value);
+
 #if defined(__cplusplus) || defined(c_plusplus)
 }
 #endif
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH libdrm 5/5] modetest: print CRTC properties
  2012-03-29 21:28 [PATCH libdrm 1/5] modetest: fix some compiler warnings Paulo Zanoni
                   ` (2 preceding siblings ...)
  2012-03-29 21:28 ` [PATCH libdrm 4/5] Add support for generic object properties IOCTLs Paulo Zanoni
@ 2012-03-29 21:28 ` Paulo Zanoni
  2012-03-30  2:31   ` Eugeni Dodonov
  2012-03-30  2:08 ` [PATCH libdrm 1/5] modetest: fix some compiler warnings Eugeni Dodonov
  4 siblings, 1 reply; 12+ messages in thread
From: Paulo Zanoni @ 2012-03-29 21:28 UTC (permalink / raw)
  To: dri-devel; +Cc: Paulo Zanoni

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

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 tests/modetest/modetest.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 02ea579..a6a2a8c 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -294,7 +294,9 @@ void dump_connectors(void)
 void dump_crtcs(void)
 {
 	drmModeCrtc *crtc;
+	drmModeObjectPropertiesPtr props;
 	int i;
+	uint32_t j;
 
 	printf("CRTCs:\n");
 	printf("id\tfb\tpos\tsize\n");
@@ -313,6 +315,19 @@ void dump_crtcs(void)
 		       crtc->width, crtc->height);
 		dump_mode(&crtc->mode);
 
+		printf("  props:\n");
+		props = drmModeObjectGetProperties(fd, crtc->crtc_id,
+						   DRM_MODE_OBJECT_CRTC);
+		if (props) {
+			for (j = 0; j < props->count_props; j++)
+				dump_prop(props->props[j],
+					  props->prop_values[j]);
+			drmModeFreeObjectProperties(props);
+		} else {
+			printf("\tcould not get crtc properties: %s\n",
+			       strerror(errno));
+		}
+
 		drmModeFreeCrtc(crtc);
 	}
 	printf("\n");
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH libdrm 1/5] modetest: fix some compiler warnings
  2012-03-29 21:28 [PATCH libdrm 1/5] modetest: fix some compiler warnings Paulo Zanoni
                   ` (3 preceding siblings ...)
  2012-03-29 21:28 ` [PATCH libdrm 5/5] modetest: print CRTC properties Paulo Zanoni
@ 2012-03-30  2:08 ` Eugeni Dodonov
  2012-04-21 20:47   ` Paulo Zanoni
  4 siblings, 1 reply; 12+ messages in thread
From: Eugeni Dodonov @ 2012-03-30  2:08 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: Paulo Zanoni, dri-devel


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

On Thu, Mar 29, 2012 at 18:28, Paulo Zanoni <przanoni@gmail.com> wrote:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Use unsigned int instead of int:
> - modetest.c:89:1: warning: comparison between signed and unsigned integer
> expressions [-Wsign-compare]
> - modetest.c:97:1: warning: comparison between signed and unsigned integer
> expressions [-Wsign-compare]
> - modetest.c:117:1: warning: comparison between signed and unsigned
> integer expressions [-Wsign-compare]
> - modetest.c:772:16: warning: comparison between signed and unsigned
> integer expressions [-Wsign-compare]
>
> The 'fd' variable is global, we don't need to pass it as an argument:
> - modetest.c:698:40: warning: unused parameter ‘fd’ [-Wunused-parameter]
>
> We don't use the 'modeset' variable:
> - modetest.c:725:8: warning: variable ‘modeset’ set but not used
> [-Wunused-but-set-variable]
>

This one is:
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>


-static int page_flipping_supported(int fd)
> +static int page_flipping_supported(void)
>

I wonder if we could drop the dead^W#ifd 0'ed code here as well, perhaps
with a separate patch? I don't think it will get used again in the future
(but I might be wrong).

-- 
Eugeni Dodonov
<http://eugeni.dodonov.net/>

[-- Attachment #1.2: Type: text/html, Size: 1968 bytes --]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH libdrm 2/5] modetest: fix memory leak
  2012-03-29 21:28 ` [PATCH libdrm 2/5] modetest: fix memory leak Paulo Zanoni
@ 2012-03-30  2:09   ` Eugeni Dodonov
  0 siblings, 0 replies; 12+ messages in thread
From: Eugeni Dodonov @ 2012-03-30  2:09 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: Paulo Zanoni, dri-devel


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

On Thu, Mar 29, 2012 at 18:28, Paulo Zanoni <przanoni@gmail.com> wrote:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Don't "continue" without freeing the connector.
>
> 192 bytes in 6 blocks are indirectly lost in loss record 6 of 12
>   at 0x4C2779D: malloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>   by 0x4E30DD8: drmMalloc (xf86drm.c:147)
>   by 0x4E35024: drmAllocCpy (xf86drmMode.c:73)
>   by 0x4E35D69: drmModeGetConnector (xf86drmMode.c:507)
>   by 0x402F22: dump_connectors (modetest.c:181)
>   by 0x40261B: main (modetest.c:801)
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>

Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

-- 
Eugeni Dodonov
<http://eugeni.dodonov.net/>

[-- Attachment #1.2: Type: text/html, Size: 1254 bytes --]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH libdrm 3/5] modetest: print more about our properties
  2012-03-29 21:28 ` [PATCH libdrm 3/5] modetest: print more about our properties Paulo Zanoni
@ 2012-03-30  2:19   ` Eugeni Dodonov
  2012-04-21 20:49     ` Paulo Zanoni
  0 siblings, 1 reply; 12+ messages in thread
From: Eugeni Dodonov @ 2012-03-30  2:19 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: Paulo Zanoni, dri-devel


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

On Thu, Mar 29, 2012 at 18:28, Paulo Zanoni <przanoni@gmail.com> wrote:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> In the future we'll have more than just connector properties, so create
> a dump_prop function that can handle any property (instead of the
> current dump_props function that only handles connector properties).
>
> Also, make this function print a lot more information about the existing
> properties.
>
> Also change the printed indentation of the modes to make the output more
> readable.
>

<really-small-bikeshedding>
I don't know if it should go into a separate patch though. But it is
aligned to the other formatting patterns you do, and it certainly looks
nicer this way, and it shouldn't break anything anyway. So I am fine either
way :).
</really-small-bikeshedding>


>
> The previous function dump_props also segfaulted when we didn't have
> enought permissions. The new function does not segfault in this case (by
> checking for the return value of drmModeGetProperty).
>

<another-bikeshedding>
I think this could be done in a separate patch..
</another-bikeshedding>

But besides those comments, this is:

Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

-- 
Eugeni Dodonov
<http://eugeni.dodonov.net/>

[-- Attachment #1.2: Type: text/html, Size: 1984 bytes --]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH libdrm 4/5] Add support for generic object properties IOCTLs
  2012-03-29 21:28 ` [PATCH libdrm 4/5] Add support for generic object properties IOCTLs Paulo Zanoni
@ 2012-03-30  2:29   ` Eugeni Dodonov
  0 siblings, 0 replies; 12+ messages in thread
From: Eugeni Dodonov @ 2012-03-30  2:29 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: Paulo Zanoni, dri-devel


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

On Thu, Mar 29, 2012 at 18:28, Paulo Zanoni <przanoni@gmail.com> wrote:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> New library calls:
> - drmModeObjectGetProperties
> - drmModeFreeObjectProperties
> - drmModeObjectSetProperties
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>

Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

-- 
Eugeni Dodonov
<http://eugeni.dodonov.net/>

[-- Attachment #1.2: Type: text/html, Size: 885 bytes --]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH libdrm 5/5] modetest: print CRTC properties
  2012-03-29 21:28 ` [PATCH libdrm 5/5] modetest: print CRTC properties Paulo Zanoni
@ 2012-03-30  2:31   ` Eugeni Dodonov
  0 siblings, 0 replies; 12+ messages in thread
From: Eugeni Dodonov @ 2012-03-30  2:31 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: Paulo Zanoni, dri-devel


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

On Thu, Mar 29, 2012 at 18:28, Paulo Zanoni <przanoni@gmail.com> wrote:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>

Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

-- 
Eugeni Dodonov
<http://eugeni.dodonov.net/>

[-- Attachment #1.2: Type: text/html, Size: 752 bytes --]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH libdrm 1/5] modetest: fix some compiler warnings
  2012-03-30  2:08 ` [PATCH libdrm 1/5] modetest: fix some compiler warnings Eugeni Dodonov
@ 2012-04-21 20:47   ` Paulo Zanoni
  0 siblings, 0 replies; 12+ messages in thread
From: Paulo Zanoni @ 2012-04-21 20:47 UTC (permalink / raw)
  To: Eugeni Dodonov; +Cc: Paulo Zanoni, dri-devel

2012/3/29 Eugeni Dodonov <eugeni@dodonov.net>:
> I wonder if we could drop the dead^W#ifd 0'ed code here as well, perhaps
> with a separate patch? I don't think it will get used again in the future
> (but I might be wrong).
>

Removing #if 0'ed code won't fix compiler warnings, so if we do this,
it should
be done in a separate patch.

I'm resending this patch with more warning fixes and a rebase.


-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH libdrm 3/5] modetest: print more about our properties
  2012-03-30  2:19   ` Eugeni Dodonov
@ 2012-04-21 20:49     ` Paulo Zanoni
  0 siblings, 0 replies; 12+ messages in thread
From: Paulo Zanoni @ 2012-04-21 20:49 UTC (permalink / raw)
  To: Eugeni Dodonov; +Cc: Paulo Zanoni, dri-devel

2012/3/29 Eugeni Dodonov <eugeni@dodonov.net>:
> On Thu, Mar 29, 2012 at 18:28, Paulo Zanoni <przanoni@gmail.com> wrote:
>
> <really-small-bikeshedding>
> I don't know if it should go into a separate patch though. But it is aligned
> to the other formatting patterns you do, and it certainly looks nicer this
> way, and it shouldn't break anything anyway. So I am fine either way :).
> </really-small-bikeshedding>

I agree, but it's a single-line change that does not make much sense without
this patch, so I decided to not have a patch that just replaces "  " with "\t".

>> The previous function dump_props also segfaulted when we didn't have
>> enought permissions. The new function does not segfault in this case (by
>> checking for the return value of drmModeGetProperty).
>
> <another-bikeshedding>
> I think this could be done in a separate patch..
> </another-bikeshedding>

No. This is just a note like "the function I just killed had a bug,
the function
that replaces it does not have the same bug". It would make no sense
to send a
patch to fix the old function, then send this patch replacing the
whole
function. Why fix it if you're going to replace it?


-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2012-04-21 20:49 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-29 21:28 [PATCH libdrm 1/5] modetest: fix some compiler warnings Paulo Zanoni
2012-03-29 21:28 ` [PATCH libdrm 2/5] modetest: fix memory leak Paulo Zanoni
2012-03-30  2:09   ` Eugeni Dodonov
2012-03-29 21:28 ` [PATCH libdrm 3/5] modetest: print more about our properties Paulo Zanoni
2012-03-30  2:19   ` Eugeni Dodonov
2012-04-21 20:49     ` Paulo Zanoni
2012-03-29 21:28 ` [PATCH libdrm 4/5] Add support for generic object properties IOCTLs Paulo Zanoni
2012-03-30  2:29   ` Eugeni Dodonov
2012-03-29 21:28 ` [PATCH libdrm 5/5] modetest: print CRTC properties Paulo Zanoni
2012-03-30  2:31   ` Eugeni Dodonov
2012-03-30  2:08 ` [PATCH libdrm 1/5] modetest: fix some compiler warnings Eugeni Dodonov
2012-04-21 20:47   ` Paulo Zanoni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.