[PATCH v2 0/3] modetest: Allow selecting a driver manually

[PATCH v2 0/3] modetest: Allow selecting a driver manually

[Laurent Pinchart]

Hello,

Here's the second version of a small patch set that adds an argument to the
modetest command line to specify the driver name instead of using the builtin
list of drivers. This is particularly handy during development of new DRM/KMS
drivers.

Changes from v1:

- Use the module name specified on the command line instead of a random module
  name (I'll blame a bad copy & paste).

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

 tests/modetest/modetest.c | 88 ++++++++++++++++++++++++++++-------------------
 1 file changed, 53 insertions(+), 35 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH v2 1/3] 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>
---
 tests/modetest/modetest.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index c91bb9d..489918e 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.7.12.4

[PATCH v2 2/3] 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>
---
 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 489918e..34457e2 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)
 
 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, "\n Test options:\n\n");
+	fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\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.7.12.4

[PATCH v2 3/3] modetest: Add a command line parameter to select the driver

[Laurent Pinchart]

If the -M parameter is specific, 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>
---
 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 34457e2..9a2d1f8 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -908,6 +908,9 @@ 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;
 	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", module);
+			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.7.12.4

Re: [PATCH v2 3/3] modetest: Add a command line parameter to select the driver

[Laurent Pinchart]

On Monday 11 February 2013 21:13:45 Laurent Pinchart wrote:
> If the -M parameter is specific, 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>
> ---
>  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 34457e2..9a2d1f8 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c

[snip]

> @@ -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", module);

I should sleep before sending patches... Sorry for the noise, v3 will fix 
that.

> +			return 1;
>  		}
>  	}

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 3/3] modetest: Add a command line parameter to select the driver

[Jani Nikula]

On Mon, 11 Feb 2013, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> On Monday 11 February 2013 21:13:45 Laurent Pinchart wrote:
>> If the -M parameter is specific, 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>
>> ---
>>  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 34457e2..9a2d1f8 100644
>> --- a/tests/modetest/modetest.c
>> +++ b/tests/modetest/modetest.c
>
> [snip]
>
>> @@ -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", module);
>
> I should sleep before sending patches... Sorry for the noise, v3 will fix 
> that.

If you fix that bit, you can slam my

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

on the series.