[igt-dev] [PATCH RESEND i-g-t 7/8] igt: fb: Fallback on KMS dumb buffer allocation for YUV buffers

[Paul Kocialkowski]

Hi,

On Tue, 2018-12-04 at 11:25 +0100, Maxime Ripard wrote:
> The current YUV buffer allocation only works on the i915 driver, since
> it uses some private ioctl. However, we can to use that code on other
> drivers that implement only KMS, so if the driver is something else
> than the i915 driver, let's allocate a dumb buffer.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard at bootlin.com>
> ---
>  lib/igt_fb.c | 35 +++++++++++++++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index d6242a6652f1..f2e6c89f3884 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -501,6 +501,8 @@ static int i915_create_gem_for_fb(struct igt_fb *fb)
>  
>  static int create_yuv_bo_for_fb(struct igt_fb *fb)
>  {
> +	unsigned int virtual_height;
> +	unsigned int bpp;
>  	uint64_t size = calc_fb_size(fb);
>  	int fd = fb->fd;
>  
> @@ -511,8 +513,37 @@ static int create_yuv_bo_for_fb(struct igt_fb *fb)
>  	if (is_i915_device(fd))
>  		return i915_create_gem_for_fb(fb);
>  
> -	/* We cannot allocate any other buffer type */
> -	igt_assert(true);
> +	switch (fb->drm_format) {
> +	case DRM_FORMAT_NV12:
> +		bpp = 8;
> +		break;
> +
> +	case DRM_FORMAT_UYVY:
> +	case DRM_FORMAT_VYUY:
> +	case DRM_FORMAT_YUYV:
> +	case DRM_FORMAT_YVYU:
> +		bpp = 16;
> +		break;

It's probably not necessary to duplicate the bpp here since it's
already described in the format_desc array, from which it gets copied
to the fb->plane_bpp[i] before calling this function. You can probably
reuse it from there directly.

> +	default:
> +		igt_assert_f(false, "Unsupported YUV format\n");
> +	}
> +
> +	switch (fb->drm_format) {
> +	case DRM_FORMAT_NV12:
> +		virtual_height = fb->height * 3 / 2;

If this is the correct way to go for allocating two planes at once,
perhaps there should be a comment about it to clarify the meaning of
this operation?

Also, this could probably be generalized to other formats with two (or
three) planes by using each plane's bpp.

What do yout think?

Cheers,

Paul

> +		break;
> +
> +	default:
> +		virtual_height = fb->height;
> +		break;
> +	}
> +
> +	fb->is_dumb = true;
> +	fb->gem_handle = kmstest_dumb_create(fd, fb->width, virtual_height,
> +					     bpp, NULL, &fb->size);
> +
> +	return fb->gem_handle;
>  }
>  
>  /* helpers to create nice-looking framebuffers */
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com