drm/i915: Split plane updates of crtc->atomic into a helper, v2.
authorMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Mon, 15 Jun 2015 10:33:44 +0000 (12:33 +0200)
committerDaniel Vetter <daniel.vetter@ffwll.ch>
Mon, 22 Jun 2015 12:20:21 +0000 (14:20 +0200)
This makes it easier to verify that no changes are done when
calling this from crtc instead.

Changes since v1:
 - Make intel_wm_need_update static and always check it.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Tested-by(IVB): Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
drivers/gpu/drm/i915/intel_atomic_plane.c
drivers/gpu/drm/i915/intel_display.c
drivers/gpu/drm/i915/intel_drv.h
drivers/gpu/drm/i915/intel_sprite.c

index 86ba4b2..aa21283 100644 (file)
@@ -114,6 +114,7 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
        struct intel_crtc_state *crtc_state;
        struct intel_plane *intel_plane = to_intel_plane(plane);
        struct intel_plane_state *intel_state = to_intel_plane_state(state);
+       int ret;
 
        crtc = crtc ? crtc : plane->crtc;
        intel_crtc = to_intel_crtc(crtc);
@@ -160,20 +161,6 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
        intel_state->clip.y2 =
                crtc_state->base.active ? crtc_state->pipe_src_h : 0;
 
-       /*
-        * Disabling a plane is always okay; we just need to update
-        * fb tracking in a special way since cleanup_fb() won't
-        * get called by the plane helpers.
-        */
-       if (state->fb == NULL && plane->state->fb != NULL) {
-               /*
-                * 'prepare' is never called when plane is being disabled, so
-                * we need to handle frontbuffer tracking as a special case
-                */
-               intel_crtc->atomic.disabled_planes |=
-                       (1 << drm_plane_index(plane));
-       }
-
        if (state->fb && intel_rotation_90_or_270(state->rotation)) {
                if (!(state->fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
                        state->fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED)) {
@@ -198,7 +185,11 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
                }
        }
 
-       return intel_plane->check_plane(plane, intel_state);
+       ret = intel_plane->check_plane(plane, intel_state);
+       if (ret || !state->state)
+               return ret;
+
+       return intel_plane_atomic_calc_changes(&crtc_state->base, state);
 }
 
 static void intel_plane_atomic_update(struct drm_plane *plane,
index 29e1258..39f6b80 100644 (file)
@@ -4403,19 +4403,19 @@ int skl_update_scaler_crtc(struct intel_crtc_state *state, int force_detach)
  * skl_update_scaler_plane - Stages update to scaler state for a given plane.
  *
  * @state: crtc's scaler state
- * @intel_plane: affected plane
  * @plane_state: atomic plane state to update
  *
  * Return
  *     0 - scaler_usage updated successfully
  *    error - requested scaling cannot be supported or other error condition
  */
-int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
-                           struct intel_plane *intel_plane,
-                           struct intel_plane_state *plane_state)
+static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
+                                  struct intel_plane_state *plane_state)
 {
 
        struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
+       struct intel_plane *intel_plane =
+               to_intel_plane(plane_state->base.plane);
        struct drm_framebuffer *fb = plane_state->base.fb;
        int ret;
 
@@ -11677,6 +11677,161 @@ retry:
        return ret;
 }
 
+
+/**
+ * intel_wm_need_update - Check whether watermarks need updating
+ * @plane: drm plane
+ * @state: new plane state
+ *
+ * Check current plane state versus the new one to determine whether
+ * watermarks need to be recalculated.
+ *
+ * Returns true or false.
+ */
+static bool intel_wm_need_update(struct drm_plane *plane,
+                                struct drm_plane_state *state)
+{
+       /* Update watermarks on tiling changes. */
+       if (!plane->state->fb || !state->fb ||
+           plane->state->fb->modifier[0] != state->fb->modifier[0] ||
+           plane->state->rotation != state->rotation)
+               return true;
+
+       if (plane->state->crtc_w != state->crtc_w)
+               return true;
+
+       return false;
+}
+
+int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
+                                   struct drm_plane_state *plane_state)
+{
+       struct drm_crtc *crtc = crtc_state->crtc;
+       struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+       struct drm_plane *plane = plane_state->plane;
+       struct drm_device *dev = crtc->dev;
+       struct drm_i915_private *dev_priv = dev->dev_private;
+       struct intel_plane_state *old_plane_state =
+               to_intel_plane_state(plane->state);
+       int idx = intel_crtc->base.base.id, ret;
+       int i = drm_plane_index(plane);
+       bool mode_changed = needs_modeset(crtc_state);
+       bool was_crtc_enabled = crtc->state->active;
+       bool is_crtc_enabled = crtc_state->active;
+
+       bool turn_off, turn_on, visible, was_visible;
+       struct drm_framebuffer *fb = plane_state->fb;
+
+       if (crtc_state && INTEL_INFO(dev)->gen >= 9 &&
+           plane->type != DRM_PLANE_TYPE_CURSOR) {
+               ret = skl_update_scaler_plane(
+                       to_intel_crtc_state(crtc_state),
+                       to_intel_plane_state(plane_state));
+               if (ret)
+                       return ret;
+       }
+
+       /*
+        * Disabling a plane is always okay; we just need to update
+        * fb tracking in a special way since cleanup_fb() won't
+        * get called by the plane helpers.
+        */
+       if (old_plane_state->base.fb && !fb)
+               intel_crtc->atomic.disabled_planes |= 1 << i;
+
+       /* don't run rest during modeset yet */
+       if (!intel_crtc->active || mode_changed)
+               return 0;
+
+       was_visible = old_plane_state->visible;
+       visible = to_intel_plane_state(plane_state)->visible;
+
+       if (!was_crtc_enabled && WARN_ON(was_visible))
+               was_visible = false;
+
+       if (!is_crtc_enabled && WARN_ON(visible))
+               visible = false;
+
+       if (!was_visible && !visible)
+               return 0;
+
+       turn_off = was_visible && (!visible || mode_changed);
+       turn_on = visible && (!was_visible || mode_changed);
+
+       DRM_DEBUG_ATOMIC("[CRTC:%i] has [PLANE:%i] with fb %i\n", idx,
+                        plane->base.id, fb ? fb->base.id : -1);
+
+       DRM_DEBUG_ATOMIC("[PLANE:%i] visible %i -> %i, off %i, on %i, ms %i\n",
+                        plane->base.id, was_visible, visible,
+                        turn_off, turn_on, mode_changed);
+
+       if (intel_wm_need_update(plane, plane_state))
+               intel_crtc->atomic.update_wm = true;
+
+       switch (plane->type) {
+       case DRM_PLANE_TYPE_PRIMARY:
+               if (visible)
+                       intel_crtc->atomic.fb_bits |=
+                           INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
+
+               intel_crtc->atomic.wait_for_flips = true;
+               intel_crtc->atomic.pre_disable_primary = turn_off;
+               intel_crtc->atomic.post_enable_primary = turn_on;
+
+               if (turn_off)
+                       intel_crtc->atomic.disable_fbc = true;
+
+               /*
+                * FBC does not work on some platforms for rotated
+                * planes, so disable it when rotation is not 0 and
+                * update it when rotation is set back to 0.
+                *
+                * FIXME: This is redundant with the fbc update done in
+                * the primary plane enable function except that that
+                * one is done too late. We eventually need to unify
+                * this.
+                */
+
+               if (visible &&
+                   INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
+                   dev_priv->fbc.crtc == intel_crtc &&
+                   plane_state->rotation != BIT(DRM_ROTATE_0))
+                       intel_crtc->atomic.disable_fbc = true;
+
+               /*
+                * BDW signals flip done immediately if the plane
+                * is disabled, even if the plane enable is already
+                * armed to occur at the next vblank :(
+                */
+               if (turn_on && IS_BROADWELL(dev))
+                       intel_crtc->atomic.wait_vblank = true;
+
+               intel_crtc->atomic.update_fbc |= visible || mode_changed;
+               break;
+       case DRM_PLANE_TYPE_CURSOR:
+               if (visible)
+                       intel_crtc->atomic.fb_bits |=
+                           INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe);
+               break;
+       case DRM_PLANE_TYPE_OVERLAY:
+               /*
+                * 'prepare' is never called when plane is being disabled, so
+                * we need to handle frontbuffer tracking as a special case
+                */
+               if (visible)
+                       intel_crtc->atomic.fb_bits |=
+                           INTEL_FRONTBUFFER_SPRITE(intel_crtc->pipe);
+
+               if (turn_off && is_crtc_enabled) {
+                       intel_crtc->atomic.wait_vblank = true;
+                       intel_crtc->atomic.update_sprite_watermarks |=
+                               1 << i;
+               }
+               break;
+       }
+       return 0;
+}
+
 static bool encoders_cloneable(const struct intel_encoder *a,
                               const struct intel_encoder *b)
 {
@@ -13444,28 +13599,6 @@ static void intel_shared_dpll_init(struct drm_device *dev)
 }
 
 /**
- * intel_wm_need_update - Check whether watermarks need updating
- * @plane: drm plane
- * @state: new plane state
- *
- * Check current plane state versus the new one to determine whether
- * watermarks need to be recalculated.
- *
- * Returns true or false.
- */
-bool intel_wm_need_update(struct drm_plane *plane,
-                         struct drm_plane_state *state)
-{
-       /* Update watermarks on tiling changes. */
-       if (!plane->state->fb || !state->fb ||
-           plane->state->fb->modifier[0] != state->fb->modifier[0] ||
-           plane->state->rotation != state->rotation)
-               return true;
-
-       return false;
-}
-
-/**
  * intel_prepare_plane_fb - Prepare fb for usage on plane
  * @plane: drm plane to prepare for
  * @fb: framebuffer to prepare for presentation
@@ -13586,7 +13719,6 @@ intel_check_primary_plane(struct drm_plane *plane,
                          struct intel_plane_state *state)
 {
        struct drm_device *dev = plane->dev;
-       struct drm_i915_private *dev_priv = dev->dev_private;
        struct drm_crtc *crtc = state->base.crtc;
        struct intel_crtc *intel_crtc;
        struct intel_crtc_state *crtc_state;
@@ -13597,7 +13729,6 @@ intel_check_primary_plane(struct drm_plane *plane,
        bool can_position = false;
        int max_scale = DRM_PLANE_HELPER_NO_SCALING;
        int min_scale = DRM_PLANE_HELPER_NO_SCALING;
-       int ret;
 
        crtc = crtc ? crtc : plane->crtc;
        intel_crtc = to_intel_crtc(crtc);
@@ -13613,73 +13744,11 @@ intel_check_primary_plane(struct drm_plane *plane,
                can_position = true;
        }
 
-       ret = drm_plane_helper_check_update(plane, crtc, fb,
-                                           src, dest, clip,
-                                           min_scale,
-                                           max_scale,
-                                           can_position, true,
-                                           &state->visible);
-       if (ret)
-               return ret;
-
-       if (intel_crtc->active) {
-               struct intel_plane_state *old_state =
-                       to_intel_plane_state(plane->state);
-
-               intel_crtc->atomic.wait_for_flips = true;
-
-               /*
-                * FBC does not work on some platforms for rotated
-                * planes, so disable it when rotation is not 0 and
-                * update it when rotation is set back to 0.
-                *
-                * FIXME: This is redundant with the fbc update done in
-                * the primary plane enable function except that that
-                * one is done too late. We eventually need to unify
-                * this.
-                */
-               if (state->visible &&
-                   INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
-                   dev_priv->fbc.crtc == intel_crtc &&
-                   state->base.rotation != BIT(DRM_ROTATE_0)) {
-                       intel_crtc->atomic.disable_fbc = true;
-               }
-
-               if (state->visible && !old_state->visible) {
-                       /*
-                        * BDW signals flip done immediately if the plane
-                        * is disabled, even if the plane enable is already
-                        * armed to occur at the next vblank :(
-                        */
-                       if (IS_BROADWELL(dev))
-                               intel_crtc->atomic.wait_vblank = true;
-
-                       if (crtc_state && !needs_modeset(&crtc_state->base))
-                               intel_crtc->atomic.post_enable_primary = true;
-               }
-
-               if (!state->visible && old_state->visible &&
-                   crtc_state && !needs_modeset(&crtc_state->base))
-                       intel_crtc->atomic.pre_disable_primary = true;
-
-               intel_crtc->atomic.fb_bits |=
-                       INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
-
-               intel_crtc->atomic.update_fbc = true;
-
-               if (intel_wm_need_update(plane, &state->base))
-                       intel_crtc->atomic.update_wm = true;
-       }
-
-       if (INTEL_INFO(dev)->gen >= 9) {
-               ret = skl_update_scaler_plane(crtc_state,
-                                             to_intel_plane(plane),
-                                             state);
-               if (ret)
-                       return ret;
-       }
-
-       return 0;
+       return drm_plane_helper_check_update(plane, crtc, fb,
+                                            src, dest, clip,
+                                            min_scale, max_scale,
+                                            can_position, true,
+                                            &state->visible);
 }
 
 static void
@@ -13939,10 +14008,9 @@ intel_check_cursor_plane(struct drm_plane *plane,
        if (ret)
                return ret;
 
-
        /* if we want to turn off the cursor ignore width and height */
        if (!obj)
-               goto finish;
+               return 0;
 
        /* Check for which cursor types we support */
        if (!cursor_size_ok(dev, state->base.crtc_w, state->base.crtc_h)) {
@@ -13959,19 +14027,10 @@ intel_check_cursor_plane(struct drm_plane *plane,
 
        if (fb->modifier[0] != DRM_FORMAT_MOD_NONE) {
                DRM_DEBUG_KMS("cursor cannot be tiled\n");
-               ret = -EINVAL;
-       }
-
-finish:
-       if (intel_crtc->active) {
-               if (plane->state->crtc_w != state->base.crtc_w)
-                       intel_crtc->atomic.update_wm = true;
-
-               intel_crtc->atomic.fb_bits |=
-                       INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe);
+               return -EINVAL;
        }
 
-       return ret;
+       return 0;
 }
 
 static void
index 2436af9..198cb20 100644 (file)
@@ -1067,6 +1067,8 @@ int intel_plane_atomic_set_property(struct drm_plane *plane,
                                    struct drm_plane_state *state,
                                    struct drm_property *property,
                                    uint64_t val);
+int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
+                                   struct drm_plane_state *plane_state);
 
 unsigned int
 intel_tile_height(struct drm_device *dev, uint32_t pixel_format,
@@ -1081,9 +1083,6 @@ intel_rotation_90_or_270(unsigned int rotation)
 void intel_create_rotation_property(struct drm_device *dev,
                                        struct intel_plane *plane);
 
-bool intel_wm_need_update(struct drm_plane *plane,
-                         struct drm_plane_state *state);
-
 /* shared dpll functions */
 struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc);
 void assert_shared_dpll(struct drm_i915_private *dev_priv,
@@ -1148,9 +1147,6 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode,
 void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc);
 void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file);
 void skl_detach_scalers(struct intel_crtc *intel_crtc);
-int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
-                           struct intel_plane *intel_plane,
-                           struct intel_plane_state *plane_state);
 
 int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state, int force_detach);
 int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state);
index f57268b..e36bef8 100644 (file)
@@ -759,7 +759,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
        int max_scale, min_scale;
        bool can_scale;
        int pixel_size;
-       int ret;
 
        intel_crtc = intel_crtc ? intel_crtc : to_intel_crtc(plane->crtc);
        crtc_state = state->base.state ?
@@ -767,7 +766,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
 
        if (!fb) {
                state->visible = false;
-               goto finish;
+               return 0;
        }
 
        /* Don't modify another pipe's plane */
@@ -920,35 +919,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
        dst->y1 = crtc_y;
        dst->y2 = crtc_y + crtc_h;
 
-finish:
-       /*
-        * If the sprite is completely covering the primary plane,
-        * we can disable the primary and save power.
-        */
-       if (intel_crtc->active) {
-               intel_crtc->atomic.fb_bits |=
-                       INTEL_FRONTBUFFER_SPRITE(intel_crtc->pipe);
-
-               if (intel_wm_need_update(plane, &state->base))
-                       intel_crtc->atomic.update_wm = true;
-
-               if (!state->visible) {
-                       /*
-                        * Avoid underruns when disabling the sprite.
-                        * FIXME remove once watermark updates are done properly.
-                        */
-                       intel_crtc->atomic.wait_vblank = true;
-                       intel_crtc->atomic.update_sprite_watermarks |=
-                               (1 << drm_plane_index(plane));
-               }
-       }
-
-       if (INTEL_INFO(dev)->gen >= 9) {
-               ret = skl_update_scaler_plane(crtc_state, intel_plane, state);
-               if (ret)
-                       return ret;
-       }
-
        return 0;
 }