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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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.

-- 
Ville Syrjälä
Intel OTC