[igt-dev] [PATCH i-g-t v5 03/22] lib/igt_fb: Refactor create_bo_for_fb to prepare for VC4 support

[Lyude Paul]

Reviewed-by: Lyude Paul <lyude at redhat.com>

On Wed, 2019-02-20 at 16:38 +0100, Paul Kocialkowski wrote:
> The current create_bo_for_fb uses a device-specific BO instead of dumb
> allocation when dumb allocation is not appropriate and the driver is
> Intel. Then, it will assert that the parameters are appropriate for
> dumb allocation.
> 
> The conditions related to tiling, size and stride are sufficient for
> needing a device-specific BO and they are not specific to Intel.
> However, a device-specific BO for YUV is only needed for Intel.
> 
> Change the conditions accordingly and set a device_bo variable. This
> variable allows making fb->size calculation common between the
> device-specific and dumb paths. Use the variable after that and
> distinguish between the device types for allocating and error out if
> it's not supported.
> 
> This makes the extra checks that dumb allocation is valid redundant,
> since these cases will always fall under device-specific allocation
> and potentially error out then.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski at bootlin.com>
> ---
>  lib/igt_fb.c | 43 ++++++++++++++++++++++---------------------
>  1 file changed, 22 insertions(+), 21 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 058cfab604b8..60e6bb00b55c 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -610,36 +610,37 @@ static int create_bo_for_fb(struct igt_fb *fb)
>  	unsigned int bpp = 0;
>  	unsigned int plane;
>  	unsigned *strides = &fb->strides[0];
> +	bool device_bo = false;
>  	int fd = fb->fd;
>  
> -	if (is_i915_device(fd) &&
> -	   (fb->tiling || fb->size || fb->strides[0] || igt_format_is_yuv(fb-
> >drm_format))) {
> -		uint64_t size;
> -
> -		size = calc_fb_size(fb);
> +	/*
> +	 * The current dumb buffer allocation API doesn't really allow to
> +	 * specify a custom size or stride. Yet the caller is free to specify
> +	 * them, so we need to make sure to use a device BO then.
> +	 */
> +	if (fb->tiling || fb->size || fb->strides[0] ||
> +	    (is_i915_device(fd) && igt_format_is_yuv(fb->drm_format)))
> +		device_bo = true;
>  
> -		/* respect the size requested by the caller */
> -		if (fb->size == 0)
> -			fb->size = size;
> +	/* Respect the size requested by the caller. */
> +	if (fb->size == 0)
> +		fb->size = calc_fb_size(fb);
>  
> +	if (device_bo) {
>  		fb->is_dumb = false;
> -		fb->gem_handle = gem_create(fd, fb->size);
> -		gem_set_tiling(fd, fb->gem_handle,
> -			       igt_fb_mod_to_tiling(fb->tiling),
> -			       fb->strides[0]);
> +
> +		if (is_i915_device(fd)) {
> +			fb->gem_handle = gem_create(fd, fb->size);
> +			gem_set_tiling(fd, fb->gem_handle,
> +				       igt_fb_mod_to_tiling(fb->tiling),
> +				       fb->strides[0]);
> +		} else {
> +			igt_assert(false);
> +		}
>  
>  		goto out;
>  	}
>  
> -	/*
> -	 * The current dumb buffer allocation API doesn't really allow to
> -	 * specify a custom size or stride. Yet the caller is free to specify
> -	 * them, so we need to make sure to error out in this case.
> -	 */
> -	igt_assert(fb->size == 0);
> -	igt_assert(fb->strides[0] == 0);
> -
> -	fb->size = calc_fb_size(fb);
>  	for (plane = 0; plane < fb->num_planes; plane++)
>  		bpp += DIV_ROUND_UP(fb->plane_bpp[plane],
>  				    plane ? fmt->hsub * fmt->vsub : 1);
-- 
Cheers,
	Lyude Paul