Re: [PATCH 14/17] drm/msm: add atomic support

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, May 28, 2014 at 05:14:21PM +0300, Ville Syrjälä wrote:
> On Wed, May 28, 2014 at 03:21:41PM +0200, Daniel Vetter wrote:
> > On Tue, May 27, 2014 at 07:32:46PM -0400, Rob Clark wrote:
> > > On Tue, May 27, 2014 at 6:09 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > > > On Tue, May 27, 2014 at 04:06:28PM -0400, Rob Clark wrote:
> > > >> On Tue, May 27, 2014 at 3:26 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > > >
> > > > [snip]
> > > >
> > > >> Well, there was the NONBLOCK atomic flag.. I'm not entirely sure if we
> > > >> should hang so much off of that one flag.
> > > >
> > > > Yeah, a separate VBLANK_SYNCED might be useful. Apparently people also
> > > > want non-blocking modesets.
> > > 
> > > random-diverging-from-original-topic-thought.. seems like userspace
> > > just wants a deadline/timeout (hopefully in units of vblanks).. at
> > > least to the level of "I want this to happen on the next vblank (after
> > > rendering completes), or give me an error" vs "I want this to complete
> > > atomically even if it takes a few extra vblanks for things to sort
> > > out"..
> > > 
> > > I guess that amounts to what you mean by VBLANK_SYNCED flag, but
> > > VBLANKED_SYNCED might not be a good name.. at least for some hw all
> > > you can do is vblank sync'd..
> > 
> > Hm, we might as well go full monty and allow userspace to request a
> > specific vblank counter. Would be useful for e.g. queuing up a bunch of
> > video frames, which some hw can do. But that would then require cancelling
> > of existing flips,
> 
> The hard part there would be rolling back the user visible state. But
> if we were to disallow it for anything but simple page flips, we could
> rather easily keep the fb pointers around in the structure that tracks
> the progress of the operation.
> 
> Otherwise cancelling should be trivial when using mmio instead of the
> cs.
> 
> But we could certainly do the target vblank thing without allowing
> multiple queued flips initially, and without a cancel capability.
> 
> However using vblank counter is a bit problematic with multiple crtcs.
> But I guess userspace can try to do the something_else->vbl conversion
> itself if it wants to use some other units.
> 
> There's also the question whether we should allow a target vbl count for
> each crtc, or just one of them.
> 
> We could make it so that you can specify the target vblank count only
> for a single crtc, and the rest of the crtcs will flip soon before or
> soon after. The reason for allowing the "soon before" case is because
> it'll make the implementation much simpler. We just have to perform
> all the register writes somewhere between 'target_vbl-1 - target_vbl'
> of the specified crtc. The order in which the flips actually happen
> then depends on the vblank period and phase of each crtc.
> 
> If user space wants target counts for all crtcs, it could issue separate
> nuclear flips for each crtc, although that does raise the issue that we
> can't check the entire target state, so we can't guarantee that all of
> the nuclear flips will succeed. So that's a bit bad.
> 
> So I guess we could allow a target vbl count for all the crtcs, just
> need to convey that information inside another array in the ioctl to
> avoid imposing a limit in the number of crtcs. But then there's the
> question what happens if only a subset of the involved crtcs have a
> target count. Return an error? Or just pick one of the crtcs that
> did get a target vblank and flip the rest at the same time? I guess
> the latter option is better to allow one crtc to act as the master
> clock for the whole thing, and the rest just hop along as best they
> can.

I guess if userspace asks for a target count on more than one crtc we'd
reject the request. That leaves a bit a window unfortunately where
userspace partially submitted a request and then the kernel starts to tell
it that it's too late. But then I don't think we can really avoid this
situation without going completely nuts on the implementation side.

So a best effort (maybe with the guarantee that if we're too late it wont
happen so that userspace could try to render the next frame and display
that for smoothness) approach should be good enough here.

Anyway, let's not get too distracted with fancy ideas before we have the
basics ;-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch