[Intel-gfx] [PATCH 6/7] drm/i915: Switch plane handling to atomic helpers

[Bob Paauwe]

On Thu, 13 Nov 2014 10:43:25 -0800
Matt Roper <matthew.d.roper at intel.com> wrote:

> Now that we have hooks to enable the atomic plane helpers, we can use
> the plane helpers for our .update_plane() and .disable_plane()
> entrypoints.
> 
> Even though we'd already refactored our plane handling code into
> check/commit, we still have to rework some of that logic here as we make
> the transition.  The atomic plane helpers now call prepare_fb/cleanup_fb
> for us, so note the following:
>  - 'commit' should not longer unpin the old object; the cleanup_fb will

s/not/no/

>    take care of this for us
>  - prepare_fb only gets called when we actually have an fb, so the
>    internal 'disable' code needs to update frontbuffer tracking
>  - the cursor plane code never had 'prepare' code separated from its
>    'commit' function, so a lot of the cursor's 'commit' needs to be
>    dropped since prepare_fb already does it for us
>  - with all the pinning work removed from cursor commit, it can no
>    longer fail and should return void rather than int
> 
> Testcase: igt/kms_plane
> Testcase: igt/kms_universal_plane
> Testcase: igt/kms_cursor_crc
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>

With the above typo fixed this is
Acknowledged-by: Bob Paauwe <bob.j.paauwe at intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 255 +++++------------------------------
>  drivers/gpu/drm/i915/intel_sprite.c  | 122 ++---------------
>  2 files changed, 41 insertions(+), 336 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 32a2b12..8d5a3b4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8285,17 +8285,15 @@ static bool cursor_size_ok(struct drm_device *dev,
>  	return true;
>  }
>  
> -static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc,
> -				     struct drm_i915_gem_object *obj,
> -				     uint32_t width, uint32_t height)
> +static void intel_crtc_cursor_set_obj(struct drm_crtc *crtc,
> +				      struct drm_i915_gem_object *obj,
> +				      uint32_t width, uint32_t height)
>  {
>  	struct drm_device *dev = crtc->dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	enum pipe pipe = intel_crtc->pipe;
>  	unsigned old_width;
>  	uint32_t addr;
> -	int ret;
>  
>  	/* if we want to turn off the cursor ignore width and height */
>  	if (!obj) {
> @@ -8305,64 +8303,14 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc,
>  		goto finish;
>  	}
>  
> -	/* we only need to pin inside GTT if cursor is non-phy */
> +	/* plane helper's prepare_fb() already handled pinning */
>  	mutex_lock(&dev->struct_mutex);
> -	if (!INTEL_INFO(dev)->cursor_needs_physical) {
> -		unsigned alignment;
> -
> -		/*
> -		 * Global gtt pte registers are special registers which actually
> -		 * forward writes to a chunk of system memory. Which means that
> -		 * there is no risk that the register values disappear as soon
> -		 * as we call intel_runtime_pm_put(), so it is correct to wrap
> -		 * only the pin/unpin/fence and not more.
> -		 */
> -		intel_runtime_pm_get(dev_priv);
> -
> -		/* Note that the w/a also requires 2 PTE of padding following
> -		 * the bo. We currently fill all unused PTE with the shadow
> -		 * page and so we should always have valid PTE following the
> -		 * cursor preventing the VT-d warning.
> -		 */
> -		alignment = 0;
> -		if (need_vtd_wa(dev))
> -			alignment = 64*1024;
> -
> -		ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL);
> -		if (ret) {
> -			DRM_DEBUG_KMS("failed to move cursor bo into the GTT\n");
> -			intel_runtime_pm_put(dev_priv);
> -			goto fail_locked;
> -		}
> -
> -		ret = i915_gem_object_put_fence(obj);
> -		if (ret) {
> -			DRM_DEBUG_KMS("failed to release fence for cursor");
> -			intel_runtime_pm_put(dev_priv);
> -			goto fail_unpin;
> -		}
> -
> +	if (!INTEL_INFO(dev)->cursor_needs_physical)
>  		addr = i915_gem_obj_ggtt_offset(obj);
> -
> -		intel_runtime_pm_put(dev_priv);
> -	} else {
> -		int align = IS_I830(dev) ? 16 * 1024 : 256;
> -		ret = i915_gem_object_attach_phys(obj, align);
> -		if (ret) {
> -			DRM_DEBUG_KMS("failed to attach phys object\n");
> -			goto fail_locked;
> -		}
> +	else
>  		addr = obj->phys_handle->busaddr;
> -	}
>  
>   finish:
> -	if (intel_crtc->cursor_bo) {
> -		if (!INTEL_INFO(dev)->cursor_needs_physical)
> -			i915_gem_object_unpin_from_display_plane(intel_crtc->cursor_bo);
> -	}
> -
> -	i915_gem_track_fb(intel_crtc->cursor_bo, obj,
> -			  INTEL_FRONTBUFFER_CURSOR(pipe));
>  	mutex_unlock(&dev->struct_mutex);
>  
>  	old_width = intel_crtc->cursor_width;
> @@ -8379,13 +8327,6 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc,
>  
>  		intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe));
>  	}
> -
> -	return 0;
> -fail_unpin:
> -	i915_gem_object_unpin_from_display_plane(obj);
> -fail_locked:
> -	mutex_unlock(&dev->struct_mutex);
> -	return ret;
>  }
>  
>  static void intel_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> @@ -11544,7 +11485,6 @@ disable_unpin:
>  	mutex_lock(&dev->struct_mutex);
>  	i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
>  			  INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe));
> -	intel_unpin_fb_obj(intel_fb_obj(plane->fb));
>  	mutex_unlock(&dev->struct_mutex);
>  	plane->fb = NULL;
>  
> @@ -11568,42 +11508,6 @@ intel_check_primary_plane(struct drm_plane *plane,
>  					     false, true, &state->visible);
>  }
>  
> -static int
> -intel_prepare_primary_plane(struct drm_plane *plane,
> -			    struct intel_plane_state *state)
> -{
> -	struct drm_crtc *crtc = state->base.crtc;
> -	struct drm_framebuffer *fb = state->base.fb;
> -	struct drm_device *dev = crtc->dev;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	enum pipe pipe = intel_crtc->pipe;
> -	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> -	struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb);
> -	int ret;
> -
> -	intel_crtc_wait_for_pending_flips(crtc);
> -
> -	if (intel_crtc_has_pending_flip(crtc)) {
> -		DRM_ERROR("pipe is still busy with an old pageflip\n");
> -		return -EBUSY;
> -	}
> -
> -	if (old_obj != obj) {
> -		mutex_lock(&dev->struct_mutex);
> -		ret = intel_pin_and_fence_fb_obj(plane, fb, NULL);
> -		if (ret == 0)
> -			i915_gem_track_fb(old_obj, obj,
> -					  INTEL_FRONTBUFFER_PRIMARY(pipe));
> -		mutex_unlock(&dev->struct_mutex);
> -		if (ret != 0) {
> -			DRM_DEBUG_KMS("pin & fence failed\n");
> -			return ret;
> -		}
> -	}
> -
> -	return 0;
> -}
> -
>  static void
>  intel_commit_primary_plane(struct drm_plane *plane,
>  			   struct intel_plane_state *state)
> @@ -11614,9 +11518,7 @@ intel_commit_primary_plane(struct drm_plane *plane,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	enum pipe pipe = intel_crtc->pipe;
> -	struct drm_framebuffer *old_fb = plane->fb;
>  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> -	struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb);
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
>  	struct drm_rect *src = &state->src;
>  
> @@ -11687,62 +11589,6 @@ intel_commit_primary_plane(struct drm_plane *plane,
>  		intel_update_fbc(dev);
>  		mutex_unlock(&dev->struct_mutex);
>  	}
> -
> -	if (old_fb && old_fb != fb) {
> -		if (intel_crtc->active)
> -			intel_wait_for_vblank(dev, intel_crtc->pipe);
> -
> -		mutex_lock(&dev->struct_mutex);
> -		intel_unpin_fb_obj(old_obj);
> -		mutex_unlock(&dev->struct_mutex);
> -	}
> -}
> -
> -static int
> -intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
> -			     struct drm_framebuffer *fb, int crtc_x, int crtc_y,
> -			     unsigned int crtc_w, unsigned int crtc_h,
> -			     uint32_t src_x, uint32_t src_y,
> -			     uint32_t src_w, uint32_t src_h)
> -{
> -	struct intel_plane_state state;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	int ret;
> -
> -	state.base.crtc = crtc;
> -	state.base.fb = fb;
> -
> -	/* sample coordinates in 16.16 fixed point */
> -	state.src.x1 = src_x;
> -	state.src.x2 = src_x + src_w;
> -	state.src.y1 = src_y;
> -	state.src.y2 = src_y + src_h;
> -
> -	/* integer pixels */
> -	state.dst.x1 = crtc_x;
> -	state.dst.x2 = crtc_x + crtc_w;
> -	state.dst.y1 = crtc_y;
> -	state.dst.y2 = crtc_y + crtc_h;
> -
> -	state.clip.x1 = 0;
> -	state.clip.y1 = 0;
> -	state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0;
> -	state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0;
> -
> -	state.orig_src = state.src;
> -	state.orig_dst = state.dst;
> -
> -	ret = intel_check_primary_plane(plane, &state);
> -	if (ret)
> -		return ret;
> -
> -	ret = intel_prepare_primary_plane(plane, &state);
> -	if (ret)
> -		return ret;
> -
> -	intel_commit_primary_plane(plane, &state);
> -
> -	return 0;
>  }
>  
>  /* Common destruction function for both primary and cursor planes */
> @@ -11755,8 +11601,8 @@ void intel_plane_destroy(struct drm_plane *plane)
>  }
>  
>  static const struct drm_plane_funcs intel_primary_plane_funcs = {
> -	.update_plane = intel_primary_plane_setplane,
> -	.disable_plane = intel_primary_plane_disable,
> +	.update_plane = drm_plane_helper_update,
> +	.disable_plane = drm_plane_helper_disable,
>  	.destroy = intel_plane_destroy,
>  	.set_property = intel_plane_set_property,
>  	.atomic_duplicate_state = intel_plane_duplicate_state,
> @@ -11816,15 +11662,22 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>  	return &primary->base;
>  }
>  
> -static int
> +static void
>  intel_cursor_plane_disable(struct drm_plane *plane)
>  {
> +	struct intel_plane *intel_plane = to_intel_plane(plane);
> +
>  	if (!plane->fb)
> -		return 0;
> +		return;
>  
>  	BUG_ON(!plane->crtc);
>  
> -	return intel_crtc_cursor_set_obj(plane->crtc, NULL, 0, 0);
> +	intel_crtc_cursor_set_obj(plane->crtc, NULL, 0, 0);
> +
> +	mutex_lock(&plane->dev->struct_mutex);
> +	i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
> +			  INTEL_FRONTBUFFER_CURSOR(intel_plane->pipe));
> +	mutex_unlock(&plane->dev->struct_mutex);
>  }
>  
>  static int
> @@ -11838,7 +11691,6 @@ intel_check_cursor_plane(struct drm_plane *plane,
>  	struct drm_rect *src = &state->src;
>  	const struct drm_rect *clip = &state->clip;
>  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> -	int crtc_w, crtc_h;
>  	unsigned stride;
>  	int ret;
>  
> @@ -11856,15 +11708,14 @@ intel_check_cursor_plane(struct drm_plane *plane,
>  		return 0;
>  
>  	/* Check for which cursor types we support */
> -	crtc_w = drm_rect_width(&state->orig_dst);
> -	crtc_h = drm_rect_height(&state->orig_dst);
> -	if (!cursor_size_ok(dev, crtc_w, crtc_h)) {
> -		DRM_DEBUG("Cursor dimension not supported\n");
> +	if (!cursor_size_ok(dev, state->base.crtc_w, state->base.crtc_h)) {
> +		DRM_DEBUG("Cursor dimension %dx%d not supported\n",
> +			  state->base.crtc_w, state->base.crtc_h);
>  		return -EINVAL;
>  	}
>  
> -	stride = roundup_pow_of_two(crtc_w) * 4;
> -	if (obj->base.size < stride * crtc_h) {
> +	stride = roundup_pow_of_two(state->base.crtc_w) * 4;
> +	if (obj->base.size < stride * state->base.crtc_h) {
>  		DRM_DEBUG_KMS("buffer is too small\n");
>  		return -ENOMEM;
>  	}
> @@ -11883,7 +11734,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
>  	return ret;
>  }
>  
> -static int
> +static void
>  intel_commit_cursor_plane(struct drm_plane *plane,
>  			  struct intel_plane_state *state)
>  {
> @@ -11893,10 +11744,9 @@ intel_commit_cursor_plane(struct drm_plane *plane,
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
>  	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
>  	struct drm_i915_gem_object *obj = intel_fb->obj;
> -	int crtc_w, crtc_h;
>  
> -	crtc->cursor_x = state->orig_dst.x1;
> -	crtc->cursor_y = state->orig_dst.y1;
> +	crtc->cursor_x = state->base.crtc_x;
> +	crtc->cursor_y = state->base.crtc_y;
>  
>  	intel_plane->crtc_x = state->orig_dst.x1;
>  	intel_plane->crtc_y = state->orig_dst.y1;
> @@ -11909,63 +11759,20 @@ intel_commit_cursor_plane(struct drm_plane *plane,
>  	intel_plane->obj = obj;
>  
>  	if (fb != crtc->cursor->fb) {
> -		crtc_w = drm_rect_width(&state->orig_dst);
> -		crtc_h = drm_rect_height(&state->orig_dst);
> -		return intel_crtc_cursor_set_obj(crtc, obj, crtc_w, crtc_h);
> +		intel_crtc_cursor_set_obj(crtc, obj,
> +					  state->base.crtc_w,
> +					  state->base.crtc_h);
>  	} else {
>  		intel_crtc_update_cursor(crtc, state->visible);
>  
>  		intel_frontbuffer_flip(crtc->dev,
>  				       INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe));
> -
> -		return 0;
>  	}
>  }
>  
> -static int
> -intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
> -			  struct drm_framebuffer *fb, int crtc_x, int crtc_y,
> -			  unsigned int crtc_w, unsigned int crtc_h,
> -			  uint32_t src_x, uint32_t src_y,
> -			  uint32_t src_w, uint32_t src_h)
> -{
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct intel_plane_state state;
> -	int ret;
> -
> -	state.base.crtc = crtc;
> -	state.base.fb = fb;
> -
> -	/* sample coordinates in 16.16 fixed point */
> -	state.src.x1 = src_x;
> -	state.src.x2 = src_x + src_w;
> -	state.src.y1 = src_y;
> -	state.src.y2 = src_y + src_h;
> -
> -	/* integer pixels */
> -	state.dst.x1 = crtc_x;
> -	state.dst.x2 = crtc_x + crtc_w;
> -	state.dst.y1 = crtc_y;
> -	state.dst.y2 = crtc_y + crtc_h;
> -
> -	state.clip.x1 = 0;
> -	state.clip.y1 = 0;
> -	state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0;
> -	state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0;
> -
> -	state.orig_src = state.src;
> -	state.orig_dst = state.dst;
> -
> -	ret = intel_check_cursor_plane(plane, &state);
> -	if (ret)
> -		return ret;
> -
> -	return intel_commit_cursor_plane(plane, &state);
> -}
> -
>  static const struct drm_plane_funcs intel_cursor_plane_funcs = {
> -	.update_plane = intel_cursor_plane_update,
> -	.disable_plane = intel_cursor_plane_disable,
> +	.update_plane = drm_plane_helper_update,
> +	.disable_plane = drm_plane_helper_disable,
>  	.destroy = intel_plane_destroy,
>  	.set_property = intel_plane_set_property,
>  	.atomic_duplicate_state = intel_plane_duplicate_state,
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index d3677c3..28ccaf7 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1106,7 +1106,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
>  	uint32_t src_x, src_y, src_w, src_h;
>  	struct drm_rect *src = &state->src;
>  	struct drm_rect *dst = &state->dst;
> -	struct drm_rect *orig_src = &state->orig_src;
>  	const struct drm_rect *clip = &state->clip;
>  	int hscale, vscale;
>  	int max_scale, min_scale;
> @@ -1187,10 +1186,10 @@ intel_check_sprite_plane(struct drm_plane *plane,
>  				    intel_plane->rotation);
>  
>  		/* sanity check to make sure the src viewport wasn't enlarged */
> -		WARN_ON(src->x1 < (int) orig_src->x1 ||
> -			src->y1 < (int) orig_src->y1 ||
> -			src->x2 > (int) orig_src->x2 ||
> -			src->y2 > (int) orig_src->y2);
> +		WARN_ON(src->x1 < (int) state->base.src_x ||
> +			src->y1 < (int) state->base.src_y ||
> +			src->x2 > (int) state->base.src_x + state->base.src_w ||
> +			src->y2 > (int) state->base.src_y + state->base.src_h);
>  
>  		/*
>  		 * Hardware doesn't handle subpixel coordinates.
> @@ -1258,41 +1257,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
>  	return 0;
>  }
>  
> -static int
> -intel_prepare_sprite_plane(struct drm_plane *plane,
> -			   struct intel_plane_state *state)
> -{
> -	struct drm_device *dev = plane->dev;
> -	struct drm_crtc *crtc = state->base.crtc;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct intel_plane *intel_plane = to_intel_plane(plane);
> -	enum pipe pipe = intel_crtc->pipe;
> -	struct drm_framebuffer *fb = state->base.fb;
> -	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> -	struct drm_i915_gem_object *old_obj = intel_plane->obj;
> -	int ret;
> -
> -	if (old_obj != obj) {
> -		mutex_lock(&dev->struct_mutex);
> -
> -		/* Note that this will apply the VT-d workaround for scanouts,
> -		 * which is more restrictive than required for sprites. (The
> -		 * primary plane requires 256KiB alignment with 64 PTE padding,
> -		 * the sprite planes only require 128KiB alignment and 32 PTE
> -		 * padding.
> -		 */
> -		ret = intel_pin_and_fence_fb_obj(plane, fb, NULL);
> -		if (ret == 0)
> -			i915_gem_track_fb(old_obj, obj,
> -					  INTEL_FRONTBUFFER_SPRITE(pipe));
> -		mutex_unlock(&dev->struct_mutex);
> -		if (ret)
> -			return ret;
> -	}
> -
> -	return 0;
> -}
> -
>  static void
>  intel_commit_sprite_plane(struct drm_plane *plane,
>  			  struct intel_plane_state *state)
> @@ -1304,7 +1268,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
>  	enum pipe pipe = intel_crtc->pipe;
>  	struct drm_framebuffer *fb = state->base.fb;
>  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> -	struct drm_i915_gem_object *old_obj = intel_plane->obj;
>  	int crtc_x, crtc_y;
>  	unsigned int crtc_w, crtc_h;
>  	uint32_t src_x, src_y, src_w, src_h;
> @@ -1362,68 +1325,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
>  		if (!primary_was_enabled && primary_enabled)
>  			intel_post_enable_primary(crtc);
>  	}
> -
> -	/* Unpin old obj after new one is active to avoid ugliness */
> -	if (old_obj && old_obj != obj) {
> -
> -		/*
> -		 * It's fairly common to simply update the position of
> -		 * an existing object.  In that case, we don't need to
> -		 * wait for vblank to avoid ugliness, we only need to
> -		 * do the pin & ref bookkeeping.
> -		 */
> -		if (intel_crtc->active)
> -			intel_wait_for_vblank(dev, intel_crtc->pipe);
> -
> -		mutex_lock(&dev->struct_mutex);
> -		intel_unpin_fb_obj(old_obj);
> -		mutex_unlock(&dev->struct_mutex);
> -	}
> -}
> -
> -static int
> -intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> -		   struct drm_framebuffer *fb, int crtc_x, int crtc_y,
> -		   unsigned int crtc_w, unsigned int crtc_h,
> -		   uint32_t src_x, uint32_t src_y,
> -		   uint32_t src_w, uint32_t src_h)
> -{
> -	struct intel_plane_state state;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	int ret;
> -
> -	state.base.crtc = crtc;
> -	state.base.fb = fb;
> -
> -	/* sample coordinates in 16.16 fixed point */
> -	state.src.x1 = src_x;
> -	state.src.x2 = src_x + src_w;
> -	state.src.y1 = src_y;
> -	state.src.y2 = src_y + src_h;
> -
> -	/* integer pixels */
> -	state.dst.x1 = crtc_x;
> -	state.dst.x2 = crtc_x + crtc_w;
> -	state.dst.y1 = crtc_y;
> -	state.dst.y2 = crtc_y + crtc_h;
> -
> -	state.clip.x1 = 0;
> -	state.clip.y1 = 0;
> -	state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0;
> -	state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0;
> -	state.orig_src = state.src;
> -	state.orig_dst = state.dst;
> -
> -	ret = intel_check_sprite_plane(plane, &state);
> -	if (ret)
> -		return ret;
> -
> -	ret = intel_prepare_sprite_plane(plane, &state);
> -	if (ret)
> -		return ret;
> -
> -	intel_commit_sprite_plane(plane, &state);
> -	return 0;
>  }
>  
>  int
> @@ -1459,7 +1360,6 @@ intel_sprite_plane_disable(struct drm_plane *plane)
>  			intel_wait_for_vblank(dev, intel_plane->pipe);
>  
>  		mutex_lock(&dev->struct_mutex);
> -		intel_unpin_fb_obj(intel_plane->obj);
>  		i915_gem_track_fb(intel_plane->obj, NULL,
>  				  INTEL_FRONTBUFFER_SPRITE(pipe));
>  		mutex_unlock(&dev->struct_mutex);
> @@ -1557,21 +1457,19 @@ int intel_plane_set_property(struct drm_plane *plane,
>  
>  int intel_plane_restore(struct drm_plane *plane)
>  {
> -	struct intel_plane *intel_plane = to_intel_plane(plane);
> -
>  	if (!plane->crtc || !plane->fb)
>  		return 0;
>  
>  	return plane->funcs->update_plane(plane, plane->crtc, plane->fb,
> -				  intel_plane->crtc_x, intel_plane->crtc_y,
> -				  intel_plane->crtc_w, intel_plane->crtc_h,
> -				  intel_plane->src_x, intel_plane->src_y,
> -				  intel_plane->src_w, intel_plane->src_h);
> +				  plane->state->crtc_x, plane->state->crtc_y,
> +				  plane->state->crtc_w, plane->state->crtc_h,
> +				  plane->state->src_x, plane->state->src_y,
> +				  plane->state->src_w, plane->state->src_h);
>  }
>  
>  static const struct drm_plane_funcs intel_plane_funcs = {
> -	.update_plane = intel_update_plane,
> -	.disable_plane = intel_sprite_plane_disable,
> +	.update_plane = drm_plane_helper_update,
> +	.disable_plane = drm_plane_helper_disable,
>  	.destroy = intel_plane_destroy,
>  	.set_property = intel_plane_set_property,
>  	.atomic_duplicate_state = intel_plane_duplicate_state,