Em Qua, 2015-11-18 às 17:38 +0100, Daniel Vetter escreveu:
> 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.
>
> > > + }
> > > } 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).
That sounds a good idea, but it will require yet another rework to the
code.
> But your cover letter talked about allocating less
> in general, and that problem really should be fixed in the library
> itself.
It is both problems actually. That's why I think we need both this
series so it's still possible to specify a custom stride+size, and also
your patch so the sizes are saner.
> -Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
