Re: [igt-dev] [PATCH i-g-t] igt/tests/kms_dp_tiled_display.c : kms test file for display port tiled displays

Ser, Simon
2019-08-19T14:50:07Z

On Fri, 2019-08-16 at 17:01 -0700, Tolakanahalli Pradeep, Madhumitha wrote:
> On Thu, 2019-08-15 at 12:30 +0100, Ser, Simon wrote:
> > wOn Wed, 2019-08-14 at 15:05 -0700, Tolakanahalli Pradeep, Madhumitha
> > wrote:
> > > On Wed, 2019-08-14 at 14:43 +0100, Ser, Simon wrote:
> > > > On Wed, 2019-08-07 at 18:10 -0700, Tolakanahalli Pradeep,
> > > > Madhumitha
> > > > wrote:
> > > > > > > I believe tests in kms_flip do a similar check. The
> > > > > > > page_flip_handler
> > > > > > > function handles the page flip event (which is given the
> > > > > > > flip
> > > > > > > timestamp
> > > > > > > and sequence number). check_state verifies that the
> > > > > > > timestamp
> > > > > > > is
> > > > > > > close
> > > > > > > enough to what we expect (in our atomic check, maybe we can
> > > > > > > compare
> > > > > > > with strict equality).
> > > > > > >
> > > > > > > Since we're dealing with multiple CRTCs, it may be a good
> > > > > > > idea
> > > > > > > to
> > > > > > > use
> > > > > > > drmEventContext.version = 3 with page_flip_handler2 so that
> > > > > > > we
> > > > > > > also
> > > > > > > get
> > > > > > > the CRTC in the page flip handler. Version 3 is a core DRM
> > > > > > > feature,
> > > > > > > all
> > > > > > > drivers support it given a recent enough kernel.
> > > > > > >
> > > > > > > Rough pseudo-code:
> > > > > > >
> > > > > > > /* When committing buffer, request a page flip */
> > > > > > > drmModeAtomicCommit(fd, req, DRM_MODE_ATOMIC_NONBLOCK |
> > > > > > > DRM_MODE_PAGE_FLIP_EVENT, data);
> > > > > >
> > > > > > Here we do the modese with igt_display_commit2 call that then
> > > > > > calls
> > > > > > drmModeAtomicCommit but
> > > > > > doesnt call it with Page flip, do we have another igt helper
> > > > > > that
> > > > > > does a atomic commit with
> > > > > > page flip event?
> > > > > >
> > > > > > Can we not just do atomic commit and get vblank events so for
> > > > > > two
> > > > > > crtcs we get two events and compare
> > > > > > the timestamp and make sure they are close enough?
> > > >
> > > > I believe doing an atomic commit with DRM_MODE_PAGE_FLIP_EVENT as
> > > > Madhumitha suggested should be enough.
> > >
> > > Looks like using DRM_MODE_PAGE_FLIP_EVENT is giving me a errno 22,
> > > invalid argument.
> >
> > Have you tried DRM_MODE_PAGE_FLIP_EVENT | DRM_MODE_ATOMIC_NONBLOCK?
> >
> > Enabling verbose dmesg DRM logging can help figuring out why the
> > kernel
> > sends "invalid ar"EINVAL:
> >
> > echo 0x00 | sudo tee /sys/module/drm/parameters/debug
>
> Yes, I did try a commit with DRM_MODE_PAGE_FLIP_EVENT |
> DRM_MODE_ATOMIC_NONBLOCK but IGT would not let me use them in
> combination and the test failed with errno 22.
> The cause of ret = -22 was the commit was failing in
> drm_atomic_check_only(). Corresponding dmesg logs -
> [drm:drm_atomic_check_only] [CRTC:189:pipe C] requesting event but off
> [drm:drm_atomic_check_only] [CRTC:189:pipe C] atomic core check failed
>
> I was able to get igt_display_commit2 to work with
> DRM_MODE_ATOMIC_NONBLOCK and DRM_MODE_ATOMIC_ALLOW_MODESET (more like a
> hacky way where I modified igt_kms.c and passed these flags). I was
> able to get a successful non-atomic commit (dmesg mentioned "non-atomic
> commit" too). However, poll() always timed out. Here's
> my implementation for poll().
>
> https://gitlab.freedesktop.org/mtolakan/igt-gpu-tools
>
> Am I missing something trivial? Would really appreciate any
> suggestions.

Hi,

Your test is hard to run for me, because I don't have a tiled display.
It would be nice to generate a tiled display setup with Chamelium, by
generating an EDID with the appropriate DisplayID. Maybe I'll have a
stab at adding basic property tests with Chamelium.

To make sure page-flips on multiple outputs in a single atomic commit
works, I implemented a very basic IGT test:
https://sr.ht/5OqA.c

It works properly on my machine. Maybe it can help you figure out
what's wrong.

Maybe this test could be added to kms_flip. I don't think we have it
already (we only test cloned connectors from what I understand).

Simon

> > > > > > Manasi
> > > > > >
> > > > > > > static void page_flip_handler(int fd, unsigned seq,
> > > > > > > unsigned tv_sec,
> > > > > > > unsigned tv_usec,
> > > > > > > unsigned crtc_id, void
> > > > > > > *data)
> > > > > > > {
> > > > > > > /* Compare params with the other page flip */
> > > > > > > }
> > > > > > >
> > > > > > > /* When we get POLLIN in the DRM FD */
> > > > > > > drmEventContext event = {
> > > > > > > .version = 3,
> > > > > > > .page_flip_handler2 = page_flip_handler,
> > > > > > > };
> > > > > > > drmHandleEvent(fd, &event);
> > > > > > >
> > > > >
> > > > > I was trying to implement the above code with
> > > > > igt_display_commit2.
> > > > > Looks like I'm timing out while poll-ing for POLLIN.
> > > > >
> > > > > Is there something I could be doing wrong? According to my
> > > > > understanding, igt_display_commit2 should cause a page-flip
> > > > > event
> > > > > right?
> > > >
> > > > Hmm. Yeah, it should work, assuming you use
> > > > DRM_MODE_PAGE_FLIP_EVENT
> > > > when doing the atomic commit. Can you push your code somewhere
> > > > (e.g.
> > > > on
> > > > a GitLab fork) so that I can have a look?
> > >
> > > Sure!
> > > Here is the link to my forked repository. The commit and polling is
> > > in
> > > igt_main().
> > > https://gitlab.freedesktop.org/mtolakan/igt-gpu-tools
> > > My suspicion is the commit is not asynchronous, which is why I am
> > > unble
> > > to capture the page flip event. Please do let me know what you
> > > think,
> > > thanks!