Re: [RFC v2 0/7] drm: asynchronous atomic plane update

[Gustavo Padovan <gustavo@padovan.org>]

2017-04-28 Ville Syrjälä <ville.syrjala@linux.intel.com>:

> On Thu, Apr 27, 2017 at 03:36:50PM -0300, Gustavo Padovan wrote:
> > 2017-04-27 Ville Syrjälä <ville.syrjala@linux.intel.com>:
> > 
> > > On Thu, Apr 27, 2017 at 12:15:12PM -0300, Gustavo Padovan wrote:
> > > > From: Gustavo Padovan <gustavo.padovan@collabora.com>
> > > > 
> > > > Hi,
> > > > 
> > > > Second take of Asynchronous Plane Updates over Atomic. Here I looked
> > > > to msm, vc4 and i915 to identify a common pattern to create atomic helpers
> > > > for async updates. So in patch 1 drm_atomic_async_check() and
> > > > drm_atomic_helper_async_commit() are introduced along with driver's plane hooks:
> > > > ->atomic_async_check() and ->atomic_async_commit().
> > > > 
> > > > For now we only support async update for one plane at a time. Also the async
> > > > update can't modify the CRTC so no modesets are allowed.
> > > > 
> > > > Then the other patches add support for it in the drivers. I did virtio mostly
> > > > for testing. i915 have been converted and I've been using it without any
> > > > problem. IGT tests seems to be fine, but there are somewhat random failures
> > > > with or without the async update changes. msm and vc4 are only compile-tested.
> > > > So I think this needs more testing
> > > > 
> > > > I started IGT changes to test the Atomic IOCTL with the new flag:
> > > > 
> > > > https://git.collabora.com/cgit/user/padovan/intel-gpu-tools.git/
> > > > 
> > > > v2:
> > > > 
> > > > Apart from all comments on v1 one extra change I made was to remove the
> > > > constraint of only updating the plane if the queued state didn't touch
> > > > that plane. I believe it was a too cautious of a change, furthermore this
> > > > constraint was affecting throughput negatively on i915.
> > > 
> > > So you're now allowing reordering the updates? As in update A is
> > > scheduled before update B, but update B happens before update A.
> > > That is not a good idea.
> > 
> > That is what already happens with legacy cursor updates. They jump ahead
> > the scheduled update and apply the cursor update.
> 
> Well, that's clearly a bug then. They are supposed to be able to jump
> ahead of other planes, but not themselves.

Right, maybe I didn't checked this correctly. Removing that was a
mistake, I blame my lack of knowledge of such a big subsystem :)
I'll bring that code back.

> 
> I think the real problem is having just one timeline for the entire
> crtc. The proper solution would be to have a timeline for each plane.
> 
> > What we propose here
> > is to do this over atomic when DRM_MODE_ATOMIC_ASYNC_UPDATE flag is set.
> 
> The cursor thing is a hack. Using it as a guideline for something else
> is not a good idea IMO. Reordering, apart from being totally unexpected
> would also cause all sorts of problems because the hardware state at
> the time of the programming the hardware won't match what you checked
> against in your async check function.
> 
> > Async PageFlips should use the same infrastructure in the future.
> 
> I don't quite see why you have to build a parallel infrastructure for
> this stuff. If the driver would do things properly then it could just
> as well do this stuff from the normal path as well. So I figured the
> point of all this was just to unify the hacks to one place pretty much.
> Expanding the hacks to some kind of big infrastrucure is not something
> I'd do.

The issue I wanted to solve in the first place was to create a way to
update cursors over the atomic ioctl as fast (or relatively fast) as
the legacy update. Then discussing this with Daniel Vetter he suggested
to solve a bigger problem: add generic async plane update that would
benefit cursors, async PageFlips (vc4 already has it) and VR (but I
don't really know all the needs for VR).

Gustavo


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel