On Wed, Jan 16, 2013 at 01:36:17PM +0100, Daniel Vetter wrote:
> On Wed, Jan 16, 2013 at 11:01 AM, Thierry Reding
> <thierry.reding@xxxxxxxxxxxxxxxxx> wrote:
> > drm_events_release() should be enough to clean up the events, but I
> > suspect the reason why Laurent put that code in was that the drm_crtc
> > private data still has a reference to the event and needs to clear it.
> > Otherwise the next page flip won't be scheduled because .page_flip()
> > would return -EBUSY.
>
> Hm, indeed we seem to have a nice bug in most drivers there :(
>
> > However, it seems like {tegra_dc,shmob_drm_crtc}_cancel_page_flip()
> > could both be simplified a lot and just set their event to NULL. Then
> > again, maybe keeping a separate reference isn't all that useful. Maybe
> > the better thing to do here is iterate over the list of pending VBLANK
> > events in *_finish_page_flip() and process each of them? That would
> > allow more than one user-space process to queue page flips.
>
> I think we need a slightly more generally useful solution, since most
> drivers are currently broken. I've read a bit through the code, but
> short of refcounting drm events and adding event->file_priv checks at
> relevent places I don't see a sane solution ... And even that one is
> rather invasive. Do you have an idea? Imo doing the cleanup in each
> driver will be rather error-prone, and since usually kms clients wait
> for flips to complete, also guaranteed to be little tested.
While this probably doesn't improve the situation much, maybe adding
more extensive tests to libdrm or so would help. I wrote a couple of
small programs to test vblank and plane support.
The vblank one basically generates two framebuffers with different
patterns and uses page-flipping to alternate between them. The plane
test does something similar, sets up a plane and associates a buffer
with it. It includes scaling the plane to test that functionality as
well.
Perhaps these tests could even be added to the existing libdrm tests,
but maybe having separate binaries wouldn't hurt.
Back to the original topic: should it not work to queue a page flip and
immediately exit(0) after that to test the cleanup code? I suppose if
that can be tested on all drivers we should have pretty good coverage.
Thierry
Attachment:
pgpyPYxk04X1R.pgp
Description: PGP signature
