All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t 1/5] lib/igt_fb: Unref the renderecopy scratch bos
@ 2019-04-17 20:35 Ville Syrjala
  2019-04-17 20:35 ` [igt-dev] [PATCH i-g-t 2/5] tests/i915/gem_render_copy: Don't leak bos between subtests Ville Syrjala
                   ` (8 more replies)
  0 siblings, 9 replies; 22+ messages in thread
From: Ville Syrjala @ 2019-04-17 20:35 UTC (permalink / raw)
  To: igt-dev

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

We're currently leaking all the temporary bos we construct
for rendercopy. That doesn't go so well when trying to test
with 1GiB framebuffers.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 lib/igt_fb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 6a43fcc4735f..c3ecd7186444 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1647,6 +1647,9 @@ static void rendercopy(struct fb_blit_upload *blit,
 	render_copy(blit->batch, NULL,
 		    &src, 0, 0, dst_fb->plane_width[0], dst_fb->plane_height[0],
 		    &dst, 0, 0);
+
+	drm_intel_bo_unreference(dst.bo);
+	drm_intel_bo_unreference(src.bo);
 }
 
 static void blitcopy(const struct igt_fb *dst_fb,
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t 2/5] tests/i915/gem_render_copy: Don't leak bos between subtests
  2019-04-17 20:35 [igt-dev] [PATCH i-g-t 1/5] lib/igt_fb: Unref the renderecopy scratch bos Ville Syrjala
@ 2019-04-17 20:35 ` Ville Syrjala
  2019-04-17 20:41   ` Chris Wilson
  2019-04-18 12:11   ` [igt-dev] [PATCH i-g-t v2 " Ville Syrjala
  2019-04-17 20:35 ` [igt-dev] [PATCH i-g-t 3/5] lib/intel_batchbuffer: Make blitter asserts more useful Ville Syrjala
                   ` (7 subsequent siblings)
  8 siblings, 2 replies; 22+ messages in thread
From: Ville Syrjala @ 2019-04-17 20:35 UTC (permalink / raw)
  To: igt-dev

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

Unref the bos after one subtest is done. The next subtest
will allocate its own bos.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tests/i915/gem_render_copy.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tests/i915/gem_render_copy.c b/tests/i915/gem_render_copy.c
index 8d62a0f43ad8..a6476a4d2d8d 100644
--- a/tests/i915/gem_render_copy.c
+++ b/tests/i915/gem_render_copy.c
@@ -662,6 +662,13 @@ static void test(data_t *data, uint32_t tiling, uint64_t ccs_modifier)
 
 	if (ccs_modifier)
 		scratch_buf_aux_check(data, &ccs);
+
+	for (int i = 0; i < num_src; i++)
+		drm_intel_bo_unreference(src[i].buf.bo);
+	drm_intel_bo_unreference(dst.bo);
+	if (ccs_modifier)
+		drm_intel_bo_unreference(ccs.bo);
+	drm_intel_bo_unreference(ref.bo);
 }
 
 static int opt_handler(int opt, int opt_index, void *data)
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t 3/5] lib/intel_batchbuffer: Make blitter asserts more useful
  2019-04-17 20:35 [igt-dev] [PATCH i-g-t 1/5] lib/igt_fb: Unref the renderecopy scratch bos Ville Syrjala
  2019-04-17 20:35 ` [igt-dev] [PATCH i-g-t 2/5] tests/i915/gem_render_copy: Don't leak bos between subtests Ville Syrjala
@ 2019-04-17 20:35 ` Ville Syrjala
  2019-04-17 20:46   ` Chris Wilson
  2019-04-18 12:11   ` [igt-dev] [PATCH i-g-t v2 " Ville Syrjala
  2019-04-17 20:35 ` [igt-dev] [PATCH i-g-t 4/5] lib/igt_fb: Nuke redundant rendercopy cairo surface variant Ville Syrjala
                   ` (6 subsequent siblings)
  8 siblings, 2 replies; 22+ messages in thread
From: Ville Syrjala @ 2019-04-17 20:35 UTC (permalink / raw)
  To: igt-dev

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

Use igt_assert_lt/lte for the blitter coord/stride asserts
so that we can see what the offending value was. gcc likes
to optimize the values away so gdb often doesn't help as
much as one would like.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 lib/intel_batchbuffer.c | 54 +++++++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
index 712ce1a32da6..422f3b6186aa 100644
--- a/lib/intel_batchbuffer.c
+++ b/lib/intel_batchbuffer.c
@@ -417,16 +417,16 @@ intel_blt_copy(struct intel_batchbuffer *batch,
 		cmd_bits |= XY_SRC_COPY_BLT_DST_TILED;
 	}
 
-#define CHECK_RANGE(x)	((x) >= 0 && (x) < (1 << 15))
-	igt_assert(CHECK_RANGE(src_x1) && CHECK_RANGE(src_y1) &&
-		   CHECK_RANGE(dst_x1) && CHECK_RANGE(dst_y1) &&
-		   CHECK_RANGE(width) && CHECK_RANGE(height) &&
-		   CHECK_RANGE(src_x1 + width) &&
-		   CHECK_RANGE(src_y1 + height) &&
-		   CHECK_RANGE(dst_x1 + width) &&
-		   CHECK_RANGE(dst_y1 + height) &&
-		   CHECK_RANGE(src_pitch) &&
-		   CHECK_RANGE(dst_pitch));
+#define CHECK_RANGE(x)	do { \
+	igt_assert_lte(0, (x)); \
+	igt_assert_lt((x), (1 << 15)); \
+} while (0)
+	CHECK_RANGE(src_x1); CHECK_RANGE(src_y1);
+	CHECK_RANGE(dst_x1); CHECK_RANGE(dst_y1);
+	CHECK_RANGE(width); CHECK_RANGE(height);
+	CHECK_RANGE(src_x1 + width); CHECK_RANGE(src_y1 + height);
+	CHECK_RANGE(dst_x1 + width); CHECK_RANGE(dst_y1 + height);
+	CHECK_RANGE(src_pitch); CHECK_RANGE(dst_pitch);
 #undef CHECK_RANGE
 
 	br13_bits = 0;
@@ -715,13 +715,16 @@ void igt_blitter_fast_copy__raw(int fd,
 	dword0 = fast_copy_dword0(src_tiling, dst_tiling);
 	dword1 = fast_copy_dword1(src_tiling, dst_tiling, bpp);
 
-#define CHECK_RANGE(x)	((x) >= 0 && (x) < (1 << 15))
-	assert(CHECK_RANGE(src_x) && CHECK_RANGE(src_y) &&
-	       CHECK_RANGE(dst_x) && CHECK_RANGE(dst_y) &&
-	       CHECK_RANGE(width) && CHECK_RANGE(height) &&
-	       CHECK_RANGE(src_x + width) && CHECK_RANGE(src_y + height) &&
-	       CHECK_RANGE(dst_x + width) && CHECK_RANGE(dst_y + height) &&
-	       CHECK_RANGE(src_pitch) && CHECK_RANGE(dst_pitch));
+#define CHECK_RANGE(x)	do { \
+	igt_assert_lte(0, (x)); \
+	igt_assert_lt((x), (1 << 15)); \
+} while (0)
+	CHECK_RANGE(src_x); CHECK_RANGE(src_y);
+	CHECK_RANGE(dst_x); CHECK_RANGE(dst_y);
+	CHECK_RANGE(width); CHECK_RANGE(height);
+	CHECK_RANGE(src_x + width); CHECK_RANGE(src_y + height);
+	CHECK_RANGE(dst_x + width); CHECK_RANGE(dst_y + height);
+	CHECK_RANGE(src_pitch); CHECK_RANGE(dst_pitch);
 #undef CHECK_RANGE
 
 	batch[i++] = dword0;
@@ -791,13 +794,16 @@ void igt_blitter_fast_copy(struct intel_batchbuffer *batch,
 	dword0 = fast_copy_dword0(src->tiling, dst->tiling);
 	dword1 = fast_copy_dword1(src->tiling, dst->tiling, dst->bpp);
 
-#define CHECK_RANGE(x)	((x) >= 0 && (x) < (1 << 15))
-	assert(CHECK_RANGE(src_x) && CHECK_RANGE(src_y) &&
-	       CHECK_RANGE(dst_x) && CHECK_RANGE(dst_y) &&
-	       CHECK_RANGE(width) && CHECK_RANGE(height) &&
-	       CHECK_RANGE(src_x + width) && CHECK_RANGE(src_y + height) &&
-	       CHECK_RANGE(dst_x + width) && CHECK_RANGE(dst_y + height) &&
-	       CHECK_RANGE(src_pitch) && CHECK_RANGE(dst_pitch));
+#define CHECK_RANGE(x)	do { \
+	igt_assert_lte(0, (x)); \
+	igt_assert_lt((x), (1 << 15)); \
+} while (0)
+	CHECK_RANGE(src_x); CHECK_RANGE(src_y);
+	CHECK_RANGE(dst_x); CHECK_RANGE(dst_y);
+	CHECK_RANGE(width); CHECK_RANGE(height);
+	CHECK_RANGE(src_x + width); CHECK_RANGE(src_y + height);
+	CHECK_RANGE(dst_x + width); CHECK_RANGE(dst_y + height);
+	CHECK_RANGE(src_pitch); CHECK_RANGE(dst_pitch);
 #undef CHECK_RANGE
 
 	BEGIN_BATCH(10, 2);
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t 4/5] lib/igt_fb: Nuke redundant rendercopy cairo surface variant
  2019-04-17 20:35 [igt-dev] [PATCH i-g-t 1/5] lib/igt_fb: Unref the renderecopy scratch bos Ville Syrjala
  2019-04-17 20:35 ` [igt-dev] [PATCH i-g-t 2/5] tests/i915/gem_render_copy: Don't leak bos between subtests Ville Syrjala
  2019-04-17 20:35 ` [igt-dev] [PATCH i-g-t 3/5] lib/intel_batchbuffer: Make blitter asserts more useful Ville Syrjala
@ 2019-04-17 20:35 ` Ville Syrjala
  2019-04-17 20:47   ` Chris Wilson
  2019-04-18 12:12   ` [igt-dev] [PATCH i-g-t v2 " Ville Syrjala
  2019-04-17 20:35 ` [igt-dev] [PATCH i-g-t 5/5] lib/igt_fb: Fix blitter limit checks Ville Syrjala
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 22+ messages in thread
From: Ville Syrjala @ 2019-04-17 20:35 UTC (permalink / raw)
  To: igt-dev

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

The blit and rendercopy implementations are now identical.
Kill one.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 lib/igt_fb.c | 41 +----------------------------------------
 1 file changed, 1 insertion(+), 40 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index c3ecd7186444..794f0eef04a3 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1714,17 +1714,6 @@ static void destroy_cairo_surface__blit(void *arg)
 	free(blit);
 }
 
-static void destroy_cairo_surface__rendercopy(void *arg)
-{
-	struct fb_blit_upload *blit = arg;
-
-	blit->fb->cairo_surface = NULL;
-
-	free_linear_mapping(blit);
-
-	free(blit);
-}
-
 static void setup_linear_mapping(struct fb_blit_upload *blit)
 {
 	int fd = blit->fd;
@@ -1795,32 +1784,6 @@ static void create_cairo_surface__blit(int fd, struct igt_fb *fb)
 				    blit, destroy_cairo_surface__blit);
 }
 
-static void create_cairo_surface__rendercopy(int fd, struct igt_fb *fb)
-{
-	struct fb_blit_upload *blit;
-	cairo_format_t cairo_format;
-
-	blit = calloc(1, sizeof(*blit));
-	igt_assert(blit);
-
-	blit->fd = fd;
-	blit->fb = fb;
-
-	setup_linear_mapping(blit);
-
-	cairo_format = drm_format_to_cairo(fb->drm_format);
-	fb->cairo_surface =
-		cairo_image_surface_create_for_data(blit->linear.map,
-						    cairo_format,
-						    fb->width, fb->height,
-						    blit->linear.fb.strides[0]);
-	fb->domain = I915_GEM_DOMAIN_GTT;
-
-	cairo_surface_set_user_data(fb->cairo_surface,
-				    (cairo_user_data_key_t *)create_cairo_surface__rendercopy,
-				    blit, destroy_cairo_surface__rendercopy);
-}
-
 /**
  * igt_dirty_fb:
  * @fd: open drm file descriptor
@@ -2880,9 +2843,7 @@ cairo_surface_t *igt_get_cairo_surface(int fd, struct igt_fb *fb)
 		    ((f->cairo_id == CAIRO_FORMAT_INVALID) &&
 		     (f->pixman_id != PIXMAN_invalid)))
 			create_cairo_surface__convert(fd, fb);
-		else if (use_rendercopy(fb))
-			create_cairo_surface__rendercopy(fd, fb);
-		else if (use_blitter(fb))
+		else if (use_blitter(fb) || use_rendercopy(fb))
 			create_cairo_surface__blit(fd, fb);
 		else
 			create_cairo_surface__gtt(fd, fb);
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t 5/5] lib/igt_fb: Fix blitter limit checks
  2019-04-17 20:35 [igt-dev] [PATCH i-g-t 1/5] lib/igt_fb: Unref the renderecopy scratch bos Ville Syrjala
                   ` (2 preceding siblings ...)
  2019-04-17 20:35 ` [igt-dev] [PATCH i-g-t 4/5] lib/igt_fb: Nuke redundant rendercopy cairo surface variant Ville Syrjala
@ 2019-04-17 20:35 ` Ville Syrjala
  2019-04-17 20:52   ` Chris Wilson
  2019-04-18 12:12   ` [igt-dev] [PATCH i-g-t v2 " Ville Syrjala
  2019-04-17 20:38 ` [igt-dev] [PATCH i-g-t 1/5] lib/igt_fb: Unref the renderecopy scratch bos Chris Wilson
                   ` (4 subsequent siblings)
  8 siblings, 2 replies; 22+ messages in thread
From: Ville Syrjala @ 2019-04-17 20:35 UTC (permalink / raw)
  To: igt-dev

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

The earlier approach of checking the higher tiled stride
limit has backfired. All out blits are between tiled and
linear, but we only ever check this for the tiled fb. Thus
we are taking the blitter path even though the linear fb
exceeds the blitter limits. So let's just check the limits
as if we are operating on linear fbs.

And let's toss in some width/height checks, and let's do
the checks for all the color planes as well.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 lib/igt_fb.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 794f0eef04a3..65fc94f98a69 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1580,29 +1580,37 @@ struct fb_blit_upload {
 	struct intel_batchbuffer *batch;
 };
 
-static int max_blitter_stride(int fd, uint64_t modifier)
+static bool blitter_ok(const struct igt_fb *fb)
 {
-	int stride = 32768;
-
-	if (intel_gen(intel_get_drm_devid(fd)) >= 4 &&
-	    modifier != DRM_FORMAT_MOD_NONE)
-		stride *= 4;
+	for (int i = 0; i < fb->num_planes; i++) {
+		/*
+		 * gen4+ stride limit is 4x this with tiling,
+		 * but since our blits are always between tiled
+		 * and linear and we do this check just for the
+		 * tiled fb we must use the lower linear stride
+		 * limit here.
+		 */
+		if (fb->plane_width[i] > 32767 ||
+		    fb->plane_height[i] > 32767 ||
+		    fb->strides[i] > 32767)
+			return false;
+	}
 
-	return stride;
+	return true;
 }
 
 static bool use_rendercopy(const struct igt_fb *fb)
 {
 	return is_ccs_modifier(fb->modifier) ||
 		(fb->modifier == I915_FORMAT_MOD_Yf_TILED &&
-		 fb->strides[0] >= max_blitter_stride(fb->fd, fb->modifier));
+		 !blitter_ok(fb));
 }
 
 static bool use_blitter(const struct igt_fb *fb)
 {
 	return (fb->modifier == I915_FORMAT_MOD_Y_TILED ||
 		fb->modifier == I915_FORMAT_MOD_Yf_TILED) &&
-		fb->strides[0] < max_blitter_stride(fb->fd, fb->modifier);
+		blitter_ok(fb);
 }
 
 static void init_buf(struct fb_blit_upload *blit,
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 1/5] lib/igt_fb: Unref the renderecopy scratch bos
  2019-04-17 20:35 [igt-dev] [PATCH i-g-t 1/5] lib/igt_fb: Unref the renderecopy scratch bos Ville Syrjala
                   ` (3 preceding siblings ...)
  2019-04-17 20:35 ` [igt-dev] [PATCH i-g-t 5/5] lib/igt_fb: Fix blitter limit checks Ville Syrjala
@ 2019-04-17 20:38 ` Chris Wilson
  2019-04-18 11:51   ` Ville Syrjälä
  2019-04-17 22:55 ` [igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,1/5] " Patchwork
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2019-04-17 20:38 UTC (permalink / raw)
  To: Ville Syrjala, igt-dev

Quoting Ville Syrjala (2019-04-17 21:35:40)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We're currently leaking all the temporary bos we construct
> for rendercopy. That doesn't go so well when trying to test
> with 1GiB framebuffers.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  lib/igt_fb.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 6a43fcc4735f..c3ecd7186444 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -1647,6 +1647,9 @@ static void rendercopy(struct fb_blit_upload *blit,
>         render_copy(blit->batch, NULL,
>                     &src, 0, 0, dst_fb->plane_width[0], dst_fb->plane_height[0],
>                     &dst, 0, 0);
> +
> +       drm_intel_bo_unreference(dst.bo);
> +       drm_intel_bo_unreference(src.bo);

For symmetry I would suggest a fini_buf().
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 2/5] tests/i915/gem_render_copy: Don't leak bos between subtests
  2019-04-17 20:35 ` [igt-dev] [PATCH i-g-t 2/5] tests/i915/gem_render_copy: Don't leak bos between subtests Ville Syrjala
@ 2019-04-17 20:41   ` Chris Wilson
  2019-04-18 12:11   ` [igt-dev] [PATCH i-g-t v2 " Ville Syrjala
  1 sibling, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2019-04-17 20:41 UTC (permalink / raw)
  To: Ville Syrjala, igt-dev

Quoting Ville Syrjala (2019-04-17 21:35:41)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Unref the bos after one subtest is done. The next subtest
> will allocate its own bos.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  tests/i915/gem_render_copy.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/tests/i915/gem_render_copy.c b/tests/i915/gem_render_copy.c
> index 8d62a0f43ad8..a6476a4d2d8d 100644
> --- a/tests/i915/gem_render_copy.c
> +++ b/tests/i915/gem_render_copy.c
> @@ -662,6 +662,13 @@ static void test(data_t *data, uint32_t tiling, uint64_t ccs_modifier)
>  
>         if (ccs_modifier)
>                 scratch_buf_aux_check(data, &ccs);
> +
> +       for (int i = 0; i < num_src; i++)
> +               drm_intel_bo_unreference(src[i].buf.bo);
> +       drm_intel_bo_unreference(dst.bo);
> +       if (ccs_modifier)
> +               drm_intel_bo_unreference(ccs.bo);
> +       drm_intel_bo_unreference(ref.bo);

True. Plaintive whisper for scratch_buf_fini(), and perhaps reverse the
ordering for onions.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 3/5] lib/intel_batchbuffer: Make blitter asserts more useful
  2019-04-17 20:35 ` [igt-dev] [PATCH i-g-t 3/5] lib/intel_batchbuffer: Make blitter asserts more useful Ville Syrjala
@ 2019-04-17 20:46   ` Chris Wilson
  2019-04-18 11:52     ` Ville Syrjälä
  2019-04-18 12:11   ` [igt-dev] [PATCH i-g-t v2 " Ville Syrjala
  1 sibling, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2019-04-17 20:46 UTC (permalink / raw)
  To: Ville Syrjala, igt-dev

Quoting Ville Syrjala (2019-04-17 21:35:42)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Use igt_assert_lt/lte for the blitter coord/stride asserts
> so that we can see what the offending value was. gcc likes
> to optimize the values away so gdb often doesn't help as
> much as one would like.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  lib/intel_batchbuffer.c | 54 +++++++++++++++++++++++------------------
>  1 file changed, 30 insertions(+), 24 deletions(-)
> 
> diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
> index 712ce1a32da6..422f3b6186aa 100644
> --- a/lib/intel_batchbuffer.c
> +++ b/lib/intel_batchbuffer.c
> @@ -417,16 +417,16 @@ intel_blt_copy(struct intel_batchbuffer *batch,
>                 cmd_bits |= XY_SRC_COPY_BLT_DST_TILED;
>         }
>  
> -#define CHECK_RANGE(x) ((x) >= 0 && (x) < (1 << 15))
> -       igt_assert(CHECK_RANGE(src_x1) && CHECK_RANGE(src_y1) &&
> -                  CHECK_RANGE(dst_x1) && CHECK_RANGE(dst_y1) &&
> -                  CHECK_RANGE(width) && CHECK_RANGE(height) &&
> -                  CHECK_RANGE(src_x1 + width) &&
> -                  CHECK_RANGE(src_y1 + height) &&
> -                  CHECK_RANGE(dst_x1 + width) &&
> -                  CHECK_RANGE(dst_y1 + height) &&
> -                  CHECK_RANGE(src_pitch) &&
> -                  CHECK_RANGE(dst_pitch));
> +#define CHECK_RANGE(x) do { \
> +       igt_assert_lte(0, (x)); \
> +       igt_assert_lt((x), (1 << 15)); \
> +} while (0)
> +       CHECK_RANGE(src_x1); CHECK_RANGE(src_y1);
> +       CHECK_RANGE(dst_x1); CHECK_RANGE(dst_y1);
> +       CHECK_RANGE(width); CHECK_RANGE(height);
> +       CHECK_RANGE(src_x1 + width); CHECK_RANGE(src_y1 + height);
> +       CHECK_RANGE(dst_x1 + width); CHECK_RANGE(dst_y1 + height);
> +       CHECK_RANGE(src_pitch); CHECK_RANGE(dst_pitch);
>  #undef CHECK_RANGE

Ok.

>         br13_bits = 0;
> @@ -715,13 +715,16 @@ void igt_blitter_fast_copy__raw(int fd,
>         dword0 = fast_copy_dword0(src_tiling, dst_tiling);
>         dword1 = fast_copy_dword1(src_tiling, dst_tiling, bpp);
>  
> -#define CHECK_RANGE(x) ((x) >= 0 && (x) < (1 << 15))
> -       assert(CHECK_RANGE(src_x) && CHECK_RANGE(src_y) &&
> -              CHECK_RANGE(dst_x) && CHECK_RANGE(dst_y) &&
> -              CHECK_RANGE(width) && CHECK_RANGE(height) &&
> -              CHECK_RANGE(src_x + width) && CHECK_RANGE(src_y + height) &&
> -              CHECK_RANGE(dst_x + width) && CHECK_RANGE(dst_y + height) &&
> -              CHECK_RANGE(src_pitch) && CHECK_RANGE(dst_pitch));

Plain assert!

> +#define CHECK_RANGE(x) do { \
> +       igt_assert_lte(0, (x)); \
> +       igt_assert_lt((x), (1 << 15)); \
> +} while (0)

This looks like a redefinition? Oh, undef.

Maybe call it CHECK_BLT_RANGE()? CHECK_XY_RANGE()?

> +       CHECK_RANGE(src_x); CHECK_RANGE(src_y);
> +       CHECK_RANGE(dst_x); CHECK_RANGE(dst_y);
> +       CHECK_RANGE(width); CHECK_RANGE(height);
> +       CHECK_RANGE(src_x + width); CHECK_RANGE(src_y + height);
> +       CHECK_RANGE(dst_x + width); CHECK_RANGE(dst_y + height);
> +       CHECK_RANGE(src_pitch); CHECK_RANGE(dst_pitch);
>  #undef CHECK_RANGE

Ok.

>         batch[i++] = dword0;
> @@ -791,13 +794,16 @@ void igt_blitter_fast_copy(struct intel_batchbuffer *batch,
>         dword0 = fast_copy_dword0(src->tiling, dst->tiling);
>         dword1 = fast_copy_dword1(src->tiling, dst->tiling, dst->bpp);
>  
> -#define CHECK_RANGE(x) ((x) >= 0 && (x) < (1 << 15))
> -       assert(CHECK_RANGE(src_x) && CHECK_RANGE(src_y) &&
> -              CHECK_RANGE(dst_x) && CHECK_RANGE(dst_y) &&
> -              CHECK_RANGE(width) && CHECK_RANGE(height) &&
> -              CHECK_RANGE(src_x + width) && CHECK_RANGE(src_y + height) &&
> -              CHECK_RANGE(dst_x + width) && CHECK_RANGE(dst_y + height) &&
> -              CHECK_RANGE(src_pitch) && CHECK_RANGE(dst_pitch));
> +#define CHECK_RANGE(x) do { \
> +       igt_assert_lte(0, (x)); \
> +       igt_assert_lt((x), (1 << 15)); \
> +} while (0)

Third time's a winner!

> +       CHECK_RANGE(src_x); CHECK_RANGE(src_y);
> +       CHECK_RANGE(dst_x); CHECK_RANGE(dst_y);
> +       CHECK_RANGE(width); CHECK_RANGE(height);
> +       CHECK_RANGE(src_x + width); CHECK_RANGE(src_y + height);
> +       CHECK_RANGE(dst_x + width); CHECK_RANGE(dst_y + height);
> +       CHECK_RANGE(src_pitch); CHECK_RANGE(dst_pitch);
>  #undef CHECK_RANGE

Ok.

Kill the repeated CHECK_RANGE() definitions.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 4/5] lib/igt_fb: Nuke redundant rendercopy cairo surface variant
  2019-04-17 20:35 ` [igt-dev] [PATCH i-g-t 4/5] lib/igt_fb: Nuke redundant rendercopy cairo surface variant Ville Syrjala
@ 2019-04-17 20:47   ` Chris Wilson
  2019-04-18 11:53     ` Ville Syrjälä
  2019-04-18 12:12   ` [igt-dev] [PATCH i-g-t v2 " Ville Syrjala
  1 sibling, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2019-04-17 20:47 UTC (permalink / raw)
  To: Ville Syrjala, igt-dev

Quoting Ville Syrjala (2019-04-17 21:35:43)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The blit and rendercopy implementations are now identical.
> Kill one.

That is true. Maybe compromise and call them __gpu()?
 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 5/5] lib/igt_fb: Fix blitter limit checks
  2019-04-17 20:35 ` [igt-dev] [PATCH i-g-t 5/5] lib/igt_fb: Fix blitter limit checks Ville Syrjala
@ 2019-04-17 20:52   ` Chris Wilson
  2019-04-18 12:07     ` Ville Syrjälä
  2019-04-18 12:12   ` [igt-dev] [PATCH i-g-t v2 " Ville Syrjala
  1 sibling, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2019-04-17 20:52 UTC (permalink / raw)
  To: Ville Syrjala, igt-dev

Quoting Ville Syrjala (2019-04-17 21:35:44)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The earlier approach of checking the higher tiled stride
> limit has backfired. All out blits are between tiled and
> linear, but we only ever check this for the tiled fb. Thus
> we are taking the blitter path even though the linear fb
> exceeds the blitter limits. So let's just check the limits
> as if we are operating on linear fbs.
> 
> And let's toss in some width/height checks, and let's do
> the checks for all the color planes as well.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  lib/igt_fb.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 794f0eef04a3..65fc94f98a69 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -1580,29 +1580,37 @@ struct fb_blit_upload {
>         struct intel_batchbuffer *batch;
>  };
>  
> -static int max_blitter_stride(int fd, uint64_t modifier)
> +static bool blitter_ok(const struct igt_fb *fb)
>  {
> -       int stride = 32768;
> -
> -       if (intel_gen(intel_get_drm_devid(fd)) >= 4 &&
> -           modifier != DRM_FORMAT_MOD_NONE)
> -               stride *= 4;
> +       for (int i = 0; i < fb->num_planes; i++) {
> +               /*
> +                * gen4+ stride limit is 4x this with tiling,
> +                * but since our blits are always between tiled
> +                * and linear and we do this check just for the
> +                * tiled fb we must use the lower linear stride
> +                * limit here.
> +                */

I had to read this a couple of times before I realised you meant
"but since our blits are always between tiled and linear surfaces,
we must use the lower limit for linear surfaces here".

So the fun fact about tiled surfaces having their limit in dwords was a
complete red herring. Sorry.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,1/5] lib/igt_fb: Unref the renderecopy scratch bos
  2019-04-17 20:35 [igt-dev] [PATCH i-g-t 1/5] lib/igt_fb: Unref the renderecopy scratch bos Ville Syrjala
                   ` (4 preceding siblings ...)
  2019-04-17 20:38 ` [igt-dev] [PATCH i-g-t 1/5] lib/igt_fb: Unref the renderecopy scratch bos Chris Wilson
@ 2019-04-17 22:55 ` Patchwork
  2019-04-18 12:09 ` [igt-dev] [PATCH i-g-t v2 1/5] " Ville Syrjala
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-04-17 22:55 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,1/5] lib/igt_fb: Unref the renderecopy scratch bos
URL   : https://patchwork.freedesktop.org/series/59666/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5951 -> IGTPW_2889
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_2889 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_2889, 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/59666/revisions/1/mbox/

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live_active:
    - fi-bsw-n3050:       PASS -> DMESG-WARN

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@semaphore:
    - fi-bdw-5557u:       NOTRUN -> SKIP [fdo#109271] +38
    - fi-kbl-7500u:       NOTRUN -> SKIP [fdo#109271] +28

  * igt@amdgpu/amd_cs_nop@sync-fork-compute0:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109315] +17

  * igt@gem_exec_basic@gtt-bsd1:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109276] +7

  * igt@gem_exec_parse@basic-rejected:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109289] +1

  * igt@i915_selftest@live_hangcheck:
    - fi-icl-y:           PASS -> INCOMPLETE [fdo#107713] / [fdo#108569]

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7500u:       NOTRUN -> DMESG-WARN [fdo#102505] / [fdo#103558] / [fdo#105079] / [fdo#105602]

  * igt@kms_chamelium@hdmi-edid-read:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109284] +8

  * igt@kms_force_connector_basic@prune-stale-modes:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109285] +3

  * igt@kms_frontbuffer_tracking@basic:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103167]

  * igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191]

  
#### Possible fixes ####

  * igt@kms_chamelium@dp-crc-fast:
    - fi-kbl-7500u:       DMESG-WARN [fdo#103841] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-byt-clapper:     FAIL [fdo#103191] -> PASS

  
  [fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#103841]: https://bugs.freedesktop.org/show_bug.cgi?id=103841
  [fdo#105079]: https://bugs.freedesktop.org/show_bug.cgi?id=105079
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315


Participating hosts (46 -> 43)
------------------------------

  Additional (2): fi-bdw-5557u fi-icl-u3 
  Missing    (5): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus 


Build changes
-------------

    * IGT: IGT_4956 -> IGTPW_2889

  CI_DRM_5951: 08cf1213114f21fc0b1a17eac081a2db8c03311f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2889: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2889/
  IGT_4956: 1d921615b0b706f25c856aa0eb096f274380c199 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@audio@hdmi-integrity
+igt@audio@hdmi-integrity-after-hibernate
+igt@audio@hdmi-integrity-after-suspend

== Logs ==

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 1/5] lib/igt_fb: Unref the renderecopy scratch bos
  2019-04-17 20:38 ` [igt-dev] [PATCH i-g-t 1/5] lib/igt_fb: Unref the renderecopy scratch bos Chris Wilson
@ 2019-04-18 11:51   ` Ville Syrjälä
  0 siblings, 0 replies; 22+ messages in thread
From: Ville Syrjälä @ 2019-04-18 11:51 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

On Wed, Apr 17, 2019 at 09:38:58PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2019-04-17 21:35:40)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We're currently leaking all the temporary bos we construct
> > for rendercopy. That doesn't go so well when trying to test
> > with 1GiB framebuffers.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  lib/igt_fb.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index 6a43fcc4735f..c3ecd7186444 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -1647,6 +1647,9 @@ static void rendercopy(struct fb_blit_upload *blit,
> >         render_copy(blit->batch, NULL,
> >                     &src, 0, 0, dst_fb->plane_width[0], dst_fb->plane_height[0],
> >                     &dst, 0, 0);
> > +
> > +       drm_intel_bo_unreference(dst.bo);
> > +       drm_intel_bo_unreference(src.bo);
> 
> For symmetry I would suggest a fini_buf().

I had that, but then decided it was pointless :/
But I can add it back.

> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 3/5] lib/intel_batchbuffer: Make blitter asserts more useful
  2019-04-17 20:46   ` Chris Wilson
@ 2019-04-18 11:52     ` Ville Syrjälä
  0 siblings, 0 replies; 22+ messages in thread
From: Ville Syrjälä @ 2019-04-18 11:52 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

On Wed, Apr 17, 2019 at 09:46:04PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2019-04-17 21:35:42)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Use igt_assert_lt/lte for the blitter coord/stride asserts
> > so that we can see what the offending value was. gcc likes
> > to optimize the values away so gdb often doesn't help as
> > much as one would like.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  lib/intel_batchbuffer.c | 54 +++++++++++++++++++++++------------------
> >  1 file changed, 30 insertions(+), 24 deletions(-)
> > 
> > diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
> > index 712ce1a32da6..422f3b6186aa 100644
> > --- a/lib/intel_batchbuffer.c
> > +++ b/lib/intel_batchbuffer.c
> > @@ -417,16 +417,16 @@ intel_blt_copy(struct intel_batchbuffer *batch,
> >                 cmd_bits |= XY_SRC_COPY_BLT_DST_TILED;
> >         }
> >  
> > -#define CHECK_RANGE(x) ((x) >= 0 && (x) < (1 << 15))
> > -       igt_assert(CHECK_RANGE(src_x1) && CHECK_RANGE(src_y1) &&
> > -                  CHECK_RANGE(dst_x1) && CHECK_RANGE(dst_y1) &&
> > -                  CHECK_RANGE(width) && CHECK_RANGE(height) &&
> > -                  CHECK_RANGE(src_x1 + width) &&
> > -                  CHECK_RANGE(src_y1 + height) &&
> > -                  CHECK_RANGE(dst_x1 + width) &&
> > -                  CHECK_RANGE(dst_y1 + height) &&
> > -                  CHECK_RANGE(src_pitch) &&
> > -                  CHECK_RANGE(dst_pitch));
> > +#define CHECK_RANGE(x) do { \
> > +       igt_assert_lte(0, (x)); \
> > +       igt_assert_lt((x), (1 << 15)); \
> > +} while (0)
> > +       CHECK_RANGE(src_x1); CHECK_RANGE(src_y1);
> > +       CHECK_RANGE(dst_x1); CHECK_RANGE(dst_y1);
> > +       CHECK_RANGE(width); CHECK_RANGE(height);
> > +       CHECK_RANGE(src_x1 + width); CHECK_RANGE(src_y1 + height);
> > +       CHECK_RANGE(dst_x1 + width); CHECK_RANGE(dst_y1 + height);
> > +       CHECK_RANGE(src_pitch); CHECK_RANGE(dst_pitch);
> >  #undef CHECK_RANGE
> 
> Ok.
> 
> >         br13_bits = 0;
> > @@ -715,13 +715,16 @@ void igt_blitter_fast_copy__raw(int fd,
> >         dword0 = fast_copy_dword0(src_tiling, dst_tiling);
> >         dword1 = fast_copy_dword1(src_tiling, dst_tiling, bpp);
> >  
> > -#define CHECK_RANGE(x) ((x) >= 0 && (x) < (1 << 15))
> > -       assert(CHECK_RANGE(src_x) && CHECK_RANGE(src_y) &&
> > -              CHECK_RANGE(dst_x) && CHECK_RANGE(dst_y) &&
> > -              CHECK_RANGE(width) && CHECK_RANGE(height) &&
> > -              CHECK_RANGE(src_x + width) && CHECK_RANGE(src_y + height) &&
> > -              CHECK_RANGE(dst_x + width) && CHECK_RANGE(dst_y + height) &&
> > -              CHECK_RANGE(src_pitch) && CHECK_RANGE(dst_pitch));
> 
> Plain assert!
> 
> > +#define CHECK_RANGE(x) do { \
> > +       igt_assert_lte(0, (x)); \
> > +       igt_assert_lt((x), (1 << 15)); \
> > +} while (0)
> 
> This looks like a redefinition? Oh, undef.
> 
> Maybe call it CHECK_BLT_RANGE()? CHECK_XY_RANGE()?
> 
> > +       CHECK_RANGE(src_x); CHECK_RANGE(src_y);
> > +       CHECK_RANGE(dst_x); CHECK_RANGE(dst_y);
> > +       CHECK_RANGE(width); CHECK_RANGE(height);
> > +       CHECK_RANGE(src_x + width); CHECK_RANGE(src_y + height);
> > +       CHECK_RANGE(dst_x + width); CHECK_RANGE(dst_y + height);
> > +       CHECK_RANGE(src_pitch); CHECK_RANGE(dst_pitch);
> >  #undef CHECK_RANGE
> 
> Ok.
> 
> >         batch[i++] = dword0;
> > @@ -791,13 +794,16 @@ void igt_blitter_fast_copy(struct intel_batchbuffer *batch,
> >         dword0 = fast_copy_dword0(src->tiling, dst->tiling);
> >         dword1 = fast_copy_dword1(src->tiling, dst->tiling, dst->bpp);
> >  
> > -#define CHECK_RANGE(x) ((x) >= 0 && (x) < (1 << 15))
> > -       assert(CHECK_RANGE(src_x) && CHECK_RANGE(src_y) &&
> > -              CHECK_RANGE(dst_x) && CHECK_RANGE(dst_y) &&
> > -              CHECK_RANGE(width) && CHECK_RANGE(height) &&
> > -              CHECK_RANGE(src_x + width) && CHECK_RANGE(src_y + height) &&
> > -              CHECK_RANGE(dst_x + width) && CHECK_RANGE(dst_y + height) &&
> > -              CHECK_RANGE(src_pitch) && CHECK_RANGE(dst_pitch));
> > +#define CHECK_RANGE(x) do { \
> > +       igt_assert_lte(0, (x)); \
> > +       igt_assert_lt((x), (1 << 15)); \
> > +} while (0)
> 
> Third time's a winner!
> 
> > +       CHECK_RANGE(src_x); CHECK_RANGE(src_y);
> > +       CHECK_RANGE(dst_x); CHECK_RANGE(dst_y);
> > +       CHECK_RANGE(width); CHECK_RANGE(height);
> > +       CHECK_RANGE(src_x + width); CHECK_RANGE(src_y + height);
> > +       CHECK_RANGE(dst_x + width); CHECK_RANGE(dst_y + height);
> > +       CHECK_RANGE(src_pitch); CHECK_RANGE(dst_pitch);
> >  #undef CHECK_RANGE
> 
> Ok.
> 
> Kill the repeated CHECK_RANGE() definitions.

Sure. Can do.

> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 4/5] lib/igt_fb: Nuke redundant rendercopy cairo surface variant
  2019-04-17 20:47   ` Chris Wilson
@ 2019-04-18 11:53     ` Ville Syrjälä
  0 siblings, 0 replies; 22+ messages in thread
From: Ville Syrjälä @ 2019-04-18 11:53 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

On Wed, Apr 17, 2019 at 09:47:51PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2019-04-17 21:35:43)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The blit and rendercopy implementations are now identical.
> > Kill one.
> 
> That is true. Maybe compromise and call them __gpu()?

Sure. I have no strong preference about the color, so might as
well go with your suggestion.

>  
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 5/5] lib/igt_fb: Fix blitter limit checks
  2019-04-17 20:52   ` Chris Wilson
@ 2019-04-18 12:07     ` Ville Syrjälä
  0 siblings, 0 replies; 22+ messages in thread
From: Ville Syrjälä @ 2019-04-18 12:07 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

On Wed, Apr 17, 2019 at 09:52:21PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2019-04-17 21:35:44)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The earlier approach of checking the higher tiled stride
> > limit has backfired. All out blits are between tiled and
> > linear, but we only ever check this for the tiled fb. Thus
> > we are taking the blitter path even though the linear fb
> > exceeds the blitter limits. So let's just check the limits
> > as if we are operating on linear fbs.
> > 
> > And let's toss in some width/height checks, and let's do
> > the checks for all the color planes as well.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  lib/igt_fb.c | 26 +++++++++++++++++---------
> >  1 file changed, 17 insertions(+), 9 deletions(-)
> > 
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index 794f0eef04a3..65fc94f98a69 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -1580,29 +1580,37 @@ struct fb_blit_upload {
> >         struct intel_batchbuffer *batch;
> >  };
> >  
> > -static int max_blitter_stride(int fd, uint64_t modifier)
> > +static bool blitter_ok(const struct igt_fb *fb)
> >  {
> > -       int stride = 32768;
> > -
> > -       if (intel_gen(intel_get_drm_devid(fd)) >= 4 &&
> > -           modifier != DRM_FORMAT_MOD_NONE)
> > -               stride *= 4;
> > +       for (int i = 0; i < fb->num_planes; i++) {
> > +               /*
> > +                * gen4+ stride limit is 4x this with tiling,
> > +                * but since our blits are always between tiled
> > +                * and linear and we do this check just for the
> > +                * tiled fb we must use the lower linear stride
> > +                * limit here.
> > +                */
> 
> I had to read this a couple of times before I realised you meant
> "but since our blits are always between tiled and linear surfaces,
> we must use the lower limit for linear surfaces here".
> 
> So the fun fact about tiled surfaces having their limit in dwords was a
> complete red herring. Sorry.

No worries. The code is admittedly confusing. Ideally we'd do the check
for both the linear and tile surfaces, but we don't have the linear one
when we do the first check. So we'd have to at least construct its
metadata ahead of time. Doable, but I decided to go with the more
minimal apporach for now.

> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t v2 1/5] lib/igt_fb: Unref the renderecopy scratch bos
  2019-04-17 20:35 [igt-dev] [PATCH i-g-t 1/5] lib/igt_fb: Unref the renderecopy scratch bos Ville Syrjala
                   ` (5 preceding siblings ...)
  2019-04-17 22:55 ` [igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,1/5] " Patchwork
@ 2019-04-18 12:09 ` Ville Syrjala
  2019-04-18 12:50 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v2,1/5] lib/igt_fb: Unref the renderecopy scratch bos (rev6) Patchwork
  2019-04-18 15:20 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  8 siblings, 0 replies; 22+ messages in thread
From: Ville Syrjala @ 2019-04-18 12:09 UTC (permalink / raw)
  To: igt-dev

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

We're currently leaking all the temporary bos we construct
for rendercopy. That doesn't go so well when trying to test
with 1GiB framebuffers.

v2: Add fini_buf() (Chris)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/igt_fb.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 6a43fcc4735f..766e504d5fce 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1628,6 +1628,11 @@ static void init_buf(struct fb_blit_upload *blit,
 	}
 }
 
+static void fini_buf(struct igt_buf *buf)
+{
+	drm_intel_bo_unreference(buf->bo);
+}
+
 static void rendercopy(struct fb_blit_upload *blit,
 		       const struct igt_fb *dst_fb,
 		       const struct igt_fb *src_fb)
@@ -1647,6 +1652,9 @@ static void rendercopy(struct fb_blit_upload *blit,
 	render_copy(blit->batch, NULL,
 		    &src, 0, 0, dst_fb->plane_width[0], dst_fb->plane_height[0],
 		    &dst, 0, 0);
+
+	fini_buf(&dst);
+	fini_buf(&src);
 }
 
 static void blitcopy(const struct igt_fb *dst_fb,
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t v2 2/5] tests/i915/gem_render_copy: Don't leak bos between subtests
  2019-04-17 20:35 ` [igt-dev] [PATCH i-g-t 2/5] tests/i915/gem_render_copy: Don't leak bos between subtests Ville Syrjala
  2019-04-17 20:41   ` Chris Wilson
@ 2019-04-18 12:11   ` Ville Syrjala
  1 sibling, 0 replies; 22+ messages in thread
From: Ville Syrjala @ 2019-04-18 12:11 UTC (permalink / raw)
  To: igt-dev

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

Unref the bos after one subtest is done. The next subtest
will allocate its own bos.

v2: Add scratch_buf_fini() and reverse the onion (Chris)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/i915/gem_render_copy.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tests/i915/gem_render_copy.c b/tests/i915/gem_render_copy.c
index 8d62a0f43ad8..b5d1f45f0422 100644
--- a/tests/i915/gem_render_copy.c
+++ b/tests/i915/gem_render_copy.c
@@ -459,6 +459,11 @@ static void scratch_buf_init(data_t *data, struct igt_buf *buf,
 	igt_assert(igt_buf_height(buf) == height);
 }
 
+static void scratch_buf_fini(struct igt_buf *buf)
+{
+	drm_intel_bo_unreference(buf->bo);
+}
+
 static void
 scratch_buf_check(data_t *data,
 		  struct igt_buf *buf,
@@ -662,6 +667,13 @@ static void test(data_t *data, uint32_t tiling, uint64_t ccs_modifier)
 
 	if (ccs_modifier)
 		scratch_buf_aux_check(data, &ccs);
+
+	scratch_buf_fini(&ref);
+	if (ccs_modifier)
+		scratch_buf_fini(&ccs);
+	scratch_buf_fini(&dst);
+	for (int i = 0; i < num_src; i++)
+		scratch_buf_fini(&src[i].buf);
 }
 
 static int opt_handler(int opt, int opt_index, void *data)
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t v2 3/5] lib/intel_batchbuffer: Make blitter asserts more useful
  2019-04-17 20:35 ` [igt-dev] [PATCH i-g-t 3/5] lib/intel_batchbuffer: Make blitter asserts more useful Ville Syrjala
  2019-04-17 20:46   ` Chris Wilson
@ 2019-04-18 12:11   ` Ville Syrjala
  1 sibling, 0 replies; 22+ messages in thread
From: Ville Syrjala @ 2019-04-18 12:11 UTC (permalink / raw)
  To: igt-dev

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

Use igt_assert_lt/lte for the blitter coord/stride asserts
so that we can see what the offending value was. gcc likes
to optimize the values away so gdb often doesn't help as
much as one would like.

v2: Remove the duplicate CHECK_RANGE() definitions (Chris)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/intel_batchbuffer.c | 52 ++++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 27 deletions(-)

diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
index 712ce1a32da6..07de5cbb4d04 100644
--- a/lib/intel_batchbuffer.c
+++ b/lib/intel_batchbuffer.c
@@ -370,6 +370,11 @@ intel_batchbuffer_copy_data(struct intel_batchbuffer *batch,
 	return intel_batchbuffer_subdata_offset(batch, subdata);
 }
 
+#define CHECK_RANGE(x)	do { \
+	igt_assert_lte(0, (x)); \
+	igt_assert_lt((x), (1 << 15)); \
+} while (0)
+
 /**
  * intel_blt_copy:
  * @batch: batchbuffer object
@@ -417,17 +422,12 @@ intel_blt_copy(struct intel_batchbuffer *batch,
 		cmd_bits |= XY_SRC_COPY_BLT_DST_TILED;
 	}
 
-#define CHECK_RANGE(x)	((x) >= 0 && (x) < (1 << 15))
-	igt_assert(CHECK_RANGE(src_x1) && CHECK_RANGE(src_y1) &&
-		   CHECK_RANGE(dst_x1) && CHECK_RANGE(dst_y1) &&
-		   CHECK_RANGE(width) && CHECK_RANGE(height) &&
-		   CHECK_RANGE(src_x1 + width) &&
-		   CHECK_RANGE(src_y1 + height) &&
-		   CHECK_RANGE(dst_x1 + width) &&
-		   CHECK_RANGE(dst_y1 + height) &&
-		   CHECK_RANGE(src_pitch) &&
-		   CHECK_RANGE(dst_pitch));
-#undef CHECK_RANGE
+	CHECK_RANGE(src_x1); CHECK_RANGE(src_y1);
+	CHECK_RANGE(dst_x1); CHECK_RANGE(dst_y1);
+	CHECK_RANGE(width); CHECK_RANGE(height);
+	CHECK_RANGE(src_x1 + width); CHECK_RANGE(src_y1 + height);
+	CHECK_RANGE(dst_x1 + width); CHECK_RANGE(dst_y1 + height);
+	CHECK_RANGE(src_pitch); CHECK_RANGE(dst_pitch);
 
 	br13_bits = 0;
 	switch (bpp) {
@@ -715,14 +715,12 @@ void igt_blitter_fast_copy__raw(int fd,
 	dword0 = fast_copy_dword0(src_tiling, dst_tiling);
 	dword1 = fast_copy_dword1(src_tiling, dst_tiling, bpp);
 
-#define CHECK_RANGE(x)	((x) >= 0 && (x) < (1 << 15))
-	assert(CHECK_RANGE(src_x) && CHECK_RANGE(src_y) &&
-	       CHECK_RANGE(dst_x) && CHECK_RANGE(dst_y) &&
-	       CHECK_RANGE(width) && CHECK_RANGE(height) &&
-	       CHECK_RANGE(src_x + width) && CHECK_RANGE(src_y + height) &&
-	       CHECK_RANGE(dst_x + width) && CHECK_RANGE(dst_y + height) &&
-	       CHECK_RANGE(src_pitch) && CHECK_RANGE(dst_pitch));
-#undef CHECK_RANGE
+	CHECK_RANGE(src_x); CHECK_RANGE(src_y);
+	CHECK_RANGE(dst_x); CHECK_RANGE(dst_y);
+	CHECK_RANGE(width); CHECK_RANGE(height);
+	CHECK_RANGE(src_x + width); CHECK_RANGE(src_y + height);
+	CHECK_RANGE(dst_x + width); CHECK_RANGE(dst_y + height);
+	CHECK_RANGE(src_pitch); CHECK_RANGE(dst_pitch);
 
 	batch[i++] = dword0;
 	batch[i++] = dword1 | dst_pitch;
@@ -791,14 +789,12 @@ void igt_blitter_fast_copy(struct intel_batchbuffer *batch,
 	dword0 = fast_copy_dword0(src->tiling, dst->tiling);
 	dword1 = fast_copy_dword1(src->tiling, dst->tiling, dst->bpp);
 
-#define CHECK_RANGE(x)	((x) >= 0 && (x) < (1 << 15))
-	assert(CHECK_RANGE(src_x) && CHECK_RANGE(src_y) &&
-	       CHECK_RANGE(dst_x) && CHECK_RANGE(dst_y) &&
-	       CHECK_RANGE(width) && CHECK_RANGE(height) &&
-	       CHECK_RANGE(src_x + width) && CHECK_RANGE(src_y + height) &&
-	       CHECK_RANGE(dst_x + width) && CHECK_RANGE(dst_y + height) &&
-	       CHECK_RANGE(src_pitch) && CHECK_RANGE(dst_pitch));
-#undef CHECK_RANGE
+	CHECK_RANGE(src_x); CHECK_RANGE(src_y);
+	CHECK_RANGE(dst_x); CHECK_RANGE(dst_y);
+	CHECK_RANGE(width); CHECK_RANGE(height);
+	CHECK_RANGE(src_x + width); CHECK_RANGE(src_y + height);
+	CHECK_RANGE(dst_x + width); CHECK_RANGE(dst_y + height);
+	CHECK_RANGE(src_pitch); CHECK_RANGE(dst_pitch);
 
 	BEGIN_BATCH(10, 2);
 	OUT_BATCH(dword0);
@@ -816,6 +812,8 @@ void igt_blitter_fast_copy(struct intel_batchbuffer *batch,
 	intel_batchbuffer_flush(batch);
 }
 
+#undef CHECK_RANGE
+
 /**
  * igt_get_render_copyfunc:
  * @devid: pci device id
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t v2 4/5] lib/igt_fb: Nuke redundant rendercopy cairo surface variant
  2019-04-17 20:35 ` [igt-dev] [PATCH i-g-t 4/5] lib/igt_fb: Nuke redundant rendercopy cairo surface variant Ville Syrjala
  2019-04-17 20:47   ` Chris Wilson
@ 2019-04-18 12:12   ` Ville Syrjala
  1 sibling, 0 replies; 22+ messages in thread
From: Ville Syrjala @ 2019-04-18 12:12 UTC (permalink / raw)
  To: igt-dev

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

The blit and rendercopy implementations are now identical.
Kill one.

v2: s/__blit/__gpu/ (Chris)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/igt_fb.c | 51 ++++++---------------------------------------------
 1 file changed, 6 insertions(+), 45 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 766e504d5fce..292c19d28c1a 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1708,18 +1708,7 @@ static void free_linear_mapping(struct fb_blit_upload *blit)
 	}
 }
 
-static void destroy_cairo_surface__blit(void *arg)
-{
-	struct fb_blit_upload *blit = arg;
-
-	blit->fb->cairo_surface = NULL;
-
-	free_linear_mapping(blit);
-
-	free(blit);
-}
-
-static void destroy_cairo_surface__rendercopy(void *arg)
+static void destroy_cairo_surface__gpu(void *arg)
 {
 	struct fb_blit_upload *blit = arg;
 
@@ -1775,7 +1764,7 @@ static void setup_linear_mapping(struct fb_blit_upload *blit)
 				    0, linear->fb.size, PROT_READ | PROT_WRITE);
 }
 
-static void create_cairo_surface__blit(int fd, struct igt_fb *fb)
+static void create_cairo_surface__gpu(int fd, struct igt_fb *fb)
 {
 	struct fb_blit_upload *blit;
 	cairo_format_t cairo_format;
@@ -1796,34 +1785,8 @@ static void create_cairo_surface__blit(int fd, struct igt_fb *fb)
 	fb->domain = I915_GEM_DOMAIN_GTT;
 
 	cairo_surface_set_user_data(fb->cairo_surface,
-				    (cairo_user_data_key_t *)create_cairo_surface__blit,
-				    blit, destroy_cairo_surface__blit);
-}
-
-static void create_cairo_surface__rendercopy(int fd, struct igt_fb *fb)
-{
-	struct fb_blit_upload *blit;
-	cairo_format_t cairo_format;
-
-	blit = calloc(1, sizeof(*blit));
-	igt_assert(blit);
-
-	blit->fd = fd;
-	blit->fb = fb;
-
-	setup_linear_mapping(blit);
-
-	cairo_format = drm_format_to_cairo(fb->drm_format);
-	fb->cairo_surface =
-		cairo_image_surface_create_for_data(blit->linear.map,
-						    cairo_format,
-						    fb->width, fb->height,
-						    blit->linear.fb.strides[0]);
-	fb->domain = I915_GEM_DOMAIN_GTT;
-
-	cairo_surface_set_user_data(fb->cairo_surface,
-				    (cairo_user_data_key_t *)create_cairo_surface__rendercopy,
-				    blit, destroy_cairo_surface__rendercopy);
+				    (cairo_user_data_key_t *)create_cairo_surface__gpu,
+				    blit, destroy_cairo_surface__gpu);
 }
 
 /**
@@ -2885,10 +2848,8 @@ cairo_surface_t *igt_get_cairo_surface(int fd, struct igt_fb *fb)
 		    ((f->cairo_id == CAIRO_FORMAT_INVALID) &&
 		     (f->pixman_id != PIXMAN_invalid)))
 			create_cairo_surface__convert(fd, fb);
-		else if (use_rendercopy(fb))
-			create_cairo_surface__rendercopy(fd, fb);
-		else if (use_blitter(fb))
-			create_cairo_surface__blit(fd, fb);
+		else if (use_blitter(fb) || use_rendercopy(fb))
+			create_cairo_surface__gpu(fd, fb);
 		else
 			create_cairo_surface__gtt(fd, fb);
 
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t v2 5/5] lib/igt_fb: Fix blitter limit checks
  2019-04-17 20:35 ` [igt-dev] [PATCH i-g-t 5/5] lib/igt_fb: Fix blitter limit checks Ville Syrjala
  2019-04-17 20:52   ` Chris Wilson
@ 2019-04-18 12:12   ` Ville Syrjala
  1 sibling, 0 replies; 22+ messages in thread
From: Ville Syrjala @ 2019-04-18 12:12 UTC (permalink / raw)
  To: igt-dev

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

The earlier approach of checking the higher tiled stride
limit has backfired. All out blits are between tiled and
linear, but we only ever check this for the tiled fb. Thus
we are taking the blitter path even though the linear fb
exceeds the blitter limits. So let's just check the limits
as if we are operating on linear fbs.

And let's toss in some width/height checks, and let's do
the checks for all the color planes as well.

v2: Reword the comment a bit to hopefully make it legible (Chris)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/igt_fb.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 292c19d28c1a..8664d1af6f31 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1580,29 +1580,37 @@ struct fb_blit_upload {
 	struct intel_batchbuffer *batch;
 };
 
-static int max_blitter_stride(int fd, uint64_t modifier)
+static bool blitter_ok(const struct igt_fb *fb)
 {
-	int stride = 32768;
-
-	if (intel_gen(intel_get_drm_devid(fd)) >= 4 &&
-	    modifier != DRM_FORMAT_MOD_NONE)
-		stride *= 4;
+	for (int i = 0; i < fb->num_planes; i++) {
+		/*
+		 * gen4+ stride limit is 4x this with tiling,
+		 * but since our blits are always between tiled
+		 * and linear surfaces (and we do this check just
+		 * for the tiled surface) we must use the lower
+		 * linear stride limit here.
+		 */
+		if (fb->plane_width[i] > 32767 ||
+		    fb->plane_height[i] > 32767 ||
+		    fb->strides[i] > 32767)
+			return false;
+	}
 
-	return stride;
+	return true;
 }
 
 static bool use_rendercopy(const struct igt_fb *fb)
 {
 	return is_ccs_modifier(fb->modifier) ||
 		(fb->modifier == I915_FORMAT_MOD_Yf_TILED &&
-		 fb->strides[0] >= max_blitter_stride(fb->fd, fb->modifier));
+		 !blitter_ok(fb));
 }
 
 static bool use_blitter(const struct igt_fb *fb)
 {
 	return (fb->modifier == I915_FORMAT_MOD_Y_TILED ||
 		fb->modifier == I915_FORMAT_MOD_Yf_TILED) &&
-		fb->strides[0] < max_blitter_stride(fb->fd, fb->modifier);
+		blitter_ok(fb);
 }
 
 static void init_buf(struct fb_blit_upload *blit,
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v2,1/5] lib/igt_fb: Unref the renderecopy scratch bos (rev6)
  2019-04-17 20:35 [igt-dev] [PATCH i-g-t 1/5] lib/igt_fb: Unref the renderecopy scratch bos Ville Syrjala
                   ` (6 preceding siblings ...)
  2019-04-18 12:09 ` [igt-dev] [PATCH i-g-t v2 1/5] " Ville Syrjala
@ 2019-04-18 12:50 ` Patchwork
  2019-04-18 15:20 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  8 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-04-18 12:50 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,v2,1/5] lib/igt_fb: Unref the renderecopy scratch bos (rev6)
URL   : https://patchwork.freedesktop.org/series/59666/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5952 -> IGTPW_2893
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/59666/revisions/6/mbox/

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@query-info:
    - fi-bsw-kefka:       NOTRUN -> SKIP [fdo#109271] +50

  * igt@gem_exec_basic@gtt-bsd2:
    - fi-byt-clapper:     NOTRUN -> SKIP [fdo#109271] +52

  * igt@kms_addfb_basic@addfb25-y-tiled-small:
    - fi-byt-n2820:       NOTRUN -> SKIP [fdo#109271] +51

  * igt@kms_busy@basic-flip-c:
    - fi-byt-clapper:     NOTRUN -> SKIP [fdo#109271] / [fdo#109278]
    - fi-bsw-kefka:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278]
    - fi-byt-n2820:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_chamelium@hdmi-edid-read:
    - fi-hsw-peppy:       NOTRUN -> SKIP [fdo#109271] +46

  * igt@kms_cursor_legacy@basic-flip-before-cursor-varying-size:
    - fi-glk-dsi:         PASS -> INCOMPLETE [fdo#103359] / [k.org#198133]

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       NOTRUN -> DMESG-FAIL [fdo#102614] / [fdo#107814]
    - fi-byt-clapper:     NOTRUN -> FAIL [fdo#103167]

  * igt@kms_pipe_crc_basic@read-crc-pipe-b:
    - fi-byt-clapper:     NOTRUN -> FAIL [fdo#103191]

  * igt@runner@aborted:
    - fi-icl-dsi:         NOTRUN -> FAIL [fdo#109593]

  
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#107814]: https://bugs.freedesktop.org/show_bug.cgi?id=107814
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109593]: https://bugs.freedesktop.org/show_bug.cgi?id=109593
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (44 -> 44)
------------------------------

  Additional (5): fi-hsw-peppy fi-icl-dsi fi-bsw-kefka fi-byt-n2820 fi-byt-clapper 
  Missing    (5): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus 


Build changes
-------------

  * IGT: IGT_4956 -> IGTPW_2893

  CI_DRM_5952: 65305a057be0e155321a0765a3a24115063f3a32 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2893: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2893/
  IGT_4956: 1d921615b0b706f25c856aa0eb096f274380c199 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@audio@hdmi-integrity
+igt@audio@hdmi-integrity-after-hibernate
+igt@audio@hdmi-integrity-after-suspend

== Logs ==

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,v2,1/5] lib/igt_fb: Unref the renderecopy scratch bos (rev6)
  2019-04-17 20:35 [igt-dev] [PATCH i-g-t 1/5] lib/igt_fb: Unref the renderecopy scratch bos Ville Syrjala
                   ` (7 preceding siblings ...)
  2019-04-18 12:50 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v2,1/5] lib/igt_fb: Unref the renderecopy scratch bos (rev6) Patchwork
@ 2019-04-18 15:20 ` Patchwork
  8 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-04-18 15:20 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,v2,1/5] lib/igt_fb: Unref the renderecopy scratch bos (rev6)
URL   : https://patchwork.freedesktop.org/series/59666/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5952_full -> IGTPW_2893_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/59666/revisions/6/mbox/

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * {igt@audio@hdmi-integrity} (NEW):
    - shard-kbl:          NOTRUN -> FAIL +1
    - shard-glk:          NOTRUN -> FAIL +1
    - shard-iclb:         NOTRUN -> FAIL +1
    - shard-snb:          NOTRUN -> FAIL +1

  * {igt@audio@hdmi-integrity-after-suspend} (NEW):
    - shard-hsw:          NOTRUN -> FAIL +1
    - shard-apl:          NOTRUN -> FAIL +1

  
New tests
---------

  New tests have been introduced between CI_DRM_5952_full and IGTPW_2893_full:

### New IGT tests (2) ###

  * igt@audio@hdmi-integrity:
    - Statuses : 6 fail(s)
    - Exec time: [0.09, 4.23] s

  * igt@audio@hdmi-integrity-after-suspend:
    - Statuses : 6 fail(s)
    - Exec time: [0.14, 4.13] s

  

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_mocs_settings@mocs-reset-bsd2:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +34

  * igt@gem_tiled_swapping@non-threaded:
    - shard-iclb:         PASS -> FAIL [fdo#108686]

  * igt@i915_suspend@fence-restore-untiled:
    - shard-apl:          PASS -> DMESG-WARN [fdo#108566] +6

  * igt@kms_busy@extended-modeset-hang-oldfb-with-reset-render-f:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_cursor_crc@cursor-256x85-sliding:
    - shard-kbl:          PASS -> FAIL [fdo#103232]
    - shard-apl:          PASS -> FAIL [fdo#103232]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-msflip-blt:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +3

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-cpu:
    - shard-snb:          PASS -> SKIP [fdo#109271]

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-shrfb-draw-render:
    - shard-glk:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-wc:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +2

  * igt@kms_lease@cursor_implicit_plane:
    - shard-snb:          NOTRUN -> FAIL [fdo#110278]

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-f:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +4

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#108566] +1

  * igt@kms_plane_scaling@pipe-b-scaler-with-pixel-format:
    - shard-glk:          PASS -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         PASS -> SKIP [fdo#109441] +2

  * igt@kms_sysfs_edid_timing:
    - shard-iclb:         PASS -> FAIL [fdo#100047]

  
#### Possible fixes ####

  * igt@gem_eio@unwedge-stress:
    - shard-glk:          FAIL [fdo#109661] -> PASS

  * igt@gem_exec_suspend@basic-s3:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         SKIP [fdo#109349] -> PASS

  * igt@kms_frontbuffer_tracking@basic:
    - shard-snb:          SKIP [fdo#109271] -> PASS +1

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          DMESG-WARN [fdo#103313] -> PASS
    - shard-apl:          DMESG-WARN [fdo#108566] -> PASS +4

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +8

  * igt@kms_plane_scaling@pipe-b-scaler-with-rotation:
    - shard-glk:          SKIP [fdo#109271] / [fdo#109278] -> PASS

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         SKIP [fdo#109642] -> PASS

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         SKIP [fdo#109441] -> PASS +2

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-kbl:          DMESG-FAIL [fdo#105763] -> PASS

  * igt@kms_setmode@basic:
    - shard-kbl:          FAIL [fdo#99912] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#100047]: https://bugs.freedesktop.org/show_bug.cgi?id=100047
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103313]: https://bugs.freedesktop.org/show_bug.cgi?id=103313
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
  [fdo#110278]: https://bugs.freedesktop.org/show_bug.cgi?id=110278
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 6)
------------------------------

  Missing    (4): pig-skl-6260u shard-skl pig-hsw-4770r pig-glk-j5005 


Build changes
-------------

  * IGT: IGT_4956 -> IGTPW_2893
  * Piglit: piglit_4509 -> None

  CI_DRM_5952: 65305a057be0e155321a0765a3a24115063f3a32 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2893: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2893/
  IGT_4956: 1d921615b0b706f25c856aa0eb096f274380c199 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2019-04-18 15:20 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-17 20:35 [igt-dev] [PATCH i-g-t 1/5] lib/igt_fb: Unref the renderecopy scratch bos Ville Syrjala
2019-04-17 20:35 ` [igt-dev] [PATCH i-g-t 2/5] tests/i915/gem_render_copy: Don't leak bos between subtests Ville Syrjala
2019-04-17 20:41   ` Chris Wilson
2019-04-18 12:11   ` [igt-dev] [PATCH i-g-t v2 " Ville Syrjala
2019-04-17 20:35 ` [igt-dev] [PATCH i-g-t 3/5] lib/intel_batchbuffer: Make blitter asserts more useful Ville Syrjala
2019-04-17 20:46   ` Chris Wilson
2019-04-18 11:52     ` Ville Syrjälä
2019-04-18 12:11   ` [igt-dev] [PATCH i-g-t v2 " Ville Syrjala
2019-04-17 20:35 ` [igt-dev] [PATCH i-g-t 4/5] lib/igt_fb: Nuke redundant rendercopy cairo surface variant Ville Syrjala
2019-04-17 20:47   ` Chris Wilson
2019-04-18 11:53     ` Ville Syrjälä
2019-04-18 12:12   ` [igt-dev] [PATCH i-g-t v2 " Ville Syrjala
2019-04-17 20:35 ` [igt-dev] [PATCH i-g-t 5/5] lib/igt_fb: Fix blitter limit checks Ville Syrjala
2019-04-17 20:52   ` Chris Wilson
2019-04-18 12:07     ` Ville Syrjälä
2019-04-18 12:12   ` [igt-dev] [PATCH i-g-t v2 " Ville Syrjala
2019-04-17 20:38 ` [igt-dev] [PATCH i-g-t 1/5] lib/igt_fb: Unref the renderecopy scratch bos Chris Wilson
2019-04-18 11:51   ` Ville Syrjälä
2019-04-17 22:55 ` [igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,1/5] " Patchwork
2019-04-18 12:09 ` [igt-dev] [PATCH i-g-t v2 1/5] " Ville Syrjala
2019-04-18 12:50 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v2,1/5] lib/igt_fb: Unref the renderecopy scratch bos (rev6) Patchwork
2019-04-18 15:20 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.