[PATCH i-g-t 00/43] Rotation test

[PATCH i-g-t 00/43] Rotation test

[Damien Lespiau]

I've taken the current rotation test to test the kernel patches and improved it
a bit along the way. It's a bit like a detailed review, but with patches
instead of comments.

With the rotation kernel patches and an IVB machine, that test now passes.

Part of the motivation has been to augment the igt_kms framework to deal with
properties.  Another reason include the number of lines we need to write for a
test. This series brings down that number from 325 to 129, something I can live
with.

HTH,

-- 
Damien

Damien Lespiau (42):
  igt_kms: Factor out a generic get_property() out of
    get_drm_plane_type()
  igt_kms: Make has_universal_planes a bitfield
  igt_kms: Provide a get_plane_property() shorthand
  igt_kms: Add a way to query of the plane supports rotation
  igt_kms: Add support for setting plane rotation
  igt_kms: Introduce a for_each_pipe() macro
  kms_rotation_crc: Make check_plane_type() static
  kms_rotation_crc: Make more functions static
  kms_rotation_crc: Align a few wrapped lines to the opening brace
  kms_rotation_crc: Update the copyright to have this year as well
  kms_rotation_crc: Test the validity of the output first
  kms_rotation_crc: Require universal planes for the testing primary
    rotation
  kms_rotation_crc: Fix style issue: '{' at the end of lines
  kms_rotation_crc: Fix style issue: single statement conditionals
  kms_rotation_crc: Factor out the square drawing function
  kms_rotation_crc: Factor out common primary/sprite code in
    prepare_crtc()
  kms_rotation_crc: Remove useless comments
  kms_rotation_crc: Use drm_plane from igt_plane_t
  kms_rotation_crc: Style issue: binary operators need spaces before and
    after
  kms_rotation_crc: Skip the tests if rotation is not supported
  kms_rotation_crc: Just store the igt_plane_t in data
  kms_rotation_crc: Unify the two tests
  kms_rotation_crc: Always disable the plane in cleanup
  kms_rotation_crc: Don't store rotate in the test state
  kms_rotation_crc: Don't store 'pipe' in the state
  kms_rotation_crc: Use igt_plane_set_rotation()
  kms_rotation_crc: Remove now unnecessary defines
  kms_rotation_crc: Use for_each_pipe()
  kms_rotation_crc: Remove the test on output->valid
  kms_rotation_crc: Remove 'output' from the state
  kms_rotation_crc: Remove the sleep(2)
  kms_rotation_crc: Remove plane from the state
  kms_rotation_crc: No need to test for NULL before freeing the pipe CRC
    object
  kms_rotation_crc: Allow the sprite test to run even without universal
    planes
  kms_rotation_crc: Don't commit with no fb set up
  kms_rotation_crc: Properly paint the whole frame buffer
  kms_rotation_crc: Add the test to .gitignore
  kms_rotation_crc: Don't compile the test on Android with no cairo
    support
  kms_rotation_crc: Document the two steps in prepare_crtc()
  kms_rotation_crc: Always use the primary plane to compute the
    reference CRC
  kms_rotation_crc: Remove unnecessary includes
  kms_rotation_crc: Use the igt_kms enum to encode the plane rotation

Sonika Jindal (1):
  tests/kms_rotation_crc: IGT for 180 degree HW rotation

 lib/igt_kms.c            | 114 ++++++++++++++++++++++---
 lib/igt_kms.h            |  26 +++++-
 tests/.gitignore         |   1 +
 tests/Android.mk         |   3 +-
 tests/Makefile.sources   |   1 +
 tests/kms_rotation_crc.c | 214 +++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 344 insertions(+), 15 deletions(-)
 create mode 100644 tests/kms_rotation_crc.c

-- 
1.8.3.1

[PATCH i-g-t 01/43] igt_kms: Factor out a generic get_property() out of get_drm_plane_type()

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 lib/igt_kms.c | 45 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 32 insertions(+), 13 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 4a8c394..666b0d0 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -495,37 +495,56 @@ static void igt_output_refresh(igt_output_t *output)
 	display->pipes_in_use |= 1 << output->config.pipe;
 }
 
-/*
- * Walk a plane's property list to determine its type.  If we don't
- * find a type property, then the kernel doesn't support universal
- * planes and we know the plane is an overlay/sprite.
- */
-static int get_drm_plane_type(igt_display_t *display, uint32_t plane_id)
+static bool
+get_property(igt_display_t *display,
+	     uint32_t object_id, uint32_t object_type, const char *name,
+	     uint32_t *prop_id /* out */, uint64_t *value /* out */)
 {
 	drmModeObjectPropertiesPtr proplist;
 	drmModePropertyPtr prop = NULL;
-	int type = DRM_PLANE_TYPE_OVERLAY;
+	bool found = false;
 	int i;
 
 	proplist = drmModeObjectGetProperties(display->drm_fd,
-					      plane_id,
-					      DRM_MODE_OBJECT_PLANE);
+					      object_id, object_type);
 	for (i = 0; i < proplist->count_props; i++) {
 		drmModeFreeProperty(prop);
 		prop = drmModeGetProperty(display->drm_fd, proplist->props[i]);
 		if (!prop)
 			continue;
 
-		if (strcmp(prop->name, "type") == 0) {
-			type = proplist->prop_values[i];
-			break;
+		if (strcmp(prop->name, name) == 0) {
+			found = true;
+			if (prop_id)
+				*prop_id = proplist->props[i];
+			if (value)
+				*value = proplist->prop_values[i];
+			goto out;
 		}
 	}
 
+out:
 	drmModeFreeProperty(prop);
 	drmModeFreeObjectProperties(proplist);
+	return found;
+}
+
+/*
+ * Walk a plane's property list to determine its type.  If we don't
+ * find a type property, then the kernel doesn't support universal
+ * planes and we know the plane is an overlay/sprite.
+ */
+static int get_drm_plane_type(igt_display_t *display, uint32_t plane_id)
+{
+	uint64_t value;
+	bool has_prop;
+
+	has_prop = get_property(display, plane_id, DRM_MODE_OBJECT_PLANE,
+				"type", NULL /* prop_id */, &value);
+	if (has_prop)
+		return (int)value;
 
-	return type;
+	return DRM_PLANE_TYPE_OVERLAY;
 }
 
 void igt_display_init(igt_display_t *display, int drm_fd)
-- 
1.8.3.1

[PATCH i-g-t 02/43] igt_kms: Make has_universal_planes a bitfield

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 lib/igt_kms.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index a079fc2..058114a 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -153,7 +153,7 @@ struct igt_display {
 	unsigned long pipes_in_use;
 	igt_output_t *outputs;
 	igt_pipe_t pipes[I915_MAX_PIPES];
-	bool has_universal_planes;
+	unsigned int has_universal_planes : 1;
 };
 
 /* set vt into graphics mode, required to prevent fbcon from interfering */
-- 
1.8.3.1

Re: [PATCH i-g-t 02/43] igt_kms: Make has_universal_planes a bitfield

[Daniel Vetter]

On Thu, Jul 10, 2014 at 07:00:03PM +0100, Damien Lespiau wrote:
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  lib/igt_kms.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index a079fc2..058114a 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -153,7 +153,7 @@ struct igt_display {
>  	unsigned long pipes_in_use;
>  	igt_output_t *outputs;
>  	igt_pipe_t pipes[I915_MAX_PIPES];
> -	bool has_universal_planes;
> +	unsigned int has_universal_planes : 1;

tbh I didn't see the point of this and didn't see any follow-up patch
which would explain the motivation ...
-Daniel

>  };
>  
>  /* set vt into graphics mode, required to prevent fbcon from interfering */
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH i-g-t 02/43] igt_kms: Make has_universal_planes a bitfield

[Damien Lespiau]

On Fri, Jul 11, 2014 at 08:34:41AM +0200, Daniel Vetter wrote:
> On Thu, Jul 10, 2014 at 07:00:03PM +0100, Damien Lespiau wrote:
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > ---
> >  lib/igt_kms.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> > index a079fc2..058114a 100644
> > --- a/lib/igt_kms.h
> > +++ b/lib/igt_kms.h
> > @@ -153,7 +153,7 @@ struct igt_display {
> >  	unsigned long pipes_in_use;
> >  	igt_output_t *outputs;
> >  	igt_pipe_t pipes[I915_MAX_PIPES];
> > -	bool has_universal_planes;
> > +	unsigned int has_universal_planes : 1;
> 
> tbh I didn't see the point of this and didn't see any follow-up patch
> which would explain the motivation ...

Fair, I dropped this one for the series.

-- 
Damien

[PATCH i-g-t 03/43] igt_kms: Provide a get_plane_property() shorthand

[Damien Lespiau]

So one doesn't have to write the plane type all the time.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 lib/igt_kms.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 666b0d0..5c8a3cc 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -529,6 +529,14 @@ out:
 	return found;
 }
 
+static bool
+get_plane_property(igt_display_t *display, uint32_t plane_id, const char *name,
+		   uint32_t *prop_id /* out */, uint64_t *value /* out */)
+{
+	return get_property(display, plane_id, DRM_MODE_OBJECT_PLANE,
+			    name, prop_id, value);
+}
+
 /*
  * Walk a plane's property list to determine its type.  If we don't
  * find a type property, then the kernel doesn't support universal
@@ -539,8 +547,8 @@ static int get_drm_plane_type(igt_display_t *display, uint32_t plane_id)
 	uint64_t value;
 	bool has_prop;
 
-	has_prop = get_property(display, plane_id, DRM_MODE_OBJECT_PLANE,
-				"type", NULL /* prop_id */, &value);
+	has_prop = get_plane_property(display, plane_id, "type",
+				      NULL /* prop_id */, &value);
 	if (has_prop)
 		return (int)value;
 
-- 
1.8.3.1

[PATCH i-g-t 04/43] igt_kms: Add a way to query of the plane supports rotation

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 lib/igt_kms.c |  7 +++++++
 lib/igt_kms.h | 10 ++++++++++
 2 files changed, 17 insertions(+)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 5c8a3cc..87f5109 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -592,6 +592,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
 		/* add the planes that can be used with that pipe */
 		for (j = 0; j < plane_resources->count_planes; j++) {
 			drmModePlane *drm_plane;
+			uint64_t prop_value;
 
 			drm_plane = drmModeGetPlane(display->drm_fd,
 						    plane_resources->planes[j]);
@@ -632,6 +633,12 @@ void igt_display_init(igt_display_t *display, int drm_fd)
 
 			plane->pipe = pipe;
 			plane->drm_plane = drm_plane;
+
+			get_plane_property(display, drm_plane->plane_id,
+					   "rotation",
+					   &plane->rotation_property,
+					   &prop_value);
+			plane->rotation = (igt_rotation_t)prop_value;
 		}
 
 		if (display->has_universal_planes) {
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 058114a..4f3474e 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -109,8 +109,10 @@ typedef struct {
 	/*< private >*/
 	igt_pipe_t *pipe;
 	int index;
+	/* capabilities */
 	unsigned int is_primary       : 1;
 	unsigned int is_cursor        : 1;
+	/* state tracking */
 	unsigned int fb_changed       : 1;
 	unsigned int position_changed : 1;
 	unsigned int panning_changed  : 1;
@@ -120,6 +122,9 @@ typedef struct {
 	 */
 	drmModePlane *drm_plane;
 	struct igt_fb *fb;
+
+	uint32_t rotation_property;
+
 	/* position within pipe_src_w x pipe_src_h */
 	int crtc_x, crtc_y;
 	/* panning offset within the fb */
@@ -171,6 +176,11 @@ drmModeModeInfo *igt_output_get_mode(igt_output_t *output);
 void igt_output_set_pipe(igt_output_t *output, enum pipe pipe);
 igt_plane_t *igt_output_get_plane(igt_output_t *output, enum igt_plane plane);
 
+static inline bool igt_plane_supports_rotation(igt_plane_t *plane)
+{
+	return plane->rotation_property != 0;
+}
+
 void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb);
 void igt_plane_set_position(igt_plane_t *plane, int x, int y);
 void igt_plane_set_panning(igt_plane_t *plane, int x, int y);
-- 
1.8.3.1

[PATCH i-g-t 05/43] igt_kms: Add support for setting plane rotation

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 lib/igt_kms.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_kms.h | 11 +++++++++++
 2 files changed, 65 insertions(+)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 87f5109..69f9977 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -537,6 +537,16 @@ get_plane_property(igt_display_t *display, uint32_t plane_id, const char *name,
 			    name, prop_id, value);
 }
 
+static void
+igt_plane_set_property(igt_plane_t *plane, uint32_t prop_id, uint64_t value)
+{
+	igt_pipe_t *pipe = plane->pipe;
+	igt_display_t *display = pipe->display;
+
+	drmModeObjectSetProperty(display->drm_fd, plane->drm_plane->plane_id,
+				 DRM_MODE_OBJECT_PLANE, prop_id, value);
+}
+
 /*
  * Walk a plane's property list to determine its type.  If we don't
  * find a type property, then the kernel doesn't support universal
@@ -882,6 +892,10 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
 
 	igt_assert(plane->drm_plane);
 
+	/* it's an error to try an unsupported feature */
+	igt_assert(igt_plane_supports_rotation(plane) ||
+		   !plane->rotation_changed);
+
 	fb_id = igt_plane_get_fb_id(plane);
 	crtc_id = output->config.crtc->crtc_id;
 
@@ -931,6 +945,14 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
 
 	plane->fb_changed = false;
 	plane->position_changed = false;
+
+	if (plane->rotation_changed) {
+		igt_plane_set_property(plane, plane->rotation_property,
+				       plane->rotation);
+
+		plane->rotation_changed = false;
+	}
+
 	return 0;
 }
 
@@ -1013,6 +1035,9 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
 	/* Primary planes can't be windowed when using a legacy commit */
 	igt_assert((primary->crtc_x == 0 && primary->crtc_y == 0));
 
+	/* nor rotated */
+	igt_assert(!primary->rotation_changed);
+
 	if (!primary->fb_changed && !primary->position_changed &&
 	    !primary->panning_changed)
 		return 0;
@@ -1304,6 +1329,35 @@ void igt_plane_set_panning(igt_plane_t *plane, int x, int y)
 	plane->panning_changed = true;
 }
 
+static const char *rotation_name(igt_rotation_t rotation)
+{
+	switch (rotation) {
+	case IGT_ROTATION_0:
+		return "0°";
+	case IGT_ROTATION_90:
+		return "90°";
+	case IGT_ROTATION_180:
+		return "180°";
+	case IGT_ROTATION_270:
+		return "270°";
+	default:
+		igt_assert(0);
+	}
+}
+
+void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation)
+{
+	igt_pipe_t *pipe = plane->pipe;
+	igt_display_t *display = pipe->display;
+
+	LOG(display, "%c.%d: plane_set_rotation(%s)\n", pipe_name(pipe->pipe),
+	    plane->index, rotation_name(rotation));
+
+	plane->rotation = rotation;
+
+	plane->rotation_changed = true;
+}
+
 void igt_wait_for_vblank(int drm_fd, enum pipe pipe)
 {
 	drmVBlank wait_vbl;
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 4f3474e..d34bcee 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -71,6 +71,14 @@ enum igt_commit_style {
 	/* We'll add atomic here eventually. */
 };
 
+typedef enum {
+	/* this maps to the kernel API */
+	IGT_ROTATION_0   = 1 << 0,
+	IGT_ROTATION_90  = 1 << 1,
+	IGT_ROTATION_180 = 1 << 2,
+	IGT_ROTATION_270 = 1 << 3,
+} igt_rotation_t;
+
 #include "igt_fb.h"
 
 struct kmstest_connector_config {
@@ -116,6 +124,7 @@ typedef struct {
 	unsigned int fb_changed       : 1;
 	unsigned int position_changed : 1;
 	unsigned int panning_changed  : 1;
+	unsigned int rotation_changed : 1;
 	/*
 	 * drm_plane can be NULL for primary and cursor planes (when not
 	 * using the atomic modeset API)
@@ -129,6 +138,7 @@ typedef struct {
 	int crtc_x, crtc_y;
 	/* panning offset within the fb */
 	unsigned int pan_x, pan_y;
+	igt_rotation_t rotation;
 } igt_plane_t;
 
 struct igt_pipe {
@@ -184,6 +194,7 @@ static inline bool igt_plane_supports_rotation(igt_plane_t *plane)
 void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb);
 void igt_plane_set_position(igt_plane_t *plane, int x, int y);
 void igt_plane_set_panning(igt_plane_t *plane, int x, int y);
+void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation);
 
 void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
 
-- 
1.8.3.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 05/43] igt_kms: Add support for setting plane rotation

[Daniel Vetter]

On Thu, Jul 10, 2014 at 07:00:06PM +0100, Damien Lespiau wrote:
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  lib/igt_kms.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_kms.h | 11 +++++++++++
>  2 files changed, 65 insertions(+)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 87f5109..69f9977 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -537,6 +537,16 @@ get_plane_property(igt_display_t *display, uint32_t plane_id, const char *name,
>  			    name, prop_id, value);
>  }
>  
> +static void
> +igt_plane_set_property(igt_plane_t *plane, uint32_t prop_id, uint64_t value)
> +{
> +	igt_pipe_t *pipe = plane->pipe;
> +	igt_display_t *display = pipe->display;
> +
> +	drmModeObjectSetProperty(display->drm_fd, plane->drm_plane->plane_id,
> +				 DRM_MODE_OBJECT_PLANE, prop_id, value);
> +}

I wonder whether we shouldn't have interfaces using the property names and
maybe for enum properties also the strings and let igt do it's thing. But
as long as this is just an internal function it should be good enough.
-Daniel

> +
>  /*
>   * Walk a plane's property list to determine its type.  If we don't
>   * find a type property, then the kernel doesn't support universal
> @@ -882,6 +892,10 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
>  
>  	igt_assert(plane->drm_plane);
>  
> +	/* it's an error to try an unsupported feature */
> +	igt_assert(igt_plane_supports_rotation(plane) ||
> +		   !plane->rotation_changed);
> +
>  	fb_id = igt_plane_get_fb_id(plane);
>  	crtc_id = output->config.crtc->crtc_id;
>  
> @@ -931,6 +945,14 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
>  
>  	plane->fb_changed = false;
>  	plane->position_changed = false;
> +
> +	if (plane->rotation_changed) {
> +		igt_plane_set_property(plane, plane->rotation_property,
> +				       plane->rotation);
> +
> +		plane->rotation_changed = false;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -1013,6 +1035,9 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
>  	/* Primary planes can't be windowed when using a legacy commit */
>  	igt_assert((primary->crtc_x == 0 && primary->crtc_y == 0));
>  
> +	/* nor rotated */
> +	igt_assert(!primary->rotation_changed);
> +
>  	if (!primary->fb_changed && !primary->position_changed &&
>  	    !primary->panning_changed)
>  		return 0;
> @@ -1304,6 +1329,35 @@ void igt_plane_set_panning(igt_plane_t *plane, int x, int y)
>  	plane->panning_changed = true;
>  }
>  
> +static const char *rotation_name(igt_rotation_t rotation)
> +{
> +	switch (rotation) {
> +	case IGT_ROTATION_0:
> +		return "0°";
> +	case IGT_ROTATION_90:
> +		return "90°";
> +	case IGT_ROTATION_180:
> +		return "180°";
> +	case IGT_ROTATION_270:
> +		return "270°";
> +	default:
> +		igt_assert(0);
> +	}
> +}
> +
> +void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation)
> +{
> +	igt_pipe_t *pipe = plane->pipe;
> +	igt_display_t *display = pipe->display;
> +
> +	LOG(display, "%c.%d: plane_set_rotation(%s)\n", pipe_name(pipe->pipe),
> +	    plane->index, rotation_name(rotation));
> +
> +	plane->rotation = rotation;
> +
> +	plane->rotation_changed = true;
> +}
> +
>  void igt_wait_for_vblank(int drm_fd, enum pipe pipe)
>  {
>  	drmVBlank wait_vbl;
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 4f3474e..d34bcee 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -71,6 +71,14 @@ enum igt_commit_style {
>  	/* We'll add atomic here eventually. */
>  };
>  
> +typedef enum {
> +	/* this maps to the kernel API */
> +	IGT_ROTATION_0   = 1 << 0,
> +	IGT_ROTATION_90  = 1 << 1,
> +	IGT_ROTATION_180 = 1 << 2,
> +	IGT_ROTATION_270 = 1 << 3,
> +} igt_rotation_t;
> +
>  #include "igt_fb.h"
>  
>  struct kmstest_connector_config {
> @@ -116,6 +124,7 @@ typedef struct {
>  	unsigned int fb_changed       : 1;
>  	unsigned int position_changed : 1;
>  	unsigned int panning_changed  : 1;
> +	unsigned int rotation_changed : 1;
>  	/*
>  	 * drm_plane can be NULL for primary and cursor planes (when not
>  	 * using the atomic modeset API)
> @@ -129,6 +138,7 @@ typedef struct {
>  	int crtc_x, crtc_y;
>  	/* panning offset within the fb */
>  	unsigned int pan_x, pan_y;
> +	igt_rotation_t rotation;
>  } igt_plane_t;
>  
>  struct igt_pipe {
> @@ -184,6 +194,7 @@ static inline bool igt_plane_supports_rotation(igt_plane_t *plane)
>  void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb);
>  void igt_plane_set_position(igt_plane_t *plane, int x, int y);
>  void igt_plane_set_panning(igt_plane_t *plane, int x, int y);
> +void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation);
>  
>  void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH i-g-t 05/43] igt_kms: Add support for setting plane rotation

[Damien Lespiau]

On Fri, Jul 11, 2014 at 08:37:42AM +0200, Daniel Vetter wrote:
> On Thu, Jul 10, 2014 at 07:00:06PM +0100, Damien Lespiau wrote:
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > ---
> >  lib/igt_kms.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  lib/igt_kms.h | 11 +++++++++++
> >  2 files changed, 65 insertions(+)
> > 
> > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > index 87f5109..69f9977 100644
> > --- a/lib/igt_kms.c
> > +++ b/lib/igt_kms.c
> > @@ -537,6 +537,16 @@ get_plane_property(igt_display_t *display, uint32_t plane_id, const char *name,
> >  			    name, prop_id, value);
> >  }
> >  
> > +static void
> > +igt_plane_set_property(igt_plane_t *plane, uint32_t prop_id, uint64_t value)
> > +{
> > +	igt_pipe_t *pipe = plane->pipe;
> > +	igt_display_t *display = pipe->display;
> > +
> > +	drmModeObjectSetProperty(display->drm_fd, plane->drm_plane->plane_id,
> > +				 DRM_MODE_OBJECT_PLANE, prop_id, value);
> > +}
> 
> I wonder whether we shouldn't have interfaces using the property names and
> maybe for enum properties also the strings and let igt do it's thing. But
> as long as this is just an internal function it should be good enough.

That'd be handy. the only issue is that you'd have to do a lookup of the
property id each time or create a prop_name -> prop_id map in the
different objects. Also, we can provide a bit more typing than a
uint64_t for the value. Trade-offs, trade-offs, ...

-- 
Damien

Re: [PATCH i-g-t 05/43] igt_kms: Add support for setting plane rotation

[Daniel Vetter]

On Thu, Jul 10, 2014 at 07:00:06PM +0100, Damien Lespiau wrote:
> +typedef enum {
> +	/* this maps to the kernel API */
> +	IGT_ROTATION_0   = 1 << 0,
> +	IGT_ROTATION_90  = 1 << 1,
> +	IGT_ROTATION_180 = 1 << 2,
> +	IGT_ROTATION_270 = 1 << 3,
> +} igt_rotation_t;

Should we also add the flip X/Y bits, even if we currently don't support
this in the kernel?
-Daniel

> +
>  #include "igt_fb.h"
>  
>  struct kmstest_connector_config {
> @@ -116,6 +124,7 @@ typedef struct {
>  	unsigned int fb_changed       : 1;
>  	unsigned int position_changed : 1;
>  	unsigned int panning_changed  : 1;
> +	unsigned int rotation_changed : 1;
>  	/*
>  	 * drm_plane can be NULL for primary and cursor planes (when not
>  	 * using the atomic modeset API)
> @@ -129,6 +138,7 @@ typedef struct {
>  	int crtc_x, crtc_y;
>  	/* panning offset within the fb */
>  	unsigned int pan_x, pan_y;
> +	igt_rotation_t rotation;
>  } igt_plane_t;
>  
>  struct igt_pipe {
> @@ -184,6 +194,7 @@ static inline bool igt_plane_supports_rotation(igt_plane_t *plane)
>  void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb);
>  void igt_plane_set_position(igt_plane_t *plane, int x, int y);
>  void igt_plane_set_panning(igt_plane_t *plane, int x, int y);
> +void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation);
>  
>  void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH i-g-t 05/43] igt_kms: Add support for setting plane rotation

[Chris Wilson]

On Fri, Jul 11, 2014 at 08:40:29AM +0200, Daniel Vetter wrote:
> On Thu, Jul 10, 2014 at 07:00:06PM +0100, Damien Lespiau wrote:
> > +typedef enum {
> > +	/* this maps to the kernel API */
> > +	IGT_ROTATION_0   = 1 << 0,
> > +	IGT_ROTATION_90  = 1 << 1,
> > +	IGT_ROTATION_180 = 1 << 2,
> > +	IGT_ROTATION_270 = 1 << 3,
> > +} igt_rotation_t;
> 
> Should we also add the flip X/Y bits, even if we currently don't support
> this in the kernel?

Presumably there are tests in here to check the kernel rejects
unsupported combination of rotations and unknown reflections? In which,
yes.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH i-g-t 05/43] igt_kms: Add support for setting plane rotation

[Daniel Vetter]

On Fri, Jul 11, 2014 at 07:42:56AM +0100, Chris Wilson wrote:
> On Fri, Jul 11, 2014 at 08:40:29AM +0200, Daniel Vetter wrote:
> > On Thu, Jul 10, 2014 at 07:00:06PM +0100, Damien Lespiau wrote:
> > > +typedef enum {
> > > +	/* this maps to the kernel API */
> > > +	IGT_ROTATION_0   = 1 << 0,
> > > +	IGT_ROTATION_90  = 1 << 1,
> > > +	IGT_ROTATION_180 = 1 << 2,
> > > +	IGT_ROTATION_270 = 1 << 3,
> > > +} igt_rotation_t;
> > 
> > Should we also add the flip X/Y bits, even if we currently don't support
> > this in the kernel?
> 
> Presumably there are tests in here to check the kernel rejects
> unsupported combination of rotations and unknown reflections? In which,
> yes.

I guess we should try everything and check whether it either gets rejected
or works properly ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH i-g-t 06/43] igt_kms: Introduce a for_each_pipe() macro

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 lib/igt_kms.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index d34bcee..9e7bc2b 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -202,6 +202,9 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
 	for (int i__ = 0;  i__ < (display)->n_outputs; i__++)	\
 		if ((output = &(display)->outputs[i__]), output->valid)
 
+#define for_each_pipe(display, pipe)					\
+	for (pipe = 0; pipe < igt_display_get_n_pipes(display); pipe++)	\
+
 /*
  * Can be used with igt_output_set_pipe() to mean we don't care about the pipe
  * that should drive this output
-- 
1.8.3.1

Re: [PATCH i-g-t 06/43] igt_kms: Introduce a for_each_pipe() macro

[Daniel Vetter]

On Thu, Jul 10, 2014 at 07:00:07PM +0100, Damien Lespiau wrote:
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  lib/igt_kms.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index d34bcee..9e7bc2b 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -202,6 +202,9 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>  	for (int i__ = 0;  i__ < (display)->n_outputs; i__++)	\
>  		if ((output = &(display)->outputs[i__]), output->valid)
>  
> +#define for_each_pipe(display, pipe)					\
> +	for (pipe = 0; pipe < igt_display_get_n_pipes(display); pipe++)	\
> +

An additional hunk for lib/igt.cocci would be awesome, especially since it
will match a few existing things already.
-Daniel

>  /*
>   * Can be used with igt_output_set_pipe() to mean we don't care about the pipe
>   * that should drive this output
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH i-g-t 07/43] tests/kms_rotation_crc: IGT for 180 degree HW rotation

[Damien Lespiau]

From: Sonika Jindal <sonika.jindal@intel.com>

Testcase for 180 degree HW rotation

Cc: sagar.a.kamble@intel.com

Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
 tests/Makefile.sources   |   1 +
 tests/kms_rotation_crc.c | 427 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 428 insertions(+)
 create mode 100644 tests/kms_rotation_crc.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 1ebbac5..cb8c3d4 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -71,6 +71,7 @@ TESTS_progs_M = \
 	kms_plane \
 	kms_psr_sink_crc \
 	kms_render \
+	kms_rotation_crc \
 	kms_setmode \
 	kms_universal_plane \
 	pm_lpsp \
diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
new file mode 100644
index 0000000..74a52cd
--- /dev/null
+++ b/tests/kms_rotation_crc.c
@@ -0,0 +1,427 @@
+/*
+ * Copyright © 2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include <errno.h>
+#include <limits.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <stdint.h>
+#include <stddef.h>
+#include <fcntl.h>
+
+#include "drmtest.h"
+#include "igt_debugfs.h"
+#include "igt_kms.h"
+#include "igt_core.h"
+
+#define DRM_ROTATE_0   0
+#define DRM_ROTATE_90  1
+#define DRM_ROTATE_180 2
+#define DRM_ROTATE_270 3
+#define DRM_REFLECT_X  4
+#define DRM_REFLECT_Y  5
+#define DRM_ROTATE_NUM	6
+
+#define BIT(x)	(1 << x)
+
+// This will be part of libdrm later. Adding here temporarily
+#define DRM_PLANE_TYPE_OVERLAY 0
+#define DRM_PLANE_TYPE_PRIMARY 1
+#define DRM_PLANE_TYPE_CURSOR  2
+#define DRM_CLIENT_CAP_EXPOSE_PRIMARY_PLANES 2
+
+typedef struct {
+	int gfx_fd;
+	igt_display_t display;
+	igt_output_t *output;
+	int type;
+	int pipe;
+	struct igt_fb fb;
+	igt_crc_t ref_crc[2];
+	igt_pipe_crc_t *pipe_crc;
+	int rotate;
+} data_t;
+
+bool check_plane_type(int drm_fd, uint32_t plane_id, uint32_t type);
+int set_plane_property(data_t *data, int plane_id, const char *prop_name, int
+		val, igt_crc_t *crc_output);
+void test_sprite_rotation(data_t *data);
+void test_primary_rotation(data_t *data);
+bool prepare_crtc(data_t *data);
+
+bool prepare_crtc(data_t *data)
+{
+	drmModeModeInfo *mode;
+	igt_display_t *display = &data->display;
+	igt_output_t *output = data->output;
+	cairo_t *cr;
+	igt_plane_t *primary, *sprite;
+	int fb_id;
+	int w, h;
+
+	igt_output_set_pipe(output, data->pipe);
+	igt_display_commit(display);
+
+	/* create the pipe_crc object for this pipe */
+	if (data->pipe_crc)
+		igt_pipe_crc_free(data->pipe_crc);
+
+	data->pipe_crc = igt_pipe_crc_new(data->pipe,
+			INTEL_PIPE_CRC_SOURCE_AUTO);
+	if (!data->pipe_crc) {
+		igt_info("auto crc not supported on this connector with pipe %i\n",
+				data->pipe);
+		return false;
+	}
+
+
+	if (!data->output->valid) {
+		igt_output_set_pipe(output, PIPE_ANY);
+		igt_display_commit(display);
+		return false;
+	}
+
+	switch (data->type) {
+
+		case DRM_PLANE_TYPE_OVERLAY: /* Sprite */
+			igt_info("Sprite plane\n");
+			mode = igt_output_get_mode(output);
+			w = mode->hdisplay;
+			h = mode->vdisplay;
+
+			fb_id = igt_create_fb(data->gfx_fd,
+					mode->hdisplay, mode->vdisplay,
+					DRM_FORMAT_XRGB8888,
+					false, /* tiled */
+					&data->fb);
+			igt_assert(fb_id);
+			cr = igt_get_cairo_ctx(data->gfx_fd, &data->fb);
+
+			/* Paint rotated image of 4 colors */
+			igt_paint_color(cr, (w/2)-1, (h/2)-1, w/2, h/2, 1.0, 0.0, 0.0);
+			igt_paint_color(cr, 0, (h/2)-1, w/2, h/2, 0.0, 1.0, 0.0);
+			igt_paint_color(cr, (w/2)-1, 0, w/2, h/2, 0.0, 0.0, 1.0);
+			igt_paint_color(cr, 0, 0, w/2, h/2, 1.0, 1.0, 1.0);
+			sprite = igt_output_get_plane(output, IGT_PLANE_2);
+			igt_plane_set_fb(sprite, &data->fb);
+			igt_display_commit(display);
+
+			/* Collect reference crc */
+			igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc[1]);
+
+			/* Paint with 4 squares of Red, Green, White, Blue Clockwise */
+			igt_paint_color(cr, 0, 0, w/2, h/2, 1.0, 0.0, 0.0);
+			igt_paint_color(cr, (w/2)-1, 0, w/2, h/2, 0.0, 1.0, 0.0);
+			igt_paint_color(cr, 0, (h/2)-1, w/2, h/2, 0.0, 0.0, 1.0);
+			igt_paint_color(cr, (w/2)-1, (h/2)-1, w/2, h/2, 1.0, 1.0, 1.0);
+			cairo_destroy(cr);
+			sprite = igt_output_get_plane(output, IGT_PLANE_2);
+			igt_plane_set_fb(sprite, &data->fb);
+			igt_display_commit(display);
+
+			break;
+		case DRM_PLANE_TYPE_PRIMARY: /* Primary */
+			igt_info("Primary plane\n");
+			mode = igt_output_get_mode(output);
+			w = mode->hdisplay;
+			h = mode->vdisplay;
+
+			fb_id = igt_create_fb(data->gfx_fd,
+					mode->hdisplay, mode->vdisplay,
+					DRM_FORMAT_XRGB8888,
+					false, /* tiled */
+					&data->fb);
+			igt_assert(fb_id);
+			cr = igt_get_cairo_ctx(data->gfx_fd, &data->fb);
+
+			/* Paint rotated image of 4 colors */
+			igt_paint_color(cr, (w/2)-1, (h/2)-1, w/2, h/2, 1.0, 0.2, 0.2);
+			igt_paint_color(cr, 0, (h/2)-1, w/2, h/2, 0.2, 1.0, 0.2);
+			igt_paint_color(cr, (w/2)-1, 0, w/2, h/2, 0.2, 0.2, 1.0);
+			igt_paint_color(cr, 0, 0, w/2, h/2, 0.8, 0.8, 0.8);
+			primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+			igt_plane_set_fb(primary, &data->fb);
+			igt_display_commit(display);
+
+			/* Collect reference crc */
+			igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc[0]);
+
+			/* Paint with 4 squares of Red, Green, White, Blue Clockwise */
+			igt_paint_color(cr, 0, 0, w/2, h/2, 1.0, 0.2, 0.2);
+			igt_paint_color(cr, (w/2)-1, 0, w/2, h/2, 0.2, 1.0, 0.2);
+			igt_paint_color(cr, 0, (h/2)-1, w/2, h/2, 0.2, 0.2, 1.0);
+			igt_paint_color(cr, (w/2)-1, (h/2)-1, w/2, h/2, 0.8, 0.8, 0.8);
+			cairo_destroy(cr);
+			primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+			igt_plane_set_fb(primary, &data->fb);
+			igt_display_commit(display);
+
+			break;
+
+
+	}
+
+	return true;
+}
+
+static int connector_find_plane(int gfx_fd, uint32_t pipe, uint32_t type)
+{
+	drmModePlaneRes *plane_resources;
+	drmModePlane *ovr;
+	uint32_t id = 0;
+	int i, ret = 0;
+
+	ret = drmSetClientCap(gfx_fd, DRM_CLIENT_CAP_EXPOSE_PRIMARY_PLANES, 1);
+	if (ret < 0) {
+		igt_info("Failed to set client cap:%d\n", ret);
+		return 0;
+	}
+
+	plane_resources = drmModeGetPlaneResources(gfx_fd);
+	if (!plane_resources) {
+		igt_info("drmModeGetPlaneResources failed: %s\n",
+				strerror(errno));
+		return 0;
+	}
+
+	for (i = 0; i < plane_resources->count_planes; i++) {
+		ovr = drmModeGetPlane(gfx_fd, plane_resources->planes[i]);
+		if (!ovr) {
+			igt_info("drmModeGetPlane failed: %s\n",
+					strerror(errno));
+			continue;
+		}
+
+		if (ovr->possible_crtcs & (1 << pipe)) {
+			id = ovr->plane_id;
+			if (check_plane_type(gfx_fd, id, type)) {
+				drmModeFreePlane(ovr);
+				return id;
+			}
+		}
+		drmModeFreePlane(ovr);
+	}
+
+	return 0;
+}
+
+bool check_plane_type(int drm_fd, uint32_t plane_id, uint32_t type)
+{
+	int i = 0;
+	drmModeObjectPropertiesPtr props = NULL;
+
+	props = drmModeObjectGetProperties(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE);
+
+	for (i = 0; i < props->count_props; i++)
+	{
+		drmModePropertyPtr prop = drmModeGetProperty(drm_fd, props->props[i]);
+
+		if (strcmp(prop->name, "type") == 0)
+		{
+			if (props->prop_values[i] == type) {
+				return true;
+			}
+			igt_info("Didn't find the requested type:%u\n", (unsigned int)props->prop_values[i]);
+		}
+	}
+	return false;
+}
+
+int set_plane_property(data_t *data, int plane_id, const char *prop_name, int
+		val, igt_crc_t *crc_output)
+{
+	int i = 0, ret = 0;
+	int drm_fd = data->gfx_fd;
+	uint64_t value;
+	drmModeObjectPropertiesPtr props = NULL;
+
+	value = (uint64_t)val;
+	props = drmModeObjectGetProperties(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE);
+
+	for (i = 0; i < props->count_props; i++)
+	{
+		drmModePropertyPtr prop = drmModeGetProperty(drm_fd, props->props[i]);
+		igt_info("\nProp->name=%s: plane_id:%d\n ", prop->name,	plane_id);
+
+		if (strcmp(prop->name, prop_name) == 0)
+		{
+			ret = drmModeObjectSetProperty(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE,
+					(uint32_t)prop->prop_id, value);
+			if (ret)
+			{
+				igt_info("set_property \"%s\" to %d for plane %d is failed,	err:%d\n", prop_name, val, plane_id, ret);
+				drmModeFreeProperty(prop);
+				drmModeFreeObjectProperties(props);
+				return ret;
+			}
+			else {
+				/* Collect crc after rotation */
+				igt_pipe_crc_collect_crc(data->pipe_crc, crc_output);
+				drmModeFreeProperty(prop);
+				break;
+			}
+		}
+		drmModeFreeProperty(prop);
+	}
+	drmModeFreeObjectProperties(props);
+	return 0;
+}
+
+static void cleanup_crtc(data_t *data, igt_output_t *output)
+{
+	igt_display_t *display = &data->display;
+	igt_plane_t *plane = NULL;
+
+	igt_pipe_crc_free(data->pipe_crc);
+	data->pipe_crc = NULL;
+
+	igt_remove_fb(data->gfx_fd, &data->fb);
+
+	if (data->type == DRM_PLANE_TYPE_PRIMARY) {
+		plane = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+	}
+	else if (data->type == DRM_PLANE_TYPE_OVERLAY) {
+		plane = igt_output_get_plane(output, IGT_PLANE_2);
+	}
+
+	if (plane != NULL)
+		igt_plane_set_fb(plane, NULL);
+
+	igt_output_set_pipe(output, PIPE_ANY);
+	igt_display_commit(display);
+}
+
+void test_sprite_rotation(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	igt_output_t *output;
+	igt_plane_t *sprite;
+	igt_crc_t crc_output;
+	int p;
+	int plane_id;
+	int ret;
+	int valid_tests = 0;
+
+	for_each_connected_output(display, output) {
+		data->output = output;
+		for (p = 0; p < igt_display_get_n_pipes(display); p++) {
+			data->pipe = p;
+			data->type = 0;
+			data->rotate = DRM_ROTATE_180;
+
+			if (!prepare_crtc(data))
+				continue;
+			sleep(2);
+
+			sprite = igt_output_get_plane(output, IGT_PLANE_2);
+			plane_id = sprite->drm_plane->plane_id;
+			if (plane_id != 0) {
+				igt_info("Setting rotation property for plane:%d\n", plane_id);
+				ret = set_plane_property(data, plane_id, "rotation", BIT(data->rotate), &crc_output);
+				if (ret < 0) {
+					igt_info("Setting rotation failed!");
+					return;
+				}
+			}
+			igt_assert(igt_crc_equal(&data->ref_crc[1], &crc_output));
+			sleep(2);
+			valid_tests++;
+			cleanup_crtc(data, output);
+		}
+	}
+	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
+}
+
+
+void test_primary_rotation(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	igt_output_t *output;
+	int p;
+	int plane_id;
+	int ret;
+	int valid_tests = 0;
+	igt_crc_t crc_output;
+
+	for_each_connected_output(display, output) {
+		data->output = output;
+		for (p = 0; p < igt_display_get_n_pipes(display); p++) {
+			data->pipe = p;
+			data->type = 1;
+			data->rotate = DRM_ROTATE_180;
+
+			if (!prepare_crtc(data))
+				continue;
+			sleep(2);
+
+			/* Find primary plane. Currently igt_plane_t returned from
+			 * igt_output_get_plane has NULL drm_plane which is needed to get
+			 * the plane_id. So finding the primary plane by checking the "type"
+			 * property of the plane */
+			plane_id = connector_find_plane(data->gfx_fd, data->pipe, data->type);
+			if (plane_id != 0) {
+				igt_info("Setting rotation property for plane:%d\n", plane_id);
+				ret = set_plane_property(data, plane_id, "rotation", BIT(data->rotate), &crc_output);
+				if (ret < 0) {
+					igt_info("Setting rotation failed!");
+					return;
+				}
+			}
+			igt_assert(igt_crc_equal(&data->ref_crc[0], &crc_output));
+			sleep(2);
+			valid_tests++;
+			cleanup_crtc(data, output);
+		}
+	}
+	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
+}
+
+igt_main
+{
+	data_t data = {};
+
+	igt_skip_on_simulation();
+
+	igt_fixture {
+		data.gfx_fd = drm_open_any();
+
+		igt_set_vt_graphics_mode();
+
+		igt_require_pipe_crc();
+
+		igt_display_init(&data.display, data.gfx_fd);
+	}
+
+	igt_subtest_f("primary-rotation")
+		test_primary_rotation(&data);
+
+	igt_subtest_f("sprite-rotation")
+		test_sprite_rotation(&data);
+
+	igt_fixture {
+		igt_display_fini(&data.display);
+	}
+}
-- 
1.8.3.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 08/43] kms_rotation_crc: Make check_plane_type() static

[Damien Lespiau]

Clearly, someone tried to solve the following warning:
  kms_rotation_crc.c:189:6: warning: no previous prototype for ‘check_plane_type’ [-Wmissing-prototypes]

Without really understanding what was the warning about. Make
check_plane_type() static and move it before its user to get rid of the forward
declaration.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 45 ++++++++++++++++++++++-----------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 74a52cd..6249e0f 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -65,7 +65,6 @@ typedef struct {
 	int rotate;
 } data_t;
 
-bool check_plane_type(int drm_fd, uint32_t plane_id, uint32_t type);
 int set_plane_property(data_t *data, int plane_id, const char *prop_name, int
 		val, igt_crc_t *crc_output);
 void test_sprite_rotation(data_t *data);
@@ -187,6 +186,28 @@ bool prepare_crtc(data_t *data)
 	return true;
 }
 
+static bool check_plane_type(int drm_fd, uint32_t plane_id, uint32_t type)
+{
+	int i = 0;
+	drmModeObjectPropertiesPtr props = NULL;
+
+	props = drmModeObjectGetProperties(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE);
+
+	for (i = 0; i < props->count_props; i++)
+	{
+		drmModePropertyPtr prop = drmModeGetProperty(drm_fd, props->props[i]);
+
+		if (strcmp(prop->name, "type") == 0)
+		{
+			if (props->prop_values[i] == type) {
+				return true;
+			}
+			igt_info("Didn't find the requested type:%u\n", (unsigned int)props->prop_values[i]);
+		}
+	}
+	return false;
+}
+
 static int connector_find_plane(int gfx_fd, uint32_t pipe, uint32_t type)
 {
 	drmModePlaneRes *plane_resources;
@@ -228,28 +249,6 @@ static int connector_find_plane(int gfx_fd, uint32_t pipe, uint32_t type)
 	return 0;
 }
 
-bool check_plane_type(int drm_fd, uint32_t plane_id, uint32_t type)
-{
-	int i = 0;
-	drmModeObjectPropertiesPtr props = NULL;
-
-	props = drmModeObjectGetProperties(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE);
-
-	for (i = 0; i < props->count_props; i++)
-	{
-		drmModePropertyPtr prop = drmModeGetProperty(drm_fd, props->props[i]);
-
-		if (strcmp(prop->name, "type") == 0)
-		{
-			if (props->prop_values[i] == type) {
-				return true;
-			}
-			igt_info("Didn't find the requested type:%u\n", (unsigned int)props->prop_values[i]);
-		}
-	}
-	return false;
-}
-
 int set_plane_property(data_t *data, int plane_id, const char *prop_name, int
 		val, igt_crc_t *crc_output)
 {
-- 
1.8.3.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 09/43] kms_rotation_crc: Make more functions static

[Damien Lespiau]

More of the same. This time no need to move code around, just adding
static.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 6249e0f..b079376 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -65,13 +65,7 @@ typedef struct {
 	int rotate;
 } data_t;
 
-int set_plane_property(data_t *data, int plane_id, const char *prop_name, int
-		val, igt_crc_t *crc_output);
-void test_sprite_rotation(data_t *data);
-void test_primary_rotation(data_t *data);
-bool prepare_crtc(data_t *data);
-
-bool prepare_crtc(data_t *data)
+static bool prepare_crtc(data_t *data)
 {
 	drmModeModeInfo *mode;
 	igt_display_t *display = &data->display;
@@ -249,7 +243,7 @@ static int connector_find_plane(int gfx_fd, uint32_t pipe, uint32_t type)
 	return 0;
 }
 
-int set_plane_property(data_t *data, int plane_id, const char *prop_name, int
+static int set_plane_property(data_t *data, int plane_id, const char *prop_name, int
 		val, igt_crc_t *crc_output)
 {
 	int i = 0, ret = 0;
@@ -313,7 +307,7 @@ static void cleanup_crtc(data_t *data, igt_output_t *output)
 	igt_display_commit(display);
 }
 
-void test_sprite_rotation(data_t *data)
+static void test_sprite_rotation(data_t *data)
 {
 	igt_display_t *display = &data->display;
 	igt_output_t *output;
@@ -355,7 +349,7 @@ void test_sprite_rotation(data_t *data)
 }
 
 
-void test_primary_rotation(data_t *data)
+static void test_primary_rotation(data_t *data)
 {
 	igt_display_t *display = &data->display;
 	igt_output_t *output;
-- 
1.8.3.1

[PATCH i-g-t 10/43] kms_rotation_crc: Align a few wrapped lines to the opening brace

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index b079376..485a6df 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -83,10 +83,10 @@ static bool prepare_crtc(data_t *data)
 		igt_pipe_crc_free(data->pipe_crc);
 
 	data->pipe_crc = igt_pipe_crc_new(data->pipe,
-			INTEL_PIPE_CRC_SOURCE_AUTO);
+					  INTEL_PIPE_CRC_SOURCE_AUTO);
 	if (!data->pipe_crc) {
 		igt_info("auto crc not supported on this connector with pipe %i\n",
-				data->pipe);
+			 data->pipe);
 		return false;
 	}
 
@@ -143,10 +143,10 @@ static bool prepare_crtc(data_t *data)
 			h = mode->vdisplay;
 
 			fb_id = igt_create_fb(data->gfx_fd,
-					mode->hdisplay, mode->vdisplay,
-					DRM_FORMAT_XRGB8888,
-					false, /* tiled */
-					&data->fb);
+					      mode->hdisplay, mode->vdisplay,
+					      DRM_FORMAT_XRGB8888,
+					      false, /* tiled */
+					      &data->fb);
 			igt_assert(fb_id);
 			cr = igt_get_cairo_ctx(data->gfx_fd, &data->fb);
 
@@ -218,7 +218,7 @@ static int connector_find_plane(int gfx_fd, uint32_t pipe, uint32_t type)
 	plane_resources = drmModeGetPlaneResources(gfx_fd);
 	if (!plane_resources) {
 		igt_info("drmModeGetPlaneResources failed: %s\n",
-				strerror(errno));
+			 strerror(errno));
 		return 0;
 	}
 
@@ -226,7 +226,7 @@ static int connector_find_plane(int gfx_fd, uint32_t pipe, uint32_t type)
 		ovr = drmModeGetPlane(gfx_fd, plane_resources->planes[i]);
 		if (!ovr) {
 			igt_info("drmModeGetPlane failed: %s\n",
-					strerror(errno));
+				 strerror(errno));
 			continue;
 		}
 
-- 
1.8.3.1

[PATCH i-g-t 11/43] kms_rotation_crc: Update the copyright to have this year as well

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 485a6df..f5a569a 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -1,5 +1,5 @@
 /*
- * Copyright © 2013 Intel Corporation
+ * Copyright © 2013,2014 Intel Corporation
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
-- 
1.8.3.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 12/43] kms_rotation_crc: Test the validity of the output first

[Damien Lespiau]

So we don't need code to unwind what we just did.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index f5a569a..1f2953d 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -78,6 +78,9 @@ static bool prepare_crtc(data_t *data)
 	igt_output_set_pipe(output, data->pipe);
 	igt_display_commit(display);
 
+	if (!data->output->valid)
+		return false;
+
 	/* create the pipe_crc object for this pipe */
 	if (data->pipe_crc)
 		igt_pipe_crc_free(data->pipe_crc);
@@ -90,13 +93,6 @@ static bool prepare_crtc(data_t *data)
 		return false;
 	}
 
-
-	if (!data->output->valid) {
-		igt_output_set_pipe(output, PIPE_ANY);
-		igt_display_commit(display);
-		return false;
-	}
-
 	switch (data->type) {
 
 		case DRM_PLANE_TYPE_OVERLAY: /* Sprite */
-- 
1.8.3.1

[PATCH i-g-t 13/43] kms_rotation_crc: Require universal planes for the testing primary rotation

[Damien Lespiau]

Otherwise the test will fail instead of just skipping.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 1f2953d..db1ad57 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -355,6 +355,8 @@ static void test_primary_rotation(data_t *data)
 	int valid_tests = 0;
 	igt_crc_t crc_output;
 
+	igt_require(data->display.has_universal_planes);
+
 	for_each_connected_output(display, output) {
 		data->output = output;
 		for (p = 0; p < igt_display_get_n_pipes(display); p++) {
-- 
1.8.3.1

[PATCH i-g-t 14/43] kms_rotation_crc: Fix style issue: '{' at the end of lines

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index db1ad57..9c194ce 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -183,12 +183,10 @@ static bool check_plane_type(int drm_fd, uint32_t plane_id, uint32_t type)
 
 	props = drmModeObjectGetProperties(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE);
 
-	for (i = 0; i < props->count_props; i++)
-	{
+	for (i = 0; i < props->count_props; i++) {
 		drmModePropertyPtr prop = drmModeGetProperty(drm_fd, props->props[i]);
 
-		if (strcmp(prop->name, "type") == 0)
-		{
+		if (strcmp(prop->name, "type") == 0) {
 			if (props->prop_values[i] == type) {
 				return true;
 			}
@@ -250,23 +248,19 @@ static int set_plane_property(data_t *data, int plane_id, const char *prop_name,
 	value = (uint64_t)val;
 	props = drmModeObjectGetProperties(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE);
 
-	for (i = 0; i < props->count_props; i++)
-	{
+	for (i = 0; i < props->count_props; i++) {
 		drmModePropertyPtr prop = drmModeGetProperty(drm_fd, props->props[i]);
 		igt_info("\nProp->name=%s: plane_id:%d\n ", prop->name,	plane_id);
 
-		if (strcmp(prop->name, prop_name) == 0)
-		{
+		if (strcmp(prop->name, prop_name) == 0) {
 			ret = drmModeObjectSetProperty(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE,
 					(uint32_t)prop->prop_id, value);
-			if (ret)
-			{
+			if (ret) {
 				igt_info("set_property \"%s\" to %d for plane %d is failed,	err:%d\n", prop_name, val, plane_id, ret);
 				drmModeFreeProperty(prop);
 				drmModeFreeObjectProperties(props);
 				return ret;
-			}
-			else {
+			} else {
 				/* Collect crc after rotation */
 				igt_pipe_crc_collect_crc(data->pipe_crc, crc_output);
 				drmModeFreeProperty(prop);
-- 
1.8.3.1

[PATCH i-g-t 15/43] kms_rotation_crc: Fix style issue: single statement conditionals

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 9c194ce..eb63326 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -187,9 +187,9 @@ static bool check_plane_type(int drm_fd, uint32_t plane_id, uint32_t type)
 		drmModePropertyPtr prop = drmModeGetProperty(drm_fd, props->props[i]);
 
 		if (strcmp(prop->name, "type") == 0) {
-			if (props->prop_values[i] == type) {
+			if (props->prop_values[i] == type)
 				return true;
-			}
+
 			igt_info("Didn't find the requested type:%u\n", (unsigned int)props->prop_values[i]);
 		}
 	}
@@ -283,12 +283,10 @@ static void cleanup_crtc(data_t *data, igt_output_t *output)
 
 	igt_remove_fb(data->gfx_fd, &data->fb);
 
-	if (data->type == DRM_PLANE_TYPE_PRIMARY) {
+	if (data->type == DRM_PLANE_TYPE_PRIMARY)
 		plane = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
-	}
-	else if (data->type == DRM_PLANE_TYPE_OVERLAY) {
+	else if (data->type == DRM_PLANE_TYPE_OVERLAY)
 		plane = igt_output_get_plane(output, IGT_PLANE_2);
-	}
 
 	if (plane != NULL)
 		igt_plane_set_fb(plane, NULL);
-- 
1.8.3.1

[PATCH i-g-t 16/43] kms_rotation_crc: Factor out the square drawing function

[Damien Lespiau]

Making function to the similar things is very common in programming.
Let's do it once again.

Cairo being a drawing library, it can be used to do the rotation!

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 65 ++++++++++++++++++++++++++----------------------
 1 file changed, 35 insertions(+), 30 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index eb63326..5f499d7 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -31,6 +31,7 @@
 #include <stdint.h>
 #include <stddef.h>
 #include <fcntl.h>
+#include <math.h>
 
 #include "drmtest.h"
 #include "igt_debugfs.h"
@@ -65,15 +66,39 @@ typedef struct {
 	int rotate;
 } data_t;
 
+static void
+paint_squares(data_t *data, struct igt_fb *fb, drmModeModeInfo *mode,
+	      uint32_t rotation)
+{
+	cairo_t *cr;
+	int w, h;
+
+	w = mode->hdisplay;
+	h = mode->vdisplay;
+
+	cr = igt_get_cairo_ctx(data->gfx_fd, &data->fb);
+
+	if (rotation == DRM_ROTATE_180) {
+		cairo_translate(cr, w, h);
+		cairo_rotate(cr, M_PI);
+	}
+
+	/* Paint with 4 squares of Red, Green, White, Blue Clockwise */
+	igt_paint_color(cr, 0, 0, w/2, h/2, 1.0, 0.0, 0.0);
+	igt_paint_color(cr, (w/2)-1, 0, w/2, h/2, 0.0, 1.0, 0.0);
+	igt_paint_color(cr, 0, (h/2)-1, w/2, h/2, 0.0, 0.0, 1.0);
+	igt_paint_color(cr, (w/2)-1, (h/2)-1, w/2, h/2, 1.0, 1.0, 1.0);
+
+	cairo_destroy(cr);
+}
+
 static bool prepare_crtc(data_t *data)
 {
 	drmModeModeInfo *mode;
 	igt_display_t *display = &data->display;
 	igt_output_t *output = data->output;
-	cairo_t *cr;
 	igt_plane_t *primary, *sprite;
 	int fb_id;
-	int w, h;
 
 	igt_output_set_pipe(output, data->pipe);
 	igt_display_commit(display);
@@ -98,8 +123,6 @@ static bool prepare_crtc(data_t *data)
 		case DRM_PLANE_TYPE_OVERLAY: /* Sprite */
 			igt_info("Sprite plane\n");
 			mode = igt_output_get_mode(output);
-			w = mode->hdisplay;
-			h = mode->vdisplay;
 
 			fb_id = igt_create_fb(data->gfx_fd,
 					mode->hdisplay, mode->vdisplay,
@@ -107,13 +130,9 @@ static bool prepare_crtc(data_t *data)
 					false, /* tiled */
 					&data->fb);
 			igt_assert(fb_id);
-			cr = igt_get_cairo_ctx(data->gfx_fd, &data->fb);
 
-			/* Paint rotated image of 4 colors */
-			igt_paint_color(cr, (w/2)-1, (h/2)-1, w/2, h/2, 1.0, 0.0, 0.0);
-			igt_paint_color(cr, 0, (h/2)-1, w/2, h/2, 0.0, 1.0, 0.0);
-			igt_paint_color(cr, (w/2)-1, 0, w/2, h/2, 0.0, 0.0, 1.0);
-			igt_paint_color(cr, 0, 0, w/2, h/2, 1.0, 1.0, 1.0);
+			paint_squares(data, &data->fb, mode, DRM_ROTATE_180);
+
 			sprite = igt_output_get_plane(output, IGT_PLANE_2);
 			igt_plane_set_fb(sprite, &data->fb);
 			igt_display_commit(display);
@@ -121,12 +140,8 @@ static bool prepare_crtc(data_t *data)
 			/* Collect reference crc */
 			igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc[1]);
 
-			/* Paint with 4 squares of Red, Green, White, Blue Clockwise */
-			igt_paint_color(cr, 0, 0, w/2, h/2, 1.0, 0.0, 0.0);
-			igt_paint_color(cr, (w/2)-1, 0, w/2, h/2, 0.0, 1.0, 0.0);
-			igt_paint_color(cr, 0, (h/2)-1, w/2, h/2, 0.0, 0.0, 1.0);
-			igt_paint_color(cr, (w/2)-1, (h/2)-1, w/2, h/2, 1.0, 1.0, 1.0);
-			cairo_destroy(cr);
+			paint_squares(data, &data->fb, mode, DRM_ROTATE_0);
+
 			sprite = igt_output_get_plane(output, IGT_PLANE_2);
 			igt_plane_set_fb(sprite, &data->fb);
 			igt_display_commit(display);
@@ -135,8 +150,6 @@ static bool prepare_crtc(data_t *data)
 		case DRM_PLANE_TYPE_PRIMARY: /* Primary */
 			igt_info("Primary plane\n");
 			mode = igt_output_get_mode(output);
-			w = mode->hdisplay;
-			h = mode->vdisplay;
 
 			fb_id = igt_create_fb(data->gfx_fd,
 					      mode->hdisplay, mode->vdisplay,
@@ -144,13 +157,9 @@ static bool prepare_crtc(data_t *data)
 					      false, /* tiled */
 					      &data->fb);
 			igt_assert(fb_id);
-			cr = igt_get_cairo_ctx(data->gfx_fd, &data->fb);
 
-			/* Paint rotated image of 4 colors */
-			igt_paint_color(cr, (w/2)-1, (h/2)-1, w/2, h/2, 1.0, 0.2, 0.2);
-			igt_paint_color(cr, 0, (h/2)-1, w/2, h/2, 0.2, 1.0, 0.2);
-			igt_paint_color(cr, (w/2)-1, 0, w/2, h/2, 0.2, 0.2, 1.0);
-			igt_paint_color(cr, 0, 0, w/2, h/2, 0.8, 0.8, 0.8);
+			paint_squares(data, &data->fb, mode, DRM_ROTATE_180);
+
 			primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
 			igt_plane_set_fb(primary, &data->fb);
 			igt_display_commit(display);
@@ -158,12 +167,8 @@ static bool prepare_crtc(data_t *data)
 			/* Collect reference crc */
 			igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc[0]);
 
-			/* Paint with 4 squares of Red, Green, White, Blue Clockwise */
-			igt_paint_color(cr, 0, 0, w/2, h/2, 1.0, 0.2, 0.2);
-			igt_paint_color(cr, (w/2)-1, 0, w/2, h/2, 0.2, 1.0, 0.2);
-			igt_paint_color(cr, 0, (h/2)-1, w/2, h/2, 0.2, 0.2, 1.0);
-			igt_paint_color(cr, (w/2)-1, (h/2)-1, w/2, h/2, 0.8, 0.8, 0.8);
-			cairo_destroy(cr);
+			paint_squares(data, &data->fb, mode, DRM_ROTATE_0);
+
 			primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
 			igt_plane_set_fb(primary, &data->fb);
 			igt_display_commit(display);
-- 
1.8.3.1

[PATCH i-g-t 17/43] kms_rotation_crc: Factor out common primary/sprite code in prepare_crtc()

[Damien Lespiau]

This results in less code, always a good thing. Also, we only really
need one reference CRC.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 89 +++++++++++++++++-------------------------------
 1 file changed, 32 insertions(+), 57 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 5f499d7..bc11b9c 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -61,7 +61,7 @@ typedef struct {
 	int type;
 	int pipe;
 	struct igt_fb fb;
-	igt_crc_t ref_crc[2];
+	igt_crc_t ref_crc;
 	igt_pipe_crc_t *pipe_crc;
 	int rotate;
 } data_t;
@@ -97,7 +97,7 @@ static bool prepare_crtc(data_t *data)
 	drmModeModeInfo *mode;
 	igt_display_t *display = &data->display;
 	igt_output_t *output = data->output;
-	igt_plane_t *primary, *sprite;
+	igt_plane_t *plane;
 	int fb_id;
 
 	igt_output_set_pipe(output, data->pipe);
@@ -106,6 +106,19 @@ static bool prepare_crtc(data_t *data)
 	if (!data->output->valid)
 		return false;
 
+	switch (data->type) {
+		case DRM_PLANE_TYPE_OVERLAY: /* Sprite */
+			igt_info("Sprite plane\n");
+			plane = igt_output_get_plane(output, IGT_PLANE_2);
+			break;
+		case DRM_PLANE_TYPE_PRIMARY: /* Primary */
+			igt_info("Primary plane\n");
+			plane = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+			break;
+		default:
+			return false;
+	}
+
 	/* create the pipe_crc object for this pipe */
 	if (data->pipe_crc)
 		igt_pipe_crc_free(data->pipe_crc);
@@ -118,65 +131,27 @@ static bool prepare_crtc(data_t *data)
 		return false;
 	}
 
-	switch (data->type) {
-
-		case DRM_PLANE_TYPE_OVERLAY: /* Sprite */
-			igt_info("Sprite plane\n");
-			mode = igt_output_get_mode(output);
-
-			fb_id = igt_create_fb(data->gfx_fd,
-					mode->hdisplay, mode->vdisplay,
-					DRM_FORMAT_XRGB8888,
-					false, /* tiled */
-					&data->fb);
-			igt_assert(fb_id);
-
-			paint_squares(data, &data->fb, mode, DRM_ROTATE_180);
-
-			sprite = igt_output_get_plane(output, IGT_PLANE_2);
-			igt_plane_set_fb(sprite, &data->fb);
-			igt_display_commit(display);
-
-			/* Collect reference crc */
-			igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc[1]);
-
-			paint_squares(data, &data->fb, mode, DRM_ROTATE_0);
-
-			sprite = igt_output_get_plane(output, IGT_PLANE_2);
-			igt_plane_set_fb(sprite, &data->fb);
-			igt_display_commit(display);
-
-			break;
-		case DRM_PLANE_TYPE_PRIMARY: /* Primary */
-			igt_info("Primary plane\n");
-			mode = igt_output_get_mode(output);
-
-			fb_id = igt_create_fb(data->gfx_fd,
-					      mode->hdisplay, mode->vdisplay,
-					      DRM_FORMAT_XRGB8888,
-					      false, /* tiled */
-					      &data->fb);
-			igt_assert(fb_id);
-
-			paint_squares(data, &data->fb, mode, DRM_ROTATE_180);
-
-			primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
-			igt_plane_set_fb(primary, &data->fb);
-			igt_display_commit(display);
+	mode = igt_output_get_mode(output);
 
-			/* Collect reference crc */
-			igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc[0]);
+	fb_id = igt_create_fb(data->gfx_fd,
+			mode->hdisplay, mode->vdisplay,
+			DRM_FORMAT_XRGB8888,
+			false, /* tiled */
+			&data->fb);
+	igt_assert(fb_id);
 
-			paint_squares(data, &data->fb, mode, DRM_ROTATE_0);
+	paint_squares(data, &data->fb, mode, DRM_ROTATE_180);
 
-			primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
-			igt_plane_set_fb(primary, &data->fb);
-			igt_display_commit(display);
+	igt_plane_set_fb(plane, &data->fb);
+	igt_display_commit(display);
 
-			break;
+	/* Collect reference crc */
+	igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc);
 
+	paint_squares(data, &data->fb, mode, DRM_ROTATE_0);
 
-	}
+	igt_plane_set_fb(plane, &data->fb);
+	igt_display_commit(display);
 
 	return true;
 }
@@ -332,7 +307,7 @@ static void test_sprite_rotation(data_t *data)
 					return;
 				}
 			}
-			igt_assert(igt_crc_equal(&data->ref_crc[1], &crc_output));
+			igt_assert(igt_crc_equal(&data->ref_crc, &crc_output));
 			sleep(2);
 			valid_tests++;
 			cleanup_crtc(data, output);
@@ -378,7 +353,7 @@ static void test_primary_rotation(data_t *data)
 					return;
 				}
 			}
-			igt_assert(igt_crc_equal(&data->ref_crc[0], &crc_output));
+			igt_assert(igt_crc_equal(&data->ref_crc, &crc_output));
 			sleep(2);
 			valid_tests++;
 			cleanup_crtc(data, output);
-- 
1.8.3.1

[PATCH i-g-t 18/43] kms_rotation_crc: Remove useless comments

[Damien Lespiau]

A typical example of what comments shouldn't be:

 case DRM_PLANE_TYPE_PRIMARY: /* primary */

Well, yes!, it's written just there, PRIMARY!

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index bc11b9c..e4af24d 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -107,11 +107,11 @@ static bool prepare_crtc(data_t *data)
 		return false;
 
 	switch (data->type) {
-		case DRM_PLANE_TYPE_OVERLAY: /* Sprite */
+		case DRM_PLANE_TYPE_OVERLAY:
 			igt_info("Sprite plane\n");
 			plane = igt_output_get_plane(output, IGT_PLANE_2);
 			break;
-		case DRM_PLANE_TYPE_PRIMARY: /* Primary */
+		case DRM_PLANE_TYPE_PRIMARY:
 			igt_info("Primary plane\n");
 			plane = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
 			break;
-- 
1.8.3.1

[PATCH i-g-t 19/43] kms_rotation_crc: Use drm_plane from igt_plane_t

[Damien Lespiau]

So we don't need all that extra code to grab the drm_plane structure for
the primary_plane.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 70 +++---------------------------------------------
 1 file changed, 3 insertions(+), 67 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index e4af24d..f062b52 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -52,7 +52,6 @@
 #define DRM_PLANE_TYPE_OVERLAY 0
 #define DRM_PLANE_TYPE_PRIMARY 1
 #define DRM_PLANE_TYPE_CURSOR  2
-#define DRM_CLIENT_CAP_EXPOSE_PRIMARY_PLANES 2
 
 typedef struct {
 	int gfx_fd;
@@ -156,67 +155,6 @@ static bool prepare_crtc(data_t *data)
 	return true;
 }
 
-static bool check_plane_type(int drm_fd, uint32_t plane_id, uint32_t type)
-{
-	int i = 0;
-	drmModeObjectPropertiesPtr props = NULL;
-
-	props = drmModeObjectGetProperties(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE);
-
-	for (i = 0; i < props->count_props; i++) {
-		drmModePropertyPtr prop = drmModeGetProperty(drm_fd, props->props[i]);
-
-		if (strcmp(prop->name, "type") == 0) {
-			if (props->prop_values[i] == type)
-				return true;
-
-			igt_info("Didn't find the requested type:%u\n", (unsigned int)props->prop_values[i]);
-		}
-	}
-	return false;
-}
-
-static int connector_find_plane(int gfx_fd, uint32_t pipe, uint32_t type)
-{
-	drmModePlaneRes *plane_resources;
-	drmModePlane *ovr;
-	uint32_t id = 0;
-	int i, ret = 0;
-
-	ret = drmSetClientCap(gfx_fd, DRM_CLIENT_CAP_EXPOSE_PRIMARY_PLANES, 1);
-	if (ret < 0) {
-		igt_info("Failed to set client cap:%d\n", ret);
-		return 0;
-	}
-
-	plane_resources = drmModeGetPlaneResources(gfx_fd);
-	if (!plane_resources) {
-		igt_info("drmModeGetPlaneResources failed: %s\n",
-			 strerror(errno));
-		return 0;
-	}
-
-	for (i = 0; i < plane_resources->count_planes; i++) {
-		ovr = drmModeGetPlane(gfx_fd, plane_resources->planes[i]);
-		if (!ovr) {
-			igt_info("drmModeGetPlane failed: %s\n",
-				 strerror(errno));
-			continue;
-		}
-
-		if (ovr->possible_crtcs & (1 << pipe)) {
-			id = ovr->plane_id;
-			if (check_plane_type(gfx_fd, id, type)) {
-				drmModeFreePlane(ovr);
-				return id;
-			}
-		}
-		drmModeFreePlane(ovr);
-	}
-
-	return 0;
-}
-
 static int set_plane_property(data_t *data, int plane_id, const char *prop_name, int
 		val, igt_crc_t *crc_output)
 {
@@ -321,6 +259,7 @@ static void test_primary_rotation(data_t *data)
 {
 	igt_display_t *display = &data->display;
 	igt_output_t *output;
+	igt_plane_t *primary;
 	int p;
 	int plane_id;
 	int ret;
@@ -340,11 +279,8 @@ static void test_primary_rotation(data_t *data)
 				continue;
 			sleep(2);
 
-			/* Find primary plane. Currently igt_plane_t returned from
-			 * igt_output_get_plane has NULL drm_plane which is needed to get
-			 * the plane_id. So finding the primary plane by checking the "type"
-			 * property of the plane */
-			plane_id = connector_find_plane(data->gfx_fd, data->pipe, data->type);
+			primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+			plane_id = primary->drm_plane->plane_id;
 			if (plane_id != 0) {
 				igt_info("Setting rotation property for plane:%d\n", plane_id);
 				ret = set_plane_property(data, plane_id, "rotation", BIT(data->rotate), &crc_output);
-- 
1.8.3.1

[PATCH i-g-t 20/43] kms_rotation_crc: Style issue: binary operators need spaces before and after

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index f062b52..341f7bc 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -83,10 +83,10 @@ paint_squares(data_t *data, struct igt_fb *fb, drmModeModeInfo *mode,
 	}
 
 	/* Paint with 4 squares of Red, Green, White, Blue Clockwise */
-	igt_paint_color(cr, 0, 0, w/2, h/2, 1.0, 0.0, 0.0);
-	igt_paint_color(cr, (w/2)-1, 0, w/2, h/2, 0.0, 1.0, 0.0);
-	igt_paint_color(cr, 0, (h/2)-1, w/2, h/2, 0.0, 0.0, 1.0);
-	igt_paint_color(cr, (w/2)-1, (h/2)-1, w/2, h/2, 1.0, 1.0, 1.0);
+	igt_paint_color(cr, 0, 0, w / 2, h / 2, 1.0, 0.0, 0.0);
+	igt_paint_color(cr, (w / 2) - 1, 0, w / 2, h / 2, 0.0, 1.0, 0.0);
+	igt_paint_color(cr, 0, (h / 2) - 1, w / 2, h / 2, 0.0, 0.0, 1.0);
+	igt_paint_color(cr, (w / 2) - 1, (h / 2) - 1, w / 2, h / 2, 1.0, 1.0, 1.0);
 
 	cairo_destroy(cr);
 }
-- 
1.8.3.1

[PATCH i-g-t 21/43] kms_rotation_crc: Skip the tests if rotation is not supported

[Damien Lespiau]

This happens when the kernel lacks the rotation patches.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 341f7bc..026c333 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -236,6 +236,9 @@ static void test_sprite_rotation(data_t *data)
 			sleep(2);
 
 			sprite = igt_output_get_plane(output, IGT_PLANE_2);
+
+			igt_require(igt_plane_supports_rotation(sprite));
+
 			plane_id = sprite->drm_plane->plane_id;
 			if (plane_id != 0) {
 				igt_info("Setting rotation property for plane:%d\n", plane_id);
@@ -280,6 +283,9 @@ static void test_primary_rotation(data_t *data)
 			sleep(2);
 
 			primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+
+			igt_require(igt_plane_supports_rotation(primary));
+
 			plane_id = primary->drm_plane->plane_id;
 			if (plane_id != 0) {
 				igt_info("Setting rotation property for plane:%d\n", plane_id);
-- 
1.8.3.1

[PATCH i-g-t 22/43] kms_rotation_crc: Just store the igt_plane_t in data

[Damien Lespiau]

Now that we're always using an igt_plane_t, we can get rid of ->type to
use ->directly without those switch() or if()/else

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 46 +++++++++++-----------------------------------
 1 file changed, 11 insertions(+), 35 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 026c333..966fd12 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -57,7 +57,7 @@ typedef struct {
 	int gfx_fd;
 	igt_display_t display;
 	igt_output_t *output;
-	int type;
+	igt_plane_t *plane;
 	int pipe;
 	struct igt_fb fb;
 	igt_crc_t ref_crc;
@@ -96,7 +96,6 @@ static bool prepare_crtc(data_t *data)
 	drmModeModeInfo *mode;
 	igt_display_t *display = &data->display;
 	igt_output_t *output = data->output;
-	igt_plane_t *plane;
 	int fb_id;
 
 	igt_output_set_pipe(output, data->pipe);
@@ -105,19 +104,6 @@ static bool prepare_crtc(data_t *data)
 	if (!data->output->valid)
 		return false;
 
-	switch (data->type) {
-		case DRM_PLANE_TYPE_OVERLAY:
-			igt_info("Sprite plane\n");
-			plane = igt_output_get_plane(output, IGT_PLANE_2);
-			break;
-		case DRM_PLANE_TYPE_PRIMARY:
-			igt_info("Primary plane\n");
-			plane = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
-			break;
-		default:
-			return false;
-	}
-
 	/* create the pipe_crc object for this pipe */
 	if (data->pipe_crc)
 		igt_pipe_crc_free(data->pipe_crc);
@@ -141,7 +127,7 @@ static bool prepare_crtc(data_t *data)
 
 	paint_squares(data, &data->fb, mode, DRM_ROTATE_180);
 
-	igt_plane_set_fb(plane, &data->fb);
+	igt_plane_set_fb(data->plane, &data->fb);
 	igt_display_commit(display);
 
 	/* Collect reference crc */
@@ -149,7 +135,7 @@ static bool prepare_crtc(data_t *data)
 
 	paint_squares(data, &data->fb, mode, DRM_ROTATE_0);
 
-	igt_plane_set_fb(plane, &data->fb);
+	igt_plane_set_fb(data->plane, &data->fb);
 	igt_display_commit(display);
 
 	return true;
@@ -194,20 +180,14 @@ static int set_plane_property(data_t *data, int plane_id, const char *prop_name,
 static void cleanup_crtc(data_t *data, igt_output_t *output)
 {
 	igt_display_t *display = &data->display;
-	igt_plane_t *plane = NULL;
 
 	igt_pipe_crc_free(data->pipe_crc);
 	data->pipe_crc = NULL;
 
 	igt_remove_fb(data->gfx_fd, &data->fb);
 
-	if (data->type == DRM_PLANE_TYPE_PRIMARY)
-		plane = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
-	else if (data->type == DRM_PLANE_TYPE_OVERLAY)
-		plane = igt_output_get_plane(output, IGT_PLANE_2);
-
-	if (plane != NULL)
-		igt_plane_set_fb(plane, NULL);
+	if (data->plane != NULL)
+		igt_plane_set_fb(data->plane, NULL);
 
 	igt_output_set_pipe(output, PIPE_ANY);
 	igt_display_commit(display);
@@ -217,7 +197,6 @@ static void test_sprite_rotation(data_t *data)
 {
 	igt_display_t *display = &data->display;
 	igt_output_t *output;
-	igt_plane_t *sprite;
 	igt_crc_t crc_output;
 	int p;
 	int plane_id;
@@ -228,18 +207,17 @@ static void test_sprite_rotation(data_t *data)
 		data->output = output;
 		for (p = 0; p < igt_display_get_n_pipes(display); p++) {
 			data->pipe = p;
-			data->type = 0;
 			data->rotate = DRM_ROTATE_180;
 
 			if (!prepare_crtc(data))
 				continue;
 			sleep(2);
 
-			sprite = igt_output_get_plane(output, IGT_PLANE_2);
+			data->plane = igt_output_get_plane(output, IGT_PLANE_2);
 
-			igt_require(igt_plane_supports_rotation(sprite));
+			igt_require(igt_plane_supports_rotation(data->plane));
 
-			plane_id = sprite->drm_plane->plane_id;
+			plane_id = data->plane->drm_plane->plane_id;
 			if (plane_id != 0) {
 				igt_info("Setting rotation property for plane:%d\n", plane_id);
 				ret = set_plane_property(data, plane_id, "rotation", BIT(data->rotate), &crc_output);
@@ -262,7 +240,6 @@ static void test_primary_rotation(data_t *data)
 {
 	igt_display_t *display = &data->display;
 	igt_output_t *output;
-	igt_plane_t *primary;
 	int p;
 	int plane_id;
 	int ret;
@@ -275,18 +252,17 @@ static void test_primary_rotation(data_t *data)
 		data->output = output;
 		for (p = 0; p < igt_display_get_n_pipes(display); p++) {
 			data->pipe = p;
-			data->type = 1;
 			data->rotate = DRM_ROTATE_180;
 
 			if (!prepare_crtc(data))
 				continue;
 			sleep(2);
 
-			primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+			data->plane = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
 
-			igt_require(igt_plane_supports_rotation(primary));
+			igt_require(igt_plane_supports_rotation(data->plane));
 
-			plane_id = primary->drm_plane->plane_id;
+			plane_id = data->plane->drm_plane->plane_id;
 			if (plane_id != 0) {
 				igt_info("Setting rotation property for plane:%d\n", plane_id);
 				ret = set_plane_property(data, plane_id, "rotation", BIT(data->rotate), &crc_output);
-- 
1.8.3.1

[PATCH i-g-t 23/43] kms_rotation_crc: Unify the two tests

[Damien Lespiau]

The only difference is which plane we are talking about. So we really
need one function here with a paramater.

Well, almost. For the primary plane we need to ensure we support
unviversal planes.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 54 ++++++------------------------------------------
 1 file changed, 6 insertions(+), 48 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 966fd12..0edc6c3 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -193,50 +193,7 @@ static void cleanup_crtc(data_t *data, igt_output_t *output)
 	igt_display_commit(display);
 }
 
-static void test_sprite_rotation(data_t *data)
-{
-	igt_display_t *display = &data->display;
-	igt_output_t *output;
-	igt_crc_t crc_output;
-	int p;
-	int plane_id;
-	int ret;
-	int valid_tests = 0;
-
-	for_each_connected_output(display, output) {
-		data->output = output;
-		for (p = 0; p < igt_display_get_n_pipes(display); p++) {
-			data->pipe = p;
-			data->rotate = DRM_ROTATE_180;
-
-			if (!prepare_crtc(data))
-				continue;
-			sleep(2);
-
-			data->plane = igt_output_get_plane(output, IGT_PLANE_2);
-
-			igt_require(igt_plane_supports_rotation(data->plane));
-
-			plane_id = data->plane->drm_plane->plane_id;
-			if (plane_id != 0) {
-				igt_info("Setting rotation property for plane:%d\n", plane_id);
-				ret = set_plane_property(data, plane_id, "rotation", BIT(data->rotate), &crc_output);
-				if (ret < 0) {
-					igt_info("Setting rotation failed!");
-					return;
-				}
-			}
-			igt_assert(igt_crc_equal(&data->ref_crc, &crc_output));
-			sleep(2);
-			valid_tests++;
-			cleanup_crtc(data, output);
-		}
-	}
-	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
-}
-
-
-static void test_primary_rotation(data_t *data)
+static void test_plane_rotation(data_t *data, enum igt_plane plane)
 {
 	igt_display_t *display = &data->display;
 	igt_output_t *output;
@@ -246,7 +203,8 @@ static void test_primary_rotation(data_t *data)
 	int valid_tests = 0;
 	igt_crc_t crc_output;
 
-	igt_require(data->display.has_universal_planes);
+	if (plane == IGT_PLANE_PRIMARY)
+		igt_require(data->display.has_universal_planes);
 
 	for_each_connected_output(display, output) {
 		data->output = output;
@@ -258,7 +216,7 @@ static void test_primary_rotation(data_t *data)
 				continue;
 			sleep(2);
 
-			data->plane = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+			data->plane = igt_output_get_plane(output, plane);
 
 			igt_require(igt_plane_supports_rotation(data->plane));
 
@@ -297,10 +255,10 @@ igt_main
 	}
 
 	igt_subtest_f("primary-rotation")
-		test_primary_rotation(&data);
+		test_plane_rotation(&data, IGT_PLANE_PRIMARY);
 
 	igt_subtest_f("sprite-rotation")
-		test_sprite_rotation(&data);
+		test_plane_rotation(&data, IGT_PLANE_2);
 
 	igt_fixture {
 		igt_display_fini(&data.display);
-- 
1.8.3.1

[PATCH i-g-t 24/43] kms_rotation_crc: Always disable the plane in cleanup

[Damien Lespiau]

There's no need for this check, always use set_fb(NULL) on the plane.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 0edc6c3..30eb227 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -185,11 +185,9 @@ static void cleanup_crtc(data_t *data, igt_output_t *output)
 	data->pipe_crc = NULL;
 
 	igt_remove_fb(data->gfx_fd, &data->fb);
-
-	if (data->plane != NULL)
-		igt_plane_set_fb(data->plane, NULL);
-
+	igt_plane_set_fb(data->plane, NULL);
 	igt_output_set_pipe(output, PIPE_ANY);
+
 	igt_display_commit(display);
 }
 
-- 
1.8.3.1

[PATCH i-g-t 25/43] kms_rotation_crc: Don't store rotate in the test state

[Damien Lespiau]

We don't use it anywhere else than the test function.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 30eb227..81d001b 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -62,7 +62,6 @@ typedef struct {
 	struct igt_fb fb;
 	igt_crc_t ref_crc;
 	igt_pipe_crc_t *pipe_crc;
-	int rotate;
 } data_t;
 
 static void
@@ -208,7 +207,6 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane)
 		data->output = output;
 		for (p = 0; p < igt_display_get_n_pipes(display); p++) {
 			data->pipe = p;
-			data->rotate = DRM_ROTATE_180;
 
 			if (!prepare_crtc(data))
 				continue;
@@ -221,7 +219,7 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane)
 			plane_id = data->plane->drm_plane->plane_id;
 			if (plane_id != 0) {
 				igt_info("Setting rotation property for plane:%d\n", plane_id);
-				ret = set_plane_property(data, plane_id, "rotation", BIT(data->rotate), &crc_output);
+				ret = set_plane_property(data, plane_id, "rotation", BIT(DRM_ROTATE_180), &crc_output);
 				if (ret < 0) {
 					igt_info("Setting rotation failed!");
 					return;
-- 
1.8.3.1

[PATCH i-g-t 26/43] kms_rotation_crc: Don't store 'pipe' in the state

[Damien Lespiau]

This variable is only needed for prepare_crtc(), need need to put it in
the test state.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 81d001b..308e884 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -58,7 +58,6 @@ typedef struct {
 	igt_display_t display;
 	igt_output_t *output;
 	igt_plane_t *plane;
-	int pipe;
 	struct igt_fb fb;
 	igt_crc_t ref_crc;
 	igt_pipe_crc_t *pipe_crc;
@@ -90,14 +89,14 @@ paint_squares(data_t *data, struct igt_fb *fb, drmModeModeInfo *mode,
 	cairo_destroy(cr);
 }
 
-static bool prepare_crtc(data_t *data)
+static bool prepare_crtc(data_t *data, enum pipe pipe)
 {
 	drmModeModeInfo *mode;
 	igt_display_t *display = &data->display;
 	igt_output_t *output = data->output;
 	int fb_id;
 
-	igt_output_set_pipe(output, data->pipe);
+	igt_output_set_pipe(output, pipe);
 	igt_display_commit(display);
 
 	if (!data->output->valid)
@@ -107,11 +106,10 @@ static bool prepare_crtc(data_t *data)
 	if (data->pipe_crc)
 		igt_pipe_crc_free(data->pipe_crc);
 
-	data->pipe_crc = igt_pipe_crc_new(data->pipe,
-					  INTEL_PIPE_CRC_SOURCE_AUTO);
+	data->pipe_crc = igt_pipe_crc_new(pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
 	if (!data->pipe_crc) {
 		igt_info("auto crc not supported on this connector with pipe %i\n",
-			 data->pipe);
+			 pipe);
 		return false;
 	}
 
@@ -206,9 +204,7 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane)
 	for_each_connected_output(display, output) {
 		data->output = output;
 		for (p = 0; p < igt_display_get_n_pipes(display); p++) {
-			data->pipe = p;
-
-			if (!prepare_crtc(data))
+			if (!prepare_crtc(data, p))
 				continue;
 			sleep(2);
 
-- 
1.8.3.1

[PATCH i-g-t 27/43] kms_rotation_crc: Use igt_plane_set_rotation()

[Damien Lespiau]

More code we can remove from the test.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 52 +++++-------------------------------------------
 1 file changed, 5 insertions(+), 47 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 308e884..15529c7 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -138,42 +138,6 @@ static bool prepare_crtc(data_t *data, enum pipe pipe)
 	return true;
 }
 
-static int set_plane_property(data_t *data, int plane_id, const char *prop_name, int
-		val, igt_crc_t *crc_output)
-{
-	int i = 0, ret = 0;
-	int drm_fd = data->gfx_fd;
-	uint64_t value;
-	drmModeObjectPropertiesPtr props = NULL;
-
-	value = (uint64_t)val;
-	props = drmModeObjectGetProperties(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE);
-
-	for (i = 0; i < props->count_props; i++) {
-		drmModePropertyPtr prop = drmModeGetProperty(drm_fd, props->props[i]);
-		igt_info("\nProp->name=%s: plane_id:%d\n ", prop->name,	plane_id);
-
-		if (strcmp(prop->name, prop_name) == 0) {
-			ret = drmModeObjectSetProperty(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE,
-					(uint32_t)prop->prop_id, value);
-			if (ret) {
-				igt_info("set_property \"%s\" to %d for plane %d is failed,	err:%d\n", prop_name, val, plane_id, ret);
-				drmModeFreeProperty(prop);
-				drmModeFreeObjectProperties(props);
-				return ret;
-			} else {
-				/* Collect crc after rotation */
-				igt_pipe_crc_collect_crc(data->pipe_crc, crc_output);
-				drmModeFreeProperty(prop);
-				break;
-			}
-		}
-		drmModeFreeProperty(prop);
-	}
-	drmModeFreeObjectProperties(props);
-	return 0;
-}
-
 static void cleanup_crtc(data_t *data, igt_output_t *output)
 {
 	igt_display_t *display = &data->display;
@@ -193,8 +157,6 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane)
 	igt_display_t *display = &data->display;
 	igt_output_t *output;
 	int p;
-	int plane_id;
-	int ret;
 	int valid_tests = 0;
 	igt_crc_t crc_output;
 
@@ -212,16 +174,12 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane)
 
 			igt_require(igt_plane_supports_rotation(data->plane));
 
-			plane_id = data->plane->drm_plane->plane_id;
-			if (plane_id != 0) {
-				igt_info("Setting rotation property for plane:%d\n", plane_id);
-				ret = set_plane_property(data, plane_id, "rotation", BIT(DRM_ROTATE_180), &crc_output);
-				if (ret < 0) {
-					igt_info("Setting rotation failed!");
-					return;
-				}
-			}
+			igt_plane_set_rotation(data->plane, IGT_ROTATION_180);
+			igt_display_commit2(display, COMMIT_UNIVERSAL);
+
+			igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output);
 			igt_assert(igt_crc_equal(&data->ref_crc, &crc_output));
+
 			sleep(2);
 			valid_tests++;
 			cleanup_crtc(data, output);
-- 
1.8.3.1

[PATCH i-g-t 28/43] kms_rotation_crc: Remove now unnecessary defines

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 15529c7..58852c2 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -42,16 +42,6 @@
 #define DRM_ROTATE_90  1
 #define DRM_ROTATE_180 2
 #define DRM_ROTATE_270 3
-#define DRM_REFLECT_X  4
-#define DRM_REFLECT_Y  5
-#define DRM_ROTATE_NUM	6
-
-#define BIT(x)	(1 << x)
-
-// This will be part of libdrm later. Adding here temporarily
-#define DRM_PLANE_TYPE_OVERLAY 0
-#define DRM_PLANE_TYPE_PRIMARY 1
-#define DRM_PLANE_TYPE_CURSOR  2
 
 typedef struct {
 	int gfx_fd;
-- 
1.8.3.1

[PATCH i-g-t 29/43] kms_rotation_crc: Use for_each_pipe()

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 58852c2..ae0ca21 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -146,7 +146,7 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane)
 {
 	igt_display_t *display = &data->display;
 	igt_output_t *output;
-	int p;
+	enum pipe pipe;
 	int valid_tests = 0;
 	igt_crc_t crc_output;
 
@@ -155,8 +155,8 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane)
 
 	for_each_connected_output(display, output) {
 		data->output = output;
-		for (p = 0; p < igt_display_get_n_pipes(display); p++) {
-			if (!prepare_crtc(data, p))
+		for_each_pipe(display, pipe) {
+			if (!prepare_crtc(data, pipe))
 				continue;
 			sleep(2);
 
-- 
1.8.3.1

[PATCH i-g-t 30/43] kms_rotation_crc: Remove the test on output->valid

[Damien Lespiau]

This test is already done by the for_each_connected_output() macro.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index ae0ca21..a68aece 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -83,15 +83,11 @@ static bool prepare_crtc(data_t *data, enum pipe pipe)
 {
 	drmModeModeInfo *mode;
 	igt_display_t *display = &data->display;
-	igt_output_t *output = data->output;
 	int fb_id;
 
 	igt_output_set_pipe(output, pipe);
 	igt_display_commit(display);
 
-	if (!data->output->valid)
-		return false;
-
 	/* create the pipe_crc object for this pipe */
 	if (data->pipe_crc)
 		igt_pipe_crc_free(data->pipe_crc);
-- 
1.8.3.1

[PATCH i-g-t 31/43] kms_rotation_crc: Remove 'output' from the state

[Damien Lespiau]

This restore the balance between prepare_crtc() and cleanup_crtc(), both
now taking the output as a parameter.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index a68aece..5b195f9 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -46,7 +46,6 @@
 typedef struct {
 	int gfx_fd;
 	igt_display_t display;
-	igt_output_t *output;
 	igt_plane_t *plane;
 	struct igt_fb fb;
 	igt_crc_t ref_crc;
@@ -79,7 +78,7 @@ paint_squares(data_t *data, struct igt_fb *fb, drmModeModeInfo *mode,
 	cairo_destroy(cr);
 }
 
-static bool prepare_crtc(data_t *data, enum pipe pipe)
+static bool prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe)
 {
 	drmModeModeInfo *mode;
 	igt_display_t *display = &data->display;
@@ -150,9 +149,8 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane)
 		igt_require(data->display.has_universal_planes);
 
 	for_each_connected_output(display, output) {
-		data->output = output;
 		for_each_pipe(display, pipe) {
-			if (!prepare_crtc(data, pipe))
+			if (!prepare_crtc(data, output, pipe))
 				continue;
 			sleep(2);
 
-- 
1.8.3.1

[PATCH i-g-t 32/43] kms_rotation_crc: Remove the sleep(2)

[Damien Lespiau]

One can inspect the output of the igt_kms API by setting
IGT_DISPLAY_WAIT_AT_COMMIT=1.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 5b195f9..701fa70 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -152,7 +152,6 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane)
 		for_each_pipe(display, pipe) {
 			if (!prepare_crtc(data, output, pipe))
 				continue;
-			sleep(2);
 
 			data->plane = igt_output_get_plane(output, plane);
 
@@ -164,7 +163,6 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane)
 			igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output);
 			igt_assert(igt_crc_equal(&data->ref_crc, &crc_output));
 
-			sleep(2);
 			valid_tests++;
 			cleanup_crtc(data, output);
 		}
-- 
1.8.3.1

[PATCH i-g-t 33/43] kms_rotation_crc: Remove plane from the state

[Damien Lespiau]

having everythin in the data_t structure makes it hard to understand
what should be set when. Replace that by explicit function parameters.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 701fa70..75cfff8 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -46,7 +46,6 @@
 typedef struct {
 	int gfx_fd;
 	igt_display_t display;
-	igt_plane_t *plane;
 	struct igt_fb fb;
 	igt_crc_t ref_crc;
 	igt_pipe_crc_t *pipe_crc;
@@ -78,7 +77,8 @@ paint_squares(data_t *data, struct igt_fb *fb, drmModeModeInfo *mode,
 	cairo_destroy(cr);
 }
 
-static bool prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe)
+static bool prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
+			 igt_plane_t *plane)
 {
 	drmModeModeInfo *mode;
 	igt_display_t *display = &data->display;
@@ -109,7 +109,7 @@ static bool prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe)
 
 	paint_squares(data, &data->fb, mode, DRM_ROTATE_180);
 
-	igt_plane_set_fb(data->plane, &data->fb);
+	igt_plane_set_fb(plane, &data->fb);
 	igt_display_commit(display);
 
 	/* Collect reference crc */
@@ -117,13 +117,13 @@ static bool prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe)
 
 	paint_squares(data, &data->fb, mode, DRM_ROTATE_0);
 
-	igt_plane_set_fb(data->plane, &data->fb);
+	igt_plane_set_fb(plane, &data->fb);
 	igt_display_commit(display);
 
 	return true;
 }
 
-static void cleanup_crtc(data_t *data, igt_output_t *output)
+static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane)
 {
 	igt_display_t *display = &data->display;
 
@@ -131,13 +131,13 @@ static void cleanup_crtc(data_t *data, igt_output_t *output)
 	data->pipe_crc = NULL;
 
 	igt_remove_fb(data->gfx_fd, &data->fb);
-	igt_plane_set_fb(data->plane, NULL);
+	igt_plane_set_fb(plane, NULL);
 	igt_output_set_pipe(output, PIPE_ANY);
 
 	igt_display_commit(display);
 }
 
-static void test_plane_rotation(data_t *data, enum igt_plane plane)
+static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
 {
 	igt_display_t *display = &data->display;
 	igt_output_t *output;
@@ -145,26 +145,29 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane)
 	int valid_tests = 0;
 	igt_crc_t crc_output;
 
-	if (plane == IGT_PLANE_PRIMARY)
+	if (plane_type == IGT_PLANE_PRIMARY)
 		igt_require(data->display.has_universal_planes);
 
 	for_each_connected_output(display, output) {
 		for_each_pipe(display, pipe) {
-			if (!prepare_crtc(data, output, pipe))
-				continue;
+			igt_plane_t *plane;
+
+			igt_output_set_pipe(output, pipe);
 
-			data->plane = igt_output_get_plane(output, plane);
+			plane = igt_output_get_plane(output, plane_type);
+			igt_require(igt_plane_supports_rotation(plane));
 
-			igt_require(igt_plane_supports_rotation(data->plane));
+			if (!prepare_crtc(data, output, pipe, plane))
+				continue;
 
-			igt_plane_set_rotation(data->plane, IGT_ROTATION_180);
+			igt_plane_set_rotation(plane, IGT_ROTATION_180);
 			igt_display_commit2(display, COMMIT_UNIVERSAL);
 
 			igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output);
 			igt_assert(igt_crc_equal(&data->ref_crc, &crc_output));
 
 			valid_tests++;
-			cleanup_crtc(data, output);
+			cleanup_crtc(data, output, plane);
 		}
 	}
 	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
-- 
1.8.3.1

[PATCH i-g-t 34/43] kms_rotation_crc: No need to test for NULL before freeing the pipe CRC object

[Damien Lespiau]

igt_pipe_crc_free() does that check already.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 75cfff8..d196c7c 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -88,9 +88,7 @@ static bool prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 	igt_display_commit(display);
 
 	/* create the pipe_crc object for this pipe */
-	if (data->pipe_crc)
-		igt_pipe_crc_free(data->pipe_crc);
-
+	igt_pipe_crc_free(data->pipe_crc);
 	data->pipe_crc = igt_pipe_crc_new(pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
 	if (!data->pipe_crc) {
 		igt_info("auto crc not supported on this connector with pipe %i\n",
-- 
1.8.3.1

[PATCH i-g-t 35/43] kms_rotation_crc: Allow the sprite test to run even without universal planes

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index d196c7c..d318cd2 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -142,9 +142,12 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
 	enum pipe pipe;
 	int valid_tests = 0;
 	igt_crc_t crc_output;
+	enum igt_commit_style commit = COMMIT_LEGACY;
 
-	if (plane_type == IGT_PLANE_PRIMARY)
+	if (plane_type == IGT_PLANE_PRIMARY) {
 		igt_require(data->display.has_universal_planes);
+		commit = COMMIT_UNIVERSAL;
+	}
 
 	for_each_connected_output(display, output) {
 		for_each_pipe(display, pipe) {
@@ -159,7 +162,7 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
 				continue;
 
 			igt_plane_set_rotation(plane, IGT_ROTATION_180);
-			igt_display_commit2(display, COMMIT_UNIVERSAL);
+			igt_display_commit2(display, commit);
 
 			igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output);
 			igt_assert(igt_crc_equal(&data->ref_crc, &crc_output));
-- 
1.8.3.1

[PATCH i-g-t 36/43] kms_rotation_crc: Don't commit with no fb set up

[Damien Lespiau]

prepare_crtc() was trying to commit a display state without any fb to
scan out...

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index d318cd2..5022b99 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -85,7 +85,6 @@ static bool prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 	int fb_id;
 
 	igt_output_set_pipe(output, pipe);
-	igt_display_commit(display);
 
 	/* create the pipe_crc object for this pipe */
 	igt_pipe_crc_free(data->pipe_crc);
-- 
1.8.3.1

[PATCH i-g-t 37/43] kms_rotation_crc: Properly paint the whole frame buffer

[Damien Lespiau]

The -1 meant we weren't properly filling the whole fb.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 5022b99..5780e40 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -70,9 +70,9 @@ paint_squares(data_t *data, struct igt_fb *fb, drmModeModeInfo *mode,
 
 	/* Paint with 4 squares of Red, Green, White, Blue Clockwise */
 	igt_paint_color(cr, 0, 0, w / 2, h / 2, 1.0, 0.0, 0.0);
-	igt_paint_color(cr, (w / 2) - 1, 0, w / 2, h / 2, 0.0, 1.0, 0.0);
-	igt_paint_color(cr, 0, (h / 2) - 1, w / 2, h / 2, 0.0, 0.0, 1.0);
-	igt_paint_color(cr, (w / 2) - 1, (h / 2) - 1, w / 2, h / 2, 1.0, 1.0, 1.0);
+	igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 0.0, 1.0, 0.0);
+	igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 0.0, 0.0, 1.0);
+	igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 1.0, 1.0, 1.0);
 
 	cairo_destroy(cr);
 }
-- 
1.8.3.1

[PATCH i-g-t 38/43] kms_rotation_crc: Add the test to .gitignore

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/.gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/.gitignore b/tests/.gitignore
index da9af6b..985afbd 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -126,6 +126,7 @@ kms_mmio_vs_cs_flip
 kms_pipe_crc_basic
 kms_plane
 kms_render
+kms_rotation_crc
 kms_setmode
 kms_sink_crc_basic
 kms_universal_plane
-- 
1.8.3.1

[PATCH i-g-t 39/43] kms_rotation_crc: Don't compile the test on Android with no cairo support

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/Android.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/Android.mk b/tests/Android.mk
index 663a6b4..22f12ad 100644
--- a/tests/Android.mk
+++ b/tests/Android.mk
@@ -70,7 +70,8 @@ else
     kms_fence_pin_leak \
     kms_mmio_vs_cs_flip \
     kms_render \
-    kms_universal_plane
+    kms_universal_plane \
+    kms_rotation_crc
     IGT_LOCAL_CFLAGS += -DANDROID_HAS_CAIRO=0
 endif
 
-- 
1.8.3.1

[PATCH i-g-t 40/43] kms_rotation_crc: Document the two steps in prepare_crtc()

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 5780e40..cb1b00b 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -104,14 +104,19 @@ static bool prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 			&data->fb);
 	igt_assert(fb_id);
 
+	/* Step 1: create a reference CRC for a software-rotated fb */
+
 	paint_squares(data, &data->fb, mode, DRM_ROTATE_180);
 
 	igt_plane_set_fb(plane, &data->fb);
 	igt_display_commit(display);
 
-	/* Collect reference crc */
 	igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc);
 
+	/*
+	 * Step 2: prepare the plane with an non-rotated fb let the hw
+	 * rotate it.
+	 */
 	paint_squares(data, &data->fb, mode, DRM_ROTATE_0);
 
 	igt_plane_set_fb(plane, &data->fb);
-- 
1.8.3.1

[PATCH i-g-t 41/43] kms_rotation_crc: Always use the primary plane to compute the reference CRC

[Damien Lespiau]

Trying to disable the primary planes isn't exactly working at the
moment. W/A it until it works.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index cb1b00b..9aacddd 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -86,6 +86,7 @@ static bool prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 
 	igt_output_set_pipe(output, pipe);
 
+
 	/* create the pipe_crc object for this pipe */
 	igt_pipe_crc_free(data->pipe_crc);
 	data->pipe_crc = igt_pipe_crc_new(pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
@@ -108,6 +109,18 @@ static bool prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 
 	paint_squares(data, &data->fb, mode, DRM_ROTATE_180);
 
+	/*
+	 * XXX: We always set the primary plane to actually enable the pipe as
+	 * there's no way (that works) to light up a pipe with only a sprite
+	 * plane enabled at the moment.
+	 */
+	if (!plane->is_primary) {
+		igt_plane_t *primary;
+
+		primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+		igt_plane_set_fb(primary, &data->fb);
+	}
+
 	igt_plane_set_fb(plane, &data->fb);
 	igt_display_commit(display);
 
@@ -133,6 +146,15 @@ static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane)
 	data->pipe_crc = NULL;
 
 	igt_remove_fb(data->gfx_fd, &data->fb);
+
+	/* XXX: see the note in prepare_crtc() */
+	if (!plane->is_primary) {
+		igt_plane_t *primary;
+
+		primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+		igt_plane_set_fb(primary, NULL);
+	}
+
 	igt_plane_set_fb(plane, NULL);
 	igt_output_set_pipe(output, PIPE_ANY);
 
-- 
1.8.3.1

[PATCH i-g-t 42/43] kms_rotation_crc: Remove unnecessary includes

[Damien Lespiau]

Turns out we didn't need most of them.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 9aacddd..2005b2a 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -22,15 +22,6 @@
  *
  */
 
-#include <errno.h>
-#include <limits.h>
-#include <stdbool.h>
-#include <stdio.h>
-#include <string.h>
-#include <sys/ioctl.h>
-#include <stdint.h>
-#include <stddef.h>
-#include <fcntl.h>
 #include <math.h>
 
 #include "drmtest.h"
-- 
1.8.3.1

[PATCH i-g-t 43/43] kms_rotation_crc: Use the igt_kms enum to encode the plane rotation

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/kms_rotation_crc.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 2005b2a..5041f90 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -29,11 +29,6 @@
 #include "igt_kms.h"
 #include "igt_core.h"
 
-#define DRM_ROTATE_0   0
-#define DRM_ROTATE_90  1
-#define DRM_ROTATE_180 2
-#define DRM_ROTATE_270 3
-
 typedef struct {
 	int gfx_fd;
 	igt_display_t display;
@@ -44,7 +39,7 @@ typedef struct {
 
 static void
 paint_squares(data_t *data, struct igt_fb *fb, drmModeModeInfo *mode,
-	      uint32_t rotation)
+	      igt_rotation_t rotation)
 {
 	cairo_t *cr;
 	int w, h;
@@ -54,7 +49,7 @@ paint_squares(data_t *data, struct igt_fb *fb, drmModeModeInfo *mode,
 
 	cr = igt_get_cairo_ctx(data->gfx_fd, &data->fb);
 
-	if (rotation == DRM_ROTATE_180) {
+	if (rotation == IGT_ROTATION_180) {
 		cairo_translate(cr, w, h);
 		cairo_rotate(cr, M_PI);
 	}
@@ -98,7 +93,7 @@ static bool prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 
 	/* Step 1: create a reference CRC for a software-rotated fb */
 
-	paint_squares(data, &data->fb, mode, DRM_ROTATE_180);
+	paint_squares(data, &data->fb, mode, IGT_ROTATION_180);
 
 	/*
 	 * XXX: We always set the primary plane to actually enable the pipe as
@@ -121,7 +116,7 @@ static bool prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 	 * Step 2: prepare the plane with an non-rotated fb let the hw
 	 * rotate it.
 	 */
-	paint_squares(data, &data->fb, mode, DRM_ROTATE_0);
+	paint_squares(data, &data->fb, mode, IGT_ROTATION_0);
 
 	igt_plane_set_fb(plane, &data->fb);
 	igt_display_commit(display);
-- 
1.8.3.1

Re: [PATCH i-g-t 00/43] Rotation test

[Damien Lespiau]

On Thu, Jul 10, 2014 at 07:00:01PM +0100, Damien Lespiau wrote:
> I've taken the current rotation test to test the kernel patches and improved it
> a bit along the way. It's a bit like a detailed review, but with patches
> instead of comments.
> 
> With the rotation kernel patches and an IVB machine, that test now passes.
> 
> Part of the motivation has been to augment the igt_kms framework to deal with
> properties.  Another reason include the number of lines we need to write for a
> test. This series brings down that number from 325 to 129, something I can live
> with.

Whole series (but one patch after Daniel's comment) pushed to i-g-t
after Chris and Daniel ack on IRC.

This shouldn't blow up too hard, the test correctly skips when the
kernel support is lacking.

-- 
Damien