Re: [igt-dev] [PATCH i-g-t 1/2] lib/igt_fb: Use cairo conversion in igt_fb_convert_with_stride, v3.

[Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>]

This change look ok to me. To me it look vc4 tiling handling stay on 
same level as what it was before as what Paul was worried about.

Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>

On 10.5.2019 19.02, Maarten Lankhorst wrote:
> Op 10-05-2019 om 17:10 schreef Paul Kocialkowski:
>> Hi,
>>
>> On Fri, 2019-05-10 at 16:19 +0200, Maarten Lankhorst wrote:
>>> Op 10-05-2019 om 15:43 schreef Paul Kocialkowski:
>>>> Hi,
>>>>
>>>> It looks like I fell behind on reviewing earlier version here, sorry
>>>> about that.
>>>>
>>>> On Fri, 2019-05-10 at 14:33 +0200, Maarten Lankhorst wrote:
>>>>> Ever since commit 3fa65f4b532bd9a5b ("fb: Add support for conversions
>>>>> through pixman") we can generate a valid cairo surface for any plane,
>>>>> use this to avoid having to implement our own conversion routine.
>>>>>
>>>>> Instead of duplicating this functionality in igt_fb_convert_with_stride,
>>>>> we can simply convert this to a few cairo calls, because we now support
>>>>> cairo calls to any of the supported framebuffer formats.
>>>> I don't see how that is the case. Cairo definitely does not support our
>>>> tiled formats, so we can't just pipe them into it.
>>>>
>>>> And we already use pixman for converting through the fb_convert call to
>>>> convert_pixman when possible. Also, my understanding is that cairo is
>>>> very limited format-wise, so we're much better off using pixman
>>>> directly (which is is what a non-accelerated cairo surface will do
>>>> anyway).
>>>>
>>>>> This is required to make this function more generic, and convert from any
>>>>> format/modifier to any other format/modifier.
>>>> We've already designed it to be generic, we just need conversion
>>>> helpers from/to (currently only to) hardware-specific tiling modifiers.
>>>>
>>>> We can't expect cairo or pixman to do that job and this change pretty
>>>> much kills support for the vc4 tiling modes we've added.
>>>>
>>>> Unless I'm missing something, that's a NACK on my side.
>>> You have a function that can convert a untiled fb to tiled, but not
>>> the other way around in igt_vc4.c
>> We don't need it for anything at the moment but it wouldn't be a
>> problem to come up with one, sure.
>>
>>> If you could provide that, we could convert from any fb/modifier
>>> format to any fb/modifier format, even with VC4 tiling.
>> I don't fully get the point of that if our tests are not going to use
>> it, but it doesn't hurt either :)
>>
>>> Our internal cairo hooks already provide helpers for conversion, see
>>> igt_get_cairo_surface() which calls
>>> create_cairo_surface__convert()
>> My feeling is that we should kill use of cairo formats and go with
>> pixman directly.
>>
>>> Some conversions can only be done through cairo, converting between
>>> P01x and XRGB8888 cannot be done directly,
>>> our pixman representation for that are done in the floating point
>>> formats.
>> I'm not sure I'm following this point, but if there's a conversion that
>> cairo can do and pixman can't, it feels like the right thing would be
>> to fix pixman to support it. Is there a reason in particular why this
>> wouldn't work?
>>
>>> The only way to convert between some framebuffer formats and
>>> modifiers in i915 is by using those conversion functions,
>>> the fact that igt_fb_convert_with_stride doesn't do those, makes a
>>> truly random hdmi-cmp-planes-random useless.
>> So it boils down to a missing format support issue, not really to an
>> issue with the existing flow.
>>
>>> I was getting CRC mismatches because the i915 modifiers weren't
>>> respected. When I made sure they were being respected
>>> I ended up with a duplicate of the cairo context stuff. So why not
>>> lose a little speed and use that?
>> My counter-suggestion would be to do the opposite and make sure pixman
>> can deal with these cases on its own.
>>
>>> If you write and test a detiler function, I can hook it up for you.
>>> :)
>>> If necessary I can do it myself, to move this patch forward.
>>>
>>> Cairo shouldn't be much slower than pixman, because internally it
>>> already uses pixman calls for the backend.
>> Sure, but that's not cairo's role: cairo is about drawing, while pixman
>> is about pixel conversion. I think it would be best to stick to that.
>>
>> Cheers,
>>
>> Paul
>>
> What about this?
> 
> We could draw reference FB's in vc4 t tiled formats now too..
> 
> ---8<----
>  From 7242edd2cd0096556080e0cda8d4ecea4e3fc58d Mon Sep 17 00:00:00 2001
> From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Date: Tue, 2 Apr 2019 16:54:30 +0200
> Subject: [PATCH i-g-t] lib/igt_fb: Use cairo conversion in
>   igt_fb_convert_with_stride, v4.
> 
> Ever since commit 3fa65f4b532bd9a5b ("fb: Add support for conversions
> through pixman") we can generate a valid cairo surface for any plane,
> use this to avoid having to implement our own conversion routine.
> 
> Instead of duplicating this functionality in igt_fb_convert_with_stride,
> we can simply convert this to a few cairo calls, because we now support
> cairo calls to any of the supported framebuffer formats.
> 
> This is required to make this function more generic, and convert from any
> format/modifier to any other format/modifier.
> 
> Changes since v1:
> - Return fb_id in the cairo case.
> Changes since v2:
> - Remove the manual conversion fallback.
> Changes since v3:
> - Integrate VC4 conversion routines.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>   lib/igt_fb.c  | 129 +++++++++++------------
>   lib/igt_vc4.c | 278 ++++++++++++++++++++++++++++++++------------------
>   lib/igt_vc4.h |  12 +--
>   3 files changed, 242 insertions(+), 177 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index d4929019971c..c6e18397b754 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -1716,17 +1716,25 @@ static void free_linear_mapping(struct fb_blit_upload *blit)
>   	struct igt_fb *fb = blit->fb;
>   	struct fb_blit_linear *linear = &blit->linear;
>   
> -	gem_munmap(linear->map, linear->fb.size);
> -	gem_set_domain(fd, linear->fb.gem_handle,
> -		       I915_GEM_DOMAIN_GTT, 0);
> +	if (igt_vc4_is_tiled(fb->modifier)) {
> +		void *map = igt_vc4_mmap_bo(fd, fb->gem_handle, fb->size, PROT_WRITE);
>   
> -	if (blit->batch)
> -		rendercopy(blit, fb, &linear->fb);
> -	else
> -		blitcopy(fb, &linear->fb);
> +		vc4_fb_convert_plane_to_tiled(fb, map, &linear->fb, &linear->map);
> +
> +		munmap(map, fb->size);
> +	} else {
> +		gem_munmap(linear->map, linear->fb.size);
> +		gem_set_domain(fd, linear->fb.gem_handle,
> +			I915_GEM_DOMAIN_GTT, 0);
> +
> +		if (blit->batch)
> +			rendercopy(blit, fb, &linear->fb);
> +		else
> +			blitcopy(fb, &linear->fb);
>   
> -	gem_sync(fd, linear->fb.gem_handle);
> -	gem_close(fd, linear->fb.gem_handle);
> +		gem_sync(fd, linear->fb.gem_handle);
> +		gem_close(fd, linear->fb.gem_handle);
> +	}
>   
>   	if (blit->batch) {
>   		intel_batchbuffer_free(blit->batch);
> @@ -1751,7 +1759,7 @@ static void setup_linear_mapping(struct fb_blit_upload *blit)
>   	struct igt_fb *fb = blit->fb;
>   	struct fb_blit_linear *linear = &blit->linear;
>   
> -	if (use_rendercopy(fb)) {
> +	if (!igt_vc4_is_tiled(fb->modifier) && use_rendercopy(fb)) {
>   		blit->bufmgr = drm_intel_bufmgr_gem_init(fd, 4096);
>   		blit->batch = intel_batchbuffer_alloc(blit->bufmgr,
>   						      intel_get_drm_devid(fd));
> @@ -1771,23 +1779,35 @@ static void setup_linear_mapping(struct fb_blit_upload *blit)
>   
>   	igt_assert(linear->fb.gem_handle > 0);
>   
> -	/* Copy fb content to linear BO */
> -	gem_set_domain(fd, linear->fb.gem_handle,
> -			I915_GEM_DOMAIN_GTT, 0);
> +	if (igt_vc4_is_tiled(fb->modifier)) {
> +		void *map = igt_vc4_mmap_bo(fd, fb->gem_handle, fb->size, PROT_READ);
>   
> -	if (blit->batch)
> -		rendercopy(blit, &linear->fb, fb);
> -	else
> -		blitcopy(&linear->fb, fb);
> +		linear->map = igt_vc4_mmap_bo(fd, linear->fb.gem_handle,
> +					      linear->fb.size,
> +					      PROT_READ | PROT_WRITE);
>   
> -	gem_sync(fd, linear->fb.gem_handle);
> +		vc4_fb_convert_plane_from_tiled(&linear->fb, &linear->map, fb, map);
>   
> -	gem_set_domain(fd, linear->fb.gem_handle,
> -		       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
> +		munmap(map, fb->size);
> +	} else {
> +		/* Copy fb content to linear BO */
> +		gem_set_domain(fd, linear->fb.gem_handle,
> +				I915_GEM_DOMAIN_GTT, 0);
> +
> +		if (blit->batch)
> +			rendercopy(blit, &linear->fb, fb);
> +		else
> +			blitcopy(&linear->fb, fb);
> +
> +		gem_sync(fd, linear->fb.gem_handle);
> +
> +		gem_set_domain(fd, linear->fb.gem_handle,
> +			I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
>   
> -	/* Setup cairo context */
> -	linear->map = gem_mmap__cpu(fd, linear->fb.gem_handle,
> -				    0, linear->fb.size, PROT_READ | PROT_WRITE);
> +		/* Setup cairo context */
> +		linear->map = gem_mmap__cpu(fd, linear->fb.gem_handle,
> +					0, linear->fb.size, PROT_READ | PROT_WRITE);
> +	}
>   }
>   
>   static void create_cairo_surface__gpu(int fd, struct igt_fb *fb)
> @@ -2902,7 +2922,7 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
>   							     &blit->shadow_fb);
>   	igt_assert(blit->shadow_ptr);
>   
> -	if (use_rendercopy(fb) || use_blitter(fb)) {
> +	if (use_rendercopy(fb) || use_blitter(fb) || igt_vc4_is_tiled(fb->modifier)) {
>   		setup_linear_mapping(&blit->base);
>   	} else {
>   		blit->base.linear.fb = *fb;
> @@ -2983,7 +3003,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_blitter(fb) || use_rendercopy(fb))
> +		else if (use_blitter(fb) || use_rendercopy(fb) || igt_vc4_is_tiled(fb->modifier))
>   			create_cairo_surface__gpu(fd, fb);
>   		else
>   			create_cairo_surface__gtt(fd, fb);
> @@ -3102,58 +3122,23 @@ unsigned int igt_fb_convert_with_stride(struct igt_fb *dst, struct igt_fb *src,
>   					uint64_t dst_modifier,
>   					unsigned int dst_stride)
>   {
> -	struct fb_convert cvt = { };
> -	struct igt_fb linear;
> -	void *dst_ptr, *src_ptr;
> -	uint64_t base_modifier;
> +	/* Use the cairo api to convert */
> +	cairo_surface_t *surf = igt_get_cairo_surface(src->fd, src);
> +	cairo_t *cr;
>   	int fb_id;
>   
> -	if (is_vc4_device(src->fd))
> -		base_modifier = fourcc_mod_broadcom_mod(dst_modifier);
> -	else
> -		base_modifier = dst_modifier;
> -
> -	fb_id = igt_create_fb_with_bo_size(src->fd, src->width, src->height,
> -					   dst_fourcc,
> -					   LOCAL_DRM_FORMAT_MOD_NONE, &linear,
> -					   0, dst_stride);
> +	fb_id = igt_create_fb_with_bo_size(src->fd, src->width,
> +					   src->height, dst_fourcc,
> +					   dst_modifier, dst, 0,
> +					   dst_stride);
>   	igt_assert(fb_id > 0);
>   
> -	src_ptr = igt_fb_map_buffer(src->fd, src);
> -	igt_assert(src_ptr);
> -
> -	dst_ptr = igt_fb_map_buffer(linear.fd, &linear);
> -	igt_assert(dst_ptr);
> -
> -	cvt.dst.ptr = dst_ptr;
> -	cvt.dst.fb = &linear;
> -	cvt.src.ptr = src_ptr;
> -	cvt.src.fb = src;
> -	fb_convert(&cvt);
> -
> -	igt_fb_unmap_buffer(dst, dst_ptr);
> -	igt_fb_unmap_buffer(src, src_ptr);
> -
> -	switch (base_modifier) {
> -	case DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED:
> -		fb_id = igt_vc4_fb_t_tiled_convert(dst, &linear);
> -		break;
> -	case DRM_FORMAT_MOD_BROADCOM_SAND32:
> -	case DRM_FORMAT_MOD_BROADCOM_SAND64:
> -	case DRM_FORMAT_MOD_BROADCOM_SAND128:
> -	case DRM_FORMAT_MOD_BROADCOM_SAND256:
> -		fb_id = vc4_fb_sand_tiled_convert(dst, &linear, dst_modifier);
> -		break;
> -	default:
> -		igt_assert(dst_modifier == LOCAL_DRM_FORMAT_MOD_NONE);
> -	}
> -
> -	igt_assert(fb_id > 0);
> +	cr = igt_get_cairo_ctx(dst->fd, dst);
> +	cairo_set_source_surface(cr, surf, 0, 0);
> +	cairo_paint(cr);
> +	igt_put_cairo_ctx(dst->fd, dst, cr);
>   
> -	if (dst_modifier == LOCAL_DRM_FORMAT_MOD_NONE)
> -		*dst = linear;
> -	else
> -		igt_remove_fb(linear.fd, &linear);
> +	cairo_surface_destroy(surf);
>   
>   	return fb_id;
>   }
> diff --git a/lib/igt_vc4.c b/lib/igt_vc4.c
> index 9a0ba30b4420..4415fa321ceb 100644
> --- a/lib/igt_vc4.c
> +++ b/lib/igt_vc4.c
> @@ -56,6 +56,23 @@
>    * tests.
>    */
>   
> +bool igt_vc4_is_tiled(uint64_t modifier)
> +{
> +	if (modifier >> 56ULL != DRM_FORMAT_MOD_VENDOR_BROADCOM)
> +		return false;
> +
> +	switch (fourcc_mod_broadcom_mod(modifier)) {
> +	case DRM_FORMAT_MOD_BROADCOM_SAND32:
> +	case DRM_FORMAT_MOD_BROADCOM_SAND64:
> +	case DRM_FORMAT_MOD_BROADCOM_SAND128:
> +	case DRM_FORMAT_MOD_BROADCOM_SAND256:
> +	case DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
>   /**
>    * igt_vc4_get_cleared_bo:
>    * @fd: device file descriptor
> @@ -178,63 +195,12 @@ bool igt_vc4_purgeable_bo(int fd, int handle, bool purgeable)
>   	return arg.retained;
>   }
>   
> -unsigned int igt_vc4_fb_t_tiled_convert(struct igt_fb *dst, struct igt_fb *src)
> -{
> -	unsigned int fb_id;
> -	unsigned int i, j;
> -	void *src_buf;
> -	void *dst_buf;
> -	size_t bpp = src->plane_bpp[0];
> -	size_t dst_stride = ALIGN(src->strides[0], 128);
> -
> -	fb_id = igt_create_fb_with_bo_size(src->fd, src->width, src->height,
> -					   src->drm_format,
> -					   DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED,
> -					   dst, 0, dst_stride);
> -	igt_assert(fb_id > 0);
> -
> -	igt_assert(bpp == 16 || bpp == 32);
> -
> -	src_buf = igt_fb_map_buffer(src->fd, src);
> -	igt_assert(src_buf);
> -
> -	dst_buf = igt_fb_map_buffer(dst->fd, dst);
> -	igt_assert(dst_buf);
> -
> -	for (i = 0; i < src->height; i++) {
> -		for (j = 0; j < src->width; j++) {
> -			size_t src_offset = src->offsets[0];
> -			size_t dst_offset = dst->offsets[0];
> -
> -			src_offset += src->strides[0] * i + j * bpp / 8;
> -			dst_offset += igt_vc4_t_tiled_offset(dst_stride,
> -							     src->height,
> -							     bpp, j, i);
> -
> -			switch (bpp) {
> -			case 16:
> -				*(uint16_t *)(dst_buf + dst_offset) =
> -					*(uint16_t *)(src_buf + src_offset);
> -				break;
> -			case 32:
> -				*(uint32_t *)(dst_buf + dst_offset) =
> -					*(uint32_t *)(src_buf + src_offset);
> -				break;
> -			}
> -		}
> -	}
> -
> -	igt_fb_unmap_buffer(src, src_buf);
> -	igt_fb_unmap_buffer(dst, dst_buf);
> -
> -	return fb_id;
> -}
>   
>   /* Calculate the t-tile width so that size = width * height * bpp / 8. */
>   #define VC4_T_TILE_W(size, height, bpp) ((size) / (height) / ((bpp) / 8))
>   
> -size_t igt_vc4_t_tiled_offset(size_t stride, size_t height, size_t bpp,
> -			      size_t x, size_t y)
> +static size_t igt_vc4_t_tiled_offset(size_t stride, size_t height, size_t bpp,
> +				     size_t x, size_t y)
>   {
>   	const size_t t1k_map_even[] = { 0, 3, 1, 2 };
>   	const size_t t1k_map_odd[] = { 2, 1, 3, 0 };
> @@ -308,18 +274,116 @@ size_t igt_vc4_t_tiled_offset(size_t stride, size_t height, size_t bpp,
>   	return offset;
>   }
>   
> -static void vc4_fb_sand_tiled_convert_plane(struct igt_fb *dst, void *dst_buf,
> +static void vc4_fb_convert_plane_to_t_tiled(struct igt_fb *dst, void *dst_buf,
>   					    struct igt_fb *src, void *src_buf,
> -					    size_t column_width_bytes,
> -					    size_t column_height,
>   					    unsigned int plane)
>   {
> +	size_t bpp = src->plane_bpp[plane];
> +	unsigned int i, j;
> +
> +	for (i = 0; i < src->height; i++) {
> +		for (j = 0; j < src->width; j++) {
> +			size_t src_offset = src->offsets[plane];
> +			size_t dst_offset = dst->offsets[plane];
> +
> +			src_offset += src->strides[plane] * i + j * bpp / 8;
> +			dst_offset += igt_vc4_t_tiled_offset(dst->strides[plane],
> +							     dst->height,
> +							     bpp, j, i);
> +
> +			switch (bpp) {
> +			case 16:
> +				*(uint16_t *)(dst_buf + dst_offset) =
> +					*(uint16_t *)(src_buf + src_offset);
> +				break;
> +			case 32:
> +				*(uint32_t *)(dst_buf + dst_offset) =
> +					*(uint32_t *)(src_buf + src_offset);
> +				break;
> +			}
> +		}
> +	}
> +}
> +
> +static void vc4_fb_convert_plane_from_t_tiled(struct igt_fb *dst, void *dst_buf,
> +					      struct igt_fb *src, void *src_buf,
> +					      unsigned int plane)
> +{
> +	size_t bpp = src->plane_bpp[plane];
> +	unsigned int i, j;
> +
> +	for (i = 0; i < src->height; i++) {
> +		for (j = 0; j < src->width; j++) {
> +			size_t src_offset = src->offsets[plane];
> +			size_t dst_offset = dst->offsets[plane];
> +
> +			src_offset += igt_vc4_t_tiled_offset(src->strides[plane],
> +							     src->height,
> +							     bpp, j, i);
> +			src_offset += dst->strides[plane] * i + j * bpp / 8;
> +
> +			switch (bpp) {
> +			case 16:
> +				*(uint16_t *)(dst_buf + dst_offset) =
> +					*(uint16_t *)(src_buf + src_offset);
> +				break;
> +			case 32:
> +				*(uint32_t *)(dst_buf + dst_offset) =
> +					*(uint32_t *)(src_buf + src_offset);
> +				break;
> +			}
> +		}
> +	}
> +}
> +
> +static size_t vc4_sand_tiled_offset(size_t column_width, size_t column_size, size_t x,
> +				    size_t y, size_t bpp)
> +{
> +	size_t offset = 0;
> +	size_t cols_x;
> +	size_t pix_x;
> +
> +	/* Offset to the beginning of the relevant column. */
> +	cols_x = x / column_width;
> +	offset += cols_x * column_size;
> +
> +	/* Offset to the relevant pixel. */
> +	pix_x = x % column_width;
> +	offset += (column_width * y + pix_x) * bpp / 8;
> +
> +	return offset;
> +}
> +
> +static void vc4_fb_convert_plane_to_sand_tiled(struct igt_fb *dst, void *dst_buf,
> +					       struct igt_fb *src, void *src_buf,
> +					       unsigned int plane)
> +{
> +	uint64_t modifier_base = fourcc_mod_broadcom_mod(dst->modifier);
> +	uint32_t column_height = fourcc_mod_broadcom_param(dst->modifier);
> +	uint32_t column_width_bytes, column_width, column_size;
>   	size_t bpp = dst->plane_bpp[plane];
> -	size_t column_width = column_width_bytes * dst->plane_width[plane] /
> -			      dst->width;
> -	size_t column_size = column_width_bytes * column_height;
>   	unsigned int i, j;
>   
> +	switch (modifier_base) {
> +	case DRM_FORMAT_MOD_BROADCOM_SAND32:
> +		column_width_bytes = 32;
> +		break;
> +	case DRM_FORMAT_MOD_BROADCOM_SAND64:
> +		column_width_bytes = 64;
> +		break;
> +	case DRM_FORMAT_MOD_BROADCOM_SAND128:
> +		column_width_bytes = 128;
> +		break;
> +	case DRM_FORMAT_MOD_BROADCOM_SAND256:
> +		column_width_bytes = 256;
> +		break;
> +	default:
> +		igt_assert(false);
> +	}
> +
> +	column_width = column_width_bytes * dst->plane_width[plane] / dst->width;
> +	column_size = column_width_bytes * column_height;
> +
>   	for (i = 0; i < dst->plane_height[plane]; i++) {
>   		for (j = 0; j < src->plane_width[plane]; j++) {
>   			size_t src_offset = src->offsets[plane];
> @@ -346,19 +410,15 @@ static void vc4_fb_sand_tiled_convert_plane(struct igt_fb *dst, void *dst_buf,
>   	}
>   }
>   
> -unsigned int vc4_fb_sand_tiled_convert(struct igt_fb *dst, struct igt_fb *src,
> -				       uint64_t modifier)
> +static void vc4_fb_convert_plane_from_sand_tiled(struct igt_fb *dst, void *dst_buf,
> +						 struct igt_fb *src, void *src_buf,
> +						 unsigned int plane)
>   {
> -	uint64_t modifier_base;
> -	size_t column_width_bytes;
> -	size_t column_height;
> -	unsigned int fb_id;
> -	unsigned int i;
> -	void *src_buf;
> -	void *dst_buf;
> -
> -	modifier_base = fourcc_mod_broadcom_mod(modifier);
> -	column_height = fourcc_mod_broadcom_param(modifier);
> +	uint64_t modifier_base = fourcc_mod_broadcom_mod(src->modifier);
> +	uint32_t column_height = fourcc_mod_broadcom_param(src->modifier);
> +	uint32_t column_width_bytes, column_width, column_size;
> +	size_t bpp = src->plane_bpp[plane];
> +	unsigned int i, j;
>   
>   	switch (modifier_base) {
>   	case DRM_FORMAT_MOD_BROADCOM_SAND32:
> @@ -377,41 +437,63 @@ unsigned int vc4_fb_sand_tiled_convert(struct igt_fb *dst, struct igt_fb *src,
>   		igt_assert(false);
>   	}
>   
> -	fb_id = igt_create_fb(src->fd, src->width, src->height, src->drm_format,
> -			      modifier, dst);
> -	igt_assert(fb_id > 0);
> +	column_width = column_width_bytes * src->plane_width[plane] / src->width;
> +	column_size = column_width_bytes * column_height;
> +
> +	for (i = 0; i < dst->plane_height[plane]; i++) {
> +		for (j = 0; j < src->plane_width[plane]; j++) {
> +			size_t src_offset = src->offsets[plane];
> +			size_t dst_offset = dst->offsets[plane];
>   
> -	src_buf = igt_fb_map_buffer(src->fd, src);
> -	igt_assert(src_buf);
> +			src_offset += vc4_sand_tiled_offset(column_width,
> +							    column_size, j, i,
> +							    bpp);
> +			dst_offset += dst->strides[plane] * i + j * bpp / 8;
>   
> -	dst_buf = igt_fb_map_buffer(dst->fd, dst);
> -	igt_assert(dst_buf);
> +			switch (bpp) {
> +			case 8:
> +				*(uint8_t *)(dst_buf + dst_offset) =
> +					*(uint8_t *)(src_buf + src_offset);
> +				break;
> +			case 16:
> +				*(uint16_t *)(dst_buf + dst_offset) =
> +					*(uint16_t *)(src_buf + src_offset);
> +				break;
> +			default:
> +				igt_assert(false);
> +			}
> +		}
> +	}
> +}
>   
> -	for (i = 0; i < dst->num_planes; i++)
> -		vc4_fb_sand_tiled_convert_plane(dst, dst_buf, src, src_buf,
> -						column_width_bytes,
> -						column_height, i);
> +void vc4_fb_convert_plane_to_tiled(struct igt_fb *dst, void *dst_buf,
> +				     struct igt_fb *src, void *src_buf)
> +{
> +	unsigned int plane;
>   
> -	igt_fb_unmap_buffer(src, src_buf);
> -	igt_fb_unmap_buffer(dst, dst_buf);
> +	igt_assert(src->modifier == DRM_FORMAT_MOD_LINEAR);
> +	igt_assert(igt_vc4_is_tiled(dst->modifier));
>   
> -	return fb_id;
> +	for (plane = 0; plane < src->num_planes; plane++) {
> +		if (dst->modifier == DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED)
> +			vc4_fb_convert_plane_to_t_tiled(dst, dst_buf, src, src_buf, plane);
> +		else
> +			vc4_fb_convert_plane_to_sand_tiled(dst, dst_buf, src, src_buf, plane);
> +	}
>   }
>   
> -size_t vc4_sand_tiled_offset(size_t column_width, size_t column_size, size_t x,
> -			     size_t y, size_t bpp)
> +void vc4_fb_convert_plane_from_tiled(struct igt_fb *dst, void *dst_buf,
> +				       struct igt_fb *src, void *src_buf)
>   {
> -	size_t offset = 0;
> -	size_t cols_x;
> -	size_t pix_x;
> -
> -	/* Offset to the beginning of the relevant column. */
> -	cols_x = x / column_width;
> -	offset += cols_x * column_size;
> +	unsigned int plane;
>   
> -	/* Offset to the relevant pixel. */
> -	pix_x = x % column_width;
> -	offset += (column_width * y + pix_x) * bpp / 8;
> +	igt_assert(igt_vc4_is_tiled(src->modifier));
> +	igt_assert(dst->modifier == DRM_FORMAT_MOD_LINEAR);
>   
> -	return offset;
> +	for (plane = 0; plane < src->num_planes; plane++) {
> +		if (src->modifier == DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED)
> +			vc4_fb_convert_plane_from_t_tiled(dst, dst_buf, src, src_buf, plane);
> +		else
> +			vc4_fb_convert_plane_from_sand_tiled(dst, dst_buf, src, src_buf, plane);
> +	}
>   }
> diff --git a/lib/igt_vc4.h b/lib/igt_vc4.h
> index a17812698fe5..f32bf398b3eb 100644
> --- a/lib/igt_vc4.h
> +++ b/lib/igt_vc4.h
> @@ -29,16 +29,14 @@ int igt_vc4_create_bo(int fd, size_t size);
>   void *igt_vc4_mmap_bo(int fd, uint32_t handle, uint32_t size, unsigned prot);
>   int igt_vc4_get_param(int fd, uint32_t param, uint64_t *val);
>   bool igt_vc4_purgeable_bo(int fd, int handle, bool purgeable);
> +bool igt_vc4_is_tiled(uint64_t modifier);
>   
>   void igt_vc4_set_tiling(int fd, uint32_t handle, uint64_t modifier);
>   uint64_t igt_vc4_get_tiling(int fd, uint32_t handle);
>   
> -unsigned int igt_vc4_fb_t_tiled_convert(struct igt_fb *dst, struct igt_fb *src);
> -size_t igt_vc4_t_tiled_offset(size_t stride, size_t height, size_t bpp,
> -			      size_t x, size_t y);
> -unsigned int vc4_fb_sand_tiled_convert(struct igt_fb *dst, struct igt_fb *src,
> -				       uint64_t modifier);
> -size_t vc4_sand_tiled_offset(size_t column_width, size_t column_size, size_t x,
> -			     size_t y, size_t bpp);
> +void vc4_fb_convert_plane_to_tiled(struct igt_fb *dst, void *dst_buf,
> +				     struct igt_fb *src, void *src_buf);
> +void vc4_fb_convert_plane_from_tiled(struct igt_fb *dst, void *dst_buf,
> +				       struct igt_fb *src, void *src_buf);
>   
>   #endif /* IGT_VC4_H */
> 

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