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

Message ID	1363704962-14077-18-git-send-email-laurent.pinchart@ideasonboard.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> To: dri-devel@lists.freedesktop.org Subject: [PATCH v4 17/21] modetest: Give the CRTC ID to the -P option Date: Tue, 19 Mar 2013 15:55:58 +0100 Message-Id: <1363704962-14077-18-git-send-email-laurent.pinchart@ideasonboard.com> In-Reply-To: <1363704962-14077-1-git-send-email-laurent.pinchart@ideasonboard.com> References: <1363704962-14077-1-git-send-email-laurent.pinchart@ideasonboard.com> Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org

Message ID

1363704962-14077-18-git-send-email-laurent.pinchart@ideasonboard.com (mailing list archive)

State

New, archived

Headers

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: dri-devel@lists.freedesktop.org
Subject: [PATCH v4 17/21] modetest: Give the CRTC ID to the -P option
Date: Tue, 19 Mar 2013 15:55:58 +0100
Message-Id: <1363704962-14077-18-git-send-email-laurent.pinchart@ideasonboard.com>
In-Reply-To: <1363704962-14077-1-git-send-email-laurent.pinchart@ideasonboard.com>
References: <1363704962-14077-1-git-send-email-laurent.pinchart@ideasonboard.com>
Precedence: list
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org
Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org

Commit Message

Laurent Pinchart March 19, 2013, 2:55 p.m. UTC

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(-)

Comments

Rob Clark March 19, 2013, 6:21 p.m. UTC | #1

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

Laurent Pinchart March 20, 2013, 3:01 p.m. UTC | #2

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 ?

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");

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

Commit Message

Comments

Patch