[igt-dev] [RESEND,v3 0/5] kms_plane: Add clipping subtests

[igt-dev] [RESEND,v3 0/5] kms_plane: Add clipping subtests

[Gwan-gyeong Mun]

This is v3 of [1] and [2], implementing an idea of Daniel to achieve
improving test speed. Besides the changes described in patch 5.

Note that the subtest takes ~10 seconds to run on a single pipe.
(a prior version took ~ 43 seconds.)

[1]
https://lists.freedesktop.org/archives/igt-dev/2018-February/000523.html

[2]
https://lists.freedesktop.org/archives/igt-dev/2018-March/001407.html

Cc: Mika Kahola <mika.kahola@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Juha-Pekka Heikkilä <juha-pekka.heikkila@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

Gwan-gyeong Mun (1):
  kms_plane: Add a helper of capturing CRC with commit style

Imre Deak (4):
  kms_plane: Remove redundant modeset after CRC capture
  lib: Export helpers to get rotation/tiling strings
  kms_plane: Split helpers creating reference FB and capturing CRC
  kms_plane: Add clipping subtests

 lib/igt_fb.c      |  23 ++
 lib/igt_fb.h      |   1 +
 lib/igt_kms.c     |  11 +-
 lib/igt_kms.h     |   1 +
 tests/kms_plane.c | 591 ++++++++++++++++++++++++++++++++++++++++++++--
 5 files changed, 605 insertions(+), 22 deletions(-)

-- 
2.18.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [RESEND, v3 1/5] kms_plane: Remove redundant modeset after CRC capture

[Gwan-gyeong Mun]

From: Imre Deak <imre.deak@intel.com>

The null modeset after capturing the CRC is redundant; detaching the FB
from the plane is enough for the next modeset to work properly. This
speed things up especially on slow panels.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 tests/kms_plane.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/kms_plane.c b/tests/kms_plane.c
index 3999dde8..aceae591 100644
--- a/tests/kms_plane.c
+++ b/tests/kms_plane.c
@@ -88,7 +88,6 @@ test_grab_crc(data_t *data, igt_output_t *output, enum pipe pipe,
 	igt_pipe_crc_collect_crc(data->pipe_crc, crc);
 
 	igt_plane_set_fb(primary, NULL);
-	igt_display_commit(&data->display);
 
 	igt_remove_fb(data->drm_fd, &fb);
 
-- 
2.18.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [RESEND, v3 2/5] lib: Export helpers to get rotation/tiling strings

[Gwan-gyeong Mun]

From: Imre Deak <imre.deak@intel.com>

This is needed for the next patch for some debug prints.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 lib/igt_fb.c  | 23 +++++++++++++++++++++++
 lib/igt_fb.h  |  1 +
 lib/igt_kms.c | 11 +++++++++--
 lib/igt_kms.h |  1 +
 4 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index ae71d967..7ac60dd3 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -2016,3 +2016,26 @@ bool igt_format_is_yuv(uint32_t drm_format)
 		return false;
 	}
 }
+
+/**
+ * igt_tiling_str:
+ * @tiling: tiling ID
+ *
+ * Returns:
+ * Human-readable tiling string for @tiling.
+ */
+const char *igt_tiling_str(uint64_t tiling)
+{
+	switch (tiling) {
+	case LOCAL_DRM_FORMAT_MOD_NONE:
+		return "linear";
+	case LOCAL_I915_FORMAT_MOD_X_TILED:
+		return "X-tiled";
+	case LOCAL_I915_FORMAT_MOD_Y_TILED:
+		return "Y-tiled";
+	case LOCAL_I915_FORMAT_MOD_Yf_TILED:
+		return "Yf-tiled";
+	default:
+		return "N/A";
+	}
+}
diff --git a/lib/igt_fb.h b/lib/igt_fb.h
index d28bc0c4..1091d13e 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -168,6 +168,7 @@ uint32_t igt_drm_format_to_bpp(uint32_t drm_format);
 const char *igt_format_str(uint32_t drm_format);
 bool igt_fb_supported_format(uint32_t drm_format);
 bool igt_format_is_yuv(uint32_t drm_format);
+const char *igt_tiling_str(uint64_t tiling);
 
 #endif /* __IGT_FB_H__ */
 
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 62d84684..8f5b647e 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -3794,7 +3794,14 @@ void igt_fb_set_size(struct igt_fb *fb, igt_plane_t *plane,
 	igt_plane_set_prop_value(plane, IGT_PLANE_SRC_H, IGT_FIXED(h, 0));
 }
 
-static const char *rotation_name(igt_rotation_t rotation)
+/**
+ * igt_rotation_degrees_str:
+ * @rotation: rotation degrees/reflect mask
+ *
+ * Returns:
+ * Human-readable string for the rotation degrees part in @rotation.
+ */
+const char *igt_rotation_degrees_str(igt_rotation_t rotation)
 {
 	switch (rotation & IGT_ROTATION_MASK) {
 	case IGT_ROTATION_0:
@@ -3826,7 +3833,7 @@ void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation)
 
 	LOG(display, "%s.%d: plane_set_rotation(%s)\n",
 	    kmstest_pipe_name(pipe->pipe),
-	    plane->index, rotation_name(rotation));
+	    plane->index, igt_rotation_degrees_str(rotation));
 
 	igt_plane_set_prop_value(plane, IGT_PLANE_ROTATION, rotation);
 }
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 3a12f278..fb62d8e3 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -410,6 +410,7 @@ void igt_plane_set_fence_fd(igt_plane_t *plane, int fence_fd);
 void igt_plane_set_position(igt_plane_t *plane, int x, int y);
 void igt_plane_set_size(igt_plane_t *plane, int w, int h);
 void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation);
+const char *igt_rotation_degrees_str(igt_rotation_t rotation);
 void igt_fb_set_position(struct igt_fb *fb, igt_plane_t *plane,
 	uint32_t x, uint32_t y);
 void igt_fb_set_size(struct igt_fb *fb, igt_plane_t *plane,
-- 
2.18.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [RESEND, v3 3/5] kms_plane: Split helpers creating reference FB and capturing CRC

[Gwan-gyeong Mun]

From: Imre Deak <imre.deak@intel.com>

Split creating a reference FB and capturing the CRC for it into separate
functions, so in a follow-up patch we can reuse the CRC capture function
for a reference FB created in a different way.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 tests/kms_plane.c | 50 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/tests/kms_plane.c b/tests/kms_plane.c
index aceae591..3f48c821 100644
--- a/tests/kms_plane.c
+++ b/tests/kms_plane.c
@@ -61,11 +61,9 @@ static void test_fini(data_t *data)
 }
 
 static void
-test_grab_crc(data_t *data, igt_output_t *output, enum pipe pipe,
-	      color_t *fb_color, igt_crc_t *crc /* out */)
+test_grab_crc_for_fb(data_t *data, igt_output_t *output, enum pipe pipe,
+		     igt_fb_t *fb, igt_crc_t *crc /* out */)
 {
-	struct igt_fb fb;
-	drmModeModeInfo *mode;
 	igt_plane_t *primary;
 	char *crc_str;
 	int ret;
@@ -74,13 +72,7 @@ test_grab_crc(data_t *data, igt_output_t *output, enum pipe pipe,
 
 	primary = igt_output_get_plane(output, 0);
 
-	mode = igt_output_get_mode(output);
-	igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
-			    DRM_FORMAT_XRGB8888,
-			    LOCAL_DRM_FORMAT_MOD_NONE,
-			    fb_color->red, fb_color->green, fb_color->blue,
-			    &fb);
-	igt_plane_set_fb(primary, &fb);
+	igt_plane_set_fb(primary, fb);
 
 	ret = igt_display_try_commit2(&data->display, COMMIT_LEGACY);
 	igt_skip_on(ret != 0);
@@ -89,14 +81,24 @@ test_grab_crc(data_t *data, igt_output_t *output, enum pipe pipe,
 
 	igt_plane_set_fb(primary, NULL);
 
-	igt_remove_fb(data->drm_fd, &fb);
-
 	crc_str = igt_crc_to_string(crc);
-	igt_debug("CRC for a (%.02f,%.02f,%.02f) fb: %s\n", fb_color->red,
-		  fb_color->green, fb_color->blue, crc_str);
+	igt_debug("CRC for fb: %s\n", crc_str);
 	free(crc_str);
 }
 
+static void
+test_create_fb_for_output(data_t *data, igt_output_t *output, color_t *fb_color,
+			  igt_fb_t *fb)
+{
+	drmModeModeInfo *mode = igt_output_get_mode(output);
+
+	igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
+			    DRM_FORMAT_XRGB8888,
+			    LOCAL_DRM_FORMAT_MOD_NONE,
+			    fb_color->red, fb_color->green, fb_color->blue,
+			    fb);
+}
+
 /*
  * Plane position test.
  *   - We start by grabbing a reference CRC of a full green fb being scanned
@@ -222,11 +224,15 @@ test_plane_position(data_t *data, enum pipe pipe, unsigned int flags)
 
 	for_each_valid_output_on_pipe(&data->display, pipe, output) {
 		int n_planes = data->display.pipes[pipe].n_planes;
+		igt_fb_t reference_fb;
 		igt_crc_t reference_crc;
 
 		test_init(data, pipe);
 
-		test_grab_crc(data, output, pipe, &green, &reference_crc);
+		test_create_fb_for_output(data, output, &green, &reference_fb);
+		test_grab_crc_for_fb(data, output, pipe, &reference_fb,
+				     &reference_crc);
+		igt_remove_fb(data->drm_fd, &reference_fb);
 
 		for (int plane = 1; plane < n_planes; plane++)
 			test_plane_position_with_output(data, pipe, plane,
@@ -343,13 +349,21 @@ test_plane_panning(data_t *data, enum pipe pipe, unsigned int flags)
 
 	for_each_valid_output_on_pipe(&data->display, pipe, output) {
 		int n_planes = data->display.pipes[pipe].n_planes;
+		igt_fb_t red_fb;
+		igt_fb_t blue_fb;
 		igt_crc_t red_crc;
 		igt_crc_t blue_crc;
 
 		test_init(data, pipe);
 
-		test_grab_crc(data, output, pipe, &red, &red_crc);
-		test_grab_crc(data, output, pipe, &blue, &blue_crc);
+		test_create_fb_for_output(data, output, &red, &red_fb);
+		test_create_fb_for_output(data, output, &blue, &blue_fb);
+
+		test_grab_crc_for_fb(data, output, pipe, &red_fb, &red_crc);
+		test_grab_crc_for_fb(data, output, pipe, &blue_fb, &blue_crc);
+
+		igt_remove_fb(data->drm_fd, &blue_fb);
+		igt_remove_fb(data->drm_fd, &red_fb);
 
 		for (int plane = 1; plane < n_planes; plane++)
 			test_plane_panning_with_output(data, pipe, plane,
-- 
2.18.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [RESEND, v3 4/5] kms_plane: Add a helper of capturing CRC with commit style

[Gwan-gyeong Mun]

As a legacy style commit does not allow changing of primary plane rotation
on non-first commit, it adds a helper of capturing CRC with commit style
argument for testing of primary plane rotation.

Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
 tests/kms_plane.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/tests/kms_plane.c b/tests/kms_plane.c
index 3f48c821..3d08893f 100644
--- a/tests/kms_plane.c
+++ b/tests/kms_plane.c
@@ -61,8 +61,9 @@ static void test_fini(data_t *data)
 }
 
 static void
-test_grab_crc_for_fb(data_t *data, igt_output_t *output, enum pipe pipe,
-		     igt_fb_t *fb, igt_crc_t *crc /* out */)
+test_grab_crc_for_fb2(data_t *data, igt_output_t *output, enum pipe pipe,
+		      igt_fb_t *fb, igt_crc_t *crc /* out */,
+		      enum igt_commit_style s)
 {
 	igt_plane_t *primary;
 	char *crc_str;
@@ -74,7 +75,8 @@ test_grab_crc_for_fb(data_t *data, igt_output_t *output, enum pipe pipe,
 
 	igt_plane_set_fb(primary, fb);
 
-	ret = igt_display_try_commit2(&data->display, COMMIT_LEGACY);
+	ret = igt_display_try_commit2(&data->display, s);
+
 	igt_skip_on(ret != 0);
 
 	igt_pipe_crc_collect_crc(data->pipe_crc, crc);
@@ -86,6 +88,13 @@ test_grab_crc_for_fb(data_t *data, igt_output_t *output, enum pipe pipe,
 	free(crc_str);
 }
 
+static void
+test_grab_crc_for_fb(data_t *data, igt_output_t *output, enum pipe pipe,
+		     igt_fb_t *fb, igt_crc_t *crc /* out */)
+{
+	test_grab_crc_for_fb2(data, output, pipe, fb, crc, COMMIT_LEGACY);
+}
+
 static void
 test_create_fb_for_output(data_t *data, igt_output_t *output, color_t *fb_color,
 			  igt_fb_t *fb)
-- 
2.18.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [RESEND,v3 5/5] kms_plane: Add clipping subtests

[Gwan-gyeong Mun]

From: Imre Deak <imre.deak@intel.com>

Add plane clipping subtests displaying a single clipped plane, with the
following test cases:
a) plane covering the whole screen, so that clipping is done at all 4
   screen edges
b) plane at either of the 4 corners of the screen clipped, so that a
   4x4 pixel part of the plane is visible
c) plane at either of the 4 corners of the screen clipped, so that a
   2x2 pixel part of the plane is visible

Each of the above cases are tested with all supported tiling modes,
rotation degrees, differing pixel format sizes (only 16 bpp and 32 bpp
for now) and certain planes. While the a) and b) cases above are
supported on all platforms c) is not fully supported on GLK and CNL
(which was the primary motivation for this testcase).

v2:
- Add missing reset for the output pipe after finishing the test for a
  given output, fixing the
  "DP-1 and eDP-1 are both trying to use pipe A" type of errors.
- Use -ERANGE instead of -EINVAL to check the return code for
  unsupported plane X positions, based on the latest kernel code.
- Add comment explaining the dependencies when doing a universal
  commit.

v3: (Gwan-gyeong)
- Add missing release of framebuffer on create_fb_for_mode__clipping_display()
  function.
- Add using multi planes per single display commit on clipping test of
  4 corners. (Daniel)
  It enables the improving of test speed. If number of planes is over
  than 4, it uses four planes from first plane to next four planes.
  (4 planes make testing of 4 corners possible as 1 display commit.)
  If number of planes is over than 2 and less than 4, it uses two planes
  from first plane to next two planes. (2 planes make testing of 4 corners
  possible as 2 display commit.) Rest of cases it uses only first plane.
  (1 plane makes testing of 4 corners possible as 4 display commit.)
  It changes to using of certain planes from using of all supported planes.
  And a cursor plane can be one of test planes.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Juha-Pekka Heikkilä <juha-pekka.heikkila@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> (v1)
---
 tests/kms_plane.c | 529 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 529 insertions(+)

diff --git a/tests/kms_plane.c b/tests/kms_plane.c
index 3d08893f..ea62efed 100644
--- a/tests/kms_plane.c
+++ b/tests/kms_plane.c
@@ -41,11 +41,40 @@ typedef struct {
 	int drm_fd;
 	igt_display_t display;
 	igt_pipe_crc_t *pipe_crc;
+	uint32_t devid;
+	uint64_t max_curw;
+	uint64_t max_curh;
 } data_t;
 
+typedef struct {
+	int x;
+	int y;
+	int size;
+} square_t;
+
 static color_t red   = { 1.0f, 0.0f, 0.0f };
 static color_t green = { 0.0f, 1.0f, 0.0f };
+static color_t yellow = { 1.0f, 1.0f, 0.0f };
 static color_t blue  = { 0.0f, 0.0f, 1.0f };
+static color_t white = { 1.0f, 1.0f, 1.0f };
+static square_t clip_squares[4];
+
+/*
+ * Size of a square plane used to test clipping at the 4 courners of the
+ * display.
+ */
+#define CLIPPED_PLANE_SMALL_SIZE	64
+
+/*
+ * Visible plane size after clipping that works on all platforms for all plane
+ * positions.
+ * The exceptions are GLK/CNL where there must be at least this many pixels
+ * visible from the plane after it's clipped to the left/right edge of the
+ * screen. Not meeting this condition may trigger FIFO underflows and screen
+ * corruption. The cursor plane is an exception that doesn't have this problem
+ * even on GLK/CNL.
+ */
+#define CLIPPED_PLANE_MIN_VALID		4
 
 /*
  * Common code across all tests, acting on data_t
@@ -151,6 +180,69 @@ create_fb_for_mode__position(data_t *data, drmModeModeInfo *mode,
 	igt_put_cairo_ctx(data->drm_fd, fb, cr);
 }
 
+/*
+ * Create a square FB for the plane in the clipping test, divided into 4
+ * quarters solid filled with different colors. Use the given tiling, format
+ * and size and rotate the FB clockwise with the given rotation degrees, so
+ * that the counterclockwise rotation with the same degrees done by the HW
+ * will always result in the same reference FB image.
+ */
+static void
+create_fb_for_mode__clipping_plane(data_t *data, igt_rotation_t rotation,
+				   uint64_t tiling,
+				   uint32_t format,
+				   int size,
+				   struct igt_fb *fb /* out */)
+{
+	color_t corners[] = { red, white, yellow, blue };
+	color_t color;
+	unsigned int fb_id;
+	cairo_t *cr;
+	const int qsize = size / 2;
+	int idx;
+
+	fb_id = igt_create_fb(data->drm_fd, size, size, format, tiling, fb);
+	igt_assert(fb_id);
+
+	cr = igt_get_cairo_ctx(data->drm_fd, fb);
+
+	switch (rotation) {
+	case IGT_ROTATION_0:
+		idx = 0;
+		break;
+	case IGT_ROTATION_90:
+		idx = 3;
+		break;
+	case IGT_ROTATION_180:
+		idx = 2;
+		break;
+	case IGT_ROTATION_270:
+		idx = 1;
+		break;
+	default:
+		igt_assert(0);
+	}
+
+	color = corners[idx];
+	igt_paint_color(cr, 0, 0, qsize, qsize,
+			color.red, color.green, color.blue);
+
+	color = corners[(++idx) % 4];
+	igt_paint_color(cr, qsize, 0, qsize, qsize,
+			color.red, color.green, color.blue);
+
+	color = corners[(++idx) % 4];
+	igt_paint_color(cr, qsize, qsize, qsize, qsize,
+			color.red, color.green, color.blue);
+
+	color = corners[(++idx) % 4];
+	igt_paint_color(cr, 0, qsize, qsize, qsize,
+			color.red, color.green, color.blue);
+
+	igt_assert(cairo_status(cr) == 0);
+	cairo_destroy(cr);
+}
+
 enum {
 	TEST_POSITION_FULLY_COVERED = 1 << 0,
 	TEST_DPMS = 1 << 1,
@@ -554,6 +646,427 @@ test_pixel_formats(data_t *data, enum pipe pipe)
 	}
 }
 
+static bool
+bogus_plane_conf(uint32_t devid, igt_plane_t *plane_obj,
+		 int hdisplay, int plane_x, int plane_width)
+{
+	if (!(IS_GEMINILAKE(devid) || IS_CANNONLAKE(devid)))
+		return false;
+
+	if (plane_obj->type == DRM_PLANE_TYPE_CURSOR)
+		return false;
+
+	if (plane_x + plane_width >= CLIPPED_PLANE_MIN_VALID &&
+	    plane_x <= hdisplay - CLIPPED_PLANE_MIN_VALID)
+		return false;
+
+	return true;
+}
+
+static bool
+supported_plane_conf(data_t *data, int plane_type, uint64_t tiling,
+		     uint32_t format, igt_rotation_t rotation,
+		     drmModeModeInfo *mode,
+		     int plane_x, int plane_y, int plane_size)
+{
+	if (intel_gen(data->devid) < 9 &&
+	    tiling != LOCAL_DRM_FORMAT_MOD_NONE)
+		return false;
+
+	/* On GEN<9 the primary plane must cover the full screen. */
+	if (intel_gen(data->devid) < 9 &&
+	    plane_type == DRM_PLANE_TYPE_PRIMARY &&
+	    (plane_x > 0 || plane_y > 0 ||
+	     (plane_x + plane_size < mode->hdisplay) ||
+	     (plane_y + plane_size < mode->vdisplay)))
+		return false;
+
+	if (plane_type == DRM_PLANE_TYPE_CURSOR) {
+		/* For cursor planes only linear/alpha format is supported. */
+		if (tiling != LOCAL_DRM_FORMAT_MOD_NONE ||
+		    format != DRM_FORMAT_ARGB8888)
+			return false;
+
+		if (plane_size > data->max_curw || plane_size > data->max_curh)
+			return false;
+	} else {
+		/*
+		 * For non-cursor planes formats with alpha may result in
+		 * undeterministic CRCs, we use the same sized
+		 * non-alpha XRGB8888 format instead.
+		 */
+		if (format == DRM_FORMAT_ARGB8888)
+			return false;
+
+		if (format == DRM_FORMAT_RGB565 &&
+		    intel_gen(data->devid) < 9 &&
+		    !IS_VALLEYVIEW(data->devid) && !IS_CHERRYVIEW(data->devid))
+			return false;
+	}
+
+	/* RGB565 with rotation is not supported for now. */
+	if (format == DRM_FORMAT_RGB565 && rotation != IGT_ROTATION_0)
+		return false;
+
+	return true;
+}
+
+/*
+ * Create a square reference FB for the whole screen in the clipping test,
+ * with the given test plane position and size. See
+ * create_fb_for_mode__clipping_plane() for the layout of the test plane.
+ */
+static void
+create_fb_for_mode__clipping_display(data_t *data, drmModeModeInfo *mode,
+				     square_t *clip_squares,
+				     int clip_squares_cnt,
+				     struct igt_fb *fb /* out */)
+{
+	int plane;
+	struct igt_fb plane_fbs[4];
+	unsigned int fb_id;
+	cairo_t *cr;
+	cairo_surface_t *srcs[4];
+
+	fb_id = igt_create_fb(data->drm_fd,
+			      mode->hdisplay, mode->vdisplay,
+			      DRM_FORMAT_XRGB8888,
+			      LOCAL_DRM_FORMAT_MOD_NONE,
+			      fb);
+	igt_assert(fb_id);
+
+	cr = igt_get_cairo_ctx(data->drm_fd, fb);
+	igt_paint_color(cr, 0, 0, mode->hdisplay, mode->vdisplay,
+			0, 0, 0);
+
+	for (plane = 0; plane < clip_squares_cnt; plane++) {
+		create_fb_for_mode__clipping_plane(data, IGT_ROTATION_0,
+						   LOCAL_DRM_FORMAT_MOD_NONE,
+						   DRM_FORMAT_XRGB8888,
+						   clip_squares[plane].size,
+						   &plane_fbs[plane]);
+
+		srcs[plane] = igt_get_cairo_surface(data->drm_fd,
+						   &plane_fbs[plane]);
+		cairo_set_source_surface(cr, srcs[plane],
+					 clip_squares[plane].x,
+					 clip_squares[plane].y);
+		cairo_rectangle(cr,
+				clip_squares[plane].x,
+				clip_squares[plane].y,
+				clip_squares[plane].size,
+				clip_squares[plane].size);
+		cairo_fill(cr);
+	}
+
+	igt_assert(cairo_status(cr) == 0);
+	cairo_destroy(cr);
+
+	for (plane = 0; plane < clip_squares_cnt; plane++) {
+		cairo_surface_destroy(srcs[plane]);
+		igt_remove_fb(data->drm_fd, &plane_fbs[plane]);
+	}
+}
+
+static void
+test_plane_clipping_format(data_t *data,
+			   enum pipe pipe,
+			   igt_output_t *output,
+			   drmModeModeInfo *mode,
+			   square_t *clip_squares,
+			   int clip_squares_cnt,
+			   uint64_t tiling,
+			   igt_rotation_t rotation,
+			   uint32_t format,
+			   igt_crc_t *reference_crc)
+{
+	int plane;
+	igt_plane_t *plane_objs[4];
+	struct igt_fb plane_fbs[4];
+	igt_crc_t crc;
+	int ret;
+	bool bogus_planes = false;
+
+	memset(plane_objs, 0, sizeof(plane_objs));
+
+	igt_debug("rotation %s tiling %s format %s\n",
+		  igt_rotation_degrees_str(rotation),
+		  igt_tiling_str(tiling),
+		  igt_format_str(format));
+
+	igt_output_set_pipe(output, pipe);
+
+	for (plane = 0; plane < clip_squares_cnt ; plane++) {
+		igt_plane_t *obj = igt_output_get_plane(output, plane);
+		struct igt_fb *fb = &plane_fbs[plane];
+		int x = clip_squares[plane].x;
+		int y = clip_squares[plane].y;
+		int size = clip_squares[plane].size;
+		uint64_t _tiling = tiling;
+		uint32_t _format = format;
+
+		if (obj->type == DRM_PLANE_TYPE_CURSOR) {
+			_tiling = LOCAL_DRM_FORMAT_MOD_NONE;
+			_format = DRM_FORMAT_ARGB8888;
+		}
+
+		if (!supported_plane_conf(data, obj->type, _tiling, _format,
+					  rotation, mode, x, y, size))
+			goto out;
+
+		create_fb_for_mode__clipping_plane(data, rotation,_tiling,
+						   _format, size, fb);
+
+		igt_plane_set_fb(obj, fb);
+		igt_fb_set_position(fb, obj, 0, 0);
+
+		igt_plane_set_size(obj, size, size);
+		igt_plane_set_rotation(obj, rotation);
+		igt_plane_set_position(obj, x, y);
+
+		plane_objs[plane] = obj;
+	}
+
+	/*
+	 * Note that a universal commit doesn't support full modesets, so we
+	 * have to make sure that the following only needs to commit changes
+	 * that are compatible with a fastset. This should be guaranteed,
+	 * since before calling this function we took the reference CRC which
+	 * left the display enabled with the mode we require here and
+	 * afterwards we only change plane parameters (FB, position, rotation
+	 * etc.).
+	 */
+	ret = igt_display_try_commit2(&data->display, COMMIT_UNIVERSAL);
+
+	for (plane = 0; plane < clip_squares_cnt; plane++) {
+		if(!bogus_plane_conf(data->devid, plane_objs[plane],
+		   mode->hdisplay, clip_squares[plane].x,
+		   clip_squares[plane].size)) {
+			bogus_planes = true;
+			break;
+		}
+	}
+
+	if (bogus_planes) {
+		igt_assert(ret == 0);
+		igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
+		igt_assert_crc_equal(reference_crc, &crc);
+	} else {
+		igt_assert(ret == -ERANGE);
+	}
+
+out:
+	for (plane = 0; plane < clip_squares_cnt; plane++) {
+		if (!plane_objs[plane])
+			continue;
+		igt_plane_set_fb(plane_objs[plane], NULL);
+		igt_plane_set_rotation(plane_objs[plane], IGT_ROTATION_0);
+		igt_plane_set_position(plane_objs[plane], 0, 0);
+
+		igt_remove_fb(data->drm_fd, &plane_fbs[plane]);
+	}
+
+	igt_output_set_pipe(output, PIPE_ANY);
+}
+
+static void
+test_plane_clipping_square(data_t *data, enum pipe pipe,
+			   igt_output_t *output, drmModeModeInfo *mode,
+			   square_t *clip_squares, int clip_squares_cnt)
+{
+	const struct {
+		uint64_t tiling;
+		igt_rotation_t rotation;
+	} rotations[] = {
+		{ LOCAL_DRM_FORMAT_MOD_NONE,		IGT_ROTATION_0 },
+
+		{ LOCAL_I915_FORMAT_MOD_X_TILED,	IGT_ROTATION_0 },
+
+		{ LOCAL_I915_FORMAT_MOD_Y_TILED,	IGT_ROTATION_0 },
+		{ LOCAL_I915_FORMAT_MOD_Y_TILED,	IGT_ROTATION_90 },
+		{ LOCAL_I915_FORMAT_MOD_Y_TILED,	IGT_ROTATION_180 },
+		{ LOCAL_I915_FORMAT_MOD_Y_TILED,	IGT_ROTATION_270 },
+
+		{ LOCAL_I915_FORMAT_MOD_Yf_TILED,	IGT_ROTATION_0 },
+		{ LOCAL_I915_FORMAT_MOD_Yf_TILED,	IGT_ROTATION_90 },
+		{ LOCAL_I915_FORMAT_MOD_Yf_TILED,	IGT_ROTATION_180 },
+		{ LOCAL_I915_FORMAT_MOD_Yf_TILED,	IGT_ROTATION_270 },
+	};
+	const uint32_t formats[] = {
+		DRM_FORMAT_RGB565,
+		DRM_FORMAT_XRGB8888,
+		DRM_FORMAT_ARGB8888,
+	};
+
+	igt_fb_t reference_fb;
+	igt_crc_t reference_crc;
+
+	for (int plane = 0; plane < clip_squares_cnt; plane++) {
+		igt_info("Testing connector %s mode %dx%d using pipe %s: %dx%d@%d,%d\n",
+			 igt_output_name(output),
+			 mode->hdisplay, mode->vdisplay,
+			 kmstest_pipe_name(pipe),
+			 clip_squares[plane].size, clip_squares[plane].size,
+			 clip_squares[plane].x, clip_squares[plane].y);
+	}
+
+	test_init(data, pipe);
+
+	create_fb_for_mode__clipping_display(data, mode,
+					     clip_squares,
+					     clip_squares_cnt,
+					     &reference_fb);
+
+	test_grab_crc_for_fb2(data, output, pipe, &reference_fb,
+			      &reference_crc, COMMIT_UNIVERSAL);
+
+	igt_remove_fb(data->drm_fd, &reference_fb);
+
+	for (int rotation = 0; rotation < ARRAY_SIZE(rotations); rotation++)
+		for (int format = 0; format < ARRAY_SIZE(formats); format++)
+			test_plane_clipping_format(data, pipe, output,
+						   mode,
+						   clip_squares,
+						   clip_squares_cnt,
+						   rotations[rotation].tiling,
+						   rotations[rotation].rotation,
+						   formats[format],
+						   &reference_crc);
+
+	test_fini(data);
+}
+
+static void
+test_plane_clipping_squares(data_t *data, enum pipe pipe,
+			    igt_output_t *output, drmModeModeInfo *mode,
+			    square_t *clip_squares, int clip_squares_cnt)
+{
+	switch (clip_squares_cnt) {
+	case 4:
+	    test_plane_clipping_square(data, pipe, output, mode,
+				       clip_squares, clip_squares_cnt);
+	    break;
+	case 2:
+	    test_plane_clipping_square(data, pipe, output, mode,
+				       clip_squares, clip_squares_cnt);
+	    test_plane_clipping_square(data, pipe, output, mode,
+				       clip_squares + clip_squares_cnt,
+				       clip_squares_cnt);
+	    break;
+	case 1:
+	default:
+	    for (int i = 0; i  < 4; i++) {
+		test_plane_clipping_square(data, pipe, output, mode,
+					   clip_squares + i,
+					   clip_squares_cnt);
+	    }
+	    break;
+	}
+}
+
+static void
+test_plane_clipping(data_t *data, enum pipe pipe)
+{
+	igt_output_t *output;
+	int connected_outs = 0;
+
+	for_each_valid_output_on_pipe(&data->display, pipe, output) {
+		drmModeModeInfo *mode;
+		int sq_size;
+		int max_planes = data->display.pipes[pipe].n_planes;
+		int clip_squares_cnt;
+
+		clip_squares_cnt = max_planes >= 4 ?
+				   4 : max_planes >= 2 ? 2 : 1;
+
+		igt_output_set_pipe(output, pipe);
+		mode = igt_output_get_mode(output);
+
+		/*
+		 * Test with a square plane bigger than either the width or
+		 * height of the mode. This should pass on all platforms.
+		 */
+		sq_size = mode->hdisplay > mode->vdisplay ?
+			  mode->hdisplay : mode->vdisplay;
+
+		clip_squares[0].x = -2;
+		clip_squares[0].y = -2;
+		clip_squares[0].size = sq_size + 4;
+
+		test_plane_clipping_square(data, pipe, output, mode,
+					   clip_squares, 1);
+
+		/*
+		 * Test positions in the 4 corners of the screen with a
+		 * CLIPPED_PLANE_MIN_VALID x CLIPPED_PLANE_MIN_VALID square
+		 * visible from the plane. These should pass on all platforms.
+		 */
+		clip_squares[0].x = -CLIPPED_PLANE_SMALL_SIZE +
+				     CLIPPED_PLANE_MIN_VALID;
+		clip_squares[0].y = -CLIPPED_PLANE_SMALL_SIZE +
+				     CLIPPED_PLANE_MIN_VALID;
+		clip_squares[0].size = CLIPPED_PLANE_SMALL_SIZE;
+
+		clip_squares[1].x = -CLIPPED_PLANE_SMALL_SIZE +
+				     CLIPPED_PLANE_MIN_VALID;
+		clip_squares[1].y = mode->vdisplay -
+				    CLIPPED_PLANE_MIN_VALID;
+		clip_squares[1].size = CLIPPED_PLANE_SMALL_SIZE;
+
+		clip_squares[2].x = mode->hdisplay -
+				    CLIPPED_PLANE_MIN_VALID;
+		clip_squares[2].y = -CLIPPED_PLANE_SMALL_SIZE +
+				     CLIPPED_PLANE_MIN_VALID;
+		clip_squares[2].size = CLIPPED_PLANE_SMALL_SIZE;
+
+		clip_squares[3].x = mode->hdisplay -
+				    CLIPPED_PLANE_MIN_VALID;
+		clip_squares[3].y = mode->vdisplay -
+				    CLIPPED_PLANE_MIN_VALID;
+		clip_squares[3].size = CLIPPED_PLANE_SMALL_SIZE;
+
+		test_plane_clipping_squares(data, pipe, output, mode,
+					    clip_squares, clip_squares_cnt);
+
+		/*
+		 * Test positions in the 4 corners of the screen with a
+		 * 2 x 2 square visible from the plane. These are valid on all
+		 * platforms except on GLK/CNL where less than
+		 * CLIPPED_PLANE_MIN_VALID pixels visible on the left/right
+		 * edges of the screen may cause FIFO underflow and display
+		 * corruption.
+		 *
+		 * The cursor plane is an exception without this problem.
+		 *
+		 * Use 2 x 2 size as other (odd) sizes may result in an
+		 * incorrect CRC for the cursor plane even though it displays
+		 * correctly and causes no underflow.
+		 */
+		clip_squares[0].x = -CLIPPED_PLANE_SMALL_SIZE + 2;
+		clip_squares[0].y = -CLIPPED_PLANE_SMALL_SIZE + 2;
+		clip_squares[0].size = CLIPPED_PLANE_SMALL_SIZE;
+
+		clip_squares[1].x = -CLIPPED_PLANE_SMALL_SIZE + 2;
+		clip_squares[1].y = mode->vdisplay - 2;
+		clip_squares[1].size = CLIPPED_PLANE_SMALL_SIZE;
+
+		clip_squares[2].x = mode->hdisplay - 2;
+		clip_squares[2].y = -CLIPPED_PLANE_SMALL_SIZE + 2;
+		clip_squares[2].size = CLIPPED_PLANE_SMALL_SIZE;
+
+		clip_squares[3].x = mode->hdisplay - 2;
+		clip_squares[3].y = mode->vdisplay - 2;
+		clip_squares[3].size = CLIPPED_PLANE_SMALL_SIZE;
+
+		test_plane_clipping_squares(data, pipe, output, mode,
+					    clip_squares, clip_squares_cnt);
+
+		connected_outs++;
+	}
+
+	igt_skip_on(connected_outs == 0);
+}
+
 static void
 run_tests_for_pipe_plane(data_t *data, enum pipe pipe)
 {
@@ -590,6 +1103,9 @@ run_tests_for_pipe_plane(data_t *data, enum pipe pipe)
 		      kmstest_pipe_name(pipe))
 		test_plane_panning(data, pipe, TEST_PANNING_BOTTOM_RIGHT |
 					       TEST_SUSPEND_RESUME);
+	igt_subtest_f("plane-clipping-pipe-%s-planes",
+		      kmstest_pipe_name(pipe))
+		test_plane_clipping(data, pipe);
 }
 
 
@@ -602,8 +1118,21 @@ igt_main
 	igt_skip_on_simulation();
 
 	igt_fixture {
+		int ret;
+
 		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
 
+		data.devid = intel_get_drm_devid(data.drm_fd);
+
+		data.max_curw = 64;
+		data.max_curh = 64;
+		ret = drmGetCap(data.drm_fd, DRM_CAP_CURSOR_WIDTH,
+				&data.max_curw);
+		igt_assert(ret == 0 || errno == EINVAL);
+		ret = drmGetCap(data.drm_fd, DRM_CAP_CURSOR_HEIGHT,
+				&data.max_curh);
+		igt_assert(ret == 0 || errno == EINVAL);
+
 		kmstest_set_vt_graphics_mode();
 
 		igt_require_pipe_crc(data.drm_fd);
-- 
2.18.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [RESEND,v3 5/5] kms_plane: Add clipping subtests

[Imre Deak]

Hi G.G.,

Thanks for doing this, looks ok in general. Few comments below.

On Thu, Sep 06, 2018 at 10:24:00AM +0300, Gwan-gyeong Mun wrote:
> From: Imre Deak <imre.deak@intel.com>
> 
> Add plane clipping subtests displaying a single clipped plane, with the
> following test cases:
> a) plane covering the whole screen, so that clipping is done at all 4
>    screen edges
> b) plane at either of the 4 corners of the screen clipped, so that a
>    4x4 pixel part of the plane is visible
> c) plane at either of the 4 corners of the screen clipped, so that a
>    2x2 pixel part of the plane is visible
> 
> Each of the above cases are tested with all supported tiling modes,
> rotation degrees, differing pixel format sizes (only 16 bpp and 32 bpp
> for now) and certain planes. While the a) and b) cases above are
> supported on all platforms c) is not fully supported on GLK and CNL
> (which was the primary motivation for this testcase).
> 
> v2:
> - Add missing reset for the output pipe after finishing the test for a
>   given output, fixing the
>   "DP-1 and eDP-1 are both trying to use pipe A" type of errors.
> - Use -ERANGE instead of -EINVAL to check the return code for
>   unsupported plane X positions, based on the latest kernel code.
> - Add comment explaining the dependencies when doing a universal
>   commit.
> 
> v3: (Gwan-gyeong)
> - Add missing release of framebuffer on create_fb_for_mode__clipping_display()
>   function.
> - Add using multi planes per single display commit on clipping test of
>   4 corners. (Daniel)
>   It enables the improving of test speed. If number of planes is over
>   than 4, it uses four planes from first plane to next four planes.
>   (4 planes make testing of 4 corners possible as 1 display commit.)
>   If number of planes is over than 2 and less than 4, it uses two planes
>   from first plane to next two planes. (2 planes make testing of 4 corners
>   possible as 2 display commit.) Rest of cases it uses only first plane.
>   (1 plane makes testing of 4 corners possible as 4 display commit.)
>   It changes to using of certain planes from using of all supported planes.
>   And a cursor plane can be one of test planes.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Cc: Mika Kahola <mika.kahola@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Juha-Pekka Heikkilä <juha-pekka.heikkila@intel.com>
> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> (v1)

> ---
>  tests/kms_plane.c | 529 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 529 insertions(+)
> 
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> index 3d08893f..ea62efed 100644
> --- a/tests/kms_plane.c
> +++ b/tests/kms_plane.c
> @@ -41,11 +41,40 @@ typedef struct {
>  	int drm_fd;
>  	igt_display_t display;
>  	igt_pipe_crc_t *pipe_crc;
> +	uint32_t devid;
> +	uint64_t max_curw;
> +	uint64_t max_curh;
>  } data_t;
>  
> +typedef struct {
> +	int x;
> +	int y;
> +	int size;
> +} square_t;
> +
>  static color_t red   = { 1.0f, 0.0f, 0.0f };
>  static color_t green = { 0.0f, 1.0f, 0.0f };
> +static color_t yellow = { 1.0f, 1.0f, 0.0f };
>  static color_t blue  = { 0.0f, 0.0f, 1.0f };
> +static color_t white = { 1.0f, 1.0f, 1.0f };
> +static square_t clip_squares[4];
> +
> +/*
> + * Size of a square plane used to test clipping at the 4 courners of the
> + * display.
> + */
> +#define CLIPPED_PLANE_SMALL_SIZE	64
> +
> +/*
> + * Visible plane size after clipping that works on all platforms for all plane
> + * positions.
> + * The exceptions are GLK/CNL where there must be at least this many pixels
> + * visible from the plane after it's clipped to the left/right edge of the
> + * screen. Not meeting this condition may trigger FIFO underflows and screen
> + * corruption. The cursor plane is an exception that doesn't have this problem
> + * even on GLK/CNL.
> + */
> +#define CLIPPED_PLANE_MIN_VALID		4
>  
>  /*
>   * Common code across all tests, acting on data_t
> @@ -151,6 +180,69 @@ create_fb_for_mode__position(data_t *data, drmModeModeInfo *mode,
>  	igt_put_cairo_ctx(data->drm_fd, fb, cr);
>  }
>  
> +/*
> + * Create a square FB for the plane in the clipping test, divided into 4
> + * quarters solid filled with different colors. Use the given tiling, format
> + * and size and rotate the FB clockwise with the given rotation degrees, so
> + * that the counterclockwise rotation with the same degrees done by the HW
> + * will always result in the same reference FB image.
> + */
> +static void
> +create_fb_for_mode__clipping_plane(data_t *data, igt_rotation_t rotation,
> +				   uint64_t tiling,
> +				   uint32_t format,
> +				   int size,
> +				   struct igt_fb *fb /* out */)
> +{
> +	color_t corners[] = { red, white, yellow, blue };
> +	color_t color;
> +	unsigned int fb_id;
> +	cairo_t *cr;
> +	const int qsize = size / 2;
> +	int idx;
> +
> +	fb_id = igt_create_fb(data->drm_fd, size, size, format, tiling, fb);
> +	igt_assert(fb_id);
> +
> +	cr = igt_get_cairo_ctx(data->drm_fd, fb);
> +
> +	switch (rotation) {
> +	case IGT_ROTATION_0:
> +		idx = 0;
> +		break;
> +	case IGT_ROTATION_90:
> +		idx = 3;
> +		break;
> +	case IGT_ROTATION_180:
> +		idx = 2;
> +		break;
> +	case IGT_ROTATION_270:
> +		idx = 1;
> +		break;
> +	default:
> +		igt_assert(0);
> +	}
> +
> +	color = corners[idx];
> +	igt_paint_color(cr, 0, 0, qsize, qsize,
> +			color.red, color.green, color.blue);
> +
> +	color = corners[(++idx) % 4];
> +	igt_paint_color(cr, qsize, 0, qsize, qsize,
> +			color.red, color.green, color.blue);
> +
> +	color = corners[(++idx) % 4];
> +	igt_paint_color(cr, qsize, qsize, qsize, qsize,
> +			color.red, color.green, color.blue);
> +
> +	color = corners[(++idx) % 4];
> +	igt_paint_color(cr, 0, qsize, qsize, qsize,
> +			color.red, color.green, color.blue);
> +
> +	igt_assert(cairo_status(cr) == 0);
> +	cairo_destroy(cr);
> +}
> +
>  enum {
>  	TEST_POSITION_FULLY_COVERED = 1 << 0,
>  	TEST_DPMS = 1 << 1,
> @@ -554,6 +646,427 @@ test_pixel_formats(data_t *data, enum pipe pipe)
>  	}
>  }
>  
> +static bool
> +bogus_plane_conf(uint32_t devid, igt_plane_t *plane_obj,
> +		 int hdisplay, int plane_x, int plane_width)
> +{
> +	if (!(IS_GEMINILAKE(devid) || IS_CANNONLAKE(devid)))
> +		return false;
> +
> +	if (plane_obj->type == DRM_PLANE_TYPE_CURSOR)
> +		return false;
> +
> +	if (plane_x + plane_width >= CLIPPED_PLANE_MIN_VALID &&
> +	    plane_x <= hdisplay - CLIPPED_PLANE_MIN_VALID)
> +		return false;
> +
> +	return true;
> +}
> +
> +static bool
> +supported_plane_conf(data_t *data, int plane_type, uint64_t tiling,
> +		     uint32_t format, igt_rotation_t rotation,
> +		     drmModeModeInfo *mode,
> +		     int plane_x, int plane_y, int plane_size)
> +{
> +	if (intel_gen(data->devid) < 9 &&
> +	    tiling != LOCAL_DRM_FORMAT_MOD_NONE)
> +		return false;
> +
> +	/* On GEN<9 the primary plane must cover the full screen. */
> +	if (intel_gen(data->devid) < 9 &&
> +	    plane_type == DRM_PLANE_TYPE_PRIMARY &&
> +	    (plane_x > 0 || plane_y > 0 ||
> +	     (plane_x + plane_size < mode->hdisplay) ||
> +	     (plane_y + plane_size < mode->vdisplay)))
> +		return false;
> +
> +	if (plane_type == DRM_PLANE_TYPE_CURSOR) {
> +		/* For cursor planes only linear/alpha format is supported. */
> +		if (tiling != LOCAL_DRM_FORMAT_MOD_NONE ||
> +		    format != DRM_FORMAT_ARGB8888)
> +			return false;
> +
> +		if (plane_size > data->max_curw || plane_size > data->max_curh)
> +			return false;
> +	} else {
> +		/*
> +		 * For non-cursor planes formats with alpha may result in
> +		 * undeterministic CRCs, we use the same sized
> +		 * non-alpha XRGB8888 format instead.
> +		 */
> +		if (format == DRM_FORMAT_ARGB8888)
> +			return false;
> +
> +		if (format == DRM_FORMAT_RGB565 &&
> +		    intel_gen(data->devid) < 9 &&
> +		    !IS_VALLEYVIEW(data->devid) && !IS_CHERRYVIEW(data->devid))
> +			return false;
> +	}
> +
> +	/* RGB565 with rotation is not supported for now. */
> +	if (format == DRM_FORMAT_RGB565 && rotation != IGT_ROTATION_0)
> +		return false;
> +
> +	return true;
> +}
> +
> +/*
> + * Create a square reference FB for the whole screen in the clipping test,
> + * with the given test plane position and size. See
> + * create_fb_for_mode__clipping_plane() for the layout of the test plane.
> + */
> +static void
> +create_fb_for_mode__clipping_display(data_t *data, drmModeModeInfo *mode,
> +				     square_t *clip_squares,
> +				     int clip_squares_cnt,
> +				     struct igt_fb *fb /* out */)
> +{
> +	int plane;
> +	struct igt_fb plane_fbs[4];
> +	unsigned int fb_id;
> +	cairo_t *cr;
> +	cairo_surface_t *srcs[4];
> +
> +	fb_id = igt_create_fb(data->drm_fd,
> +			      mode->hdisplay, mode->vdisplay,
> +			      DRM_FORMAT_XRGB8888,
> +			      LOCAL_DRM_FORMAT_MOD_NONE,
> +			      fb);
> +	igt_assert(fb_id);
> +
> +	cr = igt_get_cairo_ctx(data->drm_fd, fb);
> +	igt_paint_color(cr, 0, 0, mode->hdisplay, mode->vdisplay,
> +			0, 0, 0);
> +
> +	for (plane = 0; plane < clip_squares_cnt; plane++) {
> +		create_fb_for_mode__clipping_plane(data, IGT_ROTATION_0,
> +						   LOCAL_DRM_FORMAT_MOD_NONE,
> +						   DRM_FORMAT_XRGB8888,
> +						   clip_squares[plane].size,
> +						   &plane_fbs[plane]);
> +
> +		srcs[plane] = igt_get_cairo_surface(data->drm_fd,
> +						   &plane_fbs[plane]);
> +		cairo_set_source_surface(cr, srcs[plane],
> +					 clip_squares[plane].x,
> +					 clip_squares[plane].y);
> +		cairo_rectangle(cr,
> +				clip_squares[plane].x,
> +				clip_squares[plane].y,
> +				clip_squares[plane].size,
> +				clip_squares[plane].size);
> +		cairo_fill(cr);

Could remove the FB, cairo surface already here, so we don't need the
array and a second loop below?

> +	}
> +
> +	igt_assert(cairo_status(cr) == 0);
> +	cairo_destroy(cr);
> +
> +	for (plane = 0; plane < clip_squares_cnt; plane++) {
> +		cairo_surface_destroy(srcs[plane]);
> +		igt_remove_fb(data->drm_fd, &plane_fbs[plane]);
> +	}
> +}
> +
> +static void
> +test_plane_clipping_format(data_t *data,
> +			   enum pipe pipe,
> +			   igt_output_t *output,
> +			   drmModeModeInfo *mode,
> +			   square_t *clip_squares,
> +			   int clip_squares_cnt,
> +			   uint64_t tiling,
> +			   igt_rotation_t rotation,
> +			   uint32_t format,
> +			   igt_crc_t *reference_crc)
> +{
> +	int plane;
> +	igt_plane_t *plane_objs[4];
> +	struct igt_fb plane_fbs[4];
> +	igt_crc_t crc;
> +	int ret;
> +	bool bogus_planes = false;
> +
> +	memset(plane_objs, 0, sizeof(plane_objs));
> +
> +	igt_debug("rotation %s tiling %s format %s\n",
> +		  igt_rotation_degrees_str(rotation),
> +		  igt_tiling_str(tiling),
> +		  igt_format_str(format));
> +
> +	igt_output_set_pipe(output, pipe);
> +
> +	for (plane = 0; plane < clip_squares_cnt ; plane++) {
> +		igt_plane_t *obj = igt_output_get_plane(output, plane);
> +		struct igt_fb *fb = &plane_fbs[plane];
> +		int x = clip_squares[plane].x;
> +		int y = clip_squares[plane].y;
> +		int size = clip_squares[plane].size;
> +		uint64_t _tiling = tiling;
> +		uint32_t _format = format;
> +
> +		if (obj->type == DRM_PLANE_TYPE_CURSOR) {
> +			_tiling = LOCAL_DRM_FORMAT_MOD_NONE;
> +			_format = DRM_FORMAT_ARGB8888;
> +		}
> +
> +		if (!supported_plane_conf(data, obj->type, _tiling, _format,
> +					  rotation, mode, x, y, size))
> +			goto out;
> +
> +		create_fb_for_mode__clipping_plane(data, rotation,_tiling,
> +						   _format, size, fb);
> +
> +		igt_plane_set_fb(obj, fb);
> +		igt_fb_set_position(fb, obj, 0, 0);
> +
> +		igt_plane_set_size(obj, size, size);
> +		igt_plane_set_rotation(obj, rotation);
> +		igt_plane_set_position(obj, x, y);
> +
> +		plane_objs[plane] = obj;
> +	}
> +
> +	/*
> +	 * Note that a universal commit doesn't support full modesets, so we
> +	 * have to make sure that the following only needs to commit changes
> +	 * that are compatible with a fastset. This should be guaranteed,
> +	 * since before calling this function we took the reference CRC which
> +	 * left the display enabled with the mode we require here and
> +	 * afterwards we only change plane parameters (FB, position, rotation
> +	 * etc.).
> +	 */
> +	ret = igt_display_try_commit2(&data->display, COMMIT_UNIVERSAL);
> +
> +	for (plane = 0; plane < clip_squares_cnt; plane++) {
> +		if(!bogus_plane_conf(data->devid, plane_objs[plane],
> +		   mode->hdisplay, clip_squares[plane].x,
> +		   clip_squares[plane].size)) {

Please check formatting (kernel style).

> +			bogus_planes = true;
> +			break;
> +		}
> +	}
> +
> +	if (bogus_planes) {
> +		igt_assert(ret == 0);
> +		igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
> +		igt_assert_crc_equal(reference_crc, &crc);
> +	} else {
> +		igt_assert(ret == -ERANGE);
> +	}
> +
> +out:
> +	for (plane = 0; plane < clip_squares_cnt; plane++) {
> +		if (!plane_objs[plane])
> +			continue;
> +		igt_plane_set_fb(plane_objs[plane], NULL);
> +		igt_plane_set_rotation(plane_objs[plane], IGT_ROTATION_0);
> +		igt_plane_set_position(plane_objs[plane], 0, 0);
> +
> +		igt_remove_fb(data->drm_fd, &plane_fbs[plane]);
> +	}
> +
> +	igt_output_set_pipe(output, PIPE_ANY);
> +}
> +
> +static void
> +test_plane_clipping_square(data_t *data, enum pipe pipe,
> +			   igt_output_t *output, drmModeModeInfo *mode,
> +			   square_t *clip_squares, int clip_squares_cnt)
> +{
> +	const struct {
> +		uint64_t tiling;
> +		igt_rotation_t rotation;
> +	} rotations[] = {
> +		{ LOCAL_DRM_FORMAT_MOD_NONE,		IGT_ROTATION_0 },
> +
> +		{ LOCAL_I915_FORMAT_MOD_X_TILED,	IGT_ROTATION_0 },
> +
> +		{ LOCAL_I915_FORMAT_MOD_Y_TILED,	IGT_ROTATION_0 },
> +		{ LOCAL_I915_FORMAT_MOD_Y_TILED,	IGT_ROTATION_90 },
> +		{ LOCAL_I915_FORMAT_MOD_Y_TILED,	IGT_ROTATION_180 },
> +		{ LOCAL_I915_FORMAT_MOD_Y_TILED,	IGT_ROTATION_270 },
> +
> +		{ LOCAL_I915_FORMAT_MOD_Yf_TILED,	IGT_ROTATION_0 },
> +		{ LOCAL_I915_FORMAT_MOD_Yf_TILED,	IGT_ROTATION_90 },
> +		{ LOCAL_I915_FORMAT_MOD_Yf_TILED,	IGT_ROTATION_180 },
> +		{ LOCAL_I915_FORMAT_MOD_Yf_TILED,	IGT_ROTATION_270 },
> +	};
> +	const uint32_t formats[] = {
> +		DRM_FORMAT_RGB565,
> +		DRM_FORMAT_XRGB8888,
> +		DRM_FORMAT_ARGB8888,
> +	};
> +
> +	igt_fb_t reference_fb;
> +	igt_crc_t reference_crc;
> +
> +	for (int plane = 0; plane < clip_squares_cnt; plane++) {
> +		igt_info("Testing connector %s mode %dx%d using pipe %s: %dx%d@%d,%d\n",
> +			 igt_output_name(output),
> +			 mode->hdisplay, mode->vdisplay,
> +			 kmstest_pipe_name(pipe),
> +			 clip_squares[plane].size, clip_squares[plane].size,
> +			 clip_squares[plane].x, clip_squares[plane].y);
> +	}
> +
> +	test_init(data, pipe);
> +
> +	create_fb_for_mode__clipping_display(data, mode,
> +					     clip_squares,
> +					     clip_squares_cnt,
> +					     &reference_fb);
> +
> +	test_grab_crc_for_fb2(data, output, pipe, &reference_fb,
> +			      &reference_crc, COMMIT_UNIVERSAL);

Hm not sure why legacy commit wouldn't work here. It will result in a
modeset with only the primary plane on without rotation. Otoh universal
commit doesn't (or at least used not to) support full modesets, so if
we'd have to change the mode here then that wouldn't work. Did you try
to run the test with multiple outputs?

> +
> +	igt_remove_fb(data->drm_fd, &reference_fb);
> +
> +	for (int rotation = 0; rotation < ARRAY_SIZE(rotations); rotation++)
> +		for (int format = 0; format < ARRAY_SIZE(formats); format++)
> +			test_plane_clipping_format(data, pipe, output,
> +						   mode,
> +						   clip_squares,
> +						   clip_squares_cnt,
> +						   rotations[rotation].tiling,
> +						   rotations[rotation].rotation,
> +						   formats[format],
> +						   &reference_crc);
> +
> +	test_fini(data);
> +}
> +
> +static void
> +test_plane_clipping_squares(data_t *data, enum pipe pipe,
> +			    igt_output_t *output, drmModeModeInfo *mode,
> +			    square_t *clip_squares, int clip_squares_cnt)

Imo s/clip_squares_cnt/plane_cnt/ would be clearer.

> +{
> +	switch (clip_squares_cnt) {
> +	case 4:
> +	    test_plane_clipping_square(data, pipe, output, mode,
> +				       clip_squares, clip_squares_cnt);
> +	    break;
> +	case 2:
> +	    test_plane_clipping_square(data, pipe, output, mode,
> +				       clip_squares, clip_squares_cnt);
> +	    test_plane_clipping_square(data, pipe, output, mode,
> +				       clip_squares + clip_squares_cnt,
> +				       clip_squares_cnt);
> +	    break;
> +	case 1:
> +	default:
> +	    for (int i = 0; i  < 4; i++) {
> +		test_plane_clipping_square(data, pipe, output, mode,
> +					   clip_squares + i,
> +					   clip_squares_cnt);
> +	    }
> +	    break;
> +	}

Could be just

	for (i = 0; i < 4; i += clip_squares_cnt)
		test_plane_clipping_square(data, pipe, output, mode,
					   clip_squares + i,
					   clip_squares_cnt);
?

> +}
> +
> +static void
> +test_plane_clipping(data_t *data, enum pipe pipe)
> +{
> +	igt_output_t *output;
> +	int connected_outs = 0;
> +
> +	for_each_valid_output_on_pipe(&data->display, pipe, output) {
> +		drmModeModeInfo *mode;
> +		int sq_size;
> +		int max_planes = data->display.pipes[pipe].n_planes;
> +		int clip_squares_cnt;
> +
> +		clip_squares_cnt = max_planes >= 4 ?
> +				   4 : max_planes >= 2 ? 2 : 1;
> +
> +		igt_output_set_pipe(output, pipe);
> +		mode = igt_output_get_mode(output);
> +
> +		/*
> +		 * Test with a square plane bigger than either the width or
> +		 * height of the mode. This should pass on all platforms.
> +		 */
> +		sq_size = mode->hdisplay > mode->vdisplay ?
> +			  mode->hdisplay : mode->vdisplay;
> +
> +		clip_squares[0].x = -2;
> +		clip_squares[0].y = -2;
> +		clip_squares[0].size = sq_size + 4;
> +
> +		test_plane_clipping_square(data, pipe, output, mode,
> +					   clip_squares, 1);
> +
> +		/*
> +		 * Test positions in the 4 corners of the screen with a
> +		 * CLIPPED_PLANE_MIN_VALID x CLIPPED_PLANE_MIN_VALID square
> +		 * visible from the plane. These should pass on all platforms.
> +		 */
> +		clip_squares[0].x = -CLIPPED_PLANE_SMALL_SIZE +
> +				     CLIPPED_PLANE_MIN_VALID;
> +		clip_squares[0].y = -CLIPPED_PLANE_SMALL_SIZE +
> +				     CLIPPED_PLANE_MIN_VALID;
> +		clip_squares[0].size = CLIPPED_PLANE_SMALL_SIZE;
> +
> +		clip_squares[1].x = -CLIPPED_PLANE_SMALL_SIZE +
> +				     CLIPPED_PLANE_MIN_VALID;
> +		clip_squares[1].y = mode->vdisplay -
> +				    CLIPPED_PLANE_MIN_VALID;
> +		clip_squares[1].size = CLIPPED_PLANE_SMALL_SIZE;
> +
> +		clip_squares[2].x = mode->hdisplay -
> +				    CLIPPED_PLANE_MIN_VALID;
> +		clip_squares[2].y = -CLIPPED_PLANE_SMALL_SIZE +
> +				     CLIPPED_PLANE_MIN_VALID;
> +		clip_squares[2].size = CLIPPED_PLANE_SMALL_SIZE;
> +
> +		clip_squares[3].x = mode->hdisplay -
> +				    CLIPPED_PLANE_MIN_VALID;
> +		clip_squares[3].y = mode->vdisplay -
> +				    CLIPPED_PLANE_MIN_VALID;
> +		clip_squares[3].size = CLIPPED_PLANE_SMALL_SIZE;
> +
> +		test_plane_clipping_squares(data, pipe, output, mode,
> +					    clip_squares, clip_squares_cnt);
> +
> +		/*
> +		 * Test positions in the 4 corners of the screen with a
> +		 * 2 x 2 square visible from the plane. These are valid on all
> +		 * platforms except on GLK/CNL where less than
> +		 * CLIPPED_PLANE_MIN_VALID pixels visible on the left/right
> +		 * edges of the screen may cause FIFO underflow and display
> +		 * corruption.
> +		 *
> +		 * The cursor plane is an exception without this problem.
> +		 *
> +		 * Use 2 x 2 size as other (odd) sizes may result in an
> +		 * incorrect CRC for the cursor plane even though it displays
> +		 * correctly and causes no underflow.
> +		 */
> +		clip_squares[0].x = -CLIPPED_PLANE_SMALL_SIZE + 2;
> +		clip_squares[0].y = -CLIPPED_PLANE_SMALL_SIZE + 2;
> +		clip_squares[0].size = CLIPPED_PLANE_SMALL_SIZE;
> +
> +		clip_squares[1].x = -CLIPPED_PLANE_SMALL_SIZE + 2;
> +		clip_squares[1].y = mode->vdisplay - 2;
> +		clip_squares[1].size = CLIPPED_PLANE_SMALL_SIZE;
> +
> +		clip_squares[2].x = mode->hdisplay - 2;
> +		clip_squares[2].y = -CLIPPED_PLANE_SMALL_SIZE + 2;
> +		clip_squares[2].size = CLIPPED_PLANE_SMALL_SIZE;
> +
> +		clip_squares[3].x = mode->hdisplay - 2;
> +		clip_squares[3].y = mode->vdisplay - 2;
> +		clip_squares[3].size = CLIPPED_PLANE_SMALL_SIZE;
> +
> +		test_plane_clipping_squares(data, pipe, output, mode,
> +					    clip_squares, clip_squares_cnt);
> +
> +		connected_outs++;
> +	}
> +
> +	igt_skip_on(connected_outs == 0);
> +}
> +
>  static void
>  run_tests_for_pipe_plane(data_t *data, enum pipe pipe)
>  {
> @@ -590,6 +1103,9 @@ run_tests_for_pipe_plane(data_t *data, enum pipe pipe)
>  		      kmstest_pipe_name(pipe))
>  		test_plane_panning(data, pipe, TEST_PANNING_BOTTOM_RIGHT |
>  					       TEST_SUSPEND_RESUME);
> +	igt_subtest_f("plane-clipping-pipe-%s-planes",
> +		      kmstest_pipe_name(pipe))
> +		test_plane_clipping(data, pipe);
>  }
>  
>  
> @@ -602,8 +1118,21 @@ igt_main
>  	igt_skip_on_simulation();
>  
>  	igt_fixture {
> +		int ret;
> +
>  		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
>  
> +		data.devid = intel_get_drm_devid(data.drm_fd);
> +
> +		data.max_curw = 64;
> +		data.max_curh = 64;
> +		ret = drmGetCap(data.drm_fd, DRM_CAP_CURSOR_WIDTH,
> +				&data.max_curw);
> +		igt_assert(ret == 0 || errno == EINVAL);
> +		ret = drmGetCap(data.drm_fd, DRM_CAP_CURSOR_HEIGHT,
> +				&data.max_curh);
> +		igt_assert(ret == 0 || errno == EINVAL);
> +
>  		kmstest_set_vt_graphics_mode();
>  
>  		igt_require_pipe_crc(data.drm_fd);
> -- 
> 2.18.0
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [RESEND,v3 5/5] kms_plane: Add clipping subtests

[Mun, Gwan-gyeong]

Hi Imre,

Thanks for reviewing patches.

On Fri, 2018-09-07 at 13:48 +0300, Imre Deak wrote:
> Hi G.G.,
> 
> Thanks for doing this, looks ok in general. Few comments below.
> 
> On Thu, Sep 06, 2018 at 10:24:00AM +0300, Gwan-gyeong Mun wrote:
> > From: Imre Deak <imre.deak@intel.com>
> > 
> > Add plane clipping subtests displaying a single clipped plane, with
> > the
> > following test cases:
> > a) plane covering the whole screen, so that clipping is done at all
> > 4
> >    screen edges
> > b) plane at either of the 4 corners of the screen clipped, so that
> > a
> >    4x4 pixel part of the plane is visible
> > c) plane at either of the 4 corners of the screen clipped, so that
> > a
> >    2x2 pixel part of the plane is visible
> > 
> > Each of the above cases are tested with all supported tiling modes,
> > rotation degrees, differing pixel format sizes (only 16 bpp and 32
> > bpp
> > for now) and certain planes. While the a) and b) cases above are
> > supported on all platforms c) is not fully supported on GLK and CNL
> > (which was the primary motivation for this testcase).
> > 
> > v2:
> > - Add missing reset for the output pipe after finishing the test
> > for a
> >   given output, fixing the
> >   "DP-1 and eDP-1 are both trying to use pipe A" type of errors.
> > - Use -ERANGE instead of -EINVAL to check the return code for
> >   unsupported plane X positions, based on the latest kernel code.
> > - Add comment explaining the dependencies when doing a universal
> >   commit.
> > 
> > v3: (Gwan-gyeong)
> > - Add missing release of framebuffer on
> > create_fb_for_mode__clipping_display()
> >   function.
> > - Add using multi planes per single display commit on clipping test
> > of
> >   4 corners. (Daniel)
> >   It enables the improving of test speed. If number of planes is
> > over
> >   than 4, it uses four planes from first plane to next four planes.
> >   (4 planes make testing of 4 corners possible as 1 display
> > commit.)
> >   If number of planes is over than 2 and less than 4, it uses two
> > planes
> >   from first plane to next two planes. (2 planes make testing of 4
> > corners
> >   possible as 2 display commit.) Rest of cases it uses only first
> > plane.
> >   (1 plane makes testing of 4 corners possible as 4 display
> > commit.)
> >   It changes to using of certain planes from using of all supported
> > planes.
> >   And a cursor plane can be one of test planes.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > Cc: Mika Kahola <mika.kahola@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Juha-Pekka Heikkilä <juha-pekka.heikkila@intel.com>
> > Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> (v1)
> > ---
> >  tests/kms_plane.c | 529
> > ++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 529 insertions(+)
> > 
> > diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> > index 3d08893f..ea62efed 100644
> > --- a/tests/kms_plane.c
> > +++ b/tests/kms_plane.c
> > @@ -41,11 +41,40 @@ typedef struct {
> >  	int drm_fd;
> >  	igt_display_t display;
> >  	igt_pipe_crc_t *pipe_crc;
> > +	uint32_t devid;
> > +	uint64_t max_curw;
> > +	uint64_t max_curh;
> >  } data_t;
> >  
> > +typedef struct {
> > +	int x;
> > +	int y;
> > +	int size;
> > +} square_t;
> > +
> >  static color_t red   = { 1.0f, 0.0f, 0.0f };
> >  static color_t green = { 0.0f, 1.0f, 0.0f };
> > +static color_t yellow = { 1.0f, 1.0f, 0.0f };
> >  static color_t blue  = { 0.0f, 0.0f, 1.0f };
> > +static color_t white = { 1.0f, 1.0f, 1.0f };
> > +static square_t clip_squares[4];
> > +
> > +/*
> > + * Size of a square plane used to test clipping at the 4 courners
> > of the
> > + * display.
> > + */
> > +#define CLIPPED_PLANE_SMALL_SIZE	64
> > +
> > +/*
> > + * Visible plane size after clipping that works on all platforms
> > for all plane
> > + * positions.
> > + * The exceptions are GLK/CNL where there must be at least this
> > many pixels
> > + * visible from the plane after it's clipped to the left/right
> > edge of the
> > + * screen. Not meeting this condition may trigger FIFO underflows
> > and screen
> > + * corruption. The cursor plane is an exception that doesn't have
> > this problem
> > + * even on GLK/CNL.
> > + */
> > +#define CLIPPED_PLANE_MIN_VALID		4
> >  
> >  /*
> >   * Common code across all tests, acting on data_t
> > @@ -151,6 +180,69 @@ create_fb_for_mode__position(data_t *data,
> > drmModeModeInfo *mode,
> >  	igt_put_cairo_ctx(data->drm_fd, fb, cr);
> >  }
> >  
> > +/*
> > + * Create a square FB for the plane in the clipping test, divided
> > into 4
> > + * quarters solid filled with different colors. Use the given
> > tiling, format
> > + * and size and rotate the FB clockwise with the given rotation
> > degrees, so
> > + * that the counterclockwise rotation with the same degrees done
> > by the HW
> > + * will always result in the same reference FB image.
> > + */
> > +static void
> > +create_fb_for_mode__clipping_plane(data_t *data, igt_rotation_t
> > rotation,
> > +				   uint64_t tiling,
> > +				   uint32_t format,
> > +				   int size,
> > +				   struct igt_fb *fb /* out */)
> > +{
> > +	color_t corners[] = { red, white, yellow, blue };
> > +	color_t color;
> > +	unsigned int fb_id;
> > +	cairo_t *cr;
> > +	const int qsize = size / 2;
> > +	int idx;
> > +
> > +	fb_id = igt_create_fb(data->drm_fd, size, size, format, tiling,
> > fb);
> > +	igt_assert(fb_id);
> > +
> > +	cr = igt_get_cairo_ctx(data->drm_fd, fb);
> > +
> > +	switch (rotation) {
> > +	case IGT_ROTATION_0:
> > +		idx = 0;
> > +		break;
> > +	case IGT_ROTATION_90:
> > +		idx = 3;
> > +		break;
> > +	case IGT_ROTATION_180:
> > +		idx = 2;
> > +		break;
> > +	case IGT_ROTATION_270:
> > +		idx = 1;
> > +		break;
> > +	default:
> > +		igt_assert(0);
> > +	}
> > +
> > +	color = corners[idx];
> > +	igt_paint_color(cr, 0, 0, qsize, qsize,
> > +			color.red, color.green, color.blue);
> > +
> > +	color = corners[(++idx) % 4];
> > +	igt_paint_color(cr, qsize, 0, qsize, qsize,
> > +			color.red, color.green, color.blue);
> > +
> > +	color = corners[(++idx) % 4];
> > +	igt_paint_color(cr, qsize, qsize, qsize, qsize,
> > +			color.red, color.green, color.blue);
> > +
> > +	color = corners[(++idx) % 4];
> > +	igt_paint_color(cr, 0, qsize, qsize, qsize,
> > +			color.red, color.green, color.blue);
> > +
> > +	igt_assert(cairo_status(cr) == 0);
> > +	cairo_destroy(cr);
> > +}
> > +
> >  enum {
> >  	TEST_POSITION_FULLY_COVERED = 1 << 0,
> >  	TEST_DPMS = 1 << 1,
> > @@ -554,6 +646,427 @@ test_pixel_formats(data_t *data, enum pipe
> > pipe)
> >  	}
> >  }
> >  
> > +static bool
> > +bogus_plane_conf(uint32_t devid, igt_plane_t *plane_obj,
> > +		 int hdisplay, int plane_x, int plane_width)
> > +{
> > +	if (!(IS_GEMINILAKE(devid) || IS_CANNONLAKE(devid)))
> > +		return false;
> > +
> > +	if (plane_obj->type == DRM_PLANE_TYPE_CURSOR)
> > +		return false;
> > +
> > +	if (plane_x + plane_width >= CLIPPED_PLANE_MIN_VALID &&
> > +	    plane_x <= hdisplay - CLIPPED_PLANE_MIN_VALID)
> > +		return false;
> > +
> > +	return true;
> > +}
> > +
> > +static bool
> > +supported_plane_conf(data_t *data, int plane_type, uint64_t
> > tiling,
> > +		     uint32_t format, igt_rotation_t rotation,
> > +		     drmModeModeInfo *mode,
> > +		     int plane_x, int plane_y, int plane_size)
> > +{
> > +	if (intel_gen(data->devid) < 9 &&
> > +	    tiling != LOCAL_DRM_FORMAT_MOD_NONE)
> > +		return false;
> > +
> > +	/* On GEN<9 the primary plane must cover the full screen. */
> > +	if (intel_gen(data->devid) < 9 &&
> > +	    plane_type == DRM_PLANE_TYPE_PRIMARY &&
> > +	    (plane_x > 0 || plane_y > 0 ||
> > +	     (plane_x + plane_size < mode->hdisplay) ||
> > +	     (plane_y + plane_size < mode->vdisplay)))
> > +		return false;
> > +
> > +	if (plane_type == DRM_PLANE_TYPE_CURSOR) {
> > +		/* For cursor planes only linear/alpha format is
> > supported. */
> > +		if (tiling != LOCAL_DRM_FORMAT_MOD_NONE ||
> > +		    format != DRM_FORMAT_ARGB8888)
> > +			return false;
> > +
> > +		if (plane_size > data->max_curw || plane_size > data-
> > >max_curh)
> > +			return false;
> > +	} else {
> > +		/*
> > +		 * For non-cursor planes formats with alpha may result
> > in
> > +		 * undeterministic CRCs, we use the same sized
> > +		 * non-alpha XRGB8888 format instead.
> > +		 */
> > +		if (format == DRM_FORMAT_ARGB8888)
> > +			return false;
> > +
> > +		if (format == DRM_FORMAT_RGB565 &&
> > +		    intel_gen(data->devid) < 9 &&
> > +		    !IS_VALLEYVIEW(data->devid) && !IS_CHERRYVIEW(data-
> > >devid))
> > +			return false;
> > +	}
> > +
> > +	/* RGB565 with rotation is not supported for now. */
> > +	if (format == DRM_FORMAT_RGB565 && rotation != IGT_ROTATION_0)
> > +		return false;
> > +
> > +	return true;
> > +}
> > +
> > +/*
> > + * Create a square reference FB for the whole screen in the
> > clipping test,
> > + * with the given test plane position and size. See
> > + * create_fb_for_mode__clipping_plane() for the layout of the test
> > plane.
> > + */
> > +static void
> > +create_fb_for_mode__clipping_display(data_t *data, drmModeModeInfo
> > *mode,
> > +				     square_t *clip_squares,
> > +				     int clip_squares_cnt,
> > +				     struct igt_fb *fb /* out */)
> > +{
> > +	int plane;
> > +	struct igt_fb plane_fbs[4];
> > +	unsigned int fb_id;
> > +	cairo_t *cr;
> > +	cairo_surface_t *srcs[4];
> > +
> > +	fb_id = igt_create_fb(data->drm_fd,
> > +			      mode->hdisplay, mode->vdisplay,
> > +			      DRM_FORMAT_XRGB8888,
> > +			      LOCAL_DRM_FORMAT_MOD_NONE,
> > +			      fb);
> > +	igt_assert(fb_id);
> > +
> > +	cr = igt_get_cairo_ctx(data->drm_fd, fb);
> > +	igt_paint_color(cr, 0, 0, mode->hdisplay, mode->vdisplay,
> > +			0, 0, 0);
> > +
> > +	for (plane = 0; plane < clip_squares_cnt; plane++) {
> > +		create_fb_for_mode__clipping_plane(data,
> > IGT_ROTATION_0,
> > +						   LOCAL_DRM_FORMAT_MOD
> > _NONE,
> > +						   DRM_FORMAT_XRGB8888,
> > +						   clip_squares[plane].
> > size,
> > +						   &plane_fbs[plane]);
> > +
> > +		srcs[plane] = igt_get_cairo_surface(data->drm_fd,
> > +						   &plane_fbs[plane]);
> > +		cairo_set_source_surface(cr, srcs[plane],
> > +					 clip_squares[plane].x,
> > +					 clip_squares[plane].y);
> > +		cairo_rectangle(cr,
> > +				clip_squares[plane].x,
> > +				clip_squares[plane].y,
> > +				clip_squares[plane].size,
> > +				clip_squares[plane].size);
> > +		cairo_fill(cr);
> 
> Could remove the FB, cairo surface already here, so we don't need the
> array and a second loop below?
> 
if we remove the FB and cairo surface here, we will receive SIGABRT
like this,

kms_plane: cairo-surface.c:955: cairo_surface_destroy: Assertion
`CAIRO_REFERENCE_COUNT_HAS_REFERENCE (&surface->ref_count)' failed.
Received signal SIGABRT.

And when we call cairo_destroy(), a cairo image surface of igt calls
blit. 

> > +	}
> > +
> > +	igt_assert(cairo_status(cr) == 0);
> > +	cairo_destroy(cr);
> > +
> > +	for (plane = 0; plane < clip_squares_cnt; plane++) {
> > +		cairo_surface_destroy(srcs[plane]);
> > +		igt_remove_fb(data->drm_fd, &plane_fbs[plane]);
> > +	}
> > +}
> > +
> > +static void
> > +test_plane_clipping_format(data_t *data,
> > +			   enum pipe pipe,
> > +			   igt_output_t *output,
> > +			   drmModeModeInfo *mode,
> > +			   square_t *clip_squares,
> > +			   int clip_squares_cnt,
> > +			   uint64_t tiling,
> > +			   igt_rotation_t rotation,
> > +			   uint32_t format,
> > +			   igt_crc_t *reference_crc)
> > +{
> > +	int plane;
> > +	igt_plane_t *plane_objs[4];
> > +	struct igt_fb plane_fbs[4];
> > +	igt_crc_t crc;
> > +	int ret;
> > +	bool bogus_planes = false;
> > +
> > +	memset(plane_objs, 0, sizeof(plane_objs));
> > +
> > +	igt_debug("rotation %s tiling %s format %s\n",
> > +		  igt_rotation_degrees_str(rotation),
> > +		  igt_tiling_str(tiling),
> > +		  igt_format_str(format));
> > +
> > +	igt_output_set_pipe(output, pipe);
> > +
> > +	for (plane = 0; plane < clip_squares_cnt ; plane++) {
> > +		igt_plane_t *obj = igt_output_get_plane(output, plane);
> > +		struct igt_fb *fb = &plane_fbs[plane];
> > +		int x = clip_squares[plane].x;
> > +		int y = clip_squares[plane].y;
> > +		int size = clip_squares[plane].size;
> > +		uint64_t _tiling = tiling;
> > +		uint32_t _format = format;
> > +
> > +		if (obj->type == DRM_PLANE_TYPE_CURSOR) {
> > +			_tiling = LOCAL_DRM_FORMAT_MOD_NONE;
> > +			_format = DRM_FORMAT_ARGB8888;
> > +		}
> > +
> > +		if (!supported_plane_conf(data, obj->type, _tiling,
> > _format,
> > +					  rotation, mode, x, y, size))
> > +			goto out;
> > +
> > +		create_fb_for_mode__clipping_plane(data,
> > rotation,_tiling,
> > +						   _format, size, fb);
> > +
> > +		igt_plane_set_fb(obj, fb);
> > +		igt_fb_set_position(fb, obj, 0, 0);
> > +
> > +		igt_plane_set_size(obj, size, size);
> > +		igt_plane_set_rotation(obj, rotation);
> > +		igt_plane_set_position(obj, x, y);
> > +
> > +		plane_objs[plane] = obj;
> > +	}
> > +
> > +	/*
> > +	 * Note that a universal commit doesn't support full modesets,
> > so we
> > +	 * have to make sure that the following only needs to commit
> > changes
> > +	 * that are compatible with a fastset. This should be
> > guaranteed,
> > +	 * since before calling this function we took the reference CRC
> > which
> > +	 * left the display enabled with the mode we require here and
> > +	 * afterwards we only change plane parameters (FB, position,
> > rotation
> > +	 * etc.).
> > +	 */
> > +	ret = igt_display_try_commit2(&data->display,
> > COMMIT_UNIVERSAL);
> > +
> > +	for (plane = 0; plane < clip_squares_cnt; plane++) {
> > +		if(!bogus_plane_conf(data->devid, plane_objs[plane],
> > +		   mode->hdisplay, clip_squares[plane].x,
> > +		   clip_squares[plane].size)) {
> 
> Please check formatting (kernel style).
> 
Ok, I'll update it

> > +			bogus_planes = true;
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (bogus_planes) {
> > +		igt_assert(ret == 0);
> > +		igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
> > +		igt_assert_crc_equal(reference_crc, &crc);
> > +	} else {
> > +		igt_assert(ret == -ERANGE);
> > +	}
> > +
> > +out:
> > +	for (plane = 0; plane < clip_squares_cnt; plane++) {
> > +		if (!plane_objs[plane])
> > +			continue;
> > +		igt_plane_set_fb(plane_objs[plane], NULL);
> > +		igt_plane_set_rotation(plane_objs[plane],
> > IGT_ROTATION_0);
> > +		igt_plane_set_position(plane_objs[plane], 0, 0);
> > +
> > +		igt_remove_fb(data->drm_fd, &plane_fbs[plane]);
> > +	}
> > +
> > +	igt_output_set_pipe(output, PIPE_ANY);
> > +}
> > +
> > +static void
> > +test_plane_clipping_square(data_t *data, enum pipe pipe,
> > +			   igt_output_t *output, drmModeModeInfo *mode,
> > +			   square_t *clip_squares, int
> > clip_squares_cnt)
> > +{
> > +	const struct {
> > +		uint64_t tiling;
> > +		igt_rotation_t rotation;
> > +	} rotations[] = {
> > +		{ LOCAL_DRM_FORMAT_MOD_NONE,		IGT_ROTATIO
> > N_0 },
> > +
> > +		{ LOCAL_I915_FORMAT_MOD_X_TILED,	IGT_ROTATION_0 },
> > +
> > +		{ LOCAL_I915_FORMAT_MOD_Y_TILED,	IGT_ROTATION_0 },
> > +		{ LOCAL_I915_FORMAT_MOD_Y_TILED,	IGT_ROTATION_90 },
> > +		{ LOCAL_I915_FORMAT_MOD_Y_TILED,	IGT_ROTATION_180 },
> > +		{ LOCAL_I915_FORMAT_MOD_Y_TILED,	IGT_ROTATION_270 },
> > +
> > +		{ LOCAL_I915_FORMAT_MOD_Yf_TILED,	IGT_ROTATION_0 },
> > +		{ LOCAL_I915_FORMAT_MOD_Yf_TILED,	IGT_ROTATION_90 },
> > +		{ LOCAL_I915_FORMAT_MOD_Yf_TILED,	IGT_ROTATION_180 },
> > +		{ LOCAL_I915_FORMAT_MOD_Yf_TILED,	IGT_ROTATION_270 },
> > +	};
> > +	const uint32_t formats[] = {
> > +		DRM_FORMAT_RGB565,
> > +		DRM_FORMAT_XRGB8888,
> > +		DRM_FORMAT_ARGB8888,
> > +	};
> > +
> > +	igt_fb_t reference_fb;
> > +	igt_crc_t reference_crc;
> > +
> > +	for (int plane = 0; plane < clip_squares_cnt; plane++) {
> > +		igt_info("Testing connector %s mode %dx%d using pipe
> > %s: %dx%d@%d,%d\n",
> > +			 igt_output_name(output),
> > +			 mode->hdisplay, mode->vdisplay,
> > +			 kmstest_pipe_name(pipe),
> > +			 clip_squares[plane].size,
> > clip_squares[plane].size,
> > +			 clip_squares[plane].x, clip_squares[plane].y);
> > +	}
> > +
> > +	test_init(data, pipe);
> > +
> > +	create_fb_for_mode__clipping_display(data, mode,
> > +					     clip_squares,
> > +					     clip_squares_cnt,
> > +					     &reference_fb);
> > +
> > +	test_grab_crc_for_fb2(data, output, pipe, &reference_fb,
> > +			      &reference_crc, COMMIT_UNIVERSAL);
> 
> Hm not sure why legacy commit wouldn't work here. It will result in a
> modeset with only the primary plane on without rotation. Otoh
> universal
> commit doesn't (or at least used not to) support full modesets, so if
> we'd have to change the mode here then that wouldn't work. Did you
> try
> to run the test with multiple outputs?
> 
yes, you are right. If I tried to test with multiple outputs,it didn't work as expected. I'll fix it with other solution which you guided(resetting a first commit state and calling commit with legacy style).

> > +
> > +	igt_remove_fb(data->drm_fd, &reference_fb);
> > +
> > +	for (int rotation = 0; rotation < ARRAY_SIZE(rotations);
> > rotation++)
> > +		for (int format = 0; format < ARRAY_SIZE(formats);
> > format++)
> > +			test_plane_clipping_format(data, pipe, output,
> > +						   mode,
> > +						   clip_squares,
> > +						   clip_squares_cnt,
> > +						   rotations[rotation].
> > tiling,
> > +						   rotations[rotation].
> > rotation,
> > +						   formats[format],
> > +						   &reference_crc);
> > +
> > +	test_fini(data);
> > +}
> > +
> > +static void
> > +test_plane_clipping_squares(data_t *data, enum pipe pipe,
> > +			    igt_output_t *output, drmModeModeInfo
> > *mode,
> > +			    square_t *clip_squares, int
> > clip_squares_cnt)
> 
> Imo s/clip_squares_cnt/plane_cnt/ would be clearer.
> 
Ok I'll update it.

> > +{
> > +	switch (clip_squares_cnt) {
> > +	case 4:
> > +	    test_plane_clipping_square(data, pipe, output, mode,
> > +				       clip_squares, clip_squares_cnt);
> > +	    break;
> > +	case 2:
> > +	    test_plane_clipping_square(data, pipe, output, mode,
> > +				       clip_squares, clip_squares_cnt);
> > +	    test_plane_clipping_square(data, pipe, output, mode,
> > +				       clip_squares + clip_squares_cnt,
> > +				       clip_squares_cnt);
> > +	    break;
> > +	case 1:
> > +	default:
> > +	    for (int i = 0; i  < 4; i++) {
> > +		test_plane_clipping_square(data, pipe, output, mode,
> > +					   clip_squares + i,
> > +					   clip_squares_cnt);
> > +	    }
> > +	    break;
> > +	}
> 
> Could be just
> 
> 	for (i = 0; i < 4; i += clip_squares_cnt)
> 		test_plane_clipping_square(data, pipe, output, mode,
> 					   clip_squares + i,
> 					   clip_squares_cnt);
> ?
> 

Ok I'll update it.

> > +}
> > +
> > +static void
> > +test_plane_clipping(data_t *data, enum pipe pipe)
> > +{
> > +	igt_output_t *output;
> > +	int connected_outs = 0;
> > +
> > +	for_each_valid_output_on_pipe(&data->display, pipe, output) {
> > +		drmModeModeInfo *mode;
> > +		int sq_size;
> > +		int max_planes = data->display.pipes[pipe].n_planes;
> > +		int clip_squares_cnt;
> > +
> > +		clip_squares_cnt = max_planes >= 4 ?
> > +				   4 : max_planes >= 2 ? 2 : 1;
> > +
> > +		igt_output_set_pipe(output, pipe);
> > +		mode = igt_output_get_mode(output);
> > +
> > +		/*
> > +		 * Test with a square plane bigger than either the
> > width or
> > +		 * height of the mode. This should pass on all
> > platforms.
> > +		 */
> > +		sq_size = mode->hdisplay > mode->vdisplay ?
> > +			  mode->hdisplay : mode->vdisplay;
> > +
> > +		clip_squares[0].x = -2;
> > +		clip_squares[0].y = -2;
> > +		clip_squares[0].size = sq_size + 4;
> > +
> > +		test_plane_clipping_square(data, pipe, output, mode,
> > +					   clip_squares, 1);
> > +
> > +		/*
> > +		 * Test positions in the 4 corners of the screen with a
> > +		 * CLIPPED_PLANE_MIN_VALID x CLIPPED_PLANE_MIN_VALID
> > square
> > +		 * visible from the plane. These should pass on all
> > platforms.
> > +		 */
> > +		clip_squares[0].x = -CLIPPED_PLANE_SMALL_SIZE +
> > +				     CLIPPED_PLANE_MIN_VALID;
> > +		clip_squares[0].y = -CLIPPED_PLANE_SMALL_SIZE +
> > +				     CLIPPED_PLANE_MIN_VALID;
> > +		clip_squares[0].size = CLIPPED_PLANE_SMALL_SIZE;
> > +
> > +		clip_squares[1].x = -CLIPPED_PLANE_SMALL_SIZE +
> > +				     CLIPPED_PLANE_MIN_VALID;
> > +		clip_squares[1].y = mode->vdisplay -
> > +				    CLIPPED_PLANE_MIN_VALID;
> > +		clip_squares[1].size = CLIPPED_PLANE_SMALL_SIZE;
> > +
> > +		clip_squares[2].x = mode->hdisplay -
> > +				    CLIPPED_PLANE_MIN_VALID;
> > +		clip_squares[2].y = -CLIPPED_PLANE_SMALL_SIZE +
> > +				     CLIPPED_PLANE_MIN_VALID;
> > +		clip_squares[2].size = CLIPPED_PLANE_SMALL_SIZE;
> > +
> > +		clip_squares[3].x = mode->hdisplay -
> > +				    CLIPPED_PLANE_MIN_VALID;
> > +		clip_squares[3].y = mode->vdisplay -
> > +				    CLIPPED_PLANE_MIN_VALID;
> > +		clip_squares[3].size = CLIPPED_PLANE_SMALL_SIZE;
> > +
> > +		test_plane_clipping_squares(data, pipe, output, mode,
> > +					    clip_squares,
> > clip_squares_cnt);
> > +
> > +		/*
> > +		 * Test positions in the 4 corners of the screen with a
> > +		 * 2 x 2 square visible from the plane. These are valid
> > on all
> > +		 * platforms except on GLK/CNL where less than
> > +		 * CLIPPED_PLANE_MIN_VALID pixels visible on the
> > left/right
> > +		 * edges of the screen may cause FIFO underflow and
> > display
> > +		 * corruption.
> > +		 *
> > +		 * The cursor plane is an exception without this
> > problem.
> > +		 *
> > +		 * Use 2 x 2 size as other (odd) sizes may result in an
> > +		 * incorrect CRC for the cursor plane even though it
> > displays
> > +		 * correctly and causes no underflow.
> > +		 */
> > +		clip_squares[0].x = -CLIPPED_PLANE_SMALL_SIZE + 2;
> > +		clip_squares[0].y = -CLIPPED_PLANE_SMALL_SIZE + 2;
> > +		clip_squares[0].size = CLIPPED_PLANE_SMALL_SIZE;
> > +
> > +		clip_squares[1].x = -CLIPPED_PLANE_SMALL_SIZE + 2;
> > +		clip_squares[1].y = mode->vdisplay - 2;
> > +		clip_squares[1].size = CLIPPED_PLANE_SMALL_SIZE;
> > +
> > +		clip_squares[2].x = mode->hdisplay - 2;
> > +		clip_squares[2].y = -CLIPPED_PLANE_SMALL_SIZE + 2;
> > +		clip_squares[2].size = CLIPPED_PLANE_SMALL_SIZE;
> > +
> > +		clip_squares[3].x = mode->hdisplay - 2;
> > +		clip_squares[3].y = mode->vdisplay - 2;
> > +		clip_squares[3].size = CLIPPED_PLANE_SMALL_SIZE;
> > +
> > +		test_plane_clipping_squares(data, pipe, output, mode,
> > +					    clip_squares,
> > clip_squares_cnt);
> > +
> > +		connected_outs++;
> > +	}
> > +
> > +	igt_skip_on(connected_outs == 0);
> > +}
> > +
> >  static void
> >  run_tests_for_pipe_plane(data_t *data, enum pipe pipe)
> >  {
> > @@ -590,6 +1103,9 @@ run_tests_for_pipe_plane(data_t *data, enum
> > pipe pipe)
> >  		      kmstest_pipe_name(pipe))
> >  		test_plane_panning(data, pipe,
> > TEST_PANNING_BOTTOM_RIGHT |
> >  					       TEST_SUSPEND_RESUME);
> > +	igt_subtest_f("plane-clipping-pipe-%s-planes",
> > +		      kmstest_pipe_name(pipe))
> > +		test_plane_clipping(data, pipe);
> >  }
> >  
> >  
> > @@ -602,8 +1118,21 @@ igt_main
> >  	igt_skip_on_simulation();
> >  
> >  	igt_fixture {
> > +		int ret;
> > +
> >  		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> >  
> > +		data.devid = intel_get_drm_devid(data.drm_fd);
> > +
> > +		data.max_curw = 64;
> > +		data.max_curh = 64;
> > +		ret = drmGetCap(data.drm_fd, DRM_CAP_CURSOR_WIDTH,
> > +				&data.max_curw);
> > +		igt_assert(ret == 0 || errno == EINVAL);
> > +		ret = drmGetCap(data.drm_fd, DRM_CAP_CURSOR_HEIGHT,
> > +				&data.max_curh);
> > +		igt_assert(ret == 0 || errno == EINVAL);
> > +
> >  		kmstest_set_vt_graphics_mode();
> >  
> >  		igt_require_pipe_crc(data.drm_fd);
> > -- 
> > 2.18.0
> > 

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [RESEND,v3 5/5] kms_plane: Add clipping subtests

[Imre Deak]

On Fri, Sep 07, 2018 at 06:18:43PM +0300, Mun, Gwan-gyeong wrote:
> > > [...]
> > > +static void
> > > +create_fb_for_mode__clipping_display(data_t *data, drmModeModeInfo
> > > *mode,
> > > +				     square_t *clip_squares,
> > > +				     int clip_squares_cnt,
> > > +				     struct igt_fb *fb /* out */)
> > > +{
> > > +	int plane;
> > > +	struct igt_fb plane_fbs[4];
> > > +	unsigned int fb_id;
> > > +	cairo_t *cr;
> > > +	cairo_surface_t *srcs[4];
> > > +
> > > +	fb_id = igt_create_fb(data->drm_fd,
> > > +			      mode->hdisplay, mode->vdisplay,
> > > +			      DRM_FORMAT_XRGB8888,
> > > +			      LOCAL_DRM_FORMAT_MOD_NONE,
> > > +			      fb);
> > > +	igt_assert(fb_id);
> > > +
> > > +	cr = igt_get_cairo_ctx(data->drm_fd, fb);
> > > +	igt_paint_color(cr, 0, 0, mode->hdisplay, mode->vdisplay,
> > > +			0, 0, 0);
> > > +
> > > +	for (plane = 0; plane < clip_squares_cnt; plane++) {
> > > +		create_fb_for_mode__clipping_plane(data, IGT_ROTATION_0,
> > > +						   LOCAL_DRM_FORMAT_MOD_NONE,
> > > +						   DRM_FORMAT_XRGB8888,
> > > +						   clip_squares[plane].size,
> > > +						   &plane_fbs[plane]);
> > > +
> > > +		srcs[plane] = igt_get_cairo_surface(data->drm_fd,
> > > +						   &plane_fbs[plane]);
> > > +		cairo_set_source_surface(cr, srcs[plane],
> > > +					 clip_squares[plane].x,
> > > +					 clip_squares[plane].y);
> > > +		cairo_rectangle(cr,
> > > +				clip_squares[plane].x,
> > > +				clip_squares[plane].y,
> > > +				clip_squares[plane].size,
> > > +				clip_squares[plane].size);
> > > +		cairo_fill(cr);
> > 
> > Could remove the FB, cairo surface already here, so we don't need the
> > array and a second loop below?
> > 
> if we remove the FB and cairo surface here, we will receive SIGABRT
> like this,
> 
> kms_plane: cairo-surface.c:955: cairo_surface_destroy: Assertion
> `CAIRO_REFERENCE_COUNT_HAS_REFERENCE (&surface->ref_count)' failed.
> Received signal SIGABRT.
> 
> And when we call cairo_destroy(), a cairo image surface of igt calls
> blit.

Oops, that's true, I forgot about this oddity in the Cairo API. So your
version is fine.

--Imre
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.BAT: failure for kms_plane: Add clipping subtests (rev7)

[Patchwork]

== Series Details ==

Series: kms_plane: Add clipping subtests (rev7)
URL   : https://patchwork.freedesktop.org/series/39467/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4775 -> IGTPW_1799 =

== Summary - FAILURE ==

  Serious unknown changes coming with IGTPW_1799 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1799, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/39467/revisions/7/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1799:

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_selftest@live_evict:
      fi-bsw-n3050:       PASS -> INCOMPLETE

    igt@kms_psr@primary_page_flip:
      fi-kbl-r:           PASS -> FAIL

    
== Known issues ==

  Here are the changes found in IGTPW_1799 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-cnl-psr:         PASS -> DMESG-WARN (fdo#104951)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-blb-e6850:       PASS -> INCOMPLETE (fdo#107718)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_coherency:
      fi-gdg-551:         DMESG-FAIL (fdo#107164) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     FAIL (fdo#103167) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-byt-clapper:     FAIL (fdo#107362, fdo#103191) -> PASS

    igt@kms_psr@cursor_plane_move:
      fi-cnl-psr:         FAIL (fdo#107717) -> PASS

    igt@kms_psr@primary_page_flip:
      fi-cnl-psr:         FAIL (fdo#107336) -> PASS

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951
  fdo#107164 https://bugs.freedesktop.org/show_bug.cgi?id=107164
  fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107717 https://bugs.freedesktop.org/show_bug.cgi?id=107717
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718


== Participating hosts (54 -> 48) ==

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-u 


== Build changes ==

    * IGT: IGT_4630 -> IGTPW_1799

  CI_DRM_4775: 1a2bb6c061217718b972b3f4a74b96b61cf19d0c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1799: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1799/
  IGT_4630: 86686c6e2f7c6f0944bced11550e06d20bc6957f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@kms_plane@plane-clipping-pipe-a-planes
+igt@kms_plane@plane-clipping-pipe-b-planes
+igt@kms_plane@plane-clipping-pipe-c-planes
+igt@kms_plane@plane-clipping-pipe-d-planes
+igt@kms_plane@plane-clipping-pipe-e-planes
+igt@kms_plane@plane-clipping-pipe-f-planes

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1799/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev