[igt-dev] [PATCH i-g-t v2 07/13] igt: fb: Account for all planes bpp

[Paul Kocialkowski]

On Tue, 2019-01-08 at 16:19 +0100, Maxime Ripard wrote:
> When allocating a dumb buffer for a format with multiple plane, we need to
> account for all plane's bpp in order to allocate the proper size.
> 
> Let's add all the planes bpp and use the result to allocate our buffer.

As discussed off-list, this approach doesn't work well when multiple
planes and sub-sampling are involved, causing over-allocation.

For instance, NV12 requires at least width * height * 1.5.
The bpp for the second plane is 16 but with four times less pixels due
to sub-sampling. Here, this would amount to width * height * 3 at
least, so over-allocating.

I think dividing each plane's bpp by the sub-sampling before adding it
should work (with a round-up division to be on the safe side for cases
where bpp cannot be exactly dividided by hsub * vsub in the future).

What do you think?

Cheers,

Paul

> Signed-off-by: Maxime Ripard <maxime.ripard at bootlin.com>
> ---
>  lib/igt_fb.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 2c33a899bc04..96d56d0e63be 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -529,6 +529,7 @@ static void clear_yuv_buffer(struct igt_fb *fb)
>  static int create_bo_for_fb(struct igt_fb *fb)
>  {
>  	uint64_t size = calc_fb_size(fb);
> +	unsigned int plane, bpp;
>  	int fd = fb->fd;
>  
>  	/* respect the size requested by the caller */
> @@ -557,10 +558,12 @@ static int create_bo_for_fb(struct igt_fb *fb)
>  		}
>  	}
>  
> +	for (plane = 0; plane < fb->num_planes; plane++)
> +		bpp += fb->plane_bpp[plane];
> +
>  	fb->is_dumb = true;
>  	fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb->height,
> -					     fb->plane_bpp[0],
> -					     &fb->strides[0], &fb->size);
> +					     bpp, &fb->strides[0], &fb->size);
>  
>  	return fb->gem_handle;
>  }
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com