On Sat, Aug 06, 2016 at 10:26:09PM -0700, Keith Packard wrote:
> Daniel Vetter <daniel@xxxxxxxx> writes:
>
> > 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.
>
> Ok, here's pretty much the previous version, which works now that I've
> fixed the intel driver. Instead of just comparing the fb's, I'm using
> the framebuffer_changed function, which seems like a nice bit of
> documentation if nothing else.
>
> From a75251d5762b1c200ed2f3dca2a5b00cc85bea95 Mon Sep 17 00:00:00 2001
> From: Keith Packard <keithp@xxxxxxxxxx>
> Date: Sat, 4 Jun 2016 01:16:22 -0700
> Subject: [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v3]
>
> 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.
>
> v3: use drm_atomic_helper_framebuffer_changed in both prepare and
> cleanup phases instead of keeping state in the plane.
>
> cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> cc: David Airlie <airlied@xxxxxxxx>
> cc: Daniel Vetter <daniel@xxxxxxxx>
> Signed-off-by: Keith Packard <keithp@xxxxxxxxxx>
Rebased onto 4.8 while applying, which did simplify the patch a lot (4.8
is already using for_each_plane_in_state, but slightly differently).
Thanks, Daniel
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 25 +++++++++++++++----------
> 1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index ddfa0d1..72e50bc 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1246,18 +1246,19 @@ 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;
> + int max_prepared_i = 0;
>
> - 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)
> + if (!drm_atomic_helper_framebuffer_changed(dev, state, plane_state->crtc))
> continue;
>
> funcs = plane->helper_private;
> @@ -1267,24 +1268,25 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
> if (ret)
> goto fail;
> }
> + max_prepared_i = i;
> }
>
> 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 (i > max_prepared_i)
> + break;
> +
> + if (!drm_atomic_helper_framebuffer_changed(dev, state, plane_state->crtc))
> continue;
>
> funcs = plane->helper_private;
>
> if (funcs->cleanup_fb)
> funcs->cleanup_fb(plane, plane_state);
> -
> }
>
> return ret;
> @@ -1527,6 +1529,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 (!drm_atomic_helper_framebuffer_changed(dev, old_state, plane_state->crtc))
> + continue;
> +
> funcs = plane->helper_private;
>
> if (funcs->cleanup_fb)
> --
> 2.8.1
>
>
> --
> -keith
--
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
