[igt-dev] [PATCH i-g-t v2 05/13] igt: fb: Move size computation to the common path

[Maxime Ripard]

Hi,

On Thu, Jan 10, 2019 at 11:43:41AM +0100, Paul Kocialkowski wrote:
> On Tue, 2019-01-08 at 16:19 +0100, Maxime Ripard wrote:
> > In order to properly support the YUV buffer on non-i915 platforms, we need
> > to run calc_fb_size for the dumb buffers allocation path too. Move it out
> > of the special case and in the common code path.
> 
> Actually, I have one minor comment below.
> 
> > Signed-off-by: Maxime Ripard <maxime.ripard at bootlin.com>
> > ---
> >  lib/igt_fb.c | 12 +++++-------
> >  1 file changed, 5 insertions(+), 7 deletions(-)
> > 
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index d69c3fb2d38d..31e44d6188d9 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -528,16 +528,14 @@ static void clear_yuv_buffer(struct igt_fb *fb)
> >  /* helpers to create nice-looking framebuffers */
> >  static int create_bo_for_fb(struct igt_fb *fb)
> >  {
> > +	uint64_t size = calc_fb_size(fb);
> >  	int fd = fb->fd;
> >  
> > +	/* respect the size requested by the caller */
> > +	if (fb->size == 0)
> > +		fb->size = size;
> > +
> >  	if (fb->tiling || fb->size || fb->strides[0] || igt_format_is_yuv(fb->drm_format)) {
> 
> Calling calc_fb_size when the function begins will automatically set
> the strides if it was previously 0, so we can drop the "|| fb-
> >strides[0]" here (will never happen).

Good call, thanks!

> Generally speaking, I think it would be much more simple to always go
> with i915 gem create instead of dumb alloc as soon as i915 is detected
> and drop all these conditions.
> 
> I can't see why dumb buffer would be preferable, but I might be missing
> something here (i915 developers, feel free to point it out if that's
> the case)!

I don't have a lot of insights on the i915 hardware, but usually those
kind of pattern come from having two different pools, with one being
more constrained than the other.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/igt-dev/attachments/20190121/94f70721/attachment.sig>