dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH libdrm 1/2] modetest: Fix segmentation fault
@ 2019-07-22 16:08 Ezequiel Garcia
  2019-07-22 16:08 ` [PATCH libdrm 2/2] modetest: Add a new "-r" option to set a default mode Ezequiel Garcia
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Ezequiel Garcia @ 2019-07-22 16:08 UTC (permalink / raw)
  To: dri-devel; +Cc: Ezequiel Garcia, rohan.garg, Emil Velikov

When a mode is set with just a connector "-s foo",
we get a nasty segmentation fault. Fix it.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 tests/modetest/modetest.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index e66be6607e00..5e628127a130 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -1695,6 +1695,8 @@ static int parse_connector(struct pipe_arg *pipe, const char *arg)
 		return -1;
 
 	/* Parse the remaining parameters. */
+	if (!endp)
+		return -1;
 	if (*endp == '@') {
 		arg = endp + 1;
 		pipe->crtc_id = strtoul(arg, &endp, 10);
-- 
2.22.0

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

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

* [PATCH libdrm 2/2] modetest: Add a new "-r" option to set a default mode
  2019-07-22 16:08 [PATCH libdrm 1/2] modetest: Fix segmentation fault Ezequiel Garcia
@ 2019-07-22 16:08 ` Ezequiel Garcia
  2019-07-24 10:43   ` Rohan Garg
  2020-01-21 18:30   ` Emil Velikov
  2019-08-15 12:07 ` [PATCH libdrm 1/2] modetest: Fix segmentation fault Ezequiel Garcia
  2020-01-20 18:35 ` Ezequiel Garcia
  2 siblings, 2 replies; 7+ messages in thread
From: Ezequiel Garcia @ 2019-07-22 16:08 UTC (permalink / raw)
  To: dri-devel; +Cc: Ezequiel Garcia, rohan.garg, Emil Velikov

This option finds the first connected connector and then
sets its preferred mode on it.

Set this option to be set when no mode or plane is set
explicitily. This allows to quickly test, in cases where
one just needs something displayed.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 tests/modetest/modetest.c | 81 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 75 insertions(+), 6 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 5e628127a130..6042aaae7cca 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -901,7 +901,9 @@ static int pipe_find_crtc_and_mode(struct device *dev, struct pipe_arg *pipe)
 	drmModeModeInfo *mode = NULL;
 	int i;
 
-	pipe->mode = NULL;
+	/* If set_preferred is used, a mode is already set. */
+	if (pipe->mode)
+		goto find_crtc;
 
 	for (i = 0; i < (int)pipe->num_cons; i++) {
 		mode = connector_find_mode(dev, pipe->con_ids[i],
@@ -913,7 +915,9 @@ static int pipe_find_crtc_and_mode(struct device *dev, struct pipe_arg *pipe)
 			return -EINVAL;
 		}
 	}
+	pipe->mode = mode;
 
+find_crtc:
 	/* If the CRTC ID was specified, get the corresponding CRTC. Otherwise
 	 * locate a CRTC that can be attached to all the connectors.
 	 */
@@ -935,7 +939,6 @@ static int pipe_find_crtc_and_mode(struct device *dev, struct pipe_arg *pipe)
 		return -EINVAL;
 	}
 
-	pipe->mode = mode;
 	pipe->crtc->mode = mode;
 
 	return 0;
@@ -1813,7 +1816,7 @@ static void parse_fill_patterns(char *arg)
 
 static void usage(char *name)
 {
-	fprintf(stderr, "usage: %s [-acDdefMPpsCvw]\n", name);
+	fprintf(stderr, "usage: %s [-acDdefMPpsCvrw]\n", name);
 
 	fprintf(stderr, "\n Query options:\n\n");
 	fprintf(stderr, "\t-c\tlist connectors\n");
@@ -1826,6 +1829,7 @@ static void usage(char *name)
 	fprintf(stderr, "\t-s <connector_id>[,<connector_id>][@<crtc_id>]:<mode>[-<vrefresh>][@<format>]\tset a mode\n");
 	fprintf(stderr, "\t-C\ttest hw cursor\n");
 	fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
+	fprintf(stderr, "\t-r\tset the preferred mode\n");
 	fprintf(stderr, "\t-w <obj_id>:<prop_name>:<value>\tset property\n");
 	fprintf(stderr, "\t-a \tuse atomic API\n");
 	fprintf(stderr, "\t-F pattern1,pattern2\tspecify fill patterns\n");
@@ -1874,6 +1878,9 @@ static int pipe_resolve_connectors(struct device *dev, struct pipe_arg *pipe)
 	char *endp;
 
 	for (i = 0; i < pipe->num_cons; i++) {
+		/* If set_preferred is used, the connector is already resolved. */
+		if (pipe->con_ids[i])
+			continue;
 		id = strtoul(pipe->cons[i], &endp, 10);
 		if (endp == pipe->cons[i]) {
 			connector = get_connector_by_name(dev, pipe->cons[i]);
@@ -1885,14 +1892,62 @@ static int pipe_resolve_connectors(struct device *dev, struct pipe_arg *pipe)
 
 			id = connector->connector_id;
 		}
-
 		pipe->con_ids[i] = id;
 	}
 
 	return 0;
 }
 
-static char optstr[] = "acdD:efF:M:P:ps:Cvw:";
+static char optstr[] = "acdD:efF:M:P:ps:Cvrw:";
+
+static int pipe_find_preferred(struct device *dev, struct pipe_arg *pipe)
+{
+	drmModeRes *res = dev->resources->res;
+	drmModeConnector *con = NULL;
+	char *con_str;
+	int i;
+
+	for (i = 0; i < res->count_connectors; i++) {
+		con = drmModeGetConnector(dev->fd, res->connectors[i]);
+		if (con->connection == DRM_MODE_CONNECTED)
+			break;
+		drmModeFreeConnector(con);
+		con = NULL;
+	}
+
+	if (!con) {
+		printf("no connected connector!\n");
+		return -1;
+	}
+
+	con_str = malloc(8);
+	sprintf(con_str, "%d", con->connector_id);
+	strcpy(pipe->format_str, "XR24");
+	pipe->fourcc = util_format_fourcc(pipe->format_str);
+	pipe->num_cons = 1;
+	pipe->con_ids = calloc(1, sizeof(*pipe->con_ids));
+	pipe->cons = calloc(1, sizeof(*pipe->cons));
+	pipe->con_ids[0] = con->connector_id;
+	pipe->cons[0] = (const char*)con_str;
+
+	/* A CRTC possible will be chosen by pipe_find_crtc_and_mode. */
+	pipe->crtc_id = (uint32_t)-1;
+
+	/* Return the first mode if no preferred. */
+	pipe->mode = &con->modes[0];
+	for (i = 0; i < con->count_modes; i++) {
+		drmModeModeInfo *current_mode = &con->modes[i];
+
+		if (current_mode->type & DRM_MODE_TYPE_PREFERRED) {
+			pipe->mode = current_mode;
+			break;
+		}
+	}
+
+	sprintf(pipe->mode_str, "%dx%d", pipe->mode->hdisplay, pipe->mode->vdisplay);
+
+	return 0;
+}
 
 int main(int argc, char **argv)
 {
@@ -1903,6 +1958,7 @@ int main(int argc, char **argv)
 	int drop_master = 0;
 	int test_vsync = 0;
 	int test_cursor = 0;
+	int set_preferred = 0;
 	int use_atomic = 0;
 	char *device = NULL;
 	char *module = NULL;
@@ -1987,6 +2043,9 @@ int main(int argc, char **argv)
 		case 'v':
 			test_vsync = 1;
 			break;
+		case 'r':
+			set_preferred = 1;
+			break;
 		case 'w':
 			prop_args = realloc(prop_args,
 					   (prop_count + 1) * sizeof *prop_args);
@@ -2008,7 +2067,7 @@ int main(int argc, char **argv)
 	}
 
 	if (!args || (args == 1 && use_atomic))
-		encoders = connectors = crtcs = planes = framebuffers = 1;
+		set_preferred = encoders = connectors = crtcs = planes = framebuffers = 1;
 
 	dev.fd = util_open(device, module);
 	if (dev.fd < 0)
@@ -2044,6 +2103,16 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
+	if (set_preferred) {
+		count = 1;
+		pipe_args = calloc(1, sizeof(*pipe_args));
+		ret = pipe_find_preferred(&dev, &pipe_args[0]);
+		if (ret) {
+			fprintf(stderr, "can't get preferred connector and mode: %s\n", strerror(errno));
+			return 1;
+		}
+	}
+
 	for (i = 0; i < count; i++) {
 		if (pipe_resolve_connectors(&dev, &pipe_args[i]) < 0) {
 			free_resources(dev.resources);
-- 
2.22.0

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

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

* Re: [PATCH libdrm 2/2] modetest: Add a new "-r" option to set a default mode
  2019-07-22 16:08 ` [PATCH libdrm 2/2] modetest: Add a new "-r" option to set a default mode Ezequiel Garcia
@ 2019-07-24 10:43   ` Rohan Garg
  2020-01-21 18:30   ` Emil Velikov
  1 sibling, 0 replies; 7+ messages in thread
From: Rohan Garg @ 2019-07-24 10:43 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: Emil Velikov, dri-devel


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

On Monday, 22 July 2019 18:08:23 CEST Ezequiel Garcia wrote:
> This option finds the first connected connector and then
> sets its preferred mode on it.
> 
> Set this option to be set when no mode or plane is set
> explicitily. This allows to quickly test, in cases where
> one just needs something displayed.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  tests/modetest/modetest.c | 81 ++++++++++++++++++++++++++++++++++++---
>  1 file changed, 75 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index 5e628127a130..6042aaae7cca 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
> @@ -901,7 +901,9 @@ static int pipe_find_crtc_and_mode(struct device *dev,
> struct pipe_arg *pipe) drmModeModeInfo *mode = NULL;
>  	int i;
> 
> -	pipe->mode = NULL;
> +	/* If set_preferred is used, a mode is already set. */
> +	if (pipe->mode)
> +		goto find_crtc;
> 
>  	for (i = 0; i < (int)pipe->num_cons; i++) {
>  		mode = connector_find_mode(dev, pipe->con_ids[i],
> @@ -913,7 +915,9 @@ static int pipe_find_crtc_and_mode(struct device *dev,
> struct pipe_arg *pipe) return -EINVAL;
>  		}
>  	}
> +	pipe->mode = mode;
> 
> +find_crtc:
>  	/* If the CRTC ID was specified, get the corresponding CRTC. 
Otherwise
>  	 * locate a CRTC that can be attached to all the connectors.
>  	 */
> @@ -935,7 +939,6 @@ static int pipe_find_crtc_and_mode(struct device *dev,
> struct pipe_arg *pipe) return -EINVAL;
>  	}
> 
> -	pipe->mode = mode;
>  	pipe->crtc->mode = mode;
> 
>  	return 0;
> @@ -1813,7 +1816,7 @@ static void parse_fill_patterns(char *arg)
> 
>  static void usage(char *name)
>  {
> -	fprintf(stderr, "usage: %s [-acDdefMPpsCvw]\n", name);
> +	fprintf(stderr, "usage: %s [-acDdefMPpsCvrw]\n", name);
> 
>  	fprintf(stderr, "\n Query options:\n\n");
>  	fprintf(stderr, "\t-c\tlist connectors\n");
> @@ -1826,6 +1829,7 @@ static void usage(char *name)
>  	fprintf(stderr, "\t-s
> <connector_id>[,<connector_id>][@<crtc_id>]:<mode>[-<vrefresh>][@<format>]\
> tset a mode\n"); fprintf(stderr, "\t-C\ttest hw cursor\n");
>  	fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
> +	fprintf(stderr, "\t-r\tset the preferred mode\n");
>  	fprintf(stderr, "\t-w <obj_id>:<prop_name>:<value>\tset 
property\n");
>  	fprintf(stderr, "\t-a \tuse atomic API\n");
>  	fprintf(stderr, "\t-F pattern1,pattern2\tspecify fill 
patterns\n");
> @@ -1874,6 +1878,9 @@ static int pipe_resolve_connectors(struct device *dev,
> struct pipe_arg *pipe) char *endp;
> 
>  	for (i = 0; i < pipe->num_cons; i++) {
> +		/* If set_preferred is used, the connector is already 
resolved. */
> +		if (pipe->con_ids[i])
> +			continue;
>  		id = strtoul(pipe->cons[i], &endp, 10);
>  		if (endp == pipe->cons[i]) {
>  			connector = get_connector_by_name(dev, pipe-
>cons[i]);
> @@ -1885,14 +1892,62 @@ static int pipe_resolve_connectors(struct device
> *dev, struct pipe_arg *pipe)
> 
>  			id = connector->connector_id;
>  		}
> -
>  		pipe->con_ids[i] = id;
>  	}
> 
>  	return 0;
>  }
> 
> -static char optstr[] = "acdD:efF:M:P:ps:Cvw:";
> +static char optstr[] = "acdD:efF:M:P:ps:Cvrw:";
> +
> +static int pipe_find_preferred(struct device *dev, struct pipe_arg *pipe)
> +{
> +	drmModeRes *res = dev->resources->res;
> +	drmModeConnector *con = NULL;
> +	char *con_str;
> +	int i;
> +
> +	for (i = 0; i < res->count_connectors; i++) {
> +		con = drmModeGetConnector(dev->fd, res->connectors[i]);
> +		if (con->connection == DRM_MODE_CONNECTED)
> +			break;
> +		drmModeFreeConnector(con);
> +		con = NULL;
> +	}
> +
> +	if (!con) {
> +		printf("no connected connector!\n");
> +		return -1;
> +	}
> +
> +	con_str = malloc(8);
> +	sprintf(con_str, "%d", con->connector_id);
> +	strcpy(pipe->format_str, "XR24");
> +	pipe->fourcc = util_format_fourcc(pipe->format_str);
> +	pipe->num_cons = 1;
> +	pipe->con_ids = calloc(1, sizeof(*pipe->con_ids));
> +	pipe->cons = calloc(1, sizeof(*pipe->cons));
> +	pipe->con_ids[0] = con->connector_id;
> +	pipe->cons[0] = (const char*)con_str;
> +
> +	/* A CRTC possible will be chosen by pipe_find_crtc_and_mode. */
> +	pipe->crtc_id = (uint32_t)-1;
> +
> +	/* Return the first mode if no preferred. */
> +	pipe->mode = &con->modes[0];
> +	for (i = 0; i < con->count_modes; i++) {
> +		drmModeModeInfo *current_mode = &con->modes[i];
> +
> +		if (current_mode->type & DRM_MODE_TYPE_PREFERRED) {
> +			pipe->mode = current_mode;
> +			break;
> +		}
> +	}
> +
> +	sprintf(pipe->mode_str, "%dx%d", pipe->mode->hdisplay,
> pipe->mode->vdisplay); +
> +	return 0;
> +}
> 
>  int main(int argc, char **argv)
>  {
> @@ -1903,6 +1958,7 @@ int main(int argc, char **argv)
>  	int drop_master = 0;
>  	int test_vsync = 0;
>  	int test_cursor = 0;
> +	int set_preferred = 0;
>  	int use_atomic = 0;
>  	char *device = NULL;
>  	char *module = NULL;
> @@ -1987,6 +2043,9 @@ int main(int argc, char **argv)
>  		case 'v':
>  			test_vsync = 1;
>  			break;
> +		case 'r':
> +			set_preferred = 1;
> +			break;
>  		case 'w':
>  			prop_args = realloc(prop_args,
>  					   (prop_count + 1) * 
sizeof *prop_args);
> @@ -2008,7 +2067,7 @@ int main(int argc, char **argv)
>  	}
> 
>  	if (!args || (args == 1 && use_atomic))
> -		encoders = connectors = crtcs = planes = framebuffers = 
1;
> +		set_preferred = encoders = connectors = crtcs = planes = 
framebuffers =
> 1;
> 
>  	dev.fd = util_open(device, module);
>  	if (dev.fd < 0)
> @@ -2044,6 +2103,16 @@ int main(int argc, char **argv)
>  		return 1;
>  	}
> 
> +	if (set_preferred) {
> +		count = 1;
> +		pipe_args = calloc(1, sizeof(*pipe_args));
> +		ret = pipe_find_preferred(&dev, &pipe_args[0]);
> +		if (ret) {
> +			fprintf(stderr, "can't get preferred 
connector and mode: %s\n",
> strerror(errno)); +			return 1;
> +		}
> +	}
> +
>  	for (i = 0; i < count; i++) {
>  		if (pipe_resolve_connectors(&dev, &pipe_args[i]) < 0) {
>  			free_resources(dev.resources);

Reviewed-by: Rohan Garg <rohan.garg@collabora.com>

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

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

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

* Re: [PATCH libdrm 1/2] modetest: Fix segmentation fault
  2019-07-22 16:08 [PATCH libdrm 1/2] modetest: Fix segmentation fault Ezequiel Garcia
  2019-07-22 16:08 ` [PATCH libdrm 2/2] modetest: Add a new "-r" option to set a default mode Ezequiel Garcia
@ 2019-08-15 12:07 ` Ezequiel Garcia
  2019-09-18 22:08   ` Ezequiel Garcia
  2020-01-20 18:35 ` Ezequiel Garcia
  2 siblings, 1 reply; 7+ messages in thread
From: Ezequiel Garcia @ 2019-08-15 12:07 UTC (permalink / raw)
  To: dri-devel; +Cc: rohan.garg, Emil Velikov

On Mon, 2019-07-22 at 13:08 -0300, Ezequiel Garcia wrote:
> When a mode is set with just a connector "-s foo",
> we get a nasty segmentation fault. Fix it.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>

There's no rush, but still, here goes a reminder
of this and the next patch. :-)

> ---
>  tests/modetest/modetest.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index e66be6607e00..5e628127a130 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
> @@ -1695,6 +1695,8 @@ static int parse_connector(struct pipe_arg *pipe, const char *arg)
>  		return -1;
>  
>  	/* Parse the remaining parameters. */
> +	if (!endp)
> +		return -1;
>  	if (*endp == '@') {
>  		arg = endp + 1;
>  		pipe->crtc_id = strtoul(arg, &endp, 10);


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

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

* Re: [PATCH libdrm 1/2] modetest: Fix segmentation fault
  2019-08-15 12:07 ` [PATCH libdrm 1/2] modetest: Fix segmentation fault Ezequiel Garcia
@ 2019-09-18 22:08   ` Ezequiel Garcia
  0 siblings, 0 replies; 7+ messages in thread
From: Ezequiel Garcia @ 2019-09-18 22:08 UTC (permalink / raw)
  To: dri-devel; +Cc: rohan.garg, Emil Velikov

On Thu, 2019-08-15 at 09:07 -0300, Ezequiel Garcia wrote:
> On Mon, 2019-07-22 at 13:08 -0300, Ezequiel Garcia wrote:
> > When a mode is set with just a connector "-s foo",
> > we get a nasty segmentation fault. Fix it.
> > 
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> 
> There's no rush, but still, here goes a reminder
> of this and the next patch. :-)
> 

Still no rush, but a reminder to prevent these patches
from vanishing into thin air.

> > ---
> >  tests/modetest/modetest.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> > index e66be6607e00..5e628127a130 100644
> > --- a/tests/modetest/modetest.c
> > +++ b/tests/modetest/modetest.c
> > @@ -1695,6 +1695,8 @@ static int parse_connector(struct pipe_arg *pipe, const char *arg)
> >  		return -1;
> >  
> >  	/* Parse the remaining parameters. */
> > +	if (!endp)
> > +		return -1;
> >  	if (*endp == '@') {
> >  		arg = endp + 1;
> >  		pipe->crtc_id = strtoul(arg, &endp, 10);
> 
> 


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

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

* Re: [PATCH libdrm 1/2] modetest: Fix segmentation fault
  2019-07-22 16:08 [PATCH libdrm 1/2] modetest: Fix segmentation fault Ezequiel Garcia
  2019-07-22 16:08 ` [PATCH libdrm 2/2] modetest: Add a new "-r" option to set a default mode Ezequiel Garcia
  2019-08-15 12:07 ` [PATCH libdrm 1/2] modetest: Fix segmentation fault Ezequiel Garcia
@ 2020-01-20 18:35 ` Ezequiel Garcia
  2 siblings, 0 replies; 7+ messages in thread
From: Ezequiel Garcia @ 2020-01-20 18:35 UTC (permalink / raw)
  To: dri-devel; +Cc: rohan.garg, Emil Velikov

Hi everyone,

Any feedback here?

At least this one should be pretty straightforward
to merge, so I'm not sure it deserves a 6-month delay.

If anyone can take a look, I'd appreciate it.

Thanks!
Ezequiel

On Mon, 2019-07-22 at 13:08 -0300, Ezequiel Garcia wrote:
> When a mode is set with just a connector "-s foo",
> we get a nasty segmentation fault. Fix it.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  tests/modetest/modetest.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index e66be6607e00..5e628127a130 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
> @@ -1695,6 +1695,8 @@ static int parse_connector(struct pipe_arg *pipe, const char *arg)
>  		return -1;
>  
>  	/* Parse the remaining parameters. */
> +	if (!endp)
> +		return -1;
>  	if (*endp == '@') {
>  		arg = endp + 1;
>  		pipe->crtc_id = strtoul(arg, &endp, 10);


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

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

* Re: [PATCH libdrm 2/2] modetest: Add a new "-r" option to set a default mode
  2019-07-22 16:08 ` [PATCH libdrm 2/2] modetest: Add a new "-r" option to set a default mode Ezequiel Garcia
  2019-07-24 10:43   ` Rohan Garg
@ 2020-01-21 18:30   ` Emil Velikov
  1 sibling, 0 replies; 7+ messages in thread
From: Emil Velikov @ 2020-01-21 18:30 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: rohan.garg, ML dri-devel

Hi Ezequiel,

The first patch looks spot on. I'll commit it in a moment.

On Mon, 22 Jul 2019 at 17:08, Ezequiel Garcia <ezequiel@collabora.com> wrote:
>
> This option finds the first connected connector and then
> sets its preferred mode on it.
>
> Set this option to be set when no mode or plane is set
> explicitily. This allows to quickly test, in cases where
> one just needs something displayed.
>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  tests/modetest/modetest.c | 81 ++++++++++++++++++++++++++++++++++++---
>  1 file changed, 75 insertions(+), 6 deletions(-)
>
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index 5e628127a130..6042aaae7cca 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c

> @@ -1874,6 +1878,9 @@ static int pipe_resolve_connectors(struct device *dev, struct pipe_arg *pipe)
>         char *endp;
>
>         for (i = 0; i < pipe->num_cons; i++) {
> +               /* If set_preferred is used, the connector is already resolved. */
> +               if (pipe->con_ids[i])
> +                       continue;
This seems rather non-intuitive. How about we guard invocation of this
function altogether?
For example (minimal whitespace changes), in main() we can use:

if (!set_preferred && pipe_resolve_connectors()....)


> +static int pipe_find_preferred(struct device *dev, struct pipe_arg *pipe)
> +{
> +       drmModeRes *res = dev->resources->res;
> +       drmModeConnector *con = NULL;
> +       char *con_str;
> +       int i;
> +
> +       for (i = 0; i < res->count_connectors; i++) {
> +               con = drmModeGetConnector(dev->fd, res->connectors[i]);
> +               if (con->connection == DRM_MODE_CONNECTED)
> +                       break;
> +               drmModeFreeConnector(con);
> +               con = NULL;
> +       }
> +

> +
> +       /* A CRTC possible will be chosen by pipe_find_crtc_and_mode. */
> +       pipe->crtc_id = (uint32_t)-1;
> +
As-is this will pick the first connector, which may not work with all crtcs.

How about we tweak the loop above to pick a connector for the given crtc_id?
Feel free to do that as follow-up.


>  int main(int argc, char **argv)

>         int test_cursor = 0;
> +       int set_preferred = 0;
>         int use_atomic = 0;
>         char *device = NULL;
>         char *module = NULL;
> @@ -1987,6 +2043,9 @@ int main(int argc, char **argv)
>                 case 'v':
>                         test_vsync = 1;
>                         break;
> +               case 'r':
> +                       set_preferred = 1;
> +                       break;
>                 case 'w':
>                         prop_args = realloc(prop_args,
>                                            (prop_count + 1) * sizeof *prop_args);
> @@ -2008,7 +2067,7 @@ int main(int argc, char **argv)
>         }
>
>         if (!args || (args == 1 && use_atomic))
> -               encoders = connectors = crtcs = planes = framebuffers = 1;
> +               set_preferred = encoders = connectors = crtcs = planes = framebuffers = 1;
>
Did you mean to git add this? The variable set_preferred is already
set as needed.

Any reason why using atomic modeset (modetest -r -a), clears the
screen while legacy (modetest -r) sets the usual pattern. What am I
missing?

Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

end of thread, other threads:[~2020-01-21 18:29 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-07-22 16:08 [PATCH libdrm 1/2] modetest: Fix segmentation fault Ezequiel Garcia
2019-07-22 16:08 ` [PATCH libdrm 2/2] modetest: Add a new "-r" option to set a default mode Ezequiel Garcia
2019-07-24 10:43   ` Rohan Garg
2020-01-21 18:30   ` Emil Velikov
2019-08-15 12:07 ` [PATCH libdrm 1/2] modetest: Fix segmentation fault Ezequiel Garcia
2019-09-18 22:08   ` Ezequiel Garcia
2020-01-20 18:35 ` Ezequiel Garcia

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox