Re: [igt-dev] [PATCH v6 03/13] fb: Create common function to convert frame formats

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

On Fri, Aug 31, 2018 at 03:12:12PM +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 | 243 ++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 153 insertions(+), 90 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 4061fedec0c1..1914233786a5 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -1384,6 +1384,24 @@ struct fb_convert_blit_upload {
>  	struct fb_blit_linear linear;
>  };
>  
> +struct fb_convert_buf {
> +	void			*ptr;
> +	unsigned int		stride;
> +	unsigned int		size;
> +	uint32_t		fmt;
> +	enum igt_color_encoding	color_encoding;
> +	enum igt_color_range	color_range;
> +	uint32_t		offsets[4];
> +};

I wonder if we can just use igt_fb for that instead of duplicating most
of it?

I have a patch that did something like that for the fb_blit_upload
stuff: https://patchwork.freedesktop.org/patch/239832/

> +
> +struct fb_convert {
> +	unsigned int		width;
> +	unsigned int		height;
> +
> +	struct fb_convert_buf	dst;
> +	struct fb_convert_buf	src;
> +};
> +
>  static uint8_t clamprgb(float val)
>  {
>  	return clamp((int)(val + 0.5f), 0, 255);
> @@ -1404,27 +1422,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.stride;
> +	unsigned int planar_stride = cvt->src.stride;
> +	uint8_t *buf = malloc(cvt->src.size);
> +	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.color_encoding,
> +						    cvt->src.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.size);
> +	y = cvt->src.ptr + cvt->src.offsets[0];
> +	uv = cvt->src.ptr + cvt->src.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];
> @@ -1449,7 +1468,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];
> @@ -1473,9 +1492,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];
> @@ -1493,7 +1512,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;
> @@ -1512,22 +1531,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.offsets[0];
> +	uint8_t *uv = cvt->dst.ptr + cvt->dst.offsets[1];
> +	const uint8_t *rgb24 = cvt->src.ptr;
> +	unsigned rgb24_stride = cvt->src.stride;
> +	unsigned planar_stride = cvt->dst.stride;
> +	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(cvt->dst.color_encoding,
> +						    cvt->dst.color_range);
> +
> +	igt_assert_f(cvt->dst.fmt == 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];
> @@ -1556,7 +1575,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];
> @@ -1585,8 +1604,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];
> @@ -1603,7 +1622,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;
> @@ -1640,27 +1659,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.stride;
> +	unsigned int yuyv_stride = cvt->src.stride;
> +	uint8_t *buf = malloc(cvt->src.size);
> +	struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.color_encoding,
> +						    cvt->src.color_range);
> +	const unsigned char *swz = yuyv_swizzle(cvt->src.fmt);
>  
>  	/*
>  	 * 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.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];
> @@ -1678,7 +1698,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;
>  
> @@ -1699,25 +1719,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.stride;
> +	unsigned yuyv_stride = cvt->dst.stride;
> +	struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(cvt->dst.color_encoding,
> +						    cvt->dst.color_range);
> +	const unsigned char *swz = yuyv_swizzle(cvt->dst.fmt);
> +
> +	igt_assert_f(cvt->dst.fmt == DRM_FORMAT_YUYV ||
> +		     cvt->dst.fmt == DRM_FORMAT_YVYU ||
> +		     cvt->dst.fmt == DRM_FORMAT_UYVY ||
> +		     cvt->dst.fmt == 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];
> @@ -1734,7 +1754,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;
>  
> @@ -1752,27 +1772,65 @@ 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.fmt == DRM_FORMAT_RGB888) {
> +		switch (cvt->src.fmt) {
> +		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.fmt == DRM_FORMAT_RGB888) {
> +		switch (cvt->dst.fmt) {
> +		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.fmt, cvt->dst.fmt);
> +}
> +
>  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,
> +			.fmt		= fb->drm_format,
> +			.stride		= blit->linear.stride,
> +			.size		= blit->linear.size,
> +			.color_encoding	= fb->color_encoding,
> +			.color_range	= fb->color_range,
> +		},
> +
> +		.src	= {
> +			.ptr		= blit->rgb24.map,
> +			.fmt		= DRM_FORMAT_RGB888,
> +			.stride		= blit->rgb24.stride,
> +			.size		= blit->rgb24.size,
> +		},
> +	};
> +
> +	fb_convert(&cvt);
>  	munmap(blit->rgb24.map, blit->rgb24.size);
>  
>  	if (blit->linear.handle)
> @@ -1788,6 +1846,8 @@ 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 };
> +
>  	igt_assert(blit);
>  
>  	blit->fd = fd;
> @@ -1809,21 +1869,24 @@ 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->rgb24.map;
> +	cvt.dst.fmt = DRM_FORMAT_RGB888;
> +	cvt.dst.stride = blit->rgb24.stride;
> +	cvt.dst.size = blit->rgb24.size;
> +
> +	cvt.src.ptr = blit->linear.map;
> +	cvt.src.fmt = fb->drm_format;
> +	cvt.src.stride	= blit->linear.stride;
> +	cvt.src.size = blit->linear.size;
> +	cvt.src.color_encoding = fb->color_encoding;
> +	cvt.src.color_range = fb->color_range;
> +	memcpy(cvt.dst.offsets, blit->linear.offsets,
> +	       sizeof(blit->linear.offsets));
> +
> +	fb_convert(&cvt);
>  
>  	fb->cairo_surface =
>  		cairo_image_surface_create_for_data(blit->rgb24.map,
> -- 
> 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