Re: [igt-dev] [PATCH v7 04/14] fb: Create common function to convert frame formats

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Sep 11, 2018 at 10:47:31AM +0200, Maxime Ripard wrote:
> The current code to convert between two buffer formats is quite tied to the
> cairo surface infrastructure. Since we'll want to reuse it, make that
> function more generic by introducing a common structure that passes all the
> arguments and a common function that will call the right functions we
> needed.
> 
> Reviewed-by: Eric Anholt <eric@anholt.net>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  lib/igt_fb.c | 220 ++++++++++++++++++++++++++++++----------------------
>  1 file changed, 130 insertions(+), 90 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 66bdc02a0ce7..f829df59e9fa 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -1391,6 +1391,19 @@ struct fb_convert_blit_upload {
>  	struct fb_blit_linear linear;
>  };
>  
> +struct fb_convert_buf {
> +	void			*ptr;
> +	struct igt_fb		*fb;
> +};
> +
> +struct fb_convert {
> +	unsigned int		width;
> +	unsigned int		height;

Are we expecting to convert between different sized fbs, or why do we
need these?

> +
> +	struct fb_convert_buf	dst;
> +	struct fb_convert_buf	src;
> +};
> +
>  static uint8_t clamprgb(float val)
>  {
>  	return clamp((int)(val + 0.5f), 0, 255);
> @@ -1411,27 +1424,28 @@ static void write_rgb(uint8_t *rgb24, const struct igt_vec4 *rgb)
>  	rgb24[0] = clamprgb(rgb->d[2]);
>  }
>  
> -static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_upload *blit)
> +static void convert_nv12_to_rgb24(struct fb_convert *cvt)
>  {
>  	int i, j;
>  	const uint8_t *y, *uv;
> -	uint8_t *rgb24 = blit->rgb24.map;
> -	unsigned rgb24_stride = blit->rgb24.stride, planar_stride = blit->linear.stride;
> -	uint8_t *buf = malloc(blit->linear.size);
> -	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(fb->color_encoding,
> -						    fb->color_range);
> +	uint8_t *rgb24 = cvt->dst.ptr;
> +	unsigned int rgb24_stride = cvt->dst.fb->stride;
> +	unsigned int planar_stride = cvt->src.fb->stride;
> +	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);
>  
>  	/*
>  	 * 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, blit->linear.map, blit->linear.size);
> -	y = &buf[blit->linear.offsets[0]];
> -	uv = &buf[blit->linear.offsets[1]];
> +	igt_memcpy_from_wc(buf, cvt->src.ptr, cvt->src.fb->size);
> +	y = cvt->src.ptr + cvt->src.fb->offsets[0];
> +	uv = cvt->src.ptr + cvt->src.fb->offsets[1];
>  
> -	for (i = 0; i < fb->height / 2; i++) {
> -		for (j = 0; j < fb->width / 2; j++) {
> +	for (i = 0; i < cvt->height / 2; i++) {
> +		for (j = 0; j < cvt->width / 2; j++) {
>  			/* Convert 2x2 pixel blocks */
>  			struct igt_vec4 yuv[4];
>  			struct igt_vec4 rgb[4];
> @@ -1456,7 +1470,7 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
>  			write_rgb(&rgb24[j * 8 + 4 + rgb24_stride], &rgb[3]);
>  		}
>  
> -		if (fb->width & 1) {
> +		if (cvt->width & 1) {
>  			/* Convert 1x2 pixel block */
>  			struct igt_vec4 yuv[2];
>  			struct igt_vec4 rgb[2];
> @@ -1480,9 +1494,9 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
>  		uv += planar_stride;
>  	}
>  
> -	if (fb->height & 1) {
> +	if (cvt->height & 1) {
>  		/* Convert last row */
> -		for (j = 0; j < fb->width / 2; j++) {
> +		for (j = 0; j < cvt->width / 2; j++) {
>  			/* Convert 2x1 pixel blocks */
>  			struct igt_vec4 yuv[2];
>  			struct igt_vec4 rgb[2];
> @@ -1500,7 +1514,7 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
>  			write_rgb(&rgb24[j * 8 + 4], &rgb[0]);
>  		}
>  
> -		if (fb->width & 1) {
> +		if (cvt->width & 1) {
>  			/* Convert single pixel */
>  			struct igt_vec4 yuv;
>  			struct igt_vec4 rgb;
> @@ -1519,22 +1533,22 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
>  	free(buf);
>  }
>  
> -static void convert_rgb24_to_nv12(struct igt_fb *fb, struct fb_convert_blit_upload *blit)
> +static void convert_rgb24_to_nv12(struct fb_convert *cvt)
>  {
>  	int i, j;
> -	uint8_t *y = &blit->linear.map[blit->linear.offsets[0]];
> -	uint8_t *uv = &blit->linear.map[blit->linear.offsets[1]];
> -	const uint8_t *rgb24 = blit->rgb24.map;
> -	unsigned rgb24_stride = blit->rgb24.stride;
> -	unsigned planar_stride = blit->linear.stride;
> -	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(fb->color_encoding,
> -						    fb->color_range);
> -
> -	igt_assert_f(fb->drm_format == DRM_FORMAT_NV12,
> +	uint8_t *y = cvt->dst.ptr + cvt->dst.fb->offsets[0];
> +	uint8_t *uv = cvt->dst.ptr + cvt->dst.fb->offsets[1];
> +	const uint8_t *rgb24 = cvt->src.ptr;
> +	unsigned rgb24_stride = cvt->src.fb->stride;
> +	unsigned planar_stride = cvt->dst.fb->stride;
> +	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");
>  
> -	for (i = 0; i < fb->height / 2; i++) {
> -		for (j = 0; j < fb->width / 2; j++) {
> +	for (i = 0; i < cvt->height / 2; i++) {
> +		for (j = 0; j < cvt->width / 2; j++) {
>  			/* Convert 2x2 pixel blocks */
>  			struct igt_vec4 rgb[4];
>  			struct igt_vec4 yuv[4];
> @@ -1563,7 +1577,7 @@ static void convert_rgb24_to_nv12(struct igt_fb *fb, struct fb_convert_blit_uplo
>  			uv[j * 2 + 1] = (yuv[0].d[2] + yuv[2].d[2]) / 2.0f;
>  		}
>  
> -		if (fb->width & 1) {
> +		if (cvt->width & 1) {
>  			/* Convert 1x2 pixel block */
>  			struct igt_vec4 rgb[2];
>  			struct igt_vec4 yuv[2];
> @@ -1592,8 +1606,8 @@ static void convert_rgb24_to_nv12(struct igt_fb *fb, struct fb_convert_blit_uplo
>  	}
>  
>  	/* Last row cannot be interpolated between 2 pixels, take the single value */
> -	if (fb->height & 1) {
> -		for (j = 0; j < fb->width / 2; j++) {
> +	if (cvt->height & 1) {
> +		for (j = 0; j < cvt->width / 2; j++) {
>  			/* Convert 2x1 pixel blocks */
>  			struct igt_vec4 rgb[2];
>  			struct igt_vec4 yuv[2];
> @@ -1610,7 +1624,7 @@ static void convert_rgb24_to_nv12(struct igt_fb *fb, struct fb_convert_blit_uplo
>  			uv[j * 2 + 1] = yuv[0].d[2];
>  		}
>  
> -		if (fb->width & 1) {
> +		if (cvt->width & 1) {
>  			/* Convert single pixel */
>  			struct igt_vec4 rgb;
>  			struct igt_vec4 yuv;
> @@ -1647,27 +1661,28 @@ static const unsigned char *yuyv_swizzle(uint32_t format)
>  	}
>  }
>  
> -static void convert_yuyv_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_upload *blit)
> +static void convert_yuyv_to_rgb24(struct fb_convert *cvt)
>  {
>  	int i, j;
>  	const uint8_t *yuyv;
> -	uint8_t *rgb24 = blit->rgb24.map;
> -	unsigned rgb24_stride = blit->rgb24.stride, yuyv_stride = blit->linear.stride;
> -	uint8_t *buf = malloc(blit->linear.size);
> -	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(fb->color_encoding,
> -						    fb->color_range);
> -	const unsigned char *swz = yuyv_swizzle(fb->drm_format);
> +	uint8_t *rgb24 = cvt->dst.ptr;
> +	unsigned int rgb24_stride = cvt->dst.fb->stride;
> +	unsigned int yuyv_stride = cvt->src.fb->stride;
> +	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);
>  
>  	/*
>  	 * 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, blit->linear.map, blit->linear.size);
> +	igt_memcpy_from_wc(buf, cvt->src.ptr, cvt->src.fb->size);
>  	yuyv = buf;
>  
> -	for (i = 0; i < fb->height; i++) {
> -		for (j = 0; j < fb->width / 2; j++) {
> +	for (i = 0; i < cvt->height; i++) {
> +		for (j = 0; j < cvt->width / 2; j++) {
>  			/* Convert 2x1 pixel blocks */
>  			struct igt_vec4 yuv[2];
>  			struct igt_vec4 rgb[2];
> @@ -1685,7 +1700,7 @@ static void convert_yuyv_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
>  			write_rgb(&rgb24[j * 8 + 4], &rgb[1]);
>  		}
>  
> -		if (fb->width & 1) {
> +		if (cvt->width & 1) {
>  			struct igt_vec4 yuv;
>  			struct igt_vec4 rgb;
>  
> @@ -1706,25 +1721,25 @@ static void convert_yuyv_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
>  	free(buf);
>  }
>  
> -static void convert_rgb24_to_yuyv(struct igt_fb *fb, struct fb_convert_blit_upload *blit)
> +static void convert_rgb24_to_yuyv(struct fb_convert *cvt)
>  {
>  	int i, j;
> -	uint8_t *yuyv = blit->linear.map;
> -	const uint8_t *rgb24 = blit->rgb24.map;
> -	unsigned rgb24_stride = blit->rgb24.stride;
> -	unsigned yuyv_stride = blit->linear.stride;
> -	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(fb->color_encoding,
> -						    fb->color_range);
> -	const unsigned char *swz = yuyv_swizzle(fb->drm_format);
> -
> -	igt_assert_f(fb->drm_format == DRM_FORMAT_YUYV ||
> -		     fb->drm_format == DRM_FORMAT_YVYU ||
> -		     fb->drm_format == DRM_FORMAT_UYVY ||
> -		     fb->drm_format == DRM_FORMAT_VYUY,
> +	uint8_t *yuyv = cvt->dst.ptr;
> +	const uint8_t *rgb24 = cvt->src.ptr;
> +	unsigned rgb24_stride = cvt->src.fb->stride;
> +	unsigned yuyv_stride = cvt->dst.fb->stride;
> +	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(cvt->dst.fb->color_encoding,
> +						    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");
>  
> -	for (i = 0; i < fb->height; i++) {
> -		for (j = 0; j < fb->width / 2; j++) {
> +	for (i = 0; i < cvt->height; i++) {
> +		for (j = 0; j < cvt->width / 2; j++) {
>  			/* Convert 2x1 pixel blocks */
>  			struct igt_vec4 rgb[2];
>  			struct igt_vec4 yuv[2];
> @@ -1741,7 +1756,7 @@ static void convert_rgb24_to_yuyv(struct igt_fb *fb, struct fb_convert_blit_uplo
>  			yuyv[j * 4 + swz[3]] = (yuv[0].d[2] + yuv[1].d[2]) / 2.0f;
>  		}
>  
> -		if (fb->width & 1) {
> +		if (cvt->width & 1) {
>  			struct igt_vec4 rgb;
>  			struct igt_vec4 yuv;
>  
> @@ -1759,27 +1774,59 @@ static void convert_rgb24_to_yuyv(struct igt_fb *fb, struct fb_convert_blit_uplo
>  	}
>  }
>  
> +static void fb_convert(struct fb_convert *cvt)
> +{
> +	if (cvt->dst.fb->drm_format == DRM_FORMAT_RGB888) {
> +		switch (cvt->src.fb->drm_format) {
> +		case DRM_FORMAT_NV12:
> +			convert_nv12_to_rgb24(cvt);
> +			return;
> +		case DRM_FORMAT_YUYV:
> +		case DRM_FORMAT_YVYU:
> +		case DRM_FORMAT_UYVY:
> +		case DRM_FORMAT_VYUY:
> +			convert_yuyv_to_rgb24(cvt);
> +			return;
> +		}
> +	} else if (cvt->src.fb->drm_format == DRM_FORMAT_RGB888) {
> +		switch (cvt->dst.fb->drm_format) {
> +		case DRM_FORMAT_NV12:
> +			convert_rgb24_to_nv12(cvt);
> +			return;
> +		case DRM_FORMAT_YUYV:
> +		case DRM_FORMAT_YVYU:
> +		case DRM_FORMAT_UYVY:
> +		case DRM_FORMAT_VYUY:
> +			convert_rgb24_to_yuyv(cvt);
> +			return;
> +		}
> +	}
> +
> +	igt_assert_f(false,
> +		     "Conversion not implemented (from format 0x%x to 0x%x)\n",
> +		     cvt->src.fb->drm_format, cvt->dst.fb->drm_format);
> +}
> +
>  static void destroy_cairo_surface__convert(void *arg)
>  {
>  	struct fb_convert_blit_upload *blit = arg;
>  	struct igt_fb *fb = blit->fb;
> -
> -	/* Convert linear rgb back! */
> -	switch(fb->drm_format) {
> -	case DRM_FORMAT_NV12:
> -		convert_rgb24_to_nv12(fb, blit);
> -		break;
> -	case DRM_FORMAT_YUYV:
> -	case DRM_FORMAT_YVYU:
> -	case DRM_FORMAT_UYVY:
> -	case DRM_FORMAT_VYUY:
> -		convert_rgb24_to_yuyv(fb, blit);
> -		break;
> -	default:
> -		igt_assert_f(false, "Conversion not implemented for formats 0x%x\n",
> -			     fb->drm_format);
> -	}
> -
> +	struct fb_convert cvt = {
> +		.width	= fb->width,
> +		.height	= fb->height,
> +
> +		.dst	= {
> +			.ptr	= blit->linear.map,
> +			.fb	= blit->fb,
> +		},
> +
> +		.src	= {
> +			.ptr	= blit->shadow_ptr,
> +			.fb	= &blit->shadow_fb,
> +		},
> +	};
> +
> +	fb_convert(&cvt);
>  	igt_fb_unmap_buffer(&blit->shadow_fb, blit->shadow_ptr);
>  	igt_remove_fb(blit->fd, &blit->shadow_fb);
>  
> @@ -1796,6 +1843,7 @@ static void destroy_cairo_surface__convert(void *arg)
>  static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
>  {
>  	struct fb_convert_blit_upload *blit = malloc(sizeof(*blit));
> +	struct fb_convert cvt = { 0 };
>  	int shadow_id;
>  
>  	igt_assert(blit);
> @@ -1823,21 +1871,13 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
>  		memcpy(blit->linear.offsets, fb->offsets, sizeof(fb->offsets));
>  	}
>  
> -	/* Convert to linear rgb! */
> -	switch(fb->drm_format) {
> -	case DRM_FORMAT_NV12:
> -		convert_nv12_to_rgb24(fb, blit);
> -		break;
> -	case DRM_FORMAT_YUYV:
> -	case DRM_FORMAT_YVYU:
> -	case DRM_FORMAT_UYVY:
> -	case DRM_FORMAT_VYUY:
> -		convert_yuyv_to_rgb24(fb, blit);
> -		break;
> -	default:
> -		igt_assert_f(false, "Conversion not implemented for formats 0x%x\n",
> -			     fb->drm_format);
> -	}
> +	cvt.width = fb->width;
> +	cvt.height = fb->height;
> +	cvt.dst.ptr = blit->shadow_ptr;
> +	cvt.dst.fb = &blit->shadow_fb;
> +	cvt.src.ptr = blit->linear.map;
> +	cvt.src.fb = blit->fb;
> +	fb_convert(&cvt);
>  
>  	fb->cairo_surface =
>  		cairo_image_surface_create_for_data(blit->shadow_ptr,
> -- 
> git-series 0.9.1

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