diff mbox

[44/58] drm/i915: push crtc->fb update into pipe_set_base

Message ID 1345403595-9678-45-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State Accepted
Headers show

Commit Message

Daniel Vetter Aug. 19, 2012, 7:13 p.m. UTC
Passing in the old fb, having overwritten the current fb, leads to
some neatly convoluted code. It's much simpler if we defer the
crtc->fb update to the place that updates the hw, in pipe_set_base.
This way we also don't need to restore anything in case something
fails - we only update crtc->fb once things have succeeded.

The real reason for this change is that now we keep the old fb
assigned to crtc->fb, which allows us to finally move the crtc disable
case into the common low-level set_mode function in the next patch.

Also don't clobber crtc->x and crtc->y, we neatly pass these down the
callchain already. Unfortunately we can't do the same with crtc->mode,
because that one is being used in the mode_set callbacks.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 84 +++++++++++++++---------------------
 1 file changed, 34 insertions(+), 50 deletions(-)

Comments

Jesse Barnes Sept. 5, 2012, 5:54 p.m. UTC | #1
On Sun, 19 Aug 2012 21:13:01 +0200
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> Passing in the old fb, having overwritten the current fb, leads to
> some neatly convoluted code. It's much simpler if we defer the
> crtc->fb update to the place that updates the hw, in pipe_set_base.
> This way we also don't need to restore anything in case something
> fails - we only update crtc->fb once things have succeeded.
> 
> The real reason for this change is that now we keep the old fb
> assigned to crtc->fb, which allows us to finally move the crtc disable
> case into the common low-level set_mode function in the next patch.
> 
> Also don't clobber crtc->x and crtc->y, we neatly pass these down the
> callchain already. Unfortunately we can't do the same with crtc->mode,
> because that one is being used in the mode_set callbacks.
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 84 +++++++++++++++---------------------
>  1 file changed, 34 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e6701b4..cd3606c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2201,16 +2201,17 @@ intel_finish_fb(struct drm_framebuffer *old_fb)
>  
>  static int
>  intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
> -		    struct drm_framebuffer *old_fb)
> +		    struct drm_framebuffer *fb)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_i915_master_private *master_priv;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct drm_framebuffer *old_fb;
>  	int ret;
>  
>  	/* no fb bound */
> -	if (!crtc->fb) {
> +	if (!fb) {
>  		DRM_ERROR("No FB bound\n");
>  		return 0;
>  	}
> @@ -2224,7 +2225,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
>  
>  	mutex_lock(&dev->struct_mutex);
>  	ret = intel_pin_and_fence_fb_obj(dev,
> -					 to_intel_framebuffer(crtc->fb)->obj,
> +					 to_intel_framebuffer(fb)->obj,
>  					 NULL);
>  	if (ret != 0) {
>  		mutex_unlock(&dev->struct_mutex);
> @@ -2232,17 +2233,20 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
>  		return ret;
>  	}
>  
> -	if (old_fb)
> -		intel_finish_fb(old_fb);
> +	if (crtc->fb)
> +		intel_finish_fb(crtc->fb);
>  
> -	ret = dev_priv->display.update_plane(crtc, crtc->fb, x, y);
> +	ret = dev_priv->display.update_plane(crtc, fb, x, y);
>  	if (ret) {
> -		intel_unpin_fb_obj(to_intel_framebuffer(crtc->fb)->obj);
> +		intel_unpin_fb_obj(to_intel_framebuffer(fb)->obj);
>  		mutex_unlock(&dev->struct_mutex);
>  		DRM_ERROR("failed to update base address\n");
>  		return ret;
>  	}
>  
> +	old_fb = crtc->fb;
> +	crtc->fb = fb;
> +
>  	if (old_fb) {
>  		intel_wait_for_vblank(dev, intel_crtc->pipe);
>  		intel_unpin_fb_obj(to_intel_framebuffer(old_fb)->obj);
> @@ -3777,6 +3781,7 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
>   * true if they don't match).
>   */
>  static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
> +					 struct drm_framebuffer *fb,
>  					 unsigned int *pipe_bpp,
>  					 struct drm_display_mode *mode)
>  {
> @@ -3846,7 +3851,7 @@ static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
>  	 * also stays within the max display bpc discovered above.
>  	 */
>  
> -	switch (crtc->fb->depth) {
> +	switch (fb->depth) {
>  	case 8:
>  		bpc = 8; /* since we go through a colormap */
>  		break;
> @@ -4265,7 +4270,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  			      struct drm_display_mode *mode,
>  			      struct drm_display_mode *adjusted_mode,
>  			      int x, int y,
> -			      struct drm_framebuffer *old_fb)
> +			      struct drm_framebuffer *fb)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -4455,7 +4460,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  	I915_WRITE(DSPCNTR(plane), dspcntr);
>  	POSTING_READ(DSPCNTR(plane));
>  
> -	ret = intel_pipe_set_base(crtc, x, y, old_fb);
> +	ret = intel_pipe_set_base(crtc, x, y, fb);
>  
>  	intel_update_watermarks(dev);
>  
> @@ -4613,7 +4618,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  				  struct drm_display_mode *mode,
>  				  struct drm_display_mode *adjusted_mode,
>  				  int x, int y,
> -				  struct drm_framebuffer *old_fb)
> +				  struct drm_framebuffer *fb)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -4733,7 +4738,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	/* determine panel color depth */
>  	temp = I915_READ(PIPECONF(pipe));
>  	temp &= ~PIPE_BPC_MASK;
> -	dither = intel_choose_pipe_bpp_dither(crtc, &pipe_bpp, mode);
> +	dither = intel_choose_pipe_bpp_dither(crtc, fb, &pipe_bpp, mode);
>  	switch (pipe_bpp) {
>  	case 18:
>  		temp |= PIPE_6BPC;
> @@ -5002,7 +5007,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	I915_WRITE(DSPCNTR(plane), dspcntr);
>  	POSTING_READ(DSPCNTR(plane));
>  
> -	ret = intel_pipe_set_base(crtc, x, y, old_fb);
> +	ret = intel_pipe_set_base(crtc, x, y, fb);
>  
>  	intel_update_watermarks(dev);
>  
> @@ -5015,7 +5020,7 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
>  			       struct drm_display_mode *mode,
>  			       struct drm_display_mode *adjusted_mode,
>  			       int x, int y,
> -			       struct drm_framebuffer *old_fb)
> +			       struct drm_framebuffer *fb)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -5026,7 +5031,7 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
>  	drm_vblank_pre_modeset(dev, pipe);
>  
>  	ret = dev_priv->display.crtc_mode_set(crtc, mode, adjusted_mode,
> -					      x, y, old_fb);
> +					      x, y, fb);
>  	drm_vblank_post_modeset(dev, pipe);
>  
>  	return ret;
> @@ -5718,7 +5723,7 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
>  	struct drm_encoder *encoder = &intel_encoder->base;
>  	struct drm_crtc *crtc = NULL;
>  	struct drm_device *dev = encoder->dev;
> -	struct drm_framebuffer *old_fb;
> +	struct drm_framebuffer *fb;
>  	int i = -1;
>  
>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
> @@ -5779,8 +5784,6 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
>  	if (!mode)
>  		mode = &load_detect_mode;
>  
> -	old_fb = crtc->fb;
> -
>  	/* We need a framebuffer large enough to accommodate all accesses
>  	 * that the plane may generate whilst we perform load detection.
>  	 * We can not rely on the fbcon either being present (we get called
> @@ -5788,19 +5791,19 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
>  	 * not even exist) or that it is large enough to satisfy the
>  	 * requested mode.
>  	 */
> -	crtc->fb = mode_fits_in_fbdev(dev, mode);
> -	if (crtc->fb == NULL) {
> +	fb = mode_fits_in_fbdev(dev, mode);
> +	if (fb == NULL) {
>  		DRM_DEBUG_KMS("creating tmp fb for load-detection\n");
> -		crtc->fb = intel_framebuffer_create_for_mode(dev, mode, 24, 32);
> -		old->release_fb = crtc->fb;
> +		fb = intel_framebuffer_create_for_mode(dev, mode, 24, 32);
> +		old->release_fb = fb;
>  	} else
>  		DRM_DEBUG_KMS("reusing fbdev for load-detection framebuffer\n");
> -	if (IS_ERR(crtc->fb)) {
> +	if (IS_ERR(fb)) {
>  		DRM_DEBUG_KMS("failed to allocate framebuffer for load-detection\n");
>  		goto fail;
>  	}
>  
> -	if (!intel_set_mode(crtc, mode, 0, 0, old_fb)) {
> +	if (!intel_set_mode(crtc, mode, 0, 0, fb)) {
>  		DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
>  		if (old->release_fb)
>  			old->release_fb->funcs->destroy(old->release_fb);
> @@ -5814,7 +5817,6 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
>  fail:
>  	connector->encoder = NULL;
>  	encoder->crtc = NULL;
> -	crtc->fb = old_fb;
>  	return false;
>  }
>  
> @@ -6666,13 +6668,12 @@ static void intel_modeset_commit_output_state(struct drm_device *dev)
>  
>  bool intel_set_mode(struct drm_crtc *crtc,
>  		    struct drm_display_mode *mode,
> -		    int x, int y, struct drm_framebuffer *old_fb)
> +		    int x, int y, struct drm_framebuffer *fb)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	drm_i915_private_t *dev_priv = dev->dev_private;
>  	struct drm_display_mode *adjusted_mode, saved_mode, saved_hwmode;
>  	struct drm_encoder_helper_funcs *encoder_funcs;
> -	int saved_x, saved_y;
>  	struct drm_encoder *encoder;
>  	bool ret = true;
>  
> @@ -6686,15 +6687,11 @@ bool intel_set_mode(struct drm_crtc *crtc,
>  
>  	saved_hwmode = crtc->hwmode;
>  	saved_mode = crtc->mode;
> -	saved_x = crtc->x;
> -	saved_y = crtc->y;
>  
>  	/* Update crtc values up front so the driver can rely on them for mode
>  	 * setting.
>  	 */
>  	crtc->mode = *mode;
> -	crtc->x = x;
> -	crtc->y = y;
>  
>  	/* Pass our mode to the connectors and the CRTC to give them a chance to
>  	 * adjust it according to limitations or connector properties, and also
> @@ -6725,7 +6722,7 @@ bool intel_set_mode(struct drm_crtc *crtc,
>  	/* Set up the DPLL and any encoders state that needs to adjust or depend
>  	 * on the DPLL.
>  	 */
> -	ret = !intel_crtc_mode_set(crtc, mode, adjusted_mode, x, y, old_fb);
> +	ret = !intel_crtc_mode_set(crtc, mode, adjusted_mode, x, y, fb);
>  	if (!ret)
>  	    goto done;
>  
> @@ -6741,6 +6738,9 @@ bool intel_set_mode(struct drm_crtc *crtc,
>  		encoder_funcs->mode_set(encoder, mode, adjusted_mode);
>  	}
>  
> +	crtc->x = x;
> +	crtc->y = y;
> +
>  	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
>  	dev_priv->display.crtc_enable(crtc);
>  
> @@ -6759,8 +6759,6 @@ done:
>  	if (!ret) {
>  		crtc->hwmode = saved_hwmode;
>  		crtc->mode = saved_mode;
> -		crtc->x = saved_x;
> -		crtc->y = saved_y;
>  	}
>  
>  	return ret;
> @@ -6993,7 +6991,6 @@ next_encoder:
>  static int intel_crtc_set_config(struct drm_mode_set *set)
>  {
>  	struct drm_device *dev;
> -	struct drm_framebuffer *old_fb = NULL;
>  	struct drm_mode_set save_set;
>  	struct intel_set_config *config;
>  	int ret;
> @@ -7056,13 +7053,10 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
>  			DRM_DEBUG_KMS("attempting to set mode from"
>  					" userspace\n");
>  			drm_mode_debug_printmodeline(set->mode);
> -			old_fb = set->crtc->fb;
> -			set->crtc->fb = set->fb;
>  			if (!intel_set_mode(set->crtc, set->mode,
> -					    set->x, set->y, old_fb)) {
> +					    set->x, set->y, set->fb)) {
>  				DRM_ERROR("failed to set mode on [CRTC:%d]\n",
>  					  set->crtc->base.id);
> -				set->crtc->fb = old_fb;
>  				ret = -EINVAL;
>  				goto fail;
>  			}
> @@ -7075,18 +7069,8 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
>  		}
>  		drm_helper_disable_unused_functions(dev);
>  	} else if (config->fb_changed) {
> -		set->crtc->x = set->x;
> -		set->crtc->y = set->y;
> -
> -		old_fb = set->crtc->fb;
> -		if (set->crtc->fb != set->fb)
> -			set->crtc->fb = set->fb;
>  		ret = intel_pipe_set_base(set->crtc,
> -					  set->x, set->y, old_fb);
> -		if (ret != 0) {
> -			set->crtc->fb = old_fb;
> -			goto fail;
> -		}
> +					  set->x, set->y, set->fb);
>  	}
>  
>  	intel_set_config_free(config);

I never liked how we passed the old ones around and polluted the
callees with knowledge of potential cleanup.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e6701b4..cd3606c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2201,16 +2201,17 @@  intel_finish_fb(struct drm_framebuffer *old_fb)
 
 static int
 intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
-		    struct drm_framebuffer *old_fb)
+		    struct drm_framebuffer *fb)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_master_private *master_priv;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct drm_framebuffer *old_fb;
 	int ret;
 
 	/* no fb bound */
-	if (!crtc->fb) {
+	if (!fb) {
 		DRM_ERROR("No FB bound\n");
 		return 0;
 	}
@@ -2224,7 +2225,7 @@  intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 
 	mutex_lock(&dev->struct_mutex);
 	ret = intel_pin_and_fence_fb_obj(dev,
-					 to_intel_framebuffer(crtc->fb)->obj,
+					 to_intel_framebuffer(fb)->obj,
 					 NULL);
 	if (ret != 0) {
 		mutex_unlock(&dev->struct_mutex);
@@ -2232,17 +2233,20 @@  intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 		return ret;
 	}
 
-	if (old_fb)
-		intel_finish_fb(old_fb);
+	if (crtc->fb)
+		intel_finish_fb(crtc->fb);
 
-	ret = dev_priv->display.update_plane(crtc, crtc->fb, x, y);
+	ret = dev_priv->display.update_plane(crtc, fb, x, y);
 	if (ret) {
-		intel_unpin_fb_obj(to_intel_framebuffer(crtc->fb)->obj);
+		intel_unpin_fb_obj(to_intel_framebuffer(fb)->obj);
 		mutex_unlock(&dev->struct_mutex);
 		DRM_ERROR("failed to update base address\n");
 		return ret;
 	}
 
+	old_fb = crtc->fb;
+	crtc->fb = fb;
+
 	if (old_fb) {
 		intel_wait_for_vblank(dev, intel_crtc->pipe);
 		intel_unpin_fb_obj(to_intel_framebuffer(old_fb)->obj);
@@ -3777,6 +3781,7 @@  static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
  * true if they don't match).
  */
 static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
+					 struct drm_framebuffer *fb,
 					 unsigned int *pipe_bpp,
 					 struct drm_display_mode *mode)
 {
@@ -3846,7 +3851,7 @@  static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
 	 * also stays within the max display bpc discovered above.
 	 */
 
-	switch (crtc->fb->depth) {
+	switch (fb->depth) {
 	case 8:
 		bpc = 8; /* since we go through a colormap */
 		break;
@@ -4265,7 +4270,7 @@  static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 			      struct drm_display_mode *mode,
 			      struct drm_display_mode *adjusted_mode,
 			      int x, int y,
-			      struct drm_framebuffer *old_fb)
+			      struct drm_framebuffer *fb)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -4455,7 +4460,7 @@  static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 	I915_WRITE(DSPCNTR(plane), dspcntr);
 	POSTING_READ(DSPCNTR(plane));
 
-	ret = intel_pipe_set_base(crtc, x, y, old_fb);
+	ret = intel_pipe_set_base(crtc, x, y, fb);
 
 	intel_update_watermarks(dev);
 
@@ -4613,7 +4618,7 @@  static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 				  struct drm_display_mode *mode,
 				  struct drm_display_mode *adjusted_mode,
 				  int x, int y,
-				  struct drm_framebuffer *old_fb)
+				  struct drm_framebuffer *fb)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -4733,7 +4738,7 @@  static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	/* determine panel color depth */
 	temp = I915_READ(PIPECONF(pipe));
 	temp &= ~PIPE_BPC_MASK;
-	dither = intel_choose_pipe_bpp_dither(crtc, &pipe_bpp, mode);
+	dither = intel_choose_pipe_bpp_dither(crtc, fb, &pipe_bpp, mode);
 	switch (pipe_bpp) {
 	case 18:
 		temp |= PIPE_6BPC;
@@ -5002,7 +5007,7 @@  static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	I915_WRITE(DSPCNTR(plane), dspcntr);
 	POSTING_READ(DSPCNTR(plane));
 
-	ret = intel_pipe_set_base(crtc, x, y, old_fb);
+	ret = intel_pipe_set_base(crtc, x, y, fb);
 
 	intel_update_watermarks(dev);
 
@@ -5015,7 +5020,7 @@  static int intel_crtc_mode_set(struct drm_crtc *crtc,
 			       struct drm_display_mode *mode,
 			       struct drm_display_mode *adjusted_mode,
 			       int x, int y,
-			       struct drm_framebuffer *old_fb)
+			       struct drm_framebuffer *fb)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -5026,7 +5031,7 @@  static int intel_crtc_mode_set(struct drm_crtc *crtc,
 	drm_vblank_pre_modeset(dev, pipe);
 
 	ret = dev_priv->display.crtc_mode_set(crtc, mode, adjusted_mode,
-					      x, y, old_fb);
+					      x, y, fb);
 	drm_vblank_post_modeset(dev, pipe);
 
 	return ret;
@@ -5718,7 +5723,7 @@  bool intel_get_load_detect_pipe(struct drm_connector *connector,
 	struct drm_encoder *encoder = &intel_encoder->base;
 	struct drm_crtc *crtc = NULL;
 	struct drm_device *dev = encoder->dev;
-	struct drm_framebuffer *old_fb;
+	struct drm_framebuffer *fb;
 	int i = -1;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
@@ -5779,8 +5784,6 @@  bool intel_get_load_detect_pipe(struct drm_connector *connector,
 	if (!mode)
 		mode = &load_detect_mode;
 
-	old_fb = crtc->fb;
-
 	/* We need a framebuffer large enough to accommodate all accesses
 	 * that the plane may generate whilst we perform load detection.
 	 * We can not rely on the fbcon either being present (we get called
@@ -5788,19 +5791,19 @@  bool intel_get_load_detect_pipe(struct drm_connector *connector,
 	 * not even exist) or that it is large enough to satisfy the
 	 * requested mode.
 	 */
-	crtc->fb = mode_fits_in_fbdev(dev, mode);
-	if (crtc->fb == NULL) {
+	fb = mode_fits_in_fbdev(dev, mode);
+	if (fb == NULL) {
 		DRM_DEBUG_KMS("creating tmp fb for load-detection\n");
-		crtc->fb = intel_framebuffer_create_for_mode(dev, mode, 24, 32);
-		old->release_fb = crtc->fb;
+		fb = intel_framebuffer_create_for_mode(dev, mode, 24, 32);
+		old->release_fb = fb;
 	} else
 		DRM_DEBUG_KMS("reusing fbdev for load-detection framebuffer\n");
-	if (IS_ERR(crtc->fb)) {
+	if (IS_ERR(fb)) {
 		DRM_DEBUG_KMS("failed to allocate framebuffer for load-detection\n");
 		goto fail;
 	}
 
-	if (!intel_set_mode(crtc, mode, 0, 0, old_fb)) {
+	if (!intel_set_mode(crtc, mode, 0, 0, fb)) {
 		DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
 		if (old->release_fb)
 			old->release_fb->funcs->destroy(old->release_fb);
@@ -5814,7 +5817,6 @@  bool intel_get_load_detect_pipe(struct drm_connector *connector,
 fail:
 	connector->encoder = NULL;
 	encoder->crtc = NULL;
-	crtc->fb = old_fb;
 	return false;
 }
 
@@ -6666,13 +6668,12 @@  static void intel_modeset_commit_output_state(struct drm_device *dev)
 
 bool intel_set_mode(struct drm_crtc *crtc,
 		    struct drm_display_mode *mode,
-		    int x, int y, struct drm_framebuffer *old_fb)
+		    int x, int y, struct drm_framebuffer *fb)
 {
 	struct drm_device *dev = crtc->dev;
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct drm_display_mode *adjusted_mode, saved_mode, saved_hwmode;
 	struct drm_encoder_helper_funcs *encoder_funcs;
-	int saved_x, saved_y;
 	struct drm_encoder *encoder;
 	bool ret = true;
 
@@ -6686,15 +6687,11 @@  bool intel_set_mode(struct drm_crtc *crtc,
 
 	saved_hwmode = crtc->hwmode;
 	saved_mode = crtc->mode;
-	saved_x = crtc->x;
-	saved_y = crtc->y;
 
 	/* Update crtc values up front so the driver can rely on them for mode
 	 * setting.
 	 */
 	crtc->mode = *mode;
-	crtc->x = x;
-	crtc->y = y;
 
 	/* Pass our mode to the connectors and the CRTC to give them a chance to
 	 * adjust it according to limitations or connector properties, and also
@@ -6725,7 +6722,7 @@  bool intel_set_mode(struct drm_crtc *crtc,
 	/* Set up the DPLL and any encoders state that needs to adjust or depend
 	 * on the DPLL.
 	 */
-	ret = !intel_crtc_mode_set(crtc, mode, adjusted_mode, x, y, old_fb);
+	ret = !intel_crtc_mode_set(crtc, mode, adjusted_mode, x, y, fb);
 	if (!ret)
 	    goto done;
 
@@ -6741,6 +6738,9 @@  bool intel_set_mode(struct drm_crtc *crtc,
 		encoder_funcs->mode_set(encoder, mode, adjusted_mode);
 	}
 
+	crtc->x = x;
+	crtc->y = y;
+
 	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
 	dev_priv->display.crtc_enable(crtc);
 
@@ -6759,8 +6759,6 @@  done:
 	if (!ret) {
 		crtc->hwmode = saved_hwmode;
 		crtc->mode = saved_mode;
-		crtc->x = saved_x;
-		crtc->y = saved_y;
 	}
 
 	return ret;
@@ -6993,7 +6991,6 @@  next_encoder:
 static int intel_crtc_set_config(struct drm_mode_set *set)
 {
 	struct drm_device *dev;
-	struct drm_framebuffer *old_fb = NULL;
 	struct drm_mode_set save_set;
 	struct intel_set_config *config;
 	int ret;
@@ -7056,13 +7053,10 @@  static int intel_crtc_set_config(struct drm_mode_set *set)
 			DRM_DEBUG_KMS("attempting to set mode from"
 					" userspace\n");
 			drm_mode_debug_printmodeline(set->mode);
-			old_fb = set->crtc->fb;
-			set->crtc->fb = set->fb;
 			if (!intel_set_mode(set->crtc, set->mode,
-					    set->x, set->y, old_fb)) {
+					    set->x, set->y, set->fb)) {
 				DRM_ERROR("failed to set mode on [CRTC:%d]\n",
 					  set->crtc->base.id);
-				set->crtc->fb = old_fb;
 				ret = -EINVAL;
 				goto fail;
 			}
@@ -7075,18 +7069,8 @@  static int intel_crtc_set_config(struct drm_mode_set *set)
 		}
 		drm_helper_disable_unused_functions(dev);
 	} else if (config->fb_changed) {
-		set->crtc->x = set->x;
-		set->crtc->y = set->y;
-
-		old_fb = set->crtc->fb;
-		if (set->crtc->fb != set->fb)
-			set->crtc->fb = set->fb;
 		ret = intel_pipe_set_base(set->crtc,
-					  set->x, set->y, old_fb);
-		if (ret != 0) {
-			set->crtc->fb = old_fb;
-			goto fail;
-		}
+					  set->x, set->y, set->fb);
 	}
 
 	intel_set_config_free(config);