Re: [PATCH igt v7 4/4] kms_atomic: Add basic atomic modesetting tests

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Oct 29, 2015 at 11:33:48AM +0000, Emil Velikov wrote:
> Hi Dan,
> 
> Suspecting that these were meant for the intel-gfx list, although I
> doubt people will object seeing them here :-)
> 
> On 29 October 2015 at 10:35, Daniel Stone <daniels@collabora.com> wrote:
> > Add tests for KMS atomic modesetting, to exercise the basic interface
> > and test failure/corner cases. Should ensure coherency between the
> > legacy and atomic interfaces.
> >
> > v2: New patch.
> > v3: Disable connector checking for now, as it was causing GPU hangs on
> >     newer kernels.
> > v4: Rebase.
> > v5: Use do_ioctl or do_ioctl_err consistently. Use igt_assert_*()
> >     helper macros rather than igt_assert() directly.
> >     Move assertions into helper/check functions. Define atomic commit
> >     helper.
> > v6: Use do_ioctl_err, and define macros to move errors to
> >     actual callsite, rather than helper functions.
> > v7: Fix RELAX_MODE thinko and refresh CRTC state in find_crtc.
> >
> > Co-authored-by: Micah Fedke <micah.fedke@collabora.com>
> > Signed-off-by: Daniel Stone <daniels@collabora.com>
> > ---
> >  configure.ac           |    2 +-
> >  tests/.gitignore       |    1 +
> >  tests/Makefile.sources |    1 +
> >  tests/kms_atomic.c     | 1345 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 1348 insertions(+), 1 deletion(-)
> >  create mode 100644 tests/kms_atomic.c
> >
> > diff --git a/configure.ac b/configure.ac
> > index 5f97466..97de58a 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -85,7 +85,7 @@ if test "x$GCC" = "xyes"; then
> >  fi
> >  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
> >
> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.55 libdrm])
> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.64 libdrm])
> Afaics you're not using anything intel specific here. I'd suspect one
> wants to bump (add) the libdrm version, this way igt can remove a lot
> of the local duplication.
> 
> [snip]
> > --- /dev/null
> > +++ b/tests/kms_atomic.c
> [snip]
> > +#ifndef DRM_CLIENT_CAP_ATOMIC
> > +#define DRM_CLIENT_CAP_ATOMIC 3
> > +#endif
> > +
> > +#ifndef DRM_CAP_CURSOR_WIDTH
> > +#define DRM_CAP_CURSOR_WIDTH 0x8
> > +#endif
> > +
> > +#ifndef DRM_CAP_CURSOR_HEIGHT
> > +#define DRM_CAP_CURSOR_HEIGHT 0x9
> > +#endif
> > +
> > +#ifndef DRM_MODE_ATOMIC_TEST_ONLY
> > +#define DRM_MODE_ATOMIC_TEST_ONLY 0x0100
> > +#define DRM_MODE_ATOMIC_NONBLOCK 0x0200
> > +#define DRM_MODE_ATOMIC_ALLOW_MODESET 0x0400
> > +
> > +struct drm_mode_atomic {
> > +       __u32 flags;
> > +       __u32 count_objs;
> > +       __u64 objs_ptr;
> > +       __u64 count_props_ptr;
> > +       __u64 props_ptr;
> > +       __u64 prop_values_ptr;
> > +       __u64 reserved;
> > +       __u64 user_data;
> > +};
> > +#endif
> > +
> ... like the above but we still need the cursor bits :-(
> 
> [snip]
> > +#define MAX_PLANES 15
> > +#define MAX_CRTCS 3
> > +#define MAX_CONNECTORS 8
> > +
> Worth adding a note where these MAX values come from - intel specific,
> educated guess, etc ?

They're definitely too low for radeon (has up to 6 crtcs) and iirc other
limits are similarly funky. I guess we could go with kmalloc or just make
the limits something huge like 256 or so.
-Daniel

> 
> [snip]
> > +#if 0
> > +/* XXX: Checking this repeatedly actually hangs the GPU. I have literally no
> > + *      idea why. */
> > +static void
> > +connector_check_current_state(struct kms_atomic_connector_state *connector)
> Perhaps add a reference to the bugzilla ticket/discussion thread ?
> 
> As it goes - just my 2c, fwiw.
> 
> Cheers,
> Emil
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel