Re: [PATCH 5/5] drm/tegra: Implement page-flipping support

[Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2742 bytes --]

Hi Thierry,

On Wednesday 16 January 2013 19:56:06 Thierry Reding wrote:
> On Wed, Jan 16, 2013 at 01:36:17PM +0100, Daniel Vetter wrote:
> > On Wed, Jan 16, 2013 at 11:01 AM, Thierry Reding 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.

Further cleanup of the modetest application is somewhere on my to-do list (but 
probably so low that I'll never get to it unless there's a real incentive 
;-)). It's a good candidate for more page flip testing (there's basic page 
flip support there already).

> 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.

Maybe we need some kind of compliance test suite tool ?

-- 
Regards,

Laurent Pinchart

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]