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.vet...@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 23c0d0f..a691c76 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2200,16 +2200,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;
        }
@@ -2223,7 +2224,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);
@@ -2231,17 +2232,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);
@@ -3776,6 +3780,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)
 {
@@ -3856,7 +3861,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;
@@ -4275,7 +4280,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;
@@ -4465,7 +4470,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);
 
@@ -4623,7 +4628,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;
@@ -4743,7 +4748,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;
@@ -5012,7 +5017,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);
 
@@ -5025,7 +5030,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;
@@ -5036,7 +5041,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;
@@ -5646,7 +5651,7 @@ bool intel_get_load_detect_pipe(struct intel_encoder 
*intel_encoder,
        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",
@@ -5707,8 +5712,6 @@ bool intel_get_load_detect_pipe(struct intel_encoder 
*intel_encoder,
        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
@@ -5716,19 +5719,19 @@ bool intel_get_load_detect_pipe(struct intel_encoder 
*intel_encoder,
         * 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);
@@ -5742,7 +5745,6 @@ bool intel_get_load_detect_pipe(struct intel_encoder 
*intel_encoder,
 fail:
        connector->encoder = NULL;
        encoder->crtc = NULL;
-       crtc->fb = old_fb;
        return false;
 }
 
@@ -6593,13 +6595,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;
 
@@ -6613,15 +6614,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
@@ -6652,7 +6649,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;
 
@@ -6668,6 +6665,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);
 
@@ -6686,8 +6686,6 @@ done:
        if (!ret) {
                crtc->hwmode = saved_hwmode;
                crtc->mode = saved_mode;
-               crtc->x = saved_x;
-               crtc->y = saved_y;
        }
 
        return ret;
@@ -6916,7 +6914,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;
@@ -6979,13 +6976,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;
                        }
@@ -6998,18 +6992,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);
-- 
1.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to