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

Submitted by Maxime Ripard on Jan. 8, 2019, 3:19 p.m.

Details

Message ID 20190108152001.5367-7-maxime.ripard@bootlin.com
State New
Series "igt: chamelium: Test YUV buffers using the Chamelium"
Headers show

Commit Message

Maxime Ripard Jan. 8, 2019, 3:19 p.m.
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.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 lib/igt_fb.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

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;
 }

Comments

Paul Kocialkowski Jan. 10, 2019, 10:21 a.m.
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;
>  }