On Sun, Jul 31, 2016 at 01:08:54AM -0700, Keith Packard wrote:
> When reconfiguring a plane position (as in moving the cursor), the
> frame buffer for the cursor isn't changing, so don't call the prepare
> or cleanup driver functions.
>
> This avoids making cursor position updates block on all pending rendering.
Oops, I thought we've had this, but nope :(
> v2: Track which planes have been prepared to know which to
> cleanup. Otherwise, failure paths and success paths would need
> different tests in the cleanup code as the plane state points to
> different places in the two cases.
>
> cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> cc: David Airlie <airlied@xxxxxxxx>
> Signed-off-by: Keith Packard <keithp@xxxxxxxxxx>
Hm, I can't see v1 anywhere, but I think it'd be better. You can't store
any transient state related to the current update in struct drm_plane. In
this case the cleanup_buffers from a previous update might overlap (for
nonblocking atomic commits) with the prepare_planes for the next one.
Either we need special cleanup vs. error-path code, or some flag somewhere
in the drm_plane_state.
btw if you go with the flag, vc4 could be cleaned up to use that too. At
least I thought Eric hand-rolled this logic in there somewhere, can't find
it quickly right now.
Another bit worth considering: We could use this "has the plane switched"
instead ->legacy_cursor_update in drm_atomic_helper_wait_for_vblanks -
some drivers have iommus which get really angry when we unpin too early,
and your patch alone might be good enough.
-Daniel
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 23 +++++++++++++----------
> include/drm/drm_crtc.h | 1 +
> 2 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index ddfa0d1..f7f3a51 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1246,18 +1246,20 @@ EXPORT_SYMBOL(drm_atomic_helper_commit);
> * Returns:
> * 0 on success, negative error code on failure.
> */
> +
> int drm_atomic_helper_prepare_planes(struct drm_device *dev,
> struct drm_atomic_state *state)
> {
> - int nplanes = dev->mode_config.num_total_plane;
> + struct drm_plane *plane;
> + struct drm_plane_state *plane_state;
> int ret, i;
>
> - for (i = 0; i < nplanes; i++) {
> + for_each_plane_in_state(state, plane, plane_state, i) {
> const struct drm_plane_helper_funcs *funcs;
> - struct drm_plane *plane = state->planes[i];
> - struct drm_plane_state *plane_state = state->plane_states[i];
>
> - if (!plane)
> + plane->prepared = false;
> +
> + if (plane->state->fb == plane_state->fb)
> continue;
>
> funcs = plane->helper_private;
> @@ -1267,24 +1269,22 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
> if (ret)
> goto fail;
> }
> + plane->prepared = true;
> }
>
> return 0;
>
> fail:
> - for (i--; i >= 0; i--) {
> + for_each_plane_in_state(state, plane, plane_state, i) {
> const struct drm_plane_helper_funcs *funcs;
> - struct drm_plane *plane = state->planes[i];
> - struct drm_plane_state *plane_state = state->plane_states[i];
>
> - if (!plane)
> + if (!plane->prepared)
> continue;
>
> funcs = plane->helper_private;
>
> if (funcs->cleanup_fb)
> funcs->cleanup_fb(plane, plane_state);
> -
> }
>
> return ret;
> @@ -1527,6 +1527,9 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
> for_each_plane_in_state(old_state, plane, plane_state, i) {
> const struct drm_plane_helper_funcs *funcs;
>
> + if (!plane->prepared)
> + continue;
> +
> funcs = plane->helper_private;
>
> if (funcs->cleanup_fb)
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index d1559cd..08b2033 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1531,6 +1531,7 @@ struct drm_plane {
> uint32_t *format_types;
> unsigned int format_count;
> bool format_default;
> + bool prepared;
>
> struct drm_crtc *crtc;
> struct drm_framebuffer *fb;
> --
> 2.8.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
