[PATCH i-g-t 0/6] kms_plane_scaling fixes and enhancement

[PATCH i-g-t 0/6] kms_plane_scaling fixes and enhancement

[Vidya Srinivas]

This series fixes the current scaler igt test failures and enhances
kms_plane_scaling and kms_plane for covering subtests below:
- verify all the supported pixel formats in planes
- combination of rotation and scaling
- combination of tiling and scaling
- multi-plane/multi-pipe scaling
It also enhances igt library by adding wrappers to get the plane supported
pixel formats.

Jyoti Yadav (3):
  i-g-t kms_plane_scaling: test scaling with tiling rotation and pixel
    formats
  i-g-t: kms_plane_scaling: test scaler with clipping clamping
  i-g-t: kms_plane_scaling: test for multi pipe with scaling

Mahesh Kumar (3):
  i-g-t: kms_plane_scaling: Fix basic scaling test
  i-g-t: lib: Add plane pixel format support
  i-g-t: lib/igt_kms: Run kms_plane for all supported pixel formats

 lib/igt_fb.c              |  16 ++
 lib/igt_kms.c             |  29 +++
 lib/igt_kms.h             |   7 +
 tests/kms_plane.c         | 123 ++++++++++
 tests/kms_plane_scaling.c | 602 +++++++++++++++++++++++++++++++++++++---------
 5 files changed, 666 insertions(+), 111 deletions(-)

-- 
2.7.4

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

[PATCH i-g-t 1/6] i-g-t: kms_plane_scaling: Fix basic scaling test

[Vidya Srinivas]

From: Mahesh Kumar <mahesh1.kumar@intel.com>

PIPEC doesnt have 3rd plane in GEN9. So, we skip the
3rd plane related scaling test where 2nd OVERLAY
plane is not available.

Restricting downscaling to (9/10)x original size of the
image to avoid "Max pixel rate limitation" of the hardware.

Later patches in this series will cover corner cases of
scaling.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 tests/kms_plane_scaling.c | 234 +++++++++++++++++++++++++---------------------
 1 file changed, 125 insertions(+), 109 deletions(-)

diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
index 403df47..a827cb3 100644
--- a/tests/kms_plane_scaling.c
+++ b/tests/kms_plane_scaling.c
@@ -163,144 +163,159 @@ static void iterate_plane_scaling(data_t *d, drmModeModeInfo *mode)
 	}
 }
 
-static void test_plane_scaling(data_t *d)
+static void
+test_plane_scaling_on_pipe(data_t *d, enum pipe pipe, igt_output_t *output)
 {
 	igt_display_t *display = &d->display;
-	igt_output_t *output;
-	enum pipe pipe;
-	int valid_tests = 0;
+	drmModeModeInfo *mode;
 	int primary_plane_scaling = 0; /* For now */
 
-	igt_require(d->num_scalers);
+	igt_output_set_pipe(output, pipe);
+	mode = igt_output_get_mode(output);
+
+	/* allocate fb2 with image size */
+	d->fb_id2 = igt_create_image_fb(d->drm_fd, 0, 0,
+			DRM_FORMAT_XRGB8888,
+			LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */
+			FILE_NAME, &d->fb2);
+	igt_assert(d->fb_id2);
+
+	d->fb_id3 = igt_create_pattern_fb(d->drm_fd,
+			mode->hdisplay, mode->vdisplay,
+			DRM_FORMAT_XRGB8888,
+			LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */
+			&d->fb3);
+	igt_assert(d->fb_id3);
+
+	/* Set up display with plane 1 */
+	d->plane1 = igt_output_get_plane(output, 0);
+	prepare_crtc(d, output, pipe, d->plane1, mode, COMMIT_UNIVERSAL);
+
+	if (primary_plane_scaling) {
+		/* Primary plane upscaling */
+		igt_fb_set_position(&d->fb1, d->plane1, 100, 100);
+		igt_fb_set_size(&d->fb1, d->plane1, 500, 500);
+		igt_plane_set_position(d->plane1, 0, 0);
+		igt_plane_set_size(d->plane1, mode->hdisplay, mode->vdisplay);
+		igt_display_commit2(display, COMMIT_UNIVERSAL);
 
-	for_each_pipe_with_valid_output(display, pipe, output) {
-		drmModeModeInfo *mode;
-
-		igt_output_set_pipe(output, pipe);
-
-		mode = igt_output_get_mode(output);
-
-		/* allocate fb2 with image size */
-		d->fb_id2 = igt_create_image_fb(d->drm_fd, 0, 0,
-						DRM_FORMAT_XRGB8888,
-						LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */
-						FILE_NAME, &d->fb2);
-		igt_assert(d->fb_id2);
-
-		d->fb_id3 = igt_create_pattern_fb(d->drm_fd,
-						  mode->hdisplay, mode->vdisplay,
-						  DRM_FORMAT_XRGB8888,
-						  LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */
-						  &d->fb3);
-		igt_assert(d->fb_id3);
-
-		/* Set up display with plane 1 */
-		d->plane1 = igt_output_get_plane(output, 0);
-		prepare_crtc(d, output, pipe, d->plane1, mode, COMMIT_UNIVERSAL);
-
-		if (primary_plane_scaling) {
-			/* Primary plane upscaling */
-			igt_fb_set_position(&d->fb1, d->plane1, 100, 100);
-			igt_fb_set_size(&d->fb1, d->plane1, 500, 500);
-			igt_plane_set_position(d->plane1, 0, 0);
-			igt_plane_set_size(d->plane1, mode->hdisplay, mode->vdisplay);
-			igt_display_commit2(display, COMMIT_UNIVERSAL);
+		/* Primary plane 1:1 no scaling */
+		igt_fb_set_position(&d->fb1, d->plane1, 0, 0);
+		igt_fb_set_size(&d->fb1, d->plane1, d->fb1.width, d->fb1.height);
+		igt_plane_set_position(d->plane1, 0, 0);
+		igt_plane_set_size(d->plane1, mode->hdisplay, mode->vdisplay);
+		igt_display_commit2(display, COMMIT_UNIVERSAL);
+	}
 
-			/* Primary plane 1:1 no scaling */
-			igt_fb_set_position(&d->fb1, d->plane1, 0, 0);
-			igt_fb_set_size(&d->fb1, d->plane1, d->fb1.width, d->fb1.height);
-			igt_plane_set_position(d->plane1, 0, 0);
-			igt_plane_set_size(d->plane1, mode->hdisplay, mode->vdisplay);
-			igt_display_commit2(display, COMMIT_UNIVERSAL);
-		}
+	/* Set up fb2->plane2 mapping. */
+	d->plane2 = igt_output_get_plane(output, 1);
+	igt_plane_set_fb(d->plane2, &d->fb2);
 
-		/* Set up fb2->plane2 mapping. */
-		d->plane2 = igt_output_get_plane(output, 1);
-		igt_plane_set_fb(d->plane2, &d->fb2);
+	/* 2nd plane windowed */
+	igt_fb_set_position(&d->fb2, d->plane2, 100, 100);
+	igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width-200, d->fb2.height-200);
+	igt_plane_set_position(d->plane2, 100, 100);
+	igt_plane_set_size(d->plane2, mode->hdisplay-200, mode->vdisplay-200);
+	igt_display_commit2(display, COMMIT_UNIVERSAL);
 
-		/* 2nd plane windowed */
-		igt_fb_set_position(&d->fb2, d->plane2, 100, 100);
-		igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width-200, d->fb2.height-200);
-		igt_plane_set_position(d->plane2, 100, 100);
-		igt_plane_set_size(d->plane2, mode->hdisplay-200, mode->vdisplay-200);
-		igt_display_commit2(display, COMMIT_UNIVERSAL);
+	iterate_plane_scaling(d, mode);
 
-		iterate_plane_scaling(d, mode);
+	/* 2nd plane up scaling */
+	igt_fb_set_position(&d->fb2, d->plane2, 100, 100);
+	igt_fb_set_size(&d->fb2, d->plane2, 500, 500);
+	igt_plane_set_position(d->plane2, 10, 10);
+	igt_plane_set_size(d->plane2, mode->hdisplay-20, mode->vdisplay-20);
+	igt_display_commit2(display, COMMIT_UNIVERSAL);
 
-		/* 2nd plane up scaling */
-		igt_fb_set_position(&d->fb2, d->plane2, 100, 100);
-		igt_fb_set_size(&d->fb2, d->plane2, 500, 500);
-		igt_plane_set_position(d->plane2, 10, 10);
-		igt_plane_set_size(d->plane2, mode->hdisplay-20, mode->vdisplay-20);
-		igt_display_commit2(display, COMMIT_UNIVERSAL);
+	/* 2nd plane downscaling */
+	igt_fb_set_position(&d->fb2, d->plane2, 0, 0);
+	igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width, d->fb2.height);
+	igt_plane_set_position(d->plane2, 10, 10);
 
-		/* 2nd plane downscaling */
-		igt_fb_set_position(&d->fb2, d->plane2, 0, 0);
-		igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width, d->fb2.height);
-		igt_plane_set_position(d->plane2, 10, 10);
-		igt_plane_set_size(d->plane2, 500, 500 * d->fb2.height/d->fb2.width);
+	/* Downscale (10/9)x of original image */
+	igt_plane_set_size(d->plane2, (d->fb2.width * 10)/9, (d->fb2.height * 10)/9);
+	igt_display_commit2(display, COMMIT_UNIVERSAL);
+
+	if (primary_plane_scaling) {
+		/* Primary plane up scaling */
+		igt_fb_set_position(&d->fb1, d->plane1, 100, 100);
+		igt_fb_set_size(&d->fb1, d->plane1, 500, 500);
+		igt_plane_set_position(d->plane1, 0, 0);
+		igt_plane_set_size(d->plane1, mode->hdisplay, mode->vdisplay);
 		igt_display_commit2(display, COMMIT_UNIVERSAL);
+	}
 
-		if (primary_plane_scaling) {
-			/* Primary plane up scaling */
-			igt_fb_set_position(&d->fb1, d->plane1, 100, 100);
-			igt_fb_set_size(&d->fb1, d->plane1, 500, 500);
-			igt_plane_set_position(d->plane1, 0, 0);
-			igt_plane_set_size(d->plane1, mode->hdisplay, mode->vdisplay);
-			igt_display_commit2(display, COMMIT_UNIVERSAL);
-		}
+	/* Set up fb3->plane3 mapping. */
+	d->plane3 = igt_output_get_plane(output, 2);
+	igt_plane_set_fb(d->plane3, &d->fb3);
 
-		/* Set up fb3->plane3 mapping. */
-		d->plane3 = igt_output_get_plane(output, 2);
-		igt_plane_set_fb(d->plane3, &d->fb3);
+	if(d->plane3->type == DRM_PLANE_TYPE_CURSOR) {
+		igt_info("Plane-3 doesnt exist on pipe %s\n", kmstest_pipe_name(pipe));
+		goto cleanup;
+	}
 
-		/* 3rd plane windowed - no scaling */
-		igt_fb_set_position(&d->fb3, d->plane3, 100, 100);
-		igt_fb_set_size(&d->fb3, d->plane3, d->fb3.width-300, d->fb3.height-300);
-		igt_plane_set_position(d->plane3, 100, 100);
-		igt_plane_set_size(d->plane3, mode->hdisplay-300, mode->vdisplay-300);
-		igt_display_commit2(display, COMMIT_UNIVERSAL);
+	/* 3rd plane windowed - no scaling */
+	igt_fb_set_position(&d->fb3, d->plane3, 100, 100);
+	igt_fb_set_size(&d->fb3, d->plane3, d->fb3.width-300, d->fb3.height-300);
+	igt_plane_set_position(d->plane3, 100, 100);
+	igt_plane_set_size(d->plane3, mode->hdisplay-300, mode->vdisplay-300);
+	igt_display_commit2(display, COMMIT_UNIVERSAL);
+
+	/* Switch scaler from plane 2 to plane 3 */
+	igt_fb_set_position(&d->fb2, d->plane2, 100, 100);
+	igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width-200, d->fb2.height-200);
+	igt_plane_set_position(d->plane2, 100, 100);
+	igt_plane_set_size(d->plane2, d->fb2.width-200, d->fb2.height-200);
+
+	igt_fb_set_position(&d->fb3, d->plane3, 100, 100);
+	igt_fb_set_size(&d->fb3, d->plane3, d->fb3.width-400, d->fb3.height-400);
+	igt_plane_set_position(d->plane3, 10, 10);
+	igt_plane_set_size(d->plane3, mode->hdisplay-300, mode->vdisplay-300);
+	igt_display_commit2(display, COMMIT_UNIVERSAL);
+
+	if (primary_plane_scaling) {
+		/* Switch scaler from plane 1 to plane 2 */
+		igt_fb_set_position(&d->fb1, d->plane1, 0, 0);
+		igt_fb_set_size(&d->fb1, d->plane1, d->fb1.width, d->fb1.height);
+		igt_plane_set_position(d->plane1, 0, 0);
+		igt_plane_set_size(d->plane1, mode->hdisplay, mode->vdisplay);
 
-		/* Switch scaler from plane 2 to plane 3 */
 		igt_fb_set_position(&d->fb2, d->plane2, 100, 100);
-		igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width-200, d->fb2.height-200);
+		igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width-500,d->fb2.height-500);
 		igt_plane_set_position(d->plane2, 100, 100);
-		igt_plane_set_size(d->plane2, d->fb2.width-200, d->fb2.height-200);
-
-		igt_fb_set_position(&d->fb3, d->plane3, 100, 100);
-		igt_fb_set_size(&d->fb3, d->plane3, d->fb3.width-400, d->fb3.height-400);
-		igt_plane_set_position(d->plane3, 10, 10);
-		igt_plane_set_size(d->plane3, mode->hdisplay-300, mode->vdisplay-300);
+		igt_plane_set_size(d->plane2, mode->hdisplay-200, mode->vdisplay-200);
 		igt_display_commit2(display, COMMIT_UNIVERSAL);
+	}
 
-		if (primary_plane_scaling) {
-			/* Switch scaler from plane 1 to plane 2 */
-			igt_fb_set_position(&d->fb1, d->plane1, 0, 0);
-			igt_fb_set_size(&d->fb1, d->plane1, d->fb1.width, d->fb1.height);
-			igt_plane_set_position(d->plane1, 0, 0);
-			igt_plane_set_size(d->plane1, mode->hdisplay, mode->vdisplay);
-
-			igt_fb_set_position(&d->fb2, d->plane2, 100, 100);
-			igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width-500,d->fb2.height-500);
-			igt_plane_set_position(d->plane2, 100, 100);
-			igt_plane_set_size(d->plane2, mode->hdisplay-200, mode->vdisplay-200);
-			igt_display_commit2(display, COMMIT_UNIVERSAL);
-		}
+cleanup:
+	/* back to single plane mode */
+	igt_plane_set_fb(d->plane2, NULL);
+	igt_plane_set_fb(d->plane3, NULL);
+	igt_display_commit2(display, COMMIT_UNIVERSAL);
 
-		/* back to single plane mode */
-		igt_plane_set_fb(d->plane2, NULL);
-		igt_plane_set_fb(d->plane3, NULL);
-		igt_display_commit2(display, COMMIT_UNIVERSAL);
+	cleanup_crtc(d, output, d->plane1);
+}
+
+static void test_plane_scaling(data_t *d, enum pipe pipe)
+{
+	igt_output_t *output;
+	int valid_tests = 0;
+
+	igt_require(d->num_scalers);
 
+	for_each_valid_output_on_pipe(&d->display, pipe, output) {
+		test_plane_scaling_on_pipe(d, pipe, output);
+		igt_output_set_pipe(output, PIPE_ANY);
 		valid_tests++;
-		cleanup_crtc(d, output, d->plane1);
 	}
+
 	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
 }
 
 igt_simple_main
 {
 	data_t data = {};
+	enum pipe pipe;
 
 	igt_skip_on_simulation();
 
@@ -312,7 +327,8 @@ igt_simple_main
 
 	data.num_scalers = intel_gen(data.devid) >= 9 ? 2 : 0;
 
-	test_plane_scaling(&data);
+	for_each_pipe_static(pipe)
+		test_plane_scaling(&data, pipe);
 
 	igt_display_fini(&data.display);
 }
-- 
2.7.4

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

Re: [PATCH i-g-t 1/6] i-g-t: kms_plane_scaling: Fix basic scaling test

[Maarten Lankhorst]

Op 13-12-17 om 10:50 schreef Vidya Srinivas:
> From: Mahesh Kumar <mahesh1.kumar@intel.com>
>
> PIPEC doesnt have 3rd plane in GEN9. So, we skip the
> 3rd plane related scaling test where 2nd OVERLAY
> plane is not available.
>
> Restricting downscaling to (9/10)x original size of the
> image to avoid "Max pixel rate limitation" of the hardware.
>
> Later patches in this series will cover corner cases of
> scaling.
>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
>  tests/kms_plane_scaling.c | 234 +++++++++++++++++++++++++---------------------
>  1 file changed, 125 insertions(+), 109 deletions(-)
>
> diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> index 403df47..a827cb3 100644
> --- a/tests/kms_plane_scaling.c
> +++ b/tests/kms_plane_scaling.c
> @@ -163,144 +163,159 @@ static void iterate_plane_scaling(data_t *d, drmModeModeInfo *mode)
>  	}
>  }

The scaler tests should really require display->is_atomic. Even if there are
theoretically more limits on the amount of scalers, having a YUV mode or interlaced
mode may take a scaler, causing this test to fail.

With try_commit_atomic, it would be possible to test if enough scalers are available.

On top of that, I would really like the test to be more readable, and having
test_plane_scaling_on_pipe split up in smaller pieces would accomplish that.

I would do some split ups first before doing any fixes, it's hard to read what's
changed through the whitespace changes, even if the code is correct.

Also this test needs to be split up into subtests, this is a good start for it.

Replace igt_simple_main with igt_main, and add subtests for each pipe and various
tests being performed. See for example kms_color, this test is already trying to do too
much in one go. :)

~Maarten

>  
> -static void test_plane_scaling(data_t *d)
> +static void
> +test_plane_scaling_on_pipe(data_t *d, enum pipe pipe, igt_output_t *output)
>  {
>  	igt_display_t *display = &d->display;
> -	igt_output_t *output;
> -	enum pipe pipe;
> -	int valid_tests = 0;
> +	drmModeModeInfo *mode;
>  	int primary_plane_scaling = 0; /* For now */
>  
> -	igt_require(d->num_scalers);
> +	igt_output_set_pipe(output, pipe);
> +	mode = igt_output_get_mode(output);
> +
> +	/* allocate fb2 with image size */
> +	d->fb_id2 = igt_create_image_fb(d->drm_fd, 0, 0,
> +			DRM_FORMAT_XRGB8888,
> +			LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */
> +			FILE_NAME, &d->fb2);
> +	igt_assert(d->fb_id2);
> +
> +	d->fb_id3 = igt_create_pattern_fb(d->drm_fd,
> +			mode->hdisplay, mode->vdisplay,
> +			DRM_FORMAT_XRGB8888,
> +			LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */
> +			&d->fb3);
> +	igt_assert(d->fb_id3);
> +
> +	/* Set up display with plane 1 */
> +	d->plane1 = igt_output_get_plane(output, 0);
> +	prepare_crtc(d, output, pipe, d->plane1, mode, COMMIT_UNIVERSAL);
> +
> +	if (primary_plane_scaling) {
> +		/* Primary plane upscaling */
> +		igt_fb_set_position(&d->fb1, d->plane1, 100, 100);
> +		igt_fb_set_size(&d->fb1, d->plane1, 500, 500);
> +		igt_plane_set_position(d->plane1, 0, 0);
> +		igt_plane_set_size(d->plane1, mode->hdisplay, mode->vdisplay);
> +		igt_display_commit2(display, COMMIT_UNIVERSAL);
>  
> -	for_each_pipe_with_valid_output(display, pipe, output) {
> -		drmModeModeInfo *mode;
> -
> -		igt_output_set_pipe(output, pipe);
> -
> -		mode = igt_output_get_mode(output);
> -
> -		/* allocate fb2 with image size */
> -		d->fb_id2 = igt_create_image_fb(d->drm_fd, 0, 0,
> -						DRM_FORMAT_XRGB8888,
> -						LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */
> -						FILE_NAME, &d->fb2);
> -		igt_assert(d->fb_id2);
> -
> -		d->fb_id3 = igt_create_pattern_fb(d->drm_fd,
> -						  mode->hdisplay, mode->vdisplay,
> -						  DRM_FORMAT_XRGB8888,
> -						  LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */
> -						  &d->fb3);
> -		igt_assert(d->fb_id3);
> -
> -		/* Set up display with plane 1 */
> -		d->plane1 = igt_output_get_plane(output, 0);
> -		prepare_crtc(d, output, pipe, d->plane1, mode, COMMIT_UNIVERSAL);
> -
> -		if (primary_plane_scaling) {
> -			/* Primary plane upscaling */
> -			igt_fb_set_position(&d->fb1, d->plane1, 100, 100);
> -			igt_fb_set_size(&d->fb1, d->plane1, 500, 500);
> -			igt_plane_set_position(d->plane1, 0, 0);
> -			igt_plane_set_size(d->plane1, mode->hdisplay, mode->vdisplay);
> -			igt_display_commit2(display, COMMIT_UNIVERSAL);
> +		/* Primary plane 1:1 no scaling */
> +		igt_fb_set_position(&d->fb1, d->plane1, 0, 0);
> +		igt_fb_set_size(&d->fb1, d->plane1, d->fb1.width, d->fb1.height);
> +		igt_plane_set_position(d->plane1, 0, 0);
> +		igt_plane_set_size(d->plane1, mode->hdisplay, mode->vdisplay);
> +		igt_display_commit2(display, COMMIT_UNIVERSAL);
> +	}
>  
> -			/* Primary plane 1:1 no scaling */
> -			igt_fb_set_position(&d->fb1, d->plane1, 0, 0);
> -			igt_fb_set_size(&d->fb1, d->plane1, d->fb1.width, d->fb1.height);
> -			igt_plane_set_position(d->plane1, 0, 0);
> -			igt_plane_set_size(d->plane1, mode->hdisplay, mode->vdisplay);
> -			igt_display_commit2(display, COMMIT_UNIVERSAL);
> -		}
> +	/* Set up fb2->plane2 mapping. */
> +	d->plane2 = igt_output_get_plane(output, 1);
> +	igt_plane_set_fb(d->plane2, &d->fb2);
>  
> -		/* Set up fb2->plane2 mapping. */
> -		d->plane2 = igt_output_get_plane(output, 1);
> -		igt_plane_set_fb(d->plane2, &d->fb2);
> +	/* 2nd plane windowed */
> +	igt_fb_set_position(&d->fb2, d->plane2, 100, 100);
> +	igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width-200, d->fb2.height-200);
> +	igt_plane_set_position(d->plane2, 100, 100);
> +	igt_plane_set_size(d->plane2, mode->hdisplay-200, mode->vdisplay-200);
> +	igt_display_commit2(display, COMMIT_UNIVERSAL);
>  
> -		/* 2nd plane windowed */
> -		igt_fb_set_position(&d->fb2, d->plane2, 100, 100);
> -		igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width-200, d->fb2.height-200);
> -		igt_plane_set_position(d->plane2, 100, 100);
> -		igt_plane_set_size(d->plane2, mode->hdisplay-200, mode->vdisplay-200);
> -		igt_display_commit2(display, COMMIT_UNIVERSAL);
> +	iterate_plane_scaling(d, mode);
>  
> -		iterate_plane_scaling(d, mode);
> +	/* 2nd plane up scaling */
> +	igt_fb_set_position(&d->fb2, d->plane2, 100, 100);
> +	igt_fb_set_size(&d->fb2, d->plane2, 500, 500);
> +	igt_plane_set_position(d->plane2, 10, 10);
> +	igt_plane_set_size(d->plane2, mode->hdisplay-20, mode->vdisplay-20);
> +	igt_display_commit2(display, COMMIT_UNIVERSAL);
>  
> -		/* 2nd plane up scaling */
> -		igt_fb_set_position(&d->fb2, d->plane2, 100, 100);
> -		igt_fb_set_size(&d->fb2, d->plane2, 500, 500);
> -		igt_plane_set_position(d->plane2, 10, 10);
> -		igt_plane_set_size(d->plane2, mode->hdisplay-20, mode->vdisplay-20);
> -		igt_display_commit2(display, COMMIT_UNIVERSAL);
> +	/* 2nd plane downscaling */
> +	igt_fb_set_position(&d->fb2, d->plane2, 0, 0);
> +	igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width, d->fb2.height);
> +	igt_plane_set_position(d->plane2, 10, 10);
>  
> -		/* 2nd plane downscaling */
> -		igt_fb_set_position(&d->fb2, d->plane2, 0, 0);
> -		igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width, d->fb2.height);
> -		igt_plane_set_position(d->plane2, 10, 10);
> -		igt_plane_set_size(d->plane2, 500, 500 * d->fb2.height/d->fb2.width);
> +	/* Downscale (10/9)x of original image */
> +	igt_plane_set_size(d->plane2, (d->fb2.width * 10)/9, (d->fb2.height * 10)/9);
> +	igt_display_commit2(display, COMMIT_UNIVERSAL);
> +
> +	if (primary_plane_scaling) {
> +		/* Primary plane up scaling */
> +		igt_fb_set_position(&d->fb1, d->plane1, 100, 100);
> +		igt_fb_set_size(&d->fb1, d->plane1, 500, 500);
> +		igt_plane_set_position(d->plane1, 0, 0);
> +		igt_plane_set_size(d->plane1, mode->hdisplay, mode->vdisplay);
>  		igt_display_commit2(display, COMMIT_UNIVERSAL);
> +	}
>  
> -		if (primary_plane_scaling) {
> -			/* Primary plane up scaling */
> -			igt_fb_set_position(&d->fb1, d->plane1, 100, 100);
> -			igt_fb_set_size(&d->fb1, d->plane1, 500, 500);
> -			igt_plane_set_position(d->plane1, 0, 0);
> -			igt_plane_set_size(d->plane1, mode->hdisplay, mode->vdisplay);
> -			igt_display_commit2(display, COMMIT_UNIVERSAL);
> -		}
> +	/* Set up fb3->plane3 mapping. */
> +	d->plane3 = igt_output_get_plane(output, 2);
> +	igt_plane_set_fb(d->plane3, &d->fb3);
>  
> -		/* Set up fb3->plane3 mapping. */
> -		d->plane3 = igt_output_get_plane(output, 2);
> -		igt_plane_set_fb(d->plane3, &d->fb3);
> +	if(d->plane3->type == DRM_PLANE_TYPE_CURSOR) {
> +		igt_info("Plane-3 doesnt exist on pipe %s\n", kmstest_pipe_name(pipe));
> +		goto cleanup;
> +	}
>  
> -		/* 3rd plane windowed - no scaling */
> -		igt_fb_set_position(&d->fb3, d->plane3, 100, 100);
> -		igt_fb_set_size(&d->fb3, d->plane3, d->fb3.width-300, d->fb3.height-300);
> -		igt_plane_set_position(d->plane3, 100, 100);
> -		igt_plane_set_size(d->plane3, mode->hdisplay-300, mode->vdisplay-300);
> -		igt_display_commit2(display, COMMIT_UNIVERSAL);
> +	/* 3rd plane windowed - no scaling */
> +	igt_fb_set_position(&d->fb3, d->plane3, 100, 100);
> +	igt_fb_set_size(&d->fb3, d->plane3, d->fb3.width-300, d->fb3.height-300);
> +	igt_plane_set_position(d->plane3, 100, 100);
> +	igt_plane_set_size(d->plane3, mode->hdisplay-300, mode->vdisplay-300);
> +	igt_display_commit2(display, COMMIT_UNIVERSAL);
> +
> +	/* Switch scaler from plane 2 to plane 3 */
> +	igt_fb_set_position(&d->fb2, d->plane2, 100, 100);
> +	igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width-200, d->fb2.height-200);
> +	igt_plane_set_position(d->plane2, 100, 100);
> +	igt_plane_set_size(d->plane2, d->fb2.width-200, d->fb2.height-200);
> +
> +	igt_fb_set_position(&d->fb3, d->plane3, 100, 100);
> +	igt_fb_set_size(&d->fb3, d->plane3, d->fb3.width-400, d->fb3.height-400);
> +	igt_plane_set_position(d->plane3, 10, 10);
> +	igt_plane_set_size(d->plane3, mode->hdisplay-300, mode->vdisplay-300);
> +	igt_display_commit2(display, COMMIT_UNIVERSAL);
> +
> +	if (primary_plane_scaling) {
> +		/* Switch scaler from plane 1 to plane 2 */
> +		igt_fb_set_position(&d->fb1, d->plane1, 0, 0);
> +		igt_fb_set_size(&d->fb1, d->plane1, d->fb1.width, d->fb1.height);
> +		igt_plane_set_position(d->plane1, 0, 0);
> +		igt_plane_set_size(d->plane1, mode->hdisplay, mode->vdisplay);
>  
> -		/* Switch scaler from plane 2 to plane 3 */
>  		igt_fb_set_position(&d->fb2, d->plane2, 100, 100);
> -		igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width-200, d->fb2.height-200);
> +		igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width-500,d->fb2.height-500);
>  		igt_plane_set_position(d->plane2, 100, 100);
> -		igt_plane_set_size(d->plane2, d->fb2.width-200, d->fb2.height-200);
> -
> -		igt_fb_set_position(&d->fb3, d->plane3, 100, 100);
> -		igt_fb_set_size(&d->fb3, d->plane3, d->fb3.width-400, d->fb3.height-400);
> -		igt_plane_set_position(d->plane3, 10, 10);
> -		igt_plane_set_size(d->plane3, mode->hdisplay-300, mode->vdisplay-300);
> +		igt_plane_set_size(d->plane2, mode->hdisplay-200, mode->vdisplay-200);
>  		igt_display_commit2(display, COMMIT_UNIVERSAL);
> +	}
>  
> -		if (primary_plane_scaling) {
> -			/* Switch scaler from plane 1 to plane 2 */
> -			igt_fb_set_position(&d->fb1, d->plane1, 0, 0);
> -			igt_fb_set_size(&d->fb1, d->plane1, d->fb1.width, d->fb1.height);
> -			igt_plane_set_position(d->plane1, 0, 0);
> -			igt_plane_set_size(d->plane1, mode->hdisplay, mode->vdisplay);
> -
> -			igt_fb_set_position(&d->fb2, d->plane2, 100, 100);
> -			igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width-500,d->fb2.height-500);
> -			igt_plane_set_position(d->plane2, 100, 100);
> -			igt_plane_set_size(d->plane2, mode->hdisplay-200, mode->vdisplay-200);
> -			igt_display_commit2(display, COMMIT_UNIVERSAL);
> -		}
> +cleanup:
> +	/* back to single plane mode */
> +	igt_plane_set_fb(d->plane2, NULL);
> +	igt_plane_set_fb(d->plane3, NULL);
> +	igt_display_commit2(display, COMMIT_UNIVERSAL);
>  
> -		/* back to single plane mode */
> -		igt_plane_set_fb(d->plane2, NULL);
> -		igt_plane_set_fb(d->plane3, NULL);
> -		igt_display_commit2(display, COMMIT_UNIVERSAL);
> +	cleanup_crtc(d, output, d->plane1);
> +}
> +
> +static void test_plane_scaling(data_t *d, enum pipe pipe)
> +{
> +	igt_output_t *output;
> +	int valid_tests = 0;
> +
> +	igt_require(d->num_scalers);
>  
> +	for_each_valid_output_on_pipe(&d->display, pipe, output) {
> +		test_plane_scaling_on_pipe(d, pipe, output);
> +		igt_output_set_pipe(output, PIPE_ANY);
>  		valid_tests++;
> -		cleanup_crtc(d, output, d->plane1);
>  	}
> +
>  	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
>  }
>  
>  igt_simple_main
>  {
>  	data_t data = {};
> +	enum pipe pipe;
>  
>  	igt_skip_on_simulation();
>  
> @@ -312,7 +327,8 @@ igt_simple_main
>  
>  	data.num_scalers = intel_gen(data.devid) >= 9 ? 2 : 0;
>  
> -	test_plane_scaling(&data);
> +	for_each_pipe_static(pipe)
> +		test_plane_scaling(&data, pipe);
>  
>  	igt_display_fini(&data.display);
>  }


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

Re: [PATCH i-g-t 1/6] i-g-t: kms_plane_scaling: Fix basic scaling test

[Srinivas, Vidya]

> -----Original Message-----
> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> Sent: Thursday, January 4, 2018 8:27 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Vetter, Daniel <daniel.vetter@intel.com>
> Subject: Re: [Intel-gfx] [PATCH i-g-t 1/6] i-g-t: kms_plane_scaling: Fix basic
> scaling test
> 
> Op 13-12-17 om 10:50 schreef Vidya Srinivas:
> > From: Mahesh Kumar <mahesh1.kumar@intel.com>
> >
> > PIPEC doesnt have 3rd plane in GEN9. So, we skip the 3rd plane related
> > scaling test where 2nd OVERLAY plane is not available.
> >
> > Restricting downscaling to (9/10)x original size of the image to avoid
> > "Max pixel rate limitation" of the hardware.
> >
> > Later patches in this series will cover corner cases of scaling.
> >
> > Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> > Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> > ---
> >  tests/kms_plane_scaling.c | 234
> > +++++++++++++++++++++++++---------------------
> >  1 file changed, 125 insertions(+), 109 deletions(-)
> >
> > diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> > index 403df47..a827cb3 100644
> > --- a/tests/kms_plane_scaling.c
> > +++ b/tests/kms_plane_scaling.c
> > @@ -163,144 +163,159 @@ static void iterate_plane_scaling(data_t *d,
> drmModeModeInfo *mode)
> >  	}
> >  }
> 
> The scaler tests should really require display->is_atomic. Even if there are
> theoretically more limits on the amount of scalers, having a YUV mode or
> interlaced mode may take a scaler, causing this test to fail.
> 
> With try_commit_atomic, it would be possible to test if enough scalers are
> available.
> 
> On top of that, I would really like the test to be more readable, and having
> test_plane_scaling_on_pipe split up in smaller pieces would accomplish
> that.
> 
> I would do some split ups first before doing any fixes, it's hard to read
> what's changed through the whitespace changes, even if the code is correct.
> 
> Also this test needs to be split up into subtests, this is a good start for it.
> 
> Replace igt_simple_main with igt_main, and add subtests for each pipe and
> various tests being performed. See for example kms_color, this test is
> already trying to do too much in one go. :)
> 
> ~Maarten
> 

Thank you. Regarding igt subtests, we have done that in the subsequent patches (4th in this series).
https://patchwork.kernel.org/patch/10109581/
Regarding the split up (readability), we have kept it simple for the additional tests that we added
in the subsequent patches. In this first patch, we did not want to modify the existing code
or make huge changes. We just wanted to fix the existing test case so that it runs
without failure. Please do have a check on the other patches and provide your feedback.
It would be very helpful.

Regards
Vidya

> >
> > -static void test_plane_scaling(data_t *d)
> > +static void
> > +test_plane_scaling_on_pipe(data_t *d, enum pipe pipe, igt_output_t
> > +*output)
> >  {
> >  	igt_display_t *display = &d->display;
> > -	igt_output_t *output;
> > -	enum pipe pipe;
> > -	int valid_tests = 0;
> > +	drmModeModeInfo *mode;
> >  	int primary_plane_scaling = 0; /* For now */
> >
> > -	igt_require(d->num_scalers);
> > +	igt_output_set_pipe(output, pipe);
> > +	mode = igt_output_get_mode(output);
> > +
> > +	/* allocate fb2 with image size */
> > +	d->fb_id2 = igt_create_image_fb(d->drm_fd, 0, 0,
> > +			DRM_FORMAT_XRGB8888,
> > +			LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */
> > +			FILE_NAME, &d->fb2);
> > +	igt_assert(d->fb_id2);
> > +
> > +	d->fb_id3 = igt_create_pattern_fb(d->drm_fd,
> > +			mode->hdisplay, mode->vdisplay,
> > +			DRM_FORMAT_XRGB8888,
> > +			LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */
> > +			&d->fb3);
> > +	igt_assert(d->fb_id3);
> > +
> > +	/* Set up display with plane 1 */
> > +	d->plane1 = igt_output_get_plane(output, 0);
> > +	prepare_crtc(d, output, pipe, d->plane1, mode,
> COMMIT_UNIVERSAL);
> > +
> > +	if (primary_plane_scaling) {
> > +		/* Primary plane upscaling */
> > +		igt_fb_set_position(&d->fb1, d->plane1, 100, 100);
> > +		igt_fb_set_size(&d->fb1, d->plane1, 500, 500);
> > +		igt_plane_set_position(d->plane1, 0, 0);
> > +		igt_plane_set_size(d->plane1, mode->hdisplay, mode-
> >vdisplay);
> > +		igt_display_commit2(display, COMMIT_UNIVERSAL);
> >
> > -	for_each_pipe_with_valid_output(display, pipe, output) {
> > -		drmModeModeInfo *mode;
> > -
> > -		igt_output_set_pipe(output, pipe);
> > -
> > -		mode = igt_output_get_mode(output);
> > -
> > -		/* allocate fb2 with image size */
> > -		d->fb_id2 = igt_create_image_fb(d->drm_fd, 0, 0,
> > -						DRM_FORMAT_XRGB8888,
> > -
> 	LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */
> > -						FILE_NAME, &d->fb2);
> > -		igt_assert(d->fb_id2);
> > -
> > -		d->fb_id3 = igt_create_pattern_fb(d->drm_fd,
> > -						  mode->hdisplay, mode-
> >vdisplay,
> > -						  DRM_FORMAT_XRGB8888,
> > -
> LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */
> > -						  &d->fb3);
> > -		igt_assert(d->fb_id3);
> > -
> > -		/* Set up display with plane 1 */
> > -		d->plane1 = igt_output_get_plane(output, 0);
> > -		prepare_crtc(d, output, pipe, d->plane1, mode,
> COMMIT_UNIVERSAL);
> > -
> > -		if (primary_plane_scaling) {
> > -			/* Primary plane upscaling */
> > -			igt_fb_set_position(&d->fb1, d->plane1, 100, 100);
> > -			igt_fb_set_size(&d->fb1, d->plane1, 500, 500);
> > -			igt_plane_set_position(d->plane1, 0, 0);
> > -			igt_plane_set_size(d->plane1, mode->hdisplay,
> mode->vdisplay);
> > -			igt_display_commit2(display, COMMIT_UNIVERSAL);
> > +		/* Primary plane 1:1 no scaling */
> > +		igt_fb_set_position(&d->fb1, d->plane1, 0, 0);
> > +		igt_fb_set_size(&d->fb1, d->plane1, d->fb1.width, d-
> >fb1.height);
> > +		igt_plane_set_position(d->plane1, 0, 0);
> > +		igt_plane_set_size(d->plane1, mode->hdisplay, mode-
> >vdisplay);
> > +		igt_display_commit2(display, COMMIT_UNIVERSAL);
> > +	}
> >
> > -			/* Primary plane 1:1 no scaling */
> > -			igt_fb_set_position(&d->fb1, d->plane1, 0, 0);
> > -			igt_fb_set_size(&d->fb1, d->plane1, d->fb1.width, d-
> >fb1.height);
> > -			igt_plane_set_position(d->plane1, 0, 0);
> > -			igt_plane_set_size(d->plane1, mode->hdisplay,
> mode->vdisplay);
> > -			igt_display_commit2(display, COMMIT_UNIVERSAL);
> > -		}
> > +	/* Set up fb2->plane2 mapping. */
> > +	d->plane2 = igt_output_get_plane(output, 1);
> > +	igt_plane_set_fb(d->plane2, &d->fb2);
> >
> > -		/* Set up fb2->plane2 mapping. */
> > -		d->plane2 = igt_output_get_plane(output, 1);
> > -		igt_plane_set_fb(d->plane2, &d->fb2);
> > +	/* 2nd plane windowed */
> > +	igt_fb_set_position(&d->fb2, d->plane2, 100, 100);
> > +	igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width-200, d-
> >fb2.height-200);
> > +	igt_plane_set_position(d->plane2, 100, 100);
> > +	igt_plane_set_size(d->plane2, mode->hdisplay-200, mode->vdisplay-
> 200);
> > +	igt_display_commit2(display, COMMIT_UNIVERSAL);
> >
> > -		/* 2nd plane windowed */
> > -		igt_fb_set_position(&d->fb2, d->plane2, 100, 100);
> > -		igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width-200, d-
> >fb2.height-200);
> > -		igt_plane_set_position(d->plane2, 100, 100);
> > -		igt_plane_set_size(d->plane2, mode->hdisplay-200, mode-
> >vdisplay-200);
> > -		igt_display_commit2(display, COMMIT_UNIVERSAL);
> > +	iterate_plane_scaling(d, mode);
> >
> > -		iterate_plane_scaling(d, mode);
> > +	/* 2nd plane up scaling */
> > +	igt_fb_set_position(&d->fb2, d->plane2, 100, 100);
> > +	igt_fb_set_size(&d->fb2, d->plane2, 500, 500);
> > +	igt_plane_set_position(d->plane2, 10, 10);
> > +	igt_plane_set_size(d->plane2, mode->hdisplay-20, mode->vdisplay-
> 20);
> > +	igt_display_commit2(display, COMMIT_UNIVERSAL);
> >
> > -		/* 2nd plane up scaling */
> > -		igt_fb_set_position(&d->fb2, d->plane2, 100, 100);
> > -		igt_fb_set_size(&d->fb2, d->plane2, 500, 500);
> > -		igt_plane_set_position(d->plane2, 10, 10);
> > -		igt_plane_set_size(d->plane2, mode->hdisplay-20, mode-
> >vdisplay-20);
> > -		igt_display_commit2(display, COMMIT_UNIVERSAL);
> > +	/* 2nd plane downscaling */
> > +	igt_fb_set_position(&d->fb2, d->plane2, 0, 0);
> > +	igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width, d->fb2.height);
> > +	igt_plane_set_position(d->plane2, 10, 10);
> >
> > -		/* 2nd plane downscaling */
> > -		igt_fb_set_position(&d->fb2, d->plane2, 0, 0);
> > -		igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width, d-
> >fb2.height);
> > -		igt_plane_set_position(d->plane2, 10, 10);
> > -		igt_plane_set_size(d->plane2, 500, 500 * d->fb2.height/d-
> >fb2.width);
> > +	/* Downscale (10/9)x of original image */
> > +	igt_plane_set_size(d->plane2, (d->fb2.width * 10)/9, (d->fb2.height
> * 10)/9);
> > +	igt_display_commit2(display, COMMIT_UNIVERSAL);
> > +
> > +	if (primary_plane_scaling) {
> > +		/* Primary plane up scaling */
> > +		igt_fb_set_position(&d->fb1, d->plane1, 100, 100);
> > +		igt_fb_set_size(&d->fb1, d->plane1, 500, 500);
> > +		igt_plane_set_position(d->plane1, 0, 0);
> > +		igt_plane_set_size(d->plane1, mode->hdisplay, mode-
> >vdisplay);
> >  		igt_display_commit2(display, COMMIT_UNIVERSAL);
> > +	}
> >
> > -		if (primary_plane_scaling) {
> > -			/* Primary plane up scaling */
> > -			igt_fb_set_position(&d->fb1, d->plane1, 100, 100);
> > -			igt_fb_set_size(&d->fb1, d->plane1, 500, 500);
> > -			igt_plane_set_position(d->plane1, 0, 0);
> > -			igt_plane_set_size(d->plane1, mode->hdisplay,
> mode->vdisplay);
> > -			igt_display_commit2(display, COMMIT_UNIVERSAL);
> > -		}
> > +	/* Set up fb3->plane3 mapping. */
> > +	d->plane3 = igt_output_get_plane(output, 2);
> > +	igt_plane_set_fb(d->plane3, &d->fb3);
> >
> > -		/* Set up fb3->plane3 mapping. */
> > -		d->plane3 = igt_output_get_plane(output, 2);
> > -		igt_plane_set_fb(d->plane3, &d->fb3);
> > +	if(d->plane3->type == DRM_PLANE_TYPE_CURSOR) {
> > +		igt_info("Plane-3 doesnt exist on pipe %s\n",
> kmstest_pipe_name(pipe));
> > +		goto cleanup;
> > +	}
> >
> > -		/* 3rd plane windowed - no scaling */
> > -		igt_fb_set_position(&d->fb3, d->plane3, 100, 100);
> > -		igt_fb_set_size(&d->fb3, d->plane3, d->fb3.width-300, d-
> >fb3.height-300);
> > -		igt_plane_set_position(d->plane3, 100, 100);
> > -		igt_plane_set_size(d->plane3, mode->hdisplay-300, mode-
> >vdisplay-300);
> > -		igt_display_commit2(display, COMMIT_UNIVERSAL);
> > +	/* 3rd plane windowed - no scaling */
> > +	igt_fb_set_position(&d->fb3, d->plane3, 100, 100);
> > +	igt_fb_set_size(&d->fb3, d->plane3, d->fb3.width-300, d-
> >fb3.height-300);
> > +	igt_plane_set_position(d->plane3, 100, 100);
> > +	igt_plane_set_size(d->plane3, mode->hdisplay-300, mode->vdisplay-
> 300);
> > +	igt_display_commit2(display, COMMIT_UNIVERSAL);
> > +
> > +	/* Switch scaler from plane 2 to plane 3 */
> > +	igt_fb_set_position(&d->fb2, d->plane2, 100, 100);
> > +	igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width-200, d-
> >fb2.height-200);
> > +	igt_plane_set_position(d->plane2, 100, 100);
> > +	igt_plane_set_size(d->plane2, d->fb2.width-200, d->fb2.height-200);
> > +
> > +	igt_fb_set_position(&d->fb3, d->plane3, 100, 100);
> > +	igt_fb_set_size(&d->fb3, d->plane3, d->fb3.width-400, d-
> >fb3.height-400);
> > +	igt_plane_set_position(d->plane3, 10, 10);
> > +	igt_plane_set_size(d->plane3, mode->hdisplay-300, mode->vdisplay-
> 300);
> > +	igt_display_commit2(display, COMMIT_UNIVERSAL);
> > +
> > +	if (primary_plane_scaling) {
> > +		/* Switch scaler from plane 1 to plane 2 */
> > +		igt_fb_set_position(&d->fb1, d->plane1, 0, 0);
> > +		igt_fb_set_size(&d->fb1, d->plane1, d->fb1.width, d-
> >fb1.height);
> > +		igt_plane_set_position(d->plane1, 0, 0);
> > +		igt_plane_set_size(d->plane1, mode->hdisplay, mode-
> >vdisplay);
> >
> > -		/* Switch scaler from plane 2 to plane 3 */
> >  		igt_fb_set_position(&d->fb2, d->plane2, 100, 100);
> > -		igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width-200, d-
> >fb2.height-200);
> > +		igt_fb_set_size(&d->fb2, d->plane2,
> > +d->fb2.width-500,d->fb2.height-500);
> >  		igt_plane_set_position(d->plane2, 100, 100);
> > -		igt_plane_set_size(d->plane2, d->fb2.width-200, d-
> >fb2.height-200);
> > -
> > -		igt_fb_set_position(&d->fb3, d->plane3, 100, 100);
> > -		igt_fb_set_size(&d->fb3, d->plane3, d->fb3.width-400, d-
> >fb3.height-400);
> > -		igt_plane_set_position(d->plane3, 10, 10);
> > -		igt_plane_set_size(d->plane3, mode->hdisplay-300, mode-
> >vdisplay-300);
> > +		igt_plane_set_size(d->plane2, mode->hdisplay-200,
> > +mode->vdisplay-200);
> >  		igt_display_commit2(display, COMMIT_UNIVERSAL);
> > +	}
> >
> > -		if (primary_plane_scaling) {
> > -			/* Switch scaler from plane 1 to plane 2 */
> > -			igt_fb_set_position(&d->fb1, d->plane1, 0, 0);
> > -			igt_fb_set_size(&d->fb1, d->plane1, d->fb1.width, d-
> >fb1.height);
> > -			igt_plane_set_position(d->plane1, 0, 0);
> > -			igt_plane_set_size(d->plane1, mode->hdisplay,
> mode->vdisplay);
> > -
> > -			igt_fb_set_position(&d->fb2, d->plane2, 100, 100);
> > -			igt_fb_set_size(&d->fb2, d->plane2, d->fb2.width-
> 500,d->fb2.height-500);
> > -			igt_plane_set_position(d->plane2, 100, 100);
> > -			igt_plane_set_size(d->plane2, mode->hdisplay-200,
> mode->vdisplay-200);
> > -			igt_display_commit2(display, COMMIT_UNIVERSAL);
> > -		}
> > +cleanup:
> > +	/* back to single plane mode */
> > +	igt_plane_set_fb(d->plane2, NULL);
> > +	igt_plane_set_fb(d->plane3, NULL);
> > +	igt_display_commit2(display, COMMIT_UNIVERSAL);
> >
> > -		/* back to single plane mode */
> > -		igt_plane_set_fb(d->plane2, NULL);
> > -		igt_plane_set_fb(d->plane3, NULL);
> > -		igt_display_commit2(display, COMMIT_UNIVERSAL);
> > +	cleanup_crtc(d, output, d->plane1);
> > +}
> > +
> > +static void test_plane_scaling(data_t *d, enum pipe pipe) {
> > +	igt_output_t *output;
> > +	int valid_tests = 0;
> > +
> > +	igt_require(d->num_scalers);
> >
> > +	for_each_valid_output_on_pipe(&d->display, pipe, output) {
> > +		test_plane_scaling_on_pipe(d, pipe, output);
> > +		igt_output_set_pipe(output, PIPE_ANY);
> >  		valid_tests++;
> > -		cleanup_crtc(d, output, d->plane1);
> >  	}
> > +
> >  	igt_require_f(valid_tests, "no valid crtc/connector combinations
> > found\n");  }
> >
> >  igt_simple_main
> >  {
> >  	data_t data = {};
> > +	enum pipe pipe;
> >
> >  	igt_skip_on_simulation();
> >
> > @@ -312,7 +327,8 @@ igt_simple_main
> >
> >  	data.num_scalers = intel_gen(data.devid) >= 9 ? 2 : 0;
> >
> > -	test_plane_scaling(&data);
> > +	for_each_pipe_static(pipe)
> > +		test_plane_scaling(&data, pipe);
> >
> >  	igt_display_fini(&data.display);
> >  }
> 

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

[PATCH i-g-t 2/6] i-g-t: lib: Add plane pixel format support

[Vidya Srinivas]

From: Mahesh Kumar <mahesh1.kumar@intel.com>

This patch adds the following:
- Query supported pixel formats from kernel for a given
plane.
- Get number of supported pixel formats for a plane
- Check if format is supported by cairo
- Add support to get format name string

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 lib/igt_fb.c  | 16 ++++++++++++++++
 lib/igt_kms.c | 29 +++++++++++++++++++++++++++++
 lib/igt_kms.h |  7 +++++++
 3 files changed, 52 insertions(+)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index d4eaed7..21d666d 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -68,6 +68,22 @@ static struct format_desc_struct {
 	DF(XRGB2101010,	RGB30,		32, 30),
 	DF(ARGB8888,	ARGB32,		32, 32),
 };
+
+const char *igt_get_format_name(uint32_t format)
+{
+	switch(format) {
+		case DRM_FORMAT_RGB565:
+			return "RGB565";
+		case DRM_FORMAT_XRGB8888:
+			return "XRGB8888";
+		case DRM_FORMAT_XRGB2101010:
+			return "XRGB2101010";
+		case DRM_FORMAT_ARGB8888:
+			return "ARGB8888";
+		default:
+			return "Unknown";
+	}
+}
 #undef DF
 
 #define for_each_format(f)	\
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 223dbe4..cc17f5c 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -3639,3 +3639,32 @@ uint32_t kmstest_get_vbl_flag(uint32_t pipe_id)
 		return pipe_flag;
 	}
 }
+
+/**
+ * igt_is_cairo_supported_format:
+ * @pixel_format: pixel format to be checked in supported cairo list.
+ *
+ * Returns true if pixel format is present in the supported cairo pixel
+ * format list and returns false if not present/supported in cairo.
+ */
+bool igt_is_cairo_supported_format(uint32_t pixel_format)
+{
+	const uint32_t *igt_formats;
+	int num_igt_formats;
+	int i;
+
+	igt_get_all_cairo_formats(&igt_formats, &num_igt_formats);
+	for (i = 0; i < num_igt_formats; i++) {
+		if (pixel_format == igt_formats[i])
+			return true;
+	}
+	return false;
+}
+
+int igt_get_format_count_plane(igt_plane_t *plane)
+{
+	if (plane)
+		return plane->drm_plane->count_formats;
+
+	return -EINVAL;
+}
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 2a480bf..ced7d73 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -389,6 +389,10 @@ void igt_fb_set_size(struct igt_fb *fb, igt_plane_t *plane,
 void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
 void igt_wait_for_vblank_count(int drm_fd, enum pipe pipe, int count);
 
+bool igt_is_cairo_supported_format(uint32_t pixel_format);
+int igt_get_format_count_plane(igt_plane_t *plane);
+const char *igt_get_format_name(uint32_t format);
+
 static inline bool igt_output_is_connected(igt_output_t *output)
 {
 	/* Something went wrong during probe? */
@@ -486,6 +490,9 @@ static inline bool igt_output_is_connected(igt_output_t *output)
 	for (int j__ = 0; assert(igt_can_fail()), (plane) = &(display)->pipes[(pipe)].planes[j__], \
 		     j__ < (display)->pipes[(pipe)].n_planes; j__++)
 
+#define for_each_format_in_plane(plane, format)	\
+	for (int k__ = 0; (format) = plane->drm_plane->formats[k__], k__ < plane->drm_plane->count_formats; k__++)
+
 #define IGT_FIXED(i,f)	((i) << 16 | (f))
 
 /**
-- 
2.7.4

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

Re: [PATCH i-g-t 2/6] i-g-t: lib: Add plane pixel format support

[Maarten Lankhorst]

Op 13-12-17 om 10:50 schreef Vidya Srinivas:
> From: Mahesh Kumar <mahesh1.kumar@intel.com>
>
> This patch adds the following:
> - Query supported pixel formats from kernel for a given
> plane.
> - Get number of supported pixel formats for a plane
> - Check if format is supported by cairo
> - Add support to get format name string
>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
>  lib/igt_fb.c  | 16 ++++++++++++++++
>  lib/igt_kms.c | 29 +++++++++++++++++++++++++++++
>  lib/igt_kms.h |  7 +++++++
>  3 files changed, 52 insertions(+)
>
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index d4eaed7..21d666d 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -68,6 +68,22 @@ static struct format_desc_struct {
>  	DF(XRGB2101010,	RGB30,		32, 30),
>  	DF(ARGB8888,	ARGB32,		32, 32),
>  };
> +
> +const char *igt_get_format_name(uint32_t format)
> +{
> +	switch(format) {
> +		case DRM_FORMAT_RGB565:
> +			return "RGB565";
> +		case DRM_FORMAT_XRGB8888:
> +			return "XRGB8888";
> +		case DRM_FORMAT_XRGB2101010:
> +			return "XRGB2101010";
> +		case DRM_FORMAT_ARGB8888:
> +			return "ARGB8888";
> +		default:
> +			return "Unknown";
> +	}
> +}
>  #undef DF
Right above this function, we have a struct format_desc_struct, which has a drm format parameter, and a name parameter.
Right below this function, there's a for_each_format() defined which iterates over the formats defined in this array. :)

I think igt_get_format_name can be implemented with both, removing some duplication.
>  #define for_each_format(f)	\
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 223dbe4..cc17f5c 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -3639,3 +3639,32 @@ uint32_t kmstest_get_vbl_flag(uint32_t pipe_id)
>  		return pipe_flag;
>  	}
>  }
> +
> +/**
> + * igt_is_cairo_supported_format:
> + * @pixel_format: pixel format to be checked in supported cairo list.
> + *
> + * Returns true if pixel format is present in the supported cairo pixel
> + * format list and returns false if not present/supported in cairo.
> + */
> +bool igt_is_cairo_supported_format(uint32_t pixel_format)
> +{
> +	const uint32_t *igt_formats;
> +	int num_igt_formats;
> +	int i;
> +
> +	igt_get_all_cairo_formats(&igt_formats, &num_igt_formats);
> +	for (i = 0; i < num_igt_formats; i++) {
> +		if (pixel_format == igt_formats[i])
> +			return true;
> +	}
> +	return false;
> +}
I think this function belongs to igt_fb as igt_fb_is_cairo_supported_format.
> +int igt_get_format_count_plane(igt_plane_t *plane)
> +{
> +	if (plane)
> +		return plane->drm_plane->count_formats;
> +
> +	return -EINVAL;
> +}
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 2a480bf..ced7d73 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -389,6 +389,10 @@ void igt_fb_set_size(struct igt_fb *fb, igt_plane_t *plane,
>  void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>  void igt_wait_for_vblank_count(int drm_fd, enum pipe pipe, int count);
>  
> +bool igt_is_cairo_supported_format(uint32_t pixel_format);
> +int igt_get_format_count_plane(igt_plane_t *plane);
> +const char *igt_get_format_name(uint32_t format);
> +
>  static inline bool igt_output_is_connected(igt_output_t *output)
>  {
>  	/* Something went wrong during probe? */
> @@ -486,6 +490,9 @@ static inline bool igt_output_is_connected(igt_output_t *output)
>  	for (int j__ = 0; assert(igt_can_fail()), (plane) = &(display)->pipes[(pipe)].planes[j__], \
>  		     j__ < (display)->pipes[(pipe)].n_planes; j__++)
>  
> +#define for_each_format_in_plane(plane, format)	\
> +	for (int k__ = 0; (format) = plane->drm_plane->formats[k__], k__ < plane->drm_plane->count_formats; k__++)
> +
>  #define IGT_FIXED(i,f)	((i) << 16 | (f))
>  
>  /**


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

Re: [PATCH i-g-t 2/6] i-g-t: lib: Add plane pixel format support

[Maarten Lankhorst]

Op 09-01-18 om 13:10 schreef Maarten Lankhorst:
> Op 13-12-17 om 10:50 schreef Vidya Srinivas:
>> From: Mahesh Kumar <mahesh1.kumar@intel.com>
>>
>> This patch adds the following:
>> - Query supported pixel formats from kernel for a given
>> plane.
>> - Get number of supported pixel formats for a plane
>> - Check if format is supported by cairo
>> - Add support to get format name string
>>
>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
>> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>> ---
>>  lib/igt_fb.c  | 16 ++++++++++++++++
>>  lib/igt_kms.c | 29 +++++++++++++++++++++++++++++
>>  lib/igt_kms.h |  7 +++++++
>>  3 files changed, 52 insertions(+)
>>
>> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
>> index d4eaed7..21d666d 100644
>> --- a/lib/igt_fb.c
>> +++ b/lib/igt_fb.c
>> @@ -68,6 +68,22 @@ static struct format_desc_struct {
>>  	DF(XRGB2101010,	RGB30,		32, 30),
>>  	DF(ARGB8888,	ARGB32,		32, 32),
>>  };
>> +
>> +const char *igt_get_format_name(uint32_t format)
>> +{
>> +	switch(format) {
>> +		case DRM_FORMAT_RGB565:
>> +			return "RGB565";
>> +		case DRM_FORMAT_XRGB8888:
>> +			return "XRGB8888";
>> +		case DRM_FORMAT_XRGB2101010:
>> +			return "XRGB2101010";
>> +		case DRM_FORMAT_ARGB8888:
>> +			return "ARGB8888";
>> +		default:
>> +			return "Unknown";
>> +	}
>> +}
>>  #undef DF
> Right above this function, we have a struct format_desc_struct, which has a drm format parameter, and a name parameter.
> Right below this function, there's a for_each_format() defined which iterates over the formats defined in this array. :)
>
> I think igt_get_format_name can be implemented with both, removing some duplication.
Hmm, maybe get rid of igt_fb_is_cairo_supported_format and add a try_ version of igt_get_cairo_ctx/surface?

Other macros here look too specialized, nothing wrong with accessing plane->drm_plane directly for this information.
>>  #define for_each_format(f)	\
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 223dbe4..cc17f5c 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -3639,3 +3639,32 @@ uint32_t kmstest_get_vbl_flag(uint32_t pipe_id)
>>  		return pipe_flag;
>>  	}
>>  }
>> +
>> +/**
>> + * igt_is_cairo_supported_format:
>> + * @pixel_format: pixel format to be checked in supported cairo list.
>> + *
>> + * Returns true if pixel format is present in the supported cairo pixel
>> + * format list and returns false if not present/supported in cairo.
>> + */
>> +bool igt_is_cairo_supported_format(uint32_t pixel_format)
>> +{
>> +	const uint32_t *igt_formats;
>> +	int num_igt_formats;
>> +	int i;
>> +
>> +	igt_get_all_cairo_formats(&igt_formats, &num_igt_formats);
>> +	for (i = 0; i < num_igt_formats; i++) {
>> +		if (pixel_format == igt_formats[i])
>> +			return true;
>> +	}
>> +	return false;
>> +}
> I think this function belongs to igt_fb as igt_fb_is_cairo_supported_format.
>> +int igt_get_format_count_plane(igt_plane_t *plane)
>> +{
>> +	if (plane)
>> +		return plane->drm_plane->count_formats;
>> +
>> +	return -EINVAL;
>> +}
>> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>> index 2a480bf..ced7d73 100644
>> --- a/lib/igt_kms.h
>> +++ b/lib/igt_kms.h
>> @@ -389,6 +389,10 @@ void igt_fb_set_size(struct igt_fb *fb, igt_plane_t *plane,
>>  void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>>  void igt_wait_for_vblank_count(int drm_fd, enum pipe pipe, int count);
>>  
>> +bool igt_is_cairo_supported_format(uint32_t pixel_format);
>> +int igt_get_format_count_plane(igt_plane_t *plane);
>> +const char *igt_get_format_name(uint32_t format);
>> +
>>  static inline bool igt_output_is_connected(igt_output_t *output)
>>  {
>>  	/* Something went wrong during probe? */
>> @@ -486,6 +490,9 @@ static inline bool igt_output_is_connected(igt_output_t *output)
>>  	for (int j__ = 0; assert(igt_can_fail()), (plane) = &(display)->pipes[(pipe)].planes[j__], \
>>  		     j__ < (display)->pipes[(pipe)].n_planes; j__++)
>>  
>> +#define for_each_format_in_plane(plane, format)	\
>> +	for (int k__ = 0; (format) = plane->drm_plane->formats[k__], k__ < plane->drm_plane->count_formats; k__++)
>> +
>>  #define IGT_FIXED(i,f)	((i) << 16 | (f))
>>  
>>  /**
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

[PATCH i-g-t 3/6] i-g-t: lib/igt_kms: Run kms_plane for all supported pixel formats

[Vidya Srinivas]

From: Mahesh Kumar <mahesh1.kumar@intel.com>

This patch adds a subtest related to pixel format testing. The test
create framebuffer with all supported pixel formats for primary and
sprite planes which can be drawn using cairo and commits the same on display.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 tests/kms_plane.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 123 insertions(+)

diff --git a/tests/kms_plane.c b/tests/kms_plane.c
index 92bf67f..5f25177 100644
--- a/tests/kms_plane.c
+++ b/tests/kms_plane.c
@@ -368,6 +368,125 @@ test_plane_panning(data_t *data, enum pipe pipe, unsigned int flags)
 	igt_skip_on(connected_outs == 0);
 }
 
+static void test_format_primary(data_t *data,
+			enum pipe pipe, igt_output_t *output)
+{
+	igt_plane_t *primary;
+	struct igt_fb primary_fb;
+	drmModeModeInfo *mode;
+	cairo_t *cr;
+	int primary_id;
+	uint32_t format;
+
+	igt_info("Testing connector %s using pipe %s on primary plane\n",
+		 igt_output_name(output), kmstest_pipe_name(pipe));
+
+	igt_output_set_pipe(output, pipe);
+	mode = igt_output_get_mode(output);
+
+	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
+
+	for_each_format_in_plane(primary, format) {
+		if (igt_is_cairo_supported_format(format)) {
+			igt_info("Testing format %s on primary now\n",
+					 igt_get_format_name(format));
+			primary_id = igt_create_fb(data->drm_fd,
+					  mode->hdisplay, mode->vdisplay,
+					  format,
+					  LOCAL_DRM_FORMAT_MOD_NONE,
+					  &primary_fb);
+			igt_assert(primary_id);
+			cr = igt_get_cairo_ctx(data->drm_fd, &primary_fb);
+			igt_paint_color(cr, 0, 0, mode->hdisplay, mode->vdisplay,
+						    0.0, 1.0, 0.0);
+			igt_paint_color(cr, 100, 100, 64, 64, 0.0, 0.0, 0.0);
+			igt_assert(cairo_status(cr) == 0);
+			cairo_destroy(cr);
+
+			igt_plane_set_fb(primary, &primary_fb);
+			igt_display_commit(&data->display);
+		}
+	}
+
+	igt_remove_fb(data->drm_fd, &primary_fb);
+	igt_plane_set_fb(primary, NULL);
+}
+
+static void test_format_overlay(data_t *data,
+			enum pipe pipe, igt_output_t *output,
+			int plane_id)
+{
+	igt_plane_t *primary, *overlay;
+	struct igt_fb primary_fb, overlay_fb;
+	drmModeModeInfo *mode;
+	cairo_t *cr;
+	int primary_id, overlay_id;
+	uint32_t format;
+
+	igt_info("Testing connector %s using pipe %s on overlay plane %d\n",
+		 igt_output_name(output), kmstest_pipe_name(pipe), plane_id);
+
+	igt_output_set_pipe(output, pipe);
+	mode = igt_output_get_mode(output);
+
+	overlay = igt_output_get_plane(output, plane_id);
+	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
+	primary_id = igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
+							   DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
+							   &primary_fb);
+	igt_assert(primary_id);
+	igt_plane_set_fb(primary, &primary_fb);
+	cr = igt_get_cairo_ctx(data->drm_fd, &primary_fb);
+	igt_paint_color(cr, 0, 0, mode->hdisplay, mode->vdisplay, 0.0, 0.0, 1.0);
+	igt_assert(cairo_status(cr) == 0);
+	cairo_destroy(cr);
+
+	for_each_format_in_plane(overlay, format) {
+		if (igt_is_cairo_supported_format(format)) {
+			igt_info("Testing format %s on plane id %d\n",
+					  igt_get_format_name(format), plane_id);
+			overlay_id = igt_create_fb(data->drm_fd, 300, 300, format,
+									   LOCAL_DRM_FORMAT_MOD_NONE, &overlay_fb);
+			igt_assert(overlay_id);
+
+			igt_plane_set_fb(overlay, &overlay_fb);
+			cr = igt_get_cairo_ctx(data->drm_fd, &overlay_fb);
+			igt_paint_color(cr, 0, 0, 300, 300, 1.0, 0.0, 0.0);
+			igt_assert(cairo_status(cr) == 0);
+			cairo_destroy(cr);
+			igt_display_commit(&data->display);
+
+			igt_remove_fb(data->drm_fd, &overlay_fb);
+			igt_plane_set_fb(overlay, NULL);
+			overlay_id = 0;
+		}
+	}
+	igt_remove_fb(data->drm_fd, &primary_fb);
+	igt_plane_set_fb(primary, NULL);
+}
+
+static void
+test_pixel_formats(data_t *data, enum pipe pipe)
+{
+	igt_output_t *output;
+	int connected_outs = 0;
+
+	for_each_valid_output_on_pipe(&data->display, pipe, output) {
+		int n_planes = data->display.pipes[pipe].n_planes;
+
+		igt_info("Testing PRIMARY on pipe %d\n", pipe);
+		test_format_primary(data, pipe, output);
+
+		/* Run only for SPRITES */
+		for (int plane_id = 1; plane_id < n_planes-1; plane_id++)
+			test_format_overlay(data, pipe, output, plane_id);
+
+		connected_outs++;
+		igt_output_set_pipe(output, PIPE_ANY);
+	}
+	igt_skip_on(connected_outs == 0);
+}
+
 static void
 run_tests_for_pipe_plane(data_t *data, enum pipe pipe)
 {
@@ -376,6 +495,10 @@ run_tests_for_pipe_plane(data_t *data, enum pipe pipe)
 		igt_require(data->display.pipes[pipe].n_planes > 0);
 	}
 
+	igt_subtest_f("pixel-format-pipe-%s-planes",
+		      kmstest_pipe_name(pipe))
+		test_pixel_formats(data, pipe);
+
 	igt_subtest_f("plane-position-covered-pipe-%s-planes",
 		      kmstest_pipe_name(pipe))
 		test_plane_position(data, pipe, TEST_POSITION_FULLY_COVERED);
-- 
2.7.4

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

Re: [PATCH i-g-t 3/6] i-g-t: lib/igt_kms: Run kms_plane for all supported pixel formats

[Maarten Lankhorst]

Op 13-12-17 om 10:50 schreef Vidya Srinivas:
> From: Mahesh Kumar <mahesh1.kumar@intel.com>
>
> This patch adds a subtest related to pixel format testing. The test
> create framebuffer with all supported pixel formats for primary and
> sprite planes which can be drawn using cairo and commits the same on display.
>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
>  tests/kms_plane.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 123 insertions(+)
>
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> index 92bf67f..5f25177 100644
> --- a/tests/kms_plane.c
> +++ b/tests/kms_plane.c
> @@ -368,6 +368,125 @@ test_plane_panning(data_t *data, enum pipe pipe, unsigned int flags)
>  	igt_skip_on(connected_outs == 0);
>  }
>  
> +static void test_format_primary(data_t *data,
> +			enum pipe pipe, igt_output_t *output)
> +{
> +	igt_plane_t *primary;
> +	struct igt_fb primary_fb;
> +	drmModeModeInfo *mode;
> +	cairo_t *cr;
> +	int primary_id;
> +	uint32_t format;
> +
> +	igt_info("Testing connector %s using pipe %s on primary plane\n",
> +		 igt_output_name(output), kmstest_pipe_name(pipe));
> +
> +	igt_output_set_pipe(output, pipe);
> +	mode = igt_output_get_mode(output);
> +
> +	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> +
> +	for_each_format_in_plane(primary, format) {
> +		if (igt_is_cairo_supported_format(format)) {
> +			igt_info("Testing format %s on primary now\n",
> +					 igt_get_format_name(format));
> +			primary_id = igt_create_fb(data->drm_fd,
> +					  mode->hdisplay, mode->vdisplay,
> +					  format,
> +					  LOCAL_DRM_FORMAT_MOD_NONE,
> +					  &primary_fb);
> +			igt_assert(primary_id);
> +			cr = igt_get_cairo_ctx(data->drm_fd, &primary_fb);
> +			igt_paint_color(cr, 0, 0, mode->hdisplay, mode->vdisplay,
> +						    0.0, 1.0, 0.0);
> +			igt_paint_color(cr, 100, 100, 64, 64, 0.0, 0.0, 0.0);
> +			igt_assert(cairo_status(cr) == 0);
> +			cairo_destroy(cr);
> +
> +			igt_plane_set_fb(primary, &primary_fb);
> +			igt_display_commit(&data->display);
> +		}
> +	}
> +
> +	igt_remove_fb(data->drm_fd, &primary_fb);
> +	igt_plane_set_fb(primary, NULL);
> +}
> +
> +static void test_format_overlay(data_t *data,
> +			enum pipe pipe, igt_output_t *output,
> +			int plane_id)
> +{
> +	igt_plane_t *primary, *overlay;
> +	struct igt_fb primary_fb, overlay_fb;
> +	drmModeModeInfo *mode;
> +	cairo_t *cr;
> +	int primary_id, overlay_id;
> +	uint32_t format;
> +
> +	igt_info("Testing connector %s using pipe %s on overlay plane %d\n",
> +		 igt_output_name(output), kmstest_pipe_name(pipe), plane_id);
> +
> +	igt_output_set_pipe(output, pipe);
> +	mode = igt_output_get_mode(output);
> +
> +	overlay = igt_output_get_plane(output, plane_id);
> +	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> +	primary_id = igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
> +							   DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> +							   &primary_fb);
> +	igt_assert(primary_id);
> +	igt_plane_set_fb(primary, &primary_fb);
> +	cr = igt_get_cairo_ctx(data->drm_fd, &primary_fb);
> +	igt_paint_color(cr, 0, 0, mode->hdisplay, mode->vdisplay, 0.0, 0.0, 1.0);
> +	igt_assert(cairo_status(cr) == 0);
> +	cairo_destroy(cr);
> +
> +	for_each_format_in_plane(overlay, format) {
> +		if (igt_is_cairo_supported_format(format)) {
> +			igt_info("Testing format %s on plane id %d\n",
> +					  igt_get_format_name(format), plane_id);
> +			overlay_id = igt_create_fb(data->drm_fd, 300, 300, format,
> +									   LOCAL_DRM_FORMAT_MOD_NONE, &overlay_fb);
> +			igt_assert(overlay_id);
> +
> +			igt_plane_set_fb(overlay, &overlay_fb);
> +			cr = igt_get_cairo_ctx(data->drm_fd, &overlay_fb);
> +			igt_paint_color(cr, 0, 0, 300, 300, 1.0, 0.0, 0.0);
> +			igt_assert(cairo_status(cr) == 0);
> +			cairo_destroy(cr);
> +			igt_display_commit(&data->display);
Why does this test distinguish between primary and sprite? It should test all planes in the same way, including cursor.

In fact, we should also test formats not supported by cairo, and only skip the painting in that case.

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

[PATCH i-g-t 4/6] i-g-t kms_plane_scaling: test scaling with tiling rotation and pixel formats

[Vidya Srinivas]

From: Jyoti Yadav <jyoti.r.yadav@intel.com>

This patch adds subtest for testing scaling in combination with rotation
and pixel formats.

Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 tests/kms_plane_scaling.c | 203 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 194 insertions(+), 9 deletions(-)

diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
index a827cb3..3725d8e 100644
--- a/tests/kms_plane_scaling.c
+++ b/tests/kms_plane_scaling.c
@@ -54,6 +54,52 @@ typedef struct {
 } data_t;
 
 #define FILE_NAME   "1080p-left.png"
+#define MIN_SRC_WIDTH_HEIGHT	9
+#define MAX_SRC_WIDTH	4096
+
+#define MAX_ROTATION	4
+static igt_rotation_t get_rotation_angle(int rot)
+{
+	switch (rot) {
+	case 0:
+		return IGT_ROTATION_0;
+		break;
+	case 1:
+		return IGT_ROTATION_90;
+		break;
+	case 2:
+		return IGT_ROTATION_180;
+		break;
+	case 3:
+		return IGT_ROTATION_270;
+		break;
+	default:
+		igt_info("Unknown/Unsupported Rotation %d\n", rot);
+		return IGT_ROTATION_0;
+	}
+}
+
+#define MAX_TILING	4
+static uint64_t get_tiling(int tiling)
+{
+	switch (tiling) {
+	case 0:
+		return LOCAL_DRM_FORMAT_MOD_NONE;
+		break;
+	case 1:
+		return LOCAL_I915_FORMAT_MOD_X_TILED;
+		break;
+	case 2:
+		return LOCAL_I915_FORMAT_MOD_Y_TILED;
+		break;
+	case 3:
+		return LOCAL_I915_FORMAT_MOD_Yf_TILED;
+		break;
+	default:
+		igt_info("Unknown/Unsupported Tiling %d\n", tiling);
+		return LOCAL_DRM_FORMAT_MOD_NONE;
+	}
+}
 
 static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 			igt_plane_t *plane, drmModeModeInfo *mode, enum igt_commit_style s)
@@ -129,6 +175,128 @@ static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane)
 	igt_display_commit2(display, COMMIT_UNIVERSAL);
 }
 
+static void paint_fb(data_t *d, struct igt_fb *fb)
+{
+	cairo_t *cr;
+
+	cr = igt_get_cairo_ctx(d->drm_fd, fb);
+	igt_paint_color(cr, 0, 0, fb->width, fb->height, 0.0, 1.0, 0.0);
+	igt_assert(cairo_status(cr) == 0);
+	cairo_destroy(cr);
+}
+
+static void check_scaling_pipe_plane_rot(data_t *d, igt_plane_t *plane, uint32_t pixel_format,
+			uint64_t tiling, enum pipe pipe, igt_output_t *output,
+			igt_rotation_t rot)
+{
+	igt_display_t *display = &d->display;
+	int width, height;
+	drmModeModeInfo *mode;
+	igt_output_set_pipe(output, pipe);
+	mode = igt_output_get_mode(output);
+
+	/* create buffer in the range of  min and max source side limit.*/
+	width = height = MIN_SRC_WIDTH_HEIGHT;
+	d->fb_id1 = igt_create_fb(display->drm_fd, width, height,
+									  pixel_format, tiling, &d->fb1);
+	paint_fb(d, &d->fb1);
+	igt_assert(d->fb_id1);
+	igt_plane_set_fb(plane, &d->fb1);
+
+	/* Check min to full resolution upscaling */
+	igt_fb_set_position(&d->fb1, plane, 0, 0);
+	igt_fb_set_size(&d->fb1, plane, width, height);
+	igt_plane_set_position(plane, 0, 0);
+	igt_plane_set_size(plane, mode->hdisplay, mode->vdisplay);
+	igt_plane_set_rotation(plane, rot);
+	igt_display_commit2(display, COMMIT_ATOMIC);
+
+	igt_plane_set_fb(plane, NULL);
+	igt_plane_set_position(plane, 0, 0);
+	if (d->fb_id1) {
+		igt_remove_fb(d->drm_fd, &d->fb1);
+		d->fb_id1 = 0;
+	}
+}
+
+static void test_scaler_with_rotation_pipe(data_t *d, igt_output_t *output,
+									enum pipe pipe)
+{
+	igt_display_t *display = &d->display;
+	igt_plane_t *plane;
+
+	igt_output_set_pipe(output, pipe);
+	for_each_plane_on_pipe(display, pipe, plane) {
+
+		if (plane->type == DRM_PLANE_TYPE_CURSOR)
+			continue;
+
+		for (int i = 0; i < MAX_ROTATION; i++) {
+			igt_rotation_t rot = get_rotation_angle(i);
+			check_scaling_pipe_plane_rot(d, plane, DRM_FORMAT_XRGB8888,
+										 LOCAL_I915_FORMAT_MOD_Y_TILED,
+										 pipe, output, rot);
+		}
+	}
+}
+
+static void test_scaler_with_rotation(data_t *d, enum pipe pipe)
+{
+	igt_output_t *output;
+
+	for_each_valid_output_on_pipe(&d->display, pipe, output) {
+		igt_output_set_pipe(output, pipe);
+		test_scaler_with_rotation_pipe(d, output, pipe);
+		igt_output_set_pipe(output, PIPE_ANY);
+	}
+}
+
+static void
+test_scaler_with_pixel_format_pipe_plane(data_t *d, igt_output_t *output,
+										 enum pipe pipe, igt_plane_t *plane,
+										 uint64_t tiling)
+{
+	uint32_t format;
+
+	for_each_format_in_plane(plane, format) {
+		if (igt_is_cairo_supported_format(format)) {
+			check_scaling_pipe_plane_rot(d, plane, format, tiling,
+					pipe, output, IGT_ROTATION_0);
+		}
+	}
+}
+
+static void test_scaler_with_pixel_format_pipe(data_t *d, igt_output_t *output,
+										enum pipe pipe)
+{
+	igt_display_t *display = &d->display;
+	igt_plane_t *plane;
+
+	igt_output_set_pipe(output, pipe);
+	for_each_plane_on_pipe(display, pipe, plane) {
+
+		if (plane->type == DRM_PLANE_TYPE_CURSOR)
+			continue;
+
+		for (int i = 0; i < MAX_TILING; i++) {
+			uint64_t tiling = get_tiling(i);
+			test_scaler_with_pixel_format_pipe_plane(d, output, pipe, plane,
+													 tiling);
+		}
+	}
+}
+
+static void test_scaler_with_pixel_format(data_t *d, enum pipe pipe)
+{
+	igt_output_t *output;
+
+	for_each_valid_output_on_pipe(&d->display, pipe, output) {
+		igt_output_set_pipe(output, pipe);
+		test_scaler_with_pixel_format_pipe(d, output, pipe);
+		igt_output_set_pipe(output, PIPE_ANY);
+	}
+}
+
 /* does iterative scaling on plane2 */
 static void iterate_plane_scaling(data_t *d, drmModeModeInfo *mode)
 {
@@ -312,23 +480,40 @@ static void test_plane_scaling(data_t *d, enum pipe pipe)
 	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
 }
 
-igt_simple_main
+igt_main
 {
 	data_t data = {};
 	enum pipe pipe;
 
 	igt_skip_on_simulation();
 
-
-	data.drm_fd = drm_open_driver(DRIVER_INTEL);
-	igt_require_pipe_crc(data.drm_fd);
-	igt_display_init(&data.display, data.drm_fd);
-	data.devid = intel_get_drm_devid(data.drm_fd);
+	igt_fixture {
+		data.drm_fd = drm_open_driver(DRIVER_INTEL);
+		kmstest_set_vt_graphics_mode();
+		igt_require_pipe_crc(data.drm_fd);
+		igt_display_init(&data.display, data.drm_fd);
+		data.devid = intel_get_drm_devid(data.drm_fd);
+		igt_require_gem(data.drm_fd);
+	}
 
 	data.num_scalers = intel_gen(data.devid) >= 9 ? 2 : 0;
 
-	for_each_pipe_static(pipe)
-		test_plane_scaling(&data, pipe);
+	for_each_pipe_static(pipe) {
+
+		igt_subtest_f("scaler_basic_test") {
+			test_plane_scaling(&data, pipe);
+		}
+
+		igt_subtest_f("scaler_with_pixel_format") {
+			test_scaler_with_pixel_format(&data, pipe);
+		}
 
-	igt_display_fini(&data.display);
+		igt_subtest_f("scaler_with_rotation") {
+			test_scaler_with_rotation(&data, pipe);
+		}
+
+		igt_fixture {
+			igt_display_fini(&data.display);
+		}
+	}
 }
-- 
2.7.4

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

Re: [PATCH i-g-t 4/6] i-g-t kms_plane_scaling: test scaling with tiling rotation and pixel formats

[Daniel Vetter]

Just a quick comment at the bottom.

On Wed, Dec 13, 2017 at 03:20:50PM +0530, Vidya Srinivas wrote:
> @@ -312,23 +480,40 @@ static void test_plane_scaling(data_t *d, enum pipe pipe)
>  	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
>  }
>  
> -igt_simple_main
> +igt_main
>  {
>  	data_t data = {};
>  	enum pipe pipe;
>  
>  	igt_skip_on_simulation();
>  
> -
> -	data.drm_fd = drm_open_driver(DRIVER_INTEL);
> -	igt_require_pipe_crc(data.drm_fd);
> -	igt_display_init(&data.display, data.drm_fd);
> -	data.devid = intel_get_drm_devid(data.drm_fd);
> +	igt_fixture {
> +		data.drm_fd = drm_open_driver(DRIVER_INTEL);
> +		kmstest_set_vt_graphics_mode();
> +		igt_require_pipe_crc(data.drm_fd);
> +		igt_display_init(&data.display, data.drm_fd);
> +		data.devid = intel_get_drm_devid(data.drm_fd);
> +		igt_require_gem(data.drm_fd);
> +	}
>  
>  	data.num_scalers = intel_gen(data.devid) >= 9 ? 2 : 0;

Hm, would be nice if we could get rid of this hard-coded platform
knowledge and autodiscover. But atm the kernel doesn't expose a
"can_scale" property unfortunately. Maybe add a FIXME comment?

Real reason I'm commenting here: You probably need to put this into the
igt_fixture too, since otherwise you're test won't run correctly when just
enumerating tests.

>  
> -	for_each_pipe_static(pipe)
> -		test_plane_scaling(&data, pipe);
> +	for_each_pipe_static(pipe) {
> +
> +		igt_subtest_f("scaler_basic_test") {
> +			test_plane_scaling(&data, pipe);
> +		}
> +
> +		igt_subtest_f("scaler_with_pixel_format") {
> +			test_scaler_with_pixel_format(&data, pipe);
> +		}
>  
> -	igt_display_fini(&data.display);
> +		igt_subtest_f("scaler_with_rotation") {
> +			test_scaler_with_rotation(&data, pipe);
> +		}
> +
> +		igt_fixture {
> +			igt_display_fini(&data.display);
> +		}
> +	}

Just a quick drive-by: You probably want to convert to subtest-based
testing as the very first patch (without any functional tests). In your
current series you add more subtests while still using igt_simple_main,
which will blow up.

The goal of a split up patch series isn't just to make review easier, but
also testing: Every single step in your series is supposed to be a fully
functional codebase. When you're not much experienced with building up
such a patch series, it's good practice to double-check that before
sending. I tend to use git rebase -x $test-script to automate that if
possible. That way you can make sure that every single step in your patch
series builds (cleanly!) and results in a working testcases (which should
only change results if your patch aims to do that, not as some
side-effect).

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

Re: [PATCH i-g-t 4/6] i-g-t kms_plane_scaling: test scaling with tiling rotation and pixel formats

[Srinivas, Vidya]

> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Thursday, December 14, 2017 4:25 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Vetter, Daniel <daniel.vetter@intel.com>
> Subject: Re: [Intel-gfx] [PATCH i-g-t 4/6] i-g-t kms_plane_scaling: test
> scaling with tiling rotation and pixel formats
> 
> Just a quick comment at the bottom.
> 
> On Wed, Dec 13, 2017 at 03:20:50PM +0530, Vidya Srinivas wrote:
> > @@ -312,23 +480,40 @@ static void test_plane_scaling(data_t *d, enum
> pipe pipe)
> >  	igt_require_f(valid_tests, "no valid crtc/connector combinations
> > found\n");  }
> >
> > -igt_simple_main
> > +igt_main
> >  {
> >  	data_t data = {};
> >  	enum pipe pipe;
> >
> >  	igt_skip_on_simulation();
> >
> > -
> > -	data.drm_fd = drm_open_driver(DRIVER_INTEL);
> > -	igt_require_pipe_crc(data.drm_fd);
> > -	igt_display_init(&data.display, data.drm_fd);
> > -	data.devid = intel_get_drm_devid(data.drm_fd);
> > +	igt_fixture {
> > +		data.drm_fd = drm_open_driver(DRIVER_INTEL);
> > +		kmstest_set_vt_graphics_mode();
> > +		igt_require_pipe_crc(data.drm_fd);
> > +		igt_display_init(&data.display, data.drm_fd);
> > +		data.devid = intel_get_drm_devid(data.drm_fd);
> > +		igt_require_gem(data.drm_fd);
> > +	}
> >
> >  	data.num_scalers = intel_gen(data.devid) >= 9 ? 2 : 0;
> 
> Hm, would be nice if we could get rid of this hard-coded platform
> knowledge and autodiscover. But atm the kernel doesn't expose a
> "can_scale" property unfortunately. Maybe add a FIXME comment?
> 
> Real reason I'm commenting here: You probably need to put this into the
> igt_fixture too, since otherwise you're test won't run correctly when just
> enumerating tests.
> 
> >
> > -	for_each_pipe_static(pipe)
> > -		test_plane_scaling(&data, pipe);
> > +	for_each_pipe_static(pipe) {
> > +
> > +		igt_subtest_f("scaler_basic_test") {
> > +			test_plane_scaling(&data, pipe);
> > +		}
> > +
> > +		igt_subtest_f("scaler_with_pixel_format") {
> > +			test_scaler_with_pixel_format(&data, pipe);
> > +		}
> >
> > -	igt_display_fini(&data.display);
> > +		igt_subtest_f("scaler_with_rotation") {
> > +			test_scaler_with_rotation(&data, pipe);
> > +		}
> > +
> > +		igt_fixture {
> > +			igt_display_fini(&data.display);
> > +		}
> > +	}
> 
> Just a quick drive-by: You probably want to convert to subtest-based testing
> as the very first patch (without any functional tests). In your current series
> you add more subtests while still using igt_simple_main, which will blow
> up.
> 
> The goal of a split up patch series isn't just to make review easier, but also
> testing: Every single step in your series is supposed to be a fully functional
> codebase. When you're not much experienced with building up such a patch
> series, it's good practice to double-check that before sending. I tend to use
> git rebase -x $test-script to automate that if possible. That way you can
> make sure that every single step in your patch series builds (cleanly!) and
> results in a working testcases (which should only change results if your patch
> aims to do that, not as some side-effect).

Thank you. Sorry if there is any mistake here. But when we have added subtests we have changed it to igt_main and not simple.
Before that we have not added subtests. We followed whatever you suggested - as in, first patch would correct just whatever is
Not working as of now. Then in the later tests we added new subtests with enhancements. We ran all the tests once (I am sorry,
Not the way you mentioned) fully and it ran on our APL board.

Regards
Vidya

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

Re: [PATCH i-g-t 4/6] i-g-t kms_plane_scaling: test scaling with tiling rotation and pixel formats

[Maarten Lankhorst]

Op 14-12-17 om 11:55 schreef Daniel Vetter:
> Just a quick comment at the bottom.
>
> On Wed, Dec 13, 2017 at 03:20:50PM +0530, Vidya Srinivas wrote:
>> @@ -312,23 +480,40 @@ static void test_plane_scaling(data_t *d, enum pipe pipe)
>>  	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
>>  }
>>  
>> -igt_simple_main
>> +igt_main
>>  {
>>  	data_t data = {};
>>  	enum pipe pipe;
>>  
>>  	igt_skip_on_simulation();
>>  
>> -
>> -	data.drm_fd = drm_open_driver(DRIVER_INTEL);
>> -	igt_require_pipe_crc(data.drm_fd);
>> -	igt_display_init(&data.display, data.drm_fd);
>> -	data.devid = intel_get_drm_devid(data.drm_fd);
>> +	igt_fixture {
>> +		data.drm_fd = drm_open_driver(DRIVER_INTEL);
>> +		kmstest_set_vt_graphics_mode();
>> +		igt_require_pipe_crc(data.drm_fd);
>> +		igt_display_init(&data.display, data.drm_fd);
>> +		data.devid = intel_get_drm_devid(data.drm_fd);
>> +		igt_require_gem(data.drm_fd);
>> +	}
>>  
>>  	data.num_scalers = intel_gen(data.devid) >= 9 ? 2 : 0;
> Hm, would be nice if we could get rid of this hard-coded platform
> knowledge and autodiscover. But atm the kernel doesn't expose a
> "can_scale" property unfortunately. Maybe add a FIXME comment?
Isn't that what igt_display_try_commit_atomic + TEST_ONLY is for?

num scalers might be less if a scaler is used by the crtc as well, so not something you can skip..
> Real reason I'm commenting here: You probably need to put this into the
> igt_fixture too, since otherwise you're test won't run correctly when just
> enumerating tests.
>
>>  
>> -	for_each_pipe_static(pipe)
>> -		test_plane_scaling(&data, pipe);
>> +	for_each_pipe_static(pipe) {
>> +
>> +		igt_subtest_f("scaler_basic_test") {
>> +			test_plane_scaling(&data, pipe);
>> +		}
>> +
>> +		igt_subtest_f("scaler_with_pixel_format") {
>> +			test_scaler_with_pixel_format(&data, pipe);
>> +		}
>>  
>> -	igt_display_fini(&data.display);
>> +		igt_subtest_f("scaler_with_rotation") {
>> +			test_scaler_with_rotation(&data, pipe);
>> +		}
>> +
>> +		igt_fixture {
>> +			igt_display_fini(&data.display);
>> +		}
>> +	}
> Just a quick drive-by: You probably want to convert to subtest-based
> testing as the very first patch (without any functional tests). In your
> current series you add more subtests while still using igt_simple_main,
> which will blow up.
>
> The goal of a split up patch series isn't just to make review easier, but
> also testing: Every single step in your series is supposed to be a fully
> functional codebase. When you're not much experienced with building up
> such a patch series, it's good practice to double-check that before
> sending. I tend to use git rebase -x $test-script to automate that if
> possible. That way you can make sure that every single step in your patch
> series builds (cleanly!) and results in a working testcases (which should
> only change results if your patch aims to do that, not as some
> side-effect).
>
> Cheers, Daniel


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

[PATCH i-g-t 5/6] i-g-t: kms_plane_scaling: test scaler with clipping clamping

[Vidya Srinivas]

From: Jyoti Yadav <jyoti.r.yadav@intel.com>

This patch adds subtest to test scaler clipping and clamping
scenario

Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 tests/kms_plane_scaling.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
index 3725d8e..b80cafb 100644
--- a/tests/kms_plane_scaling.c
+++ b/tests/kms_plane_scaling.c
@@ -480,6 +480,71 @@ static void test_plane_scaling(data_t *d, enum pipe pipe)
 	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
 }
 
+static void
+test_scaler_with_clipping_clamping_scenario(data_t *d, enum pipe pipe)
+{
+	igt_output_t *output;
+	igt_require(d->num_scalers);
+
+	for_each_valid_output_on_pipe(&d->display, pipe, output) {
+		drmModeModeInfo *mode;
+
+		/* Gen9 has single scaler in PIPEC */
+		if (intel_gen(d->devid) == 9 && pipe == PIPE_C)
+			continue;
+
+		igt_output_set_pipe(output, pipe);
+		mode = igt_output_get_mode(output);
+		d->fb_id2 = igt_create_pattern_fb(d->drm_fd,
+										  mode->hdisplay, mode->vdisplay,
+										  DRM_FORMAT_XRGB8888,
+										  LOCAL_I915_FORMAT_MOD_X_TILED,
+										  &d->fb2);
+		igt_assert(d->fb_id2);
+
+		d->fb_id3 = igt_create_pattern_fb(d->drm_fd,
+										  mode->hdisplay, mode->vdisplay,
+										  DRM_FORMAT_XRGB8888,
+										  LOCAL_I915_FORMAT_MOD_Y_TILED,
+										  &d->fb3);
+		igt_assert(d->fb_id3);
+
+		d->plane1 = igt_output_get_plane(output, 0);
+		igt_plane_set_fb(d->plane1, &d->fb2);
+		d->plane2 = igt_output_get_plane(output, 1);
+		igt_plane_set_fb(d->plane2, &d->fb3);
+
+		igt_fb_set_position(&d->fb2, d->plane1, 0, 0);
+		igt_fb_set_size(&d->fb2, d->plane1, 300, 300);
+		igt_plane_set_position(d->plane1, 100, 400);
+		igt_fb_set_position(&d->fb3, d->plane2, 0, 0);
+		igt_fb_set_size(&d->fb3, d->plane2, 400, 400);
+		igt_plane_set_position(d->plane2, 100, 100);
+
+		/* scaled window size is outside the modeset area.*/
+		igt_plane_set_size(d->plane1, mode->hdisplay + 200,
+						   mode->vdisplay + 200);
+		igt_plane_set_size(d->plane2, mode->hdisplay + 100,
+						   mode->vdisplay + 100);
+		igt_display_commit2(&d->display, COMMIT_ATOMIC);
+
+		/* disable above 2 planes */
+		igt_plane_set_fb(d->plane1, NULL);
+		igt_plane_set_position(d->plane1, 0, 0);
+		igt_plane_set_fb(d->plane2, NULL);
+		igt_plane_set_position(d->plane2, 0, 0);
+		if (d->fb_id2) {
+			igt_remove_fb(d->drm_fd, &d->fb2);
+			d->fb_id2 = 0;
+		}
+		if (d->fb_id3) {
+			igt_remove_fb(d->drm_fd, &d->fb3);
+			d->fb_id3 = 0;
+		}
+		igt_output_set_pipe(output, PIPE_ANY);
+	}
+}
+
 igt_main
 {
 	data_t data = {};
@@ -512,6 +577,10 @@ igt_main
 			test_scaler_with_rotation(&data, pipe);
 		}
 
+		igt_subtest_f("scaler_with_clipping_clamping") {
+			test_scaler_with_clipping_clamping_scenario(&data, pipe);
+		}
+
 		igt_fixture {
 			igt_display_fini(&data.display);
 		}
-- 
2.7.4

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

[PATCH i-g-t 6/6] i-g-t: kms_plane_scaling: test for multi pipe with scaling

[Vidya Srinivas]

From: Jyoti Yadav <jyoti.r.yadav@intel.com>

Patch adds subtest to display primary and overlay planes on two
connected pipes and runs scaling test on both pipes

Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 tests/kms_plane_scaling.c | 114 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 112 insertions(+), 2 deletions(-)

diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
index b80cafb..bbdf3f3 100644
--- a/tests/kms_plane_scaling.c
+++ b/tests/kms_plane_scaling.c
@@ -43,9 +43,11 @@ typedef struct {
 	struct igt_fb fb1;
 	struct igt_fb fb2;
 	struct igt_fb fb3;
+	struct igt_fb fb4;
 	int fb_id1;
 	int fb_id2;
 	int fb_id3;
+	int fb_id4;
 
 	igt_plane_t *plane1;
 	igt_plane_t *plane2;
@@ -101,6 +103,24 @@ static uint64_t get_tiling(int tiling)
 	}
 }
 
+static igt_output_t *get_next_valid_output(igt_display_t *data, int i)
+{
+	int j = 0, valid_output = 0;
+	drmModeModeInfo *mode;
+
+	for (j = 0; j < data->n_outputs; j++) {
+		if (igt_output_is_connected(&data->outputs[j])) {
+			mode = igt_output_get_mode(&data->outputs[j]);
+			if (mode->hdisplay != 0 && mode->vdisplay != 0) {
+				valid_output++;
+				if (valid_output == i)
+					return &data->outputs[j];
+			}
+		}
+	}
+	return NULL;
+}
+
 static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 			igt_plane_t *plane, drmModeModeInfo *mode, enum igt_commit_style s)
 {
@@ -545,6 +565,93 @@ test_scaler_with_clipping_clamping_scenario(data_t *d, enum pipe pipe)
 	}
 }
 
+static void test_scaler_with_multi_pipe_plane(data_t *d)
+{
+	igt_display_t *display = &d->display;
+	igt_output_t *output1, *output2;
+	drmModeModeInfo *mode1, *mode2;
+
+	output1 = get_next_valid_output(display, 1);
+	output2 = get_next_valid_output(display, 2);
+
+	igt_skip_on(!output1 || !output2);
+
+	igt_output_set_pipe(output1, PIPE_ANY);
+	igt_output_set_pipe(output2, PIPE_ANY);
+
+	igt_output_set_pipe(output1, 0);
+	igt_output_set_pipe(output2, 1);
+
+	d->plane1 = igt_output_get_plane(output1, 0);
+	d->plane2 = igt_output_get_plane(output1, 1);
+	d->plane3 = igt_output_get_plane(output2, 0);
+	d->plane4 = igt_output_get_plane(output2, 1);
+
+	mode1 = igt_output_get_mode(output1);
+	mode2 = igt_output_get_mode(output2);
+
+	d->fb_id1 = igt_create_pattern_fb(d->drm_fd, 600, 600,
+									  DRM_FORMAT_XRGB8888,
+									  LOCAL_I915_FORMAT_MOD_Y_TILED, &d->fb1);
+	igt_assert(d->fb_id1);
+
+	d->fb_id2 = igt_create_pattern_fb(d->drm_fd, 500, 500,
+									  DRM_FORMAT_XRGB8888,
+									  LOCAL_I915_FORMAT_MOD_Y_TILED, &d->fb2);
+	igt_assert(d->fb_id2);
+
+	d->fb_id3 = igt_create_pattern_fb(d->drm_fd, 700, 700,
+									  DRM_FORMAT_XRGB8888,
+									  LOCAL_I915_FORMAT_MOD_Y_TILED, &d->fb3);
+	igt_assert(d->fb_id3);
+
+	d->fb_id4 = igt_create_pattern_fb(d->drm_fd, 400, 400,
+									  DRM_FORMAT_XRGB8888,
+									  LOCAL_I915_FORMAT_MOD_Y_TILED, &d->fb4);
+	igt_assert(d->fb_id4);
+
+	igt_plane_set_fb(d->plane1, &d->fb1);
+	igt_plane_set_fb(d->plane2, &d->fb2);
+	igt_plane_set_fb(d->plane3, &d->fb3);
+	igt_plane_set_fb(d->plane4, &d->fb4);
+
+	/* Upscaling Primary */
+	igt_plane_set_size(d->plane1, mode1->hdisplay, mode1->vdisplay);
+	igt_plane_set_size(d->plane3, mode2->hdisplay, mode2->vdisplay);
+	igt_display_commit2(display, COMMIT_ATOMIC);
+
+	/* Upscaling Sprites */
+	igt_plane_set_size(d->plane2, mode1->hdisplay, mode1->vdisplay);
+	igt_plane_set_size(d->plane4, mode2->hdisplay, mode2->vdisplay);
+	igt_display_commit2(display, COMMIT_ATOMIC);
+
+	/* disable above 4 planes and cleanup */
+	igt_plane_set_fb(d->plane1, NULL);
+	igt_plane_set_position(d->plane1, 0, 0);
+	igt_plane_set_fb(d->plane2, NULL);
+	igt_plane_set_position(d->plane2, 0, 0);
+	igt_plane_set_fb(d->plane3, NULL);
+	igt_plane_set_position(d->plane3, 0, 0);
+	igt_plane_set_fb(d->plane4, NULL);
+	igt_plane_set_position(d->plane4, 0, 0);
+	if (d->fb_id1) {
+		igt_remove_fb(d->drm_fd, &d->fb1);
+		d->fb_id1 = 0;
+	}
+	if (d->fb_id2) {
+		igt_remove_fb(d->drm_fd, &d->fb2);
+		d->fb_id2 = 0;
+	}
+	if (d->fb_id3) {
+		igt_remove_fb(d->drm_fd, &d->fb3);
+		d->fb_id3 = 0;
+	}
+	if (d->fb_id4) {
+		igt_remove_fb(d->drm_fd, &d->fb4);
+		d->fb_id4 = 0;
+	}
+}
+
 igt_main
 {
 	data_t data = {};
@@ -580,9 +687,12 @@ igt_main
 		igt_subtest_f("scaler_with_clipping_clamping") {
 			test_scaler_with_clipping_clamping_scenario(&data, pipe);
 		}
+	}
 
-		igt_fixture {
+	igt_subtest_f("scaler_with_multi_pipe_plane") {
+		test_scaler_with_multi_pipe_plane(&data);
+	}
+	igt_fixture {
 			igt_display_fini(&data.display);
-		}
 	}
 }
-- 
2.7.4

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

Re: [PATCH i-g-t 6/6] i-g-t: kms_plane_scaling: test for multi pipe with scaling

[Maarten Lankhorst]

Op 13-12-17 om 10:50 schreef Vidya Srinivas:
> From: Jyoti Yadav <jyoti.r.yadav@intel.com>
>
> Patch adds subtest to display primary and overlay planes on two
> connected pipes and runs scaling test on both pipes
>
> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
>  tests/kms_plane_scaling.c | 114 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 112 insertions(+), 2 deletions(-)
>
> diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> index b80cafb..bbdf3f3 100644
> --- a/tests/kms_plane_scaling.c
> +++ b/tests/kms_plane_scaling.c
> @@ -43,9 +43,11 @@ typedef struct {
>  	struct igt_fb fb1;
>  	struct igt_fb fb2;
>  	struct igt_fb fb3;
> +	struct igt_fb fb4;
>  	int fb_id1;
>  	int fb_id2;
>  	int fb_id3;
> +	int fb_id4;
>  
>  	igt_plane_t *plane1;
>  	igt_plane_t *plane2;
> @@ -101,6 +103,24 @@ static uint64_t get_tiling(int tiling)
>  	}
>  }
>  
> +static igt_output_t *get_next_valid_output(igt_display_t *data, int i)
> +{
> +	int j = 0, valid_output = 0;
> +	drmModeModeInfo *mode;
> +
> +	for (j = 0; j < data->n_outputs; j++) {
> +		if (igt_output_is_connected(&data->outputs[j])) {
> +			mode = igt_output_get_mode(&data->outputs[j]);
> +			if (mode->hdisplay != 0 && mode->vdisplay != 0) {
> +				valid_output++;
> +				if (valid_output == i)
> +					return &data->outputs[j];
> +			}
> +		}
> +	}
> +	return NULL;
> +}
> +
>  static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
>  			igt_plane_t *plane, drmModeModeInfo *mode, enum igt_commit_style s)
>  {
> @@ -545,6 +565,93 @@ test_scaler_with_clipping_clamping_scenario(data_t *d, enum pipe pipe)
>  	}
>  }
>  
> +static void test_scaler_with_multi_pipe_plane(data_t *d)
> +{
> +	igt_display_t *display = &d->display;
> +	igt_output_t *output1, *output2;
> +	drmModeModeInfo *mode1, *mode2;
> +
> +	output1 = get_next_valid_output(display, 1);
> +	output2 = get_next_valid_output(display, 2);
> +
> +	igt_skip_on(!output1 || !output2);
> +
> +	igt_output_set_pipe(output1, PIPE_ANY);
> +	igt_output_set_pipe(output2, PIPE_ANY);
> +
> +	igt_output_set_pipe(output1, 0);
> +	igt_output_set_pipe(output2, 1);
You can't do this, hardcoding pipe 0 (A) and 1(B).. See

find_connected_pipe(igt_display_t *display, bool second)

in kms_cursor_legacy how to do it correctly.

Some outputs may have restraints to which pipe they're connected, and that is completely ignored here.


> +	d->plane1 = igt_output_get_plane(output1, 0);
> +	d->plane2 = igt_output_get_plane(output1, 1);
> +	d->plane3 = igt_output_get_plane(output2, 0);
> +	d->plane4 = igt_output_get_plane(output2, 1);
> +
> +	mode1 = igt_output_get_mode(output1);
> +	mode2 = igt_output_get_mode(output2);
> +
> +	d->fb_id1 = igt_create_pattern_fb(d->drm_fd, 600, 600,
> +									  DRM_FORMAT_XRGB8888,
> +									  LOCAL_I915_FORMAT_MOD_Y_TILED, &d->fb1);
> +	igt_assert(d->fb_id1);
> +
> +	d->fb_id2 = igt_create_pattern_fb(d->drm_fd, 500, 500,
> +									  DRM_FORMAT_XRGB8888,
> +									  LOCAL_I915_FORMAT_MOD_Y_TILED, &d->fb2);
> +	igt_assert(d->fb_id2);
> +
> +	d->fb_id3 = igt_create_pattern_fb(d->drm_fd, 700, 700,
> +									  DRM_FORMAT_XRGB8888,
> +									  LOCAL_I915_FORMAT_MOD_Y_TILED, &d->fb3);
> +	igt_assert(d->fb_id3);
> +
> +	d->fb_id4 = igt_create_pattern_fb(d->drm_fd, 400, 400,
> +									  DRM_FORMAT_XRGB8888,
> +									  LOCAL_I915_FORMAT_MOD_Y_TILED, &d->fb4);
> +	igt_assert(d->fb_id4);
This looks a bit double, how come we don't have a helper for this?
> +	igt_plane_set_fb(d->plane1, &d->fb1);
> +	igt_plane_set_fb(d->plane2, &d->fb2);
> +	igt_plane_set_fb(d->plane3, &d->fb3);
> +	igt_plane_set_fb(d->plane4, &d->fb4);
> +
> +	/* Upscaling Primary */
> +	igt_plane_set_size(d->plane1, mode1->hdisplay, mode1->vdisplay);
> +	igt_plane_set_size(d->plane3, mode2->hdisplay, mode2->vdisplay);
> +	igt_display_commit2(display, COMMIT_ATOMIC);
> +
> +	/* Upscaling Sprites */
> +	igt_plane_set_size(d->plane2, mode1->hdisplay, mode1->vdisplay);
> +	igt_plane_set_size(d->plane4, mode2->hdisplay, mode2->vdisplay);
> +	igt_display_commit2(display, COMMIT_ATOMIC);
What about downscale?

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