[PATCH v6 00/23] modetest enhancements

[PATCH v6 00/23] modetest enhancements

[Laurent Pinchart]

Hello,

Here's the sixth (and hopefully final) version of my modeset enhancements
patch set.

Beside various cleanups, these patches allow dropping master after mode set,
configuring more than two pipes and planes, setting properties from the
command line, setting plane positions and configuring pipes with multiple
connectors in cloned mode.

The code has been tested with the R-Car DU DRM driver.

Changes since v5:

- Fixed a bug in the -M argument handling that broke the default behaviour

Changes since v4:

- Update the usage line with the newly added parameters
- Fixed a bad copy&paste in the -w argument handling
- Use widthxheight[+-]x[+-]y to specify plane size and position
- Fixed two bugs in buffer allocation and pattern generation (22/23 and 23/23)

Laurent Pinchart (23):
  modetest: Fix warnings
  modetest: Remove extern declarations of opt(arg|ind|err|opt)
  modetest: Sort command line arguments
  modetest: Add a command line parameter to select the driver
  modetest: Add a command line parameter to drop master after mode set
  modetest: Retrieve all resources in one go
  modetest: Don't limit mode set and planes to two instances
  modetest: Add a command line parameter to set properties
  modetest: Allow specifying plane position
  modetest: Print the plane ID when setting up a plane
  modetest: Remove the -m argument
  modetest: Create a device structure
  modetest: Compute CRTC pipe number as needed
  modetest: Remove the struct connector_arg encoder field
  modetest: Store the crtc in the connector_arg structure
  modetest: Store the mode in the crtc structure
  modetest: Give the CRTC ID to the -P option
  modetest: Split mode setting and plane setup
  modetest: Rename struct connector_arg to struct pipe_arg
  modetest: Support pipes with multiple connectors
  modetest: Try all possible encoders for a connector
  modetest: Fix line stride in SMPTE YUV packet pattern generator
  modetest: Allocate NV buffers large enough for the two planes

 tests/modetest/Makefile.am |    4 +-
 tests/modetest/buffers.c   |   44 +-
 tests/modetest/buffers.h   |    5 +-
 tests/modetest/modetest.c  | 1197 +++++++++++++++++++++++++++++++-------------
 4 files changed, 884 insertions(+), 366 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH v6 01/23] modetest: Fix warnings

[Laurent Pinchart]

Enable all standard automake warnings except for -Wpointer-arith (as the
test pattern generation code uses void pointer arithmetics) and fix
them.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/modetest/Makefile.am |  4 +++-
 tests/modetest/buffers.c   | 13 +++++++------
 tests/modetest/buffers.h   |  5 +++--
 tests/modetest/modetest.c  | 44 ++++++++++++++++++++++----------------------
 4 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/tests/modetest/Makefile.am b/tests/modetest/Makefile.am
index 410c632..6e7ff14 100644
--- a/tests/modetest/Makefile.am
+++ b/tests/modetest/Makefile.am
@@ -1,4 +1,6 @@
-AM_CFLAGS = \
+AM_CFLAGS = $(filter-out -Wpointer-arith, $(WARN_CFLAGS))
+
+AM_CFLAGS += \
 	-I$(top_srcdir)/include/drm \
 	-I$(top_srcdir)/libkms/ \
 	-I$(top_srcdir)
diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c
index b75cca7..1ca3be5 100644
--- a/tests/modetest/buffers.c
+++ b/tests/modetest/buffers.c
@@ -999,8 +999,8 @@ fill_pattern(unsigned int format, enum fill_pattern pattern, void *planes[3],
  */
 
 static struct kms_bo *
-allocate_buffer(struct kms_driver *kms,
-		int width, int height, int *stride)
+allocate_buffer(struct kms_driver *kms, unsigned int width, unsigned int height,
+		unsigned int *stride)
 {
 	struct kms_bo *bo;
 	unsigned bo_attribs[] = {
@@ -1034,13 +1034,14 @@ allocate_buffer(struct kms_driver *kms,
 
 struct kms_bo *
 create_test_buffer(struct kms_driver *kms, unsigned int format,
-		   int width, int height, int handles[4],
-		   int pitches[4], int offsets[4], enum fill_pattern pattern)
+		   unsigned int width, unsigned int height,
+		   unsigned int handles[4], unsigned int pitches[4],
+		   unsigned int offsets[4], enum fill_pattern pattern)
 {
 	struct kms_bo *bo;
-	int ret, stride;
-	void *planes[3];
+	void *planes[3] = { 0, };
 	void *virtual;
+	int ret;
 
 	bo = allocate_buffer(kms, width, height, &pitches[0]);
 	if (!bo)
diff --git a/tests/modetest/buffers.h b/tests/modetest/buffers.h
index 2b15ce5..e320389 100644
--- a/tests/modetest/buffers.h
+++ b/tests/modetest/buffers.h
@@ -37,8 +37,9 @@ enum fill_pattern {
 };
 
 struct kms_bo *create_test_buffer(struct kms_driver *kms, unsigned int format,
-		   int width, int height, int handles[4], int pitches[4],
-		   int offsets[4], enum fill_pattern pattern);
+		   unsigned int width, unsigned int height,
+		   unsigned int handles[4], unsigned int pitches[4],
+		   unsigned int offsets[4], enum fill_pattern pattern);
 
 unsigned int format_fourcc(const char *name);
 
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 8afd2b1..47704db 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -64,11 +64,11 @@ int fd, modes;
 
 struct type_name {
 	int type;
-	char *name;
+	const char *name;
 };
 
 #define type_name_fn(res) \
-char * res##_str(int type) {			\
+const char * res##_str(int type) {			\
 	unsigned int i;					\
 	for (i = 0; i < ARRAY_SIZE(res##_names); i++) { \
 		if (res##_names[i].type == type)	\
@@ -85,7 +85,7 @@ struct type_name encoder_type_names[] = {
 	{ DRM_MODE_ENCODER_TVDAC, "TVDAC" },
 };
 
-type_name_fn(encoder_type)
+static type_name_fn(encoder_type)
 
 struct type_name connector_status_names[] = {
 	{ DRM_MODE_CONNECTED, "connected" },
@@ -93,7 +93,7 @@ struct type_name connector_status_names[] = {
 	{ DRM_MODE_UNKNOWNCONNECTION, "unknown" },
 };
 
-type_name_fn(connector_status)
+static type_name_fn(connector_status)
 
 struct type_name connector_type_names[] = {
 	{ DRM_MODE_CONNECTOR_Unknown, "unknown" },
@@ -113,11 +113,11 @@ struct type_name connector_type_names[] = {
 	{ DRM_MODE_CONNECTOR_eDP, "eDP" },
 };
 
-type_name_fn(connector_type)
+static type_name_fn(connector_type)
 
 #define bit_name_fn(res)					\
-char * res##_str(int type) {					\
-	int i;							\
+const char * res##_str(int type) {				\
+	unsigned int i;						\
 	const char *sep = "";					\
 	for (i = 0; i < ARRAY_SIZE(res##_names); i++) {		\
 		if (type & (1 << i)) {				\
@@ -138,7 +138,7 @@ static const char *mode_type_names[] = {
 	"driver",
 };
 
-bit_name_fn(mode_type)
+static bit_name_fn(mode_type)
 
 static const char *mode_flag_names[] = {
 	"phsync",
@@ -157,9 +157,9 @@ static const char *mode_flag_names[] = {
 	"clkdiv2"
 };
 
-bit_name_fn(mode_flag)
+static bit_name_fn(mode_flag)
 
-void dump_encoders(void)
+static void dump_encoders(void)
 {
 	drmModeEncoder *encoder;
 	int i;
@@ -185,7 +185,7 @@ void dump_encoders(void)
 	printf("\n");
 }
 
-void dump_mode(drmModeModeInfo *mode)
+static void dump_mode(drmModeModeInfo *mode)
 {
 	printf("  %s %d %d %d %d %d %d %d %d %d",
 	       mode->name,
@@ -301,7 +301,7 @@ dump_prop(uint32_t prop_id, uint64_t value)
 	drmModeFreeProperty(prop);
 }
 
-void dump_connectors(void)
+static void dump_connectors(void)
 {
 	drmModeConnector *connector;
 	int i, j;
@@ -347,7 +347,7 @@ void dump_connectors(void)
 	printf("\n");
 }
 
-void dump_crtcs(void)
+static void dump_crtcs(void)
 {
 	drmModeCrtc *crtc;
 	drmModeObjectPropertiesPtr props;
@@ -389,7 +389,7 @@ void dump_crtcs(void)
 	printf("\n");
 }
 
-void dump_framebuffers(void)
+static void dump_framebuffers(void)
 {
 	drmModeFB *fb;
 	int i;
@@ -574,7 +574,7 @@ connector_find_mode(struct connector *c)
 
 	/* and figure out which crtc index it is: */
 	for (i = 0; i < resources->count_crtcs; i++) {
-		if (c->crtc == resources->crtcs[i]) {
+		if (c->crtc == (int)resources->crtcs[i]) {
 			c->pipe = i;
 			break;
 		}
@@ -584,7 +584,7 @@ connector_find_mode(struct connector *c)
 
 /* -------------------------------------------------------------------------- */
 
-void
+static void
 page_flip_handler(int fd, unsigned int frame,
 		  unsigned int sec, unsigned int usec, void *data)
 {
@@ -622,7 +622,7 @@ set_plane(struct kms_driver *kms, struct connector *c, struct plane *p)
 	uint32_t plane_id = 0;
 	struct kms_bo *plane_bo;
 	uint32_t plane_flags = 0;
-	int ret, crtc_x, crtc_y, crtc_w, crtc_h;
+	int crtc_x, crtc_y, crtc_w, crtc_h;
 	unsigned int i;
 
 	/* find an unused plane which can be connected to our crtc */
@@ -861,7 +861,7 @@ static int parse_connector(struct connector *c, const char *arg)
 	arg = endp + 1;
 
 	p = strchrnul(arg, '@');
-	len = min(sizeof c->mode_str - 1, p - arg);
+	len = min(sizeof c->mode_str - 1, (unsigned int)(p - arg));
 	strncpy(c->mode_str, arg, len);
 	c->mode_str[len] = '\0';
 
@@ -883,7 +883,7 @@ static int parse_plane(struct plane *p, const char *arg)
 {
 	strcpy(p->format_str, "XR24");
 
-	if (sscanf(arg, "%d:%dx%d@%4s", &p->con_id, &p->w, &p->h, &p->format_str) != 4 &&
+	if (sscanf(arg, "%d:%dx%d@%4s", &p->con_id, &p->w, &p->h, p->format_str) != 4 &&
 	    sscanf(arg, "%d:%dx%d", &p->con_id, &p->w, &p->h) != 3)
 		return -1;
 
@@ -896,7 +896,7 @@ static int parse_plane(struct plane *p, const char *arg)
 	return 0;
 }
 
-void usage(char *name)
+static void usage(char *name)
 {
 	fprintf(stderr, "usage: %s [-ecpmf]\n", name);
 	fprintf(stderr, "\t-e\tlist encoders\n");
@@ -939,11 +939,11 @@ int main(int argc, char **argv)
 	int c;
 	int encoders = 0, connectors = 0, crtcs = 0, planes = 0, framebuffers = 0;
 	int test_vsync = 0;
-	char *modules[] = { "i915", "radeon", "nouveau", "vmwgfx", "omapdrm", "exynos" };
+	const char *modules[] = { "i915", "radeon", "nouveau", "vmwgfx", "omapdrm", "exynos" };
 	unsigned int i;
 	int count = 0, plane_count = 0;
 	struct connector con_args[2];
-	struct plane plane_args[2] = {0};
+	struct plane plane_args[2] = { { 0, }, };
 	
 	opterr = 0;
 	while ((c = getopt(argc, argv, optstr)) != -1) {
-- 
1.8.1.5

[PATCH v6 02/23] modetest: Remove extern declarations of opt(arg|ind|err|opt)

[Laurent Pinchart]

Those variables are declared in unistd.h, there's no need to redeclare
them here.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
---
 tests/modetest/modetest.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 47704db..e0d7d72 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -835,8 +835,6 @@ set_mode(struct connector *c, int count, struct plane *p, int plane_count,
 	kms_destroy(&kms);
 }
 
-extern char *optarg;
-extern int optind, opterr, optopt;
 static char optstr[] = "ecpmfs:P:v";
 
 #define min(a, b)	((a) < (b) ? (a) : (b))
-- 
1.8.1.5

[PATCH v6 03/23] modetest: Sort command line arguments

[Laurent Pinchart]

The current mostly random sort order hinders code readability. Sort the
options alphabetically in the code, and by group in the help message.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
---
 tests/modetest/modetest.c | 49 ++++++++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index e0d7d72..2bb4b19 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -835,8 +835,6 @@ set_mode(struct connector *c, int count, struct plane *p, int plane_count,
 	kms_destroy(&kms);
 }
 
-static char optstr[] = "ecpmfs:P:v";
-
 #define min(a, b)	((a) < (b) ? (a) : (b))
 
 static int parse_connector(struct connector *c, const char *arg)
@@ -896,15 +894,20 @@ static int parse_plane(struct plane *p, const char *arg)
 
 static void usage(char *name)
 {
-	fprintf(stderr, "usage: %s [-ecpmf]\n", name);
-	fprintf(stderr, "\t-e\tlist encoders\n");
+	fprintf(stderr, "usage: %s [-cefmPpsv]\n", name);
+
+	fprintf(stderr, "\n Query options:\n\n");
 	fprintf(stderr, "\t-c\tlist connectors\n");
-	fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n");
-	fprintf(stderr, "\t-m\tlist modes\n");
+	fprintf(stderr, "\t-e\tlist encoders\n");
 	fprintf(stderr, "\t-f\tlist framebuffers\n");
-	fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
-	fprintf(stderr, "\t-s <connector_id>[@<crtc_id>]:<mode>[@<format>]\tset a mode\n");
+	fprintf(stderr, "\t-m\tlist modes\n");
+	fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n");
+
+	fprintf(stderr, "\n Test options:\n\n");
 	fprintf(stderr, "\t-P <connector_id>:<w>x<h>[@<format>]\tset a plane\n");
+	fprintf(stderr, "\t-s <connector_id>[@<crtc_id>]:<mode>[@<format>]\tset a mode\n");
+	fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
+
 	fprintf(stderr, "\n\tDefault is to dump all info.\n");
 	exit(0);
 }
@@ -932,6 +935,8 @@ static int page_flipping_supported(void)
 #endif
 }
 
+static char optstr[] = "cefmP:ps:v";
+
 int main(int argc, char **argv)
 {
 	int c;
@@ -946,34 +951,34 @@ int main(int argc, char **argv)
 	opterr = 0;
 	while ((c = getopt(argc, argv, optstr)) != -1) {
 		switch (c) {
-		case 'e':
-			encoders = 1;
-			break;
 		case 'c':
 			connectors = 1;
 			break;
-		case 'p':
-			crtcs = 1;
-			planes = 1;
+		case 'e':
+			encoders = 1;
+			break;
+		case 'f':
+			framebuffers = 1;
 			break;
 		case 'm':
 			modes = 1;
 			break;
-		case 'f':
-			framebuffers = 1;
+		case 'P':
+			if (parse_plane(&plane_args[plane_count], optarg) < 0)
+				usage(argv[0]);
+			plane_count++;
 			break;
-		case 'v':
-			test_vsync = 1;
+		case 'p':
+			crtcs = 1;
+			planes = 1;
 			break;
 		case 's':
 			if (parse_connector(&con_args[count], optarg) < 0)
 				usage(argv[0]);
 			count++;				      
 			break;
-		case 'P':
-			if (parse_plane(&plane_args[plane_count], optarg) < 0)
-				usage(argv[0]);
-			plane_count++;
+		case 'v':
+			test_vsync = 1;
 			break;
 		default:
 			usage(argv[0]);
-- 
1.8.1.5

[PATCH v6 04/23] modetest: Add a command line parameter to select the driver

[Laurent Pinchart]

If the -M parameter is specified, modetest will use the requested device
name instead of trying its builtin list of device names.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
---
 tests/modetest/modetest.c | 50 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 35 insertions(+), 15 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 2bb4b19..5802c8e 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -894,7 +894,7 @@ static int parse_plane(struct plane *p, const char *arg)
 
 static void usage(char *name)
 {
-	fprintf(stderr, "usage: %s [-cefmPpsv]\n", name);
+	fprintf(stderr, "usage: %s [-cefMmPpsv]\n", name);
 
 	fprintf(stderr, "\n Query options:\n\n");
 	fprintf(stderr, "\t-c\tlist connectors\n");
@@ -908,6 +908,9 @@ static void usage(char *name)
 	fprintf(stderr, "\t-s <connector_id>[@<crtc_id>]:<mode>[@<format>]\tset a mode\n");
 	fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
 
+	fprintf(stderr, "\n Generic options:\n\n");
+	fprintf(stderr, "\t-M module\tuse the given driver\n");
+
 	fprintf(stderr, "\n\tDefault is to dump all info.\n");
 	exit(0);
 }
@@ -935,7 +938,7 @@ static int page_flipping_supported(void)
 #endif
 }
 
-static char optstr[] = "cefmP:ps:v";
+static char optstr[] = "cefM:mP:ps:v";
 
 int main(int argc, char **argv)
 {
@@ -943,13 +946,17 @@ int main(int argc, char **argv)
 	int encoders = 0, connectors = 0, crtcs = 0, planes = 0, framebuffers = 0;
 	int test_vsync = 0;
 	const char *modules[] = { "i915", "radeon", "nouveau", "vmwgfx", "omapdrm", "exynos" };
+	char *module = NULL;
 	unsigned int i;
 	int count = 0, plane_count = 0;
 	struct connector con_args[2];
 	struct plane plane_args[2] = { { 0, }, };
+	unsigned int args = 0;
 	
 	opterr = 0;
 	while ((c = getopt(argc, argv, optstr)) != -1) {
+		args++;
+
 		switch (c) {
 		case 'c':
 			connectors = 1;
@@ -960,6 +967,11 @@ int main(int argc, char **argv)
 		case 'f':
 			framebuffers = 1;
 			break;
+		case 'M':
+			module = optarg;
+			/* Preserve the default behaviour of dumping all information. */
+			args--;
+			break;
 		case 'm':
 			modes = 1;
 			break;
@@ -986,17 +998,30 @@ int main(int argc, char **argv)
 		}
 	}
 
-	if (argc == 1)
+	if (!args)
 		encoders = connectors = crtcs = planes = modes = framebuffers = 1;
 
-	for (i = 0; i < ARRAY_SIZE(modules); i++) {
-		printf("trying to load module %s...", modules[i]);
-		fd = drmOpen(modules[i], NULL);
+	if (module) {
+		fd = drmOpen(module, NULL);
 		if (fd < 0) {
-			printf("failed.\n");
-		} else {
-			printf("success.\n");
-			break;
+			fprintf(stderr, "failed to open device '%s'.\n", module);
+			return 1;
+		}
+	} else {
+		for (i = 0; i < ARRAY_SIZE(modules); i++) {
+			printf("trying to open device '%s'...", modules[i]);
+			fd = drmOpen(modules[i], NULL);
+			if (fd < 0) {
+				printf("failed.\n");
+			} else {
+				printf("success.\n");
+				break;
+			}
+		}
+
+		if (fd < 0) {
+			fprintf(stderr, "no device found.\n");
+			return 1;
 		}
 	}
 
@@ -1005,11 +1030,6 @@ int main(int argc, char **argv)
 		return -1;
 	}
 
-	if (i == ARRAY_SIZE(modules)) {
-		fprintf(stderr, "failed to load any modules, aborting.\n");
-		return -1;
-	}
-
 	resources = drmModeGetResources(fd);
 	if (!resources) {
 		fprintf(stderr, "drmModeGetResources failed: %s\n",
-- 
1.8.1.5

[PATCH v6 05/23] modetest: Add a command line parameter to drop master after mode set

[Laurent Pinchart]

If the -d parameter is specified, modetest will drop master permissions
after setting the mode.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/modetest/modetest.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 5802c8e..218d26a 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -894,7 +894,7 @@ static int parse_plane(struct plane *p, const char *arg)
 
 static void usage(char *name)
 {
-	fprintf(stderr, "usage: %s [-cefMmPpsv]\n", name);
+	fprintf(stderr, "usage: %s [-cdefMmPpsv]\n", name);
 
 	fprintf(stderr, "\n Query options:\n\n");
 	fprintf(stderr, "\t-c\tlist connectors\n");
@@ -909,6 +909,7 @@ static void usage(char *name)
 	fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
 
 	fprintf(stderr, "\n Generic options:\n\n");
+	fprintf(stderr, "\t-d\tdrop master after mode set\n");
 	fprintf(stderr, "\t-M module\tuse the given driver\n");
 
 	fprintf(stderr, "\n\tDefault is to dump all info.\n");
@@ -938,12 +939,13 @@ static int page_flipping_supported(void)
 #endif
 }
 
-static char optstr[] = "cefM:mP:ps:v";
+static char optstr[] = "cdefM:mP:ps:v";
 
 int main(int argc, char **argv)
 {
 	int c;
 	int encoders = 0, connectors = 0, crtcs = 0, planes = 0, framebuffers = 0;
+	int drop_master = 0;
 	int test_vsync = 0;
 	const char *modules[] = { "i915", "radeon", "nouveau", "vmwgfx", "omapdrm", "exynos" };
 	char *module = NULL;
@@ -961,6 +963,9 @@ int main(int argc, char **argv)
 		case 'c':
 			connectors = 1;
 			break;
+		case 'd':
+			drop_master = 1;
+			break;
 		case 'e':
 			encoders = 1;
 			break;
@@ -1046,6 +1051,8 @@ int main(int argc, char **argv)
 
 	if (count > 0) {
 		set_mode(con_args, count, plane_args, plane_count, test_vsync);
+		if (drop_master)
+			drmDropMaster(fd);
 		getchar();
 	}
 
-- 
1.8.1.5

[PATCH v6 06/23] modetest: Retrieve all resources in one go

[Laurent Pinchart]

Instead of retrieving resources as they are needed, retrieve them all
(except property blobs) in one go at startup.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/modetest/modetest.c | 408 +++++++++++++++++++++++++++++-----------------
 1 file changed, 261 insertions(+), 147 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 218d26a..8ad138e 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -57,7 +57,44 @@
 
 #include "buffers.h"
 
-drmModeRes *resources;
+struct crtc {
+	drmModeCrtc *crtc;
+	drmModeObjectProperties *props;
+	drmModePropertyRes **props_info;
+};
+
+struct encoder {
+	drmModeEncoder *encoder;
+};
+
+struct connector {
+	drmModeConnector *connector;
+	drmModeObjectProperties *props;
+	drmModePropertyRes **props_info;
+};
+
+struct fb {
+	drmModeFB *fb;
+};
+
+struct plane {
+	drmModePlane *plane;
+	drmModeObjectProperties *props;
+	drmModePropertyRes **props_info;
+};
+
+struct resources {
+	drmModeRes *res;
+	drmModePlaneRes *plane_res;
+
+	struct crtc *crtcs;
+	struct encoder *encoders;
+	struct connector *connectors;
+	struct fb *fbs;
+	struct plane *planes;
+};
+
+struct resources *resources;
 int fd, modes;
 
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
@@ -166,21 +203,17 @@ static void dump_encoders(void)
 
 	printf("Encoders:\n");
 	printf("id\tcrtc\ttype\tpossible crtcs\tpossible clones\t\n");
-	for (i = 0; i < resources->count_encoders; i++) {
-		encoder = drmModeGetEncoder(fd, resources->encoders[i]);
-
-		if (!encoder) {
-			fprintf(stderr, "could not get encoder %i: %s\n",
-				resources->encoders[i], strerror(errno));
+	for (i = 0; i < resources->res->count_encoders; i++) {
+		encoder = resources->encoders[i].encoder;
+		if (!encoder)
 			continue;
-		}
+
 		printf("%d\t%d\t%s\t0x%08x\t0x%08x\n",
 		       encoder->encoder_id,
 		       encoder->crtc_id,
 		       encoder_type_str(encoder->encoder_type),
 		       encoder->possible_crtcs,
 		       encoder->possible_clones);
-		drmModeFreeEncoder(encoder);
 	}
 	printf("\n");
 }
@@ -230,13 +263,9 @@ dump_blob(uint32_t blob_id)
 }
 
 static void
-dump_prop(uint32_t prop_id, uint64_t value)
+dump_prop(drmModePropertyPtr prop, uint32_t prop_id, uint64_t value)
 {
 	int i;
-	drmModePropertyPtr prop;
-
-	prop = drmModeGetProperty(fd, prop_id);
-
 	printf("\t%d", prop_id);
 	if (!prop) {
 		printf("\n");
@@ -297,25 +326,19 @@ dump_prop(uint32_t prop_id, uint64_t value)
 		dump_blob(value);
 	else
 		printf(" %"PRIu64"\n", value);
-
-	drmModeFreeProperty(prop);
 }
 
 static void dump_connectors(void)
 {
-	drmModeConnector *connector;
 	int i, j;
 
 	printf("Connectors:\n");
 	printf("id\tencoder\tstatus\t\ttype\tsize (mm)\tmodes\tencoders\n");
-	for (i = 0; i < resources->count_connectors; i++) {
-		connector = drmModeGetConnector(fd, resources->connectors[i]);
-
-		if (!connector) {
-			fprintf(stderr, "could not get connector %i: %s\n",
-				resources->connectors[i], strerror(errno));
+	for (i = 0; i < resources->res->count_connectors; i++) {
+		struct connector *_connector = &resources->connectors[i];
+		drmModeConnector *connector = _connector->connector;
+		if (!connector)
 			continue;
-		}
 
 		printf("%d\t%d\t%s\t%s\t%dx%d\t\t%d\t",
 		       connector->connector_id,
@@ -335,35 +358,32 @@ static void dump_connectors(void)
 			       "vss vse vtot)\n");
 			for (j = 0; j < connector->count_modes; j++)
 				dump_mode(&connector->modes[j]);
+		}
 
+		if (_connector->props) {
 			printf("  props:\n");
-			for (j = 0; j < connector->count_props; j++)
-				dump_prop(connector->props[j],
-					  connector->prop_values[j]);
+			for (j = 0; j < (int)_connector->props->count_props; j++)
+				dump_prop(_connector->props_info[j],
+					  _connector->props->props[j],
+					  _connector->props->prop_values[j]);
 		}
-
-		drmModeFreeConnector(connector);
 	}
 	printf("\n");
 }
 
 static void dump_crtcs(void)
 {
-	drmModeCrtc *crtc;
-	drmModeObjectPropertiesPtr props;
 	int i;
 	uint32_t j;
 
 	printf("CRTCs:\n");
 	printf("id\tfb\tpos\tsize\n");
-	for (i = 0; i < resources->count_crtcs; i++) {
-		crtc = drmModeGetCrtc(fd, resources->crtcs[i]);
-
-		if (!crtc) {
-			fprintf(stderr, "could not get crtc %i: %s\n",
-				resources->crtcs[i], strerror(errno));
+	for (i = 0; i < resources->res->count_crtcs; i++) {
+		struct crtc *_crtc = &resources->crtcs[i];
+		drmModeCrtc *crtc = _crtc->crtc;
+		if (!crtc)
 			continue;
-		}
+
 		printf("%d\t%d\t(%d,%d)\t(%dx%d)\n",
 		       crtc->crtc_id,
 		       crtc->buffer_id,
@@ -371,20 +391,15 @@ static 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);
+		if (_crtc->props) {
+			printf("  props:\n");
+			for (j = 0; j < _crtc->props->count_props; j++)
+				dump_prop(_crtc->props_info[j],
+					  _crtc->props->props[j],
+					  _crtc->props->prop_values[j]);
 		} else {
-			printf("\tcould not get crtc properties: %s\n",
-			       strerror(errno));
+			printf("  no properties found\n");
 		}
-
-		drmModeFreeCrtc(crtc);
 	}
 	printf("\n");
 }
@@ -396,47 +411,34 @@ static void dump_framebuffers(void)
 
 	printf("Frame buffers:\n");
 	printf("id\tsize\tpitch\n");
-	for (i = 0; i < resources->count_fbs; i++) {
-		fb = drmModeGetFB(fd, resources->fbs[i]);
-
-		if (!fb) {
-			fprintf(stderr, "could not get fb %i: %s\n",
-				resources->fbs[i], strerror(errno));
+	for (i = 0; i < resources->res->count_fbs; i++) {
+		fb = resources->fbs[i].fb;
+		if (!fb)
 			continue;
-		}
+
 		printf("%u\t(%ux%u)\t%u\n",
 		       fb->fb_id,
 		       fb->width, fb->height,
 		       fb->pitch);
-
-		drmModeFreeFB(fb);
 	}
 	printf("\n");
 }
 
 static void dump_planes(void)
 {
-	drmModeObjectPropertiesPtr props;
-	drmModePlaneRes *plane_resources;
-	drmModePlane *ovr;
 	unsigned int i, j;
 
-	plane_resources = drmModeGetPlaneResources(fd);
-	if (!plane_resources) {
-		fprintf(stderr, "drmModeGetPlaneResources failed: %s\n",
-			strerror(errno));
-		return;
-	}
-
 	printf("Planes:\n");
 	printf("id\tcrtc\tfb\tCRTC x,y\tx,y\tgamma size\tpossible crtcs\n");
-	for (i = 0; i < plane_resources->count_planes; i++) {
-		ovr = drmModeGetPlane(fd, plane_resources->planes[i]);
-		if (!ovr) {
-			fprintf(stderr, "drmModeGetPlane failed: %s\n",
-				strerror(errno));
+
+	if (!resources->plane_res)
+		return;
+
+	for (i = 0; i < resources->plane_res->count_planes; i++) {
+		struct plane *plane = &resources->planes[i];
+		drmModePlane *ovr = plane->plane;
+		if (!ovr)
 			continue;
-		}
 
 		printf("%d\t%d\t%d\t%d,%d\t\t%d,%d\t%-8d\t0x%08x\n",
 		       ovr->plane_id, ovr->crtc_id, ovr->fb_id,
@@ -451,27 +453,170 @@ static void dump_planes(void)
 			printf(" %4.4s", (char *)&ovr->formats[j]);
 		printf("\n");
 
-		printf("  props:\n");
-		props = drmModeObjectGetProperties(fd, ovr->plane_id,
-						   DRM_MODE_OBJECT_PLANE);
-		if (props) {
-			for (j = 0; j < props->count_props; j++)
-				dump_prop(props->props[j],
-					  props->prop_values[j]);
-			drmModeFreeObjectProperties(props);
+		if (plane->props) {
+			printf("  props:\n");
+			for (j = 0; j < plane->props->count_props; j++)
+				dump_prop(plane->props_info[j],
+					  plane->props->props[j],
+					  plane->props->prop_values[j]);
 		} else {
-			printf("\tcould not get plane properties: %s\n",
-			       strerror(errno));
+			printf("  no properties found\n");
 		}
-
-		drmModeFreePlane(ovr);
 	}
 	printf("\n");
 
-	drmModeFreePlaneResources(plane_resources);
 	return;
 }
 
+static void free_resources(struct resources *res)
+{
+	if (!res)
+		return;
+
+#define free_resource(_res, __res, type, Type)					\
+	do {									\
+		int i;								\
+		if (!(_res)->type##s)						\
+			break;							\
+		for (i = 0; i < (int)(_res)->__res->count_##type##s; ++i) {	\
+			if (!(_res)->type##s[i].type)				\
+				break;						\
+			drmModeFree##Type((_res)->type##s[i].type);		\
+		}								\
+		free((_res)->type##s);						\
+	} while (0)
+
+#define free_properties(_res, __res, type)					\
+	do {									\
+		int i;								\
+		for (i = 0; i < (int)(_res)->__res->count_##type##s; ++i) {	\
+			drmModeFreeObjectProperties(res->type##s[i].props);	\
+			free(res->type##s[i].props_info);			\
+		}								\
+	} while (0)
+
+	if (res->res) {
+		free_properties(res, res, crtc);
+
+		free_resource(res, res, crtc, Crtc);
+		free_resource(res, res, encoder, Encoder);
+		free_resource(res, res, connector, Connector);
+		free_resource(res, res, fb, FB);
+
+		drmModeFreeResources(res->res);
+	}
+
+	if (res->plane_res) {
+		free_properties(res, plane_res, plane);
+
+		free_resource(res, plane_res, plane, Plane);
+
+		drmModeFreePlaneResources(res->plane_res);
+	}
+
+	free(res);
+}
+
+static struct resources *get_resources(int fd)
+{
+	struct resources *res;
+
+	res = malloc(sizeof *res);
+	if (res == 0)
+		return NULL;
+
+	memset(res, 0, sizeof *res);
+
+	res->res = drmModeGetResources(fd);
+	if (!res->res) {
+		fprintf(stderr, "drmModeGetResources failed: %s\n",
+			strerror(errno));
+		goto error;
+	}
+
+	res->crtcs = malloc(res->res->count_crtcs * sizeof *res->crtcs);
+	res->encoders = malloc(res->res->count_encoders * sizeof *res->encoders);
+	res->connectors = malloc(res->res->count_connectors * sizeof *res->connectors);
+	res->fbs = malloc(res->res->count_fbs * sizeof *res->fbs);
+
+	if (!res->crtcs || !res->encoders || !res->connectors || !res->fbs)
+		goto error;
+
+	memset(res->crtcs , 0, res->res->count_crtcs * sizeof *res->crtcs);
+	memset(res->encoders, 0, res->res->count_encoders * sizeof *res->encoders);
+	memset(res->connectors, 0, res->res->count_connectors * sizeof *res->connectors);
+	memset(res->fbs, 0, res->res->count_fbs * sizeof *res->fbs);
+
+#define get_resource(_res, __res, type, Type)					\
+	do {									\
+		int i;								\
+		for (i = 0; i < (int)(_res)->__res->count_##type##s; ++i) {	\
+			(_res)->type##s[i].type =				\
+				drmModeGet##Type(fd, (_res)->__res->type##s[i]); \
+			if (!(_res)->type##s[i].type)				\
+				fprintf(stderr, "could not get %s %i: %s\n",	\
+					#type, (_res)->__res->type##s[i],	\
+					strerror(errno));			\
+		}								\
+	} while (0)
+
+	get_resource(res, res, crtc, Crtc);
+	get_resource(res, res, encoder, Encoder);
+	get_resource(res, res, connector, Connector);
+	get_resource(res, res, fb, FB);
+
+#define get_properties(_res, __res, type, Type)					\
+	do {									\
+		int i;								\
+		for (i = 0; i < (int)(_res)->__res->count_##type##s; ++i) {	\
+			struct type *obj = &res->type##s[i];			\
+			unsigned int j;						\
+			obj->props =						\
+				drmModeObjectGetProperties(fd, obj->type->type##_id, \
+							   DRM_MODE_OBJECT_##Type); \
+			if (!obj->props) {					\
+				fprintf(stderr,					\
+					"could not get %s %i properties: %s\n", \
+					#type, obj->type->type##_id,		\
+					strerror(errno));			\
+				continue;					\
+			}							\
+			obj->props_info = malloc(obj->props->count_props *	\
+						 sizeof *obj->props_info);	\
+			if (!obj->props_info)					\
+				continue;					\
+			for (j = 0; j < obj->props->count_props; ++j)		\
+				obj->props_info[j] =				\
+					drmModeGetProperty(fd, obj->props->props[j]); \
+		}								\
+	} while (0)
+
+	get_properties(res, res, crtc, CRTC);
+	get_properties(res, res, connector, CONNECTOR);
+
+	res->plane_res = drmModeGetPlaneResources(fd);
+	if (!res->plane_res) {
+		fprintf(stderr, "drmModeGetPlaneResources failed: %s\n",
+			strerror(errno));
+		return res;
+	}
+
+	res->planes = malloc(res->plane_res->count_planes * sizeof *res->planes);
+	if (!res->planes)
+		goto error;
+
+	memset(res->planes, 0, res->plane_res->count_planes * sizeof *res->planes);
+
+	get_resource(res, plane_res, plane, Plane);
+	get_properties(res, plane_res, plane, PLANE);
+
+	return res;
+
+error:
+	free_resources(res);
+	return NULL;
+}
+
 /* -----------------------------------------------------------------------------
  * Connectors and planes
  */
@@ -483,7 +628,7 @@ static void dump_planes(void)
  * Then you need to find the encoder attached to that connector so you
  * can bind it with a free crtc.
  */
-struct connector {
+struct connector_arg {
 	uint32_t id;
 	char mode_str[64];
 	char format_str[5];
@@ -498,7 +643,7 @@ struct connector {
 	int swap_count;
 };
 
-struct plane {
+struct plane_arg {
 	uint32_t con_id;  /* the id of connector to bind to */
 	uint32_t w, h;
 	unsigned int fb_id;
@@ -507,32 +652,23 @@ struct plane {
 };
 
 static void
-connector_find_mode(struct connector *c)
+connector_find_mode(struct connector_arg *c)
 {
 	drmModeConnector *connector;
 	int i, j;
 
 	/* First, find the connector & mode */
 	c->mode = NULL;
-	for (i = 0; i < resources->count_connectors; i++) {
-		connector = drmModeGetConnector(fd, resources->connectors[i]);
-
-		if (!connector) {
-			fprintf(stderr, "could not get connector %i: %s\n",
-				resources->connectors[i], strerror(errno));
-			drmModeFreeConnector(connector);
+	for (i = 0; i < resources->res->count_connectors; i++) {
+		connector = resources->connectors[i].connector;
+		if (!connector)
 			continue;
-		}
 
-		if (!connector->count_modes) {
-			drmModeFreeConnector(connector);
+		if (!connector->count_modes)
 			continue;
-		}
 
-		if (connector->connector_id != c->id) {
-			drmModeFreeConnector(connector);
+		if (connector->connector_id != c->id)
 			continue;
-		}
 
 		for (j = 0; j < connector->count_modes; j++) {
 			c->mode = &connector->modes[j];
@@ -543,8 +679,6 @@ connector_find_mode(struct connector *c)
 		/* Found it, break out */
 		if (c->mode)
 			break;
-
-		drmModeFreeConnector(connector);
 	}
 
 	if (!c->mode) {
@@ -553,28 +687,21 @@ connector_find_mode(struct connector *c)
 	}
 
 	/* Now get the encoder */
-	for (i = 0; i < resources->count_encoders; i++) {
-		c->encoder = drmModeGetEncoder(fd, resources->encoders[i]);
-
-		if (!c->encoder) {
-			fprintf(stderr, "could not get encoder %i: %s\n",
-				resources->encoders[i], strerror(errno));
-			drmModeFreeEncoder(c->encoder);
+	for (i = 0; i < resources->res->count_encoders; i++) {
+		c->encoder = resources->encoders[i].encoder;
+		if (!c->encoder)
 			continue;
-		}
 
 		if (c->encoder->encoder_id  == connector->encoder_id)
 			break;
-
-		drmModeFreeEncoder(c->encoder);
 	}
 
 	if (c->crtc == -1)
 		c->crtc = c->encoder->crtc_id;
 
 	/* and figure out which crtc index it is: */
-	for (i = 0; i < resources->count_crtcs; i++) {
-		if (c->crtc == (int)resources->crtcs[i]) {
+	for (i = 0; i < resources->res->count_crtcs; i++) {
+		if (c->crtc == (int)resources->res->crtcs[i]) {
 			c->pipe = i;
 			break;
 		}
@@ -588,7 +715,7 @@ static void
 page_flip_handler(int fd, unsigned int frame,
 		  unsigned int sec, unsigned int usec, void *data)
 {
-	struct connector *c;
+	struct connector_arg *c;
 	unsigned int new_fb_id;
 	struct timeval end;
 	double t;
@@ -614,9 +741,8 @@ page_flip_handler(int fd, unsigned int frame,
 }
 
 static int
-set_plane(struct kms_driver *kms, struct connector *c, struct plane *p)
+set_plane(struct kms_driver *kms, struct connector_arg *c, struct plane_arg *p)
 {
-	drmModePlaneRes *plane_resources;
 	drmModePlane *ovr;
 	uint32_t handles[4], pitches[4], offsets[4] = {0}; /* we only use [0] */
 	uint32_t plane_id = 0;
@@ -626,25 +752,13 @@ set_plane(struct kms_driver *kms, struct connector *c, struct plane *p)
 	unsigned int i;
 
 	/* find an unused plane which can be connected to our crtc */
-	plane_resources = drmModeGetPlaneResources(fd);
-	if (!plane_resources) {
-		fprintf(stderr, "drmModeGetPlaneResources failed: %s\n",
-			strerror(errno));
-		return -1;
-	}
-
-	for (i = 0; i < plane_resources->count_planes && !plane_id; i++) {
-		ovr = drmModeGetPlane(fd, plane_resources->planes[i]);
-		if (!ovr) {
-			fprintf(stderr, "drmModeGetPlane failed: %s\n",
-				strerror(errno));
-			return -1;
-		}
+	for (i = 0; i < resources->plane_res->count_planes && !plane_id; i++) {
+		ovr = resources->planes[i].plane;
+		if (!ovr)
+			continue;
 
 		if ((ovr->possible_crtcs & (1 << c->pipe)) && !ovr->crtc_id)
 			plane_id = ovr->plane_id;
-
-		drmModeFreePlane(ovr);
 	}
 
 	fprintf(stderr, "testing %dx%d@%s overlay plane\n",
@@ -682,11 +796,13 @@ set_plane(struct kms_driver *kms, struct connector *c, struct plane *p)
 		return -1;
 	}
 
+	ovr->crtc_id = c->crtc;
+
 	return 0;
 }
 
 static void
-set_mode(struct connector *c, int count, struct plane *p, int plane_count,
+set_mode(struct connector_arg *c, int count, struct plane_arg *p, int plane_count,
 		int page_flip)
 {
 	struct kms_driver *kms;
@@ -837,7 +953,7 @@ set_mode(struct connector *c, int count, struct plane *p, int plane_count,
 
 #define min(a, b)	((a) < (b) ? (a) : (b))
 
-static int parse_connector(struct connector *c, const char *arg)
+static int parse_connector(struct connector_arg *c, const char *arg)
 {
 	unsigned int len;
 	const char *p;
@@ -875,7 +991,7 @@ static int parse_connector(struct connector *c, const char *arg)
 	return 0;
 }
 
-static int parse_plane(struct plane *p, const char *arg)
+static int parse_plane(struct plane_arg *p, const char *arg)
 {
 	strcpy(p->format_str, "XR24");
 
@@ -951,8 +1067,8 @@ int main(int argc, char **argv)
 	char *module = NULL;
 	unsigned int i;
 	int count = 0, plane_count = 0;
-	struct connector con_args[2];
-	struct plane plane_args[2] = { { 0, }, };
+	struct connector_arg con_args[2];
+	struct plane_arg plane_args[2] = { { 0, }, };
 	unsigned int args = 0;
 	
 	opterr = 0;
@@ -1035,10 +1151,8 @@ int main(int argc, char **argv)
 		return -1;
 	}
 
-	resources = drmModeGetResources(fd);
+	resources = get_resources(fd);
 	if (!resources) {
-		fprintf(stderr, "drmModeGetResources failed: %s\n",
-			strerror(errno));
 		drmClose(fd);
 		return 1;
 	}
@@ -1056,7 +1170,7 @@ int main(int argc, char **argv)
 		getchar();
 	}
 
-	drmModeFreeResources(resources);
+	free_resources(resources);
 
 	return 0;
 }
-- 
1.8.1.5

[PATCH v6 07/23] modetest: Don't limit mode set and planes to two instances

[Laurent Pinchart]

Configuring mode on more than two connectors or two planes is perfectly
valid. Support it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/modetest/modetest.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 8ad138e..778af62 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -1067,8 +1067,8 @@ int main(int argc, char **argv)
 	char *module = NULL;
 	unsigned int i;
 	int count = 0, plane_count = 0;
-	struct connector_arg con_args[2];
-	struct plane_arg plane_args[2] = { { 0, }, };
+	struct connector_arg *con_args = NULL;
+	struct plane_arg *plane_args = NULL;
 	unsigned int args = 0;
 	
 	opterr = 0;
@@ -1097,8 +1097,16 @@ int main(int argc, char **argv)
 			modes = 1;
 			break;
 		case 'P':
+			plane_args = realloc(plane_args,
+					     (plane_count + 1) * sizeof *plane_args);
+			if (plane_args == NULL) {
+				fprintf(stderr, "memory allocation failed\n");
+				return 1;
+			}
+
 			if (parse_plane(&plane_args[plane_count], optarg) < 0)
 				usage(argv[0]);
+
 			plane_count++;
 			break;
 		case 'p':
@@ -1106,8 +1114,16 @@ int main(int argc, char **argv)
 			planes = 1;
 			break;
 		case 's':
+			con_args = realloc(con_args,
+					   (count + 1) * sizeof *con_args);
+			if (con_args == NULL) {
+				fprintf(stderr, "memory allocation failed\n");
+				return 1;
+			}
+
 			if (parse_connector(&con_args[count], optarg) < 0)
 				usage(argv[0]);
+
 			count++;				      
 			break;
 		case 'v':
-- 
1.8.1.5

[PATCH v6 08/23] modetest: Add a command line parameter to set properties

[Laurent Pinchart]

The -w parameter can be used to set a property value from the command
line, using the target object ID and the property name.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/modetest/modetest.c | 108 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 106 insertions(+), 2 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 778af62..858d480 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -709,6 +709,80 @@ connector_find_mode(struct connector_arg *c)
 
 }
 
+/* -----------------------------------------------------------------------------
+ * Properties
+ */
+
+struct property_arg {
+	uint32_t obj_id;
+	uint32_t obj_type;
+	char name[DRM_PROP_NAME_LEN+1];
+	uint32_t prop_id;
+	uint64_t value;
+};
+
+static void set_property(struct property_arg *p)
+{
+	drmModeObjectProperties *props;
+	drmModePropertyRes **props_info;
+	const char *obj_type;
+	int ret;
+	int i;
+
+	p->obj_type = 0;
+	p->prop_id = 0;
+
+#define find_object(_res, __res, type, Type)					\
+	do {									\
+		for (i = 0; i < (int)(_res)->__res->count_##type##s; ++i) {	\
+			struct type *obj = &(_res)->type##s[i];			\
+			if (obj->type->type##_id != p->obj_id)			\
+				continue;					\
+			p->obj_type = DRM_MODE_OBJECT_##Type;			\
+			obj_type = #Type;					\
+			props = obj->props;					\
+			props_info = obj->props_info;				\
+		}								\
+	} while(0)								\
+
+	find_object(resources, res, crtc, CRTC);
+	if (p->obj_type == 0)
+		find_object(resources, res, connector, CONNECTOR);
+	if (p->obj_type == 0)
+		find_object(resources, plane_res, plane, PLANE);
+	if (p->obj_type == 0) {
+		fprintf(stderr, "Object %i not found, can't set property\n",
+			p->obj_id);
+			return;
+	}
+
+	if (!props) {
+		fprintf(stderr, "%s %i has no properties\n",
+			obj_type, p->obj_id);
+		return;
+	}
+
+	for (i = 0; i < (int)props->count_props; ++i) {
+		if (!props_info[i])
+			continue;
+		if (strcmp(props_info[i]->name, p->name) == 0)
+			break;
+	}
+
+	if (i == (int)props->count_props) {
+		fprintf(stderr, "%s %i has no %s property\n",
+			obj_type, p->obj_id, p->name);
+		return;
+	}
+
+	p->prop_id = props->props[i];
+
+	ret = drmModeObjectSetProperty(fd, p->obj_id, p->obj_type, p->prop_id, p->value);
+	if (ret < 0)
+		fprintf(stderr, "failed to set %s %i property %s to %" PRIu64 ": %s\n",
+			obj_type, p->obj_id, p->name, p->value, strerror(errno));
+}
+
 /* -------------------------------------------------------------------------- */
 
 static void
@@ -1008,9 +1082,20 @@ static int parse_plane(struct plane_arg *p, const char *arg)
 	return 0;
 }
 
+static int parse_property(struct property_arg *p, const char *arg)
+{
+	if (sscanf(arg, "%d:%32[^:]:%" SCNu64, &p->obj_id, p->name, &p->value) != 3)
+		return -1;
+
+	p->obj_type = 0;
+	p->name[DRM_PROP_NAME_LEN] = '\0';
+
+	return 0;
+}
+
 static void usage(char *name)
 {
-	fprintf(stderr, "usage: %s [-cdefMmPpsv]\n", name);
+	fprintf(stderr, "usage: %s [-cdefMmPpsvw]\n", name);
 
 	fprintf(stderr, "\n Query options:\n\n");
 	fprintf(stderr, "\t-c\tlist connectors\n");
@@ -1023,6 +1108,7 @@ static void usage(char *name)
 	fprintf(stderr, "\t-P <connector_id>:<w>x<h>[@<format>]\tset a plane\n");
 	fprintf(stderr, "\t-s <connector_id>[@<crtc_id>]:<mode>[@<format>]\tset a mode\n");
 	fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
+	fprintf(stderr, "\t-w <obj_id>:<prop_name>:<value>\tset property\n");
 
 	fprintf(stderr, "\n Generic options:\n\n");
 	fprintf(stderr, "\t-d\tdrop master after mode set\n");
@@ -1055,7 +1141,7 @@ static int page_flipping_supported(void)
 #endif
 }
 
-static char optstr[] = "cdefM:mP:ps:v";
+static char optstr[] = "cdefM:mP:ps:vw:";
 
 int main(int argc, char **argv)
 {
@@ -1067,8 +1153,10 @@ int main(int argc, char **argv)
 	char *module = NULL;
 	unsigned int i;
 	int count = 0, plane_count = 0;
+	unsigned int prop_count = 0;
 	struct connector_arg *con_args = NULL;
 	struct plane_arg *plane_args = NULL;
+	struct property_arg *prop_args = NULL;
 	unsigned int args = 0;
 	
 	opterr = 0;
@@ -1129,6 +1217,19 @@ int main(int argc, char **argv)
 		case 'v':
 			test_vsync = 1;
 			break;
+		case 'w':
+			prop_args = realloc(prop_args,
+					   (prop_count + 1) * sizeof *prop_args);
+			if (prop_args == NULL) {
+				fprintf(stderr, "memory allocation failed\n");
+				return 1;
+			}
+
+			if (parse_property(&prop_args[prop_count], optarg) < 0)
+				usage(argv[0]);
+
+			prop_count++;
+			break;
 		default:
 			usage(argv[0]);
 			break;
@@ -1179,6 +1280,9 @@ int main(int argc, char **argv)
 	dump_resource(planes);
 	dump_resource(framebuffers);
 
+	for (i = 0; i < prop_count; ++i)
+		set_property(&prop_args[i]);
+
 	if (count > 0) {
 		set_mode(con_args, count, plane_args, plane_count, test_vsync);
 		if (drop_master)
-- 
1.8.1.5

[PATCH v6 09/23] modetest: Allow specifying plane position

[Laurent Pinchart]

Extend the -P option to allow specifying the plane x and y offsets. The
position is optional, if not specified the plane will be positioned at
the center of the screen as before.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/modetest/modetest.c | 67 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 52 insertions(+), 15 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 858d480..897a66c 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -40,6 +40,7 @@
 #include "config.h"
 
 #include <assert.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdint.h>
@@ -645,6 +646,8 @@ struct connector_arg {
 
 struct plane_arg {
 	uint32_t con_id;  /* the id of connector to bind to */
+	bool has_position;
+	int32_t x, y;
 	uint32_t w, h;
 	unsigned int fb_id;
 	char format_str[5]; /* need to leave room for terminating \0 */
@@ -855,11 +858,16 @@ set_plane(struct kms_driver *kms, struct connector_arg *c, struct plane_arg *p)
 		return -1;
 	}
 
-	/* ok, boring.. but for now put in middle of screen: */
-	crtc_x = c->mode->hdisplay / 3;
-	crtc_y = c->mode->vdisplay / 3;
-	crtc_w = crtc_x;
-	crtc_h = crtc_y;
+	if (!p->has_position) {
+		/* Default to the middle of the screen */
+		crtc_x = (c->mode->hdisplay - p->w) / 2;
+		crtc_y = (c->mode->vdisplay - p->h) / 2;
+	} else {
+		crtc_x = p->x;
+		crtc_y = p->y;
+	}
+	crtc_w = p->w;
+	crtc_h = p->h;
 
 	/* note src coords (last 4 args) are in Q16 format */
 	if (drmModeSetPlane(fd, plane_id, c->crtc, p->fb_id,
@@ -1065,18 +1073,47 @@ static int parse_connector(struct connector_arg *c, const char *arg)
 	return 0;
 }
 
-static int parse_plane(struct plane_arg *p, const char *arg)
+static int parse_plane(struct plane_arg *plane, const char *p)
 {
-	strcpy(p->format_str, "XR24");
+	char *end;
 
-	if (sscanf(arg, "%d:%dx%d@%4s", &p->con_id, &p->w, &p->h, p->format_str) != 4 &&
-	    sscanf(arg, "%d:%dx%d", &p->con_id, &p->w, &p->h) != 3)
-		return -1;
+	memset(plane, 0, sizeof *plane);
 
-	p->fourcc = format_fourcc(p->format_str);
-	if (p->fourcc == 0) {
-		fprintf(stderr, "unknown format %s\n", p->format_str);
-		return -1;
+	plane->con_id = strtoul(p, &end, 10);
+	if (*end != ':')
+		return -EINVAL;
+
+	p = end + 1;
+	plane->w = strtoul(p, &end, 10);
+	if (*end != 'x')
+		return -EINVAL;
+
+	p = end + 1;
+	plane->h = strtoul(p, &end, 10);
+
+	if (*end == '+' || *end == '-') {
+		plane->x = strtol(end, &end, 10);
+		if (*end != '+' && *end != '-')
+			return -EINVAL;
+		plane->y = strtol(end, &end, 10);
+
+		plane->has_position = true;
+	}
+
+	if (*end == '@') {
+		p = end + 1;
+		if (strlen(p) != 4)
+			return -EINVAL;
+
+		strcpy(plane->format_str, p);
+	} else {
+		strcpy(plane->format_str, "XR24");
+	}
+
+	plane->fourcc = format_fourcc(plane->format_str);
+	if (plane->fourcc == 0) {
+		fprintf(stderr, "unknown format %s\n", plane->format_str);
+		return -EINVAL;
 	}
 
 	return 0;
@@ -1105,7 +1142,7 @@ static void usage(char *name)
 	fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n");
 
 	fprintf(stderr, "\n Test options:\n\n");
-	fprintf(stderr, "\t-P <connector_id>:<w>x<h>[@<format>]\tset a plane\n");
+	fprintf(stderr, "\t-P <connector_id>:<w>x<h>[+<x>+<y>][@<format>]\tset a plane\n");
 	fprintf(stderr, "\t-s <connector_id>[@<crtc_id>]:<mode>[@<format>]\tset a mode\n");
 	fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
 	fprintf(stderr, "\t-w <obj_id>:<prop_name>:<value>\tset property\n");
-- 
1.8.1.5

[PATCH v6 10/23] modetest: Print the plane ID when setting up a plane

[Laurent Pinchart]

As modetest automatically selects an unused plan, providing the plane ID
allows modifying plane properties for the selected planes.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/modetest/modetest.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 897a66c..7f7a3a2 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -838,14 +838,14 @@ set_plane(struct kms_driver *kms, struct connector_arg *c, struct plane_arg *p)
 			plane_id = ovr->plane_id;
 	}
 
-	fprintf(stderr, "testing %dx%d@%s overlay plane\n",
-			p->w, p->h, p->format_str);
-
 	if (!plane_id) {
-		fprintf(stderr, "failed to find plane!\n");
+		fprintf(stderr, "no unused plane available for CRTC %u\n", c->crtc);
 		return -1;
 	}
 
+	fprintf(stderr, "testing %dx%d@%s overlay plane %u\n",
+		p->w, p->h, p->format_str, plane_id);
+
 	plane_bo = create_test_buffer(kms, p->fourcc, p->w, p->h, handles,
 				      pitches, offsets, PATTERN_TILES);
 	if (plane_bo == NULL)
-- 
1.8.1.5

[PATCH v6 11/23] modetest: Remove the -m argument

[Laurent Pinchart]

The argument isn't used, remove it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/modetest/modetest.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 7f7a3a2..89bfc53 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -96,7 +96,7 @@ struct resources {
 };
 
 struct resources *resources;
-int fd, modes;
+int fd;
 
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
 
@@ -1138,7 +1138,6 @@ static void usage(char *name)
 	fprintf(stderr, "\t-c\tlist connectors\n");
 	fprintf(stderr, "\t-e\tlist encoders\n");
 	fprintf(stderr, "\t-f\tlist framebuffers\n");
-	fprintf(stderr, "\t-m\tlist modes\n");
 	fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n");
 
 	fprintf(stderr, "\n Test options:\n\n");
@@ -1178,7 +1177,7 @@ static int page_flipping_supported(void)
 #endif
 }
 
-static char optstr[] = "cdefM:mP:ps:vw:";
+static char optstr[] = "cdefM:P:ps:vw:";
 
 int main(int argc, char **argv)
 {
@@ -1218,9 +1217,6 @@ int main(int argc, char **argv)
 			/* Preserve the default behaviour of dumping all information. */
 			args--;
 			break;
-		case 'm':
-			modes = 1;
-			break;
 		case 'P':
 			plane_args = realloc(plane_args,
 					     (plane_count + 1) * sizeof *plane_args);
@@ -1274,7 +1270,7 @@ int main(int argc, char **argv)
 	}
 
 	if (!args)
-		encoders = connectors = crtcs = planes = modes = framebuffers = 1;
+		encoders = connectors = crtcs = planes = framebuffers = 1;
 
 	if (module) {
 		fd = drmOpen(module, NULL);
-- 
1.8.1.5

[PATCH v6 12/23] modetest: Create a device structure

[Laurent Pinchart]

Instead of passing the device fd and resources as global variables group
them in a device structure and pass it explictly to all functions that
need it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/modetest/modetest.c | 201 ++++++++++++++++++++++++----------------------
 1 file changed, 103 insertions(+), 98 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 89bfc53..cfcb989 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -95,8 +95,12 @@ struct resources {
 	struct plane *planes;
 };
 
-struct resources *resources;
-int fd;
+struct device {
+	int fd;
+
+	struct resources *resources;
+	struct kms_driver *kms;
+};
 
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
 
@@ -197,15 +201,15 @@ static const char *mode_flag_names[] = {
 
 static bit_name_fn(mode_flag)
 
-static void dump_encoders(void)
+static void dump_encoders(struct device *dev)
 {
 	drmModeEncoder *encoder;
 	int i;
 
 	printf("Encoders:\n");
 	printf("id\tcrtc\ttype\tpossible crtcs\tpossible clones\t\n");
-	for (i = 0; i < resources->res->count_encoders; i++) {
-		encoder = resources->encoders[i].encoder;
+	for (i = 0; i < dev->resources->res->count_encoders; i++) {
+		encoder = dev->resources->encoders[i].encoder;
 		if (!encoder)
 			continue;
 
@@ -240,14 +244,13 @@ static void dump_mode(drmModeModeInfo *mode)
 	printf("\n");
 }
 
-static void
-dump_blob(uint32_t blob_id)
+static void dump_blob(struct device *dev, uint32_t blob_id)
 {
 	uint32_t i;
 	unsigned char *blob_data;
 	drmModePropertyBlobPtr blob;
 
-	blob = drmModeGetPropertyBlob(fd, blob_id);
+	blob = drmModeGetPropertyBlob(dev->fd, blob_id);
 	if (!blob)
 		return;
 
@@ -263,8 +266,8 @@ dump_blob(uint32_t blob_id)
 	drmModeFreePropertyBlob(blob);
 }
 
-static void
-dump_prop(drmModePropertyPtr prop, uint32_t prop_id, uint64_t value)
+static void dump_prop(struct device *dev, drmModePropertyPtr prop,
+		      uint32_t prop_id, uint64_t value)
 {
 	int i;
 	printf("\t%d", prop_id);
@@ -316,7 +319,7 @@ dump_prop(drmModePropertyPtr prop, uint32_t prop_id, uint64_t value)
 	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]);
+			dump_blob(dev, prop->blob_ids[i]);
 		printf("\n");
 	} else {
 		assert(prop->count_blobs == 0);
@@ -324,19 +327,19 @@ dump_prop(drmModePropertyPtr prop, uint32_t prop_id, uint64_t value)
 
 	printf("\t\tvalue:");
 	if (prop->flags & DRM_MODE_PROP_BLOB)
-		dump_blob(value);
+		dump_blob(dev, value);
 	else
 		printf(" %"PRIu64"\n", value);
 }
 
-static void dump_connectors(void)
+static void dump_connectors(struct device *dev)
 {
 	int i, j;
 
 	printf("Connectors:\n");
 	printf("id\tencoder\tstatus\t\ttype\tsize (mm)\tmodes\tencoders\n");
-	for (i = 0; i < resources->res->count_connectors; i++) {
-		struct connector *_connector = &resources->connectors[i];
+	for (i = 0; i < dev->resources->res->count_connectors; i++) {
+		struct connector *_connector = &dev->resources->connectors[i];
 		drmModeConnector *connector = _connector->connector;
 		if (!connector)
 			continue;
@@ -364,7 +367,7 @@ static void dump_connectors(void)
 		if (_connector->props) {
 			printf("  props:\n");
 			for (j = 0; j < (int)_connector->props->count_props; j++)
-				dump_prop(_connector->props_info[j],
+				dump_prop(dev, _connector->props_info[j],
 					  _connector->props->props[j],
 					  _connector->props->prop_values[j]);
 		}
@@ -372,15 +375,15 @@ static void dump_connectors(void)
 	printf("\n");
 }
 
-static void dump_crtcs(void)
+static void dump_crtcs(struct device *dev)
 {
 	int i;
 	uint32_t j;
 
 	printf("CRTCs:\n");
 	printf("id\tfb\tpos\tsize\n");
-	for (i = 0; i < resources->res->count_crtcs; i++) {
-		struct crtc *_crtc = &resources->crtcs[i];
+	for (i = 0; i < dev->resources->res->count_crtcs; i++) {
+		struct crtc *_crtc = &dev->resources->crtcs[i];
 		drmModeCrtc *crtc = _crtc->crtc;
 		if (!crtc)
 			continue;
@@ -395,7 +398,7 @@ static void dump_crtcs(void)
 		if (_crtc->props) {
 			printf("  props:\n");
 			for (j = 0; j < _crtc->props->count_props; j++)
-				dump_prop(_crtc->props_info[j],
+				dump_prop(dev, _crtc->props_info[j],
 					  _crtc->props->props[j],
 					  _crtc->props->prop_values[j]);
 		} else {
@@ -405,15 +408,15 @@ static void dump_crtcs(void)
 	printf("\n");
 }
 
-static void dump_framebuffers(void)
+static void dump_framebuffers(struct device *dev)
 {
 	drmModeFB *fb;
 	int i;
 
 	printf("Frame buffers:\n");
 	printf("id\tsize\tpitch\n");
-	for (i = 0; i < resources->res->count_fbs; i++) {
-		fb = resources->fbs[i].fb;
+	for (i = 0; i < dev->resources->res->count_fbs; i++) {
+		fb = dev->resources->fbs[i].fb;
 		if (!fb)
 			continue;
 
@@ -425,18 +428,18 @@ static void dump_framebuffers(void)
 	printf("\n");
 }
 
-static void dump_planes(void)
+static void dump_planes(struct device *dev)
 {
 	unsigned int i, j;
 
 	printf("Planes:\n");
 	printf("id\tcrtc\tfb\tCRTC x,y\tx,y\tgamma size\tpossible crtcs\n");
 
-	if (!resources->plane_res)
+	if (!dev->resources->plane_res)
 		return;
 
-	for (i = 0; i < resources->plane_res->count_planes; i++) {
-		struct plane *plane = &resources->planes[i];
+	for (i = 0; i < dev->resources->plane_res->count_planes; i++) {
+		struct plane *plane = &dev->resources->planes[i];
 		drmModePlane *ovr = plane->plane;
 		if (!ovr)
 			continue;
@@ -457,7 +460,7 @@ static void dump_planes(void)
 		if (plane->props) {
 			printf("  props:\n");
 			for (j = 0; j < plane->props->count_props; j++)
-				dump_prop(plane->props_info[j],
+				dump_prop(dev, plane->props_info[j],
 					  plane->props->props[j],
 					  plane->props->prop_values[j]);
 		} else {
@@ -518,7 +521,7 @@ static void free_resources(struct resources *res)
 	free(res);
 }
 
-static struct resources *get_resources(int fd)
+static struct resources *get_resources(struct device *dev)
 {
 	struct resources *res;
 
@@ -528,7 +531,7 @@ static struct resources *get_resources(int fd)
 
 	memset(res, 0, sizeof *res);
 
-	res->res = drmModeGetResources(fd);
+	res->res = drmModeGetResources(dev->fd);
 	if (!res->res) {
 		fprintf(stderr, "drmModeGetResources failed: %s\n",
 			strerror(errno));
@@ -553,7 +556,7 @@ static struct resources *get_resources(int fd)
 		int i;								\
 		for (i = 0; i < (int)(_res)->__res->count_##type##s; ++i) {	\
 			(_res)->type##s[i].type =				\
-				drmModeGet##Type(fd, (_res)->__res->type##s[i]); \
+				drmModeGet##Type(dev->fd, (_res)->__res->type##s[i]); \
 			if (!(_res)->type##s[i].type)				\
 				fprintf(stderr, "could not get %s %i: %s\n",	\
 					#type, (_res)->__res->type##s[i],	\
@@ -573,7 +576,7 @@ static struct resources *get_resources(int fd)
 			struct type *obj = &res->type##s[i];			\
 			unsigned int j;						\
 			obj->props =						\
-				drmModeObjectGetProperties(fd, obj->type->type##_id, \
+				drmModeObjectGetProperties(dev->fd, obj->type->type##_id, \
 							   DRM_MODE_OBJECT_##Type); \
 			if (!obj->props) {					\
 				fprintf(stderr,					\
@@ -588,14 +591,14 @@ static struct resources *get_resources(int fd)
 				continue;					\
 			for (j = 0; j < obj->props->count_props; ++j)		\
 				obj->props_info[j] =				\
-					drmModeGetProperty(fd, obj->props->props[j]); \
+					drmModeGetProperty(dev->fd, obj->props->props[j]); \
 		}								\
 	} while (0)
 
 	get_properties(res, res, crtc, CRTC);
 	get_properties(res, res, connector, CONNECTOR);
 
-	res->plane_res = drmModeGetPlaneResources(fd);
+	res->plane_res = drmModeGetPlaneResources(dev->fd);
 	if (!res->plane_res) {
 		fprintf(stderr, "drmModeGetPlaneResources failed: %s\n",
 			strerror(errno));
@@ -654,16 +657,15 @@ struct plane_arg {
 	unsigned int fourcc;
 };
 
-static void
-connector_find_mode(struct connector_arg *c)
+static void connector_find_mode(struct device *dev, struct connector_arg *c)
 {
 	drmModeConnector *connector;
 	int i, j;
 
 	/* First, find the connector & mode */
 	c->mode = NULL;
-	for (i = 0; i < resources->res->count_connectors; i++) {
-		connector = resources->connectors[i].connector;
+	for (i = 0; i < dev->resources->res->count_connectors; i++) {
+		connector = dev->resources->connectors[i].connector;
 		if (!connector)
 			continue;
 
@@ -690,8 +692,8 @@ connector_find_mode(struct connector_arg *c)
 	}
 
 	/* Now get the encoder */
-	for (i = 0; i < resources->res->count_encoders; i++) {
-		c->encoder = resources->encoders[i].encoder;
+	for (i = 0; i < dev->resources->res->count_encoders; i++) {
+		c->encoder = dev->resources->encoders[i].encoder;
 		if (!c->encoder)
 			continue;
 
@@ -703,8 +705,8 @@ connector_find_mode(struct connector_arg *c)
 		c->crtc = c->encoder->crtc_id;
 
 	/* and figure out which crtc index it is: */
-	for (i = 0; i < resources->res->count_crtcs; i++) {
-		if (c->crtc == (int)resources->res->crtcs[i]) {
+	for (i = 0; i < dev->resources->res->count_crtcs; i++) {
+		if (c->crtc == (int)dev->resources->res->crtcs[i]) {
 			c->pipe = i;
 			break;
 		}
@@ -724,7 +726,7 @@ struct property_arg {
 	uint64_t value;
 };
 
-static void set_property(struct property_arg *p)
+static void set_property(struct device *dev, struct property_arg *p)
 {
 	drmModeObjectProperties *props;
 	drmModePropertyRes **props_info;
@@ -748,11 +750,11 @@ static void set_property(struct property_arg *p)
 		}								\
 	} while(0)								\
 
-	find_object(resources, res, crtc, CRTC);
+	find_object(dev->resources, res, crtc, CRTC);
 	if (p->obj_type == 0)
-		find_object(resources, res, connector, CONNECTOR);
+		find_object(dev->resources, res, connector, CONNECTOR);
 	if (p->obj_type == 0)
-		find_object(resources, plane_res, plane, PLANE);
+		find_object(dev->resources, plane_res, plane, PLANE);
 	if (p->obj_type == 0) {
 		fprintf(stderr, "Object %i not found, can't set property\n",
 			p->obj_id);
@@ -780,7 +782,8 @@ static void set_property(struct property_arg *p)
 
 	p->prop_id = props->props[i];
 
-	ret = drmModeObjectSetProperty(fd, p->obj_id, p->obj_type, p->prop_id, p->value);
+	ret = drmModeObjectSetProperty(dev->fd, p->obj_id, p->obj_type,
+				       p->prop_id, p->value);
 	if (ret < 0)
 		fprintf(stderr, "failed to set %s %i property %s to %" PRIu64 ": %s\n",
 			obj_type, p->obj_id, p->name, p->value, strerror(errno));
@@ -818,7 +821,7 @@ page_flip_handler(int fd, unsigned int frame,
 }
 
 static int
-set_plane(struct kms_driver *kms, struct connector_arg *c, struct plane_arg *p)
+set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p)
 {
 	drmModePlane *ovr;
 	uint32_t handles[4], pitches[4], offsets[4] = {0}; /* we only use [0] */
@@ -829,8 +832,8 @@ set_plane(struct kms_driver *kms, struct connector_arg *c, struct plane_arg *p)
 	unsigned int i;
 
 	/* find an unused plane which can be connected to our crtc */
-	for (i = 0; i < resources->plane_res->count_planes && !plane_id; i++) {
-		ovr = resources->planes[i].plane;
+	for (i = 0; i < dev->resources->plane_res->count_planes && !plane_id; i++) {
+		ovr = dev->resources->planes[i].plane;
 		if (!ovr)
 			continue;
 
@@ -846,13 +849,13 @@ set_plane(struct kms_driver *kms, struct connector_arg *c, struct plane_arg *p)
 	fprintf(stderr, "testing %dx%d@%s overlay plane %u\n",
 		p->w, p->h, p->format_str, plane_id);
 
-	plane_bo = create_test_buffer(kms, p->fourcc, p->w, p->h, handles,
+	plane_bo = create_test_buffer(dev->kms, p->fourcc, p->w, p->h, handles,
 				      pitches, offsets, PATTERN_TILES);
 	if (plane_bo == NULL)
 		return -1;
 
 	/* just use single plane format for now.. */
-	if (drmModeAddFB2(fd, p->w, p->h, p->fourcc,
+	if (drmModeAddFB2(dev->fd, p->w, p->h, p->fourcc,
 			handles, pitches, offsets, &p->fb_id, plane_flags)) {
 		fprintf(stderr, "failed to add fb: %s\n", strerror(errno));
 		return -1;
@@ -870,7 +873,7 @@ set_plane(struct kms_driver *kms, struct connector_arg *c, struct plane_arg *p)
 	crtc_h = p->h;
 
 	/* note src coords (last 4 args) are in Q16 format */
-	if (drmModeSetPlane(fd, plane_id, c->crtc, p->fb_id,
+	if (drmModeSetPlane(dev->fd, plane_id, c->crtc, p->fb_id,
 			    plane_flags, crtc_x, crtc_y, crtc_w, crtc_h,
 			    0, 0, p->w << 16, p->h << 16)) {
 		fprintf(stderr, "failed to enable plane: %s\n",
@@ -883,11 +886,9 @@ set_plane(struct kms_driver *kms, struct connector_arg *c, struct plane_arg *p)
 	return 0;
 }
 
-static void
-set_mode(struct connector_arg *c, int count, struct plane_arg *p, int plane_count,
-		int page_flip)
+static void set_mode(struct device *dev, struct connector_arg *c, int count,
+		     struct plane_arg *p, int plane_count, int page_flip)
 {
-	struct kms_driver *kms;
 	struct kms_bo *bo, *other_bo;
 	unsigned int fb_id, other_fb_id;
 	int i, j, ret, width, height, x;
@@ -897,7 +898,7 @@ set_mode(struct connector_arg *c, int count, struct plane_arg *p, int plane_coun
 	width = 0;
 	height = 0;
 	for (i = 0; i < count; i++) {
-		connector_find_mode(&c[i]);
+		connector_find_mode(dev, &c[i]);
 		if (c[i].mode == NULL)
 			continue;
 		width += c[i].mode->hdisplay;
@@ -905,19 +906,12 @@ set_mode(struct connector_arg *c, int count, struct plane_arg *p, int plane_coun
 			height = c[i].mode->vdisplay;
 	}
 
-	ret = kms_create(fd, &kms);
-	if (ret) {
-		fprintf(stderr, "failed to create kms driver: %s\n",
-			strerror(-ret));
-		return;
-	}
-
-	bo = create_test_buffer(kms, c->fourcc, width, height, handles,
+	bo = create_test_buffer(dev->kms, c->fourcc, width, height, handles,
 				pitches, offsets, PATTERN_SMPTE);
 	if (bo == NULL)
 		return;
 
-	ret = drmModeAddFB2(fd, width, height, c->fourcc,
+	ret = drmModeAddFB2(dev->fd, width, height, c->fourcc,
 			    handles, pitches, offsets, &fb_id, 0);
 	if (ret) {
 		fprintf(stderr, "failed to add fb (%ux%u): %s\n",
@@ -933,11 +927,11 @@ set_mode(struct connector_arg *c, int count, struct plane_arg *p, int plane_coun
 		printf("setting mode %s@%s on connector %d, crtc %d\n",
 		       c[i].mode_str, c[i].format_str, c[i].id, c[i].crtc);
 
-		ret = drmModeSetCrtc(fd, c[i].crtc, fb_id, x, 0,
+		ret = drmModeSetCrtc(dev->fd, c[i].crtc, fb_id, x, 0,
 				     &c[i].id, 1, c[i].mode);
 
 		/* XXX: Actually check if this is needed */
-		drmModeDirtyFB(fd, fb_id, NULL, 0);
+		drmModeDirtyFB(dev->fd, fb_id, NULL, 0);
 
 		x += c[i].mode->hdisplay;
 
@@ -949,19 +943,19 @@ set_mode(struct connector_arg *c, int count, struct plane_arg *p, int plane_coun
 		/* if we have a plane/overlay to show, set that up now: */
 		for (j = 0; j < plane_count; j++)
 			if (p[j].con_id == c[i].id)
-				if (set_plane(kms, &c[i], &p[j]))
+				if (set_plane(dev, &c[i], &p[j]))
 					return;
 	}
 
 	if (!page_flip)
 		return;
-	
-	other_bo = create_test_buffer(kms, c->fourcc, width, height, handles,
+
+	other_bo = create_test_buffer(dev->kms, c->fourcc, width, height, handles,
 				      pitches, offsets, PATTERN_PLAIN);
 	if (other_bo == NULL)
 		return;
 
-	ret = drmModeAddFB2(fd, width, height, c->fourcc, handles, pitches, offsets,
+	ret = drmModeAddFB2(dev->fd, width, height, c->fourcc, handles, pitches, offsets,
 			    &other_fb_id, 0);
 	if (ret) {
 		fprintf(stderr, "failed to add fb: %s\n", strerror(errno));
@@ -972,7 +966,7 @@ set_mode(struct connector_arg *c, int count, struct plane_arg *p, int plane_coun
 		if (c[i].mode == NULL)
 			continue;
 
-		ret = drmModePageFlip(fd, c[i].crtc, other_fb_id,
+		ret = drmModePageFlip(dev->fd, c[i].crtc, other_fb_id,
 				      DRM_MODE_PAGE_FLIP_EVENT, &c[i]);
 		if (ret) {
 			fprintf(stderr, "failed to page flip: %s\n", strerror(errno));
@@ -1013,8 +1007,8 @@ set_mode(struct connector_arg *c, int count, struct plane_arg *p, int plane_coun
 
 		FD_ZERO(&fds);
 		FD_SET(0, &fds);
-		FD_SET(fd, &fds);
-		ret = select(fd + 1, &fds, NULL, NULL, &timeout);
+		FD_SET(dev->fd, &fds);
+		ret = select(dev->fd + 1, &fds, NULL, NULL, &timeout);
 
 		if (ret <= 0) {
 			fprintf(stderr, "select timed out or error (ret %d)\n",
@@ -1025,12 +1019,11 @@ set_mode(struct connector_arg *c, int count, struct plane_arg *p, int plane_coun
 		}
 #endif
 
-		drmHandleEvent(fd, &evctx);
+		drmHandleEvent(dev->fd, &evctx);
 	}
 
 	kms_bo_destroy(&bo);
 	kms_bo_destroy(&other_bo);
-	kms_destroy(&kms);
 }
 
 #define min(a, b)	((a) < (b) ? (a) : (b))
@@ -1154,8 +1147,6 @@ static void usage(char *name)
 	exit(0);
 }
 
-#define dump_resource(res) if (res) dump_##res()
-
 static int page_flipping_supported(void)
 {
 	/*FIXME: generic ioctl needed? */
@@ -1181,6 +1172,8 @@ static char optstr[] = "cdefM:P:ps:vw:";
 
 int main(int argc, char **argv)
 {
+	struct device dev;
+
 	int c;
 	int encoders = 0, connectors = 0, crtcs = 0, planes = 0, framebuffers = 0;
 	int drop_master = 0;
@@ -1194,7 +1187,8 @@ int main(int argc, char **argv)
 	struct plane_arg *plane_args = NULL;
 	struct property_arg *prop_args = NULL;
 	unsigned int args = 0;
-	
+	int ret;
+
 	opterr = 0;
 	while ((c = getopt(argc, argv, optstr)) != -1) {
 		args++;
@@ -1273,16 +1267,16 @@ int main(int argc, char **argv)
 		encoders = connectors = crtcs = planes = framebuffers = 1;
 
 	if (module) {
-		fd = drmOpen(module, NULL);
-		if (fd < 0) {
+		dev.fd = drmOpen(module, NULL);
+		if (dev.fd < 0) {
 			fprintf(stderr, "failed to open device '%s'.\n", module);
 			return 1;
 		}
 	} else {
 		for (i = 0; i < ARRAY_SIZE(modules); i++) {
 			printf("trying to open device '%s'...", modules[i]);
-			fd = drmOpen(modules[i], NULL);
-			if (fd < 0) {
+			dev.fd = drmOpen(modules[i], NULL);
+			if (dev.fd < 0) {
 				printf("failed.\n");
 			} else {
 				printf("success.\n");
@@ -1290,7 +1284,7 @@ int main(int argc, char **argv)
 			}
 		}
 
-		if (fd < 0) {
+		if (dev.fd < 0) {
 			fprintf(stderr, "no device found.\n");
 			return 1;
 		}
@@ -1301,29 +1295,40 @@ int main(int argc, char **argv)
 		return -1;
 	}
 
-	resources = get_resources(fd);
-	if (!resources) {
-		drmClose(fd);
+	dev.resources = get_resources(&dev);
+	if (!dev.resources) {
+		drmClose(dev.fd);
 		return 1;
 	}
 
-	dump_resource(encoders);
-	dump_resource(connectors);
-	dump_resource(crtcs);
-	dump_resource(planes);
-	dump_resource(framebuffers);
+#define dump_resource(dev, res) if (res) dump_##res(dev)
+
+	dump_resource(&dev, encoders);
+	dump_resource(&dev, connectors);
+	dump_resource(&dev, crtcs);
+	dump_resource(&dev, planes);
+	dump_resource(&dev, framebuffers);
 
 	for (i = 0; i < prop_count; ++i)
-		set_property(&prop_args[i]);
+		set_property(&dev, &prop_args[i]);
 
 	if (count > 0) {
-		set_mode(con_args, count, plane_args, plane_count, test_vsync);
+		ret = kms_create(dev.fd, &dev.kms);
+		if (ret) {
+			fprintf(stderr, "failed to create kms driver: %s\n",
+				strerror(-ret));
+			return 1;
+		}
+
+		set_mode(&dev, con_args, count, plane_args, plane_count, test_vsync);
 		if (drop_master)
-			drmDropMaster(fd);
+			drmDropMaster(dev.fd);
+
+		kms_destroy(&dev.kms);
 		getchar();
 	}
 
-	free_resources(resources);
+	free_resources(dev.resources);
 
 	return 0;
 }
-- 
1.8.1.5

[PATCH v6 13/23] modetest: Compute CRTC pipe number as needed

[Laurent Pinchart]

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/modetest/modetest.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index cfcb989..a1a683f 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -640,7 +640,6 @@ struct connector_arg {
 	drmModeModeInfo *mode;
 	drmModeEncoder *encoder;
 	int crtc;
-	int pipe;
 	unsigned int fb_id[2], current_fb_id;
 	struct timeval start;
 
@@ -703,15 +702,6 @@ static void connector_find_mode(struct device *dev, struct connector_arg *c)
 
 	if (c->crtc == -1)
 		c->crtc = c->encoder->crtc_id;
-
-	/* and figure out which crtc index it is: */
-	for (i = 0; i < dev->resources->res->count_crtcs; i++) {
-		if (c->crtc == (int)dev->resources->res->crtcs[i]) {
-			c->pipe = i;
-			break;
-		}
-	}
-
 }
 
 /* -----------------------------------------------------------------------------
@@ -829,15 +819,30 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p)
 	struct kms_bo *plane_bo;
 	uint32_t plane_flags = 0;
 	int crtc_x, crtc_y, crtc_w, crtc_h;
+	unsigned int pipe;
 	unsigned int i;
 
-	/* find an unused plane which can be connected to our crtc */
+	/* Find an unused plane which can be connected to our CRTC. Find the
+	 * CRTC index first, then iterate over available planes.
+	 */
+	for (i = 0; i < (unsigned int)dev->resources->res->count_crtcs; i++) {
+		if (c->crtc == (int)dev->resources->res->crtcs[i]) {
+			pipe = i;
+			break;
+		}
+	}
+
+	if (pipe == (unsigned int)dev->resources->res->count_crtcs) {
+		fprintf(stderr, "CRTC %u not found\n", c->crtc);
+		return -1;
+	}
+
 	for (i = 0; i < dev->resources->plane_res->count_planes && !plane_id; i++) {
 		ovr = dev->resources->planes[i].plane;
 		if (!ovr)
 			continue;
 
-		if ((ovr->possible_crtcs & (1 << c->pipe)) && !ovr->crtc_id)
+		if ((ovr->possible_crtcs & (1 << pipe)) && !ovr->crtc_id)
 			plane_id = ovr->plane_id;
 	}
 
-- 
1.8.1.5

[PATCH v6 14/23] modetest: Remove the struct connector_arg encoder field

[Laurent Pinchart]

The field is no needed, make it a local variable where used.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/modetest/modetest.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index a1a683f..2c3e093 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -638,7 +638,6 @@ struct connector_arg {
 	char format_str[5];
 	unsigned int fourcc;
 	drmModeModeInfo *mode;
-	drmModeEncoder *encoder;
 	int crtc;
 	unsigned int fb_id[2], current_fb_id;
 	struct timeval start;
@@ -659,6 +658,7 @@ struct plane_arg {
 static void connector_find_mode(struct device *dev, struct connector_arg *c)
 {
 	drmModeConnector *connector;
+	drmModeEncoder *encoder;
 	int i, j;
 
 	/* First, find the connector & mode */
@@ -692,16 +692,16 @@ static void connector_find_mode(struct device *dev, struct connector_arg *c)
 
 	/* Now get the encoder */
 	for (i = 0; i < dev->resources->res->count_encoders; i++) {
-		c->encoder = dev->resources->encoders[i].encoder;
-		if (!c->encoder)
+		encoder = dev->resources->encoders[i].encoder;
+		if (!encoder)
 			continue;
 
-		if (c->encoder->encoder_id  == connector->encoder_id)
+		if (encoder->encoder_id  == connector->encoder_id)
 			break;
 	}
 
 	if (c->crtc == -1)
-		c->crtc = c->encoder->crtc_id;
+		c->crtc = encoder->crtc_id;
 }
 
 /* -----------------------------------------------------------------------------
-- 
1.8.1.5

[PATCH v6 15/23] modetest: Store the crtc in the connector_arg structure

[Laurent Pinchart]

This prepares the code for the split in separate functions of CRTC and
planes setup.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/modetest/modetest.c | 58 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 21 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 2c3e093..6fbaf09 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -634,11 +634,12 @@ error:
  */
 struct connector_arg {
 	uint32_t id;
+	uint32_t crtc_id;
 	char mode_str[64];
 	char format_str[5];
 	unsigned int fourcc;
 	drmModeModeInfo *mode;
-	int crtc;
+	struct crtc *crtc;
 	unsigned int fb_id[2], current_fb_id;
 	struct timeval start;
 
@@ -690,18 +691,33 @@ static void connector_find_mode(struct device *dev, struct connector_arg *c)
 		return;
 	}
 
-	/* Now get the encoder */
-	for (i = 0; i < dev->resources->res->count_encoders; i++) {
-		encoder = dev->resources->encoders[i].encoder;
-		if (!encoder)
-			continue;
+	/* If the CRTC ID was specified, get the corresponding CRTC. Otherwise
+	 * locate a CRTC that can be attached to the connector.
+	 */
+	if (c->crtc_id == (uint32_t)-1) {
+		for (i = 0; i < dev->resources->res->count_encoders; i++) {
+			encoder = dev->resources->encoders[i].encoder;
+			if (!encoder)
+				continue;
+
+			if (encoder->encoder_id  == connector->encoder_id) {
+				c->crtc_id = encoder->crtc_id;
+				break;
+			}
+		}
+	}
+
+	if (c->crtc_id == (uint32_t)-1)
+		return;
 
-		if (encoder->encoder_id  == connector->encoder_id)
+	for (i = 0; i < dev->resources->res->count_crtcs; i++) {
+		struct crtc *crtc = &dev->resources->crtcs[i];
+
+		if (c->crtc_id == crtc->crtc->crtc_id) {
+			c->crtc = crtc;
 			break;
+		}
 	}
-
-	if (c->crtc == -1)
-		c->crtc = encoder->crtc_id;
 }
 
 /* -----------------------------------------------------------------------------
@@ -796,7 +812,7 @@ page_flip_handler(int fd, unsigned int frame,
 	else
 		new_fb_id = c->fb_id[0];
 
-	drmModePageFlip(fd, c->crtc, new_fb_id,
+	drmModePageFlip(fd, c->crtc_id, new_fb_id,
 			DRM_MODE_PAGE_FLIP_EVENT, c);
 	c->current_fb_id = new_fb_id;
 	c->swap_count++;
@@ -826,14 +842,14 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p)
 	 * CRTC index first, then iterate over available planes.
 	 */
 	for (i = 0; i < (unsigned int)dev->resources->res->count_crtcs; i++) {
-		if (c->crtc == (int)dev->resources->res->crtcs[i]) {
+		if (c->crtc_id == dev->resources->res->crtcs[i]) {
 			pipe = i;
 			break;
 		}
 	}
 
 	if (pipe == (unsigned int)dev->resources->res->count_crtcs) {
-		fprintf(stderr, "CRTC %u not found\n", c->crtc);
+		fprintf(stderr, "CRTC %u not found\n", c->crtc_id);
 		return -1;
 	}
 
@@ -847,7 +863,7 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p)
 	}
 
 	if (!plane_id) {
-		fprintf(stderr, "no unused plane available for CRTC %u\n", c->crtc);
+		fprintf(stderr, "no unused plane available for CRTC %u\n", c->crtc_id);
 		return -1;
 	}
 
@@ -878,7 +894,7 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p)
 	crtc_h = p->h;
 
 	/* note src coords (last 4 args) are in Q16 format */
-	if (drmModeSetPlane(dev->fd, plane_id, c->crtc, p->fb_id,
+	if (drmModeSetPlane(dev->fd, plane_id, c->crtc_id, p->fb_id,
 			    plane_flags, crtc_x, crtc_y, crtc_w, crtc_h,
 			    0, 0, p->w << 16, p->h << 16)) {
 		fprintf(stderr, "failed to enable plane: %s\n",
@@ -886,7 +902,7 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p)
 		return -1;
 	}
 
-	ovr->crtc_id = c->crtc;
+	ovr->crtc_id = c->crtc_id;
 
 	return 0;
 }
@@ -930,9 +946,9 @@ static void set_mode(struct device *dev, struct connector_arg *c, int count,
 			continue;
 
 		printf("setting mode %s@%s on connector %d, crtc %d\n",
-		       c[i].mode_str, c[i].format_str, c[i].id, c[i].crtc);
+		       c[i].mode_str, c[i].format_str, c[i].id, c[i].crtc_id);
 
-		ret = drmModeSetCrtc(dev->fd, c[i].crtc, fb_id, x, 0,
+		ret = drmModeSetCrtc(dev->fd, c[i].crtc_id, fb_id, x, 0,
 				     &c[i].id, 1, c[i].mode);
 
 		/* XXX: Actually check if this is needed */
@@ -971,7 +987,7 @@ static void set_mode(struct device *dev, struct connector_arg *c, int count,
 		if (c[i].mode == NULL)
 			continue;
 
-		ret = drmModePageFlip(dev->fd, c[i].crtc, other_fb_id,
+		ret = drmModePageFlip(dev->fd, c[i].crtc_id, other_fb_id,
 				      DRM_MODE_PAGE_FLIP_EVENT, &c[i]);
 		if (ret) {
 			fprintf(stderr, "failed to page flip: %s\n", strerror(errno));
@@ -1039,13 +1055,13 @@ static int parse_connector(struct connector_arg *c, const char *arg)
 	const char *p;
 	char *endp;
 
-	c->crtc = -1;
+	c->crtc_id = (uint32_t)-1;
 	strcpy(c->format_str, "XR24");
 
 	c->id = strtoul(arg, &endp, 10);
 	if (*endp == '@') {
 		arg = endp + 1;
-		c->crtc = strtoul(arg, &endp, 10);
+		c->crtc_id = strtoul(arg, &endp, 10);
 	}
 	if (*endp != ':')
 		return -1;
-- 
1.8.1.5

[PATCH v6 16/23] modetest: Store the mode in the crtc structure

[Laurent Pinchart]

This prepares the code for the split in separate functions of CRTC and
planes setup.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/modetest/modetest.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 6fbaf09..3de611e 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -62,6 +62,7 @@ struct crtc {
 	drmModeCrtc *crtc;
 	drmModeObjectProperties *props;
 	drmModePropertyRes **props_info;
+	drmModeModeInfo *mode;
 };
 
 struct encoder {
@@ -524,6 +525,7 @@ static void free_resources(struct resources *res)
 static struct resources *get_resources(struct device *dev)
 {
 	struct resources *res;
+	int i;
 
 	res = malloc(sizeof *res);
 	if (res == 0)
@@ -598,6 +600,9 @@ static struct resources *get_resources(struct device *dev)
 	get_properties(res, res, crtc, CRTC);
 	get_properties(res, res, connector, CONNECTOR);
 
+	for (i = 0; i < res->res->count_crtcs; ++i)
+		res->crtcs[i].mode = &res->crtcs[i].crtc->mode;
+
 	res->plane_res = drmModeGetPlaneResources(dev->fd);
 	if (!res->plane_res) {
 		fprintf(stderr, "drmModeGetPlaneResources failed: %s\n",
@@ -714,6 +719,7 @@ static void connector_find_mode(struct device *dev, struct connector_arg *c)
 		struct crtc *crtc = &dev->resources->crtcs[i];
 
 		if (c->crtc_id == crtc->crtc->crtc_id) {
+			crtc->mode = c->mode;
 			c->crtc = crtc;
 			break;
 		}
@@ -884,8 +890,8 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p)
 
 	if (!p->has_position) {
 		/* Default to the middle of the screen */
-		crtc_x = (c->mode->hdisplay - p->w) / 2;
-		crtc_y = (c->mode->vdisplay - p->h) / 2;
+		crtc_x = (c->crtc->mode->hdisplay - p->w) / 2;
+		crtc_y = (c->crtc->mode->vdisplay - p->h) / 2;
 	} else {
 		crtc_x = p->x;
 		crtc_y = p->y;
-- 
1.8.1.5

[PATCH v6 17/23] modetest: Give the CRTC ID to the -P option

[Laurent Pinchart]

Planes are associated with CRTCs, not connectors. Don't try to be too
clever, use the CRTC ID in the -P option. This prepares for splitting
CRTC and planes setup.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/modetest/modetest.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 3de611e..1a48cc3 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -652,7 +652,7 @@ struct connector_arg {
 };
 
 struct plane_arg {
-	uint32_t con_id;  /* the id of connector to bind to */
+	uint32_t crtc_id;  /* the id of CRTC to bind to */
 	bool has_position;
 	int32_t x, y;
 	uint32_t w, h;
@@ -832,8 +832,7 @@ page_flip_handler(int fd, unsigned int frame,
 	}
 }
 
-static int
-set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p)
+static int set_plane(struct device *dev, struct plane_arg *p)
 {
 	drmModePlane *ovr;
 	uint32_t handles[4], pitches[4], offsets[4] = {0}; /* we only use [0] */
@@ -841,6 +840,7 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p)
 	struct kms_bo *plane_bo;
 	uint32_t plane_flags = 0;
 	int crtc_x, crtc_y, crtc_w, crtc_h;
+	struct crtc *crtc;
 	unsigned int pipe;
 	unsigned int i;
 
@@ -848,14 +848,15 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p)
 	 * CRTC index first, then iterate over available planes.
 	 */
 	for (i = 0; i < (unsigned int)dev->resources->res->count_crtcs; i++) {
-		if (c->crtc_id == dev->resources->res->crtcs[i]) {
+		if (p->crtc_id == dev->resources->res->crtcs[i]) {
+			crtc = &dev->resources->crtcs[i];
 			pipe = i;
 			break;
 		}
 	}
 
-	if (pipe == (unsigned int)dev->resources->res->count_crtcs) {
-		fprintf(stderr, "CRTC %u not found\n", c->crtc_id);
+	if (!crtc) {
+		fprintf(stderr, "CRTC %u not found\n", p->crtc_id);
 		return -1;
 	}
 
@@ -869,7 +870,8 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p)
 	}
 
 	if (!plane_id) {
-		fprintf(stderr, "no unused plane available for CRTC %u\n", c->crtc_id);
+		fprintf(stderr, "no unused plane available for CRTC %u\n",
+			crtc->crtc->crtc_id);
 		return -1;
 	}
 
@@ -890,8 +892,8 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p)
 
 	if (!p->has_position) {
 		/* Default to the middle of the screen */
-		crtc_x = (c->crtc->mode->hdisplay - p->w) / 2;
-		crtc_y = (c->crtc->mode->vdisplay - p->h) / 2;
+		crtc_x = (crtc->mode->hdisplay - p->w) / 2;
+		crtc_y = (crtc->mode->vdisplay - p->h) / 2;
 	} else {
 		crtc_x = p->x;
 		crtc_y = p->y;
@@ -900,7 +902,7 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p)
 	crtc_h = p->h;
 
 	/* note src coords (last 4 args) are in Q16 format */
-	if (drmModeSetPlane(dev->fd, plane_id, c->crtc_id, p->fb_id,
+	if (drmModeSetPlane(dev->fd, plane_id, crtc->crtc->crtc_id, p->fb_id,
 			    plane_flags, crtc_x, crtc_y, crtc_w, crtc_h,
 			    0, 0, p->w << 16, p->h << 16)) {
 		fprintf(stderr, "failed to enable plane: %s\n",
@@ -908,7 +910,7 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p)
 		return -1;
 	}
 
-	ovr->crtc_id = c->crtc_id;
+	ovr->crtc_id = crtc->crtc->crtc_id;
 
 	return 0;
 }
@@ -969,8 +971,8 @@ static void set_mode(struct device *dev, struct connector_arg *c, int count,
 
 		/* if we have a plane/overlay to show, set that up now: */
 		for (j = 0; j < plane_count; j++)
-			if (p[j].con_id == c[i].id)
-				if (set_plane(dev, &c[i], &p[j]))
+			if (p[j].crtc_id == c[i].crtc_id)
+				if (set_plane(dev, &p[j]))
 					return;
 	}
 
@@ -1099,7 +1101,7 @@ static int parse_plane(struct plane_arg *plane, const char *p)
 
 	memset(plane, 0, sizeof *plane);
 
-	plane->con_id = strtoul(p, &end, 10);
+	plane->crtc_id = strtoul(p, &end, 10);
 	if (*end != ':')
 		return -EINVAL;
 
@@ -1161,7 +1163,7 @@ static void usage(char *name)
 	fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n");
 
 	fprintf(stderr, "\n Test options:\n\n");
-	fprintf(stderr, "\t-P <connector_id>:<w>x<h>[+<x>+<y>][@<format>]\tset a plane\n");
+	fprintf(stderr, "\t-P <crtc_id>:<w>x<h>[+<x>+<y>][@<format>]\tset a plane\n");
 	fprintf(stderr, "\t-s <connector_id>[@<crtc_id>]:<mode>[@<format>]\tset a mode\n");
 	fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
 	fprintf(stderr, "\t-w <obj_id>:<prop_name>:<value>\tset property\n");
-- 
1.8.1.5

[PATCH v6 18/23] modetest: Split mode setting and plane setup

[Laurent Pinchart]

There's not reason to require setting a mode to test planes. Split the
two operations.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/modetest/modetest.c | 104 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 72 insertions(+), 32 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 1a48cc3..93a0864 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -101,6 +101,14 @@ struct device {
 
 	struct resources *resources;
 	struct kms_driver *kms;
+
+	struct {
+		unsigned int width;
+		unsigned int height;
+
+		unsigned int fb_id;
+		struct kms_bo *bo;
+	} mode;
 };
 
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
@@ -840,7 +848,7 @@ static int set_plane(struct device *dev, struct plane_arg *p)
 	struct kms_bo *plane_bo;
 	uint32_t plane_flags = 0;
 	int crtc_x, crtc_y, crtc_w, crtc_h;
-	struct crtc *crtc;
+	struct crtc *crtc = NULL;
 	unsigned int pipe;
 	unsigned int i;
 
@@ -915,36 +923,37 @@ static int set_plane(struct device *dev, struct plane_arg *p)
 	return 0;
 }
 
-static void set_mode(struct device *dev, struct connector_arg *c, int count,
-		     struct plane_arg *p, int plane_count, int page_flip)
+static void set_mode(struct device *dev, struct connector_arg *c, unsigned int count)
 {
-	struct kms_bo *bo, *other_bo;
-	unsigned int fb_id, other_fb_id;
-	int i, j, ret, width, height, x;
 	uint32_t handles[4], pitches[4], offsets[4] = {0}; /* we only use [0] */
-	drmEventContext evctx;
+	unsigned int fb_id;
+	struct kms_bo *bo;
+	unsigned int i;
+	int ret, x;
+
+	dev->mode.width = 0;
+	dev->mode.height = 0;
 
-	width = 0;
-	height = 0;
 	for (i = 0; i < count; i++) {
 		connector_find_mode(dev, &c[i]);
 		if (c[i].mode == NULL)
 			continue;
-		width += c[i].mode->hdisplay;
-		if (height < c[i].mode->vdisplay)
-			height = c[i].mode->vdisplay;
+		dev->mode.width += c[i].mode->hdisplay;
+		if (dev->mode.height < c[i].mode->vdisplay)
+			dev->mode.height = c[i].mode->vdisplay;
 	}
 
-	bo = create_test_buffer(dev->kms, c->fourcc, width, height, handles,
-				pitches, offsets, PATTERN_SMPTE);
+	bo = create_test_buffer(dev->kms, c->fourcc,
+				dev->mode.width, dev->mode.height,
+				handles, pitches, offsets, PATTERN_SMPTE);
 	if (bo == NULL)
 		return;
 
-	ret = drmModeAddFB2(dev->fd, width, height, c->fourcc,
+	ret = drmModeAddFB2(dev->fd, dev->mode.width, dev->mode.height, c->fourcc,
 			    handles, pitches, offsets, &fb_id, 0);
 	if (ret) {
 		fprintf(stderr, "failed to add fb (%ux%u): %s\n",
-			width, height, strerror(errno));
+			dev->mode.width, dev->mode.height, strerror(errno));
 		return;
 	}
 
@@ -968,24 +977,39 @@ static void set_mode(struct device *dev, struct connector_arg *c, int count,
 			fprintf(stderr, "failed to set mode: %s\n", strerror(errno));
 			return;
 		}
-
-		/* if we have a plane/overlay to show, set that up now: */
-		for (j = 0; j < plane_count; j++)
-			if (p[j].crtc_id == c[i].crtc_id)
-				if (set_plane(dev, &p[j]))
-					return;
 	}
 
-	if (!page_flip)
-		return;
+	dev->mode.bo = bo;
+	dev->mode.fb_id = fb_id;
+}
+
+static void set_planes(struct device *dev, struct plane_arg *p, unsigned int count)
+{
+	unsigned int i;
+
+	/* set up planes/overlays */
+	for (i = 0; i < count; i++)
+		if (set_plane(dev, &p[i]))
+			return;
+}
+
+static void test_page_flip(struct device *dev, struct connector_arg *c, unsigned int count)
+{
+	uint32_t handles[4], pitches[4], offsets[4] = {0}; /* we only use [0] */
+	unsigned int other_fb_id;
+	struct kms_bo *other_bo;
+	drmEventContext evctx;
+	unsigned int i;
+	int ret;
 
-	other_bo = create_test_buffer(dev->kms, c->fourcc, width, height, handles,
-				      pitches, offsets, PATTERN_PLAIN);
+	other_bo = create_test_buffer(dev->kms, c->fourcc,
+				      dev->mode.width, dev->mode.height,
+				      handles, pitches, offsets, PATTERN_PLAIN);
 	if (other_bo == NULL)
 		return;
 
-	ret = drmModeAddFB2(dev->fd, width, height, c->fourcc, handles, pitches, offsets,
-			    &other_fb_id, 0);
+	ret = drmModeAddFB2(dev->fd, dev->mode.width, dev->mode.height, c->fourcc,
+			    handles, pitches, offsets, &other_fb_id, 0);
 	if (ret) {
 		fprintf(stderr, "failed to add fb: %s\n", strerror(errno));
 		return;
@@ -1003,7 +1027,7 @@ static void set_mode(struct device *dev, struct connector_arg *c, int count,
 		}
 		gettimeofday(&c[i].start, NULL);
 		c[i].swap_count = 0;
-		c[i].fb_id[0] = fb_id;
+		c[i].fb_id[0] = dev->mode.fb_id;
 		c[i].fb_id[1] = other_fb_id;
 		c[i].current_fb_id = other_fb_id;
 	}
@@ -1051,7 +1075,6 @@ static void set_mode(struct device *dev, struct connector_arg *c, int count,
 		drmHandleEvent(dev->fd, &evctx);
 	}
 
-	kms_bo_destroy(&bo);
 	kms_bo_destroy(&other_bo);
 }
 
@@ -1218,6 +1241,8 @@ int main(int argc, char **argv)
 	unsigned int args = 0;
 	int ret;
 
+	memset(&dev, 0, sizeof dev);
+
 	opterr = 0;
 	while ((c = getopt(argc, argv, optstr)) != -1) {
 		args++;
@@ -1324,6 +1349,11 @@ int main(int argc, char **argv)
 		return -1;
 	}
 
+	if (test_vsync && !count) {
+		fprintf(stderr, "page flipping requires at least one -s option.\n");
+		return -1;
+	}
+
 	dev.resources = get_resources(&dev);
 	if (!dev.resources) {
 		drmClose(dev.fd);
@@ -1341,7 +1371,7 @@ int main(int argc, char **argv)
 	for (i = 0; i < prop_count; ++i)
 		set_property(&dev, &prop_args[i]);
 
-	if (count > 0) {
+	if (count || plane_count) {
 		ret = kms_create(dev.fd, &dev.kms);
 		if (ret) {
 			fprintf(stderr, "failed to create kms driver: %s\n",
@@ -1349,11 +1379,21 @@ int main(int argc, char **argv)
 			return 1;
 		}
 
-		set_mode(&dev, con_args, count, plane_args, plane_count, test_vsync);
+		if (count)
+			set_mode(&dev, con_args, count);
+
+		if (plane_count)
+			set_planes(&dev, plane_args, plane_count);
+
+		if (test_vsync)
+			test_page_flip(&dev, con_args, count);
+
 		if (drop_master)
 			drmDropMaster(dev.fd);
 
+		kms_bo_destroy(&dev.mode.bo);
 		kms_destroy(&dev.kms);
+
 		getchar();
 	}
 
-- 
1.8.1.5

[PATCH v6 19/23] modetest: Rename struct connector_arg to struct pipe_arg

[Laurent Pinchart]

This prepares the code for handling multiple connectors in a single
pipeline in a cloned configuration.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/modetest/modetest.c | 162 ++++++++++++++++++++++++----------------------
 1 file changed, 85 insertions(+), 77 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 93a0864..922d9b0 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -635,7 +635,7 @@ error:
 }
 
 /* -----------------------------------------------------------------------------
- * Connectors and planes
+ * Pipes and planes
  */
 
 /*
@@ -645,8 +645,8 @@ error:
  * Then you need to find the encoder attached to that connector so you
  * can bind it with a free crtc.
  */
-struct connector_arg {
-	uint32_t id;
+struct pipe_arg {
+	uint32_t con_id;
 	uint32_t crtc_id;
 	char mode_str[64];
 	char format_str[5];
@@ -669,14 +669,14 @@ struct plane_arg {
 	unsigned int fourcc;
 };
 
-static void connector_find_mode(struct device *dev, struct connector_arg *c)
+static void pipe_find_mode(struct device *dev, struct pipe_arg *pipe)
 {
 	drmModeConnector *connector;
 	drmModeEncoder *encoder;
 	int i, j;
 
 	/* First, find the connector & mode */
-	c->mode = NULL;
+	pipe->mode = NULL;
 	for (i = 0; i < dev->resources->res->count_connectors; i++) {
 		connector = dev->resources->connectors[i].connector;
 		if (!connector)
@@ -685,50 +685,50 @@ static void connector_find_mode(struct device *dev, struct connector_arg *c)
 		if (!connector->count_modes)
 			continue;
 
-		if (connector->connector_id != c->id)
+		if (connector->connector_id != pipe->con_id)
 			continue;
 
 		for (j = 0; j < connector->count_modes; j++) {
-			c->mode = &connector->modes[j];
-			if (!strcmp(c->mode->name, c->mode_str))
+			pipe->mode = &connector->modes[j];
+			if (!strcmp(pipe->mode->name, pipe->mode_str))
 				break;
 		}
 
 		/* Found it, break out */
-		if (c->mode)
+		if (pipe->mode)
 			break;
 	}
 
-	if (!c->mode) {
-		fprintf(stderr, "failed to find mode \"%s\"\n", c->mode_str);
+	if (!pipe->mode) {
+		fprintf(stderr, "failed to find mode \"%s\"\n", pipe->mode_str);
 		return;
 	}
 
 	/* If the CRTC ID was specified, get the corresponding CRTC. Otherwise
 	 * locate a CRTC that can be attached to the connector.
 	 */
-	if (c->crtc_id == (uint32_t)-1) {
+	if (pipe->crtc_id == (uint32_t)-1) {
 		for (i = 0; i < dev->resources->res->count_encoders; i++) {
 			encoder = dev->resources->encoders[i].encoder;
 			if (!encoder)
 				continue;
 
 			if (encoder->encoder_id  == connector->encoder_id) {
-				c->crtc_id = encoder->crtc_id;
+				pipe->crtc_id = encoder->crtc_id;
 				break;
 			}
 		}
 	}
 
-	if (c->crtc_id == (uint32_t)-1)
+	if (pipe->crtc_id == (uint32_t)-1)
 		return;
 
 	for (i = 0; i < dev->resources->res->count_crtcs; i++) {
 		struct crtc *crtc = &dev->resources->crtcs[i];
 
-		if (c->crtc_id == crtc->crtc->crtc_id) {
-			crtc->mode = c->mode;
-			c->crtc = crtc;
+		if (pipe->crtc_id == crtc->crtc->crtc_id) {
+			crtc->mode = pipe->mode;
+			pipe->crtc = crtc;
 			break;
 		}
 	}
@@ -815,28 +815,28 @@ static void
 page_flip_handler(int fd, unsigned int frame,
 		  unsigned int sec, unsigned int usec, void *data)
 {
-	struct connector_arg *c;
+	struct pipe_arg *pipe;
 	unsigned int new_fb_id;
 	struct timeval end;
 	double t;
 
-	c = data;
-	if (c->current_fb_id == c->fb_id[0])
-		new_fb_id = c->fb_id[1];
+	pipe = data;
+	if (pipe->current_fb_id == pipe->fb_id[0])
+		new_fb_id = pipe->fb_id[1];
 	else
-		new_fb_id = c->fb_id[0];
+		new_fb_id = pipe->fb_id[0];
 
-	drmModePageFlip(fd, c->crtc_id, new_fb_id,
-			DRM_MODE_PAGE_FLIP_EVENT, c);
-	c->current_fb_id = new_fb_id;
-	c->swap_count++;
-	if (c->swap_count == 60) {
+	drmModePageFlip(fd, pipe->crtc_id, new_fb_id,
+			DRM_MODE_PAGE_FLIP_EVENT, pipe);
+	pipe->current_fb_id = new_fb_id;
+	pipe->swap_count++;
+	if (pipe->swap_count == 60) {
 		gettimeofday(&end, NULL);
 		t = end.tv_sec + end.tv_usec * 1e-6 -
-			(c->start.tv_sec + c->start.tv_usec * 1e-6);
-		fprintf(stderr, "freq: %.02fHz\n", c->swap_count / t);
-		c->swap_count = 0;
-		c->start = end;
+			(pipe->start.tv_sec + pipe->start.tv_usec * 1e-6);
+		fprintf(stderr, "freq: %.02fHz\n", pipe->swap_count / t);
+		pipe->swap_count = 0;
+		pipe->start = end;
 	}
 }
 
@@ -923,7 +923,7 @@ static int set_plane(struct device *dev, struct plane_arg *p)
 	return 0;
 }
 
-static void set_mode(struct device *dev, struct connector_arg *c, unsigned int count)
+static void set_mode(struct device *dev, struct pipe_arg *pipes, unsigned int count)
 {
 	uint32_t handles[4], pitches[4], offsets[4] = {0}; /* we only use [0] */
 	unsigned int fb_id;
@@ -935,22 +935,24 @@ static void set_mode(struct device *dev, struct connector_arg *c, unsigned int c
 	dev->mode.height = 0;
 
 	for (i = 0; i < count; i++) {
-		connector_find_mode(dev, &c[i]);
-		if (c[i].mode == NULL)
+		struct pipe_arg *pipe = &pipes[i];
+
+		pipe_find_mode(dev, pipe);
+		if (pipe->mode == NULL)
 			continue;
-		dev->mode.width += c[i].mode->hdisplay;
-		if (dev->mode.height < c[i].mode->vdisplay)
-			dev->mode.height = c[i].mode->vdisplay;
+		dev->mode.width += pipe->mode->hdisplay;
+		if (dev->mode.height < pipe->mode->vdisplay)
+			dev->mode.height = pipe->mode->vdisplay;
 	}
 
-	bo = create_test_buffer(dev->kms, c->fourcc,
+	bo = create_test_buffer(dev->kms, pipes[0].fourcc,
 				dev->mode.width, dev->mode.height,
 				handles, pitches, offsets, PATTERN_SMPTE);
 	if (bo == NULL)
 		return;
 
-	ret = drmModeAddFB2(dev->fd, dev->mode.width, dev->mode.height, c->fourcc,
-			    handles, pitches, offsets, &fb_id, 0);
+	ret = drmModeAddFB2(dev->fd, dev->mode.width, dev->mode.height,
+			    pipes[0].fourcc, handles, pitches, offsets, &fb_id, 0);
 	if (ret) {
 		fprintf(stderr, "failed to add fb (%ux%u): %s\n",
 			dev->mode.width, dev->mode.height, strerror(errno));
@@ -959,19 +961,22 @@ static void set_mode(struct device *dev, struct connector_arg *c, unsigned int c
 
 	x = 0;
 	for (i = 0; i < count; i++) {
-		if (c[i].mode == NULL)
+		struct pipe_arg *pipe = &pipes[i];
+
+		if (pipe->mode == NULL)
 			continue;
 
 		printf("setting mode %s@%s on connector %d, crtc %d\n",
-		       c[i].mode_str, c[i].format_str, c[i].id, c[i].crtc_id);
+		       pipe->mode_str, pipe->format_str, pipe->con_id,
+		       pipe->crtc_id);
 
-		ret = drmModeSetCrtc(dev->fd, c[i].crtc_id, fb_id, x, 0,
-				     &c[i].id, 1, c[i].mode);
+		ret = drmModeSetCrtc(dev->fd, pipe->crtc_id, fb_id, x, 0,
+				     &pipe->con_id, 1, pipe->mode);
 
 		/* XXX: Actually check if this is needed */
 		drmModeDirtyFB(dev->fd, fb_id, NULL, 0);
 
-		x += c[i].mode->hdisplay;
+		x += pipe->mode->hdisplay;
 
 		if (ret) {
 			fprintf(stderr, "failed to set mode: %s\n", strerror(errno));
@@ -993,7 +998,7 @@ static void set_planes(struct device *dev, struct plane_arg *p, unsigned int cou
 			return;
 }
 
-static void test_page_flip(struct device *dev, struct connector_arg *c, unsigned int count)
+static void test_page_flip(struct device *dev, struct pipe_arg *pipes, unsigned int count)
 {
 	uint32_t handles[4], pitches[4], offsets[4] = {0}; /* we only use [0] */
 	unsigned int other_fb_id;
@@ -1002,34 +1007,37 @@ static void test_page_flip(struct device *dev, struct connector_arg *c, unsigned
 	unsigned int i;
 	int ret;
 
-	other_bo = create_test_buffer(dev->kms, c->fourcc,
+	other_bo = create_test_buffer(dev->kms, pipes[0].fourcc,
 				      dev->mode.width, dev->mode.height,
 				      handles, pitches, offsets, PATTERN_PLAIN);
 	if (other_bo == NULL)
 		return;
 
-	ret = drmModeAddFB2(dev->fd, dev->mode.width, dev->mode.height, c->fourcc,
-			    handles, pitches, offsets, &other_fb_id, 0);
+	ret = drmModeAddFB2(dev->fd, dev->mode.width, dev->mode.height,
+			    pipes[0].fourcc, handles, pitches, offsets,
+			    &other_fb_id, 0);
 	if (ret) {
 		fprintf(stderr, "failed to add fb: %s\n", strerror(errno));
 		return;
 	}
 
 	for (i = 0; i < count; i++) {
-		if (c[i].mode == NULL)
+		struct pipe_arg *pipe = &pipes[i];
+
+		if (pipe->mode == NULL)
 			continue;
 
-		ret = drmModePageFlip(dev->fd, c[i].crtc_id, other_fb_id,
-				      DRM_MODE_PAGE_FLIP_EVENT, &c[i]);
+		ret = drmModePageFlip(dev->fd, pipe->crtc_id, other_fb_id,
+				      DRM_MODE_PAGE_FLIP_EVENT, pipe);
 		if (ret) {
 			fprintf(stderr, "failed to page flip: %s\n", strerror(errno));
 			return;
 		}
-		gettimeofday(&c[i].start, NULL);
-		c[i].swap_count = 0;
-		c[i].fb_id[0] = dev->mode.fb_id;
-		c[i].fb_id[1] = other_fb_id;
-		c[i].current_fb_id = other_fb_id;
+		gettimeofday(&pipe->start, NULL);
+		pipe->swap_count = 0;
+		pipe->fb_id[0] = dev->mode.fb_id;
+		pipe->fb_id[1] = other_fb_id;
+		pipe->current_fb_id = other_fb_id;
 	}
 
 	memset(&evctx, 0, sizeof evctx);
@@ -1080,19 +1088,19 @@ static void test_page_flip(struct device *dev, struct connector_arg *c, unsigned
 
 #define min(a, b)	((a) < (b) ? (a) : (b))
 
-static int parse_connector(struct connector_arg *c, const char *arg)
+static int parse_connector(struct pipe_arg *pipe, const char *arg)
 {
 	unsigned int len;
 	const char *p;
 	char *endp;
 
-	c->crtc_id = (uint32_t)-1;
-	strcpy(c->format_str, "XR24");
+	pipe->crtc_id = (uint32_t)-1;
+	strcpy(pipe->format_str, "XR24");
 
-	c->id = strtoul(arg, &endp, 10);
+	pipe->con_id = strtoul(arg, &endp, 10);
 	if (*endp == '@') {
 		arg = endp + 1;
-		c->crtc_id = strtoul(arg, &endp, 10);
+		pipe->crtc_id = strtoul(arg, &endp, 10);
 	}
 	if (*endp != ':')
 		return -1;
@@ -1100,18 +1108,18 @@ static int parse_connector(struct connector_arg *c, const char *arg)
 	arg = endp + 1;
 
 	p = strchrnul(arg, '@');
-	len = min(sizeof c->mode_str - 1, (unsigned int)(p - arg));
-	strncpy(c->mode_str, arg, len);
-	c->mode_str[len] = '\0';
+	len = min(sizeof pipe->mode_str - 1, (unsigned int)(p - arg));
+	strncpy(pipe->mode_str, arg, len);
+	pipe->mode_str[len] = '\0';
 
 	if (*p == '@') {
-		strncpy(c->format_str, p + 1, 4);
-		c->format_str[4] = '\0';
+		strncpy(pipe->format_str, p + 1, 4);
+		pipe->format_str[4] = '\0';
 	}
 
-	c->fourcc = format_fourcc(c->format_str);
-	if (c->fourcc == 0)  {
-		fprintf(stderr, "unknown format %s\n", c->format_str);
+	pipe->fourcc = format_fourcc(pipe->format_str);
+	if (pipe->fourcc == 0)  {
+		fprintf(stderr, "unknown format %s\n", pipe->format_str);
 		return -1;
 	}
 
@@ -1235,7 +1243,7 @@ int main(int argc, char **argv)
 	unsigned int i;
 	int count = 0, plane_count = 0;
 	unsigned int prop_count = 0;
-	struct connector_arg *con_args = NULL;
+	struct pipe_arg *pipe_args = NULL;
 	struct plane_arg *plane_args = NULL;
 	struct property_arg *prop_args = NULL;
 	unsigned int args = 0;
@@ -1283,14 +1291,14 @@ int main(int argc, char **argv)
 			planes = 1;
 			break;
 		case 's':
-			con_args = realloc(con_args,
-					   (count + 1) * sizeof *con_args);
-			if (con_args == NULL) {
+			pipe_args = realloc(pipe_args,
+					    (count + 1) * sizeof *pipe_args);
+			if (pipe_args == NULL) {
 				fprintf(stderr, "memory allocation failed\n");
 				return 1;
 			}
 
-			if (parse_connector(&con_args[count], optarg) < 0)
+			if (parse_connector(&pipe_args[count], optarg) < 0)
 				usage(argv[0]);
 
 			count++;				      
@@ -1380,13 +1388,13 @@ int main(int argc, char **argv)
 		}
 
 		if (count)
-			set_mode(&dev, con_args, count);
+			set_mode(&dev, pipe_args, count);
 
 		if (plane_count)
 			set_planes(&dev, plane_args, plane_count);
 
 		if (test_vsync)
-			test_page_flip(&dev, con_args, count);
+			test_page_flip(&dev, pipe_args, count);
 
 		if (drop_master)
 			drmDropMaster(dev.fd);
-- 
1.8.1.5

[PATCH v6 20/23] modetest: Support pipes with multiple connectors

[Laurent Pinchart]

The -s argument can now take a list of connectors. Configure all of them
in cloned mode using a single CRTC.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/modetest/modetest.c | 211 ++++++++++++++++++++++++++++++++++------------
 1 file changed, 157 insertions(+), 54 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 922d9b0..450c86d 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -40,6 +40,7 @@
 #include "config.h"
 
 #include <assert.h>
+#include <ctype.h>
 #include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -634,6 +635,34 @@ error:
 	return NULL;
 }
 
+static drmModeConnector *get_connector_by_id(struct device *dev, uint32_t id)
+{
+	drmModeConnector *connector;
+	int i;
+
+	for (i = 0; i < dev->resources->res->count_connectors; i++) {
+		connector = dev->resources->connectors[i].connector;
+		if (connector && connector->connector_id == id)
+			return connector;
+	}
+
+	return NULL;
+}
+
+static drmModeEncoder *get_encoder_by_id(struct device *dev, uint32_t id)
+{
+	drmModeEncoder *encoder;
+	int i;
+
+	for (i = 0; i < dev->resources->res->count_encoders; i++) {
+		encoder = dev->resources->encoders[i].encoder;
+		if (encoder && encoder->encoder_id == id)
+			return encoder;
+	}
+
+	return NULL;
+}
+
 /* -----------------------------------------------------------------------------
  * Pipes and planes
  */
@@ -646,7 +675,8 @@ error:
  * can bind it with a free crtc.
  */
 struct pipe_arg {
-	uint32_t con_id;
+	uint32_t *con_ids;
+	unsigned int num_cons;
 	uint32_t crtc_id;
 	char mode_str[64];
 	char format_str[5];
@@ -669,69 +699,114 @@ struct plane_arg {
 	unsigned int fourcc;
 };
 
-static void pipe_find_mode(struct device *dev, struct pipe_arg *pipe)
+static drmModeModeInfo *
+connector_find_mode(struct device *dev, uint32_t con_id, const char *mode_str)
 {
 	drmModeConnector *connector;
-	drmModeEncoder *encoder;
-	int i, j;
+	drmModeModeInfo *mode;
+	int i;
 
-	/* First, find the connector & mode */
-	pipe->mode = NULL;
-	for (i = 0; i < dev->resources->res->count_connectors; i++) {
-		connector = dev->resources->connectors[i].connector;
+	connector = get_connector_by_id(dev, con_id);
+	if (!connector || !connector->count_modes)
+		return NULL;
+
+	for (i = 0; i < connector->count_modes; i++) {
+		mode = &connector->modes[i];
+		if (!strcmp(mode->name, mode_str))
+			return mode;
+	}
+
+	return NULL;
+}
+
+static struct crtc *pipe_find_crtc(struct device *dev, struct pipe_arg *pipe)
+{
+	uint32_t possible_crtcs = ~0;
+	uint32_t active_crtcs = 0;
+	unsigned int crtc_idx;
+	unsigned int i;
+	int j;
+
+	for (i = 0; i < pipe->num_cons; ++i) {
+		drmModeConnector *connector;
+		drmModeEncoder *encoder;
+
+		connector = get_connector_by_id(dev, pipe->con_ids[i]);
 		if (!connector)
-			continue;
+			return NULL;
 
-		if (!connector->count_modes)
-			continue;
+		encoder = get_encoder_by_id(dev, connector->encoder_id);
+		if (!encoder)
+			return NULL;
 
-		if (connector->connector_id != pipe->con_id)
-			continue;
+		possible_crtcs &= encoder->possible_crtcs;
 
-		for (j = 0; j < connector->count_modes; j++) {
-			pipe->mode = &connector->modes[j];
-			if (!strcmp(pipe->mode->name, pipe->mode_str))
+		for (j = 0; j < dev->resources->res->count_crtcs; ++j) {
+			drmModeCrtc *crtc = dev->resources->crtcs[j].crtc;
+			if (crtc && crtc->crtc_id == encoder->crtc_id) {
+				active_crtcs |= 1 << j;
 				break;
+			}
 		}
-
-		/* Found it, break out */
-		if (pipe->mode)
-			break;
 	}
 
-	if (!pipe->mode) {
-		fprintf(stderr, "failed to find mode \"%s\"\n", pipe->mode_str);
-		return;
+	if (!possible_crtcs)
+		return NULL;
+
+	/* Return the first possible and active CRTC if one exists, or the first
+	 * possible CRTC otherwise.
+	 */
+	if (possible_crtcs & active_crtcs)
+		crtc_idx = ffs(possible_crtcs & active_crtcs);
+	else
+		crtc_idx = ffs(possible_crtcs);
+
+	return &dev->resources->crtcs[crtc_idx - 1];
+}
+
+static int pipe_find_crtc_and_mode(struct device *dev, struct pipe_arg *pipe)
+{
+	drmModeModeInfo *mode;
+	int i;
+
+	pipe->mode = NULL;
+
+	for (i = 0; i < (int)pipe->num_cons; i++) {
+		mode = connector_find_mode(dev, pipe->con_ids[i],
+					   pipe->mode_str);
+		if (mode == NULL) {
+			fprintf(stderr,
+				"failed to find mode \"%s\" for connector %u\n",
+				pipe->mode_str, pipe->con_ids[i]);
+			return -EINVAL;
+		}
 	}
 
 	/* If the CRTC ID was specified, get the corresponding CRTC. Otherwise
-	 * locate a CRTC that can be attached to the connector.
+	 * locate a CRTC that can be attached to all the connectors.
 	 */
-	if (pipe->crtc_id == (uint32_t)-1) {
-		for (i = 0; i < dev->resources->res->count_encoders; i++) {
-			encoder = dev->resources->encoders[i].encoder;
-			if (!encoder)
-				continue;
-
-			if (encoder->encoder_id  == connector->encoder_id) {
-				pipe->crtc_id = encoder->crtc_id;
+	if (pipe->crtc_id != (uint32_t)-1) {
+		for (i = 0; i < dev->resources->res->count_crtcs; i++) {
+			struct crtc *crtc = &dev->resources->crtcs[i];
+
+			if (pipe->crtc_id == crtc->crtc->crtc_id) {
+				pipe->crtc = crtc;
 				break;
 			}
 		}
+	} else {
+		pipe->crtc = pipe_find_crtc(dev, pipe);
 	}
 
-	if (pipe->crtc_id == (uint32_t)-1)
-		return;
+	if (!pipe->crtc) {
+		fprintf(stderr, "failed to find CRTC for pipe\n");
+		return -EINVAL;
+	}
 
-	for (i = 0; i < dev->resources->res->count_crtcs; i++) {
-		struct crtc *crtc = &dev->resources->crtcs[i];
+	pipe->mode = mode;
+	pipe->crtc->mode = mode;
 
-		if (pipe->crtc_id == crtc->crtc->crtc_id) {
-			crtc->mode = pipe->mode;
-			pipe->crtc = crtc;
-			break;
-		}
-	}
+	return 0;
 }
 
 /* -----------------------------------------------------------------------------
@@ -826,7 +901,7 @@ page_flip_handler(int fd, unsigned int frame,
 	else
 		new_fb_id = pipe->fb_id[0];
 
-	drmModePageFlip(fd, pipe->crtc_id, new_fb_id,
+	drmModePageFlip(fd, pipe->crtc->crtc->crtc_id, new_fb_id,
 			DRM_MODE_PAGE_FLIP_EVENT, pipe);
 	pipe->current_fb_id = new_fb_id;
 	pipe->swap_count++;
@@ -929,6 +1004,7 @@ static void set_mode(struct device *dev, struct pipe_arg *pipes, unsigned int co
 	unsigned int fb_id;
 	struct kms_bo *bo;
 	unsigned int i;
+	unsigned int j;
 	int ret, x;
 
 	dev->mode.width = 0;
@@ -937,9 +1013,10 @@ static void set_mode(struct device *dev, struct pipe_arg *pipes, unsigned int co
 	for (i = 0; i < count; i++) {
 		struct pipe_arg *pipe = &pipes[i];
 
-		pipe_find_mode(dev, pipe);
-		if (pipe->mode == NULL)
+		ret = pipe_find_crtc_and_mode(dev, pipe);
+		if (ret < 0)
 			continue;
+
 		dev->mode.width += pipe->mode->hdisplay;
 		if (dev->mode.height < pipe->mode->vdisplay)
 			dev->mode.height = pipe->mode->vdisplay;
@@ -966,12 +1043,15 @@ static void set_mode(struct device *dev, struct pipe_arg *pipes, unsigned int co
 		if (pipe->mode == NULL)
 			continue;
 
-		printf("setting mode %s@%s on connector %d, crtc %d\n",
-		       pipe->mode_str, pipe->format_str, pipe->con_id,
-		       pipe->crtc_id);
+		printf("setting mode %s@%s on connectors ",
+		       pipe->mode_str, pipe->format_str);
+		for (j = 0; j < pipe->num_cons; ++j)
+			printf("%u, ", pipe->con_ids[j]);
+		printf("crtc %d\n", pipe->crtc->crtc->crtc_id);
 
-		ret = drmModeSetCrtc(dev->fd, pipe->crtc_id, fb_id, x, 0,
-				     &pipe->con_id, 1, pipe->mode);
+		ret = drmModeSetCrtc(dev->fd, pipe->crtc->crtc->crtc_id, fb_id,
+				     x, 0, pipe->con_ids, pipe->num_cons,
+				     pipe->mode);
 
 		/* XXX: Actually check if this is needed */
 		drmModeDirtyFB(dev->fd, fb_id, NULL, 0);
@@ -1027,8 +1107,9 @@ static void test_page_flip(struct device *dev, struct pipe_arg *pipes, unsigned
 		if (pipe->mode == NULL)
 			continue;
 
-		ret = drmModePageFlip(dev->fd, pipe->crtc_id, other_fb_id,
-				      DRM_MODE_PAGE_FLIP_EVENT, pipe);
+		ret = drmModePageFlip(dev->fd, pipe->crtc->crtc->crtc_id,
+				      other_fb_id, DRM_MODE_PAGE_FLIP_EVENT,
+				      pipe);
 		if (ret) {
 			fprintf(stderr, "failed to page flip: %s\n", strerror(errno));
 			return;
@@ -1091,13 +1172,35 @@ static void test_page_flip(struct device *dev, struct pipe_arg *pipes, unsigned
 static int parse_connector(struct pipe_arg *pipe, const char *arg)
 {
 	unsigned int len;
+	unsigned int i;
 	const char *p;
 	char *endp;
 
 	pipe->crtc_id = (uint32_t)-1;
 	strcpy(pipe->format_str, "XR24");
 
-	pipe->con_id = strtoul(arg, &endp, 10);
+	/* Count the number of connectors and allocate them. */
+	pipe->num_cons = 1;
+	for (p = arg; isdigit(*p) || *p == ','; ++p) {
+		if (*p == ',')
+			pipe->num_cons++;
+	}
+
+	pipe->con_ids = malloc(pipe->num_cons * sizeof *pipe->con_ids);
+	if (pipe->con_ids == NULL)
+		return -1;
+
+	/* Parse the connectors. */
+	for (i = 0, p = arg; i < pipe->num_cons; ++i, p = endp + 1) {
+		pipe->con_ids[i] = strtoul(p, &endp, 10);
+		if (*endp != ',')
+			break;
+	}
+
+	if (i != pipe->num_cons - 1)
+		return -1;
+
+	/* Parse the remaining parameters. */
 	if (*endp == '@') {
 		arg = endp + 1;
 		pipe->crtc_id = strtoul(arg, &endp, 10);
@@ -1195,7 +1298,7 @@ static void usage(char *name)
 
 	fprintf(stderr, "\n Test options:\n\n");
 	fprintf(stderr, "\t-P <crtc_id>:<w>x<h>[+<x>+<y>][@<format>]\tset a plane\n");
-	fprintf(stderr, "\t-s <connector_id>[@<crtc_id>]:<mode>[@<format>]\tset a mode\n");
+	fprintf(stderr, "\t-s <connector_id>[,<connector_id>][@<crtc_id>]:<mode>[@<format>]\tset a mode\n");
 	fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
 	fprintf(stderr, "\t-w <obj_id>:<prop_name>:<value>\tset property\n");
 
-- 
1.8.1.5

[PATCH v6 21/23] modetest: Try all possible encoders for a connector

[Laurent Pinchart]

When building the pipeline, instead of using only the encoders attached
to a connector, take all possible encoders into account to locate a
CRTC.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/modetest/modetest.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 450c86d..e49838b 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -635,6 +635,19 @@ error:
 	return NULL;
 }
 
+static int get_crtc_index(struct device *dev, uint32_t id)
+{
+	int i;
+
+	for (i = 0; i < dev->resources->res->count_crtcs; ++i) {
+		drmModeCrtc *crtc = dev->resources->crtcs[i].crtc;
+		if (crtc && crtc->crtc_id == id)
+			return i;
+	}
+
+	return -1;
+}
+
 static drmModeConnector *get_connector_by_id(struct device *dev, uint32_t id)
 {
 	drmModeConnector *connector;
@@ -728,26 +741,28 @@ static struct crtc *pipe_find_crtc(struct device *dev, struct pipe_arg *pipe)
 	int j;
 
 	for (i = 0; i < pipe->num_cons; ++i) {
+		uint32_t crtcs_for_connector = 0;
 		drmModeConnector *connector;
 		drmModeEncoder *encoder;
+		int idx;
 
 		connector = get_connector_by_id(dev, pipe->con_ids[i]);
 		if (!connector)
 			return NULL;
 
-		encoder = get_encoder_by_id(dev, connector->encoder_id);
-		if (!encoder)
-			return NULL;
+		for (j = 0; j < connector->count_encoders; ++j) {
+			encoder = get_encoder_by_id(dev, connector->encoders[j]);
+			if (!encoder)
+				continue;
 
-		possible_crtcs &= encoder->possible_crtcs;
+			crtcs_for_connector |= encoder->possible_crtcs;
 
-		for (j = 0; j < dev->resources->res->count_crtcs; ++j) {
-			drmModeCrtc *crtc = dev->resources->crtcs[j].crtc;
-			if (crtc && crtc->crtc_id == encoder->crtc_id) {
-				active_crtcs |= 1 << j;
-				break;
-			}
+			idx = get_crtc_index(dev, encoder->crtc_id);
+			if (idx >= 0)
+				active_crtcs |= 1 << idx;
 		}
+
+		possible_crtcs &= crtcs_for_connector;
 	}
 
 	if (!possible_crtcs)
-- 
1.8.1.5

[PATCH v6 22/23] modetest: Fix line stride in SMPTE YUV packet pattern generator

[Laurent Pinchart]

The line stride passed to the function is expressed in bytes, there's no
need to multiply it by 2.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/modetest/buffers.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c
index 1ca3be5..1575bf6 100644
--- a/tests/modetest/buffers.c
+++ b/tests/modetest/buffers.c
@@ -337,13 +337,13 @@ fill_smpte_yuv_packed(const struct yuv_info *yuv, unsigned char *mem,
 	for (y = 0; y < height * 6 / 9; ++y) {
 		for (x = 0; x < width; ++x)
 			y_mem[2*x] = colors_top[x * 7 / width].y;
-		y_mem += stride * 2;
+		y_mem += stride;
 	}
 
 	for (; y < height * 7 / 9; ++y) {
 		for (x = 0; x < width; ++x)
 			y_mem[2*x] = colors_middle[x * 7 / width].y;
-		y_mem += stride * 2;
+		y_mem += stride;
 	}
 
 	for (; y < height; ++y) {
@@ -354,7 +354,7 @@ fill_smpte_yuv_packed(const struct yuv_info *yuv, unsigned char *mem,
 						   / (width / 7) + 4].y;
 		for (; x < width; ++x)
 			y_mem[2*x] = colors_bottom[7].y;
-		y_mem += stride * 2;
+		y_mem += stride;
 	}
 
 	/* Chroma */
@@ -363,7 +363,7 @@ fill_smpte_yuv_packed(const struct yuv_info *yuv, unsigned char *mem,
 			c_mem[2*x+u] = colors_top[x * 7 / width].u;
 			c_mem[2*x+v] = colors_top[x * 7 / width].v;
 		}
-		c_mem += stride * 2;
+		c_mem += stride;
 	}
 
 	for (; y < height * 7 / 9; ++y) {
@@ -371,7 +371,7 @@ fill_smpte_yuv_packed(const struct yuv_info *yuv, unsigned char *mem,
 			c_mem[2*x+u] = colors_middle[x * 7 / width].u;
 			c_mem[2*x+v] = colors_middle[x * 7 / width].v;
 		}
-		c_mem += stride * 2;
+		c_mem += stride;
 	}
 
 	for (; y < height; ++y) {
@@ -389,7 +389,7 @@ fill_smpte_yuv_packed(const struct yuv_info *yuv, unsigned char *mem,
 			c_mem[2*x+u] = colors_bottom[7].u;
 			c_mem[2*x+v] = colors_bottom[7].v;
 		}
-		c_mem += stride * 2;
+		c_mem += stride;
 	}
 }
 
-- 
1.8.1.5

[PATCH v6 23/23] modetest: Allocate NV buffers large enough for the two planes

[Laurent Pinchart]

Multiple the image height by 1.5 for NV12/NV21 and by 2 for NV16/NV61 to
make room for the chroma plane.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/modetest/buffers.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c
index 1575bf6..8206ce3 100644
--- a/tests/modetest/buffers.c
+++ b/tests/modetest/buffers.c
@@ -1038,12 +1038,29 @@ create_test_buffer(struct kms_driver *kms, unsigned int format,
 		   unsigned int handles[4], unsigned int pitches[4],
 		   unsigned int offsets[4], enum fill_pattern pattern)
 {
+	unsigned int virtual_height;
 	struct kms_bo *bo;
 	void *planes[3] = { 0, };
 	void *virtual;
 	int ret;
 
-	bo = allocate_buffer(kms, width, height, &pitches[0]);
+	switch (format) {
+	case DRM_FORMAT_NV12:
+	case DRM_FORMAT_NV21:
+		virtual_height = height * 3 / 2;
+		break;
+
+	case DRM_FORMAT_NV16:
+	case DRM_FORMAT_NV61:
+		virtual_height = height * 2;
+		break;
+
+	default:
+		virtual_height = height;
+		break;
+	}
+
+	bo = allocate_buffer(kms, width, virtual_height, &pitches[0]);
 	if (!bo)
 		return NULL;
 
-- 
1.8.1.5

Re: [PATCH v6 08/23] modetest: Add a command line parameter to set properties

[Ville Syrjälä]

On Fri, Jun 14, 2013 at 11:34:42PM +0200, Laurent Pinchart wrote:
> The -w parameter can be used to set a property value from the command
> line, using the target object ID and the property name.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  tests/modetest/modetest.c | 108 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 106 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index 778af62..858d480 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
<snip>
> @@ -1008,9 +1082,20 @@ static int parse_plane(struct plane_arg *p, const char *arg)
>  	return 0;
>  }
>  
> +static int parse_property(struct property_arg *p, const char *arg)
> +{
> +	if (sscanf(arg, "%d:%32[^:]:%" SCNu64, &p->obj_id, p->name, &p->value) != 3)

nit: could use stringification to get rid of the magic number 32 here.

I didn't spot any real problems in the series. But I must admit that I
mainly just glanced at most of the changes in since many of the diffs
are a bit hard to read.

I also gave it a quick try using sprites and setting a few modes. And I
found a bug in i915 while doing that, so clearly it has already proved
useful ;)

> +		return -1;
> +
> +	p->obj_type = 0;
> +	p->name[DRM_PROP_NAME_LEN] = '\0';
> +
> +	return 0;
> +}
> +
>  static void usage(char *name)
>  {
> -	fprintf(stderr, "usage: %s [-cdefMmPpsv]\n", name);
> +	fprintf(stderr, "usage: %s [-cdefMmPpsvw]\n", name);
>  
>  	fprintf(stderr, "\n Query options:\n\n");
>  	fprintf(stderr, "\t-c\tlist connectors\n");

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH v6 08/23] modetest: Add a command line parameter to set properties

[Laurent Pinchart]

Hi Ville,

Thank you for the review.

On Monday 24 June 2013 18:08:37 Ville Syrjälä wrote:
> On Fri, Jun 14, 2013 at 11:34:42PM +0200, Laurent Pinchart wrote:
> > The -w parameter can be used to set a property value from the command
> > line, using the target object ID and the property name.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  tests/modetest/modetest.c | 108 ++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 106 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> > index 778af62..858d480 100644
> > --- a/tests/modetest/modetest.c
> > +++ b/tests/modetest/modetest.c
> 
> <snip>
> 
> > @@ -1008,9 +1082,20 @@ static int parse_plane(struct plane_arg *p, const
> > char *arg)> 
> >  	return 0;
> >  
> >  }
> > 
> > +static int parse_property(struct property_arg *p, const char *arg)
> > +{
> > +	if (sscanf(arg, "%d:%32[^:]:%" SCNu64, &p->obj_id, p->name, &p-
>value)
> > != 3)
> nit: could use stringification to get rid of the magic number 32 here.

What do you mean exactly ?

> I didn't spot any real problems in the series. But I must admit that I
> mainly just glanced at most of the changes in since many of the diffs
> are a bit hard to read.
> 
> I also gave it a quick try using sprites and setting a few modes. And I
> found a bug in i915 while doing that, so clearly it has already proved
> useful ;)

That's nice to know :-)

> > +		return -1;
> > +
> > +	p->obj_type = 0;
> > +	p->name[DRM_PROP_NAME_LEN] = '\0';
> > +
> > +	return 0;
> > +}
> > +
> > 
> >  static void usage(char *name)
> >  {
> > 
> > -	fprintf(stderr, "usage: %s [-cdefMmPpsv]\n", name);
> > +	fprintf(stderr, "usage: %s [-cdefMmPpsvw]\n", name);
> > 
> >  	fprintf(stderr, "\n Query options:\n\n");
> >  	fprintf(stderr, "\t-c\tlist connectors\n");
-- 
Regards,

Laurent Pinchart

Re: [PATCH v6 00/23] modetest enhancements

[Laurent Pinchart]

Hi Jerome, Marek,

On Friday 14 June 2013 23:34:34 Laurent Pinchart wrote:
> Hello,
> 
> Here's the sixth (and hopefully final) version of my modeset enhancements
> patch set.
> 
> Beside various cleanups, these patches allow dropping master after mode set,
> configuring more than two pipes and planes, setting properties from the
> command line, setting plane positions and configuring pipes with multiple
> connectors in cloned mode.
> 
> The code has been tested with the R-Car DU DRM driver.

Any chance to get this series applied ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v6 08/23] modetest: Add a command line parameter to set properties

[Ville Syrjälä]

On Thu, Jun 27, 2013 at 10:10:43AM +0200, Laurent Pinchart wrote:
> Hi Ville,
> 
> Thank you for the review.
> 
> On Monday 24 June 2013 18:08:37 Ville Syrjälä wrote:
> > On Fri, Jun 14, 2013 at 11:34:42PM +0200, Laurent Pinchart wrote:
> > > The -w parameter can be used to set a property value from the command
> > > line, using the target object ID and the property name.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > > 
> > >  tests/modetest/modetest.c | 108 ++++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 106 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> > > index 778af62..858d480 100644
> > > --- a/tests/modetest/modetest.c
> > > +++ b/tests/modetest/modetest.c
> > 
> > <snip>
> > 
> > > @@ -1008,9 +1082,20 @@ static int parse_plane(struct plane_arg *p, const
> > > char *arg)> 
> > >  	return 0;
> > >  
> > >  }
> > > 
> > > +static int parse_property(struct property_arg *p, const char *arg)
> > > +{
> > > +	if (sscanf(arg, "%d:%32[^:]:%" SCNu64, &p->obj_id, p->name, &p-
> >value)
> > > != 3)
> > nit: could use stringification to get rid of the magic number 32 here.
> 
> What do you mean exactly ?

Something like this:

#define str(x) #x
#define xstr(x) str(x)
sscanf(arg, "%d:%" xstr(DRM_PROP_NAME_LEN) "[^:]:%" SCNu64, ...

Although it does make it a bit hard to parse for a human.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH v6 08/23] modetest: Add a command line parameter to set properties

[Laurent Pinchart]

On Thursday 27 June 2013 11:31:48 Ville Syrjälä wrote:
> On Thu, Jun 27, 2013 at 10:10:43AM +0200, Laurent Pinchart wrote:
> > On Monday 24 June 2013 18:08:37 Ville Syrjälä wrote:
> > > On Fri, Jun 14, 2013 at 11:34:42PM +0200, Laurent Pinchart wrote:
> > > > The -w parameter can be used to set a property value from the command
> > > > line, using the target object ID and the property name.
> > > > 
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > ---
> > > > 
> > > >  tests/modetest/modetest.c | 108 ++++++++++++++++++++++++++++++++++++-
> > > >  1 file changed, 106 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> > > > index 778af62..858d480 100644
> > > > --- a/tests/modetest/modetest.c
> > > > +++ b/tests/modetest/modetest.c
> > > 
> > > <snip>
> > > 
> > > > @@ -1008,9 +1082,20 @@ static int parse_plane(struct plane_arg *p,
> > > > const char *arg)
> > > >  	return 0;
> > > >  }
> > > > 
> > > > +static int parse_property(struct property_arg *p, const char *arg)
> > > > +{
> > > > +	if (sscanf(arg, "%d:%32[^:]:%" SCNu64, &p->obj_id, p->name, &p-
> > >
> > >value)
> > >
> > > > != 3)
> > > 
> > > nit: could use stringification to get rid of the magic number 32 here.
> > 
> > What do you mean exactly ?
> 
> Something like this:
> 
> #define str(x) #x
> #define xstr(x) str(x)
> sscanf(arg, "%d:%" xstr(DRM_PROP_NAME_LEN) "[^:]:%" SCNu64, ...
> 
> Although it does make it a bit hard to parse for a human.

Right. I'm fine with both. "%m[^:]" might be an interesting alternative 
option.

-- 
Regards,

Laurent Pinchart