[igt-dev] [PATCH i-g-t v4 08/14] igt: fb: Don't pass the stride when allocating a dumb, multi-planar buffer

[Paul Kocialkowski]

Hi,

On Tue, 2019-01-22 at 15:28 +0100, Maxime Ripard wrote:
> The dumb buffer allocation API only considers a single plane, and even
> though allocating multi-planar buffers through it is allowed, the stride it
> gives back is the the width times the bpp passed as an argument.
> 
> That doesn't work in our case, since the bpp is going to be the one we give
> as an argument, but split over three planes so the stride doesn't match
> anymore.
> 
> A proper fix for this would be to have a better dumb buffer allocation API,
> but for the time being, let's do it that way.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard at bootlin.com>

Although this is somewhat related to the concerns of patch 5/, it
definitely doesn't make sense to return this stride in any case, so:

Reviewed-by: Paul Kocialkowski <paul.kocialkowski at bootlin.com>

Cheers,

Paul

> ---
>  lib/igt_fb.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 0f043acce29d..490563c0ea4b 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -531,6 +531,7 @@ static int create_bo_for_fb(struct igt_fb *fb)
>  	const struct format_desc_struct *fmt = lookup_drm_format(fb->drm_format);
>  	uint64_t size = calc_fb_size(fb);
>  	unsigned int plane, bpp;
> +	unsigned *strides = &fb->strides[0];
>  	int fd = fb->fd;
>  
>  	/* respect the size requested by the caller */
> @@ -564,8 +565,22 @@ static int create_bo_for_fb(struct igt_fb *fb)
>  				    plane ? fmt->hsub * fmt->vsub : 1);
>  
>  	fb->is_dumb = true;
> +
> +	/*
> +	 * We can't really pass the stride array here since the dumb
> +	 * buffer allocation is assuming that it operates on one
> +	 * plane, and therefore will calculate the stride as if each
> +	 * pixels were stored on a single plane.
> +	 *
> +	 * This might cause issues at some point on drivers that would
> +	 * change the stride of YUV buffers, but we haven't
> +	 * encountered any yet.
> +	 */
> +	if (fb->num_planes > 1)
> +		strides = NULL;
> +
>  	fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb->height,
> -					     bpp, &fb->strides[0], &fb->size);
> +					     bpp, strides, &fb->size);
>  
>  	return fb->gem_handle;
>  }
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com