Re: [PATCH i-g-t 2/2] Adding kms_nv12 to test display NV12 feature

Konduru, Chandra
2015-09-03T18:34:50Z

> > > > +
> > > > + /* redo AddFB */
> > > > + memset(&f, 0, sizeof(f));
> > > > +
> > > > + f.width = d->fb1_nv12.width;
> > > > + f.height = d->fb1_nv12.height;
> > > > + f.pixel_format = d->fb1_nv12.drm_format;
> > > > + f.flags = LOCAL_DRM_MODE_FB_MODIFIERS;
> > > > + f.handles[0] = d->fb1_nv12.gem_handle;
> > > > + f.pitches[0] = d->fb1_nv12.stride;
> > > > + f.modifier[0] = LOCAL_I915_FORMAT_MOD_Yf_TILED;
> > > > + f.modifier[1] = LOCAL_I915_FORMAT_MOD_Yf_TILED;
> > > > +
> > > > + /* test invalid uv start */
> > > > + f.handles[1] = d->fb1_nv12.gem_handle;
> > > > + f.pitches[1] = d->fb1_nv12.stride;
> > > > + f.offsets[1] = 0; /* invalid uv start */
> > > > + igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2,
> > > &f) != 0);
> > >
> > > For such simple invalid input paramaters tests the usual approach is to
> > > split them out. Also you don't have to do a modeset for addfb (which will
> > > speed things up). Essentially each block with an igt_assert should be its
> > > own subtest.
> > > -Daniel
> > >
> >
> > If each block is split into it's own subtest, if one runs just a subtest
> > and without a modeset, I don't know in which state the system is to start
> > the test and doing a modeset will start with a known state.
>
> ADDFB ioctl doesn't need any known modeset state at all, and that seems to
> be the only ioctl you do. Which means you really don't need to do a
> modeset (it's just overhead). See e.g. the other addfb testcases we have
> in kms_addfb_basic.c.
> -Daniel
I see your point. will check on that and make any necessary updates.