[Intel-gfx] [PATCH igt 00/10] igt_fb buffer sizes + kms_frontbuffer_tracking

[Paulo Zanoni]

2015-11-18 13:59 GMT-02:00 Daniel Vetter <daniel at ffwll.ch>:
> On Fri, Nov 13, 2015 at 03:12:41PM -0200, Paulo Zanoni wrote:
>> Hello
>>
>> I've been carrying some local IGT patches that reduced the size of buffers
>> created by igt_create_fb() so they would fit the stolen memory, but when I
>> decided to test the tree without them, I concluded the lack of sane sizes was
>> even causing test failures. So here's my attempt to fix this. This series alone
>> should help reducing the number of kms_frontbuffer_tracking failures seen by QA.
>>
>> The last few patches make the FBC tests a little harder. They are all based on
>> the feedback I got from the last patches I sent.
>
> The point of a helper library is that it helps, not that every caller has
> to work around it's choice of size and stride.

Judging by the amount of users, it is helping even without my changes :)

>
> The only thing we need to do here is fix up the selection of stride and
> size to make it not pick the super-conservative value that work even on
> gen2&3. Something like:
>
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 13a6a34982e0..9eb97952ed95 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -87,21 +87,26 @@ static int create_bo_for_fb(int fd, int width, int height, int bpp,
>         if (tiling != LOCAL_DRM_FORMAT_MOD_NONE) {
>                 int v;
>
> -               /* Round the tiling up to the next power-of-two and the
> -                * region up to the next pot fence size so that this works
> -                * on all generations.
> -                *
> -                * This can still fail if the framebuffer is too large to
> -                * be tiled. But then that failure is expected.
> -                */
> -
> -               v = width * bpp / 8;
> -               for (stride = 512; stride < v; stride *= 2)
> -                       ;
> -
> -               v = stride * height;
> -               for (size = 1024*1024; size < v; size *= 2)
> -                       ;
> +               if (gen < 4) {
> +                       /* Round the tiling up to the next power-of-two and the
> +                        * region up to the next pot fence size so that this works
> +                        * on all generations.
> +                        *
> +                        * This can still fail if the framebuffer is too large to
> +                        * be tiled. But then that failure is expected.
> +                        */
> +
> +                       v = width * bpp / 8;
> +                       for (stride = 512; stride < v; stride *= 2)
> +                               ;
> +
> +                       v = stride * height;
> +                       for (size = 1024*1024; size < v; size *= 2)
> +                               ;
> +               } else {
> +                       stride = ALIGN(stride, 512);
> +                       size = ALIGN(size, stride * 32);

Shouldn't it be size = stride * ALIGN(height, 32)?
(it still wouldn't be the minimal size, but would be close to it)

> +               }
>         } else {
>                 /* Scan-out has a 64 byte alignment restriction */
>                 stride = (width * (bpp / 8) + 63) & ~63;
>
>
> Or whatever is the right thing to pick that works on gen4+.

While that sounds like an improvement, it won't solve the
kms_frontbuffer_tracking problem where we want to specify size+stride
since we want all buffers using the same size+stride independently of
tiling/no-tiling.


> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni