drm/i915: Check for invalid cloning earlier during modeset

Move the encoder cloning check to happen earlier in the modeset. The
main benefit will be that the debug output from a failed modeset will
be less confusing as output_types can not indicate an invalid
configuration during the later computation stages.

For instance, what happened to me was kms_setmode was attempting one
of its invalid cloning checks during which it asked for DP+VGA cloning
on HSW. In this case the DP .compute_config() was executed after
the FDI .compute_config() leaving the DP link clock (1.62 in this case)
in port_clock, and then later the FDI BW computation tried to use that
as the FDI link clock (which should always be 2.7). 1.62 x 2 wasn't
enough for the mode it was trying to use, and so it ended up rejecting
the modeset, not because of an invalid cloning configuration, but
because of supposedly running out of FDI bandwidth. Took me a while
to figure out what had actually happened.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1466621833-5054-12-git-send-email-ville.syrjala@linux.intel.com

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8704355e471a519ab6318e0180765edbbb7420ac..487ed2165322e59ad2a37752fcfe26f3f8327fac 100644 (file)
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11997,26 +11997,6 @@ static bool check_single_encoder_cloning(struct drm_atomic_state *state,
        return true;
 }
 
-static bool check_encoder_cloning(struct drm_atomic_state *state,
-                                 struct intel_crtc *crtc)
-{
-       struct intel_encoder *encoder;
-       struct drm_connector *connector;
-       struct drm_connector_state *connector_state;
-       int i;
-
-       for_each_connector_in_state(state, connector, connector_state, i) {
-               if (connector_state->crtc != &crtc->base)
-                       continue;
-
-               encoder = to_intel_encoder(connector_state->best_encoder);
-               if (!check_single_encoder_cloning(state, crtc, encoder))
-                       return false;
-       }
-
-       return true;
-}
-
 static int intel_crtc_atomic_check(struct drm_crtc *crtc,
                                   struct drm_crtc_state *crtc_state)
 {
@@ -12029,11 +12009,6 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
        int ret;
        bool mode_changed = needs_modeset(crtc_state);
 
-       if (mode_changed && !check_encoder_cloning(state, intel_crtc)) {
-               DRM_DEBUG_KMS("rejecting invalid cloning configuration\n");
-               return -EINVAL;
-       }
-
        if (mode_changed && !crtc_state->active)
                pipe_config->update_wm_post = true;
 
@@ -12472,6 +12447,11 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 
                encoder = to_intel_encoder(connector_state->best_encoder);
 
+               if (!check_single_encoder_cloning(state, to_intel_crtc(crtc), encoder)) {
+                       DRM_DEBUG_KMS("rejecting invalid cloning configuration\n");
+                       goto fail;
+               }
+
                /*
                 * Determine output_types before calling the .compute_config()
                 * hooks so that the hooks can use this information safely.