[PATCH v4 00/21] modetest enhancements

[PATCH v4 00/21] modetest enhancements

[Laurent Pinchart]

Hello,

Here's the fourth version of my modetest enhancements patch set.

The third version only contained patches 01/21 to 04/21. I then felt the need
for a test tool that allows testing more driver features and ended up extending
modetest accordingly.

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.

Laurent Pinchart (21):
  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

 tests/modetest/Makefile.am |    4 +-
 tests/modetest/buffers.c   |   13 +-
 tests/modetest/buffers.h   |    5 +-
 tests/modetest/modetest.c  | 1192 +++++++++++++++++++++++++++++++-------------
 4 files changed, 856 insertions(+), 358 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH v4 01/21] 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 065ae13..9376767 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 5086381..3c93994 100644
--- a/tests/modetest/buffers.c
+++ b/tests/modetest/buffers.c
@@ -882,8 +882,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[] = {
@@ -917,13 +917,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 c91bb9d..1081c78 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, "embedded displayport" },
 };
 
-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 v4 02/21] 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 1081c78..92c758b 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 v4 03/21] 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 92c758b..1d1f49d 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 v4 04/21] 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 | 41 ++++++++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 1d1f49d..91dabfc 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -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,6 +946,7 @@ 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];
@@ -960,6 +964,9 @@ int main(int argc, char **argv)
 		case 'f':
 			framebuffers = 1;
 			break;
+		case 'M':
+			module = optarg;
+			break;
 		case 'm':
 			modes = 1;
 			break;
@@ -989,14 +996,27 @@ int main(int argc, char **argv)
 	if (argc == 1)
 		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 +1025,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

Re: [PATCH v4 04/21] modetest: Add a command line parameter to select the driver

[Ville Syrjälä]

On Tue, Mar 19, 2013 at 03:55:45PM +0100, Laurent Pinchart wrote:
> 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 | 41 ++++++++++++++++++++++++++++-------------
>  1 file changed, 28 insertions(+), 13 deletions(-)
> 
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index 1d1f49d..91dabfc 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
> @@ -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");

You didn't add it to the "usage: ..." line. Same goes for patch 05/21.

> +
>  	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,6 +946,7 @@ 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];
> @@ -960,6 +964,9 @@ int main(int argc, char **argv)
>  		case 'f':
>  			framebuffers = 1;
>  			break;
> +		case 'M':
> +			module = optarg;
> +			break;
>  		case 'm':
>  			modes = 1;
>  			break;
> @@ -989,14 +996,27 @@ int main(int argc, char **argv)
>  	if (argc == 1)
>  		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 +1025,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
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH v4 04/21] modetest: Add a command line parameter to select the driver

[Laurent Pinchart]

Hi Ville,

On Wednesday 03 April 2013 13:06:30 Ville Syrjälä wrote:
> On Tue, Mar 19, 2013 at 03:55:45PM +0100, Laurent Pinchart wrote:
> > 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 | 41 ++++++++++++++++++++++++++++-------------
> >  1 file changed, 28 insertions(+), 13 deletions(-)
> > 
> > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> > index 1d1f49d..91dabfc 100644
> > --- a/tests/modetest/modetest.c
> > +++ b/tests/modetest/modetest.c
> > @@ -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");
> 
> You didn't add it to the "usage: ..." line. Same goes for patch 05/21.

Good catch, thanks. I'll fix that.

-- 
Regards,

Laurent Pinchart

[PATCH v4 05/21] 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 | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 91dabfc..2648b0b 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -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;
@@ -958,6 +960,9 @@ int main(int argc, char **argv)
 		case 'c':
 			connectors = 1;
 			break;
+		case 'd':
+			drop_master = 1;
+			break;
 		case 'e':
 			encoders = 1;
 			break;
@@ -1041,6 +1046,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 v4 06/21] 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 | 406 +++++++++++++++++++++++++++++-----------------
 1 file changed, 259 insertions(+), 147 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 2648b0b..31205ee 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\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%d\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",
@@ -686,7 +800,7 @@ set_plane(struct kms_driver *kms, struct connector *c, struct plane *p)
 }
 
 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 +951,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 +989,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 +1065,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, }, };
 	
 	opterr = 0;
 	while ((c = getopt(argc, argv, optstr)) != -1) {
@@ -1030,10 +1144,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;
 	}
@@ -1051,7 +1163,7 @@ int main(int argc, char **argv)
 		getchar();
 	}
 
-	drmModeFreeResources(resources);
+	free_resources(resources);
 
 	return 0;
 }
-- 
1.8.1.5

[PATCH v4 07/21] 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 31205ee..f58c01d 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -1065,8 +1065,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;
 	
 	opterr = 0;
 	while ((c = getopt(argc, argv, optstr)) != -1) {
@@ -1090,8 +1090,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':
@@ -1099,8 +1107,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 v4 08/21] 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 f58c01d..7153a40 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
@@ -1006,9 +1080,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 [-cefmPpsv]\n", name);
+	fprintf(stderr, "usage: %s [-cefmPpsvw]\n", name);
 
 	fprintf(stderr, "\n Query options:\n\n");
 	fprintf(stderr, "\t-c\tlist connectors\n");
@@ -1021,6 +1106,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");
@@ -1053,7 +1139,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)
 {
@@ -1065,8 +1151,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;
 	
 	opterr = 0;
 	while ((c = getopt(argc, argv, optstr)) != -1) {
@@ -1122,6 +1210,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 (con_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;
@@ -1172,6 +1273,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

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

[Laurent Pinchart]

On Tuesday 19 March 2013 15:55:49 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 f58c01d..7153a40 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c

[snip]

> @@ -1122,6 +1210,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 (con_args == NULL) {

This should obviously be prop_args. I'll fix it in v5.

> +				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;

-- 
Regards,

Laurent Pinchart

[PATCH v4 09/21] 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 | 72 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 57 insertions(+), 15 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 7153a40..f95efe6 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -645,6 +645,7 @@ struct connector_arg {
 
 struct plane_arg {
 	uint32_t con_id;  /* the id of connector to bind to */
+	uint32_t x, y;
 	uint32_t w, h;
 	unsigned int fb_id;
 	char format_str[5]; /* need to leave room for terminating \0 */
@@ -855,11 +856,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->x == (uint32_t)-1 || p->y == (uint32_t)-1) {
+		/* 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,
@@ -870,6 +876,8 @@ set_plane(struct kms_driver *kms, struct connector_arg *c, struct plane_arg *p)
 		return -1;
 	}
 
+	ovr->crtc_id = c->crtc;
+
 	return 0;
 }
 
@@ -1063,18 +1071,52 @@ 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;
+
+	plane->con_id = strtoul(p, &end, 10);
+	if (*end != ':')
+		return -EINVAL;
+
+	p = end + 1;
+	if (*p == '(') {
+		plane->x = strtoul(p + 1, &end, 10);
+		if (*end != ',')
+			return -EINVAL;
+		p = end + 1;
+		plane->y = strtoul(p, &end, 10);
+		if (*end++ != ')')
+			return -EINVAL;
+		if (*end++ != '/')
+			return -EINVAL;
+		p = end;
+	} else {
+		plane->x = (uint32_t)-1;
+		plane->y = (uint32_t)-1;
+	}
 
-	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;
+	plane->w = strtoul(p, &end, 10);
+	if (*end != 'x')
+		return -EINVAL;
 
-	p->fourcc = format_fourcc(p->format_str);
-	if (p->fourcc == 0) {
-		fprintf(stderr, "unknown format %s\n", p->format_str);
-		return -1;
+	p = end + 1;
+	plane->h = strtoul(p, &end, 10);
+
+	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;
@@ -1103,7 +1145,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>:[(x,y)/]<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");
-- 
1.8.1.5

Re: [PATCH v4 09/21] modetest: Allow specifying plane position

[Archit Taneja]

Hi Laurent,

On Tuesday 19 March 2013 08:25 PM, Laurent Pinchart wrote:
> 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.

Thanks for this series. I tested the patches with a Panda ES board.

I was facing issues with the plane position though, when I execute this 
on the command line:

./modetest -s 12:1440x900 -P 6:(0,0)/300x200

I get a syntax error by bash saying it doesn't expect "(". I guess there 
are ways around to get over this, but I was wondering if we could get 
rid of the braces all together to keep it simple? The "/" character 
could be used to figure out whether the user has also mentioned position 
or not.

Archit

>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>   tests/modetest/modetest.c | 72 +++++++++++++++++++++++++++++++++++++----------
>   1 file changed, 57 insertions(+), 15 deletions(-)
>
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index 7153a40..f95efe6 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
> @@ -645,6 +645,7 @@ struct connector_arg {
>
>   struct plane_arg {
>   	uint32_t con_id;  /* the id of connector to bind to */
> +	uint32_t x, y;
>   	uint32_t w, h;
>   	unsigned int fb_id;
>   	char format_str[5]; /* need to leave room for terminating \0 */
> @@ -855,11 +856,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->x == (uint32_t)-1 || p->y == (uint32_t)-1) {
> +		/* 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,
> @@ -870,6 +876,8 @@ set_plane(struct kms_driver *kms, struct connector_arg *c, struct plane_arg *p)
>   		return -1;
>   	}
>
> +	ovr->crtc_id = c->crtc;
> +
>   	return 0;
>   }
>
> @@ -1063,18 +1071,52 @@ 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;
> +
> +	plane->con_id = strtoul(p, &end, 10);
> +	if (*end != ':')
> +		return -EINVAL;
> +
> +	p = end + 1;
> +	if (*p == '(') {
> +		plane->x = strtoul(p + 1, &end, 10);
> +		if (*end != ',')
> +			return -EINVAL;
> +		p = end + 1;
> +		plane->y = strtoul(p, &end, 10);
> +		if (*end++ != ')')
> +			return -EINVAL;
> +		if (*end++ != '/')
> +			return -EINVAL;
> +		p = end;
> +	} else {
> +		plane->x = (uint32_t)-1;
> +		plane->y = (uint32_t)-1;
> +	}
>
> -	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;
> +	plane->w = strtoul(p, &end, 10);
> +	if (*end != 'x')
> +		return -EINVAL;
>
> -	p->fourcc = format_fourcc(p->format_str);
> -	if (p->fourcc == 0) {
> -		fprintf(stderr, "unknown format %s\n", p->format_str);
> -		return -1;
> +	p = end + 1;
> +	plane->h = strtoul(p, &end, 10);
> +
> +	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;
> @@ -1103,7 +1145,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>:[(x,y)/]<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");
>

Re: [PATCH v4 09/21] modetest: Allow specifying plane position

[Laurent Pinchart]

Hi Archit,

On Monday 25 March 2013 11:44:35 Archit Taneja wrote:
> Hi Laurent,
> 
> On Tuesday 19 March 2013 08:25 PM, Laurent Pinchart wrote:
> > 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.
> 
> Thanks for this series. I tested the patches with a Panda ES board.
> 
> I was facing issues with the plane position though, when I execute this
> on the command line:
> 
> ./modetest -s 12:1440x900 -P 6:(0,0)/300x200
> 
> I get a syntax error by bash saying it doesn't expect "(". I guess there
> are ways around to get over this,

I use

./modetest -s 12:1440x900 -P '6:(0,0)/300x200'

> but I was wondering if we could get rid of the braces all together to keep
> it simple? The "/" character could be used to figure out whether the user
> has also mentioned position or not.

It makes parsing the option a bit more complex, but I can do that if you think 
it's better.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 09/21] modetest: Allow specifying plane position

[Archit Taneja]

On Tuesday 26 March 2013 04:11 AM, Laurent Pinchart wrote:
> Hi Archit,
>
> On Monday 25 March 2013 11:44:35 Archit Taneja wrote:
>> Hi Laurent,
>>
>> On Tuesday 19 March 2013 08:25 PM, Laurent Pinchart wrote:
>>> 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.
>>
>> Thanks for this series. I tested the patches with a Panda ES board.
>>
>> I was facing issues with the plane position though, when I execute this
>> on the command line:
>>
>> ./modetest -s 12:1440x900 -P 6:(0,0)/300x200
>>
>> I get a syntax error by bash saying it doesn't expect "(". I guess there
>> are ways around to get over this,
>
> I use
>
> ./modetest -s 12:1440x900 -P '6:(0,0)/300x200'
>
>> but I was wondering if we could get rid of the braces all together to keep
>> it simple? The "/" character could be used to figure out whether the user
>> has also mentioned position or not.
>
> It makes parsing the option a bit more complex, but I can do that if you think
> it's better.

I think it's fine. After googling a bit on the syntax error issue, I 
thought that putting the command in a script file was the only option, 
but your method above is convenient enough.

Thanks,
Archit

Re: [PATCH v4 09/21] modetest: Allow specifying plane position

[Ville Syrjälä]

On Tue, Mar 19, 2013 at 03:55:50PM +0100, Laurent Pinchart wrote:
> 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 | 72 +++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 57 insertions(+), 15 deletions(-)
> 
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index 7153a40..f95efe6 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
> @@ -645,6 +645,7 @@ struct connector_arg {
>  
>  struct plane_arg {
>  	uint32_t con_id;  /* the id of connector to bind to */
> +	uint32_t x, y;

I'd like the coordinates to allow negative values too. I just posted my
latest version of the plane clipping patches. The main feature there is
to allow coordinates to be partially/fully offscreen and the driver will
clip then the plane to the screen dimensions. Hence negative coordinates
become somewhat useful.

And if you're looking into plane stuff in general, maybe you can review
my patches? The drm_rect utility functions should be useful for all
drivers that have to deal with movable or scalable planes.

Now I'll see about trying this stuff out with my latest patches.

<snip>

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH v4 09/21] modetest: Allow specifying plane position

[Ville Syrjälä]

On Wed, Mar 27, 2013 at 05:57:20PM +0200, Ville Syrjälä wrote:
> On Tue, Mar 19, 2013 at 03:55:50PM +0100, Laurent Pinchart wrote:
> > 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 | 72 +++++++++++++++++++++++++++++++++++++----------
> >  1 file changed, 57 insertions(+), 15 deletions(-)
> > 
> > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> > index 7153a40..f95efe6 100644
> > --- a/tests/modetest/modetest.c
> > +++ b/tests/modetest/modetest.c
> > @@ -645,6 +645,7 @@ struct connector_arg {
> >  
> >  struct plane_arg {
> >  	uint32_t con_id;  /* the id of connector to bind to */
> > +	uint32_t x, y;
> 
> I'd like the coordinates to allow negative values too.

Tested it and it actually works w/ negative values thanks to the way
strtoul() works :) The only real obstacle is the magic '-1' handling.
I guess you should just give up on magic values and add some flag to
indicate whether the user provided the coords or not.

Also I must say that I don't like the syntax you used for specifying the
coords. '(' and ')' need to be escaped or the shell eats them. I've been
using the x11 -geometry syntax whenever I have to deal with the x/y/w/h
combination. It's a reasonably well known syntax and doesn't have these
shell issues. Maybe you could use it here as well.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH v4 09/21] modetest: Allow specifying plane position

[Laurent Pinchart]

Hi Ville,

On Wednesday 27 March 2013 19:15:31 Ville Syrjälä wrote:
> On Wed, Mar 27, 2013 at 05:57:20PM +0200, Ville Syrjälä wrote:
> > On Tue, Mar 19, 2013 at 03:55:50PM +0100, Laurent Pinchart wrote:
> > > 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 | 72 ++++++++++++++++++++++++++++++----------
> > >  1 file changed, 57 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> > > index 7153a40..f95efe6 100644
> > > --- a/tests/modetest/modetest.c
> > > +++ b/tests/modetest/modetest.c
> > > @@ -645,6 +645,7 @@ struct connector_arg {
> > > 
> > >  struct plane_arg {
> > >  
> > >  	uint32_t con_id;  /* the id of connector to bind to */
> > > 
> > > +	uint32_t x, y;
> > 
> > I'd like the coordinates to allow negative values too.
> 
> Tested it and it actually works w/ negative values thanks to the way
> strtoul() works :) The only real obstacle is the magic '-1' handling.
> I guess you should just give up on magic values and add some flag to
> indicate whether the user provided the coords or not.
> 
> Also I must say that I don't like the syntax you used for specifying the
> coords. '(' and ')' need to be escaped or the shell eats them.

You're not the first one to complain, I don't mind changing the syntax 
(although escaping is not mandatory, you can just enclose the whole argument 
in quotes).

> I've been using the x11 -geometry syntax whenever I have to deal with the
> x/y/w/h combination. It's a reasonably well known syntax and doesn't have
> these shell issues. Maybe you could use it here as well.

The issue with the geometry syntax is that you can't put the top-left corner 
at negative coordinates, as -XOFF places the right edge of the plane XOFF 
pixels from the right edge of the screen, and similarly for -YOFF. Should we 
deviate from that spec and consider -XOFF to mean XOFF pixels on the left side 
of the left edge (outside of the screen) ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 09/21] modetest: Allow specifying plane position

[Ville Syrjälä]

On Thu, Mar 28, 2013 at 01:16:09AM +0100, Laurent Pinchart wrote:
> Hi Ville,
> 
> On Wednesday 27 March 2013 19:15:31 Ville Syrjälä wrote:
> > On Wed, Mar 27, 2013 at 05:57:20PM +0200, Ville Syrjälä wrote:
> > > On Tue, Mar 19, 2013 at 03:55:50PM +0100, Laurent Pinchart wrote:
> > > > 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 | 72 ++++++++++++++++++++++++++++++----------
> > > >  1 file changed, 57 insertions(+), 15 deletions(-)
> > > > 
> > > > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> > > > index 7153a40..f95efe6 100644
> > > > --- a/tests/modetest/modetest.c
> > > > +++ b/tests/modetest/modetest.c
> > > > @@ -645,6 +645,7 @@ struct connector_arg {
> > > > 
> > > >  struct plane_arg {
> > > >  
> > > >  	uint32_t con_id;  /* the id of connector to bind to */
> > > > 
> > > > +	uint32_t x, y;
> > > 
> > > I'd like the coordinates to allow negative values too.
> > 
> > Tested it and it actually works w/ negative values thanks to the way
> > strtoul() works :) The only real obstacle is the magic '-1' handling.
> > I guess you should just give up on magic values and add some flag to
> > indicate whether the user provided the coords or not.
> > 
> > Also I must say that I don't like the syntax you used for specifying the
> > coords. '(' and ')' need to be escaped or the shell eats them.
> 
> You're not the first one to complain, I don't mind changing the syntax 
> (although escaping is not mandatory, you can just enclose the whole argument 
> in quotes).
> 
> > I've been using the x11 -geometry syntax whenever I have to deal with the
> > x/y/w/h combination. It's a reasonably well known syntax and doesn't have
> > these shell issues. Maybe you could use it here as well.
> 
> The issue with the geometry syntax is that you can't put the top-left corner 
> at negative coordinates, as -XOFF places the right edge of the plane XOFF 
> pixels from the right edge of the screen, and similarly for -YOFF. Should we 
> deviate from that spec and consider -XOFF to mean XOFF pixels on the left side 
> of the left edge (outside of the screen) ?

I forgot that there's this kind of magic change of origin in the
specification. I guess it's been too long since I've used negative
geometry coordinates under X.

I've just been using it so that the origin is always the top left
corner. That's how I chose to interpret things in my
drm_rect_debug_print() function for example. But since it's a bit
contrary to the X geometry spec, maybe it's not the best syntax
either. If anyone has a better idea in mind, I'm open to suggestions.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH v4 09/21] modetest: Allow specifying plane position

[Laurent Pinchart]

Hi Ville,

On Wednesday 27 March 2013 19:15:31 Ville Syrjälä wrote:
> On Wed, Mar 27, 2013 at 05:57:20PM +0200, Ville Syrjälä wrote:
> > On Tue, Mar 19, 2013 at 03:55:50PM +0100, Laurent Pinchart wrote:
> > > 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 | 72 ++++++++++++++++++++++++++++++----------
> > >  1 file changed, 57 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> > > index 7153a40..f95efe6 100644
> > > --- a/tests/modetest/modetest.c
> > > +++ b/tests/modetest/modetest.c
> > > @@ -645,6 +645,7 @@ struct connector_arg {
> > > 
> > >  struct plane_arg {
> > >  
> > >  	uint32_t con_id;  /* the id of connector to bind to */
> > > 
> > > +	uint32_t x, y;
> > 
> > I'd like the coordinates to allow negative values too.
> 
> Tested it and it actually works w/ negative values thanks to the way
> strtoul() works :) The only real obstacle is the magic '-1' handling.
> I guess you should just give up on magic values and add some flag to
> indicate whether the user provided the coords or not.

Done :-) I'll post a new version.

> Also I must say that I don't like the syntax you used for specifying the
> coords. '(' and ')' need to be escaped or the shell eats them. I've been
> using the x11 -geometry syntax whenever I have to deal with the x/y/w/h
> combination. It's a reasonably well known syntax and doesn't have these
> shell issues. Maybe you could use it here as well.

Given that negative coordinates in the X geometry syntax don't match we what 
need, should I keep the current syntax, abuse the X geometry syntax, or use 
something else ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 09/21] modetest: Allow specifying plane position

[Ville Syrjälä]

On Thu, Apr 04, 2013 at 06:24:24PM +0200, Laurent Pinchart wrote:
> Hi Ville,
> 
> On Wednesday 27 March 2013 19:15:31 Ville Syrjälä wrote:
> > On Wed, Mar 27, 2013 at 05:57:20PM +0200, Ville Syrjälä wrote:
> > > On Tue, Mar 19, 2013 at 03:55:50PM +0100, Laurent Pinchart wrote:
> > > > 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 | 72 ++++++++++++++++++++++++++++++----------
> > > >  1 file changed, 57 insertions(+), 15 deletions(-)
> > > > 
> > > > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> > > > index 7153a40..f95efe6 100644
> > > > --- a/tests/modetest/modetest.c
> > > > +++ b/tests/modetest/modetest.c
> > > > @@ -645,6 +645,7 @@ struct connector_arg {
> > > > 
> > > >  struct plane_arg {
> > > >  
> > > >  	uint32_t con_id;  /* the id of connector to bind to */
> > > > 
> > > > +	uint32_t x, y;
> > > 
> > > I'd like the coordinates to allow negative values too.
> > 
> > Tested it and it actually works w/ negative values thanks to the way
> > strtoul() works :) The only real obstacle is the magic '-1' handling.
> > I guess you should just give up on magic values and add some flag to
> > indicate whether the user provided the coords or not.
> 
> Done :-) I'll post a new version.
> 
> > Also I must say that I don't like the syntax you used for specifying the
> > coords. '(' and ')' need to be escaped or the shell eats them. I've been
> > using the x11 -geometry syntax whenever I have to deal with the x/y/w/h
> > combination. It's a reasonably well known syntax and doesn't have these
> > shell issues. Maybe you could use it here as well.
> 
> Given that negative coordinates in the X geometry syntax don't match we what 
> need, should I keep the current syntax, abuse the X geometry syntax, or use 
> something else ?

Since no-one suggeste anything better, I vote for the abuse. I've
already gotten used to it :)

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH v4 09/21] modetest: Allow specifying plane position

[Laurent Pinchart]

Hi Ville,

On Wednesday 27 March 2013 17:57:20 Ville Syrjälä wrote:
> On Tue, Mar 19, 2013 at 03:55:50PM +0100, Laurent Pinchart wrote:
> > 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 | 72 ++++++++++++++++++++++++++++++++----------
> >  1 file changed, 57 insertions(+), 15 deletions(-)
> > 
> > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> > index 7153a40..f95efe6 100644
> > --- a/tests/modetest/modetest.c
> > +++ b/tests/modetest/modetest.c
> > @@ -645,6 +645,7 @@ struct connector_arg {
> > 
> >  struct plane_arg {
> >  	uint32_t con_id;  /* the id of connector to bind to */
> > +	uint32_t x, y;
> 
> I'd like the coordinates to allow negative values too. I just posted my
> latest version of the plane clipping patches. The main feature there is
> to allow coordinates to be partially/fully offscreen and the driver will
> clip then the plane to the screen dimensions. Hence negative coordinates
> become somewhat useful.
> 
> And if you're looking into plane stuff in general, maybe you can review
> my patches? The drm_rect utility functions should be useful for all
> drivers that have to deal with movable or scalable planes.

I'll try to, but I'm very busy nowadays with pinctrl patches :-/

> Now I'll see about trying this stuff out with my latest patches.

-- 
Regards,

Laurent Pinchart

[PATCH v4 10/21] 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 f95efe6..ffdb0aa 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -836,14 +836,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 v4 11/21] 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 ffdb0aa..d6eed4a 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -95,7 +95,7 @@ struct resources {
 };
 
 struct resources *resources;
-int fd, modes;
+int fd;
 
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
 
@@ -1141,7 +1141,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");
@@ -1181,7 +1180,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)
 {
@@ -1216,9 +1215,6 @@ int main(int argc, char **argv)
 		case 'M':
 			module = optarg;
 			break;
-		case 'm':
-			modes = 1;
-			break;
 		case 'P':
 			plane_args = realloc(plane_args,
 					     (plane_count + 1) * sizeof *plane_args);
@@ -1272,7 +1268,7 @@ int main(int argc, char **argv)
 	}
 
 	if (argc == 1)
-		encoders = connectors = crtcs = planes = modes = framebuffers = 1;
+		encoders = connectors = crtcs = planes = framebuffers = 1;
 
 	if (module) {
 		fd = drmOpen(module, NULL);
-- 
1.8.1.5

[PATCH v4 12/21] 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 | 199 ++++++++++++++++++++++++----------------------
 1 file changed, 102 insertions(+), 97 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index d6eed4a..164e7e1 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -94,8 +94,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]))
 
@@ -196,15 +200,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;
 
@@ -239,14 +243,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;
 
@@ -262,8 +265,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);
@@ -315,7 +318,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);
@@ -323,19 +326,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;
@@ -363,7 +366,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]);
 		}
@@ -371,15 +374,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;
@@ -394,7 +397,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 {
@@ -404,15 +407,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;
 
@@ -424,18 +427,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\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;
@@ -456,7 +459,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 {
@@ -517,7 +520,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;
 
@@ -527,7 +530,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));
@@ -552,7 +555,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],	\
@@ -572,7 +575,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,					\
@@ -587,14 +590,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));
@@ -652,16 +655,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;
 
@@ -688,8 +690,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;
 
@@ -701,8 +703,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;
 		}
@@ -722,7 +724,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;
@@ -746,11 +748,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);
@@ -778,7 +780,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));
@@ -816,7 +819,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] */
@@ -827,8 +830,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;
 
@@ -844,13 +847,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;
@@ -868,7 +871,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",
@@ -881,11 +884,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;
@@ -895,7 +896,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;
@@ -903,19 +904,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",
@@ -931,11 +925,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;
 
@@ -947,19 +941,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));
@@ -970,7 +964,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));
@@ -1011,8 +1005,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",
@@ -1023,12 +1017,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))
@@ -1157,8 +1150,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? */
@@ -1184,6 +1175,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;
@@ -1196,6 +1189,7 @@ int main(int argc, char **argv)
 	struct connector_arg *con_args = NULL;
 	struct plane_arg *plane_args = NULL;
 	struct property_arg *prop_args = NULL;
+	int ret;
 	
 	opterr = 0;
 	while ((c = getopt(argc, argv, optstr)) != -1) {
@@ -1271,16 +1265,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");
@@ -1288,7 +1282,7 @@ int main(int argc, char **argv)
 			}
 		}
 
-		if (fd < 0) {
+		if (dev.fd < 0) {
 			fprintf(stderr, "no device found.\n");
 			return 1;
 		}
@@ -1299,29 +1293,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 v4 13/21] 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 164e7e1..70fa262 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -639,7 +639,6 @@ struct connector_arg {
 	drmModeModeInfo *mode;
 	drmModeEncoder *encoder;
 	int crtc;
-	int pipe;
 	unsigned int fb_id[2], current_fb_id;
 	struct timeval start;
 
@@ -701,15 +700,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;
-		}
-	}
-
 }
 
 /* -----------------------------------------------------------------------------
@@ -827,15 +817,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 v4 14/21] 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 70fa262..ba025d6 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -637,7 +637,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;
@@ -657,6 +656,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 */
@@ -690,16 +690,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 v4 15/21] 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 ba025d6..74e82ef 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -633,11 +633,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;
 
@@ -688,18 +689,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;
 }
 
 /* -----------------------------------------------------------------------------
@@ -794,7 +810,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++;
@@ -824,14 +840,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;
 	}
 
@@ -845,7 +861,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;
 	}
 
@@ -876,7 +892,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",
@@ -884,7 +900,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;
 }
@@ -928,9 +944,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 */
@@ -969,7 +985,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));
@@ -1037,13 +1053,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 v4 16/21] 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 74e82ef..0e3f396 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -61,6 +61,7 @@ struct crtc {
 	drmModeCrtc *crtc;
 	drmModeObjectProperties *props;
 	drmModePropertyRes **props_info;
+	drmModeModeInfo *mode;
 };
 
 struct encoder {
@@ -523,6 +524,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)
@@ -597,6 +599,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",
@@ -712,6 +717,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;
 		}
@@ -882,8 +888,8 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p)
 
 	if (p->x == (uint32_t)-1 || p->y == (uint32_t)-1) {
 		/* 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 v4 17/21] 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 0e3f396..1766916 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -651,7 +651,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 */
 	uint32_t x, y;
 	uint32_t w, h;
 	unsigned int fb_id;
@@ -830,8 +830,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] */
@@ -839,6 +838,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;
 
@@ -846,14 +846,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;
 	}
 
@@ -867,7 +868,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;
 	}
 
@@ -888,8 +890,8 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p)
 
 	if (p->x == (uint32_t)-1 || p->y == (uint32_t)-1) {
 		/* 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;
@@ -898,7 +900,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",
@@ -906,7 +908,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;
 }
@@ -967,8 +969,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;
 	}
 
@@ -1095,7 +1097,7 @@ static int parse_plane(struct plane_arg *plane, const char *p)
 {
 	char *end;
 
-	plane->con_id = strtoul(p, &end, 10);
+	plane->crtc_id = strtoul(p, &end, 10);
 	if (*end != ':')
 		return -EINVAL;
 
@@ -1164,7 +1166,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>:[(x,y)/]<w>x<h>[@<format>]\tset a plane\n");
+	fprintf(stderr, "\t-P <crtc_id>:[(x,y)/]<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");
-- 
1.8.1.5

Re: [PATCH v4 17/21] modetest: Give the CRTC ID to the -P option

[Rob Clark]

On Tue, Mar 19, 2013 at 10:55 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> 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.

hmm, I was thinking that it would be nice to someday make modetest
clever enough to deal w/ connector names instead of connector-id..
this kinda goes in the opposite direction.

BR,
-R

>
> 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 0e3f396..1766916 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
> @@ -651,7 +651,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 */
>         uint32_t x, y;
>         uint32_t w, h;
>         unsigned int fb_id;
> @@ -830,8 +830,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] */
> @@ -839,6 +838,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;
>
> @@ -846,14 +846,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;
>         }
>
> @@ -867,7 +868,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;
>         }
>
> @@ -888,8 +890,8 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p)
>
>         if (p->x == (uint32_t)-1 || p->y == (uint32_t)-1) {
>                 /* 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;
> @@ -898,7 +900,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",
> @@ -906,7 +908,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;
>  }
> @@ -967,8 +969,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;
>         }
>
> @@ -1095,7 +1097,7 @@ static int parse_plane(struct plane_arg *plane, const char *p)
>  {
>         char *end;
>
> -       plane->con_id = strtoul(p, &end, 10);
> +       plane->crtc_id = strtoul(p, &end, 10);
>         if (*end != ':')
>                 return -EINVAL;
>
> @@ -1164,7 +1166,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>:[(x,y)/]<w>x<h>[@<format>]\tset a plane\n");
> +       fprintf(stderr, "\t-P <crtc_id>:[(x,y)/]<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");
> --
> 1.8.1.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 17/21] modetest: Give the CRTC ID to the -P option

[Laurent Pinchart]

Hi Rob,

On Tuesday 19 March 2013 14:21:32 Rob Clark wrote:
> On Tue, Mar 19, 2013 at 10:55 AM, Laurent Pinchart wrote:
> > 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.
> 
> hmm, I was thinking that it would be nice to someday make modetest clever
> enough to deal w/ connector names instead of connector-id..

That's a good idea (and it shouldn't be too difficult to implement).

> this kinda goes in the opposite direction.

Please note that I use the CRTC ID instead of the connector ID for planes 
setup only. Planes are associated with a CRTC, not a connector. I thus thought 
it made sense to use the CRTC ID in the argument.

If you think that's a bad idea I can drop the patch. Should modetest then 
associate the planes with the CRTC currently associated with the given 
connector ?

-- 
Regards,

Laurent Pinchart

[PATCH v4 18/21] 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 | 105 +++++++++++++++++++++++++++++++---------------
 1 file changed, 72 insertions(+), 33 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 1766916..f40d6e6 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -100,6 +100,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]))
@@ -838,7 +846,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;
 
@@ -913,36 +921,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;
 	}
 
@@ -966,24 +975,38 @@ 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;
+}
+
+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;
+}
 
-	other_bo = create_test_buffer(dev->kms, c->fourcc, width, height, handles,
-				      pitches, offsets, PATTERN_PLAIN);
+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,
+				      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;
@@ -1001,7 +1024,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;
 	}
@@ -1049,7 +1072,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);
 }
 
@@ -1219,7 +1241,9 @@ int main(int argc, char **argv)
 	struct plane_arg *plane_args = NULL;
 	struct property_arg *prop_args = NULL;
 	int ret;
-	
+
+	memset(&dev, 0, sizeof dev);
+
 	opterr = 0;
 	while ((c = getopt(argc, argv, optstr)) != -1) {
 		switch (c) {
@@ -1322,6 +1346,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);
@@ -1339,7 +1368,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",
@@ -1347,11 +1376,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 v4 19/21] 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 | 164 ++++++++++++++++++++++++----------------------
 1 file changed, 86 insertions(+), 78 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index f40d6e6..cd87c69 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -634,7 +634,7 @@ error:
 }
 
 /* -----------------------------------------------------------------------------
- * Connectors and planes
+ * Pipes and planes
  */
 
 /*
@@ -644,8 +644,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];
@@ -667,14 +667,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)
@@ -683,50 +683,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;
 		}
 	}
@@ -813,28 +813,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;
 	}
 }
 
@@ -921,7 +921,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;
@@ -933,22 +933,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));
@@ -957,19 +959,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));
@@ -990,7 +995,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;
@@ -999,34 +1004,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);
@@ -1077,19 +1085,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;
@@ -1097,18 +1105,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;
 	}
 
@@ -1237,7 +1245,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;
 	int ret;
@@ -1280,14 +1288,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++;				      
@@ -1298,7 +1306,7 @@ int main(int argc, char **argv)
 		case 'w':
 			prop_args = realloc(prop_args,
 					   (prop_count + 1) * sizeof *prop_args);
-			if (con_args == NULL) {
+			if (pipe_args == NULL) {
 				fprintf(stderr, "memory allocation failed\n");
 				return 1;
 			}
@@ -1377,13 +1385,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 v4 20/21] 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 cd87c69..489cf2c 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 <stdio.h>
 #include <stdlib.h>
 #include <stdint.h>
@@ -633,6 +634,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
  */
@@ -645,7 +674,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];
@@ -667,69 +697,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;
 }
 
 /* -----------------------------------------------------------------------------
@@ -824,7 +899,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++;
@@ -927,6 +1002,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;
@@ -935,9 +1011,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;
@@ -964,12 +1041,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);
@@ -1024,8 +1104,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;
@@ -1088,13 +1169,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);
@@ -1197,7 +1300,7 @@ static void usage(char *name)
 
 	fprintf(stderr, "\n Test options:\n\n");
 	fprintf(stderr, "\t-P <crtc_id>:[(x,y)/]<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-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 v4 21/21] 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 489cf2c..15aedc8 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -634,6 +634,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;
@@ -726,26 +739,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