Re: [Intel-gfx] [PATCH 15/23] drm: Stop updating plane->crtc/fb/old_fb on atomic drivers

[Ville Syrjälä]

On Tue, Mar 27, 2018 at 09:57:41AM +0200, Daniel Vetter wrote:
> On Thu, Mar 22, 2018 at 05:23:05PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> > 
> > Stop playing around with plane->crtc/fb/old_fb with atomic
> > drivers. Make life a lot simpler when we don't have to do the
> > magic old_fb vs. fb dance around plane updates. That way we
> > can't risk plane->fb getting out of sync with plane->state->fb
> > and we're less likely to leak any refcounts as well.
> > 
> > Signed-off-by: Ville Syrjälä 
> 
> What's the reason for this patch being in the middle of the patch series?
> I figured it's savest if we put this at the end? If you need parts of this
> here, we definitely need to split out the WARN_ON hunk, since you haven't
> fixed up everything yet at this point here.

Yeah, the ordering is probably not great. I think I had some idea why
I had to do "cleanup drivers a bit, do core/helper stuff, cleanup some
more drivers, do other core/helper stuff". But I can't even recall that
reason now. Most likely I had just managed to confuse myself by that
time.

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/drm_atomic.c| 55 
> > -
> >  drivers/gpu/drm/drm_atomic_helper.c | 15 +-
> >  drivers/gpu/drm/drm_crtc.c  |  8 --
> >  drivers/gpu/drm/drm_fb_helper.c |  7 -
> >  drivers/gpu/drm/drm_framebuffer.c   |  5 
> >  drivers/gpu/drm/drm_plane.c | 14 ++
> >  drivers/gpu/drm/drm_plane_helper.c  |  4 ++-
> >  include/drm/drm_atomic.h|  3 --
> >  8 files changed, 24 insertions(+), 87 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 7d25c42f22db..b16cc37e2adf 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -692,6 +692,11 @@ drm_atomic_get_plane_state(struct drm_atomic_state 
> > *state,
> >  
> > WARN_ON(!state->acquire_ctx);
> >  
> > +   /* the legacy pointers should never be set */
> > +   WARN_ON(plane->fb);
> > +   WARN_ON(plane->old_fb);
> > +   WARN_ON(plane->crtc);
> > +
> > plane_state = drm_atomic_get_existing_plane_state(state, plane);
> > if (plane_state)
> > return plane_state;
> > @@ -2021,45 +2026,6 @@ int drm_atomic_set_property(struct drm_atomic_state 
> > *state,
> > return ret;
> >  }
> >  
> > -/**
> > - * drm_atomic_clean_old_fb -- Unset old_fb pointers and set plane->fb 
> > pointers.
> > - *
> > - * @dev: drm device to check.
> > - * @plane_mask: plane mask for planes that were updated.
> > - * @ret: return value, can be -EDEADLK for a retry.
> > - *
> > - * Before doing an update _plane.old_fb is set to _plane.fb, but 
> > before
> > - * dropping the locks old_fb needs to be set to NULL and plane->fb 
> > updated. This
> > - * is a common operation for each atomic update, so this call is split off 
> > as a
> > - * helper.
> > - */
> > -void drm_atomic_clean_old_fb(struct drm_device *dev,
> > -unsigned plane_mask,
> > -int ret)
> > -{
> > -   struct drm_plane *plane;
> > -
> > -   /* if succeeded, fixup legacy plane crtc/fb ptrs before dropping
> > -* locks (ie. while it is still safe to deref plane->state).  We
> > -* need to do this here because the driver entry points cannot
> > -* distinguish between legacy and atomic ioctls.
> > -*/
> > -   drm_for_each_plane_mask(plane, dev, plane_mask) {
> > -   if (ret == 0) {
> > -   struct drm_framebuffer *new_fb = plane->state->fb;
> > -   if (new_fb)
> > -   drm_framebuffer_get(new_fb);
> > -   plane->fb = new_fb;
> > -   plane->crtc = plane->state->crtc;
> > -
> > -   if (plane->old_fb)
> > -   drm_framebuffer_put(plane->old_fb);
> > -   }
> > -   plane->old_fb = NULL;
> > -   }
> > -}
> > -EXPORT_SYMBOL(drm_atomic_clean_old_fb);
> > -
> >  /**
> >   * DOC: explicit fencing properties
> >   *
> > @@ -2280,9 +2246,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> > unsigned int copied_objs, copied_props;
> > struct drm_atomic_state *state;
> > struct drm_modeset_acquire_ctx ctx;
> > -   struct drm_plane *plane;
> > struct drm_out_fence_state *fence_state;
> > -   unsigned plane_mask;
> > int ret = 0;
> > unsigned int i, j, num_fences;
> >  
> > @@ -2322,7 +2286,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> > state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET);
> >  
> >  retry:
> > -   plane_mask = 0;
> > copied_objs = 0;
> > copied_props = 0;
> > fence_state = NULL;
> > @@ -2393,12 +2356,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> > copied_props++;
> > }
> >  
> > -   if (obj->type == DRM_MODE_OBJECT_PLANE && 

Re: [Intel-gfx] [PATCH 15/23] drm: Stop updating plane->crtc/fb/old_fb on atomic drivers

[Daniel Vetter]

On Thu, Mar 22, 2018 at 05:23:05PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Stop playing around with plane->crtc/fb/old_fb with atomic
> drivers. Make life a lot simpler when we don't have to do the
> magic old_fb vs. fb dance around plane updates. That way we
> can't risk plane->fb getting out of sync with plane->state->fb
> and we're less likely to leak any refcounts as well.
> 
> Signed-off-by: Ville Syrjälä 

What's the reason for this patch being in the middle of the patch series?
I figured it's savest if we put this at the end? If you need parts of this
here, we definitely need to split out the WARN_ON hunk, since you haven't
fixed up everything yet at this point here.
-Daniel

> ---
>  drivers/gpu/drm/drm_atomic.c| 55 
> -
>  drivers/gpu/drm/drm_atomic_helper.c | 15 +-
>  drivers/gpu/drm/drm_crtc.c  |  8 --
>  drivers/gpu/drm/drm_fb_helper.c |  7 -
>  drivers/gpu/drm/drm_framebuffer.c   |  5 
>  drivers/gpu/drm/drm_plane.c | 14 ++
>  drivers/gpu/drm/drm_plane_helper.c  |  4 ++-
>  include/drm/drm_atomic.h|  3 --
>  8 files changed, 24 insertions(+), 87 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 7d25c42f22db..b16cc37e2adf 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -692,6 +692,11 @@ drm_atomic_get_plane_state(struct drm_atomic_state 
> *state,
>  
>   WARN_ON(!state->acquire_ctx);
>  
> + /* the legacy pointers should never be set */
> + WARN_ON(plane->fb);
> + WARN_ON(plane->old_fb);
> + WARN_ON(plane->crtc);
> +
>   plane_state = drm_atomic_get_existing_plane_state(state, plane);
>   if (plane_state)
>   return plane_state;
> @@ -2021,45 +2026,6 @@ int drm_atomic_set_property(struct drm_atomic_state 
> *state,
>   return ret;
>  }
>  
> -/**
> - * drm_atomic_clean_old_fb -- Unset old_fb pointers and set plane->fb 
> pointers.
> - *
> - * @dev: drm device to check.
> - * @plane_mask: plane mask for planes that were updated.
> - * @ret: return value, can be -EDEADLK for a retry.
> - *
> - * Before doing an update _plane.old_fb is set to _plane.fb, but 
> before
> - * dropping the locks old_fb needs to be set to NULL and plane->fb updated. 
> This
> - * is a common operation for each atomic update, so this call is split off 
> as a
> - * helper.
> - */
> -void drm_atomic_clean_old_fb(struct drm_device *dev,
> -  unsigned plane_mask,
> -  int ret)
> -{
> - struct drm_plane *plane;
> -
> - /* if succeeded, fixup legacy plane crtc/fb ptrs before dropping
> -  * locks (ie. while it is still safe to deref plane->state).  We
> -  * need to do this here because the driver entry points cannot
> -  * distinguish between legacy and atomic ioctls.
> -  */
> - drm_for_each_plane_mask(plane, dev, plane_mask) {
> - if (ret == 0) {
> - struct drm_framebuffer *new_fb = plane->state->fb;
> - if (new_fb)
> - drm_framebuffer_get(new_fb);
> - plane->fb = new_fb;
> - plane->crtc = plane->state->crtc;
> -
> - if (plane->old_fb)
> - drm_framebuffer_put(plane->old_fb);
> - }
> - plane->old_fb = NULL;
> - }
> -}
> -EXPORT_SYMBOL(drm_atomic_clean_old_fb);
> -
>  /**
>   * DOC: explicit fencing properties
>   *
> @@ -2280,9 +2246,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>   unsigned int copied_objs, copied_props;
>   struct drm_atomic_state *state;
>   struct drm_modeset_acquire_ctx ctx;
> - struct drm_plane *plane;
>   struct drm_out_fence_state *fence_state;
> - unsigned plane_mask;
>   int ret = 0;
>   unsigned int i, j, num_fences;
>  
> @@ -2322,7 +2286,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>   state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET);
>  
>  retry:
> - plane_mask = 0;
>   copied_objs = 0;
>   copied_props = 0;
>   fence_state = NULL;
> @@ -2393,12 +2356,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>   copied_props++;
>   }
>  
> - if (obj->type == DRM_MODE_OBJECT_PLANE && count_props &&
> - !(arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)) {
> - plane = obj_to_plane(obj);
> - plane_mask |= (1 << drm_plane_index(plane));
> - plane->old_fb = plane->fb;
> - }
>   drm_mode_object_put(obj);
>   }
>  
> @@ -2419,8 +2376,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>   }
>  
>  out:
> - drm_atomic_clean_old_fb(dev, plane_mask, ret);
> -
>   

Re: [Intel-gfx] [PATCH 15/23] drm: Stop updating plane->crtc/fb/old_fb on atomic drivers

[Daniel Vetter]

On Mon, Mar 26, 2018 at 10:52:58PM +0200, Daniel Vetter wrote:
> On Thu, Mar 22, 2018 at 05:23:05PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> > 
> > Stop playing around with plane->crtc/fb/old_fb with atomic
> > drivers. Make life a lot simpler when we don't have to do the
> > magic old_fb vs. fb dance around plane updates. That way we
> > can't risk plane->fb getting out of sync with plane->state->fb
> > and we're less likely to leak any refcounts as well.
> > 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/drm_atomic.c| 55 
> > -
> >  drivers/gpu/drm/drm_atomic_helper.c | 15 +-
> >  drivers/gpu/drm/drm_crtc.c  |  8 --
> >  drivers/gpu/drm/drm_fb_helper.c |  7 -
> >  drivers/gpu/drm/drm_framebuffer.c   |  5 
> >  drivers/gpu/drm/drm_plane.c | 14 ++
> >  drivers/gpu/drm/drm_plane_helper.c  |  4 ++-
> >  include/drm/drm_atomic.h|  3 --
> >  8 files changed, 24 insertions(+), 87 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 7d25c42f22db..b16cc37e2adf 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -692,6 +692,11 @@ drm_atomic_get_plane_state(struct drm_atomic_state 
> > *state,
> >  
> > WARN_ON(!state->acquire_ctx);
> >  
> > +   /* the legacy pointers should never be set */
> > +   WARN_ON(plane->fb);
> > +   WARN_ON(plane->old_fb);
> > +   WARN_ON(plane->crtc);
> 
> I did already review all the users for plane->crtc and found:
> 
> armada + shmob: not atomic, should be fine
> 
> But there's also exynos, msm/mdp5, sti and vc4 doing various silly things
> with setting plane->crtc. I think before you can add this WARN_ON we need
> to clean up that cruft (it looks like 100% cargo culting, so should be
> quit).

Ah, follow-up patches take care of most of this. But note that msm sets
plane->crtc in its _init function, so will trip over your WARN_ON here.

And you seem to have missed sti, which looks at plane->crtc instead of
plane->state->crtc (and appropriate locking) in its debugfs code.
-Daniel

> 
> Going to think about the other patches tomorrow.
> -Daniel
> 
> > +
> > plane_state = drm_atomic_get_existing_plane_state(state, plane);
> > if (plane_state)
> > return plane_state;
> > @@ -2021,45 +2026,6 @@ int drm_atomic_set_property(struct drm_atomic_state 
> > *state,
> > return ret;
> >  }
> >  
> > -/**
> > - * drm_atomic_clean_old_fb -- Unset old_fb pointers and set plane->fb 
> > pointers.
> > - *
> > - * @dev: drm device to check.
> > - * @plane_mask: plane mask for planes that were updated.
> > - * @ret: return value, can be -EDEADLK for a retry.
> > - *
> > - * Before doing an update _plane.old_fb is set to _plane.fb, but 
> > before
> > - * dropping the locks old_fb needs to be set to NULL and plane->fb 
> > updated. This
> > - * is a common operation for each atomic update, so this call is split off 
> > as a
> > - * helper.
> > - */
> > -void drm_atomic_clean_old_fb(struct drm_device *dev,
> > -unsigned plane_mask,
> > -int ret)
> > -{
> > -   struct drm_plane *plane;
> > -
> > -   /* if succeeded, fixup legacy plane crtc/fb ptrs before dropping
> > -* locks (ie. while it is still safe to deref plane->state).  We
> > -* need to do this here because the driver entry points cannot
> > -* distinguish between legacy and atomic ioctls.
> > -*/
> > -   drm_for_each_plane_mask(plane, dev, plane_mask) {
> > -   if (ret == 0) {
> > -   struct drm_framebuffer *new_fb = plane->state->fb;
> > -   if (new_fb)
> > -   drm_framebuffer_get(new_fb);
> > -   plane->fb = new_fb;
> > -   plane->crtc = plane->state->crtc;
> > -
> > -   if (plane->old_fb)
> > -   drm_framebuffer_put(plane->old_fb);
> > -   }
> > -   plane->old_fb = NULL;
> > -   }
> > -}
> > -EXPORT_SYMBOL(drm_atomic_clean_old_fb);
> > -
> >  /**
> >   * DOC: explicit fencing properties
> >   *
> > @@ -2280,9 +2246,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> > unsigned int copied_objs, copied_props;
> > struct drm_atomic_state *state;
> > struct drm_modeset_acquire_ctx ctx;
> > -   struct drm_plane *plane;
> > struct drm_out_fence_state *fence_state;
> > -   unsigned plane_mask;
> > int ret = 0;
> > unsigned int i, j, num_fences;
> >  
> > @@ -2322,7 +2286,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> > state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET);
> >  
> >  retry:
> > -   plane_mask = 0;
> > copied_objs = 0;
> > copied_props = 0;
> > fence_state = NULL;
> > @@ -2393,12 +2356,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> 

Re: [Intel-gfx] [PATCH 15/23] drm: Stop updating plane->crtc/fb/old_fb on atomic drivers

[Daniel Vetter]

On Thu, Mar 22, 2018 at 05:23:05PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Stop playing around with plane->crtc/fb/old_fb with atomic
> drivers. Make life a lot simpler when we don't have to do the
> magic old_fb vs. fb dance around plane updates. That way we
> can't risk plane->fb getting out of sync with plane->state->fb
> and we're less likely to leak any refcounts as well.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/drm_atomic.c| 55 
> -
>  drivers/gpu/drm/drm_atomic_helper.c | 15 +-
>  drivers/gpu/drm/drm_crtc.c  |  8 --
>  drivers/gpu/drm/drm_fb_helper.c |  7 -
>  drivers/gpu/drm/drm_framebuffer.c   |  5 
>  drivers/gpu/drm/drm_plane.c | 14 ++
>  drivers/gpu/drm/drm_plane_helper.c  |  4 ++-
>  include/drm/drm_atomic.h|  3 --
>  8 files changed, 24 insertions(+), 87 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 7d25c42f22db..b16cc37e2adf 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -692,6 +692,11 @@ drm_atomic_get_plane_state(struct drm_atomic_state 
> *state,
>  
>   WARN_ON(!state->acquire_ctx);
>  
> + /* the legacy pointers should never be set */
> + WARN_ON(plane->fb);
> + WARN_ON(plane->old_fb);
> + WARN_ON(plane->crtc);

I did already review all the users for plane->crtc and found:

armada + shmob: not atomic, should be fine

But there's also exynos, msm/mdp5, sti and vc4 doing various silly things
with setting plane->crtc. I think before you can add this WARN_ON we need
to clean up that cruft (it looks like 100% cargo culting, so should be
quit).

Going to think about the other patches tomorrow.
-Daniel

> +
>   plane_state = drm_atomic_get_existing_plane_state(state, plane);
>   if (plane_state)
>   return plane_state;
> @@ -2021,45 +2026,6 @@ int drm_atomic_set_property(struct drm_atomic_state 
> *state,
>   return ret;
>  }
>  
> -/**
> - * drm_atomic_clean_old_fb -- Unset old_fb pointers and set plane->fb 
> pointers.
> - *
> - * @dev: drm device to check.
> - * @plane_mask: plane mask for planes that were updated.
> - * @ret: return value, can be -EDEADLK for a retry.
> - *
> - * Before doing an update _plane.old_fb is set to _plane.fb, but 
> before
> - * dropping the locks old_fb needs to be set to NULL and plane->fb updated. 
> This
> - * is a common operation for each atomic update, so this call is split off 
> as a
> - * helper.
> - */
> -void drm_atomic_clean_old_fb(struct drm_device *dev,
> -  unsigned plane_mask,
> -  int ret)
> -{
> - struct drm_plane *plane;
> -
> - /* if succeeded, fixup legacy plane crtc/fb ptrs before dropping
> -  * locks (ie. while it is still safe to deref plane->state).  We
> -  * need to do this here because the driver entry points cannot
> -  * distinguish between legacy and atomic ioctls.
> -  */
> - drm_for_each_plane_mask(plane, dev, plane_mask) {
> - if (ret == 0) {
> - struct drm_framebuffer *new_fb = plane->state->fb;
> - if (new_fb)
> - drm_framebuffer_get(new_fb);
> - plane->fb = new_fb;
> - plane->crtc = plane->state->crtc;
> -
> - if (plane->old_fb)
> - drm_framebuffer_put(plane->old_fb);
> - }
> - plane->old_fb = NULL;
> - }
> -}
> -EXPORT_SYMBOL(drm_atomic_clean_old_fb);
> -
>  /**
>   * DOC: explicit fencing properties
>   *
> @@ -2280,9 +2246,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>   unsigned int copied_objs, copied_props;
>   struct drm_atomic_state *state;
>   struct drm_modeset_acquire_ctx ctx;
> - struct drm_plane *plane;
>   struct drm_out_fence_state *fence_state;
> - unsigned plane_mask;
>   int ret = 0;
>   unsigned int i, j, num_fences;
>  
> @@ -2322,7 +2286,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>   state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET);
>  
>  retry:
> - plane_mask = 0;
>   copied_objs = 0;
>   copied_props = 0;
>   fence_state = NULL;
> @@ -2393,12 +2356,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>   copied_props++;
>   }
>  
> - if (obj->type == DRM_MODE_OBJECT_PLANE && count_props &&
> - !(arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)) {
> - plane = obj_to_plane(obj);
> - plane_mask |= (1 << drm_plane_index(plane));
> - plane->old_fb = plane->fb;
> - }
>   drm_mode_object_put(obj);
>   }
>  
> @@ -2419,8 +2376,6 @@ int drm_mode_atomic_ioctl(struct drm_device 

[Intel-gfx] [PATCH 15/23] drm: Stop updating plane->crtc/fb/old_fb on atomic drivers

[Ville Syrjala]

From: Ville Syrjälä 

Stop playing around with plane->crtc/fb/old_fb with atomic
drivers. Make life a lot simpler when we don't have to do the
magic old_fb vs. fb dance around plane updates. That way we
can't risk plane->fb getting out of sync with plane->state->fb
and we're less likely to leak any refcounts as well.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/drm_atomic.c| 55 -
 drivers/gpu/drm/drm_atomic_helper.c | 15 +-
 drivers/gpu/drm/drm_crtc.c  |  8 --
 drivers/gpu/drm/drm_fb_helper.c |  7 -
 drivers/gpu/drm/drm_framebuffer.c   |  5 
 drivers/gpu/drm/drm_plane.c | 14 ++
 drivers/gpu/drm/drm_plane_helper.c  |  4 ++-
 include/drm/drm_atomic.h|  3 --
 8 files changed, 24 insertions(+), 87 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 7d25c42f22db..b16cc37e2adf 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -692,6 +692,11 @@ drm_atomic_get_plane_state(struct drm_atomic_state *state,
 
WARN_ON(!state->acquire_ctx);
 
+   /* the legacy pointers should never be set */
+   WARN_ON(plane->fb);
+   WARN_ON(plane->old_fb);
+   WARN_ON(plane->crtc);
+
plane_state = drm_atomic_get_existing_plane_state(state, plane);
if (plane_state)
return plane_state;
@@ -2021,45 +2026,6 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
return ret;
 }
 
-/**
- * drm_atomic_clean_old_fb -- Unset old_fb pointers and set plane->fb pointers.
- *
- * @dev: drm device to check.
- * @plane_mask: plane mask for planes that were updated.
- * @ret: return value, can be -EDEADLK for a retry.
- *
- * Before doing an update _plane.old_fb is set to _plane.fb, but before
- * dropping the locks old_fb needs to be set to NULL and plane->fb updated. 
This
- * is a common operation for each atomic update, so this call is split off as a
- * helper.
- */
-void drm_atomic_clean_old_fb(struct drm_device *dev,
-unsigned plane_mask,
-int ret)
-{
-   struct drm_plane *plane;
-
-   /* if succeeded, fixup legacy plane crtc/fb ptrs before dropping
-* locks (ie. while it is still safe to deref plane->state).  We
-* need to do this here because the driver entry points cannot
-* distinguish between legacy and atomic ioctls.
-*/
-   drm_for_each_plane_mask(plane, dev, plane_mask) {
-   if (ret == 0) {
-   struct drm_framebuffer *new_fb = plane->state->fb;
-   if (new_fb)
-   drm_framebuffer_get(new_fb);
-   plane->fb = new_fb;
-   plane->crtc = plane->state->crtc;
-
-   if (plane->old_fb)
-   drm_framebuffer_put(plane->old_fb);
-   }
-   plane->old_fb = NULL;
-   }
-}
-EXPORT_SYMBOL(drm_atomic_clean_old_fb);
-
 /**
  * DOC: explicit fencing properties
  *
@@ -2280,9 +2246,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
unsigned int copied_objs, copied_props;
struct drm_atomic_state *state;
struct drm_modeset_acquire_ctx ctx;
-   struct drm_plane *plane;
struct drm_out_fence_state *fence_state;
-   unsigned plane_mask;
int ret = 0;
unsigned int i, j, num_fences;
 
@@ -2322,7 +2286,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET);
 
 retry:
-   plane_mask = 0;
copied_objs = 0;
copied_props = 0;
fence_state = NULL;
@@ -2393,12 +2356,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
copied_props++;
}
 
-   if (obj->type == DRM_MODE_OBJECT_PLANE && count_props &&
-   !(arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)) {
-   plane = obj_to_plane(obj);
-   plane_mask |= (1 << drm_plane_index(plane));
-   plane->old_fb = plane->fb;
-   }
drm_mode_object_put(obj);
}
 
@@ -2419,8 +2376,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
}
 
 out:
-   drm_atomic_clean_old_fb(dev, plane_mask, ret);
-
complete_crtc_signaling(dev, state, fence_state, num_fences, !ret);
 
if (ret == -EDEADLK) {
diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 0e806f070d00..d42d88b97396 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2892,7 +2892,6 @@ static int __drm_atomic_helper_disable_all(struct 
drm_device *dev,
struct drm_plane *plane;
struct drm_crtc_state *crtc_state;
struct drm_crtc *crtc;
-