| Message ID | 20170525195144.18134-2-gustavo@padovan.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 05/26/2017 01:21 AM, Gustavo Padovan wrote: > From: Gustavo Padovan <gustavo.padovan@collabora.com> > > In some cases, like cursor updates, it is interesting to update the > plane in an asynchronous fashion to avoid big delays. The current queued > update could be still waiting for a fence to signal and thus block any > subsequent update until its scan out. In cases like this if we update the > cursor synchronously through the atomic API it will cause significant > delays that would even be noticed by the final user. > > This patch creates a fast path to jump ahead the current queued state and > do single planes updates without going through all atomic steps in > drm_atomic_helper_commit(). We take this path for legacy cursor updates. > > For now only single plane updates are supported, but we plan to support > multiple planes updates and async PageFlips through this interface as well > in the near future. Reviewed-by: Archit Taneja <architt@codeaurora.org> > > v4: > - fix state->crtc NULL check (Archit Taneja) > > v3: > - fix iteration on the wrong crtc state > - put back code to forbid updates if there is a queued update for > the same plane (Ville Syrjälä) > - move size checks back to drivers (Ville Syrjälä) > - move ASYNC_UPDATE flag addition to its own patch (Ville Syrjälä) > > v2: > - allow updates even if there is a queued update for the same > plane. > - fixes on the documentation (Emil Velikov) > - unconditionally call ->atomic_async_update (Emil Velikov) > - check for ->atomic_async_update earlier (Daniel Vetter) > - make ->atomic_async_check() the last step (Daniel Vetter) > - add ASYNC_UPDATE flag (Eric Anholt) > - update state in core after ->atomic_async_update (Eric Anholt) > - update docs (Eric Anholt) > > Cc: Daniel Vetter <daniel.vetter@intel.com> > Cc: Rob Clark <robdclark@gmail.com> > Cc: Eric Anholt <eric@anholt.net> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com> > --- > drivers/gpu/drm/drm_atomic.c | 65 ++++++++++++++++++++++++++++++++ > drivers/gpu/drm/drm_atomic_helper.c | 35 +++++++++++++++++ > include/drm/drm_atomic.h | 2 + > include/drm/drm_atomic_helper.h | 2 + > include/drm/drm_modeset_helper_vtables.h | 48 +++++++++++++++++++++++ > 5 files changed, 152 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index be62774..2a0c297 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -631,6 +631,68 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc, > return 0; > } > > +static bool drm_atomic_async_check(struct drm_atomic_state *state) > +{ > + struct drm_crtc *crtc; > + struct drm_crtc_state *crtc_state; > + struct drm_crtc_commit *commit; > + struct drm_plane *__plane, *plane = NULL; > + struct drm_plane_state *__plane_state, *plane_state = NULL; > + const struct drm_plane_helper_funcs *funcs; > + int i, j, n_planes = 0; > + > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { > + if (drm_atomic_crtc_needs_modeset(crtc_state)) > + return false; > + } > + > + for_each_new_plane_in_state(state, __plane, __plane_state, i) { > + n_planes++; > + plane = __plane; > + plane_state = __plane_state; > + } > + > + /* FIXME: we support only single plane updates for now */ > + if (!plane || n_planes != 1) > + return false; > + > + if (!plane_state->crtc) > + return false; > + > + funcs = plane->helper_private; > + if (!funcs->atomic_async_update) > + return false; > + > + if (plane_state->fence) > + return false; > + > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { > + if (plane->crtc != crtc) > + continue; > + > + spin_lock(&crtc->commit_lock); > + commit = list_first_entry_or_null(&crtc->commit_list, > + struct drm_crtc_commit, > + commit_entry); > + if (!commit) { > + spin_unlock(&crtc->commit_lock); > + continue; > + } > + spin_unlock(&crtc->commit_lock); > + > + if (!crtc->state->state) > + continue; > + > + for_each_plane_in_state(crtc->state->state, __plane, > + __plane_state, j) { > + if (__plane == plane) > + return false; > + } > + } > + > + return !funcs->atomic_async_check(plane, plane_state); > +} > + > static void drm_atomic_crtc_print_state(struct drm_printer *p, > const struct drm_crtc_state *state) > { > @@ -1591,6 +1653,9 @@ int drm_atomic_check_only(struct drm_atomic_state *state) > } > } > > + if (state->legacy_cursor_update) > + state->async_update = drm_atomic_async_check(state); > + > return ret; > } > EXPORT_SYMBOL(drm_atomic_check_only); > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 5a3c9c0..a8efdfe 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1235,6 +1235,30 @@ static void commit_work(struct work_struct *work) > } > > /** > + * drm_atomic_helper_async_commit - commit state asynchronously > + * > + * This function commits a state asynchronously, i.e., not vblank > + * synchronized. It should be used on a state only when > + * drm_atomic_async_check() succeeds. Async commits are not supposed to swap > + * the states like normal sync commits, but just do in-place changes on the > + * current state. > + */ > +void drm_atomic_helper_async_commit(struct drm_device *dev, > + struct drm_atomic_state *state) > +{ > + struct drm_plane *plane; > + struct drm_plane_state *plane_state; > + const struct drm_plane_helper_funcs *funcs; > + int i; > + > + for_each_new_plane_in_state(state, plane, plane_state, i) { > + funcs = plane->helper_private; > + funcs->atomic_async_update(plane, plane_state); > + } > +} > +EXPORT_SYMBOL(drm_atomic_helper_async_commit); > + > +/** > * drm_atomic_helper_commit - commit validated state object > * @dev: DRM device > * @state: the driver state object > @@ -1258,6 +1282,17 @@ int drm_atomic_helper_commit(struct drm_device *dev, > { > int ret; > > + if (state->async_update) { > + ret = drm_atomic_helper_prepare_planes(dev, state); > + if (ret) > + return ret; > + > + drm_atomic_helper_async_commit(dev, state); > + drm_atomic_helper_cleanup_planes(dev, state); > + > + return 0; > + } > + > ret = drm_atomic_helper_setup_commit(state, nonblock); > if (ret) > return ret; > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index 788daf7..70ca279 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -160,6 +160,7 @@ struct __drm_connnectors_state { > * @dev: parent DRM device > * @allow_modeset: allow full modeset > * @legacy_cursor_update: hint to enforce legacy cursor IOCTL semantics > + * @async_update: hint for asynchronous plane update > * @planes: pointer to array of structures with per-plane data > * @crtcs: pointer to array of CRTC pointers > * @num_connector: size of the @connectors and @connector_states arrays > @@ -172,6 +173,7 @@ struct drm_atomic_state { > struct drm_device *dev; > bool allow_modeset : 1; > bool legacy_cursor_update : 1; > + bool async_update : 1; > struct __drm_planes_state *planes; > struct __drm_crtcs_state *crtcs; > int num_connector; > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h > index f0a8678..8571b51 100644 > --- a/include/drm/drm_atomic_helper.h > +++ b/include/drm/drm_atomic_helper.h > @@ -44,6 +44,8 @@ void drm_atomic_helper_commit_tail(struct drm_atomic_state *state); > int drm_atomic_helper_commit(struct drm_device *dev, > struct drm_atomic_state *state, > bool nonblock); > +void drm_atomic_helper_async_commit(struct drm_device *dev, > + struct drm_atomic_state *state); > > int drm_atomic_helper_wait_for_fences(struct drm_device *dev, > struct drm_atomic_state *state, > diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h > index c01c328..7964a78 100644 > --- a/include/drm/drm_modeset_helper_vtables.h > +++ b/include/drm/drm_modeset_helper_vtables.h > @@ -1048,6 +1048,54 @@ struct drm_plane_helper_funcs { > */ > void (*atomic_disable)(struct drm_plane *plane, > struct drm_plane_state *old_state); > + > + /** > + * @atomic_async_check: > + * > + * Drivers should set this function pointer to check if the plane state > + * can be updated in a async fashion. Here async means "not vblank > + * synchronized". > + * > + * This hook is called by drm_atomic_async_check() to establish if a > + * given update can be committed asynchronously, that is, if it can > + * jump ahead the state currently queued for update. > + * > + * RETURNS: > + * > + * Return 0 on success and any error returned indicates that the update > + * can not be applied in asynchronous manner. > + */ > + int (*atomic_async_check)(struct drm_plane *plane, > + struct drm_plane_state *state); > + > + /** > + * @atomic_async_update: > + * > + * Drivers should set this function pointer to perform asynchronous > + * updates of planes, that is, jump ahead the currently queued > + * state for and update the plane. Here async means "not vblank > + * synchronized". > + * > + * This hook is called by drm_atomic_helper_async_commit(). > + * > + * An async update will happen on legacy cursor updates. > + * > + * Note that unlike &drm_plane_helper_funcs.atomic_update this hook > + * takes the new &drm_plane_state as parameter. When doing async_update > + * drivers shouldn't replace the &drm_plane_state but update the > + * current one with the new plane configurations in the new > + * plane_state. > + * > + * FIXME: > + * - It only works for single plane updates > + * - Async Pageflips are not supported yet > + * - Some hm might still scan out the old buffer until the next > + * vblank, however we let go of the fb references as soon as > + * we run this hook. For now drivers must implement their own workers > + * for defering if needed, until a common solution is created. > + */ > + void (*atomic_async_update)(struct drm_plane *plane, > + struct drm_plane_state *new_state); > }; > > /** >
Gustavo Padovan <gustavo@padovan.org> writes: > From: Gustavo Padovan <gustavo.padovan@collabora.com> > > In some cases, like cursor updates, it is interesting to update the > plane in an asynchronous fashion to avoid big delays. The current queued > update could be still waiting for a fence to signal and thus block any > subsequent update until its scan out. In cases like this if we update the > cursor synchronously through the atomic API it will cause significant > delays that would even be noticed by the final user. > > This patch creates a fast path to jump ahead the current queued state and > do single planes updates without going through all atomic steps in > drm_atomic_helper_commit(). We take this path for legacy cursor updates. > > For now only single plane updates are supported, but we plan to support > multiple planes updates and async PageFlips through this interface as well > in the near future. Sorry for taking so long to do some review for this. > --- > drivers/gpu/drm/drm_atomic.c | 65 ++++++++++++++++++++++++++++++++ > drivers/gpu/drm/drm_atomic_helper.c | 35 +++++++++++++++++ > include/drm/drm_atomic.h | 2 + > include/drm/drm_atomic_helper.h | 2 + > include/drm/drm_modeset_helper_vtables.h | 48 +++++++++++++++++++++++ > 5 files changed, 152 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index be62774..2a0c297 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -631,6 +631,68 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc, > return 0; > } > > +static bool drm_atomic_async_check(struct drm_atomic_state *state) > +{ > + struct drm_crtc *crtc; > + struct drm_crtc_state *crtc_state; > + struct drm_crtc_commit *commit; > + struct drm_plane *__plane, *plane = NULL; > + struct drm_plane_state *__plane_state, *plane_state = NULL; > + const struct drm_plane_helper_funcs *funcs; > + int i, j, n_planes = 0; > + > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { > + if (drm_atomic_crtc_needs_modeset(crtc_state)) > + return false; > + } > + > + for_each_new_plane_in_state(state, __plane, __plane_state, i) { > + n_planes++; > + plane = __plane; > + plane_state = __plane_state; > + } > + > + /* FIXME: we support only single plane updates for now */ > + if (!plane || n_planes != 1) > + return false; > + > + if (!plane_state->crtc) > + return false; > + > + funcs = plane->helper_private; > + if (!funcs->atomic_async_update) > + return false; > + > + if (plane_state->fence) > + return false; > + Could you add a comment here like: /* Don't do an async update if there is an outstanding commit modifying * the plane. This prevents our async update's changes from getting * overridden by a previous synchronous update's state. */ (assuming I understand its intent correctly) I don't understand KMS locking enough to say if this loop is actually safely guaranteeing that. If you're convinced of that, then with my little cleanups this patch will be: Acked-by: Eric Anholt <eric@anholt.net> > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { > + if (plane->crtc != crtc) > + continue; > + > + spin_lock(&crtc->commit_lock); > + commit = list_first_entry_or_null(&crtc->commit_list, > + struct drm_crtc_commit, > + commit_entry); > + if (!commit) { > + spin_unlock(&crtc->commit_lock); > + continue; > + } > + spin_unlock(&crtc->commit_lock); > + > + if (!crtc->state->state) > + continue; > + > + for_each_plane_in_state(crtc->state->state, __plane, > + __plane_state, j) { > + if (__plane == plane) > + return false; > + } > + } > + > + return !funcs->atomic_async_check(plane, plane_state); > +} > + > static void drm_atomic_crtc_print_state(struct drm_printer *p, > const struct drm_crtc_state *state) > { > @@ -1591,6 +1653,9 @@ int drm_atomic_check_only(struct drm_atomic_state *state) > } > } > > + if (state->legacy_cursor_update) > + state->async_update = drm_atomic_async_check(state); > + > return ret; > } > EXPORT_SYMBOL(drm_atomic_check_only); > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 5a3c9c0..a8efdfe 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1235,6 +1235,30 @@ static void commit_work(struct work_struct *work) > } > > /** > + * drm_atomic_helper_async_commit - commit state asynchronously > + * > + * This function commits a state asynchronously, i.e., not vblank > + * synchronized. It should be used on a state only when > + * drm_atomic_async_check() succeeds. Async commits are not supposed to swap > + * the states like normal sync commits, but just do in-place changes on the > + * current state. > + */ > +void drm_atomic_helper_async_commit(struct drm_device *dev, > + struct drm_atomic_state *state) > +{ > + struct drm_plane *plane; > + struct drm_plane_state *plane_state; > + const struct drm_plane_helper_funcs *funcs; > + int i; > + > + for_each_new_plane_in_state(state, plane, plane_state, i) { > + funcs = plane->helper_private; > + funcs->atomic_async_update(plane, plane_state); > + } > +} > +EXPORT_SYMBOL(drm_atomic_helper_async_commit); > + > +/** > * drm_atomic_helper_commit - commit validated state object > * @dev: DRM device > * @state: the driver state object > @@ -1258,6 +1282,17 @@ int drm_atomic_helper_commit(struct drm_device *dev, > { > int ret; > > + if (state->async_update) { > + ret = drm_atomic_helper_prepare_planes(dev, state); > + if (ret) > + return ret; > + > + drm_atomic_helper_async_commit(dev, state); > + drm_atomic_helper_cleanup_planes(dev, state); > + > + return 0; > + } > + > ret = drm_atomic_helper_setup_commit(state, nonblock); > if (ret) > return ret; > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index 788daf7..70ca279 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -160,6 +160,7 @@ struct __drm_connnectors_state { > * @dev: parent DRM device > * @allow_modeset: allow full modeset > * @legacy_cursor_update: hint to enforce legacy cursor IOCTL semantics > + * @async_update: hint for asynchronous plane update > * @planes: pointer to array of structures with per-plane data > * @crtcs: pointer to array of CRTC pointers > * @num_connector: size of the @connectors and @connector_states arrays > @@ -172,6 +173,7 @@ struct drm_atomic_state { > struct drm_device *dev; > bool allow_modeset : 1; > bool legacy_cursor_update : 1; > + bool async_update : 1; > struct __drm_planes_state *planes; > struct __drm_crtcs_state *crtcs; > int num_connector; > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h > index f0a8678..8571b51 100644 > --- a/include/drm/drm_atomic_helper.h > +++ b/include/drm/drm_atomic_helper.h > @@ -44,6 +44,8 @@ void drm_atomic_helper_commit_tail(struct drm_atomic_state *state); > int drm_atomic_helper_commit(struct drm_device *dev, > struct drm_atomic_state *state, > bool nonblock); > +void drm_atomic_helper_async_commit(struct drm_device *dev, > + struct drm_atomic_state *state); > > int drm_atomic_helper_wait_for_fences(struct drm_device *dev, > struct drm_atomic_state *state, > diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h > index c01c328..7964a78 100644 > --- a/include/drm/drm_modeset_helper_vtables.h > +++ b/include/drm/drm_modeset_helper_vtables.h > @@ -1048,6 +1048,54 @@ struct drm_plane_helper_funcs { > */ > void (*atomic_disable)(struct drm_plane *plane, > struct drm_plane_state *old_state); > + > + /** > + * @atomic_async_check: > + * > + * Drivers should set this function pointer to check if the plane state > + * can be updated in a async fashion. Here async means "not vblank > + * synchronized". > + * > + * This hook is called by drm_atomic_async_check() to establish if a > + * given update can be committed asynchronously, that is, if it can > + * jump ahead the state currently queued for update. > + * > + * RETURNS: > + * > + * Return 0 on success and any error returned indicates that the update > + * can not be applied in asynchronous manner. > + */ > + int (*atomic_async_check)(struct drm_plane *plane, > + struct drm_plane_state *state); > + > + /** > + * @atomic_async_update: > + * > + * Drivers should set this function pointer to perform asynchronous > + * updates of planes, that is, jump ahead the currently queued "jump ahead of the" > + * state for and update the plane. Here async means "not vblank s/for // > + * synchronized". > + * > + * This hook is called by drm_atomic_helper_async_commit(). > + * > + * An async update will happen on legacy cursor updates. > + * > + * Note that unlike &drm_plane_helper_funcs.atomic_update this hook > + * takes the new &drm_plane_state as parameter. When doing async_update > + * drivers shouldn't replace the &drm_plane_state but update the > + * current one with the new plane configurations in the new > + * plane_state. Also maybe document that this won't be called if there is an outstanding commit modifying the plane? > + * > + * FIXME: > + * - It only works for single plane updates > + * - Async Pageflips are not supported yet > + * - Some hm might still scan out the old buffer until the next "Some hw"? > + * vblank, however we let go of the fb references as soon as > + * we run this hook. For now drivers must implement their own workers > + * for defering if needed, until a common solution is created. > + */ "deferring" > + void (*atomic_async_update)(struct drm_plane *plane, > + struct drm_plane_state *new_state); Is there a simple list of what parts of drm_plane_state may change in this hook? I noticed that vc4 wasn't checking that the format doesn't change, for example. I'm also not sure if we're guaranteed that the plane-is-enabled state isn't changing, either. If it's "everything", we'll need to improve the vc4 async check before we expose this under anything but the cursor ioctl.
2017-05-31 Eric Anholt <eric@anholt.net>: > Gustavo Padovan <gustavo@padovan.org> writes: > > > From: Gustavo Padovan <gustavo.padovan@collabora.com> > > > > In some cases, like cursor updates, it is interesting to update the > > plane in an asynchronous fashion to avoid big delays. The current queued > > update could be still waiting for a fence to signal and thus block any > > subsequent update until its scan out. In cases like this if we update the > > cursor synchronously through the atomic API it will cause significant > > delays that would even be noticed by the final user. > > > > This patch creates a fast path to jump ahead the current queued state and > > do single planes updates without going through all atomic steps in > > drm_atomic_helper_commit(). We take this path for legacy cursor updates. > > > > For now only single plane updates are supported, but we plan to support > > multiple planes updates and async PageFlips through this interface as well > > in the near future. > > Sorry for taking so long to do some review for this. > > > --- > > drivers/gpu/drm/drm_atomic.c | 65 ++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/drm_atomic_helper.c | 35 +++++++++++++++++ > > include/drm/drm_atomic.h | 2 + > > include/drm/drm_atomic_helper.h | 2 + > > include/drm/drm_modeset_helper_vtables.h | 48 +++++++++++++++++++++++ > > 5 files changed, 152 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > > index be62774..2a0c297 100644 > > --- a/drivers/gpu/drm/drm_atomic.c > > +++ b/drivers/gpu/drm/drm_atomic.c > > @@ -631,6 +631,68 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc, > > return 0; > > } > > > > +static bool drm_atomic_async_check(struct drm_atomic_state *state) > > +{ > > + struct drm_crtc *crtc; > > + struct drm_crtc_state *crtc_state; > > + struct drm_crtc_commit *commit; > > + struct drm_plane *__plane, *plane = NULL; > > + struct drm_plane_state *__plane_state, *plane_state = NULL; > > + const struct drm_plane_helper_funcs *funcs; > > + int i, j, n_planes = 0; > > + > > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { > > + if (drm_atomic_crtc_needs_modeset(crtc_state)) > > + return false; > > + } > > + > > + for_each_new_plane_in_state(state, __plane, __plane_state, i) { > > + n_planes++; > > + plane = __plane; > > + plane_state = __plane_state; > > + } > > + > > + /* FIXME: we support only single plane updates for now */ > > + if (!plane || n_planes != 1) > > + return false; > > + > > + if (!plane_state->crtc) > > + return false; > > + > > + funcs = plane->helper_private; > > + if (!funcs->atomic_async_update) > > + return false; > > + > > + if (plane_state->fence) > > + return false; > > + > > Could you add a comment here like: > > /* Don't do an async update if there is an outstanding commit modifying > * the plane. This prevents our async update's changes from getting > * overridden by a previous synchronous update's state. > */ > > (assuming I understand its intent correctly) > > I don't understand KMS locking enough to say if this loop is actually > safely guaranteeing that. If you're convinced of that, then with my > little cleanups this patch will be: I'm going to check the locking again just to make sure. > > Acked-by: Eric Anholt <eric@anholt.net> > > > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { > > + if (plane->crtc != crtc) > > + continue; > > + > > + spin_lock(&crtc->commit_lock); > > + commit = list_first_entry_or_null(&crtc->commit_list, > > + struct drm_crtc_commit, > > + commit_entry); > > + if (!commit) { > > + spin_unlock(&crtc->commit_lock); > > + continue; > > + } > > + spin_unlock(&crtc->commit_lock); > > + > > + if (!crtc->state->state) > > + continue; > > + > > + for_each_plane_in_state(crtc->state->state, __plane, > > + __plane_state, j) { > > + if (__plane == plane) > > + return false; > > + } > > + } > > + > > + return !funcs->atomic_async_check(plane, plane_state); > > +} > > + > > static void drm_atomic_crtc_print_state(struct drm_printer *p, > > const struct drm_crtc_state *state) > > { > > @@ -1591,6 +1653,9 @@ int drm_atomic_check_only(struct drm_atomic_state *state) > > } > > } > > > > + if (state->legacy_cursor_update) > > + state->async_update = drm_atomic_async_check(state); > > + > > return ret; > > } > > EXPORT_SYMBOL(drm_atomic_check_only); > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > > index 5a3c9c0..a8efdfe 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -1235,6 +1235,30 @@ static void commit_work(struct work_struct *work) > > } > > > > /** > > + * drm_atomic_helper_async_commit - commit state asynchronously > > + * > > + * This function commits a state asynchronously, i.e., not vblank > > + * synchronized. It should be used on a state only when > > + * drm_atomic_async_check() succeeds. Async commits are not supposed to swap > > + * the states like normal sync commits, but just do in-place changes on the > > + * current state. > > + */ > > +void drm_atomic_helper_async_commit(struct drm_device *dev, > > + struct drm_atomic_state *state) > > +{ > > + struct drm_plane *plane; > > + struct drm_plane_state *plane_state; > > + const struct drm_plane_helper_funcs *funcs; > > + int i; > > + > > + for_each_new_plane_in_state(state, plane, plane_state, i) { > > + funcs = plane->helper_private; > > + funcs->atomic_async_update(plane, plane_state); > > + } > > +} > > +EXPORT_SYMBOL(drm_atomic_helper_async_commit); > > + > > +/** > > * drm_atomic_helper_commit - commit validated state object > > * @dev: DRM device > > * @state: the driver state object > > @@ -1258,6 +1282,17 @@ int drm_atomic_helper_commit(struct drm_device *dev, > > { > > int ret; > > > > + if (state->async_update) { > > + ret = drm_atomic_helper_prepare_planes(dev, state); > > + if (ret) > > + return ret; > > + > > + drm_atomic_helper_async_commit(dev, state); > > + drm_atomic_helper_cleanup_planes(dev, state); > > + > > + return 0; > > + } > > + > > ret = drm_atomic_helper_setup_commit(state, nonblock); > > if (ret) > > return ret; > > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > > index 788daf7..70ca279 100644 > > --- a/include/drm/drm_atomic.h > > +++ b/include/drm/drm_atomic.h > > @@ -160,6 +160,7 @@ struct __drm_connnectors_state { > > * @dev: parent DRM device > > * @allow_modeset: allow full modeset > > * @legacy_cursor_update: hint to enforce legacy cursor IOCTL semantics > > + * @async_update: hint for asynchronous plane update > > * @planes: pointer to array of structures with per-plane data > > * @crtcs: pointer to array of CRTC pointers > > * @num_connector: size of the @connectors and @connector_states arrays > > @@ -172,6 +173,7 @@ struct drm_atomic_state { > > struct drm_device *dev; > > bool allow_modeset : 1; > > bool legacy_cursor_update : 1; > > + bool async_update : 1; > > struct __drm_planes_state *planes; > > struct __drm_crtcs_state *crtcs; > > int num_connector; > > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h > > index f0a8678..8571b51 100644 > > --- a/include/drm/drm_atomic_helper.h > > +++ b/include/drm/drm_atomic_helper.h > > @@ -44,6 +44,8 @@ void drm_atomic_helper_commit_tail(struct drm_atomic_state *state); > > int drm_atomic_helper_commit(struct drm_device *dev, > > struct drm_atomic_state *state, > > bool nonblock); > > +void drm_atomic_helper_async_commit(struct drm_device *dev, > > + struct drm_atomic_state *state); > > > > int drm_atomic_helper_wait_for_fences(struct drm_device *dev, > > struct drm_atomic_state *state, > > diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h > > index c01c328..7964a78 100644 > > --- a/include/drm/drm_modeset_helper_vtables.h > > +++ b/include/drm/drm_modeset_helper_vtables.h > > @@ -1048,6 +1048,54 @@ struct drm_plane_helper_funcs { > > */ > > void (*atomic_disable)(struct drm_plane *plane, > > struct drm_plane_state *old_state); > > + > > + /** > > + * @atomic_async_check: > > + * > > + * Drivers should set this function pointer to check if the plane state > > + * can be updated in a async fashion. Here async means "not vblank > > + * synchronized". > > + * > > + * This hook is called by drm_atomic_async_check() to establish if a > > + * given update can be committed asynchronously, that is, if it can > > + * jump ahead the state currently queued for update. > > + * > > + * RETURNS: > > + * > > + * Return 0 on success and any error returned indicates that the update > > + * can not be applied in asynchronous manner. > > + */ > > + int (*atomic_async_check)(struct drm_plane *plane, > > + struct drm_plane_state *state); > > + > > + /** > > + * @atomic_async_update: > > + * > > + * Drivers should set this function pointer to perform asynchronous > > + * updates of planes, that is, jump ahead the currently queued > > "jump ahead of the" > > > + * state for and update the plane. Here async means "not vblank > > s/for // > > > + * synchronized". > > + * > > + * This hook is called by drm_atomic_helper_async_commit(). > > + * > > + * An async update will happen on legacy cursor updates. > > + * > > + * Note that unlike &drm_plane_helper_funcs.atomic_update this hook > > + * takes the new &drm_plane_state as parameter. When doing async_update > > + * drivers shouldn't replace the &drm_plane_state but update the > > + * current one with the new plane configurations in the new > > + * plane_state. > > Also maybe document that this won't be called if there is an outstanding > commit modifying the plane? > > > + * > > + * FIXME: > > + * - It only works for single plane updates > > + * - Async Pageflips are not supported yet > > + * - Some hm might still scan out the old buffer until the next > > "Some hw"? > > > + * vblank, however we let go of the fb references as soon as > > + * we run this hook. For now drivers must implement their own workers > > + * for defering if needed, until a common solution is created. > > + */ > > "deferring" > > > + void (*atomic_async_update)(struct drm_plane *plane, > > + struct drm_plane_state *new_state); > > Is there a simple list of what parts of drm_plane_state may change in > this hook? I noticed that vc4 wasn't checking that the format doesn't > change, for example. I'm also not sure if we're guaranteed that the > plane-is-enabled state isn't changing, either. I can look into building such a list. I'll do that and send a proposed one here. > > If it's "everything", we'll need to improve the vc4 async check before > we expose this under anything but the cursor ioctl. Gustavo
2017-06-01 Gustavo Padovan <gustavo@padovan.org>: > 2017-05-31 Eric Anholt <eric@anholt.net>: > > > Gustavo Padovan <gustavo@padovan.org> writes: > > > > > From: Gustavo Padovan <gustavo.padovan@collabora.com> > > > > > > In some cases, like cursor updates, it is interesting to update the > > > plane in an asynchronous fashion to avoid big delays. The current queued > > > update could be still waiting for a fence to signal and thus block any > > > subsequent update until its scan out. In cases like this if we update the > > > cursor synchronously through the atomic API it will cause significant > > > delays that would even be noticed by the final user. > > > > > > This patch creates a fast path to jump ahead the current queued state and > > > do single planes updates without going through all atomic steps in > > > drm_atomic_helper_commit(). We take this path for legacy cursor updates. > > > > > > For now only single plane updates are supported, but we plan to support > > > multiple planes updates and async PageFlips through this interface as well > > > in the near future. > > > > Sorry for taking so long to do some review for this. > > > > > --- > > > drivers/gpu/drm/drm_atomic.c | 65 ++++++++++++++++++++++++++++++++ > > > drivers/gpu/drm/drm_atomic_helper.c | 35 +++++++++++++++++ > > > include/drm/drm_atomic.h | 2 + > > > include/drm/drm_atomic_helper.h | 2 + > > > include/drm/drm_modeset_helper_vtables.h | 48 +++++++++++++++++++++++ > > > 5 files changed, 152 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > > > index be62774..2a0c297 100644 > > > --- a/drivers/gpu/drm/drm_atomic.c > > > +++ b/drivers/gpu/drm/drm_atomic.c > > > @@ -631,6 +631,68 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc, > > > return 0; > > > } > > > > > > +static bool drm_atomic_async_check(struct drm_atomic_state *state) > > > +{ > > > + struct drm_crtc *crtc; > > > + struct drm_crtc_state *crtc_state; > > > + struct drm_crtc_commit *commit; > > > + struct drm_plane *__plane, *plane = NULL; > > > + struct drm_plane_state *__plane_state, *plane_state = NULL; > > > + const struct drm_plane_helper_funcs *funcs; > > > + int i, j, n_planes = 0; > > > + > > > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { > > > + if (drm_atomic_crtc_needs_modeset(crtc_state)) > > > + return false; > > > + } > > > + > > > + for_each_new_plane_in_state(state, __plane, __plane_state, i) { > > > + n_planes++; > > > + plane = __plane; > > > + plane_state = __plane_state; > > > + } > > > + > > > + /* FIXME: we support only single plane updates for now */ > > > + if (!plane || n_planes != 1) > > > + return false; > > > + > > > + if (!plane_state->crtc) > > > + return false; > > > + > > > + funcs = plane->helper_private; > > > + if (!funcs->atomic_async_update) > > > + return false; > > > + > > > + if (plane_state->fence) > > > + return false; > > > + > > > > Could you add a comment here like: > > > > /* Don't do an async update if there is an outstanding commit modifying > > * the plane. This prevents our async update's changes from getting > > * overridden by a previous synchronous update's state. > > */ > > > > (assuming I understand its intent correctly) > > > > I don't understand KMS locking enough to say if this loop is actually > > safely guaranteeing that. If you're convinced of that, then with my > > little cleanups this patch will be: > > I'm going to check the locking again just to make sure. > > > > > Acked-by: Eric Anholt <eric@anholt.net> > > > > > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { > > > + if (plane->crtc != crtc) > > > + continue; > > > + > > > + spin_lock(&crtc->commit_lock); > > > + commit = list_first_entry_or_null(&crtc->commit_list, > > > + struct drm_crtc_commit, > > > + commit_entry); > > > + if (!commit) { > > > + spin_unlock(&crtc->commit_lock); > > > + continue; > > > + } > > > + spin_unlock(&crtc->commit_lock); > > > + > > > + if (!crtc->state->state) > > > + continue; > > > + > > > + for_each_plane_in_state(crtc->state->state, __plane, > > > + __plane_state, j) { > > > + if (__plane == plane) > > > + return false; > > > + } > > > + } > > > + > > > + return !funcs->atomic_async_check(plane, plane_state); > > > +} > > > + > > > static void drm_atomic_crtc_print_state(struct drm_printer *p, > > > const struct drm_crtc_state *state) > > > { > > > @@ -1591,6 +1653,9 @@ int drm_atomic_check_only(struct drm_atomic_state *state) > > > } > > > } > > > > > > + if (state->legacy_cursor_update) > > > + state->async_update = drm_atomic_async_check(state); > > > + > > > return ret; > > > } > > > EXPORT_SYMBOL(drm_atomic_check_only); > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > > > index 5a3c9c0..a8efdfe 100644 > > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > > @@ -1235,6 +1235,30 @@ static void commit_work(struct work_struct *work) > > > } > > > > > > /** > > > + * drm_atomic_helper_async_commit - commit state asynchronously > > > + * > > > + * This function commits a state asynchronously, i.e., not vblank > > > + * synchronized. It should be used on a state only when > > > + * drm_atomic_async_check() succeeds. Async commits are not supposed to swap > > > + * the states like normal sync commits, but just do in-place changes on the > > > + * current state. > > > + */ > > > +void drm_atomic_helper_async_commit(struct drm_device *dev, > > > + struct drm_atomic_state *state) > > > +{ > > > + struct drm_plane *plane; > > > + struct drm_plane_state *plane_state; > > > + const struct drm_plane_helper_funcs *funcs; > > > + int i; > > > + > > > + for_each_new_plane_in_state(state, plane, plane_state, i) { > > > + funcs = plane->helper_private; > > > + funcs->atomic_async_update(plane, plane_state); > > > + } > > > +} > > > +EXPORT_SYMBOL(drm_atomic_helper_async_commit); > > > + > > > +/** > > > * drm_atomic_helper_commit - commit validated state object > > > * @dev: DRM device > > > * @state: the driver state object > > > @@ -1258,6 +1282,17 @@ int drm_atomic_helper_commit(struct drm_device *dev, > > > { > > > int ret; > > > > > > + if (state->async_update) { > > > + ret = drm_atomic_helper_prepare_planes(dev, state); > > > + if (ret) > > > + return ret; > > > + > > > + drm_atomic_helper_async_commit(dev, state); > > > + drm_atomic_helper_cleanup_planes(dev, state); > > > + > > > + return 0; > > > + } > > > + > > > ret = drm_atomic_helper_setup_commit(state, nonblock); > > > if (ret) > > > return ret; > > > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > > > index 788daf7..70ca279 100644 > > > --- a/include/drm/drm_atomic.h > > > +++ b/include/drm/drm_atomic.h > > > @@ -160,6 +160,7 @@ struct __drm_connnectors_state { > > > * @dev: parent DRM device > > > * @allow_modeset: allow full modeset > > > * @legacy_cursor_update: hint to enforce legacy cursor IOCTL semantics > > > + * @async_update: hint for asynchronous plane update > > > * @planes: pointer to array of structures with per-plane data > > > * @crtcs: pointer to array of CRTC pointers > > > * @num_connector: size of the @connectors and @connector_states arrays > > > @@ -172,6 +173,7 @@ struct drm_atomic_state { > > > struct drm_device *dev; > > > bool allow_modeset : 1; > > > bool legacy_cursor_update : 1; > > > + bool async_update : 1; > > > struct __drm_planes_state *planes; > > > struct __drm_crtcs_state *crtcs; > > > int num_connector; > > > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h > > > index f0a8678..8571b51 100644 > > > --- a/include/drm/drm_atomic_helper.h > > > +++ b/include/drm/drm_atomic_helper.h > > > @@ -44,6 +44,8 @@ void drm_atomic_helper_commit_tail(struct drm_atomic_state *state); > > > int drm_atomic_helper_commit(struct drm_device *dev, > > > struct drm_atomic_state *state, > > > bool nonblock); > > > +void drm_atomic_helper_async_commit(struct drm_device *dev, > > > + struct drm_atomic_state *state); > > > > > > int drm_atomic_helper_wait_for_fences(struct drm_device *dev, > > > struct drm_atomic_state *state, > > > diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h > > > index c01c328..7964a78 100644 > > > --- a/include/drm/drm_modeset_helper_vtables.h > > > +++ b/include/drm/drm_modeset_helper_vtables.h > > > @@ -1048,6 +1048,54 @@ struct drm_plane_helper_funcs { > > > */ > > > void (*atomic_disable)(struct drm_plane *plane, > > > struct drm_plane_state *old_state); > > > + > > > + /** > > > + * @atomic_async_check: > > > + * > > > + * Drivers should set this function pointer to check if the plane state > > > + * can be updated in a async fashion. Here async means "not vblank > > > + * synchronized". > > > + * > > > + * This hook is called by drm_atomic_async_check() to establish if a > > > + * given update can be committed asynchronously, that is, if it can > > > + * jump ahead the state currently queued for update. > > > + * > > > + * RETURNS: > > > + * > > > + * Return 0 on success and any error returned indicates that the update > > > + * can not be applied in asynchronous manner. > > > + */ > > > + int (*atomic_async_check)(struct drm_plane *plane, > > > + struct drm_plane_state *state); > > > + > > > + /** > > > + * @atomic_async_update: > > > + * > > > + * Drivers should set this function pointer to perform asynchronous > > > + * updates of planes, that is, jump ahead the currently queued > > > > "jump ahead of the" > > > > > + * state for and update the plane. Here async means "not vblank > > > > s/for // > > > > > + * synchronized". > > > + * > > > + * This hook is called by drm_atomic_helper_async_commit(). > > > + * > > > + * An async update will happen on legacy cursor updates. > > > + * > > > + * Note that unlike &drm_plane_helper_funcs.atomic_update this hook > > > + * takes the new &drm_plane_state as parameter. When doing async_update > > > + * drivers shouldn't replace the &drm_plane_state but update the > > > + * current one with the new plane configurations in the new > > > + * plane_state. > > > > Also maybe document that this won't be called if there is an outstanding > > commit modifying the plane? > > > > > + * > > > + * FIXME: > > > + * - It only works for single plane updates > > > + * - Async Pageflips are not supported yet > > > + * - Some hm might still scan out the old buffer until the next > > > > "Some hw"? > > > > > + * vblank, however we let go of the fb references as soon as > > > + * we run this hook. For now drivers must implement their own workers > > > + * for defering if needed, until a common solution is created. > > > + */ > > > > "deferring" > > > > > + void (*atomic_async_update)(struct drm_plane *plane, > > > + struct drm_plane_state *new_state); > > > > Is there a simple list of what parts of drm_plane_state may change in > > this hook? I noticed that vc4 wasn't checking that the format doesn't > > change, for example. I'm also not sure if we're guaranteed that the > > plane-is-enabled state isn't changing, either. > > I can look into building such a list. I'll do that and send a proposed > one here. The list of things that may change on this hook really depends on the driver, so I believe we need to improve the vc4 async check before enabling it through the atomic ioctl. > > > > > If it's "everything", we'll need to improve the vc4 async check before > > we expose this under anything but the cursor ioctl. > > > Gustavo >
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index be62774..2a0c297 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -631,6 +631,68 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc, return 0; } +static bool drm_atomic_async_check(struct drm_atomic_state *state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + struct drm_crtc_commit *commit; + struct drm_plane *__plane, *plane = NULL; + struct drm_plane_state *__plane_state, *plane_state = NULL; + const struct drm_plane_helper_funcs *funcs; + int i, j, n_planes = 0; + + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { + if (drm_atomic_crtc_needs_modeset(crtc_state)) + return false; + } + + for_each_new_plane_in_state(state, __plane, __plane_state, i) { + n_planes++; + plane = __plane; + plane_state = __plane_state; + } + + /* FIXME: we support only single plane updates for now */ + if (!plane || n_planes != 1) + return false; + + if (!plane_state->crtc) + return false; + + funcs = plane->helper_private; + if (!funcs->atomic_async_update) + return false; + + if (plane_state->fence) + return false; + + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { + if (plane->crtc != crtc) + continue; + + spin_lock(&crtc->commit_lock); + commit = list_first_entry_or_null(&crtc->commit_list, + struct drm_crtc_commit, + commit_entry); + if (!commit) { + spin_unlock(&crtc->commit_lock); + continue; + } + spin_unlock(&crtc->commit_lock); + + if (!crtc->state->state) + continue; + + for_each_plane_in_state(crtc->state->state, __plane, + __plane_state, j) { + if (__plane == plane) + return false; + } + } + + return !funcs->atomic_async_check(plane, plane_state); +} + static void drm_atomic_crtc_print_state(struct drm_printer *p, const struct drm_crtc_state *state) { @@ -1591,6 +1653,9 @@ int drm_atomic_check_only(struct drm_atomic_state *state) } } + if (state->legacy_cursor_update) + state->async_update = drm_atomic_async_check(state); + return ret; } EXPORT_SYMBOL(drm_atomic_check_only); diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 5a3c9c0..a8efdfe 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1235,6 +1235,30 @@ static void commit_work(struct work_struct *work) } /** + * drm_atomic_helper_async_commit - commit state asynchronously + * + * This function commits a state asynchronously, i.e., not vblank + * synchronized. It should be used on a state only when + * drm_atomic_async_check() succeeds. Async commits are not supposed to swap + * the states like normal sync commits, but just do in-place changes on the + * current state. + */ +void drm_atomic_helper_async_commit(struct drm_device *dev, + struct drm_atomic_state *state) +{ + struct drm_plane *plane; + struct drm_plane_state *plane_state; + const struct drm_plane_helper_funcs *funcs; + int i; + + for_each_new_plane_in_state(state, plane, plane_state, i) { + funcs = plane->helper_private; + funcs->atomic_async_update(plane, plane_state); + } +} +EXPORT_SYMBOL(drm_atomic_helper_async_commit); + +/** * drm_atomic_helper_commit - commit validated state object * @dev: DRM device * @state: the driver state object @@ -1258,6 +1282,17 @@ int drm_atomic_helper_commit(struct drm_device *dev, { int ret; + if (state->async_update) { + ret = drm_atomic_helper_prepare_planes(dev, state); + if (ret) + return ret; + + drm_atomic_helper_async_commit(dev, state); + drm_atomic_helper_cleanup_planes(dev, state); + + return 0; + } + ret = drm_atomic_helper_setup_commit(state, nonblock); if (ret) return ret; diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index 788daf7..70ca279 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -160,6 +160,7 @@ struct __drm_connnectors_state { * @dev: parent DRM device * @allow_modeset: allow full modeset * @legacy_cursor_update: hint to enforce legacy cursor IOCTL semantics + * @async_update: hint for asynchronous plane update * @planes: pointer to array of structures with per-plane data * @crtcs: pointer to array of CRTC pointers * @num_connector: size of the @connectors and @connector_states arrays @@ -172,6 +173,7 @@ struct drm_atomic_state { struct drm_device *dev; bool allow_modeset : 1; bool legacy_cursor_update : 1; + bool async_update : 1; struct __drm_planes_state *planes; struct __drm_crtcs_state *crtcs; int num_connector; diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index f0a8678..8571b51 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -44,6 +44,8 @@ void drm_atomic_helper_commit_tail(struct drm_atomic_state *state); int drm_atomic_helper_commit(struct drm_device *dev, struct drm_atomic_state *state, bool nonblock); +void drm_atomic_helper_async_commit(struct drm_device *dev, + struct drm_atomic_state *state); int drm_atomic_helper_wait_for_fences(struct drm_device *dev, struct drm_atomic_state *state, diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h index c01c328..7964a78 100644 --- a/include/drm/drm_modeset_helper_vtables.h +++ b/include/drm/drm_modeset_helper_vtables.h @@ -1048,6 +1048,54 @@ struct drm_plane_helper_funcs { */ void (*atomic_disable)(struct drm_plane *plane, struct drm_plane_state *old_state); + + /** + * @atomic_async_check: + * + * Drivers should set this function pointer to check if the plane state + * can be updated in a async fashion. Here async means "not vblank + * synchronized". + * + * This hook is called by drm_atomic_async_check() to establish if a + * given update can be committed asynchronously, that is, if it can + * jump ahead the state currently queued for update. + * + * RETURNS: + * + * Return 0 on success and any error returned indicates that the update + * can not be applied in asynchronous manner. + */ + int (*atomic_async_check)(struct drm_plane *plane, + struct drm_plane_state *state); + + /** + * @atomic_async_update: + * + * Drivers should set this function pointer to perform asynchronous + * updates of planes, that is, jump ahead the currently queued + * state for and update the plane. Here async means "not vblank + * synchronized". + * + * This hook is called by drm_atomic_helper_async_commit(). + * + * An async update will happen on legacy cursor updates. + * + * Note that unlike &drm_plane_helper_funcs.atomic_update this hook + * takes the new &drm_plane_state as parameter. When doing async_update + * drivers shouldn't replace the &drm_plane_state but update the + * current one with the new plane configurations in the new + * plane_state. + * + * FIXME: + * - It only works for single plane updates + * - Async Pageflips are not supported yet + * - Some hm might still scan out the old buffer until the next + * vblank, however we let go of the fb references as soon as + * we run this hook. For now drivers must implement their own workers + * for defering if needed, until a common solution is created. + */ + void (*atomic_async_update)(struct drm_plane *plane, + struct drm_plane_state *new_state); }; /**