[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 at gmail.com
Tue May 14 06:34:45 UTC 2019


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 at 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 at 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 at linux.intel.com>
> Cc: Paul Kocialkowski <paul.kocialkowski at 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 */
> 



More information about the igt-dev mailing list