[igt-dev] [PATCH i-g-t 4/6] lib: Don't call igt_require_fb_modifiers() when no modifier

[Deepak Singh Rawat]

> On Wed, Oct 10, 2018 at 05:21:02PM -0700, Deepak Rawat wrote:
> > vmwgfx doesn't support fb modifier so skip igt_require_fb_modifiers()
> > when modifier are not passed.
> >
> > Signed-off-by: Deepak Rawat <drawat at vmware.com>
> > ---
> >  lib/ioctl_wrappers.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> > index 0929c43f..3a11cb6e 100644
> > --- a/lib/ioctl_wrappers.c
> > +++ b/lib/ioctl_wrappers.c
> > @@ -1678,7 +1678,10 @@ int __kms_addfb(int fd, uint32_t handle,
> >  	struct drm_mode_fb_cmd2 f;
> >  	int ret, i;
> >
> > -	igt_require_fb_modifiers(fd);
> > +	if (modifier)
> > +		igt_require_fb_modifiers(fd);
> > +	else
> > +		flags &= ~DRM_MODE_FB_MODIFIERS;
> 
> This would theoretically change the behviour for i915 at least. Without
> the modifiers flag the kernel will pick the modifier for us based on
> the bo tiling, which in theory might not be what we wanted. But at least
> igt_fb should be fine with that.
> 
> Maybe it would be better to just not pass the flags from the caller at
> all, and instead have __kms_addfb() check if the driver has modifiers
> or not and set the flag based on that?
> 

Thanks Ville for the review. I thought of checking for modifiers support
in __kms_addfb() but I discarded the idea as it looked an overkill to me,
checking each time __kms_addfb() is called. May be static variable will
be a good idea? And yes resetting the flag from caller looks a better way.

Thanks,
Deepak