[igt-dev] [PATCH i-g-t 1/4] lib/igt_fb: Fix the pixman converter

[igt-dev] [PATCH i-g-t 1/4] lib/igt_fb: Fix the pixman converter

[Ville Syrjala]

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

Cairo doesn't do RGB888. The shadow buffer must be in XRGB888 (which
is what our YUV converters also assume). We did calculate the shadow
stride/size with four bytes per pixel, but we just put the wrong
format in the fb metadata.

Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 lib/igt_fb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index fe0c99b9f0f3..ac43f15803ce 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1464,7 +1464,7 @@ static void *igt_fb_create_cairo_shadow_buffer(int fd,
 	igt_assert(shadow);
 
 	fb_init(shadow, fd, width, height,
-		DRM_FORMAT_RGB888, LOCAL_DRM_FORMAT_MOD_NONE,
+		DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
 		IGT_COLOR_YCBCR_BT709, IGT_COLOR_YCBCR_LIMITED_RANGE);
 
 	shadow->strides[0] = ALIGN(width * 4, 16);
@@ -1899,7 +1899,7 @@ static void fb_convert(struct fb_convert *cvt)
 	    (drm_format_to_pixman(cvt->dst.fb->drm_format) != PIXMAN_invalid)) {
 		convert_pixman(cvt);
 		return;
-	} else if (cvt->dst.fb->drm_format == DRM_FORMAT_RGB888) {
+	} else if (cvt->dst.fb->drm_format == DRM_FORMAT_XRGB8888) {
 		switch (cvt->src.fb->drm_format) {
 		case DRM_FORMAT_NV12:
 			convert_nv12_to_rgb24(cvt);
@@ -1911,7 +1911,7 @@ static void fb_convert(struct fb_convert *cvt)
 			convert_yuyv_to_rgb24(cvt);
 			return;
 		}
-	} else if (cvt->src.fb->drm_format == DRM_FORMAT_RGB888) {
+	} else if (cvt->src.fb->drm_format == DRM_FORMAT_XRGB8888) {
 		switch (cvt->dst.fb->drm_format) {
 		case DRM_FORMAT_NV12:
 			convert_rgb24_to_nv12(cvt);
-- 
2.18.1

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

[igt-dev] [PATCH i-g-t 2/4] lib/igt_fb: Assert converted formats harder

[Ville Syrjala]

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

Add more asserts to make sure the converted formats are
correct.

Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 lib/igt_fb.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index ac43f15803ce..a59b5a884d46 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1524,6 +1524,9 @@ static void convert_nv12_to_rgb24(struct fb_convert *cvt)
 	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.fb->color_encoding,
 						    cvt->src.fb->color_range);
 
+	igt_assert(cvt->src.fb->drm_format == DRM_FORMAT_NV12 &&
+		   cvt->dst.fb->drm_format == DRM_FORMAT_XRGB8888);
+
 	/*
 	 * Reading from the BO is awfully slow because of lack of read caching,
 	 * it's faster to copy the whole BO to a temporary buffer and convert
@@ -1633,8 +1636,8 @@ static void convert_rgb24_to_nv12(struct fb_convert *cvt)
 	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(cvt->dst.fb->color_encoding,
 						    cvt->dst.fb->color_range);
 
-	igt_assert_f(cvt->dst.fb->drm_format == DRM_FORMAT_NV12,
-		     "Conversion not implemented for !NV12 planar formats\n");
+	igt_assert(cvt->src.fb->drm_format == DRM_FORMAT_XRGB8888 &&
+		   cvt->dst.fb->drm_format == DRM_FORMAT_NV12);
 
 	for (i = 0; i < cvt->dst.fb->height / 2; i++) {
 		for (j = 0; j < cvt->dst.fb->width / 2; j++) {
@@ -1762,6 +1765,12 @@ static void convert_yuyv_to_rgb24(struct fb_convert *cvt)
 						    cvt->src.fb->color_range);
 	const unsigned char *swz = yuyv_swizzle(cvt->src.fb->drm_format);
 
+	igt_assert((cvt->src.fb->drm_format == DRM_FORMAT_YUYV ||
+		    cvt->src.fb->drm_format == DRM_FORMAT_UYVY ||
+		    cvt->src.fb->drm_format == DRM_FORMAT_YVYU ||
+		    cvt->src.fb->drm_format == DRM_FORMAT_VYUY) &&
+		   cvt->dst.fb->drm_format == DRM_FORMAT_XRGB8888);
+
 	/*
 	 * Reading from the BO is awfully slow because of lack of read caching,
 	 * it's faster to copy the whole BO to a temporary buffer and convert
@@ -1821,11 +1830,11 @@ static void convert_rgb24_to_yuyv(struct fb_convert *cvt)
 						    cvt->dst.fb->color_range);
 	const unsigned char *swz = yuyv_swizzle(cvt->dst.fb->drm_format);
 
-	igt_assert_f(cvt->dst.fb->drm_format == DRM_FORMAT_YUYV ||
-		     cvt->dst.fb->drm_format == DRM_FORMAT_YVYU ||
-		     cvt->dst.fb->drm_format == DRM_FORMAT_UYVY ||
-		     cvt->dst.fb->drm_format == DRM_FORMAT_VYUY,
-		     "Conversion not implemented for !YUYV planar formats\n");
+	igt_assert(cvt->src.fb->drm_format == DRM_FORMAT_XRGB8888 &&
+		   (cvt->dst.fb->drm_format == DRM_FORMAT_YUYV ||
+		    cvt->dst.fb->drm_format == DRM_FORMAT_UYVY ||
+		    cvt->dst.fb->drm_format == DRM_FORMAT_YVYU ||
+		    cvt->dst.fb->drm_format == DRM_FORMAT_VYUY));
 
 	for (i = 0; i < cvt->dst.fb->height; i++) {
 		for (j = 0; j < cvt->dst.fb->width / 2; j++) {
-- 
2.18.1

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

[igt-dev] [PATCH i-g-t 3/4] lib/igt_fb: Use linear.fb in the converter

[Ville Syrjala]

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

The converter operates between the linear and shadow fbs. When using
the blitter path there is no particular reason to assume that the
linear fb and actual fb have the same strides for instance. Thus
consulting the actual fb for metadata is not really correct.

We can also simplify the mmap() path by copying all the original
fb metadata into linear.fb as there we map the original fb directly.
We just have to keep clearing linear.fb.gem_handle so that the
dtor knows which kind of beast it's got.

Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 lib/igt_fb.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index a59b5a884d46..97310f70b635 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1946,7 +1946,7 @@ static void destroy_cairo_surface__convert(void *arg)
 	struct fb_convert cvt = {
 		.dst	= {
 			.ptr	= blit->base.linear.map,
-			.fb	= blit->base.fb,
+			.fb	= &blit->base.linear.fb,
 		},
 
 		.src	= {
@@ -1987,18 +1987,16 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
 	    fb->tiling == LOCAL_I915_FORMAT_MOD_Yf_TILED) {
 		setup_linear_mapping(fd, fb, &blit->base.linear);
 	} else {
+		blit->base.linear.fb = *fb;
 		blit->base.linear.fb.gem_handle = 0;
 		blit->base.linear.map = map_bo(fd, fb);
 		igt_assert(blit->base.linear.map);
-		blit->base.linear.fb.size = fb->size;
-		memcpy(blit->base.linear.fb.strides, fb->strides, sizeof(fb->strides));
-		memcpy(blit->base.linear.fb.offsets, fb->offsets, sizeof(fb->offsets));
 	}
 
 	cvt.dst.ptr = blit->shadow_ptr;
 	cvt.dst.fb = &blit->shadow_fb;
 	cvt.src.ptr = blit->base.linear.map;
-	cvt.src.fb = blit->base.fb;
+	cvt.src.fb = &blit->base.linear.fb;
 	fb_convert(&cvt);
 
 	fb->cairo_surface =
-- 
2.18.1

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

[igt-dev] [PATCH i-g-t 4/4] lib/igt_fb: Generalize the slow read from gtt mmap handling

[Ville Syrjala]

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

Make the handling of slow gtt mmap reads generic, and extend it to
the pixman converter. Makes the pixman path a bit faster.

With testing just XRGB8888 and XBGR8888 on KBL:
$ time kms_plane --r pixel-format-pipe-A-planes
- real	0m18,757s
+ real	0m2,635s

Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 lib/igt_fb.c | 63 ++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 46 insertions(+), 17 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 97310f70b635..9375b6c5583e 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1506,6 +1506,7 @@ static void write_rgb(uint8_t *rgb24, const struct igt_vec4 *rgb)
 struct fb_convert_buf {
 	void			*ptr;
 	struct igt_fb		*fb;
+	bool                     slow_reads;
 };
 
 struct fb_convert {
@@ -1513,6 +1514,36 @@ struct fb_convert {
 	struct fb_convert_buf	src;
 };
 
+static void *convert_src_get(const struct fb_convert *cvt)
+{
+	void *buf;
+
+	if (!cvt->src.slow_reads)
+		return cvt->src.ptr;
+
+	/*
+	 * Reading from the BO is awfully slow because of lack of read caching,
+	 * it's faster to copy the whole BO to a temporary buffer and convert
+	 * from there.
+	 */
+	buf = malloc(cvt->src.fb->size);
+	igt_memcpy_from_wc(buf, cvt->src.ptr, cvt->src.fb->size);
+
+	return buf;
+}
+
+static void convert_src_put(const struct fb_convert *cvt,
+			    void *src_buf)
+{
+	/*
+	 * Reading from the BO is awfully slow because of lack of read caching,
+	 * it's faster to copy the whole BO to a temporary buffer and convert
+	 * from there.
+	 */
+	if (cvt->src.slow_reads)
+		free(src_buf);
+}
+
 static void convert_nv12_to_rgb24(struct fb_convert *cvt)
 {
 	int i, j;
@@ -1520,19 +1551,14 @@ static void convert_nv12_to_rgb24(struct fb_convert *cvt)
 	uint8_t *rgb24 = cvt->dst.ptr;
 	unsigned int rgb24_stride = cvt->dst.fb->strides[0];
 	unsigned int planar_stride = cvt->src.fb->strides[0];
-	uint8_t *buf = malloc(cvt->src.fb->size);
 	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.fb->color_encoding,
 						    cvt->src.fb->color_range);
+	uint8_t *buf;
 
 	igt_assert(cvt->src.fb->drm_format == DRM_FORMAT_NV12 &&
 		   cvt->dst.fb->drm_format == DRM_FORMAT_XRGB8888);
 
-	/*
-	 * Reading from the BO is awfully slow because of lack of read caching,
-	 * it's faster to copy the whole BO to a temporary buffer and convert
-	 * from there.
-	 */
-	igt_memcpy_from_wc(buf, cvt->src.ptr, cvt->src.fb->size);
+	buf = convert_src_get(cvt);
 	y = buf + cvt->src.fb->offsets[0];
 	uv = buf + cvt->src.fb->offsets[1];
 
@@ -1622,7 +1648,7 @@ static void convert_nv12_to_rgb24(struct fb_convert *cvt)
 		}
 	}
 
-	free(buf);
+	convert_src_put(cvt, buf);
 }
 
 static void convert_rgb24_to_nv12(struct fb_convert *cvt)
@@ -1760,10 +1786,10 @@ static void convert_yuyv_to_rgb24(struct fb_convert *cvt)
 	uint8_t *rgb24 = cvt->dst.ptr;
 	unsigned int rgb24_stride = cvt->dst.fb->strides[0];
 	unsigned int yuyv_stride = cvt->src.fb->strides[0];
-	uint8_t *buf = malloc(cvt->src.fb->size);
 	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.fb->color_encoding,
 						    cvt->src.fb->color_range);
 	const unsigned char *swz = yuyv_swizzle(cvt->src.fb->drm_format);
+	uint8_t *buf;
 
 	igt_assert((cvt->src.fb->drm_format == DRM_FORMAT_YUYV ||
 		    cvt->src.fb->drm_format == DRM_FORMAT_UYVY ||
@@ -1771,12 +1797,7 @@ static void convert_yuyv_to_rgb24(struct fb_convert *cvt)
 		    cvt->src.fb->drm_format == DRM_FORMAT_VYUY) &&
 		   cvt->dst.fb->drm_format == DRM_FORMAT_XRGB8888);
 
-	/*
-	 * Reading from the BO is awfully slow because of lack of read caching,
-	 * it's faster to copy the whole BO to a temporary buffer and convert
-	 * from there.
-	 */
-	igt_memcpy_from_wc(buf, cvt->src.ptr, cvt->src.fb->size);
+	buf = convert_src_get(cvt);
 	yuyv = buf;
 
 	for (i = 0; i < cvt->dst.fb->height; i++) {
@@ -1816,7 +1837,7 @@ static void convert_yuyv_to_rgb24(struct fb_convert *cvt)
 		yuyv += yuyv_stride;
 	}
 
-	free(buf);
+	convert_src_put(cvt, buf);
 }
 
 static void convert_rgb24_to_yuyv(struct fb_convert *cvt)
@@ -1877,14 +1898,17 @@ static void convert_pixman(struct fb_convert *cvt)
 	pixman_format_code_t src_pixman = drm_format_to_pixman(cvt->src.fb->drm_format);
 	pixman_format_code_t dst_pixman = drm_format_to_pixman(cvt->dst.fb->drm_format);
 	pixman_image_t *dst_image, *src_image;
+	void *src_ptr;
 
 	igt_assert((src_pixman != PIXMAN_invalid) &&
 		   (dst_pixman != PIXMAN_invalid));
 
+	src_ptr = convert_src_get(cvt);
+
 	src_image = pixman_image_create_bits(src_pixman,
 					     cvt->src.fb->width,
 					     cvt->src.fb->height,
-					     cvt->src.ptr,
+					     src_ptr,
 					     cvt->src.fb->strides[0]);
 	igt_assert(src_image);
 
@@ -1900,6 +1924,8 @@ static void convert_pixman(struct fb_convert *cvt)
 			       cvt->dst.fb->width, cvt->dst.fb->height);
 	pixman_image_unref(dst_image);
 	pixman_image_unref(src_image);
+
+	convert_src_put(cvt, src_ptr);
 }
 
 static void fb_convert(struct fb_convert *cvt)
@@ -1991,6 +2017,9 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
 		blit->base.linear.fb.gem_handle = 0;
 		blit->base.linear.map = map_bo(fd, fb);
 		igt_assert(blit->base.linear.map);
+
+		/* reading via gtt mmap is slow */
+		cvt.src.slow_reads = is_i915_device(fd);
 	}
 
 	cvt.dst.ptr = blit->shadow_ptr;
-- 
2.18.1

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

Re: [igt-dev] [PATCH i-g-t 4/4] lib/igt_fb: Generalize the slow read from gtt mmap handling

[Chris Wilson]

Quoting Ville Syrjala (2018-11-02 19:06:51)
> +static void *convert_src_get(const struct fb_convert *cvt)
> +{
> +       void *buf;
> +
> +       if (!cvt->src.slow_reads)
> +               return cvt->src.ptr;
> +
> +       /*
> +        * Reading from the BO is awfully slow because of lack of read caching,
> +        * it's faster to copy the whole BO to a temporary buffer and convert
> +        * from there.
> +        */
> +       buf = malloc(cvt->src.fb->size);

if (!buf)
	return cvt->src.ptr;

> +       igt_memcpy_from_wc(buf, cvt->src.ptr, cvt->src.fb->size);
> +
> +       return buf;
> +}
> +
> +static void convert_src_put(const struct fb_convert *cvt,
> +                           void *src_buf)
> +{
> +       /*
> +        * Reading from the BO is awfully slow because of lack of read caching,
> +        * it's faster to copy the whole BO to a temporary buffer and convert
> +        * from there.
> +        */
> +       if (cvt->src.slow_reads)

	if (src_buf != cvt->src.ptr)

> +               free(src_buf);
> +}
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 4/4] lib/igt_fb: Generalize the slow read from gtt mmap handling

[Ville Syrjälä]

On Fri, Nov 02, 2018 at 07:17:03PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-11-02 19:06:51)
> > +static void *convert_src_get(const struct fb_convert *cvt)
> > +{
> > +       void *buf;
> > +
> > +       if (!cvt->src.slow_reads)
> > +               return cvt->src.ptr;
> > +
> > +       /*
> > +        * Reading from the BO is awfully slow because of lack of read caching,
> > +        * it's faster to copy the whole BO to a temporary buffer and convert
> > +        * from there.
> > +        */
> > +       buf = malloc(cvt->src.fb->size);
> 
> if (!buf)
> 	return cvt->src.ptr;

Sure, why not.

> 
> > +       igt_memcpy_from_wc(buf, cvt->src.ptr, cvt->src.fb->size);
> > +
> > +       return buf;
> > +}
> > +
> > +static void convert_src_put(const struct fb_convert *cvt,
> > +                           void *src_buf)
> > +{
> > +       /*
> > +        * Reading from the BO is awfully slow because of lack of read caching,
> > +        * it's faster to copy the whole BO to a temporary buffer and convert
> > +        * from there.
> > +        */
> > +       if (cvt->src.slow_reads)
> 
> 	if (src_buf != cvt->src.ptr)

Yes, that is better (tm).

> 
> > +               free(src_buf);
> > +}

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

Re: [igt-dev] [PATCH i-g-t 4/4] lib/igt_fb: Generalize the slow read from gtt mmap handling

[Chris Wilson]

Quoting Ville Syrjälä (2018-11-02 19:30:29)
> On Fri, Nov 02, 2018 at 07:17:03PM +0000, Chris Wilson wrote:
> > Quoting Ville Syrjala (2018-11-02 19:06:51)
> > > +static void *convert_src_get(const struct fb_convert *cvt)
> > > +{
> > > +       void *buf;
> > > +
> > > +       if (!cvt->src.slow_reads)
> > > +               return cvt->src.ptr;
> > > +
> > > +       /*
> > > +        * Reading from the BO is awfully slow because of lack of read caching,
> > > +        * it's faster to copy the whole BO to a temporary buffer and convert
> > > +        * from there.
> > > +        */
> > > +       buf = malloc(cvt->src.fb->size);
> > 
> > if (!buf)
> >       return cvt->src.ptr;
> 
> Sure, why not.

The other choice being igt_assert(buf) or igt_xmalloc() :)
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 4/4] lib/igt_fb: Generalize the slow read from gtt mmap handling

[Ville Syrjälä]

On Fri, Nov 02, 2018 at 07:32:02PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjälä (2018-11-02 19:30:29)
> > On Fri, Nov 02, 2018 at 07:17:03PM +0000, Chris Wilson wrote:
> > > Quoting Ville Syrjala (2018-11-02 19:06:51)
> > > > +static void *convert_src_get(const struct fb_convert *cvt)
> > > > +{
> > > > +       void *buf;
> > > > +
> > > > +       if (!cvt->src.slow_reads)
> > > > +               return cvt->src.ptr;
> > > > +
> > > > +       /*
> > > > +        * Reading from the BO is awfully slow because of lack of read caching,
> > > > +        * it's faster to copy the whole BO to a temporary buffer and convert
> > > > +        * from there.
> > > > +        */
> > > > +       buf = malloc(cvt->src.fb->size);
> > > 
> > > if (!buf)
> > >       return cvt->src.ptr;
> > 
> > Sure, why not.
> 
> The other choice being igt_assert(buf) or igt_xmalloc() :)

I figured the NULL ptr explosion is as good as an assert. But
since it's trivial to handle it gracefully let's do that.

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

[igt-dev] [PATCH i-g-t v2 4/4] lib/igt_fb: Generalize the slow read from gtt mmap handling

[Ville Syrjala]

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

Make the handling of slow gtt mmap reads generic, and extend it to
the pixman converter. Makes the pixman path a bit faster.

With testing just XRGB8888 and XBGR8888 on KBL:
$ time kms_plane --r pixel-format-pipe-A-planes
- real	0m18,757s
+ real	0m2,635s

v2: Use the original src buffer if the malloc fails (Chris)
    Drop the duplicated comment about things being slow

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 lib/igt_fb.c | 61 +++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 44 insertions(+), 17 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 97310f70b635..01efd269714b 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1506,6 +1506,7 @@ static void write_rgb(uint8_t *rgb24, const struct igt_vec4 *rgb)
 struct fb_convert_buf {
 	void			*ptr;
 	struct igt_fb		*fb;
+	bool                     slow_reads;
 };
 
 struct fb_convert {
@@ -1513,6 +1514,34 @@ struct fb_convert {
 	struct fb_convert_buf	src;
 };
 
+static void *convert_src_get(const struct fb_convert *cvt)
+{
+	void *buf;
+
+	if (!cvt->src.slow_reads)
+		return cvt->src.ptr;
+
+	/*
+	 * Reading from the BO is awfully slow because of lack of read caching,
+	 * it's faster to copy the whole BO to a temporary buffer and convert
+	 * from there.
+	 */
+	buf = malloc(cvt->src.fb->size);
+	if (!buf)
+		return cvt->src.ptr;
+
+	igt_memcpy_from_wc(buf, cvt->src.ptr, cvt->src.fb->size);
+
+	return buf;
+}
+
+static void convert_src_put(const struct fb_convert *cvt,
+			    void *src_buf)
+{
+	if (src_buf != cvt->src.ptr)
+		free(src_buf);
+}
+
 static void convert_nv12_to_rgb24(struct fb_convert *cvt)
 {
 	int i, j;
@@ -1520,19 +1549,14 @@ static void convert_nv12_to_rgb24(struct fb_convert *cvt)
 	uint8_t *rgb24 = cvt->dst.ptr;
 	unsigned int rgb24_stride = cvt->dst.fb->strides[0];
 	unsigned int planar_stride = cvt->src.fb->strides[0];
-	uint8_t *buf = malloc(cvt->src.fb->size);
 	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.fb->color_encoding,
 						    cvt->src.fb->color_range);
+	uint8_t *buf;
 
 	igt_assert(cvt->src.fb->drm_format == DRM_FORMAT_NV12 &&
 		   cvt->dst.fb->drm_format == DRM_FORMAT_XRGB8888);
 
-	/*
-	 * Reading from the BO is awfully slow because of lack of read caching,
-	 * it's faster to copy the whole BO to a temporary buffer and convert
-	 * from there.
-	 */
-	igt_memcpy_from_wc(buf, cvt->src.ptr, cvt->src.fb->size);
+	buf = convert_src_get(cvt);
 	y = buf + cvt->src.fb->offsets[0];
 	uv = buf + cvt->src.fb->offsets[1];
 
@@ -1622,7 +1646,7 @@ static void convert_nv12_to_rgb24(struct fb_convert *cvt)
 		}
 	}
 
-	free(buf);
+	convert_src_put(cvt, buf);
 }
 
 static void convert_rgb24_to_nv12(struct fb_convert *cvt)
@@ -1760,10 +1784,10 @@ static void convert_yuyv_to_rgb24(struct fb_convert *cvt)
 	uint8_t *rgb24 = cvt->dst.ptr;
 	unsigned int rgb24_stride = cvt->dst.fb->strides[0];
 	unsigned int yuyv_stride = cvt->src.fb->strides[0];
-	uint8_t *buf = malloc(cvt->src.fb->size);
 	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.fb->color_encoding,
 						    cvt->src.fb->color_range);
 	const unsigned char *swz = yuyv_swizzle(cvt->src.fb->drm_format);
+	uint8_t *buf;
 
 	igt_assert((cvt->src.fb->drm_format == DRM_FORMAT_YUYV ||
 		    cvt->src.fb->drm_format == DRM_FORMAT_UYVY ||
@@ -1771,12 +1795,7 @@ static void convert_yuyv_to_rgb24(struct fb_convert *cvt)
 		    cvt->src.fb->drm_format == DRM_FORMAT_VYUY) &&
 		   cvt->dst.fb->drm_format == DRM_FORMAT_XRGB8888);
 
-	/*
-	 * Reading from the BO is awfully slow because of lack of read caching,
-	 * it's faster to copy the whole BO to a temporary buffer and convert
-	 * from there.
-	 */
-	igt_memcpy_from_wc(buf, cvt->src.ptr, cvt->src.fb->size);
+	buf = convert_src_get(cvt);
 	yuyv = buf;
 
 	for (i = 0; i < cvt->dst.fb->height; i++) {
@@ -1816,7 +1835,7 @@ static void convert_yuyv_to_rgb24(struct fb_convert *cvt)
 		yuyv += yuyv_stride;
 	}
 
-	free(buf);
+	convert_src_put(cvt, buf);
 }
 
 static void convert_rgb24_to_yuyv(struct fb_convert *cvt)
@@ -1877,14 +1896,17 @@ static void convert_pixman(struct fb_convert *cvt)
 	pixman_format_code_t src_pixman = drm_format_to_pixman(cvt->src.fb->drm_format);
 	pixman_format_code_t dst_pixman = drm_format_to_pixman(cvt->dst.fb->drm_format);
 	pixman_image_t *dst_image, *src_image;
+	void *src_ptr;
 
 	igt_assert((src_pixman != PIXMAN_invalid) &&
 		   (dst_pixman != PIXMAN_invalid));
 
+	src_ptr = convert_src_get(cvt);
+
 	src_image = pixman_image_create_bits(src_pixman,
 					     cvt->src.fb->width,
 					     cvt->src.fb->height,
-					     cvt->src.ptr,
+					     src_ptr,
 					     cvt->src.fb->strides[0]);
 	igt_assert(src_image);
 
@@ -1900,6 +1922,8 @@ static void convert_pixman(struct fb_convert *cvt)
 			       cvt->dst.fb->width, cvt->dst.fb->height);
 	pixman_image_unref(dst_image);
 	pixman_image_unref(src_image);
+
+	convert_src_put(cvt, src_ptr);
 }
 
 static void fb_convert(struct fb_convert *cvt)
@@ -1991,6 +2015,9 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
 		blit->base.linear.fb.gem_handle = 0;
 		blit->base.linear.map = map_bo(fd, fb);
 		igt_assert(blit->base.linear.map);
+
+		/* reading via gtt mmap is slow */
+		cvt.src.slow_reads = is_i915_device(fd);
 	}
 
 	cvt.dst.ptr = blit->shadow_ptr;
-- 
2.18.1

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

[igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/4] lib/igt_fb: Fix the pixman converter (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/4] lib/igt_fb: Fix the pixman converter (rev2)
URL   : https://patchwork.freedesktop.org/series/51974/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5081 -> IGTPW_2031 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/51974/revisions/2/mbox/

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s3:
      fi-kbl-soraka:      NOTRUN -> INCOMPLETE (fdo#107556, fdo#107859, fdo#107774)
      fi-blb-e6850:       PASS -> INCOMPLETE (fdo#107718)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-byt-clapper:     PASS -> INCOMPLETE (fdo#102657)

    
    ==== Possible fixes ====

    igt@gem_cpu_reloc@basic:
      fi-kbl-7560u:       INCOMPLETE (fdo#103665) -> PASS

    igt@kms_flip@basic-flip-vs-modeset:
      fi-skl-6700hq:      DMESG-WARN (fdo#105998) -> PASS

    
  fdo#102657 https://bugs.freedesktop.org/show_bug.cgi?id=102657
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998
  fdo#107556 https://bugs.freedesktop.org/show_bug.cgi?id=107556
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107774 https://bugs.freedesktop.org/show_bug.cgi?id=107774
  fdo#107859 https://bugs.freedesktop.org/show_bug.cgi?id=107859


== Participating hosts (50 -> 46) ==

  Additional (1): fi-kbl-soraka 
  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


== Build changes ==

    * IGT: IGT_4705 -> IGTPW_2031

  CI_DRM_5081: f5e16acf6c85d38756c3efdb77ec6aede55df0ba @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2031: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2031/
  IGT_4705: 7983e19ed62ec8db1884f55e07e458a62cc51e37 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

[igt-dev] ✗ Fi.CI.IGT: failure for series starting with [i-g-t,1/4] lib/igt_fb: Fix the pixman converter (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/4] lib/igt_fb: Fix the pixman converter (rev2)
URL   : https://patchwork.freedesktop.org/series/51974/
State : failure

== Summary ==

= CI Bug Log - changes from IGT_4705_full -> IGTPW_2031_full =

== Summary - FAILURE ==

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_properties@connector-properties-atomic:
      shard-apl:          PASS -> FAIL
      shard-glk:          PASS -> FAIL
      shard-hsw:          PASS -> FAIL
      shard-kbl:          PASS -> FAIL

    
    ==== Warnings ====

    igt@kms_content_protection@atomic:
      shard-kbl:          DMESG-FAIL (fdo#108550) -> FAIL

    igt@kms_vblank@pipe-b-wait-busy-hang:
      shard-snb:          PASS -> SKIP

    igt@perf_pmu@rc6:
      shard-kbl:          SKIP -> PASS

    igt@pm_rc6_residency@rc6-accuracy:
      shard-snb:          SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_busy@extended-modeset-hang-newfb-render-a:
      shard-hsw:          PASS -> DMESG-WARN (fdo#107956)

    igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
      shard-snb:          PASS -> DMESG-WARN (fdo#107956)

    igt@kms_color@pipe-c-degamma:
      shard-kbl:          PASS -> FAIL (fdo#104782)
      shard-apl:          PASS -> FAIL (fdo#104782)

    igt@kms_cursor_crc@cursor-256x85-random:
      shard-apl:          PASS -> FAIL (fdo#103232) +1
      shard-glk:          PASS -> FAIL (fdo#103232) +1

    igt@kms_cursor_legacy@cursorb-vs-flipb-toggle:
      shard-glk:          PASS -> DMESG-WARN (fdo#105763, fdo#106538)

    igt@kms_flip@2x-flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#105363)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-onoff:
      shard-glk:          PASS -> FAIL (fdo#103167) +4
      shard-apl:          PASS -> FAIL (fdo#103167)
      shard-kbl:          PASS -> FAIL (fdo#103167)

    igt@kms_plane@plane-position-covered-pipe-a-planes:
      shard-glk:          PASS -> FAIL (fdo#103166) +2
      shard-apl:          PASS -> FAIL (fdo#103166) +2

    igt@kms_plane_alpha_blend@pipe-a-alpha-7efc:
      shard-apl:          PASS -> FAIL (fdo#108145)

    igt@kms_plane_alpha_blend@pipe-b-alpha-transparant-fb:
      shard-glk:          NOTRUN -> FAIL (fdo#108145)

    igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
      shard-kbl:          PASS -> FAIL (fdo#103166)

    igt@kms_rotation_crc@primary-rotation-180:
      shard-snb:          PASS -> FAIL (fdo#103925)

    igt@kms_setmode@basic:
      shard-apl:          PASS -> FAIL (fdo#99912)

    igt@perf@blocking:
      shard-hsw:          PASS -> FAIL (fdo#102252)

    
    ==== Possible fixes ====

    igt@gem_exec_schedule@preempt-contexts-render:
      shard-glk:          DMESG-WARN (fdo#105763, fdo#106538) -> PASS +1

    igt@kms_available_modes_crc@available_mode_test_crc:
      shard-snb:          FAIL (fdo#106641) -> PASS

    igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
      shard-glk:          FAIL (fdo#108145) -> PASS

    igt@kms_color@pipe-c-ctm-max:
      shard-kbl:          FAIL (fdo#108147) -> PASS
      shard-apl:          FAIL (fdo#108147) -> PASS

    igt@kms_color@pipe-c-legacy-gamma:
      shard-apl:          FAIL (fdo#104782) -> PASS

    igt@kms_cursor_crc@cursor-128x42-sliding:
      shard-apl:          FAIL (fdo#103232) -> PASS +1

    igt@kms_cursor_crc@cursor-256x256-sliding:
      shard-glk:          FAIL (fdo#103232) -> PASS +1

    igt@kms_draw_crc@draw-method-rgb565-render-ytiled:
      shard-apl:          INCOMPLETE (fdo#103927) -> SKIP

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
      shard-apl:          FAIL (fdo#103167) -> PASS +1

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
      shard-glk:          FAIL (fdo#103167) -> PASS +4
      shard-kbl:          FAIL (fdo#103167) -> PASS +1

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-indfb-draw-mmap-wc:
      shard-glk:          INCOMPLETE (k.org#198133, fdo#103359) -> PASS

    igt@kms_plane@pixel-format-pipe-b-planes:
      shard-kbl:          FAIL (fdo#103166) -> PASS +2

    igt@kms_plane@pixel-format-pipe-c-planes:
      shard-apl:          FAIL (fdo#103166) -> PASS +4

    igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS

    igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
      shard-glk:          FAIL (fdo#103166) -> PASS +5

    
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104782 https://bugs.freedesktop.org/show_bug.cgi?id=104782
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#106641 https://bugs.freedesktop.org/show_bug.cgi?id=106641
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#108147 https://bugs.freedesktop.org/show_bug.cgi?id=108147
  fdo#108550 https://bugs.freedesktop.org/show_bug.cgi?id=108550
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (6 -> 5) ==

  Missing    (1): shard-skl 


== Build changes ==

    * IGT: IGT_4705 -> IGTPW_2031
    * Linux: CI_DRM_5079 -> CI_DRM_5081

  CI_DRM_5079: fc3d54b430337be9c61a524c65b521761d6664a8 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_5081: f5e16acf6c85d38756c3efdb77ec6aede55df0ba @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2031: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2031/
  IGT_4705: 7983e19ed62ec8db1884f55e07e458a62cc51e37 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

Re: [igt-dev] [PATCH i-g-t v2 4/4] lib/igt_fb: Generalize the slow read from gtt mmap handling

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1047 bytes --]

On Fri, Nov 02, 2018 at 09:37:31PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Make the handling of slow gtt mmap reads generic, and extend it to
> the pixman converter. Makes the pixman path a bit faster.
> 
> With testing just XRGB8888 and XBGR8888 on KBL:
> $ time kms_plane --r pixel-format-pipe-A-planes
> - real	0m18,757s
> + real	0m2,635s
> 
> v2: Use the original src buffer if the malloc fails (Chris)
>     Drop the duplicated comment about things being slow
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

For the 4 patches,
Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH i-g-t v2 4/4] lib/igt_fb: Generalize the slow read from gtt mmap handling

[Maarten Lankhorst]

Op 05-11-18 om 16:19 schreef Maxime Ripard:
> On Fri, Nov 02, 2018 at 09:37:31PM +0200, Ville Syrjala wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> Make the handling of slow gtt mmap reads generic, and extend it to
>> the pixman converter. Makes the pixman path a bit faster.
>>
>> With testing just XRGB8888 and XBGR8888 on KBL:
>> $ time kms_plane --r pixel-format-pipe-A-planes
>> - real	0m18,757s
>> + real	0m2,635s
>>
>> v2: Use the original src buffer if the malloc fails (Chris)
>>     Drop the duplicated comment about things being slow
>>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Maxime Ripard <maxime.ripard@bootlin.com>
>> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> For the 4 patches,
> Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>
>
> Thanks!
> Maxime
>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

I already had a similar fix to 1/4 in my tree but didn't send it out yet. :)

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

Re: [igt-dev] [PATCH i-g-t v2 4/4] lib/igt_fb: Generalize the slow read from gtt mmap handling

[Maxime Ripard]

On Mon, Nov 05, 2018 at 04:33:28PM +0100, Maarten Lankhorst wrote:
> Op 05-11-18 om 16:19 schreef Maxime Ripard:
> > On Fri, Nov 02, 2018 at 09:37:31PM +0200, Ville Syrjala wrote:
> >> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >> Make the handling of slow gtt mmap reads generic, and extend it to
> >> the pixman converter. Makes the pixman path a bit faster.
> >>
> >> With testing just XRGB8888 and XBGR8888 on KBL:
> >> $ time kms_plane --r pixel-format-pipe-A-planes
> >> - real	0m18,757s
> >> + real	0m2,635s
> >>
> >> v2: Use the original src buffer if the malloc fails (Chris)
> >>     Drop the duplicated comment about things being slow
> >>
> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >> Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> >> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> >> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> >> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > For the 4 patches,
> > Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>
> >
> > Thanks!
> > Maxime
> >
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> 
> I already had a similar fix to 1/4 in my tree but didn't send it out yet. :)

I'm curious, how did you find that issue in the first place?

It went through CI and I could run the kms_plane tests on my test
laptop, so I'm confused.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v2 4/4] lib/igt_fb: Generalize the slow read from gtt mmap handling

[Ville Syrjälä]

On Mon, Nov 05, 2018 at 04:41:22PM +0100, Maxime Ripard wrote:
> On Mon, Nov 05, 2018 at 04:33:28PM +0100, Maarten Lankhorst wrote:
> > Op 05-11-18 om 16:19 schreef Maxime Ripard:
> > > On Fri, Nov 02, 2018 at 09:37:31PM +0200, Ville Syrjala wrote:
> > >> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >>
> > >> Make the handling of slow gtt mmap reads generic, and extend it to
> > >> the pixman converter. Makes the pixman path a bit faster.
> > >>
> > >> With testing just XRGB8888 and XBGR8888 on KBL:
> > >> $ time kms_plane --r pixel-format-pipe-A-planes
> > >> - real	0m18,757s
> > >> + real	0m2,635s
> > >>
> > >> v2: Use the original src buffer if the malloc fails (Chris)
> > >>     Drop the duplicated comment about things being slow
> > >>
> > >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > >> Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> > >> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > >> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> > >> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > For the 4 patches,
> > > Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > >
> > > Thanks!
> > > Maxime
> > >
> > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > 
> > I already had a similar fix to 1/4 in my tree but didn't send it out yet. :)
> 
> I'm curious, how did you find that issue in the first place?
> 
> It went through CI and I could run the kms_plane tests on my test
> laptop, so I'm confused.

XBGR8888 fb contents were gibberish. Since you disabled that
format in kms_plane we don't normally see the problem at all.

Unfortunately this doesn't fix the XBGR8888 crc fail. That one looks to
be some odd hardware issue as it still fails even if I change the LUT
programming to keep only the single msb. The colors on the screen do
look reasonably correct, and the fb data looked correct (apart from
0 vs. 0xff alpha, but manually changing that didn't make a different
either). So for now this remains a mystery :(

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

Re: [igt-dev] [PATCH i-g-t v2 4/4] lib/igt_fb: Generalize the slow read from gtt mmap handling

[Maarten Lankhorst]

Op 05-11-18 om 16:41 schreef Maxime Ripard:
> On Mon, Nov 05, 2018 at 04:33:28PM +0100, Maarten Lankhorst wrote:
>> Op 05-11-18 om 16:19 schreef Maxime Ripard:
>>> On Fri, Nov 02, 2018 at 09:37:31PM +0200, Ville Syrjala wrote:
>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>
>>>> Make the handling of slow gtt mmap reads generic, and extend it to
>>>> the pixman converter. Makes the pixman path a bit faster.
>>>>
>>>> With testing just XRGB8888 and XBGR8888 on KBL:
>>>> $ time kms_plane --r pixel-format-pipe-A-planes
>>>> - real	0m18,757s
>>>> + real	0m2,635s
>>>>
>>>> v2: Use the original src buffer if the malloc fails (Chris)
>>>>     Drop the duplicated comment about things being slow
>>>>
>>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>>> Cc: Maxime Ripard <maxime.ripard@bootlin.com>
>>>> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>>>> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> For the 4 patches,
>>> Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>
>>>
>>> Thanks!
>>> Maxime
>>>
>> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>
>> I already had a similar fix to 1/4 in my tree but didn't send it out yet. :)
> I'm curious, how did you find that issue in the first place?
>
> It went through CI and I could run the kms_plane tests on my test
> laptop, so I'm confused.
>
> Maxime
>
I have some patches internally that converted color encodings and used cvt->dst/src.fb->drm_format. It failed with the RG24 there.

Took a bit to figure it out. :)

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