[PATCH libdrm] modetest: Allow the user to specify the plane ID

[PATCH libdrm] modetest: Allow the user to specify the plane ID

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Devices can have multiple planes, so allow the user to choose between
them.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tests/modetest/modetest.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index dedd28621ef8..c390d875e37b 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -704,6 +704,7 @@ struct pipe_arg {
 };
 
 struct plane_arg {
+	uint32_t plane_id;  /* the id of plane to use */
 	uint32_t crtc_id;  /* the id of CRTC to bind to */
 	bool has_position;
 	int32_t x, y;
@@ -958,7 +959,7 @@ static int set_plane(struct device *dev, struct plane_arg *p)
 {
 	drmModePlane *ovr;
 	uint32_t handles[4] = {0}, pitches[4] = {0}, offsets[4] = {0};
-	uint32_t plane_id = 0;
+	uint32_t plane_id;
 	struct bo *plane_bo;
 	uint32_t plane_flags = 0;
 	int crtc_x, crtc_y, crtc_w, crtc_h;
@@ -982,16 +983,26 @@ static int set_plane(struct device *dev, struct plane_arg *p)
 		return -1;
 	}
 
-	for (i = 0; i < dev->resources->plane_res->count_planes && !plane_id; i++) {
+	plane_id = p->plane_id;
+
+	for (i = 0; i < dev->resources->plane_res->count_planes; i++) {
 		ovr = dev->resources->planes[i].plane;
-		if (!ovr || !format_support(ovr, p->fourcc))
+		if (!ovr)
+			continue;
+
+		if (plane_id && plane_id != ovr->plane_id)
+			continue;
+
+		if (!format_support(ovr, p->fourcc))
 			continue;
 
-		if ((ovr->possible_crtcs & (1 << pipe)) && !ovr->crtc_id)
+		if ((ovr->possible_crtcs & (1 << pipe)) && !ovr->crtc_id) {
 			plane_id = ovr->plane_id;
+			break;
+		}
 	}
 
-	if (!plane_id) {
+	if (i == dev->resources->plane_res->count_planes) {
 		fprintf(stderr, "no unused plane available for CRTC %u\n",
 			crtc->crtc->crtc_id);
 		return -1;
@@ -1359,6 +1370,11 @@ static int parse_plane(struct plane_arg *plane, const char *p)
 {
 	char *end;
 
+	plane->plane_id = strtoul(p, &end, 10);
+	if (*end != '@')
+		return -EINVAL;
+
+	p = end + 1;
 	plane->crtc_id = strtoul(p, &end, 10);
 	if (*end != ':')
 		return -EINVAL;
@@ -1430,7 +1446,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 <crtc_id>:<w>x<h>[+<x>+<y>][*<scale>][@<format>]\tset a plane\n");
+	fprintf(stderr, "\t-P <plane_id>@<crtc_id>:<w>x<h>[+<x>+<y>][*<scale>][@<format>]\tset a plane\n");
 	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");
-- 
2.7.4

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

Re: [PATCH libdrm] modetest: Allow the user to specify the plane ID

[Emil Velikov]

On 28 September 2016 at 15:27,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Devices can have multiple planes, so allow the user to choose between
> them.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
In the long term I'm wondering if we don't want to nuke/deprecate the
clunky modetest and use a slimmed down atomic one.

Either way, the patch is great but we want to update modeprint to
provide plane(s) info. Otherwise one has no way of knowing which
plane_id to feed without explicit knowledge about the driver/hardware.

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

Re: [PATCH libdrm] modetest: Allow the user to specify the plane ID

[Ville Syrjälä]

On Thu, Oct 06, 2016 at 02:49:34PM +0100, Emil Velikov wrote:
> On 28 September 2016 at 15:27,  <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Devices can have multiple planes, so allow the user to choose between
> > them.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> In the long term I'm wondering if we don't want to nuke/deprecate the
> clunky modetest and use a slimmed down atomic one.
> 
> Either way, the patch is great but we want to update modeprint to
> provide plane(s) info. Otherwise one has no way of knowing which
> plane_id to feed without explicit knowledge about the driver/hardware.

There's a modeprint? I wonder how that differs from modetest's output?

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm] modetest: Allow the user to specify the plane ID

[Ville Syrjälä]

On Wed, Sep 28, 2016 at 05:27:25PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Devices can have multiple planes, so allow the user to choose between
> them.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Didn't get any objectioms, so I pushed this to master.

> ---
>  tests/modetest/modetest.c | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index dedd28621ef8..c390d875e37b 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
> @@ -704,6 +704,7 @@ struct pipe_arg {
>  };
>  
>  struct plane_arg {
> +	uint32_t plane_id;  /* the id of plane to use */
>  	uint32_t crtc_id;  /* the id of CRTC to bind to */
>  	bool has_position;
>  	int32_t x, y;
> @@ -958,7 +959,7 @@ static int set_plane(struct device *dev, struct plane_arg *p)
>  {
>  	drmModePlane *ovr;
>  	uint32_t handles[4] = {0}, pitches[4] = {0}, offsets[4] = {0};
> -	uint32_t plane_id = 0;
> +	uint32_t plane_id;
>  	struct bo *plane_bo;
>  	uint32_t plane_flags = 0;
>  	int crtc_x, crtc_y, crtc_w, crtc_h;
> @@ -982,16 +983,26 @@ static int set_plane(struct device *dev, struct plane_arg *p)
>  		return -1;
>  	}
>  
> -	for (i = 0; i < dev->resources->plane_res->count_planes && !plane_id; i++) {
> +	plane_id = p->plane_id;
> +
> +	for (i = 0; i < dev->resources->plane_res->count_planes; i++) {
>  		ovr = dev->resources->planes[i].plane;
> -		if (!ovr || !format_support(ovr, p->fourcc))
> +		if (!ovr)
> +			continue;
> +
> +		if (plane_id && plane_id != ovr->plane_id)
> +			continue;
> +
> +		if (!format_support(ovr, p->fourcc))
>  			continue;
>  
> -		if ((ovr->possible_crtcs & (1 << pipe)) && !ovr->crtc_id)
> +		if ((ovr->possible_crtcs & (1 << pipe)) && !ovr->crtc_id) {
>  			plane_id = ovr->plane_id;
> +			break;
> +		}
>  	}
>  
> -	if (!plane_id) {
> +	if (i == dev->resources->plane_res->count_planes) {
>  		fprintf(stderr, "no unused plane available for CRTC %u\n",
>  			crtc->crtc->crtc_id);
>  		return -1;
> @@ -1359,6 +1370,11 @@ static int parse_plane(struct plane_arg *plane, const char *p)
>  {
>  	char *end;
>  
> +	plane->plane_id = strtoul(p, &end, 10);
> +	if (*end != '@')
> +		return -EINVAL;
> +
> +	p = end + 1;
>  	plane->crtc_id = strtoul(p, &end, 10);
>  	if (*end != ':')
>  		return -EINVAL;
> @@ -1430,7 +1446,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 <crtc_id>:<w>x<h>[+<x>+<y>][*<scale>][@<format>]\tset a plane\n");
> +	fprintf(stderr, "\t-P <plane_id>@<crtc_id>:<w>x<h>[+<x>+<y>][*<scale>][@<format>]\tset a plane\n");
>  	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");
> -- 
> 2.7.4

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm] modetest: Allow the user to specify the plane ID

[Emil Velikov]

On 21 December 2016 at 09:48, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Wed, Sep 28, 2016 at 05:27:25PM +0300, ville.syrjala@linux.intel.com wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> Devices can have multiple planes, so allow the user to choose between
>> them.
>>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Didn't get any objectioms, so I pushed this to master.
>
Thanks I've completely forgot about this patch.

As you get a few minutes please check/add any missing functionality
from modeprint and nuke it ;-)

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