[PATCH i-g-t 1/3] lib/igt_fb: Allow specifiying object tiling when creating frame buffers

[PATCH i-g-t 1/3] lib/igt_fb: Allow specifiying object tiling when creating frame buffers

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Currently object tiling is inferred from the frame buffer modifier
and only for legacy X scanout.

It is useful to support overriding this selection for certain tests
so add the capability.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 lib/igt_fb.c     | 21 ++++++++++++++-------
 lib/igt_fb.h     |  3 ++-
 tests/kms_flip.c |  3 ++-
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index d04f02c0d035..73be93cb6e63 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -75,7 +75,8 @@ static struct format_desc_struct {
 
 /* helpers to create nice-looking framebuffers */
 static int create_bo_for_fb(int fd, int width, int height, int bpp,
-			    uint64_t tiling, unsigned bo_size,
+			    uint64_t tiling, unsigned int obj_tiling,
+			    unsigned bo_size,
 			    uint32_t *gem_handle_ret,
 			    unsigned *size_ret,
 			    unsigned *stride_ret)
@@ -113,7 +114,9 @@ static int create_bo_for_fb(int fd, int width, int height, int bpp,
 	gem_handle = gem_create(fd, bo_size);
 
 	if (tiling == LOCAL_I915_FORMAT_MOD_X_TILED)
-		ret = __gem_set_tiling(fd, gem_handle, I915_TILING_X, stride);
+		obj_tiling = I915_TILING_X;
+	if (obj_tiling != I915_TILING_NONE)
+		ret = __gem_set_tiling(fd, gem_handle, obj_tiling, stride);
 
 	*stride_ret = stride;
 	*size_ret = size;
@@ -414,7 +417,8 @@ void igt_paint_image(cairo_t *cr, const char *filename,
  */
 unsigned int
 igt_create_fb_with_bo_size(int fd, int width, int height,
-			   uint32_t format, uint64_t tiling,
+			   uint32_t format,
+			   uint64_t tiling, unsigned int obj_tiling,
 			   struct igt_fb *fb, unsigned bo_size)
 {
 	uint32_t fb_id;
@@ -426,8 +430,9 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
 
 	igt_debug("%s(width=%d, height=%d, format=0x%x [bpp=%d], tiling=0x%"PRIx64", size=%d)\n",
 		  __func__, width, height, format, bpp, tiling, bo_size);
-	do_or_die(create_bo_for_fb(fd, width, height, bpp, tiling, bo_size,
-				   &fb->gem_handle, &fb->size, &fb->stride));
+	do_or_die(create_bo_for_fb(fd, width, height, bpp, tiling, obj_tiling,
+				   bo_size, &fb->gem_handle, &fb->size,
+				   &fb->stride));
 
 	igt_debug("%s(handle=%d, pitch=%d)\n",
 		  __func__, fb->gem_handle, fb->stride);
@@ -485,7 +490,8 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
 unsigned int igt_create_fb(int fd, int width, int height, uint32_t format,
 			   uint64_t tiling, struct igt_fb *fb)
 {
-	return igt_create_fb_with_bo_size(fd, width, height, format, tiling, fb, 0);
+	return igt_create_fb_with_bo_size(fd, width, height, format,
+					  tiling, I915_TILING_NONE, fb, 0);
 }
 
 /**
@@ -714,7 +720,8 @@ static void create_cairo_surface__blit(int fd, struct igt_fb *fb)
 	 */
 	bpp = igt_drm_format_to_bpp(fb->drm_format);
 	ret = create_bo_for_fb(fd, fb->width, fb->height, bpp,
-				LOCAL_DRM_FORMAT_MOD_NONE, 0,
+				LOCAL_DRM_FORMAT_MOD_NONE, I915_TILING_NONE,
+				0,
 				&blit->linear.handle,
 				&blit->linear.size,
 				&blit->linear.stride);
diff --git a/lib/igt_fb.h b/lib/igt_fb.h
index a07acd2444b8..fa92fd4efd5b 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -71,7 +71,8 @@ enum igt_text_align {
 
 unsigned int
 igt_create_fb_with_bo_size(int fd, int width, int height,
-			   uint32_t format, uint64_t tiling,
+			   uint32_t format,
+			   uint64_t tiling, unsigned int obj_tiling,
 			   struct igt_fb *fb, unsigned bo_size);
 unsigned int igt_create_fb(int fd, int width, int height, uint32_t format,
 			   uint64_t tiling, struct igt_fb *fb);
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index a139d402608e..60577d23b9a4 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -1366,7 +1366,8 @@ static void run_test_on_crtc_set(struct test_output *o, int *crtc_idxs,
 					 tiling, &o->fb_info[0]);
 	o->fb_ids[1] = igt_create_fb_with_bo_size(drm_fd, o->fb_width, o->fb_height,
 					 igt_bpp_depth_to_drm_format(o->bpp, o->depth),
-					 tiling, &o->fb_info[1], bo_size);
+					 tiling, I915_TILING_NONE,
+					 &o->fb_info[1], bo_size);
 	o->fb_ids[2] = igt_create_fb(drm_fd, o->fb_width, o->fb_height,
 					 igt_bpp_depth_to_drm_format(o->bpp, o->depth),
 					 LOCAL_I915_FORMAT_MOD_X_TILED, &o->fb_info[2]);
-- 
1.9.1

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

[PATCH i-g-t 2/3] lib/igt_kms: Set new rotation property before displaying

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

This is not really tested that much apart that it doesn't break
kms_rotation_crc and it makes one new test case work.

It only serves as proof of concept to demonstrate a particular bug.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
---
 lib/igt_kms.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 51d735d29970..5c9b358ae23f 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1316,6 +1316,14 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
 	igt_assert(igt_plane_supports_rotation(plane) ||
 		   !plane->rotation_changed);
 
+	if (plane->rotation_changed) {
+		ret = igt_plane_set_property(plane, plane->rotation_property,
+				       plane->rotation);
+
+		plane->rotation_changed = false;
+		CHECK_RETURN(ret, fail_on_error);
+	}
+
 	fb_id = igt_plane_get_fb_id(plane);
 	crtc_id = output->config.crtc->crtc_id;
 
@@ -1377,14 +1385,6 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
 	plane->position_changed = false;
 	plane->size_changed = false;
 
-	if (plane->rotation_changed) {
-		ret = igt_plane_set_property(plane, plane->rotation_property,
-				       plane->rotation);
-
-		plane->rotation_changed = false;
-		CHECK_RETURN(ret, fail_on_error);
-	}
-
 	return 0;
 }
 
-- 
1.9.1

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

[PATCH i-g-t 3/3] kms_rotation_crc: Test case for rotated VMA first with legacy tiling

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Specifically targetting a WARN_ON in the kernel, i915_gem_fence.c:

		if (WARN_ON(!obj->map_and_fenceable))
			return -EINVAL;

Which happens for objects with legacy tiling set to Y at the point
object is pinned to display with no normal VMA in existance and
hence obj->map_and_fenceable false.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 tests/kms_rotation_crc.c | 121 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 96 insertions(+), 25 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index cc9847ecc113..6b16ec73bb9a 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -81,7 +81,8 @@ paint_squares(data_t *data, drmModeModeInfo *mode, igt_rotation_t rotation,
 	cairo_destroy(cr);
 }
 
-static void commit_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane)
+static void commit_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane,
+			bool render)
 {
 	igt_display_t *display = &data->display;
 	enum igt_commit_style commit = COMMIT_LEGACY;
@@ -108,17 +109,18 @@ static void commit_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane)
 		commit = COMMIT_UNIVERSAL;
 	}
 
-	igt_display_commit2(display, commit);
+	if (render)
+		igt_display_commit2(display, commit);
 }
 
 static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
-			 igt_plane_t *plane)
+			 igt_plane_t *plane, bool render)
 {
 	drmModeModeInfo *mode;
 	int fb_id, fb_modeset_id;
 	unsigned int w, h;
-	uint64_t tiling = data->override_tiling ?
-			  data->override_tiling : LOCAL_DRM_FORMAT_MOD_NONE;
+	uint64_t tiling;
+	unsigned int obj_tiling = !render ? I915_TILING_Y : I915_TILING_NONE;
 	uint32_t pixel_format = data->override_fmt ?
 				data->override_fmt : DRM_FORMAT_XRGB8888;
 
@@ -133,6 +135,14 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 	w = mode->hdisplay;
 	h = mode->vdisplay;
 
+	if (data->override_tiling)
+		tiling = data->override_tiling;
+	else if (data->rotation == IGT_ROTATION_90 ||
+		 data->rotation == IGT_ROTATION_270)
+		tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
+	else
+		tiling = LOCAL_DRM_FORMAT_MOD_NONE;
+
 	fb_modeset_id = igt_create_fb(data->gfx_fd,
 				      w, h,
 				      pixel_format,
@@ -146,8 +156,6 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 	 */
 	if (data->rotation == IGT_ROTATION_90 ||
 	    data->rotation == IGT_ROTATION_270) {
-		tiling = data->override_tiling ?
-			 data->override_tiling : LOCAL_I915_FORMAT_MOD_Y_TILED;
 		w = h =  mode->vdisplay;
 	} else if (plane->is_cursor) {
 		pixel_format = data->override_fmt ?
@@ -158,24 +166,29 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 	data->w = w;
 	data->h = h;
 
-	fb_id = igt_create_fb(data->gfx_fd,
-			      w, h,
-			      pixel_format,
-			      tiling,
-			      &data->fb);
+	fb_id = igt_create_fb_with_bo_size(data->gfx_fd,
+					   w, h,
+					   pixel_format,
+					   tiling, obj_tiling,
+					   &data->fb, 0);
 	igt_assert(fb_id);
 
-	/* Step 1: create a reference CRC for a software-rotated fb */
-	paint_squares(data, mode, data->rotation, plane);
-	commit_crtc(data, output, plane);
-	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, mode, IGT_ROTATION_0, plane);
-	igt_plane_set_fb(plane, &data->fb);
+	if (render) {
+		/* Step 1: create a reference CRC for a software-rotated fb */
+		paint_squares(data, mode, data->rotation, plane);
+		commit_crtc(data, output, plane, render);
+		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, mode, IGT_ROTATION_0, plane);
+		igt_plane_set_fb(plane, &data->fb);
+	} else {
+		commit_crtc(data, output, plane, render);
+		igt_plane_set_fb(plane, &data->fb);
+	}
 }
 
 static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane)
@@ -226,7 +239,7 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
 			plane = igt_output_get_plane(output, plane_type);
 			igt_require(igt_plane_supports_rotation(plane));
 
-			prepare_crtc(data, output, pipe, plane);
+			prepare_crtc(data, output, pipe, plane, true);
 
 			igt_display_commit2(display, commit);
 
@@ -252,7 +265,7 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
 			kmstest_restore_vt_mode();
 			kmstest_set_vt_graphics_mode();
 
-			commit_crtc(data, output, plane);
+			commit_crtc(data, output, plane, true);
 
 			igt_pipe_crc_collect_crc(data->pipe_crc, &crc_output);
 			igt_assert_crc_equal(&crc_unrotated, &crc_output);
@@ -264,6 +277,52 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
 	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
 }
 
+static void
+test_plane_rotation_first(data_t *data, enum igt_plane plane_type)
+{
+	igt_display_t *display = &data->display;
+	igt_output_t *output;
+	enum pipe pipe;
+	int valid_tests = 0;
+	enum igt_commit_style commit = COMMIT_LEGACY;
+	int ret;
+
+	if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
+		igt_require(data->display.has_universal_planes);
+		commit = COMMIT_UNIVERSAL;
+	}
+
+	for_each_connected_output(display, output) {
+		for_each_pipe(display, pipe) {
+			igt_plane_t *plane;
+
+			igt_output_set_pipe(output, pipe);
+
+			plane = igt_output_get_plane(output, plane_type);
+			igt_require(igt_plane_supports_rotation(plane));
+
+			prepare_crtc(data, output, pipe, plane, false);
+
+			igt_plane_set_rotation(plane, data->rotation);
+			ret = igt_display_try_commit2(display, commit);
+			igt_assert(ret == 0);
+
+			/*
+			 * check the rotation state has been reset when the VT
+			 * mode is restored
+			 */
+			kmstest_restore_vt_mode();
+			kmstest_set_vt_graphics_mode();
+
+			commit_crtc(data, output, plane, false);
+
+			valid_tests++;
+			cleanup_crtc(data, output, plane);
+		}
+	}
+	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
+}
+
 igt_main
 {
 	data_t data = {};
@@ -302,6 +361,18 @@ igt_main
 		test_plane_rotation(&data, IGT_PLANE_PRIMARY);
 	}
 
+	/*
+	 * Test rotated VMA instantiated first for the frame buffer object,
+	 * in other words with no normal VMA in existance first. Done by
+	 * avoiding rendering anything to the frame buffer and setting
+	 * rotation before commiting to display.
+	 */
+	igt_subtest_f("primary-rotation-90-first") {
+		igt_require(gen >= 9);
+		data.rotation = IGT_ROTATION_90;
+		test_plane_rotation_first(&data, IGT_PLANE_PRIMARY);
+	}
+
 	igt_subtest_f("primary-rotation-270") {
 		igt_require(gen >= 9);
 		data.rotation = IGT_ROTATION_270;
-- 
1.9.1

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

Re: [PATCH i-g-t 1/3] lib/igt_fb: Allow specifiying object tiling when creating frame buffers

[Ville Syrjälä]

On Fri, Oct 16, 2015 at 11:59:47AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Currently object tiling is inferred from the frame buffer modifier
> and only for legacy X scanout.
> 
> It is useful to support overriding this selection for certain tests
> so add the capability.

So you want to set up the object tiling differently from the fb
tiling? Why is that? And don't we reject it in the kernel? If we don't
need a fence for scanout (ie. FBC or gen2/3) we could allow it I
suppose, but not sure it it really helps with anything.

BTW I was thinking of allowing the test to pass in an already created
bo to igt_create_fb to use in some fb->offsets[] tests.

> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>  lib/igt_fb.c     | 21 ++++++++++++++-------
>  lib/igt_fb.h     |  3 ++-
>  tests/kms_flip.c |  3 ++-
>  3 files changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index d04f02c0d035..73be93cb6e63 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -75,7 +75,8 @@ static struct format_desc_struct {
>  
>  /* helpers to create nice-looking framebuffers */
>  static int create_bo_for_fb(int fd, int width, int height, int bpp,
> -			    uint64_t tiling, unsigned bo_size,
> +			    uint64_t tiling, unsigned int obj_tiling,
> +			    unsigned bo_size,
>  			    uint32_t *gem_handle_ret,
>  			    unsigned *size_ret,
>  			    unsigned *stride_ret)
> @@ -113,7 +114,9 @@ static int create_bo_for_fb(int fd, int width, int height, int bpp,
>  	gem_handle = gem_create(fd, bo_size);
>  
>  	if (tiling == LOCAL_I915_FORMAT_MOD_X_TILED)
> -		ret = __gem_set_tiling(fd, gem_handle, I915_TILING_X, stride);
> +		obj_tiling = I915_TILING_X;
> +	if (obj_tiling != I915_TILING_NONE)
> +		ret = __gem_set_tiling(fd, gem_handle, obj_tiling, stride);
>  
>  	*stride_ret = stride;
>  	*size_ret = size;
> @@ -414,7 +417,8 @@ void igt_paint_image(cairo_t *cr, const char *filename,
>   */
>  unsigned int
>  igt_create_fb_with_bo_size(int fd, int width, int height,
> -			   uint32_t format, uint64_t tiling,
> +			   uint32_t format,
> +			   uint64_t tiling, unsigned int obj_tiling,
>  			   struct igt_fb *fb, unsigned bo_size)
>  {
>  	uint32_t fb_id;
> @@ -426,8 +430,9 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
>  
>  	igt_debug("%s(width=%d, height=%d, format=0x%x [bpp=%d], tiling=0x%"PRIx64", size=%d)\n",
>  		  __func__, width, height, format, bpp, tiling, bo_size);
> -	do_or_die(create_bo_for_fb(fd, width, height, bpp, tiling, bo_size,
> -				   &fb->gem_handle, &fb->size, &fb->stride));
> +	do_or_die(create_bo_for_fb(fd, width, height, bpp, tiling, obj_tiling,
> +				   bo_size, &fb->gem_handle, &fb->size,
> +				   &fb->stride));
>  
>  	igt_debug("%s(handle=%d, pitch=%d)\n",
>  		  __func__, fb->gem_handle, fb->stride);
> @@ -485,7 +490,8 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
>  unsigned int igt_create_fb(int fd, int width, int height, uint32_t format,
>  			   uint64_t tiling, struct igt_fb *fb)
>  {
> -	return igt_create_fb_with_bo_size(fd, width, height, format, tiling, fb, 0);
> +	return igt_create_fb_with_bo_size(fd, width, height, format,
> +					  tiling, I915_TILING_NONE, fb, 0);
>  }
>  
>  /**
> @@ -714,7 +720,8 @@ static void create_cairo_surface__blit(int fd, struct igt_fb *fb)
>  	 */
>  	bpp = igt_drm_format_to_bpp(fb->drm_format);
>  	ret = create_bo_for_fb(fd, fb->width, fb->height, bpp,
> -				LOCAL_DRM_FORMAT_MOD_NONE, 0,
> +				LOCAL_DRM_FORMAT_MOD_NONE, I915_TILING_NONE,
> +				0,
>  				&blit->linear.handle,
>  				&blit->linear.size,
>  				&blit->linear.stride);
> diff --git a/lib/igt_fb.h b/lib/igt_fb.h
> index a07acd2444b8..fa92fd4efd5b 100644
> --- a/lib/igt_fb.h
> +++ b/lib/igt_fb.h
> @@ -71,7 +71,8 @@ enum igt_text_align {
>  
>  unsigned int
>  igt_create_fb_with_bo_size(int fd, int width, int height,
> -			   uint32_t format, uint64_t tiling,
> +			   uint32_t format,
> +			   uint64_t tiling, unsigned int obj_tiling,
>  			   struct igt_fb *fb, unsigned bo_size);
>  unsigned int igt_create_fb(int fd, int width, int height, uint32_t format,
>  			   uint64_t tiling, struct igt_fb *fb);
> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> index a139d402608e..60577d23b9a4 100644
> --- a/tests/kms_flip.c
> +++ b/tests/kms_flip.c
> @@ -1366,7 +1366,8 @@ static void run_test_on_crtc_set(struct test_output *o, int *crtc_idxs,
>  					 tiling, &o->fb_info[0]);
>  	o->fb_ids[1] = igt_create_fb_with_bo_size(drm_fd, o->fb_width, o->fb_height,
>  					 igt_bpp_depth_to_drm_format(o->bpp, o->depth),
> -					 tiling, &o->fb_info[1], bo_size);
> +					 tiling, I915_TILING_NONE,
> +					 &o->fb_info[1], bo_size);
>  	o->fb_ids[2] = igt_create_fb(drm_fd, o->fb_width, o->fb_height,
>  					 igt_bpp_depth_to_drm_format(o->bpp, o->depth),
>  					 LOCAL_I915_FORMAT_MOD_X_TILED, &o->fb_info[2]);
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 1/3] lib/igt_fb: Allow specifiying object tiling when creating frame buffers

[Tvrtko Ursulin]

Hi,

On 16/10/15 13:03, Ville Syrjälä wrote:
> On Fri, Oct 16, 2015 at 11:59:47AM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Currently object tiling is inferred from the frame buffer modifier
>> and only for legacy X scanout.
>>
>> It is useful to support overriding this selection for certain tests
>> so add the capability.
>
> So you want to set up the object tiling differently from the fb
> tiling? Why is that? And don't we reject it in the kernel? If we don't
> need a fence for scanout (ie. FBC or gen2/3) we could allow it I
> suppose, but not sure it it really helps with anything.

Hm, yes and no. Only different in a sense that currently igt_fb leaves 
object tiling at linear regardless of the fb modifier tiling. (Apart for 
the legacy X where it requires that they match.)

I needed a way of having Y tiled fb modifier and Y tiled object to hit a 
warning in i915_gem_object_get_fence when only the rotated view exists.

Patch series is probably to invasive anyway, especially I did not spend 
any time evaluating if 2/3 is safe. So hopefully Vivek can refine his 
version of the testcase which would then be completely confined to 
kms_rotation_crc.

Regards,

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

Re: [PATCH i-g-t 1/3] lib/igt_fb: Allow specifiying object tiling when creating frame buffers

[Ville Syrjälä]

On Fri, Oct 16, 2015 at 01:19:35PM +0100, Tvrtko Ursulin wrote:
> 
> Hi,
> 
> On 16/10/15 13:03, Ville Syrjälä wrote:
> > On Fri, Oct 16, 2015 at 11:59:47AM +0100, Tvrtko Ursulin wrote:
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> Currently object tiling is inferred from the frame buffer modifier
> >> and only for legacy X scanout.
> >>
> >> It is useful to support overriding this selection for certain tests
> >> so add the capability.
> >
> > So you want to set up the object tiling differently from the fb
> > tiling? Why is that? And don't we reject it in the kernel? If we don't
> > need a fence for scanout (ie. FBC or gen2/3) we could allow it I
> > suppose, but not sure it it really helps with anything.
> 
> Hm, yes and no. Only different in a sense that currently igt_fb leaves 
> object tiling at linear regardless of the fb modifier tiling. (Apart for 
> the legacy X where it requires that they match.)
> 
> I needed a way of having Y tiled fb modifier and Y tiled object to hit a 
> warning in i915_gem_object_get_fence when only the rotated view exists.

Is there a problem of just doing what X tiled did also for Y tiled?

> 
> Patch series is probably to invasive anyway, especially I did not spend 
> any time evaluating if 2/3 is safe. So hopefully Vivek can refine his 
> version of the testcase which would then be completely confined to 
> kms_rotation_crc.
> 
> Regards,
> 
> Tvrtko

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 1/3] lib/igt_fb: Allow specifiying object tiling when creating frame buffers

[Tvrtko Ursulin]

On 16/10/15 13:29, Ville Syrjälä wrote:
> On Fri, Oct 16, 2015 at 01:19:35PM +0100, Tvrtko Ursulin wrote:
>>
>> Hi,
>>
>> On 16/10/15 13:03, Ville Syrjälä wrote:
>>> On Fri, Oct 16, 2015 at 11:59:47AM +0100, Tvrtko Ursulin wrote:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> Currently object tiling is inferred from the frame buffer modifier
>>>> and only for legacy X scanout.
>>>>
>>>> It is useful to support overriding this selection for certain tests
>>>> so add the capability.
>>>
>>> So you want to set up the object tiling differently from the fb
>>> tiling? Why is that? And don't we reject it in the kernel? If we don't
>>> need a fence for scanout (ie. FBC or gen2/3) we could allow it I
>>> suppose, but not sure it it really helps with anything.
>>
>> Hm, yes and no. Only different in a sense that currently igt_fb leaves
>> object tiling at linear regardless of the fb modifier tiling. (Apart for
>> the legacy X where it requires that they match.)
>>
>> I needed a way of having Y tiled fb modifier and Y tiled object to hit a
>> warning in i915_gem_object_get_fence when only the rotated view exists.
>
> Is there a problem of just doing what X tiled did also for Y tiled?

What do you mean? In the kernel or igt_fb? If latter then it needs to be 
able to have object tiling as linear by default since that is how fb 
modifiers were intended to be used - decoupled from obj tiling. Is that 
what you meant?

Regards,

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

Re: [PATCH i-g-t 1/3] lib/igt_fb: Allow specifiying object tiling when creating frame buffers

[Ville Syrjälä]

On Fri, Oct 16, 2015 at 01:54:51PM +0100, Tvrtko Ursulin wrote:
> 
> On 16/10/15 13:29, Ville Syrjälä wrote:
> > On Fri, Oct 16, 2015 at 01:19:35PM +0100, Tvrtko Ursulin wrote:
> >>
> >> Hi,
> >>
> >> On 16/10/15 13:03, Ville Syrjälä wrote:
> >>> On Fri, Oct 16, 2015 at 11:59:47AM +0100, Tvrtko Ursulin wrote:
> >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>
> >>>> Currently object tiling is inferred from the frame buffer modifier
> >>>> and only for legacy X scanout.
> >>>>
> >>>> It is useful to support overriding this selection for certain tests
> >>>> so add the capability.
> >>>
> >>> So you want to set up the object tiling differently from the fb
> >>> tiling? Why is that? And don't we reject it in the kernel? If we don't
> >>> need a fence for scanout (ie. FBC or gen2/3) we could allow it I
> >>> suppose, but not sure it it really helps with anything.
> >>
> >> Hm, yes and no. Only different in a sense that currently igt_fb leaves
> >> object tiling at linear regardless of the fb modifier tiling. (Apart for
> >> the legacy X where it requires that they match.)
> >>
> >> I needed a way of having Y tiled fb modifier and Y tiled object to hit a
> >> warning in i915_gem_object_get_fence when only the rotated view exists.
> >
> > Is there a problem of just doing what X tiled did also for Y tiled?
> 
> What do you mean? In the kernel or igt_fb? If latter then it needs to be 
> able to have object tiling as linear by default since that is how fb 
> modifiers were intended to be used - decoupled from obj tiling. Is that 
> what you meant?

Just setting obj to Y tiled when fb is Y tiled from igt_fb.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 1/3] lib/igt_fb: Allow specifiying object tiling when creating frame buffers

[Tvrtko Ursulin]

On 16/10/15 14:01, Ville Syrjälä wrote:
> On Fri, Oct 16, 2015 at 01:54:51PM +0100, Tvrtko Ursulin wrote:
>>
>> On 16/10/15 13:29, Ville Syrjälä wrote:
>>> On Fri, Oct 16, 2015 at 01:19:35PM +0100, Tvrtko Ursulin wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 16/10/15 13:03, Ville Syrjälä wrote:
>>>>> On Fri, Oct 16, 2015 at 11:59:47AM +0100, Tvrtko Ursulin wrote:
>>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>
>>>>>> Currently object tiling is inferred from the frame buffer modifier
>>>>>> and only for legacy X scanout.
>>>>>>
>>>>>> It is useful to support overriding this selection for certain tests
>>>>>> so add the capability.
>>>>>
>>>>> So you want to set up the object tiling differently from the fb
>>>>> tiling? Why is that? And don't we reject it in the kernel? If we don't
>>>>> need a fence for scanout (ie. FBC or gen2/3) we could allow it I
>>>>> suppose, but not sure it it really helps with anything.
>>>>
>>>> Hm, yes and no. Only different in a sense that currently igt_fb leaves
>>>> object tiling at linear regardless of the fb modifier tiling. (Apart for
>>>> the legacy X where it requires that they match.)
>>>>
>>>> I needed a way of having Y tiled fb modifier and Y tiled object to hit a
>>>> warning in i915_gem_object_get_fence when only the rotated view exists.
>>>
>>> Is there a problem of just doing what X tiled did also for Y tiled?
>>
>> What do you mean? In the kernel or igt_fb? If latter then it needs to be
>> able to have object tiling as linear by default since that is how fb
>> modifiers were intended to be used - decoupled from obj tiling. Is that
>> what you meant?
>
> Just setting obj to Y tiled when fb is Y tiled from igt_fb.

Right, so I explained that - we want to have Y tiled fb modifier + 
linear object in the majority of cases, _apart_ from this special test 
case which tries to hit a WARN_ON on !obj->map_and_fenceable. Which 
happens when obj tiling is Y, and fb tiling is Y, _and_ no normal VMA 
exists, just the rotate one.

Regards,

Tvrtko



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

[PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier

[Vivek Kasireddy]

The main goal of this subtest is to verify whether flipping a
framebuffer with a Y fb modifier (90/270 degree rotation) and
an associated Y-tiled object works or not.

Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 tests/kms_rotation_crc.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index cc9847e..bb9aecb 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -264,6 +264,83 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
 	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
 }
 
+static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_type)
+{
+	igt_display_t *display = &data->display;
+	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
+	uint32_t format = DRM_FORMAT_XRGB8888;
+	int bpp = igt_drm_format_to_bpp(format);
+	enum igt_commit_style commit = COMMIT_LEGACY;
+	int fd = data->gfx_fd;
+	int valid_tests = 0;
+	igt_output_t *output;
+	int ret;
+
+	if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
+		igt_require(data->display.has_universal_planes);
+		commit = COMMIT_UNIVERSAL;
+	}
+
+	for_each_connected_output(display, output) {
+		igt_plane_t *plane;
+		drmModeModeInfo *mode = igt_output_get_mode(output);
+		unsigned int w = mode->hdisplay;
+		unsigned int h = mode->vdisplay;
+		unsigned int stride, size;
+		uint32_t gem_handle;
+
+		for (stride = 512; stride < (w * bpp / 8); stride *= 2)
+			;
+		for (size = 1024*1024; size < stride * h; size *= 2)
+			;
+
+		gem_handle = gem_create(fd, size);
+		ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride);
+		igt_assert(ret == 0);
+
+		do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
+			  format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
+			  &data->fb.fb_id));
+
+		data->fb.width = w;
+		data->fb.height = h;
+		data->fb.gem_handle = gem_handle;
+		data->fb.stride = stride;
+		data->fb.size = size;
+		data->fb.tiling = tiling;
+		data->fb.drm_format = format;
+
+		plane = igt_output_get_plane(output, plane_type);
+		igt_require(igt_plane_supports_rotation(plane));
+
+		igt_plane_set_fb(plane, NULL);
+		igt_display_commit(display);
+
+		igt_plane_set_rotation(plane, data->rotation);
+		paint_squares(data, mode, IGT_ROTATION_0, plane);
+		igt_plane_set_fb(plane, &data->fb);
+
+		drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
+					 DRM_MODE_OBJECT_PLANE,
+					 plane->rotation_property,
+					 plane->rotation);
+		ret = igt_display_try_commit2(display, commit);
+		igt_assert(ret == 0);
+
+		kmstest_restore_vt_mode();
+		kmstest_set_vt_graphics_mode();
+		igt_display_commit2(display, commit);
+
+		valid_tests++;
+		igt_remove_fb(fd, &data->fb);
+		igt_output_set_pipe(output, PIPE_ANY);
+
+		igt_plane_set_fb(plane, NULL);
+		igt_display_commit(display);
+	}
+	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
+}
+
 igt_main
 {
 	data_t data = {};
@@ -345,6 +422,12 @@ igt_main
 		test_plane_rotation(&data, IGT_PLANE_PRIMARY);
 	}
 
+	igt_subtest_f("primary-rotation-90-Y-tiled") {
+		igt_require(gen >= 9);
+		data.rotation = IGT_ROTATION_90;
+		test_plane_rotation_ytiled_obj(&data, IGT_PLANE_PRIMARY);
+	}
+
 	igt_fixture {
 		igt_display_fini(&data.display);
 	}
-- 
2.4.3

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

Re: [PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier

[Tvrtko Ursulin]

Hi,

On 17/10/15 03:47, Vivek Kasireddy wrote:
> The main goal of this subtest is to verify whether flipping a
> framebuffer with a Y fb modifier (90/270 degree rotation) and
> an associated Y-tiled object works or not.
>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   tests/kms_rotation_crc.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 83 insertions(+)
>
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index cc9847e..bb9aecb 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -264,6 +264,83 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
>   	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
>   }
>
> +static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_type)
> +{
> +	igt_display_t *display = &data->display;
> +	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
> +	uint32_t format = DRM_FORMAT_XRGB8888;
> +	int bpp = igt_drm_format_to_bpp(format);
> +	enum igt_commit_style commit = COMMIT_LEGACY;
> +	int fd = data->gfx_fd;
> +	int valid_tests = 0;
> +	igt_output_t *output;
> +	int ret;
> +
> +	if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
> +		igt_require(data->display.has_universal_planes);
> +		commit = COMMIT_UNIVERSAL;
> +	}
> +
> +	for_each_connected_output(display, output) {
> +		igt_plane_t *plane;
> +		drmModeModeInfo *mode = igt_output_get_mode(output);
> +		unsigned int w = mode->hdisplay;
> +		unsigned int h = mode->vdisplay;
> +		unsigned int stride, size;
> +		uint32_t gem_handle;
> +
> +		for (stride = 512; stride < (w * bpp / 8); stride *= 2)
> +			;
> +		for (size = 1024*1024; size < stride * h; size *= 2)
> +			;
> +
> +		gem_handle = gem_create(fd, size);
> +		ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride);
> +		igt_assert(ret == 0);
> +
> +		do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
> +			  format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
> +			  &data->fb.fb_id));
> +
> +		data->fb.width = w;
> +		data->fb.height = h;
> +		data->fb.gem_handle = gem_handle;
> +		data->fb.stride = stride;
> +		data->fb.size = size;
> +		data->fb.tiling = tiling;
> +		data->fb.drm_format = format;
> +
> +		plane = igt_output_get_plane(output, plane_type);
> +		igt_require(igt_plane_supports_rotation(plane));
> +
> +		igt_plane_set_fb(plane, NULL);
> +		igt_display_commit(display);
> +
> +		igt_plane_set_rotation(plane, data->rotation);
> +		paint_squares(data, mode, IGT_ROTATION_0, plane);
> +		igt_plane_set_fb(plane, &data->fb);
> +
> +		drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
> +					 DRM_MODE_OBJECT_PLANE,
> +					 plane->rotation_property,
> +					 plane->rotation);
> +		ret = igt_display_try_commit2(display, commit);
> +		igt_assert(ret == 0);

This manages to trigger the WARN? How come since paint_squares above 
will have created the normal VMA?

Regards,

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

Re: [PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier

[Vivek Kasireddy]

Hi Tvrtko,

On Mon, 19 Oct 2015 11:20:05 +0100
Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:

> 
> Hi,
> 
> On 17/10/15 03:47, Vivek Kasireddy wrote:
> > The main goal of this subtest is to verify whether flipping a
> > framebuffer with a Y fb modifier (90/270 degree rotation) and
> > an associated Y-tiled object works or not.
> >
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > ---
> >   tests/kms_rotation_crc.c | 83
> > ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83
> > insertions(+)
> >
> > diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> > index cc9847e..bb9aecb 100644
> > --- a/tests/kms_rotation_crc.c
> > +++ b/tests/kms_rotation_crc.c
> > @@ -264,6 +264,83 @@ static void test_plane_rotation(data_t *data,
> > enum igt_plane plane_type) igt_require_f(valid_tests, "no valid
> > crtc/connector combinations found\n"); }
> >
> > +static void test_plane_rotation_ytiled_obj(data_t *data, enum
> > igt_plane plane_type) +{
> > +	igt_display_t *display = &data->display;
> > +	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
> > +	uint32_t format = DRM_FORMAT_XRGB8888;
> > +	int bpp = igt_drm_format_to_bpp(format);
> > +	enum igt_commit_style commit = COMMIT_LEGACY;
> > +	int fd = data->gfx_fd;
> > +	int valid_tests = 0;
> > +	igt_output_t *output;
> > +	int ret;
> > +
> > +	if (plane_type == IGT_PLANE_PRIMARY || plane_type ==
> > IGT_PLANE_CURSOR) {
> > +		igt_require(data->display.has_universal_planes);
> > +		commit = COMMIT_UNIVERSAL;
> > +	}
> > +
> > +	for_each_connected_output(display, output) {
> > +		igt_plane_t *plane;
> > +		drmModeModeInfo *mode =
> > igt_output_get_mode(output);
> > +		unsigned int w = mode->hdisplay;
> > +		unsigned int h = mode->vdisplay;
> > +		unsigned int stride, size;
> > +		uint32_t gem_handle;
> > +
> > +		for (stride = 512; stride < (w * bpp / 8); stride
> > *= 2)
> > +			;
> > +		for (size = 1024*1024; size < stride * h; size *=
> > 2)
> > +			;
> > +
> > +		gem_handle = gem_create(fd, size);
> > +		ret = __gem_set_tiling(fd, gem_handle,
> > I915_TILING_Y, stride);
> > +		igt_assert(ret == 0);
> > +
> > +		do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
> > +			  format, tiling,
> > LOCAL_DRM_MODE_FB_MODIFIERS,
> > +			  &data->fb.fb_id));
> > +
> > +		data->fb.width = w;
> > +		data->fb.height = h;
> > +		data->fb.gem_handle = gem_handle;
> > +		data->fb.stride = stride;
> > +		data->fb.size = size;
> > +		data->fb.tiling = tiling;
> > +		data->fb.drm_format = format;
> > +
> > +		plane = igt_output_get_plane(output, plane_type);
> > +		igt_require(igt_plane_supports_rotation(plane));
> > +
> > +		igt_plane_set_fb(plane, NULL);
> > +		igt_display_commit(display);
> > +
> > +		igt_plane_set_rotation(plane, data->rotation);
> > +		paint_squares(data, mode, IGT_ROTATION_0, plane);
> > +		igt_plane_set_fb(plane, &data->fb);
> > +
> > +		drmModeObjectSetProperty(fd,
> > plane->drm_plane->plane_id,
> > +					 DRM_MODE_OBJECT_PLANE,
> > +					 plane->rotation_property,
> > +					 plane->rotation);
> > +		ret = igt_display_try_commit2(display, commit);
> > +		igt_assert(ret == 0);
> 
> This manages to trigger the WARN? How come since paint_squares above 
> will have created the normal VMA?

Yes, this does trigger the WARN regardless of whether paint_squares is
called or not. I am going to have to find out why the normal VMA was
not created when paint_squares was called. And, this subtest does not
restore the console cleanly which is something I also have to fix.


Thanks,
Vivek

> 
> Regards,
> 
> Tvrtko

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

Re: [PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier

[Tvrtko Ursulin]

On 20/10/15 02:14, Vivek Kasireddy wrote:
> Hi Tvrtko,
>
> On Mon, 19 Oct 2015 11:20:05 +0100
> Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>
>>
>> Hi,
>>
>> On 17/10/15 03:47, Vivek Kasireddy wrote:
>>> The main goal of this subtest is to verify whether flipping a
>>> framebuffer with a Y fb modifier (90/270 degree rotation) and
>>> an associated Y-tiled object works or not.
>>>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>>> ---
>>>    tests/kms_rotation_crc.c | 83
>>> ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83
>>> insertions(+)
>>>
>>> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
>>> index cc9847e..bb9aecb 100644
>>> --- a/tests/kms_rotation_crc.c
>>> +++ b/tests/kms_rotation_crc.c
>>> @@ -264,6 +264,83 @@ static void test_plane_rotation(data_t *data,
>>> enum igt_plane plane_type) igt_require_f(valid_tests, "no valid
>>> crtc/connector combinations found\n"); }
>>>
>>> +static void test_plane_rotation_ytiled_obj(data_t *data, enum
>>> igt_plane plane_type) +{
>>> +	igt_display_t *display = &data->display;
>>> +	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
>>> +	uint32_t format = DRM_FORMAT_XRGB8888;
>>> +	int bpp = igt_drm_format_to_bpp(format);
>>> +	enum igt_commit_style commit = COMMIT_LEGACY;
>>> +	int fd = data->gfx_fd;
>>> +	int valid_tests = 0;
>>> +	igt_output_t *output;
>>> +	int ret;
>>> +
>>> +	if (plane_type == IGT_PLANE_PRIMARY || plane_type ==
>>> IGT_PLANE_CURSOR) {
>>> +		igt_require(data->display.has_universal_planes);
>>> +		commit = COMMIT_UNIVERSAL;
>>> +	}
>>> +
>>> +	for_each_connected_output(display, output) {
>>> +		igt_plane_t *plane;
>>> +		drmModeModeInfo *mode =
>>> igt_output_get_mode(output);
>>> +		unsigned int w = mode->hdisplay;
>>> +		unsigned int h = mode->vdisplay;
>>> +		unsigned int stride, size;
>>> +		uint32_t gem_handle;
>>> +
>>> +		for (stride = 512; stride < (w * bpp / 8); stride
>>> *= 2)
>>> +			;
>>> +		for (size = 1024*1024; size < stride * h; size *=
>>> 2)
>>> +			;
>>> +
>>> +		gem_handle = gem_create(fd, size);
>>> +		ret = __gem_set_tiling(fd, gem_handle,
>>> I915_TILING_Y, stride);
>>> +		igt_assert(ret == 0);
>>> +
>>> +		do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
>>> +			  format, tiling,
>>> LOCAL_DRM_MODE_FB_MODIFIERS,
>>> +			  &data->fb.fb_id));
>>> +
>>> +		data->fb.width = w;
>>> +		data->fb.height = h;
>>> +		data->fb.gem_handle = gem_handle;
>>> +		data->fb.stride = stride;
>>> +		data->fb.size = size;
>>> +		data->fb.tiling = tiling;
>>> +		data->fb.drm_format = format;
>>> +
>>> +		plane = igt_output_get_plane(output, plane_type);
>>> +		igt_require(igt_plane_supports_rotation(plane));
>>> +
>>> +		igt_plane_set_fb(plane, NULL);
>>> +		igt_display_commit(display);
>>> +
>>> +		igt_plane_set_rotation(plane, data->rotation);
>>> +		paint_squares(data, mode, IGT_ROTATION_0, plane);
>>> +		igt_plane_set_fb(plane, &data->fb);
>>> +
>>> +		drmModeObjectSetProperty(fd,
>>> plane->drm_plane->plane_id,
>>> +					 DRM_MODE_OBJECT_PLANE,
>>> +					 plane->rotation_property,
>>> +					 plane->rotation);
>>> +		ret = igt_display_try_commit2(display, commit);
>>> +		igt_assert(ret == 0);
>>
>> This manages to trigger the WARN? How come since paint_squares above
>> will have created the normal VMA?
>
> Yes, this does trigger the WARN regardless of whether paint_squares is
> called or not. I am going to have to find out why the normal VMA was
> not created when paint_squares was called. And, this subtest does not
> restore the console cleanly which is something I also have to fix.

Could be that normal VMA is not created in the Cairo rendering path for 
Y tiled buffers. Since the final rendering is done via a blit and if you 
have PPGTT that would do it I think.

If PPGTT is always true for gen9 then this could work.

Regards,

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

Re: [PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier

[Vivek Kasireddy]

On Tue, 20 Oct 2015 10:10:20 +0100
Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:

> 
> On 20/10/15 02:14, Vivek Kasireddy wrote:
> > Hi Tvrtko,
> >
> > On Mon, 19 Oct 2015 11:20:05 +0100
> > Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> >
> >>
> >> Hi,
> >>
> >> On 17/10/15 03:47, Vivek Kasireddy wrote:
> >>> The main goal of this subtest is to verify whether flipping a
> >>> framebuffer with a Y fb modifier (90/270 degree rotation) and
> >>> an associated Y-tiled object works or not.
> >>>
> >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> >>> ---
> >>>    tests/kms_rotation_crc.c | 83
> >>> ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed,
> >>> 83 insertions(+)
> >>>
> >>> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> >>> index cc9847e..bb9aecb 100644
> >>> --- a/tests/kms_rotation_crc.c
> >>> +++ b/tests/kms_rotation_crc.c
> >>> @@ -264,6 +264,83 @@ static void test_plane_rotation(data_t *data,
> >>> enum igt_plane plane_type) igt_require_f(valid_tests, "no valid
> >>> crtc/connector combinations found\n"); }
> >>>
> >>> +static void test_plane_rotation_ytiled_obj(data_t *data, enum
> >>> igt_plane plane_type) +{
> >>> +	igt_display_t *display = &data->display;
> >>> +	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
> >>> +	uint32_t format = DRM_FORMAT_XRGB8888;
> >>> +	int bpp = igt_drm_format_to_bpp(format);
> >>> +	enum igt_commit_style commit = COMMIT_LEGACY;
> >>> +	int fd = data->gfx_fd;
> >>> +	int valid_tests = 0;
> >>> +	igt_output_t *output;
> >>> +	int ret;
> >>> +
> >>> +	if (plane_type == IGT_PLANE_PRIMARY || plane_type ==
> >>> IGT_PLANE_CURSOR) {
> >>> +		igt_require(data->display.has_universal_planes);
> >>> +		commit = COMMIT_UNIVERSAL;
> >>> +	}
> >>> +
> >>> +	for_each_connected_output(display, output) {
> >>> +		igt_plane_t *plane;
> >>> +		drmModeModeInfo *mode =
> >>> igt_output_get_mode(output);
> >>> +		unsigned int w = mode->hdisplay;
> >>> +		unsigned int h = mode->vdisplay;
> >>> +		unsigned int stride, size;
> >>> +		uint32_t gem_handle;
> >>> +
> >>> +		for (stride = 512; stride < (w * bpp / 8); stride
> >>> *= 2)
> >>> +			;
> >>> +		for (size = 1024*1024; size < stride * h; size *=
> >>> 2)
> >>> +			;
> >>> +
> >>> +		gem_handle = gem_create(fd, size);
> >>> +		ret = __gem_set_tiling(fd, gem_handle,
> >>> I915_TILING_Y, stride);
> >>> +		igt_assert(ret == 0);
> >>> +
> >>> +		do_or_die(__kms_addfb(fd, gem_handle, w, h,
> >>> stride,
> >>> +			  format, tiling,
> >>> LOCAL_DRM_MODE_FB_MODIFIERS,
> >>> +			  &data->fb.fb_id));
> >>> +
> >>> +		data->fb.width = w;
> >>> +		data->fb.height = h;
> >>> +		data->fb.gem_handle = gem_handle;
> >>> +		data->fb.stride = stride;
> >>> +		data->fb.size = size;
> >>> +		data->fb.tiling = tiling;
> >>> +		data->fb.drm_format = format;
> >>> +
> >>> +		plane = igt_output_get_plane(output, plane_type);
> >>> +		igt_require(igt_plane_supports_rotation(plane));
> >>> +
> >>> +		igt_plane_set_fb(plane, NULL);
> >>> +		igt_display_commit(display);
> >>> +
> >>> +		igt_plane_set_rotation(plane, data->rotation);
> >>> +		paint_squares(data, mode, IGT_ROTATION_0, plane);
> >>> +		igt_plane_set_fb(plane, &data->fb);
> >>> +
> >>> +		drmModeObjectSetProperty(fd,
> >>> plane->drm_plane->plane_id,
> >>> +					 DRM_MODE_OBJECT_PLANE,
> >>> +
> >>> plane->rotation_property,
> >>> +					 plane->rotation);
> >>> +		ret = igt_display_try_commit2(display, commit);
> >>> +		igt_assert(ret == 0);
> >>
> >> This manages to trigger the WARN? How come since paint_squares
> >> above will have created the normal VMA?
> >
> > Yes, this does trigger the WARN regardless of whether paint_squares
> > is called or not. I am going to have to find out why the normal VMA
> > was not created when paint_squares was called. And, this subtest
> > does not restore the console cleanly which is something I also have
> > to fix.
> 
> Could be that normal VMA is not created in the Cairo rendering path
> for Y tiled buffers. Since the final rendering is done via a blit and
> if you have PPGTT that would do it I think.
> 
> If PPGTT is always true for gen9 then this could work.

Hi Tvrtko,
Yes, I see that PPGTT is always true for gen9 and perhaps the reason
why the normal VMA is not created. Do you suggest that I get rid of the
call to paint_squares and further simplify the test by just flipping the
fb to only one output?

Thanks and Regards,
Vivek

> 
> Regards,
> 
> Tvrtko

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

[PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier (v2)

[Vivek Kasireddy]

The main goal of this subtest is to verify whether flipping a
framebuffer with a Y fb modifier (90/270 degree rotation) and
with an associated Y-tiled object works or not.

v2: Do not call paint_squares and just use one output.

Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 tests/kms_rotation_crc.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index cc9847e..63f27f8 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -264,6 +264,73 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
 	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
 }
 
+static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_type)
+{
+	igt_display_t *display = &data->display;
+	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
+	uint32_t format = DRM_FORMAT_XRGB8888;
+	int bpp = igt_drm_format_to_bpp(format);
+	enum igt_commit_style commit = COMMIT_LEGACY;
+	int fd = data->gfx_fd;
+	int valid_tests = 0;
+	igt_output_t *output = &display->outputs[0];
+	int ret;
+
+	if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
+		igt_require(data->display.has_universal_planes);
+		commit = COMMIT_UNIVERSAL;
+	}
+
+	if (output && output->valid) {
+		igt_plane_t *plane;
+		drmModeModeInfo *mode = igt_output_get_mode(output);
+		unsigned int w = mode->hdisplay;
+		unsigned int h = mode->vdisplay;
+		unsigned int stride, size;
+		uint32_t gem_handle;
+
+		for (stride = 512; stride < (w * bpp / 8); stride *= 2)
+			;
+		for (size = 1024*1024; size < stride * h; size *= 2)
+			;
+
+		gem_handle = gem_create(fd, size);
+		ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride);
+		igt_assert(ret == 0);
+
+		do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
+			  format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
+			  &data->fb.fb_id));
+		data->fb.width = w;
+		data->fb.height = h;
+		data->fb.gem_handle = gem_handle;
+
+		plane = igt_output_get_plane(output, plane_type);
+		igt_require(igt_plane_supports_rotation(plane));
+
+		igt_plane_set_fb(plane, NULL);
+		igt_display_commit(display);
+
+		igt_plane_set_rotation(plane, data->rotation);
+		igt_plane_set_fb(plane, &data->fb);
+
+		drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
+					 DRM_MODE_OBJECT_PLANE,
+					 plane->rotation_property,
+					 plane->rotation);
+		ret = igt_display_try_commit2(display, commit);
+
+		kmstest_restore_vt_mode();
+		kmstest_set_vt_graphics_mode();
+
+		igt_remove_fb(fd, &data->fb);
+		igt_assert(ret == 0);
+
+		valid_tests++;
+	}
+	igt_require_f(valid_tests, "no valid output found\n");
+}
+
 igt_main
 {
 	data_t data = {};
@@ -345,6 +412,12 @@ igt_main
 		test_plane_rotation(&data, IGT_PLANE_PRIMARY);
 	}
 
+	igt_subtest_f("primary-rotation-90-Y-tiled") {
+		igt_require(gen >= 9);
+		data.rotation = IGT_ROTATION_90;
+		test_plane_rotation_ytiled_obj(&data, IGT_PLANE_PRIMARY);
+	}
+
 	igt_fixture {
 		igt_display_fini(&data.display);
 	}
-- 
2.4.3

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

Re: [PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier (v2)

[Tvrtko Ursulin]

Hi,

On 22/10/15 02:24, Vivek Kasireddy wrote:
> The main goal of this subtest is to verify whether flipping a

Need to change to commit message since there is no flipping involved.

> framebuffer with a Y fb modifier (90/270 degree rotation) and
> with an associated Y-tiled object works or not.

And also explain in more detail the requirement to not have a normal VMA 
before the rotated one is displayed.

> v2: Do not call paint_squares and just use one output.
>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   tests/kms_rotation_crc.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 73 insertions(+)
>
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index cc9847e..63f27f8 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -264,6 +264,73 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
>   	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
>   }
>
> +static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_type)
> +{
> +	igt_display_t *display = &data->display;
> +	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
> +	uint32_t format = DRM_FORMAT_XRGB8888;
> +	int bpp = igt_drm_format_to_bpp(format);
> +	enum igt_commit_style commit = COMMIT_LEGACY;
> +	int fd = data->gfx_fd;
> +	int valid_tests = 0;
> +	igt_output_t *output = &display->outputs[0];
> +	int ret;
> +
> +	if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
> +		igt_require(data->display.has_universal_planes);
> +		commit = COMMIT_UNIVERSAL;
> +	}
> +
> +	if (output && output->valid) {

How about getting rid of this indentation level and valid_tests by just 
doing igt_require on the condition above?

> +		igt_plane_t *plane;
> +		drmModeModeInfo *mode = igt_output_get_mode(output);
> +		unsigned int w = mode->hdisplay;
> +		unsigned int h = mode->vdisplay;
> +		unsigned int stride, size;
> +		uint32_t gem_handle;
> +
> +		for (stride = 512; stride < (w * bpp / 8); stride *= 2)
> +			;
> +		for (size = 1024*1024; size < stride * h; size *= 2)
> +			;
> +
> +		gem_handle = gem_create(fd, size);
> +		ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride);
> +		igt_assert(ret == 0);
> +
> +		do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
> +			  format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
> +			  &data->fb.fb_id));
> +		data->fb.width = w;
> +		data->fb.height = h;
> +		data->fb.gem_handle = gem_handle;
> +
> +		plane = igt_output_get_plane(output, plane_type);
> +		igt_require(igt_plane_supports_rotation(plane));

This would maybe be cleaner above the fb allocation, to group all the 
asserts together.

Otherwise looks fine to me.

Regards,

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

[PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier (v3)

[Vivek Kasireddy]

The main goal of this subtest is to trigger the following warning in
the function i915_gem_object_get_fence():
	if (WARN_ON(!obj->map_and_fenceable))

To trigger this warning, the subtest first creates a Y-tiled object and
an associated framebuffer with the Y-fb modifier. Furthermore, to
prevent the map_and_fenceable from being set, we make sure that
the object does not have a normal VMA by refraining from rendering to the
object and by setting the rotation property upfront before calling commit.

v2: Do not call paint_squares and just use one output.

v3: Convert an if condition to igt_require and move the plane rotation
requirement further up before the fb allocation.

Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 tests/kms_rotation_crc.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index cc9847e..b25a949 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -264,6 +264,68 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
 	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
 }
 
+static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_type)
+{
+	igt_display_t *display = &data->display;
+	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
+	uint32_t format = DRM_FORMAT_XRGB8888;
+	int bpp = igt_drm_format_to_bpp(format);
+	enum igt_commit_style commit = COMMIT_LEGACY;
+	int fd = data->gfx_fd;
+	igt_output_t *output = &display->outputs[0];
+	igt_plane_t *plane;
+	drmModeModeInfo *mode;
+	unsigned int stride, size, w, h;
+	uint32_t gem_handle;
+	int ret;
+
+	igt_require(output != NULL && output->valid == true);
+
+	plane = igt_output_get_plane(output, plane_type);
+	igt_require(igt_plane_supports_rotation(plane));
+
+	if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
+		igt_require(data->display.has_universal_planes);
+		commit = COMMIT_UNIVERSAL;
+	}
+
+	mode = igt_output_get_mode(output);
+	w = mode->hdisplay;
+	h = mode->vdisplay;
+
+	for (stride = 512; stride < (w * bpp / 8); stride *= 2)
+		;
+	for (size = 1024*1024; size < stride * h; size *= 2)
+		;
+
+	gem_handle = gem_create(fd, size);
+	ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride);
+	igt_assert(ret == 0);
+
+	do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
+		  format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
+		  &data->fb.fb_id));
+	data->fb.width = w;
+	data->fb.height = h;
+	data->fb.gem_handle = gem_handle;
+
+	igt_plane_set_fb(plane, NULL);
+	igt_display_commit(display);
+
+	igt_plane_set_rotation(plane, data->rotation);
+	igt_plane_set_fb(plane, &data->fb);
+
+	drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
+				 DRM_MODE_OBJECT_PLANE,
+				 plane->rotation_property,
+				 plane->rotation);
+	ret = igt_display_try_commit2(display, commit);
+
+	kmstest_restore_vt_mode();
+	igt_remove_fb(fd, &data->fb);
+	igt_assert(ret == 0);
+}
+
 igt_main
 {
 	data_t data = {};
@@ -345,6 +407,12 @@ igt_main
 		test_plane_rotation(&data, IGT_PLANE_PRIMARY);
 	}
 
+	igt_subtest_f("primary-rotation-90-Y-tiled") {
+		igt_require(gen >= 9);
+		data.rotation = IGT_ROTATION_90;
+		test_plane_rotation_ytiled_obj(&data, IGT_PLANE_PRIMARY);
+	}
+
 	igt_fixture {
 		igt_display_fini(&data.display);
 	}
-- 
2.4.3

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

Re: [PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier (v3)

[Tvrtko Ursulin]

Hi,

On 23/10/15 02:34, Vivek Kasireddy wrote:
> The main goal of this subtest is to trigger the following warning in
> the function i915_gem_object_get_fence():
> 	if (WARN_ON(!obj->map_and_fenceable))
>
> To trigger this warning, the subtest first creates a Y-tiled object and
> an associated framebuffer with the Y-fb modifier. Furthermore, to
> prevent the map_and_fenceable from being set, we make sure that
> the object does not have a normal VMA by refraining from rendering to the
> object and by setting the rotation property upfront before calling commit.
>
> v2: Do not call paint_squares and just use one output.
>
> v3: Convert an if condition to igt_require and move the plane rotation
> requirement further up before the fb allocation.
>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   tests/kms_rotation_crc.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 68 insertions(+)
>
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index cc9847e..b25a949 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -264,6 +264,68 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
>   	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
>   }
>
> +static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_type)
> +{
> +	igt_display_t *display = &data->display;
> +	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
> +	uint32_t format = DRM_FORMAT_XRGB8888;
> +	int bpp = igt_drm_format_to_bpp(format);
> +	enum igt_commit_style commit = COMMIT_LEGACY;
> +	int fd = data->gfx_fd;
> +	igt_output_t *output = &display->outputs[0];
> +	igt_plane_t *plane;
> +	drmModeModeInfo *mode;
> +	unsigned int stride, size, w, h;
> +	uint32_t gem_handle;
> +	int ret;
> +
> +	igt_require(output != NULL && output->valid == true);
> +
> +	plane = igt_output_get_plane(output, plane_type);
> +	igt_require(igt_plane_supports_rotation(plane));
> +
> +	if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
> +		igt_require(data->display.has_universal_planes);
> +		commit = COMMIT_UNIVERSAL;
> +	}
> +
> +	mode = igt_output_get_mode(output);
> +	w = mode->hdisplay;
> +	h = mode->vdisplay;
> +
> +	for (stride = 512; stride < (w * bpp / 8); stride *= 2)
> +		;
> +	for (size = 1024*1024; size < stride * h; size *= 2)
> +		;
> +
> +	gem_handle = gem_create(fd, size);
> +	ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride);
> +	igt_assert(ret == 0);
> +
> +	do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
> +		  format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
> +		  &data->fb.fb_id));
> +	data->fb.width = w;
> +	data->fb.height = h;
> +	data->fb.gem_handle = gem_handle;
> +
> +	igt_plane_set_fb(plane, NULL);
> +	igt_display_commit(display);
> +
> +	igt_plane_set_rotation(plane, data->rotation);
> +	igt_plane_set_fb(plane, &data->fb);
> +
> +	drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
> +				 DRM_MODE_OBJECT_PLANE,
> +				 plane->rotation_property,
> +				 plane->rotation);
> +	ret = igt_display_try_commit2(display, commit);
> +
> +	kmstest_restore_vt_mode();
> +	igt_remove_fb(fd, &data->fb);
> +	igt_assert(ret == 0);
> +}
> +
>   igt_main
>   {
>   	data_t data = {};
> @@ -345,6 +407,12 @@ igt_main
>   		test_plane_rotation(&data, IGT_PLANE_PRIMARY);
>   	}
>
> +	igt_subtest_f("primary-rotation-90-Y-tiled") {
> +		igt_require(gen >= 9);
> +		data.rotation = IGT_ROTATION_90;
> +		test_plane_rotation_ytiled_obj(&data, IGT_PLANE_PRIMARY);
> +	}
> +
>   	igt_fixture {
>   		igt_display_fini(&data.display);
>   	}
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

Re: [PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier (v3)

[Daniel Vetter]

On Fri, Oct 23, 2015 at 09:51:06AM +0100, Tvrtko Ursulin wrote:
> 
> Hi,
> 
> On 23/10/15 02:34, Vivek Kasireddy wrote:
> >The main goal of this subtest is to trigger the following warning in
> >the function i915_gem_object_get_fence():
> >	if (WARN_ON(!obj->map_and_fenceable))
> >
> >To trigger this warning, the subtest first creates a Y-tiled object and
> >an associated framebuffer with the Y-fb modifier. Furthermore, to
> >prevent the map_and_fenceable from being set, we make sure that
> >the object does not have a normal VMA by refraining from rendering to the
> >object and by setting the rotation property upfront before calling commit.
> >
> >v2: Do not call paint_squares and just use one output.
> >
> >v3: Convert an if condition to igt_require and move the plane rotation
> >requirement further up before the fb allocation.
> >
> >Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> >---
> >  tests/kms_rotation_crc.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 68 insertions(+)
> >
> >diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> >index cc9847e..b25a949 100644
> >--- a/tests/kms_rotation_crc.c
> >+++ b/tests/kms_rotation_crc.c
> >@@ -264,6 +264,68 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
> >  	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
> >  }
> >
> >+static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_type)
> >+{
> >+	igt_display_t *display = &data->display;
> >+	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
> >+	uint32_t format = DRM_FORMAT_XRGB8888;
> >+	int bpp = igt_drm_format_to_bpp(format);
> >+	enum igt_commit_style commit = COMMIT_LEGACY;
> >+	int fd = data->gfx_fd;
> >+	igt_output_t *output = &display->outputs[0];
> >+	igt_plane_t *plane;
> >+	drmModeModeInfo *mode;
> >+	unsigned int stride, size, w, h;
> >+	uint32_t gem_handle;
> >+	int ret;
> >+
> >+	igt_require(output != NULL && output->valid == true);
> >+
> >+	plane = igt_output_get_plane(output, plane_type);
> >+	igt_require(igt_plane_supports_rotation(plane));
> >+
> >+	if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
> >+		igt_require(data->display.has_universal_planes);
> >+		commit = COMMIT_UNIVERSAL;
> >+	}
> >+
> >+	mode = igt_output_get_mode(output);
> >+	w = mode->hdisplay;
> >+	h = mode->vdisplay;
> >+
> >+	for (stride = 512; stride < (w * bpp / 8); stride *= 2)
> >+		;
> >+	for (size = 1024*1024; size < stride * h; size *= 2)
> >+		;
> >+
> >+	gem_handle = gem_create(fd, size);
> >+	ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride);
> >+	igt_assert(ret == 0);
> >+
> >+	do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
> >+		  format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
> >+		  &data->fb.fb_id));
> >+	data->fb.width = w;
> >+	data->fb.height = h;
> >+	data->fb.gem_handle = gem_handle;
> >+
> >+	igt_plane_set_fb(plane, NULL);
> >+	igt_display_commit(display);
> >+
> >+	igt_plane_set_rotation(plane, data->rotation);
> >+	igt_plane_set_fb(plane, &data->fb);
> >+
> >+	drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
> >+				 DRM_MODE_OBJECT_PLANE,
> >+				 plane->rotation_property,
> >+				 plane->rotation);
> >+	ret = igt_display_try_commit2(display, commit);
> >+
> >+	kmstest_restore_vt_mode();
> >+	igt_remove_fb(fd, &data->fb);
> >+	igt_assert(ret == 0);
> >+}
> >+
> >  igt_main
> >  {
> >  	data_t data = {};
> >@@ -345,6 +407,12 @@ igt_main
> >  		test_plane_rotation(&data, IGT_PLANE_PRIMARY);
> >  	}
> >
> >+	igt_subtest_f("primary-rotation-90-Y-tiled") {
> >+		igt_require(gen >= 9);
> >+		data.rotation = IGT_ROTATION_90;
> >+		test_plane_rotation_ytiled_obj(&data, IGT_PLANE_PRIMARY);
> >+	}
> >+
> >  	igt_fixture {
> >  		igt_display_fini(&data.display);
> >  	}
> >
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Applied, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier (v3)

[Vivek Kasireddy]

On Fri, 23 Oct 2015 13:35:21 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Fri, Oct 23, 2015 at 09:51:06AM +0100, Tvrtko Ursulin wrote:
> > 
> > Hi,
> > 
> > On 23/10/15 02:34, Vivek Kasireddy wrote:
> > >The main goal of this subtest is to trigger the following warning
> > >in the function i915_gem_object_get_fence():
> > >	if (WARN_ON(!obj->map_and_fenceable))
> > >
> > >To trigger this warning, the subtest first creates a Y-tiled
> > >object and an associated framebuffer with the Y-fb modifier.
> > >Furthermore, to prevent the map_and_fenceable from being set, we
> > >make sure that the object does not have a normal VMA by refraining
> > >from rendering to the object and by setting the rotation property
> > >upfront before calling commit.
> > >
> > >v2: Do not call paint_squares and just use one output.
> > >
> > >v3: Convert an if condition to igt_require and move the plane
> > >rotation requirement further up before the fb allocation.
> > >
> > >Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > >Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > >---
> > >  tests/kms_rotation_crc.c | 68
> > > ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed,
> > > 68 insertions(+)
> > >
> > >diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> > >index cc9847e..b25a949 100644
> > >--- a/tests/kms_rotation_crc.c
> > >+++ b/tests/kms_rotation_crc.c
> > >@@ -264,6 +264,68 @@ static void test_plane_rotation(data_t *data,
> > >enum igt_plane plane_type)
> > >  	igt_require_f(valid_tests, "no valid crtc/connector
> > > combinations found\n"); }
> > >
> > >+static void test_plane_rotation_ytiled_obj(data_t *data, enum
> > >igt_plane plane_type) +{
> > >+	igt_display_t *display = &data->display;
> > >+	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
> > >+	uint32_t format = DRM_FORMAT_XRGB8888;
> > >+	int bpp = igt_drm_format_to_bpp(format);
> > >+	enum igt_commit_style commit = COMMIT_LEGACY;
> > >+	int fd = data->gfx_fd;
> > >+	igt_output_t *output = &display->outputs[0];
> > >+	igt_plane_t *plane;
> > >+	drmModeModeInfo *mode;
> > >+	unsigned int stride, size, w, h;
> > >+	uint32_t gem_handle;
> > >+	int ret;
> > >+
> > >+	igt_require(output != NULL && output->valid == true);
> > >+
> > >+	plane = igt_output_get_plane(output, plane_type);
> > >+	igt_require(igt_plane_supports_rotation(plane));
> > >+
> > >+	if (plane_type == IGT_PLANE_PRIMARY || plane_type ==
> > >IGT_PLANE_CURSOR) {
> > >+		igt_require(data->display.has_universal_planes);
> > >+		commit = COMMIT_UNIVERSAL;
> > >+	}
> > >+
> > >+	mode = igt_output_get_mode(output);
> > >+	w = mode->hdisplay;
> > >+	h = mode->vdisplay;
> > >+
> > >+	for (stride = 512; stride < (w * bpp / 8); stride *= 2)
> > >+		;
> > >+	for (size = 1024*1024; size < stride * h; size *= 2)
> > >+		;
> > >+
> > >+	gem_handle = gem_create(fd, size);
> > >+	ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y,
> > >stride);
> > >+	igt_assert(ret == 0);
> > >+
> > >+	do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
> > >+		  format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
> > >+		  &data->fb.fb_id));
> > >+	data->fb.width = w;
> > >+	data->fb.height = h;
> > >+	data->fb.gem_handle = gem_handle;
> > >+
> > >+	igt_plane_set_fb(plane, NULL);
> > >+	igt_display_commit(display);
> > >+
> > >+	igt_plane_set_rotation(plane, data->rotation);
> > >+	igt_plane_set_fb(plane, &data->fb);
> > >+
> > >+	drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
> > >+				 DRM_MODE_OBJECT_PLANE,
> > >+				 plane->rotation_property,
> > >+				 plane->rotation);
> > >+	ret = igt_display_try_commit2(display, commit);
> > >+
> > >+	kmstest_restore_vt_mode();
> > >+	igt_remove_fb(fd, &data->fb);
> > >+	igt_assert(ret == 0);
> > >+}
> > >+
> > >  igt_main
> > >  {
> > >  	data_t data = {};
> > >@@ -345,6 +407,12 @@ igt_main
> > >  		test_plane_rotation(&data, IGT_PLANE_PRIMARY);
> > >  	}
> > >
> > >+	igt_subtest_f("primary-rotation-90-Y-tiled") {
> > >+		igt_require(gen >= 9);
> > >+		data.rotation = IGT_ROTATION_90;
> > >+		test_plane_rotation_ytiled_obj(&data,
> > >IGT_PLANE_PRIMARY);
> > >+	}
> > >+
> > >  	igt_fixture {
> > >  		igt_display_fini(&data.display);
> > >  	}
> > >
> > 
> > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Applied, thanks.
> -Daniel
Hi Daniel,
Not sure whether Tvrtko or Chris have any futher comments on the fix
for this bug but do you have any comments?

This was the fix:
http://lists.freedesktop.org/archives/intel-gfx/2015-September/076183.html

Thanks and Regards,
Vivek

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

Re: [PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier (v3)

[Tvrtko Ursulin]

On 23/10/15 12:35, Daniel Vetter wrote:
> On Fri, Oct 23, 2015 at 09:51:06AM +0100, Tvrtko Ursulin wrote:
>>
>> Hi,
>>
>> On 23/10/15 02:34, Vivek Kasireddy wrote:
>>> The main goal of this subtest is to trigger the following warning in
>>> the function i915_gem_object_get_fence():
>>> 	if (WARN_ON(!obj->map_and_fenceable))
>>>
>>> To trigger this warning, the subtest first creates a Y-tiled object and
>>> an associated framebuffer with the Y-fb modifier. Furthermore, to
>>> prevent the map_and_fenceable from being set, we make sure that
>>> the object does not have a normal VMA by refraining from rendering to the
>>> object and by setting the rotation property upfront before calling commit.
>>>
>>> v2: Do not call paint_squares and just use one output.
>>>
>>> v3: Convert an if condition to igt_require and move the plane rotation
>>> requirement further up before the fb allocation.
>>>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>>> ---
>>>   tests/kms_rotation_crc.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 68 insertions(+)
>>>
>>> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
>>> index cc9847e..b25a949 100644
>>> --- a/tests/kms_rotation_crc.c
>>> +++ b/tests/kms_rotation_crc.c
>>> @@ -264,6 +264,68 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
>>>   	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
>>>   }
>>>
>>> +static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_type)
>>> +{
>>> +	igt_display_t *display = &data->display;
>>> +	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
>>> +	uint32_t format = DRM_FORMAT_XRGB8888;
>>> +	int bpp = igt_drm_format_to_bpp(format);
>>> +	enum igt_commit_style commit = COMMIT_LEGACY;
>>> +	int fd = data->gfx_fd;
>>> +	igt_output_t *output = &display->outputs[0];
>>> +	igt_plane_t *plane;
>>> +	drmModeModeInfo *mode;
>>> +	unsigned int stride, size, w, h;
>>> +	uint32_t gem_handle;
>>> +	int ret;
>>> +
>>> +	igt_require(output != NULL && output->valid == true);
>>> +
>>> +	plane = igt_output_get_plane(output, plane_type);
>>> +	igt_require(igt_plane_supports_rotation(plane));
>>> +
>>> +	if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
>>> +		igt_require(data->display.has_universal_planes);
>>> +		commit = COMMIT_UNIVERSAL;
>>> +	}
>>> +
>>> +	mode = igt_output_get_mode(output);
>>> +	w = mode->hdisplay;
>>> +	h = mode->vdisplay;
>>> +
>>> +	for (stride = 512; stride < (w * bpp / 8); stride *= 2)
>>> +		;
>>> +	for (size = 1024*1024; size < stride * h; size *= 2)
>>> +		;
>>> +
>>> +	gem_handle = gem_create(fd, size);
>>> +	ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride);
>>> +	igt_assert(ret == 0);
>>> +
>>> +	do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
>>> +		  format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
>>> +		  &data->fb.fb_id));
>>> +	data->fb.width = w;
>>> +	data->fb.height = h;
>>> +	data->fb.gem_handle = gem_handle;
>>> +
>>> +	igt_plane_set_fb(plane, NULL);
>>> +	igt_display_commit(display);
>>> +
>>> +	igt_plane_set_rotation(plane, data->rotation);
>>> +	igt_plane_set_fb(plane, &data->fb);
>>> +
>>> +	drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
>>> +				 DRM_MODE_OBJECT_PLANE,
>>> +				 plane->rotation_property,
>>> +				 plane->rotation);
>>> +	ret = igt_display_try_commit2(display, commit);
>>> +
>>> +	kmstest_restore_vt_mode();
>>> +	igt_remove_fb(fd, &data->fb);
>>> +	igt_assert(ret == 0);
>>> +}
>>> +
>>>   igt_main
>>>   {
>>>   	data_t data = {};
>>> @@ -345,6 +407,12 @@ igt_main
>>>   		test_plane_rotation(&data, IGT_PLANE_PRIMARY);
>>>   	}
>>>
>>> +	igt_subtest_f("primary-rotation-90-Y-tiled") {
>>> +		igt_require(gen >= 9);
>>> +		data.rotation = IGT_ROTATION_90;
>>> +		test_plane_rotation_ytiled_obj(&data, IGT_PLANE_PRIMARY);
>>> +	}
>>> +
>>>   	igt_fixture {
>>>   		igt_display_fini(&data.display);
>>>   	}
>>>
>>
>> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Applied, thanks.

Hasn't hit the repo yet.

Regards,

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

[PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier (v4)

[Vivek Kasireddy]

The main goal of this subtest is to trigger the following warning in
the function i915_gem_object_get_fence():
	if (WARN_ON(!obj->map_and_fenceable))

To trigger this warning, the subtest first creates a Y-tiled object and
an associated framebuffer with the Y-fb modifier. Furthermore, to
prevent the map_and_fenceable from being set, we make sure that
the object does not have a normal VMA by refraining from rendering to the
object and by setting the rotation property upfront before calling commit.

v2: Do not call paint_squares and just use one output.

v3: Convert an if condition to igt_require and move the plane rotation
requirement further up before the fb allocation.

v4: After setting rotation to 90 and committing, change the rotation to
0 and commit once more. This is to test if the i915 driver hits any
warnings while pinning and unpinning an object that has both normal
and rotated views.

Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 tests/kms_rotation_crc.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index cc9847e..31cece2 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -264,6 +264,84 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
 	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
 }
 
+static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_type)
+{
+	igt_display_t *display = &data->display;
+	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
+	uint32_t format = DRM_FORMAT_XRGB8888;
+	int bpp = igt_drm_format_to_bpp(format);
+	enum igt_commit_style commit = COMMIT_LEGACY;
+	int fd = data->gfx_fd;
+	igt_output_t *output = &display->outputs[0];
+	igt_plane_t *plane;
+	drmModeModeInfo *mode;
+	unsigned int stride, size, w, h;
+	uint32_t gem_handle;
+	int ret;
+
+	igt_require(output != NULL && output->valid == true);
+
+	plane = igt_output_get_plane(output, plane_type);
+	igt_require(igt_plane_supports_rotation(plane));
+
+	if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
+		igt_require(data->display.has_universal_planes);
+		commit = COMMIT_UNIVERSAL;
+	}
+
+	mode = igt_output_get_mode(output);
+	w = mode->hdisplay;
+	h = mode->vdisplay;
+
+	for (stride = 512; stride < (w * bpp / 8); stride *= 2)
+		;
+	for (size = 1024*1024; size < stride * h; size *= 2)
+		;
+
+	gem_handle = gem_create(fd, size);
+	ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride);
+	igt_assert(ret == 0);
+
+	do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
+		  format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
+		  &data->fb.fb_id));
+	data->fb.width = w;
+	data->fb.height = h;
+	data->fb.gem_handle = gem_handle;
+
+	igt_plane_set_fb(plane, NULL);
+	igt_display_commit(display);
+
+	igt_plane_set_rotation(plane, data->rotation);
+	igt_plane_set_fb(plane, &data->fb);
+
+	/*
+	 * Set the rotation property before commit to make sure the object
+	 * only has a rotated view.
+	 */
+	drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
+				 DRM_MODE_OBJECT_PLANE,
+				 plane->rotation_property,
+				 plane->rotation);
+	ret = igt_display_try_commit2(display, commit);
+
+
+	/*
+	 * We set the rotation to 0 so that the underlying object will now
+	 * have a normal view.
+	 */
+	igt_plane_set_rotation(plane, IGT_ROTATION_0);
+	drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
+				 DRM_MODE_OBJECT_PLANE,
+				 plane->rotation_property,
+				 plane->rotation);
+	ret = igt_display_try_commit2(display, commit);
+
+	kmstest_restore_vt_mode();
+	igt_remove_fb(fd, &data->fb);
+	igt_assert(ret == 0);
+}
+
 igt_main
 {
 	data_t data = {};
@@ -345,6 +423,12 @@ igt_main
 		test_plane_rotation(&data, IGT_PLANE_PRIMARY);
 	}
 
+	igt_subtest_f("primary-rotation-90-Y-tiled") {
+		igt_require(gen >= 9);
+		data.rotation = IGT_ROTATION_90;
+		test_plane_rotation_ytiled_obj(&data, IGT_PLANE_PRIMARY);
+	}
+
 	igt_fixture {
 		igt_display_fini(&data.display);
 	}
-- 
2.4.3

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

Re: [PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier (v4)

[Tvrtko Ursulin]

On 29/10/15 01:48, Vivek Kasireddy wrote:
> The main goal of this subtest is to trigger the following warning in
> the function i915_gem_object_get_fence():
> 	if (WARN_ON(!obj->map_and_fenceable))
>
> To trigger this warning, the subtest first creates a Y-tiled object and
> an associated framebuffer with the Y-fb modifier. Furthermore, to
> prevent the map_and_fenceable from being set, we make sure that
> the object does not have a normal VMA by refraining from rendering to the
> object and by setting the rotation property upfront before calling commit.
>
> v2: Do not call paint_squares and just use one output.
>
> v3: Convert an if condition to igt_require and move the plane rotation
> requirement further up before the fb allocation.
>
> v4: After setting rotation to 90 and committing, change the rotation to
> 0 and commit once more. This is to test if the i915 driver hits any
> warnings while pinning and unpinning an object that has both normal
> and rotated views.
>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   tests/kms_rotation_crc.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 84 insertions(+)
>
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index cc9847e..31cece2 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -264,6 +264,84 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
>   	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
>   }
>
> +static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_type)
> +{
> +	igt_display_t *display = &data->display;
> +	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
> +	uint32_t format = DRM_FORMAT_XRGB8888;
> +	int bpp = igt_drm_format_to_bpp(format);
> +	enum igt_commit_style commit = COMMIT_LEGACY;
> +	int fd = data->gfx_fd;
> +	igt_output_t *output = &display->outputs[0];
> +	igt_plane_t *plane;
> +	drmModeModeInfo *mode;
> +	unsigned int stride, size, w, h;
> +	uint32_t gem_handle;
> +	int ret;
> +
> +	igt_require(output != NULL && output->valid == true);
> +
> +	plane = igt_output_get_plane(output, plane_type);
> +	igt_require(igt_plane_supports_rotation(plane));
> +
> +	if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
> +		igt_require(data->display.has_universal_planes);
> +		commit = COMMIT_UNIVERSAL;
> +	}
> +
> +	mode = igt_output_get_mode(output);
> +	w = mode->hdisplay;
> +	h = mode->vdisplay;
> +
> +	for (stride = 512; stride < (w * bpp / 8); stride *= 2)
> +		;
> +	for (size = 1024*1024; size < stride * h; size *= 2)
> +		;
> +
> +	gem_handle = gem_create(fd, size);
> +	ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride);
> +	igt_assert(ret == 0);
> +
> +	do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
> +		  format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
> +		  &data->fb.fb_id));
> +	data->fb.width = w;
> +	data->fb.height = h;
> +	data->fb.gem_handle = gem_handle;
> +
> +	igt_plane_set_fb(plane, NULL);
> +	igt_display_commit(display);
> +
> +	igt_plane_set_rotation(plane, data->rotation);
> +	igt_plane_set_fb(plane, &data->fb);
> +
> +	/*
> +	 * Set the rotation property before commit to make sure the object
> +	 * only has a rotated view.
> +	 */
> +	drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
> +				 DRM_MODE_OBJECT_PLANE,
> +				 plane->rotation_property,
> +				 plane->rotation);
> +	ret = igt_display_try_commit2(display, commit);
> +
> +
> +	/*
> +	 * We set the rotation to 0 so that the underlying object will now
> +	 * have a normal view.
> +	 */
> +	igt_plane_set_rotation(plane, IGT_ROTATION_0);
> +	drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
> +				 DRM_MODE_OBJECT_PLANE,
> +				 plane->rotation_property,
> +				 plane->rotation);
> +	ret = igt_display_try_commit2(display, commit);
> +
> +	kmstest_restore_vt_mode();
> +	igt_remove_fb(fd, &data->fb);
> +	igt_assert(ret == 0);
> +}
> +
>   igt_main
>   {
>   	data_t data = {};
> @@ -345,6 +423,12 @@ igt_main
>   		test_plane_rotation(&data, IGT_PLANE_PRIMARY);
>   	}
>
> +	igt_subtest_f("primary-rotation-90-Y-tiled") {
> +		igt_require(gen >= 9);
> +		data.rotation = IGT_ROTATION_90;
> +		test_plane_rotation_ytiled_obj(&data, IGT_PLANE_PRIMARY);
> +	}
> +
>   	igt_fixture {
>   		igt_display_fini(&data.display);
>   	}
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

[PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier (v5)

[Vivek Kasireddy]

The main goal of this subtest is to trigger the following warning in
the function i915_gem_object_get_fence():
	if (WARN_ON(!obj->map_and_fenceable))

To trigger this warning, the subtest first creates a Y-tiled object and
an associated framebuffer with the Y-fb modifier. Furthermore, to
prevent the map_and_fenceable from being set, we make sure that
the object does not have a normal VMA by refraining from rendering to the
object and by setting the rotation property upfront before calling commit.

v2: Do not call paint_squares and just use one output.

v3: Convert an if condition to igt_require and move the plane rotation
requirement further up before the fb allocation.

v4: After setting rotation to 90 and committing, change the rotation to
0 and commit once more. This is to test if the i915 driver hits any
warnings while pinning and unpinning an object that has both normal
and rotated views.

v5:
- Add another subtest to toggle the order of rotation
- Exhaustively test the i915 driver's pinning and unpinning code paths
  for any fence leaks by iterating until MAX available fences.

Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 tests/kms_rotation_crc.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index cc9847e..34f8150 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -264,6 +264,80 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
 	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
 }
 
+static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_type,
+					   int toggle)
+{
+	igt_display_t *display = &data->display;
+	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
+	uint32_t format = DRM_FORMAT_XRGB8888;
+	int bpp = igt_drm_format_to_bpp(format);
+	enum igt_commit_style commit = COMMIT_LEGACY;
+	int fd = data->gfx_fd;
+	igt_output_t *output = &display->outputs[0];
+	igt_plane_t *plane;
+	drmModeModeInfo *mode;
+	unsigned int stride, size, w, h;
+	uint32_t gem_handle;
+	int num_fences = gem_available_fences(fd);
+	int i, ret;
+
+	igt_require(output != NULL && output->valid == true);
+
+	plane = igt_output_get_plane(output, plane_type);
+	igt_require(igt_plane_supports_rotation(plane));
+
+	if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
+		igt_require(data->display.has_universal_planes);
+		commit = COMMIT_UNIVERSAL;
+	}
+
+	mode = igt_output_get_mode(output);
+	w = mode->hdisplay;
+	h = mode->vdisplay;
+
+	for (stride = 512; stride < (w * bpp / 8); stride *= 2)
+		;
+	for (size = 1024*1024; size < stride * h; size *= 2)
+		;
+
+	gem_handle = gem_create(fd, size);
+	ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride);
+	igt_assert(ret == 0);
+
+	do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
+		  format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
+		  &data->fb.fb_id));
+	data->fb.width = w;
+	data->fb.height = h;
+	data->fb.gem_handle = gem_handle;
+
+	igt_plane_set_fb(plane, NULL);
+	igt_display_commit(display);
+
+	igt_plane_set_fb(plane, &data->fb);
+
+	for (i = 0; i < num_fences + 1; i++) {
+		igt_plane_set_rotation(plane, toggle ? IGT_ROTATION_0 : IGT_ROTATION_90);
+		drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
+					 DRM_MODE_OBJECT_PLANE,
+					 plane->rotation_property,
+					 plane->rotation);
+		ret = igt_display_try_commit2(display, commit);
+		igt_assert(ret == 0);
+
+		igt_plane_set_rotation(plane, toggle ? IGT_ROTATION_90 : IGT_ROTATION_0);
+		drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
+					 DRM_MODE_OBJECT_PLANE,
+					 plane->rotation_property,
+					 plane->rotation);
+		ret = igt_display_try_commit2(display, commit);
+		igt_assert(ret == 0);
+	}
+
+	kmstest_restore_vt_mode();
+	igt_remove_fb(fd, &data->fb);
+}
+
 igt_main
 {
 	data_t data = {};
@@ -345,6 +419,16 @@ igt_main
 		test_plane_rotation(&data, IGT_PLANE_PRIMARY);
 	}
 
+	igt_subtest_f("primary-rotation-90-to-0-Y-tiled") {
+		igt_require(gen >= 9);
+		test_plane_rotation_ytiled_obj(&data, IGT_PLANE_PRIMARY, 0);
+	}
+
+	igt_subtest_f("primary-rotation-0-to-90-Y-tiled") {
+		igt_require(gen >= 9);
+		test_plane_rotation_ytiled_obj(&data, IGT_PLANE_PRIMARY, 1);
+	}
+
 	igt_fixture {
 		igt_display_fini(&data.display);
 	}
-- 
2.4.3

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

Re: [PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier (v5)

[Tvrtko Ursulin]

On 30/10/15 01:44, Vivek Kasireddy wrote:
> The main goal of this subtest is to trigger the following warning in
> the function i915_gem_object_get_fence():
> 	if (WARN_ON(!obj->map_and_fenceable))
>
> To trigger this warning, the subtest first creates a Y-tiled object and
> an associated framebuffer with the Y-fb modifier. Furthermore, to
> prevent the map_and_fenceable from being set, we make sure that
> the object does not have a normal VMA by refraining from rendering to the
> object and by setting the rotation property upfront before calling commit.
>
> v2: Do not call paint_squares and just use one output.
>
> v3: Convert an if condition to igt_require and move the plane rotation
> requirement further up before the fb allocation.
>
> v4: After setting rotation to 90 and committing, change the rotation to
> 0 and commit once more. This is to test if the i915 driver hits any
> warnings while pinning and unpinning an object that has both normal
> and rotated views.
>
> v5:
> - Add another subtest to toggle the order of rotation
> - Exhaustively test the i915 driver's pinning and unpinning code paths
>    for any fence leaks by iterating until MAX available fences.
>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   tests/kms_rotation_crc.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 84 insertions(+)
>
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index cc9847e..34f8150 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -264,6 +264,80 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
>   	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
>   }
>
> +static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_type,
> +					   int toggle)
> +{
> +	igt_display_t *display = &data->display;
> +	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
> +	uint32_t format = DRM_FORMAT_XRGB8888;
> +	int bpp = igt_drm_format_to_bpp(format);
> +	enum igt_commit_style commit = COMMIT_LEGACY;
> +	int fd = data->gfx_fd;
> +	igt_output_t *output = &display->outputs[0];
> +	igt_plane_t *plane;
> +	drmModeModeInfo *mode;
> +	unsigned int stride, size, w, h;
> +	uint32_t gem_handle;
> +	int num_fences = gem_available_fences(fd);
> +	int i, ret;
> +
> +	igt_require(output != NULL && output->valid == true);
> +
> +	plane = igt_output_get_plane(output, plane_type);
> +	igt_require(igt_plane_supports_rotation(plane));
> +
> +	if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
> +		igt_require(data->display.has_universal_planes);
> +		commit = COMMIT_UNIVERSAL;
> +	}
> +
> +	mode = igt_output_get_mode(output);
> +	w = mode->hdisplay;
> +	h = mode->vdisplay;
> +
> +	for (stride = 512; stride < (w * bpp / 8); stride *= 2)
> +		;
> +	for (size = 1024*1024; size < stride * h; size *= 2)
> +		;
> +
> +	gem_handle = gem_create(fd, size);
> +	ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride);
> +	igt_assert(ret == 0);
> +
> +	do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
> +		  format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
> +		  &data->fb.fb_id));
> +	data->fb.width = w;
> +	data->fb.height = h;
> +	data->fb.gem_handle = gem_handle;
> +
> +	igt_plane_set_fb(plane, NULL);
> +	igt_display_commit(display);
> +
> +	igt_plane_set_fb(plane, &data->fb);
> +
> +	for (i = 0; i < num_fences + 1; i++) {
> +		igt_plane_set_rotation(plane, toggle ? IGT_ROTATION_0 : IGT_ROTATION_90);
> +		drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
> +					 DRM_MODE_OBJECT_PLANE,
> +					 plane->rotation_property,
> +					 plane->rotation);
> +		ret = igt_display_try_commit2(display, commit);
> +		igt_assert(ret == 0);
> +
> +		igt_plane_set_rotation(plane, toggle ? IGT_ROTATION_90 : IGT_ROTATION_0);
> +		drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
> +					 DRM_MODE_OBJECT_PLANE,
> +					 plane->rotation_property,
> +					 plane->rotation);
> +		ret = igt_display_try_commit2(display, commit);
> +		igt_assert(ret == 0);
> +	}

It manages to exhaust the fences with only one object? I was expecting 
that multiple objects will be required since if it is only one how come 
it allocates more than one fence register?

Regards,

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

Re: [PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier (v5)

[Vivek Kasireddy]

Hi Tvrtko,

On Fri, 30 Oct 2015 10:22:08 +0000
Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:

> 
> On 30/10/15 01:44, Vivek Kasireddy wrote:
> > The main goal of this subtest is to trigger the following warning in
> > the function i915_gem_object_get_fence():
> > 	if (WARN_ON(!obj->map_and_fenceable))
> >
> > To trigger this warning, the subtest first creates a Y-tiled object
> > and an associated framebuffer with the Y-fb modifier. Furthermore,
> > to prevent the map_and_fenceable from being set, we make sure that
> > the object does not have a normal VMA by refraining from rendering
> > to the object and by setting the rotation property upfront before
> > calling commit.
> >
> > v2: Do not call paint_squares and just use one output.
> >
> > v3: Convert an if condition to igt_require and move the plane
> > rotation requirement further up before the fb allocation.
> >
> > v4: After setting rotation to 90 and committing, change the
> > rotation to 0 and commit once more. This is to test if the i915
> > driver hits any warnings while pinning and unpinning an object that
> > has both normal and rotated views.
> >
> > v5:
> > - Add another subtest to toggle the order of rotation
> > - Exhaustively test the i915 driver's pinning and unpinning code
> > paths for any fence leaks by iterating until MAX available fences.
> >
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > ---
> >   tests/kms_rotation_crc.c | 84
> > ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84
> > insertions(+)
> >
> > diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> > index cc9847e..34f8150 100644
> > --- a/tests/kms_rotation_crc.c
> > +++ b/tests/kms_rotation_crc.c
> > @@ -264,6 +264,80 @@ static void test_plane_rotation(data_t *data,
> > enum igt_plane plane_type) igt_require_f(valid_tests, "no valid
> > crtc/connector combinations found\n"); }
> >
> > +static void test_plane_rotation_ytiled_obj(data_t *data, enum
> > igt_plane plane_type,
> > +					   int toggle)
> > +{
> > +	igt_display_t *display = &data->display;
> > +	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
> > +	uint32_t format = DRM_FORMAT_XRGB8888;
> > +	int bpp = igt_drm_format_to_bpp(format);
> > +	enum igt_commit_style commit = COMMIT_LEGACY;
> > +	int fd = data->gfx_fd;
> > +	igt_output_t *output = &display->outputs[0];
> > +	igt_plane_t *plane;
> > +	drmModeModeInfo *mode;
> > +	unsigned int stride, size, w, h;
> > +	uint32_t gem_handle;
> > +	int num_fences = gem_available_fences(fd);
> > +	int i, ret;
> > +
> > +	igt_require(output != NULL && output->valid == true);
> > +
> > +	plane = igt_output_get_plane(output, plane_type);
> > +	igt_require(igt_plane_supports_rotation(plane));
> > +
> > +	if (plane_type == IGT_PLANE_PRIMARY || plane_type ==
> > IGT_PLANE_CURSOR) {
> > +		igt_require(data->display.has_universal_planes);
> > +		commit = COMMIT_UNIVERSAL;
> > +	}
> > +
> > +	mode = igt_output_get_mode(output);
> > +	w = mode->hdisplay;
> > +	h = mode->vdisplay;
> > +
> > +	for (stride = 512; stride < (w * bpp / 8); stride *= 2)
> > +		;
> > +	for (size = 1024*1024; size < stride * h; size *= 2)
> > +		;
> > +
> > +	gem_handle = gem_create(fd, size);
> > +	ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y,
> > stride);
> > +	igt_assert(ret == 0);
> > +
> > +	do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
> > +		  format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
> > +		  &data->fb.fb_id));
> > +	data->fb.width = w;
> > +	data->fb.height = h;
> > +	data->fb.gem_handle = gem_handle;
> > +
> > +	igt_plane_set_fb(plane, NULL);
> > +	igt_display_commit(display);
> > +
> > +	igt_plane_set_fb(plane, &data->fb);
> > +
> > +	for (i = 0; i < num_fences + 1; i++) {
> > +		igt_plane_set_rotation(plane, toggle ?
> > IGT_ROTATION_0 : IGT_ROTATION_90);
> > +		drmModeObjectSetProperty(fd,
> > plane->drm_plane->plane_id,
> > +					 DRM_MODE_OBJECT_PLANE,
> > +					 plane->rotation_property,
> > +					 plane->rotation);
> > +		ret = igt_display_try_commit2(display, commit);
> > +		igt_assert(ret == 0);
> > +
> > +		igt_plane_set_rotation(plane, toggle ?
> > IGT_ROTATION_90 : IGT_ROTATION_0);
> > +		drmModeObjectSetProperty(fd,
> > plane->drm_plane->plane_id,
> > +					 DRM_MODE_OBJECT_PLANE,
> > +					 plane->rotation_property,
> > +					 plane->rotation);
> > +		ret = igt_display_try_commit2(display, commit);
> > +		igt_assert(ret == 0);
> > +	}
> 
> It manages to exhaust the fences with only one object? I was
> expecting that multiple objects will be required since if it is only
> one how come it allocates more than one fence register?

Before I sent out this version, I did try with two objects to see if it
triggers any WARNs but it didn't. I am not sure if I did it right
though, I'll take a look at it again. However, when I tried this
version, that is, single-object-multiple-views, it did trigger the
following WARNS:

WARN_ON(!ggtt_vma || dev_priv->fence_regs[obj->fence_reg].pin_count >
ggtt_vma->pin_count)

if (WARN_ON(fence->pin_count)) 

I tested these with v3 of my i915 driver patch. Of-course, with v4,
I don't seen any WARNs. I'll take a closer look at triggering
any WARNs with 2 or more objects. However, one thing that was not clear
to me was when you said unpin both (in your previous comment), I was
wondering how I can do that from the test. Did you mean something like
this:

igt_plane_set_fb(plane, NULL);
igt_display_commit(display);

Thanks and Regards,
Vivek


> 
> Regards,
> 
> Tvrtko

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

Re: [PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier (v5)

[Tvrtko Ursulin]

On 31/10/15 01:45, Vivek Kasireddy wrote:
> Hi Tvrtko,
>
> On Fri, 30 Oct 2015 10:22:08 +0000
> Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>
>>
>> On 30/10/15 01:44, Vivek Kasireddy wrote:
>>> The main goal of this subtest is to trigger the following warning in
>>> the function i915_gem_object_get_fence():
>>> 	if (WARN_ON(!obj->map_and_fenceable))
>>>
>>> To trigger this warning, the subtest first creates a Y-tiled object
>>> and an associated framebuffer with the Y-fb modifier. Furthermore,
>>> to prevent the map_and_fenceable from being set, we make sure that
>>> the object does not have a normal VMA by refraining from rendering
>>> to the object and by setting the rotation property upfront before
>>> calling commit.
>>>
>>> v2: Do not call paint_squares and just use one output.
>>>
>>> v3: Convert an if condition to igt_require and move the plane
>>> rotation requirement further up before the fb allocation.
>>>
>>> v4: After setting rotation to 90 and committing, change the
>>> rotation to 0 and commit once more. This is to test if the i915
>>> driver hits any warnings while pinning and unpinning an object that
>>> has both normal and rotated views.
>>>
>>> v5:
>>> - Add another subtest to toggle the order of rotation
>>> - Exhaustively test the i915 driver's pinning and unpinning code
>>> paths for any fence leaks by iterating until MAX available fences.
>>>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>>> ---
>>>    tests/kms_rotation_crc.c | 84
>>> ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84
>>> insertions(+)
>>>
>>> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
>>> index cc9847e..34f8150 100644
>>> --- a/tests/kms_rotation_crc.c
>>> +++ b/tests/kms_rotation_crc.c
>>> @@ -264,6 +264,80 @@ static void test_plane_rotation(data_t *data,
>>> enum igt_plane plane_type) igt_require_f(valid_tests, "no valid
>>> crtc/connector combinations found\n"); }
>>>
>>> +static void test_plane_rotation_ytiled_obj(data_t *data, enum
>>> igt_plane plane_type,
>>> +					   int toggle)
>>> +{
>>> +	igt_display_t *display = &data->display;
>>> +	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
>>> +	uint32_t format = DRM_FORMAT_XRGB8888;
>>> +	int bpp = igt_drm_format_to_bpp(format);
>>> +	enum igt_commit_style commit = COMMIT_LEGACY;
>>> +	int fd = data->gfx_fd;
>>> +	igt_output_t *output = &display->outputs[0];
>>> +	igt_plane_t *plane;
>>> +	drmModeModeInfo *mode;
>>> +	unsigned int stride, size, w, h;
>>> +	uint32_t gem_handle;
>>> +	int num_fences = gem_available_fences(fd);
>>> +	int i, ret;
>>> +
>>> +	igt_require(output != NULL && output->valid == true);
>>> +
>>> +	plane = igt_output_get_plane(output, plane_type);
>>> +	igt_require(igt_plane_supports_rotation(plane));
>>> +
>>> +	if (plane_type == IGT_PLANE_PRIMARY || plane_type ==
>>> IGT_PLANE_CURSOR) {
>>> +		igt_require(data->display.has_universal_planes);
>>> +		commit = COMMIT_UNIVERSAL;
>>> +	}
>>> +
>>> +	mode = igt_output_get_mode(output);
>>> +	w = mode->hdisplay;
>>> +	h = mode->vdisplay;
>>> +
>>> +	for (stride = 512; stride < (w * bpp / 8); stride *= 2)
>>> +		;
>>> +	for (size = 1024*1024; size < stride * h; size *= 2)
>>> +		;
>>> +
>>> +	gem_handle = gem_create(fd, size);
>>> +	ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y,
>>> stride);
>>> +	igt_assert(ret == 0);
>>> +
>>> +	do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
>>> +		  format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
>>> +		  &data->fb.fb_id));
>>> +	data->fb.width = w;
>>> +	data->fb.height = h;
>>> +	data->fb.gem_handle = gem_handle;
>>> +
>>> +	igt_plane_set_fb(plane, NULL);
>>> +	igt_display_commit(display);
>>> +
>>> +	igt_plane_set_fb(plane, &data->fb);
>>> +
>>> +	for (i = 0; i < num_fences + 1; i++) {
>>> +		igt_plane_set_rotation(plane, toggle ?
>>> IGT_ROTATION_0 : IGT_ROTATION_90);
>>> +		drmModeObjectSetProperty(fd,
>>> plane->drm_plane->plane_id,
>>> +					 DRM_MODE_OBJECT_PLANE,
>>> +					 plane->rotation_property,
>>> +					 plane->rotation);
>>> +		ret = igt_display_try_commit2(display, commit);
>>> +		igt_assert(ret == 0);
>>> +
>>> +		igt_plane_set_rotation(plane, toggle ?
>>> IGT_ROTATION_90 : IGT_ROTATION_0);
>>> +		drmModeObjectSetProperty(fd,
>>> plane->drm_plane->plane_id,
>>> +					 DRM_MODE_OBJECT_PLANE,
>>> +					 plane->rotation_property,
>>> +					 plane->rotation);
>>> +		ret = igt_display_try_commit2(display, commit);
>>> +		igt_assert(ret == 0);
>>> +	}
>>
>> It manages to exhaust the fences with only one object? I was
>> expecting that multiple objects will be required since if it is only
>> one how come it allocates more than one fence register?
>
> Before I sent out this version, I did try with two objects to see if it
> triggers any WARNs but it didn't. I am not sure if I did it right
> though, I'll take a look at it again. However, when I tried this
> version, that is, single-object-multiple-views, it did trigger the
> following WARNS:
>
> WARN_ON(!ggtt_vma || dev_priv->fence_regs[obj->fence_reg].pin_count >
> ggtt_vma->pin_count)
>
> if (WARN_ON(fence->pin_count))
>
> I tested these with v3 of my i915 driver patch. Of-course, with v4,
> I don't seen any WARNs. I'll take a closer look at triggering
> any WARNs with 2 or more objects. However, one thing that was not clear

I thought you would need number of framebuffers / objects > num_fences 
and display all of them to trigger the running out of fences ref leak.

> to me was when you said unpin both (in your previous comment), I was
> wondering how I can do that from the test. Did you mean something like
> this:
>
> igt_plane_set_fb(plane, NULL);
> igt_display_commit(display);

Couldn't find in this thread where I said "unpin both" so don't know.

Regards,

Tvrtko

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

Re: [PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier (v3)

[Thomas Wood]

On 27 October 2015 at 10:37, Tvrtko Ursulin
<tvrtko.ursulin@linux.intel.com> wrote:
>
> On 23/10/15 12:35, Daniel Vetter wrote:
>>
>> On Fri, Oct 23, 2015 at 09:51:06AM +0100, Tvrtko Ursulin wrote:
>>>
>>>
>>> Hi,
>>>
>>> On 23/10/15 02:34, Vivek Kasireddy wrote:
>>>>
>>>> The main goal of this subtest is to trigger the following warning in
>>>> the function i915_gem_object_get_fence():
>>>>         if (WARN_ON(!obj->map_and_fenceable))
>>>>
>>>> To trigger this warning, the subtest first creates a Y-tiled object and
>>>> an associated framebuffer with the Y-fb modifier. Furthermore, to
>>>> prevent the map_and_fenceable from being set, we make sure that
>>>> the object does not have a normal VMA by refraining from rendering to
>>>> the
>>>> object and by setting the rotation property upfront before calling
>>>> commit.
>>>>
>>>> v2: Do not call paint_squares and just use one output.
>>>>
>>>> v3: Convert an if condition to igt_require and move the plane rotation
>>>> requirement further up before the fb allocation.
>>>>
>>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>>>> ---
>>>>   tests/kms_rotation_crc.c | 68
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>   1 file changed, 68 insertions(+)
>>>>
>>>> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
>>>> index cc9847e..b25a949 100644
>>>> --- a/tests/kms_rotation_crc.c
>>>> +++ b/tests/kms_rotation_crc.c
>>>> @@ -264,6 +264,68 @@ static void test_plane_rotation(data_t *data, enum
>>>> igt_plane plane_type)
>>>>         igt_require_f(valid_tests, "no valid crtc/connector combinations
>>>> found\n");
>>>>   }
>>>>
>>>> +static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane
>>>> plane_type)
>>>> +{
>>>> +       igt_display_t *display = &data->display;
>>>> +       uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
>>>> +       uint32_t format = DRM_FORMAT_XRGB8888;
>>>> +       int bpp = igt_drm_format_to_bpp(format);
>>>> +       enum igt_commit_style commit = COMMIT_LEGACY;
>>>> +       int fd = data->gfx_fd;
>>>> +       igt_output_t *output = &display->outputs[0];
>>>> +       igt_plane_t *plane;
>>>> +       drmModeModeInfo *mode;
>>>> +       unsigned int stride, size, w, h;
>>>> +       uint32_t gem_handle;
>>>> +       int ret;
>>>> +
>>>> +       igt_require(output != NULL && output->valid == true);
>>>> +
>>>> +       plane = igt_output_get_plane(output, plane_type);
>>>> +       igt_require(igt_plane_supports_rotation(plane));
>>>> +
>>>> +       if (plane_type == IGT_PLANE_PRIMARY || plane_type ==
>>>> IGT_PLANE_CURSOR) {
>>>> +               igt_require(data->display.has_universal_planes);
>>>> +               commit = COMMIT_UNIVERSAL;
>>>> +       }
>>>> +
>>>> +       mode = igt_output_get_mode(output);
>>>> +       w = mode->hdisplay;
>>>> +       h = mode->vdisplay;
>>>> +
>>>> +       for (stride = 512; stride < (w * bpp / 8); stride *= 2)
>>>> +               ;
>>>> +       for (size = 1024*1024; size < stride * h; size *= 2)
>>>> +               ;
>>>> +
>>>> +       gem_handle = gem_create(fd, size);
>>>> +       ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride);
>>>> +       igt_assert(ret == 0);
>>>> +
>>>> +       do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
>>>> +                 format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
>>>> +                 &data->fb.fb_id));
>>>> +       data->fb.width = w;
>>>> +       data->fb.height = h;
>>>> +       data->fb.gem_handle = gem_handle;
>>>> +
>>>> +       igt_plane_set_fb(plane, NULL);
>>>> +       igt_display_commit(display);
>>>> +
>>>> +       igt_plane_set_rotation(plane, data->rotation);
>>>> +       igt_plane_set_fb(plane, &data->fb);
>>>> +
>>>> +       drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
>>>> +                                DRM_MODE_OBJECT_PLANE,
>>>> +                                plane->rotation_property,
>>>> +                                plane->rotation);
>>>> +       ret = igt_display_try_commit2(display, commit);
>>>> +
>>>> +       kmstest_restore_vt_mode();
>>>> +       igt_remove_fb(fd, &data->fb);
>>>> +       igt_assert(ret == 0);
>>>> +}
>>>> +
>>>>   igt_main
>>>>   {
>>>>         data_t data = {};
>>>> @@ -345,6 +407,12 @@ igt_main
>>>>                 test_plane_rotation(&data, IGT_PLANE_PRIMARY);
>>>>         }
>>>>
>>>> +       igt_subtest_f("primary-rotation-90-Y-tiled") {
>>>> +               igt_require(gen >= 9);
>>>> +               data.rotation = IGT_ROTATION_90;
>>>> +               test_plane_rotation_ytiled_obj(&data,
>>>> IGT_PLANE_PRIMARY);
>>>> +       }
>>>> +
>>>>         igt_fixture {
>>>>                 igt_display_fini(&data.display);
>>>>         }
>>>>
>>>
>>> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>>
>> Applied, thanks.
>
>
> Hasn't hit the repo yet.

Daniel has now pushed this patch (v3), so the further changes that
have been made will need to be split into new patches.


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

[PATCH] igt/kms_rotation_crc: Add a new subtest to exhaustively test for fence leaks

[Vivek Kasireddy]

In this subtest, as a first step, MAX_FENCES+1 number of framebuffers are
created backed up by objects that have multiple GGTT views (normal and
rotated). Next, we have the i915 driver instantiate a normal view followed
by a rotated view. We continue doing the above MAX_FENCES + 1 times.

Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 tests/kms_rotation_crc.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index ed6eeef..44691d1 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -25,6 +25,7 @@
 #include "igt.h"
 #include <math.h>
 
+#define MAX_FENCES 32
 
 typedef struct {
 	int gfx_fd;
@@ -376,6 +377,78 @@ static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_ty
 	igt_assert(ret == 0);
 }
 
+static void test_plane_rotation_exhaust_fences(data_t *data, data_t *data2,
+					       enum igt_plane plane_type)
+{
+	igt_display_t *display = &data->display;
+	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
+	uint32_t format = DRM_FORMAT_XRGB8888;
+	int bpp = igt_drm_format_to_bpp(format);
+	enum igt_commit_style commit = COMMIT_LEGACY;
+	int fd = data->gfx_fd;
+	igt_output_t *output = &display->outputs[0];
+	igt_plane_t *plane;
+	drmModeModeInfo *mode;
+	unsigned int stride, size, w, h;
+	uint32_t gem_handle;
+	int i, ret;
+
+	igt_require(output != NULL && output->valid == true);
+
+	plane = igt_output_get_plane(output, plane_type);
+	igt_require(igt_plane_supports_rotation(plane));
+
+	if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
+		igt_require(data->display.has_universal_planes);
+		commit = COMMIT_UNIVERSAL;
+	}
+
+	mode = igt_output_get_mode(output);
+	w = mode->hdisplay;
+	h = mode->vdisplay;
+
+	for (stride = 512; stride < (w * bpp / 8); stride *= 2)
+		;
+	for (size = 1024*1024; size < stride * h; size *= 2)
+		;
+
+	igt_plane_set_fb(plane, NULL);
+	igt_display_commit(display);
+
+	for (i = 0; i < MAX_FENCES + 1; i++) {
+		gem_handle = gem_create(fd, size);
+		ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride);
+		igt_assert(ret == 0);
+
+		do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
+			  format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
+			  &data2[i].fb.fb_id));
+		data2[i].fb.width = w;
+		data2[i].fb.height = h;
+		data2[i].fb.gem_handle = gem_handle;
+
+		igt_plane_set_fb(plane, &data2[i].fb);
+		igt_plane_set_rotation(plane, IGT_ROTATION_0);
+
+		ret = igt_display_try_commit2(display, commit);
+		igt_assert(ret == 0);
+
+		igt_plane_set_rotation(plane, IGT_ROTATION_90);
+
+		drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
+					 DRM_MODE_OBJECT_PLANE,
+					 plane->rotation_property,
+					 plane->rotation);
+		ret = igt_display_try_commit2(display, commit);
+		igt_assert(ret == 0);
+	}
+
+	kmstest_restore_vt_mode();
+
+	for (i = 0; i < MAX_FENCES + 1; i++)
+		igt_remove_fb(fd, &data2[i].fb);
+}
+
 igt_main
 {
 	data_t data = {};
@@ -471,6 +544,12 @@ igt_main
 		test_plane_rotation_ytiled_obj(&data, IGT_PLANE_PRIMARY);
 	}
 
+	igt_subtest_f("exhaust-fences") {
+		data_t data2[MAX_FENCES+1] = {};
+		igt_require(gen >= 9);
+		test_plane_rotation_exhaust_fences(&data, data2, IGT_PLANE_PRIMARY);
+	}
+
 	igt_fixture {
 		igt_display_fini(&data.display);
 	}
-- 
2.4.3

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

Re: [PATCH] igt/kms_rotation_crc: Add a new subtest to exhaustively test for fence leaks

[Tvrtko Ursulin]

Hi,

On 03/11/15 01:25, Vivek Kasireddy wrote:
> In this subtest, as a first step, MAX_FENCES+1 number of framebuffers are
> created backed up by objects that have multiple GGTT views (normal and
> rotated). Next, we have the i915 driver instantiate a normal view followed
> by a rotated view. We continue doing the above MAX_FENCES + 1 times.
>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   tests/kms_rotation_crc.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 79 insertions(+)
>
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index ed6eeef..44691d1 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -25,6 +25,7 @@
>   #include "igt.h"
>   #include <math.h>
>
> +#define MAX_FENCES 32
>
>   typedef struct {
>   	int gfx_fd;
> @@ -376,6 +377,78 @@ static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_ty
>   	igt_assert(ret == 0);
>   }
>
> +static void test_plane_rotation_exhaust_fences(data_t *data, data_t *data2,
> +					       enum igt_plane plane_type)
> +{
> +	igt_display_t *display = &data->display;
> +	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
> +	uint32_t format = DRM_FORMAT_XRGB8888;
> +	int bpp = igt_drm_format_to_bpp(format);
> +	enum igt_commit_style commit = COMMIT_LEGACY;
> +	int fd = data->gfx_fd;
> +	igt_output_t *output = &display->outputs[0];
> +	igt_plane_t *plane;
> +	drmModeModeInfo *mode;
> +	unsigned int stride, size, w, h;
> +	uint32_t gem_handle;
> +	int i, ret;
> +
> +	igt_require(output != NULL && output->valid == true);
> +
> +	plane = igt_output_get_plane(output, plane_type);
> +	igt_require(igt_plane_supports_rotation(plane));
> +
> +	if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
> +		igt_require(data->display.has_universal_planes);
> +		commit = COMMIT_UNIVERSAL;
> +	}
> +
> +	mode = igt_output_get_mode(output);
> +	w = mode->hdisplay;
> +	h = mode->vdisplay;
> +
> +	for (stride = 512; stride < (w * bpp / 8); stride *= 2)
> +		;
> +	for (size = 1024*1024; size < stride * h; size *= 2)
> +		;

Looking good, just two minor comments:

I think a good addition would be to verify that we have enough GTT space 
for size * MAX_FENCES objects. Because if we don't then the test can't 
test what it is trying to test I think. So something like:

igt_require(size * MAX_FENCES < gem_total_aperture()) or whatever it is 
called. Maybe fudge down by a percentage to allow for other stuff, 10% 
or so.

> +	igt_plane_set_fb(plane, NULL);
> +	igt_display_commit(display);
> +
> +	for (i = 0; i < MAX_FENCES + 1; i++) {
> +		gem_handle = gem_create(fd, size);
> +		ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride);
> +		igt_assert(ret == 0);
> +
> +		do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
> +			  format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
> +			  &data2[i].fb.fb_id));
> +		data2[i].fb.width = w;
> +		data2[i].fb.height = h;
> +		data2[i].fb.gem_handle = gem_handle;
> +
> +		igt_plane_set_fb(plane, &data2[i].fb);
> +		igt_plane_set_rotation(plane, IGT_ROTATION_0);
> +
> +		ret = igt_display_try_commit2(display, commit);
> +		igt_assert(ret == 0);
> +
> +		igt_plane_set_rotation(plane, IGT_ROTATION_90);
> +
> +		drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
> +					 DRM_MODE_OBJECT_PLANE,
> +					 plane->rotation_property,
> +					 plane->rotation);
> +		ret = igt_display_try_commit2(display, commit);
> +		igt_assert(ret == 0);
> +	}
> +
> +	kmstest_restore_vt_mode();
> +
> +	for (i = 0; i < MAX_FENCES + 1; i++)
> +		igt_remove_fb(fd, &data2[i].fb);
> +}
> +
>   igt_main
>   {
>   	data_t data = {};
> @@ -471,6 +544,12 @@ igt_main
>   		test_plane_rotation_ytiled_obj(&data, IGT_PLANE_PRIMARY);
>   	}
>
> +	igt_subtest_f("exhaust-fences") {
> +		data_t data2[MAX_FENCES+1] = {};
> +		igt_require(gen >= 9);
> +		test_plane_rotation_exhaust_fences(&data, data2, IGT_PLANE_PRIMARY);

Second minor comment is that data2 could be local to 
test_plane_rotation_exhaust_fences just as well and would probably be 
cleaner like that.

Regards,

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

[PATCH] igt/kms_rotation_crc: Add a new subtest to exhaustively test for fence leaks (v2)

[Vivek Kasireddy]

In this subtest, as a first step, MAX_FENCES+1 number of framebuffers are
created backed up by objects that have multiple GGTT views (normal and
rotated). Next, we have the i915 driver instantiate a normal view followed
by a rotated view. We continue doing the above MAX_FENCES + 1 times.

v2:
- Add a igt_require() to check if there is enough GTT space left for
  MAX_FENCES+1 framebuffers. (Tvrtko)
- Make data2 local to test_plane_rotation_exhaust_fences(). (Tvrtko)
- If there is a failure, deallocate all the previously allocated
  framebuffers before asserting.

Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 tests/kms_rotation_crc.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 106 insertions(+)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index ed6eeef..154c6a1 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -25,6 +25,7 @@
 #include "igt.h"
 #include <math.h>
 
+#define MAX_FENCES 32
 
 typedef struct {
 	int gfx_fd;
@@ -376,6 +377,106 @@ static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_ty
 	igt_assert(ret == 0);
 }
 
+static void test_plane_rotation_exhaust_fences(data_t *data, enum igt_plane plane_type)
+{
+	igt_display_t *display = &data->display;
+	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
+	uint32_t format = DRM_FORMAT_XRGB8888;
+	int bpp = igt_drm_format_to_bpp(format);
+	enum igt_commit_style commit = COMMIT_LEGACY;
+	int fd = data->gfx_fd;
+	igt_output_t *output = &display->outputs[0];
+	igt_plane_t *plane;
+	drmModeModeInfo *mode;
+	data_t data2[MAX_FENCES+1] = {};
+	unsigned int stride, size, w, h;
+	uint32_t gem_handle;
+	uint64_t total_aperture_size, total_fbs_size;
+	int i, ret;
+
+	igt_require(output != NULL && output->valid == true);
+
+	plane = igt_output_get_plane(output, plane_type);
+	igt_require(igt_plane_supports_rotation(plane));
+
+	if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
+		igt_require(data->display.has_universal_planes);
+		commit = COMMIT_UNIVERSAL;
+	}
+
+	mode = igt_output_get_mode(output);
+	w = mode->hdisplay;
+	h = mode->vdisplay;
+
+	for (stride = 512; stride < (w * bpp / 8); stride *= 2)
+		;
+	for (size = 1024*1024; size < stride * h; size *= 2)
+		;
+
+	/*
+	 * Make sure there is atleast 90% of the available GTT space left
+	 * for creating (MAX_FENCES+1) framebuffers.
+	 */
+	total_fbs_size = size * (MAX_FENCES + 1);
+	total_aperture_size = gem_available_aperture_size(fd);
+	igt_require(total_fbs_size < total_aperture_size * 0.9);
+
+	igt_plane_set_fb(plane, NULL);
+	igt_display_commit(display);
+
+	for (i = 0; i < MAX_FENCES + 1; i++) {
+		gem_handle = gem_create(fd, size);
+		ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride);
+		if (ret) {
+			igt_warn("failed to set tiling\n");
+			goto err_alloc;
+		}
+
+		ret = (__kms_addfb(fd, gem_handle, w, h, stride,
+		       format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
+		       &data2[i].fb.fb_id));
+		if (ret) {
+			igt_warn("failed to create framebuffer\n");
+			goto err_alloc;
+		}
+
+		data2[i].fb.width = w;
+		data2[i].fb.height = h;
+		data2[i].fb.gem_handle = gem_handle;
+
+		igt_plane_set_fb(plane, &data2[i].fb);
+		igt_plane_set_rotation(plane, IGT_ROTATION_0);
+
+		ret = igt_display_try_commit2(display, commit);
+		if (ret) {
+			igt_warn("failed to commit unrotated fb\n");
+			goto err_commit;
+		}
+
+		igt_plane_set_rotation(plane, IGT_ROTATION_90);
+
+		drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
+					 DRM_MODE_OBJECT_PLANE,
+					 plane->rotation_property,
+					 plane->rotation);
+		ret = igt_display_try_commit2(display, commit);
+		if (ret) {
+			igt_warn("failed to commit hardware rotated fb\n");
+			goto err_commit;
+		}
+	}
+
+err_alloc:
+	i--;
+err_commit:
+	kmstest_restore_vt_mode();
+
+	for (; i >= 0; i--)
+		igt_remove_fb(fd, &data2[i].fb);
+
+	igt_assert(ret == 0);
+}
+
 igt_main
 {
 	data_t data = {};
@@ -471,6 +572,11 @@ igt_main
 		test_plane_rotation_ytiled_obj(&data, IGT_PLANE_PRIMARY);
 	}
 
+	igt_subtest_f("exhaust-fences") {
+		igt_require(gen >= 9);
+		test_plane_rotation_exhaust_fences(&data, IGT_PLANE_PRIMARY);
+	}
+
 	igt_fixture {
 		igt_display_fini(&data.display);
 	}
-- 
2.4.3

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

Re: [PATCH] igt/kms_rotation_crc: Add a new subtest to exhaustively test for fence leaks (v2)

[Tvrtko Ursulin]

On 04/11/15 02:25, Vivek Kasireddy wrote:
> In this subtest, as a first step, MAX_FENCES+1 number of framebuffers are
> created backed up by objects that have multiple GGTT views (normal and
> rotated). Next, we have the i915 driver instantiate a normal view followed
> by a rotated view. We continue doing the above MAX_FENCES + 1 times.
>
> v2:
> - Add a igt_require() to check if there is enough GTT space left for
>    MAX_FENCES+1 framebuffers. (Tvrtko)
> - Make data2 local to test_plane_rotation_exhaust_fences(). (Tvrtko)
> - If there is a failure, deallocate all the previously allocated
>    framebuffers before asserting.
>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   tests/kms_rotation_crc.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 106 insertions(+)
>
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index ed6eeef..154c6a1 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -25,6 +25,7 @@
>   #include "igt.h"
>   #include <math.h>
>
> +#define MAX_FENCES 32
>
>   typedef struct {
>   	int gfx_fd;
> @@ -376,6 +377,106 @@ static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_ty
>   	igt_assert(ret == 0);
>   }
>
> +static void test_plane_rotation_exhaust_fences(data_t *data, enum igt_plane plane_type)
> +{
> +	igt_display_t *display = &data->display;
> +	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
> +	uint32_t format = DRM_FORMAT_XRGB8888;
> +	int bpp = igt_drm_format_to_bpp(format);
> +	enum igt_commit_style commit = COMMIT_LEGACY;
> +	int fd = data->gfx_fd;
> +	igt_output_t *output = &display->outputs[0];
> +	igt_plane_t *plane;
> +	drmModeModeInfo *mode;
> +	data_t data2[MAX_FENCES+1] = {};
> +	unsigned int stride, size, w, h;
> +	uint32_t gem_handle;
> +	uint64_t total_aperture_size, total_fbs_size;
> +	int i, ret;
> +
> +	igt_require(output != NULL && output->valid == true);
> +
> +	plane = igt_output_get_plane(output, plane_type);
> +	igt_require(igt_plane_supports_rotation(plane));
> +
> +	if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
> +		igt_require(data->display.has_universal_planes);
> +		commit = COMMIT_UNIVERSAL;
> +	}
> +
> +	mode = igt_output_get_mode(output);
> +	w = mode->hdisplay;
> +	h = mode->vdisplay;
> +
> +	for (stride = 512; stride < (w * bpp / 8); stride *= 2)
> +		;
> +	for (size = 1024*1024; size < stride * h; size *= 2)
> +		;
> +
> +	/*
> +	 * Make sure there is atleast 90% of the available GTT space left
> +	 * for creating (MAX_FENCES+1) framebuffers.
> +	 */
> +	total_fbs_size = size * (MAX_FENCES + 1);
> +	total_aperture_size = gem_available_aperture_size(fd);
> +	igt_require(total_fbs_size < total_aperture_size * 0.9);
> +
> +	igt_plane_set_fb(plane, NULL);
> +	igt_display_commit(display);
> +
> +	for (i = 0; i < MAX_FENCES + 1; i++) {
> +		gem_handle = gem_create(fd, size);
> +		ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride);
> +		if (ret) {
> +			igt_warn("failed to set tiling\n");
> +			goto err_alloc;

If it hits this path it leaks the gem_handle I think.

> +		}
> +
> +		ret = (__kms_addfb(fd, gem_handle, w, h, stride,
> +		       format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
> +		       &data2[i].fb.fb_id));
> +		if (ret) {
> +			igt_warn("failed to create framebuffer\n");
> +			goto err_alloc;

And here.

> +		}
> +
> +		data2[i].fb.width = w;
> +		data2[i].fb.height = h;
> +		data2[i].fb.gem_handle = gem_handle;
> +
> +		igt_plane_set_fb(plane, &data2[i].fb);
> +		igt_plane_set_rotation(plane, IGT_ROTATION_0);
> +
> +		ret = igt_display_try_commit2(display, commit);
> +		if (ret) {
> +			igt_warn("failed to commit unrotated fb\n");
> +			goto err_commit;
> +		}
> +
> +		igt_plane_set_rotation(plane, IGT_ROTATION_90);
> +
> +		drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
> +					 DRM_MODE_OBJECT_PLANE,
> +					 plane->rotation_property,
> +					 plane->rotation);
> +		ret = igt_display_try_commit2(display, commit);
> +		if (ret) {
> +			igt_warn("failed to commit hardware rotated fb\n");
> +			goto err_commit;
> +		}
> +	}
> +
> +err_alloc:
> +	i--;
> +err_commit:
> +	kmstest_restore_vt_mode();
> +
> +	for (; i >= 0; i--)
> +		igt_remove_fb(fd, &data2[i].fb);
> +
> +	igt_assert(ret == 0);
> +}
> +
>   igt_main
>   {
>   	data_t data = {};
> @@ -471,6 +572,11 @@ igt_main
>   		test_plane_rotation_ytiled_obj(&data, IGT_PLANE_PRIMARY);
>   	}
>
> +	igt_subtest_f("exhaust-fences") {
> +		igt_require(gen >= 9);
> +		test_plane_rotation_exhaust_fences(&data, IGT_PLANE_PRIMARY);
> +	}
> +
>   	igt_fixture {
>   		igt_display_fini(&data.display);
>   	}
>

Regards,

Tvrtko

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

[PATCH] igt/kms_rotation_crc: Add a new subtest to exhaustively test for fence leaks (v3)

[Vivek Kasireddy]

In this subtest, as a first step, MAX_FENCES+1 number of framebuffers are
created backed up by objects that have multiple GGTT views (normal and
rotated). Next, we have the i915 driver instantiate a normal view followed
by a rotated view. We continue doing the above MAX_FENCES + 1 times.

v2:
- Add a igt_require() to check if there is enough GTT space left for
  MAX_FENCES+1 framebuffers. (Tvrtko)
- Make data2 local to test_plane_rotation_exhaust_fences(). (Tvrtko)
- If there is a failure, deallocate all the previously allocated
  framebuffers before asserting.

v3: Close the gem handle if set_tiling or addfb fails. (Tvrtko)

Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 tests/kms_rotation_crc.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 108 insertions(+)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index ed6eeef..7e18b1e 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -25,6 +25,7 @@
 #include "igt.h"
 #include <math.h>
 
+#define MAX_FENCES 32
 
 typedef struct {
 	int gfx_fd;
@@ -376,6 +377,108 @@ static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_ty
 	igt_assert(ret == 0);
 }
 
+static void test_plane_rotation_exhaust_fences(data_t *data, enum igt_plane plane_type)
+{
+	igt_display_t *display = &data->display;
+	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
+	uint32_t format = DRM_FORMAT_XRGB8888;
+	int bpp = igt_drm_format_to_bpp(format);
+	enum igt_commit_style commit = COMMIT_LEGACY;
+	int fd = data->gfx_fd;
+	igt_output_t *output = &display->outputs[0];
+	igt_plane_t *plane;
+	drmModeModeInfo *mode;
+	data_t data2[MAX_FENCES+1] = {};
+	unsigned int stride, size, w, h;
+	uint32_t gem_handle;
+	uint64_t total_aperture_size, total_fbs_size;
+	int i, ret;
+
+	igt_require(output != NULL && output->valid == true);
+
+	plane = igt_output_get_plane(output, plane_type);
+	igt_require(igt_plane_supports_rotation(plane));
+
+	if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
+		igt_require(data->display.has_universal_planes);
+		commit = COMMIT_UNIVERSAL;
+	}
+
+	mode = igt_output_get_mode(output);
+	w = mode->hdisplay;
+	h = mode->vdisplay;
+
+	for (stride = 512; stride < (w * bpp / 8); stride *= 2)
+		;
+	for (size = 1024*1024; size < stride * h; size *= 2)
+		;
+
+	/*
+	 * Make sure there is atleast 90% of the available GTT space left
+	 * for creating (MAX_FENCES+1) framebuffers.
+	 */
+	total_fbs_size = size * (MAX_FENCES + 1);
+	total_aperture_size = gem_available_aperture_size(fd);
+	igt_require(total_fbs_size < total_aperture_size * 0.9);
+
+	igt_plane_set_fb(plane, NULL);
+	igt_display_commit(display);
+
+	for (i = 0; i < MAX_FENCES + 1; i++) {
+		gem_handle = gem_create(fd, size);
+		ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride);
+		if (ret) {
+			igt_warn("failed to set tiling\n");
+			goto err_alloc;
+		}
+
+		ret = (__kms_addfb(fd, gem_handle, w, h, stride,
+		       format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
+		       &data2[i].fb.fb_id));
+		if (ret) {
+			igt_warn("failed to create framebuffer\n");
+			goto err_alloc;
+		}
+
+		data2[i].fb.width = w;
+		data2[i].fb.height = h;
+		data2[i].fb.gem_handle = gem_handle;
+
+		igt_plane_set_fb(plane, &data2[i].fb);
+		igt_plane_set_rotation(plane, IGT_ROTATION_0);
+
+		ret = igt_display_try_commit2(display, commit);
+		if (ret) {
+			igt_warn("failed to commit unrotated fb\n");
+			goto err_commit;
+		}
+
+		igt_plane_set_rotation(plane, IGT_ROTATION_90);
+
+		drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
+					 DRM_MODE_OBJECT_PLANE,
+					 plane->rotation_property,
+					 plane->rotation);
+		ret = igt_display_try_commit2(display, commit);
+		if (ret) {
+			igt_warn("failed to commit hardware rotated fb\n");
+			goto err_commit;
+		}
+	}
+
+err_alloc:
+	if (ret)
+		gem_close(fd, gem_handle);
+
+	i--;
+err_commit:
+	for (; i >= 0; i--)
+		igt_remove_fb(fd, &data2[i].fb);
+
+	kmstest_restore_vt_mode();
+	igt_assert(ret == 0);
+}
+
 igt_main
 {
 	data_t data = {};
@@ -471,6 +574,11 @@ igt_main
 		test_plane_rotation_ytiled_obj(&data, IGT_PLANE_PRIMARY);
 	}
 
+	igt_subtest_f("exhaust-fences") {
+		igt_require(gen >= 9);
+		test_plane_rotation_exhaust_fences(&data, IGT_PLANE_PRIMARY);
+	}
+
 	igt_fixture {
 		igt_display_fini(&data.display);
 	}
-- 
2.4.3

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

Re: [PATCH] igt/kms_rotation_crc: Add a new subtest to exhaustively test for fence leaks (v3)

[Tvrtko Ursulin]

On 05/11/15 00:10, Vivek Kasireddy wrote:
> In this subtest, as a first step, MAX_FENCES+1 number of framebuffers are
> created backed up by objects that have multiple GGTT views (normal and
> rotated). Next, we have the i915 driver instantiate a normal view followed
> by a rotated view. We continue doing the above MAX_FENCES + 1 times.
>
> v2:
> - Add a igt_require() to check if there is enough GTT space left for
>    MAX_FENCES+1 framebuffers. (Tvrtko)
> - Make data2 local to test_plane_rotation_exhaust_fences(). (Tvrtko)
> - If there is a failure, deallocate all the previously allocated
>    framebuffers before asserting.
>
> v3: Close the gem handle if set_tiling or addfb fails. (Tvrtko)
>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   tests/kms_rotation_crc.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 108 insertions(+)
>
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index ed6eeef..7e18b1e 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -25,6 +25,7 @@
>   #include "igt.h"
>   #include <math.h>
>
> +#define MAX_FENCES 32
>
>   typedef struct {
>   	int gfx_fd;
> @@ -376,6 +377,108 @@ static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_ty
>   	igt_assert(ret == 0);
>   }
>
> +static void test_plane_rotation_exhaust_fences(data_t *data, enum igt_plane plane_type)
> +{
> +	igt_display_t *display = &data->display;
> +	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
> +	uint32_t format = DRM_FORMAT_XRGB8888;
> +	int bpp = igt_drm_format_to_bpp(format);
> +	enum igt_commit_style commit = COMMIT_LEGACY;
> +	int fd = data->gfx_fd;
> +	igt_output_t *output = &display->outputs[0];
> +	igt_plane_t *plane;
> +	drmModeModeInfo *mode;
> +	data_t data2[MAX_FENCES+1] = {};
> +	unsigned int stride, size, w, h;
> +	uint32_t gem_handle;
> +	uint64_t total_aperture_size, total_fbs_size;
> +	int i, ret;
> +
> +	igt_require(output != NULL && output->valid == true);
> +
> +	plane = igt_output_get_plane(output, plane_type);
> +	igt_require(igt_plane_supports_rotation(plane));
> +
> +	if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
> +		igt_require(data->display.has_universal_planes);
> +		commit = COMMIT_UNIVERSAL;
> +	}
> +
> +	mode = igt_output_get_mode(output);
> +	w = mode->hdisplay;
> +	h = mode->vdisplay;
> +
> +	for (stride = 512; stride < (w * bpp / 8); stride *= 2)
> +		;
> +	for (size = 1024*1024; size < stride * h; size *= 2)
> +		;
> +
> +	/*
> +	 * Make sure there is atleast 90% of the available GTT space left
> +	 * for creating (MAX_FENCES+1) framebuffers.
> +	 */
> +	total_fbs_size = size * (MAX_FENCES + 1);
> +	total_aperture_size = gem_available_aperture_size(fd);
> +	igt_require(total_fbs_size < total_aperture_size * 0.9);
> +
> +	igt_plane_set_fb(plane, NULL);
> +	igt_display_commit(display);
> +
> +	for (i = 0; i < MAX_FENCES + 1; i++) {
> +		gem_handle = gem_create(fd, size);
> +		ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride);
> +		if (ret) {
> +			igt_warn("failed to set tiling\n");
> +			goto err_alloc;
> +		}
> +
> +		ret = (__kms_addfb(fd, gem_handle, w, h, stride,
> +		       format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
> +		       &data2[i].fb.fb_id));
> +		if (ret) {
> +			igt_warn("failed to create framebuffer\n");
> +			goto err_alloc;
> +		}
> +
> +		data2[i].fb.width = w;
> +		data2[i].fb.height = h;
> +		data2[i].fb.gem_handle = gem_handle;
> +
> +		igt_plane_set_fb(plane, &data2[i].fb);
> +		igt_plane_set_rotation(plane, IGT_ROTATION_0);
> +
> +		ret = igt_display_try_commit2(display, commit);
> +		if (ret) {
> +			igt_warn("failed to commit unrotated fb\n");
> +			goto err_commit;
> +		}
> +
> +		igt_plane_set_rotation(plane, IGT_ROTATION_90);
> +
> +		drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
> +					 DRM_MODE_OBJECT_PLANE,
> +					 plane->rotation_property,
> +					 plane->rotation);
> +		ret = igt_display_try_commit2(display, commit);
> +		if (ret) {
> +			igt_warn("failed to commit hardware rotated fb\n");
> +			goto err_commit;
> +		}
> +	}
> +
> +err_alloc:
> +	if (ret)
> +		gem_close(fd, gem_handle);
> +
> +	i--;
> +err_commit:
> +	for (; i >= 0; i--)
> +		igt_remove_fb(fd, &data2[i].fb);
> +
> +	kmstest_restore_vt_mode();
> +	igt_assert(ret == 0);
> +}
> +
>   igt_main
>   {
>   	data_t data = {};
> @@ -471,6 +574,11 @@ igt_main
>   		test_plane_rotation_ytiled_obj(&data, IGT_PLANE_PRIMARY);
>   	}
>
> +	igt_subtest_f("exhaust-fences") {
> +		igt_require(gen >= 9);
> +		test_plane_rotation_exhaust_fences(&data, IGT_PLANE_PRIMARY);
> +	}
> +
>   	igt_fixture {
>   		igt_display_fini(&data.display);
>   	}
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

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