On Wed, Nov 18, 2015 at 05:38:47PM +0100, Daniel Vetter wrote:
> On Wed, Nov 18, 2015 at 5:20 PM, Paulo Zanoni <przanoni@xxxxxxxxx> wrote:
> > 2015-11-18 13:59 GMT-02:00 Daniel Vetter <daniel@xxxxxxxx>:
> >> 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)
>
> Yeah that's probably what we want.
Or just have a helper to get us the actual tile size and use that.
>
> >> + }
> >> } 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.
>
> Matching stride is a good reason for your changes (and then
> kms_frontbuffer_tracking should allocate the tiled fb first and then
> ask for an untiled fb with matching stride to avoid reimplementing the
> stride rounding). But your cover letter talked about allocating less
> in general, and that problem really should be fixed in the library
> itself.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
