[PATCH 16/19] drm/atomic-helpers: document how to implement async commit
robdclark at gmail.com
Fri Aug 1 04:52:21 PDT 2014
On Sun, Jul 27, 2014 at 5:41 PM, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> No helper function to do it all yet provided since no driver has
> support for driver core fences yet. Which we'd need to make the
> implementation really generic.
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> drivers/gpu/drm/drm_atomic_helper.c | 37 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 9f7c45b91fe2..423f6bcec362 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -716,6 +716,43 @@ int drm_atomic_helper_commit(struct drm_device *dev,
> + * DOC: Implementing async commit
> + *
> + * For now the atomic helpers don't support async commit directly. If there is
> + * real need it could be added though, using the dma-buf fence infrastructure
> + * for generic synchronization with outstanding rendering.
> + *
> + * For now drivers have to implement async commit themselves, with the following
> + * sequence being the recommened one:
> + *
> + * 1. Run drm_atomic_helper_prepare_planes() first. This is the only function
> + * which commit needs to call which can fail, so we want to run it first and
> + * synchronously.
Without some converted driver to look at for reference, I wasn't
completely sure whether prepare_fb/cleanup_fb was intended to do some
refcnt'ing, etc. Or just driver internal pin/unpin?
> + * 2. Synchronize with any outstanding async commit worker threads which might
> + * affect the new state update. This can be done by either cancelling or
> + * flushing the work items, depending upon whether the driver can deal with
> + * cancelled updates. Note that it is important to ensure that the framebuffer
> + * cleanup is still done when cancelling.
maybe just an issue of wording.. but if an async update was writing
the state object, then we'd need to block earlier than commit. I
don't think that is what you meant, because the state object data
structure synchronization you have relies on read-only access from the
async commit. Which would make me expect it would be sufficient to
move the synchronization part into the async commit worker (ie. it
would only need to sync w/ other workers, ie if you had multiple
> + *
> + * For sufficient parallelism it is recommended to have a work item per crtc
> + * (for updates which don't touch global state) and a global one. Then we only
> + * need to synchronize with the crtc work items for changed crtcs and the global
> + * work item, which allows nice concurrent updates on disjoint sets of crtcs.
s/work item/work queue/?
> + * 3. The software state is updated synchronously with
> + * drm_atomic_helper_swap_state. Doing this under the protection of all modeset
> + * locks means concurrent callers never see inconsistent state. And doing this
> + * while it's guranateed that no relevant async worker runs means that async
> + * workers do not need grab any locks. Actually they must not grab locks, for
> + * otherwise the work flushing will deadlock.
> + *
> + * 4. Schedule a work item to do all subsequent steps, using the split-out
> + * commit helpers: a) pre-plane commit b) plane commit c) post-plane commit and
> + * then cleaning up the framebuffers after one vblank has passed.
the *old* framebuffers..
I assume the atomic helpers don't really care how the driver does it,
as long as it does. Somehow or another the driver would want to
schedule cleanup on the vblank of the *old* crtc, I guess.
> + */
> * drm_atomic_helper_prepare_planes - prepare plane resources after commit
> * @dev: DRM device
> * @state: atomic state object with old state structures
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
More information about the dri-devel