Hi!

On Tue, Jan 29, 2019 at 02:46:55PM -0500, Lyude Paul wrote:
> On Fri, 2019-01-25 at 15:58 +0100, Maxime Ripard wrote:
> > The else condition is not needed, since all the other conditions return
> > when they are done.
> > 
> > Move the KMS dumb buffer allocation outside of the outer else condition,
> > this will also allow to ease later changes.
> > 
> > Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > ---
> >  lib/igt_fb.c | 25 +++++++++++++++++--------
> >  1 file changed, 17 insertions(+), 8 deletions(-)
> > 
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index d69c3fb2d38d..270db8d6dc90 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -557,15 +557,24 @@ static int create_bo_for_fb(struct igt_fb *fb)
> >  			igt_require(driver_has_gem_api);
> >  			return -EINVAL;
> >  		}
> > -	} else {
> > -		fb->is_dumb = true;
> > -
> > -		fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb-
> > >height,
> > -						     fb->plane_bpp[0],
> > -						     &fb->strides[0], &fb-
> > >size);
> > -
> > -		return fb->gem_handle;
> >  	}
> > +
> > +	/*
> > +	 * The current dumb buffer allocation API doesn't really allow to
> > +	 * specify a custom size or stride. Yet the caller is free to specify
> > +	 * them, so we need to make sure to error out in this case.
> > +	 */
> > +	igt_assert(fb->size == 0);
> > +	igt_assert(fb->strides[0] == 0);
> > +
> > +	fb->size = calc_fb_size(fb);
> 
> ^ Is this supposed to be part of a seperate patch?

Yeah, definitely, I'll split it in another patch, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com