From: Ville Syrjälä <ville.syrj...@linux.intel.com>

Now that we have atomic modesetting, we can use it to solve the IVB
pipe B/C FDI lane sharing issue. That is, we can force a modeset on
pipe B if pipe C requires the FDI lanes currently used by pipe B.

To achieve this we do the following:

First stage:
- first compute the initial pipe config for each modeset crtc
- initial max FDI estimate will be 4 for pipe B, 2 for pipe C
- also need to run through the encoder .compute_config()
  hooks once to update has_pch_encoder
- and we need to compute an initial number of required FDI lanes.
  Note that we only need something that makes fdi_lanes > 0 speak
  the truth, so there's no need to check against the max here

Second stage:
- Check if pipe C requires any FDI lanes, and based on that
  update the max_fdi_lanes for both pipe B and pipe C
- Check if pipe B conflicts with the new requirements,
  and if so, add it to the state forcing a modeset (in case
  it already wasn't included)

Third stage:
- run through the rest of the pipe config computation, incuding
  recomputing the FDI lane requirement and checking it against the
  max
- this will signal that we should recompute things with a lower
  bpp by returning -EAGAIN
- we keep doing this in a loop as long -EAGAIN is returned
  If some other error occurs we bail out as usual

Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 353 ++++++++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_drv.h     |   1 +
 2 files changed, 222 insertions(+), 132 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 68fb449ded77..082323c2aa8c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6492,87 +6492,120 @@ static int pipe_required_fdi_lanes(struct 
intel_crtc_state *crtc_state)
        return 0;
 }
 
-static int ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
-                                    struct intel_crtc_state *pipe_config)
+static void
+intel_modeset_pipe_config_start(struct intel_crtc *crtc,
+                               struct intel_crtc_state *pipe_config);
+static int
+intel_modeset_pipe_config_continue(struct intel_crtc *crtc,
+                                  struct intel_crtc_state *pipe_config);
+
+static int intel_add_crtc_modeset_to_state(struct drm_atomic_state *state,
+                                          struct intel_crtc *crtc)
 {
-       struct drm_atomic_state *state = pipe_config->base.state;
-       struct intel_crtc *other_crtc;
-       struct intel_crtc_state *other_crtc_state;
+       struct intel_crtc_state *crtc_state;
+       int ret;
 
-       DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n",
-                     pipe_name(pipe), pipe_config->fdi_lanes);
-       if (pipe_config->fdi_lanes > 4) {
-               DRM_DEBUG_KMS("invalid fdi lane config on pipe %c: %i lanes\n",
-                             pipe_name(pipe), pipe_config->fdi_lanes);
-               return -EINVAL;
-       }
+       crtc_state = intel_atomic_get_crtc_state(state, crtc);
+       if (IS_ERR(crtc_state))
+               return PTR_ERR(crtc_state);
 
-       if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
-               if (pipe_config->fdi_lanes > 2) {
-                       DRM_DEBUG_KMS("only 2 lanes on haswell, required: %i 
lanes\n",
-                                     pipe_config->fdi_lanes);
-                       return -EINVAL;
-               } else {
-                       return 0;
-               }
-       }
+       crtc_state->base.mode_changed = true;
 
-       if (INTEL_INFO(dev)->num_pipes == 2)
-               return 0;
+       ret = drm_atomic_add_affected_connectors(state, &crtc->base);
+       if (ret)
+               return ret;
 
-       /* Ivybridge 3 pipe is really complicated */
-       switch (pipe) {
-       case PIPE_A:
-               return 0;
-       case PIPE_B:
-               if (pipe_config->fdi_lanes <= 2)
-                       return 0;
+       intel_modeset_pipe_config_start(crtc, crtc_state);
 
-               other_crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, 
PIPE_C));
-               other_crtc_state =
-                       intel_atomic_get_crtc_state(state, other_crtc);
-               if (IS_ERR(other_crtc_state))
-                       return PTR_ERR(other_crtc_state);
+       /*
+        * Must run through this at least once for every new
+        * pipe to make sure has_pch_encoder and fdi_lanes are
+        * set up to something half decent.
+        */
+       ret = intel_modeset_pipe_config_continue(crtc, crtc_state);
+       if (ret)
+               return ret;
 
-               if (pipe_required_fdi_lanes(other_crtc_state) > 0) {
-                       DRM_DEBUG_KMS("invalid shared fdi lane config on pipe 
%c: %i lanes\n",
-                                     pipe_name(pipe), pipe_config->fdi_lanes);
-                       return -EINVAL;
-               }
-               return 0;
-       case PIPE_C:
-               if (pipe_config->fdi_lanes > 2) {
-                       DRM_DEBUG_KMS("only 2 lanes on pipe %c: required %i 
lanes\n",
-                                     pipe_name(pipe), pipe_config->fdi_lanes);
-                       return -EINVAL;
-               }
-
-               other_crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, 
PIPE_B));
-               other_crtc_state =
-                       intel_atomic_get_crtc_state(state, other_crtc);
-               if (IS_ERR(other_crtc_state))
-                       return PTR_ERR(other_crtc_state);
-
-               if (pipe_required_fdi_lanes(other_crtc_state) > 2) {
-                       DRM_DEBUG_KMS("fdi link B uses too many lanes to enable 
link C\n");
-                       return -EINVAL;
-               }
-               return 0;
-       default:
-               BUG();
-       }
+       return 0;
 }
 
-#define RETRY 1
-static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
-                                      struct intel_crtc_state *pipe_config)
+static bool intel_crtc_needs_modeset(struct drm_atomic_state *state,
+                                    struct intel_crtc *crtc)
 {
-       struct drm_device *dev = intel_crtc->base.dev;
-       const struct drm_display_mode *adjusted_mode = 
&pipe_config->base.adjusted_mode;
-       int lane, link_bw, fdi_dotclock, ret;
-       bool needs_recompute = false;
+       struct drm_crtc_state *crtc_state =
+               drm_atomic_get_existing_crtc_state(state, &crtc->base);
+
+       return crtc_state && needs_modeset(crtc_state);
+}
+
+static int ivybridge_compute_fdi_bc_bifurcation(struct drm_atomic_state *state)
+{
+       struct drm_device *dev = state->dev;
+       struct intel_crtc *crtc_b, *crtc_c;
+       struct intel_crtc_state *crtc_state_b, *crtc_state_c;
+       int ret = 0;
+
+       crtc_b = to_intel_crtc(intel_get_crtc_for_pipe(dev, PIPE_B));
+       crtc_c = to_intel_crtc(intel_get_crtc_for_pipe(dev, PIPE_C));
+
+       /* Nothing to do if we're not modesetting pipe B or C */
+       if (!intel_crtc_needs_modeset(state, crtc_b) &&
+           !intel_crtc_needs_modeset(state, crtc_c))
+               return 0;
+
+       crtc_state_b = intel_atomic_get_crtc_state(state, crtc_b);
+       if (IS_ERR(crtc_state_b))
+               return PTR_ERR(crtc_state_b);
+
+       crtc_state_c = intel_atomic_get_crtc_state(state, crtc_c);
+       if (IS_ERR(crtc_state_c))
+               return PTR_ERR(crtc_state_c);
+
+       if (pipe_required_fdi_lanes(crtc_state_c) > 0) {
+               DRM_DEBUG_KMS("pipe C requires FDI lanes\n");
+
+               /* need to modeset pipe B to make room for pipe C */
+               if (pipe_required_fdi_lanes(crtc_state_b) > 2) {
+                       DRM_DEBUG_KMS("forcing a modeset in pipe B to free up 
FDI lanes\n");
+                       ret = intel_add_crtc_modeset_to_state(state, crtc_b);
+               }
+
+               /*
+                * must do this _after_ intel_add_crtc_modeset_to_state() since
+                * otherwise we end up with the defaults
+                */
+               crtc_state_b->max_fdi_lanes = 2;
+               crtc_state_c->max_fdi_lanes = 2;
+       } else {
+               DRM_DEBUG_KMS("pipe C doesn't require FDI lanes\n");
+
+               /* pipe C no longer needs FDI lanes, can give them to pipe B */
+               if (pipe_required_fdi_lanes(crtc_state_b) > 0 &&
+                   pipe_required_fdi_lanes(crtc_state_b) <= 2 &&
+                   crtc_state_b->bw_constrained) {
+                       DRM_DEBUG_KMS("forcing a modeset in pipe B to use up 
FDI lanes\n");
+                       ret = intel_add_crtc_modeset_to_state(state, crtc_b);
+               }
+
+               /*
+                * must do this _after_ intel_add_crtc_modeset_to_state() since
+                * otherwise we end up with the defaults
+                */
+               crtc_state_b->max_fdi_lanes = 4;
+               crtc_state_c->max_fdi_lanes = 0;
+       }
+
+       return ret;
+}
+
+static void ironlake_fdi_compute_config(struct intel_crtc *crtc,
+                                       struct intel_crtc_state *pipe_config)
+{
+       struct drm_device *dev = crtc->base.dev;
+       const struct drm_display_mode *adjusted_mode =
+               &pipe_config->base.adjusted_mode;
+       int link_bw, fdi_dotclock;
 
-retry:
        /* FDI is a binary signal running at ~2.7GHz, encoding
         * each output octet as 10 bits. The actual frequency
         * is stored as a divider into a 100MHz clock, and the
@@ -6584,30 +6617,36 @@ retry:
 
        fdi_dotclock = adjusted_mode->crtc_clock;
 
-       lane = ironlake_get_lanes_required(fdi_dotclock, link_bw,
-                                          pipe_config->pipe_bpp);
+       pipe_config->fdi_lanes =
+               ironlake_get_lanes_required(fdi_dotclock, link_bw,
+                                           pipe_config->pipe_bpp);
 
-       pipe_config->fdi_lanes = lane;
+       intel_link_compute_m_n(pipe_config->pipe_bpp, pipe_config->fdi_lanes,
+                              fdi_dotclock, link_bw, &pipe_config->fdi_m_n);
+}
 
-       intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
-                              link_bw, &pipe_config->fdi_m_n);
+static int ironlake_fdi_check_config(struct intel_crtc *crtc,
+                                    struct intel_crtc_state *pipe_config)
+{
+       DRM_DEBUG_KMS("fdi lane config on pipe %c: %i lanes (max: %i)\n",
+                     pipe_name(crtc->pipe), pipe_config->fdi_lanes,
+                     pipe_config->max_fdi_lanes);
 
-       ret = ironlake_check_fdi_lanes(intel_crtc->base.dev,
-                                      intel_crtc->pipe, pipe_config);
-       if (ret == -EINVAL && pipe_config->pipe_bpp > 6*3) {
+       if (pipe_config->fdi_lanes <= pipe_config->max_fdi_lanes)
+               return 0;
+
+       DRM_DEBUG_KMS("invalid fdi lane config on pipe %c: %i lanes (max: 
%i)\n",
+                     pipe_name(crtc->pipe), pipe_config->fdi_lanes,
+                     pipe_config->max_fdi_lanes);
+
+       if (pipe_config->pipe_bpp > 6*3) {
                pipe_config->pipe_bpp -= 2*3;
-               DRM_DEBUG_KMS("fdi link bw constraint, reducing pipe bpp to 
%i\n",
-                             pipe_config->pipe_bpp);
-               needs_recompute = true;
                pipe_config->bw_constrained = true;
 
-               goto retry;
+               return -EAGAIN;
        }
 
-       if (needs_recompute)
-               return RETRY;
-
-       return ret;
+       return -EINVAL;
 }
 
 static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv,
@@ -6701,7 +6740,7 @@ static int intel_crtc_compute_config(struct intel_crtc 
*crtc,
                hsw_compute_ips_config(crtc, pipe_config);
 
        if (pipe_config->has_pch_encoder)
-               return ironlake_fdi_compute_config(crtc, pipe_config);
+               return ironlake_fdi_check_config(crtc, pipe_config);
 
        return 0;
 }
@@ -11986,7 +12025,7 @@ connected_sink_compute_bpp(struct intel_connector 
*connector,
        }
 }
 
-static int
+static void
 compute_baseline_pipe_bpp(struct intel_crtc *crtc,
                          struct intel_crtc_state *pipe_config)
 {
@@ -12016,8 +12055,6 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc,
                connected_sink_compute_bpp(to_intel_connector(connector),
                                           pipe_config);
        }
-
-       return bpp;
 }
 
 static void intel_dump_crtc_timings(const struct drm_display_mode *mode)
@@ -12047,9 +12084,10 @@ static void intel_dump_pipe_config(struct intel_crtc 
*crtc,
        DRM_DEBUG_KMS("cpu_transcoder: %c\n", 
transcoder_name(pipe_config->cpu_transcoder));
        DRM_DEBUG_KMS("pipe bpp: %i, dithering: %i\n",
                      pipe_config->pipe_bpp, pipe_config->dither);
-       DRM_DEBUG_KMS("fdi/pch: %i, lanes: %i, gmch_m: %u, gmch_n: %u, link_m: 
%u, link_n: %u, tu: %u\n",
+       DRM_DEBUG_KMS("fdi/pch: %i, lanes: %i, max_lanes: %i, gmch_m: %u, 
gmch_n: %u, link_m: %u, link_n: %u, tu: %u\n",
                      pipe_config->has_pch_encoder,
                      pipe_config->fdi_lanes,
+                     pipe_config->max_fdi_lanes,
                      pipe_config->fdi_m_n.gmch_m, pipe_config->fdi_m_n.gmch_n,
                      pipe_config->fdi_m_n.link_m, pipe_config->fdi_m_n.link_n,
                      pipe_config->fdi_m_n.tu);
@@ -12245,22 +12283,14 @@ clear_intel_crtc_state(struct intel_crtc_state 
*crtc_state)
        crtc_state->pch_pfit.force_thru = force_thru;
 }
 
-static int
-intel_modeset_pipe_config(struct drm_crtc *crtc,
-                         struct intel_crtc_state *pipe_config)
+static void
+intel_modeset_pipe_config_start(struct intel_crtc *crtc,
+                               struct intel_crtc_state *pipe_config)
 {
-       struct drm_atomic_state *state = pipe_config->base.state;
-       struct intel_encoder *encoder;
-       struct drm_connector *connector;
-       struct drm_connector_state *connector_state;
-       int base_bpp, ret = -EINVAL;
-       int i;
-       bool retry = true;
-
        clear_intel_crtc_state(pipe_config);
 
        pipe_config->cpu_transcoder =
-               (enum transcoder) to_intel_crtc(crtc)->pipe;
+               (enum transcoder) crtc->pipe;
 
        /*
         * Sanitize sync polarity flags based on requested ones. If neither
@@ -12275,10 +12305,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
              (DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NVSYNC)))
                pipe_config->base.adjusted_mode.flags |= DRM_MODE_FLAG_NVSYNC;
 
-       base_bpp = compute_baseline_pipe_bpp(to_intel_crtc(crtc),
-                                            pipe_config);
-       if (base_bpp < 0)
-               goto fail;
+       compute_baseline_pipe_bpp(crtc, pipe_config);
 
        /*
         * Determine the real pipe dimensions. Note that stereo modes can
@@ -12292,7 +12319,23 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
                               &pipe_config->pipe_src_w,
                               &pipe_config->pipe_src_h);
 
-encoder_retry:
+       /* initial assumption is no shared lanes */
+       if (HAS_DDI(crtc->base.dev) || crtc->pipe == PIPE_C)
+               pipe_config->max_fdi_lanes = 2;
+       else
+               pipe_config->max_fdi_lanes = 4;
+}
+
+static int
+intel_modeset_pipe_config_continue(struct intel_crtc *crtc,
+                                  struct intel_crtc_state *pipe_config)
+{
+       struct drm_atomic_state *state = pipe_config->base.state;
+       struct intel_encoder *encoder;
+       struct drm_connector *connector;
+       struct drm_connector_state *connector_state;
+       int i;
+
        /* Ensure the port clock defaults are reset when retrying. */
        pipe_config->port_clock = 0;
        pipe_config->pixel_multiplier = 1;
@@ -12306,14 +12349,14 @@ encoder_retry:
         * a chance to reject the mode entirely.
         */
        for_each_connector_in_state(state, connector, connector_state, i) {
-               if (connector_state->crtc != crtc)
+               if (connector_state->crtc != &crtc->base)
                        continue;
 
                encoder = to_intel_encoder(connector_state->best_encoder);
 
                if (!(encoder->compute_config(encoder, pipe_config))) {
                        DRM_DEBUG_KMS("Encoder config failure\n");
-                       goto fail;
+                       return -EINVAL;
                }
        }
 
@@ -12323,31 +12366,37 @@ encoder_retry:
                pipe_config->port_clock = 
pipe_config->base.adjusted_mode.crtc_clock
                        * pipe_config->pixel_multiplier;
 
-       ret = intel_crtc_compute_config(to_intel_crtc(crtc), pipe_config);
-       if (ret < 0) {
-               DRM_DEBUG_KMS("CRTC fixup failed\n");
-               goto fail;
+       if (pipe_config->has_pch_encoder)
+               ironlake_fdi_compute_config(crtc, pipe_config);
+
+       return 0;
+}
+
+static int
+intel_modeset_pipe_config_finish(struct intel_crtc *crtc,
+                                struct intel_crtc_state *pipe_config)
+{
+       int ret;
+
+       ret = intel_crtc_compute_config(crtc, pipe_config);
+
+       if (ret == -EAGAIN) {
+               DRM_DEBUG_KMS("CRTC bw constrained, retrying\n");
+               return ret;
        }
 
-       if (ret == RETRY) {
-               if (WARN(!retry, "loop in pipe configuration computation\n")) {
-                       ret = -EINVAL;
-                       goto fail;
-               }
-
-               DRM_DEBUG_KMS("CRTC bw constrained, retrying\n");
-               retry = false;
-               goto encoder_retry;
+       if (ret) {
+               DRM_DEBUG_KMS("CRTC fixup failed\n");
+               return ret;
        }
 
        /* Dithering seems to not pass-through bits correctly when it should, so
         * only enable it on 6bpc panels. */
        pipe_config->dither = pipe_config->pipe_bpp == 6*3;
-       DRM_DEBUG_KMS("hw max bpp: %i, pipe bpp: %i, dithering: %i\n",
-                     base_bpp, pipe_config->pipe_bpp, pipe_config->dither);
+       DRM_DEBUG_KMS("pipe bpp: %i, dithering: %i\n",
+                     pipe_config->pipe_bpp, pipe_config->dither);
 
-fail:
-       return ret;
+       return 0;
 }
 
 static void
@@ -13194,15 +13243,13 @@ static int intel_atomic_check(struct drm_device *dev,
        struct drm_crtc_state *crtc_state;
        int ret, i;
        bool any_ms = false;
+       bool retry;
 
        ret = drm_atomic_helper_check_modeset(dev, state);
        if (ret)
                return ret;
 
        for_each_crtc_in_state(state, crtc, crtc_state, i) {
-               struct intel_crtc_state *pipe_config =
-                       to_intel_crtc_state(crtc_state);
-
                memset(&to_intel_crtc(crtc)->atomic, 0,
                       sizeof(struct intel_crtc_atomic_commit));
 
@@ -13222,13 +13269,55 @@ static int intel_atomic_check(struct drm_device *dev,
                /* FIXME: For only active_changed we shouldn't need to do any
                 * state recomputation at all. */
 
-               ret = drm_atomic_add_affected_connectors(state, crtc);
+               ret = intel_add_crtc_modeset_to_state(state,
+                                                     to_intel_crtc(crtc));
                if (ret)
                        return ret;
+       }
 
-               ret = intel_modeset_pipe_config(crtc, pipe_config);
+       if (IS_IVYBRIDGE(dev)) {
+               ret = ivybridge_compute_fdi_bc_bifurcation(state);
                if (ret)
                        return ret;
+       }
+
+       /*
+        * Keep at it until we run out of ways
+        * to reduce FDI bandwidth utilization.
+        */
+       do {
+               retry = false;
+
+               for_each_crtc_in_state(state, crtc, crtc_state, i) {
+                       struct intel_crtc_state *pipe_config =
+                               to_intel_crtc_state(crtc_state);
+
+                       if (!crtc_state->enable) {
+                               if (needs_modeset(crtc_state))
+                                       any_ms = true;
+                               continue;
+                       }
+
+                       if (!needs_modeset(crtc_state))
+                               continue;
+
+                       ret = 
intel_modeset_pipe_config_continue(to_intel_crtc(crtc),
+                                                                pipe_config);
+                       if (ret)
+                               return ret;
+
+                       ret = 
intel_modeset_pipe_config_finish(to_intel_crtc(crtc),
+                                                              pipe_config);
+                       if (ret == -EAGAIN)
+                               retry = true;
+                       else if (ret)
+                               return ret;
+               }
+       } while (retry);
+
+       for_each_crtc_in_state(state, crtc, crtc_state, i) {
+               struct intel_crtc_state *pipe_config =
+                       to_intel_crtc_state(crtc_state);
 
                if (i915.fastboot &&
                    intel_pipe_config_compare(state->dev,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ab5c147fa9e9..631a9be89fdf 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -471,6 +471,7 @@ struct intel_crtc_state {
        } pch_pfit;
 
        /* FDI configuration, only valid if has_pch_encoder is set. */
+       int max_fdi_lanes;
        int fdi_lanes;
        struct intel_link_m_n fdi_m_n;
 
-- 
2.4.10

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

Reply via email to