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

Paul Kocialkowski
2019-01-10T10:21:35Z

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@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