drm/i915: consolidate and tighten encoder cloning checks
authorDaniel Vetter <daniel.vetter@ffwll.ch>
Thu, 30 May 2013 13:04:25 +0000 (15:04 +0200)
committerDaniel Vetter <daniel.vetter@ffwll.ch>
Wed, 5 Jun 2013 10:33:14 +0000 (12:33 +0200)
Only lvds/tv did actually check for cloning or not, but many more
places should.

Notices because my ivb tried to enable both cpu edp and vga on the
first crtc - the resulting confusion between has_pch_encoder,
has_dp_encoder but not actually being a pch dp encoder resulting in
hilarity (hitting a BUG).

We _really_ need an igt to random-walk our modeset space more
exhaustively.

The bug seems to have been exposed due to a race in the hw load
detection support for VGA: Right after a hotplug VGA was still
detected as connected, but obviously reading the EDID wasn't possible
any more. Hence why restarting X a bit later fixed things. Due to the
1024x756 fallback resolution suddenly more outputs had the same
resolution.

On top of that SNA was confused with the possible_clones mask, trying
to clone outputs which cannot be cloned. That bug is now fixed with

commit fc1e0702b25e647cb423851fb7228989fec28bd6
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Wed May 29 11:25:28 2013 +0100

    sna: fixup up possible_clones kms->X impedance mismatch

v2: Kill intel_encoder_check_is_cloned, spotted by Paulo.

v3: Drop the now unused pipe param.

v4: Kill the stray printk Chris spotted.

v5: Elaborate on how the bug in userspace happened and why it was racy
to reproduce.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@vger.kernel.org
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
drivers/gpu/drm/i915/intel_display.c
drivers/gpu/drm/i915/intel_drv.h
drivers/gpu/drm/i915/intel_lvds.c
drivers/gpu/drm/i915/intel_tv.c

index a4b97caf85b0dfd5652484a837cdcfb6f810183d..827d7ca2d9d99b941a5e04f9d424c00088286a39 100644 (file)
@@ -5868,27 +5868,9 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
        struct drm_device *dev = crtc->dev;
        struct drm_i915_private *dev_priv = dev->dev_private;
        struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-       int pipe = intel_crtc->pipe;
        int plane = intel_crtc->plane;
-       int num_connectors = 0;
-       bool is_cpu_edp = false;
-       struct intel_encoder *encoder;
        int ret;
 
-       for_each_encoder_on_crtc(dev, crtc, encoder) {
-               switch (encoder->type) {
-               case INTEL_OUTPUT_EDP:
-                       if (enc_to_dig_port(&encoder->base)->port == PORT_A)
-                               is_cpu_edp = true;
-                       break;
-               }
-
-               num_connectors++;
-       }
-
-       WARN(num_connectors != 1, "%d connectors attached to pipe %c\n",
-            num_connectors, pipe_name(pipe));
-
        if (!intel_ddi_pll_mode_set(crtc))
                return -EINVAL;
 
@@ -7564,28 +7546,6 @@ static struct drm_crtc_helper_funcs intel_helper_funcs = {
        .load_lut = intel_crtc_load_lut,
 };
 
-bool intel_encoder_check_is_cloned(struct intel_encoder *encoder)
-{
-       struct intel_encoder *other_encoder;
-       struct drm_crtc *crtc = &encoder->new_crtc->base;
-
-       if (WARN_ON(!crtc))
-               return false;
-
-       list_for_each_entry(other_encoder,
-                           &crtc->dev->mode_config.encoder_list,
-                           base.head) {
-
-               if (&other_encoder->new_crtc->base != crtc ||
-                   encoder == other_encoder)
-                       continue;
-               else
-                       return true;
-       }
-
-       return false;
-}
-
 static bool intel_encoder_crtc_ok(struct drm_encoder *encoder,
                                  struct drm_crtc *crtc)
 {
@@ -7769,6 +7729,25 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
        DRM_DEBUG_KMS("ips: %i\n", pipe_config->ips_enabled);
 }
 
+static bool check_encoder_cloning(struct drm_crtc *crtc)
+{
+       int num_encoders = 0;
+       bool uncloneable_encoders = false;
+       struct intel_encoder *encoder;
+
+       list_for_each_entry(encoder, &crtc->dev->mode_config.encoder_list,
+                           base.head) {
+               if (&encoder->new_crtc->base != crtc)
+                       continue;
+
+               num_encoders++;
+               if (!encoder->cloneable)
+                       uncloneable_encoders = true;
+       }
+
+       return !(num_encoders > 1 && uncloneable_encoders);
+}
+
 static struct intel_crtc_config *
 intel_modeset_pipe_config(struct drm_crtc *crtc,
                          struct drm_framebuffer *fb,
@@ -7781,6 +7760,11 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
        int plane_bpp, ret = -EINVAL;
        bool retry = true;
 
+       if (!check_encoder_cloning(crtc)) {
+               DRM_DEBUG_KMS("rejecting invalid cloning configuration\n");
+               return ERR_PTR(-EINVAL);
+       }
+
        pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
        if (!pipe_config)
                return ERR_PTR(-ENOMEM);
index afda71fdb4a63429dbf165ed795d24035e2616ba..eae3dbc4e3eeb5c4be3acf4acf2ee661834ad606 100644 (file)
@@ -629,7 +629,6 @@ extern void intel_crtc_load_lut(struct drm_crtc *crtc);
 extern void intel_crtc_update_dpms(struct drm_crtc *crtc);
 extern void intel_encoder_destroy(struct drm_encoder *encoder);
 extern void intel_encoder_dpms(struct intel_encoder *encoder, int mode);
-extern bool intel_encoder_check_is_cloned(struct intel_encoder *encoder);
 extern void intel_connector_dpms(struct drm_connector *, int mode);
 extern bool intel_connector_get_hw_state(struct intel_connector *connector);
 extern void intel_modeset_check_state(struct drm_device *dev);
index 655486099b76483bd2300744af1918d410f743bb..10c3d56332f56f638459db3e0e7c8a78e86aa509 100644 (file)
@@ -264,9 +264,6 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
                return false;
        }
 
-       if (intel_encoder_check_is_cloned(&lvds_encoder->base))
-               return false;
-
        if ((I915_READ(lvds_encoder->reg) & LVDS_A3_POWER_MASK) ==
            LVDS_A3_POWER_UP)
                lvds_bpp = 8*3;
index 7d11a5adc985109b57ce1f1cb943463f09e0c8c1..39debd80d190241836e89b8c6794bdb713e80653 100644 (file)
@@ -914,9 +914,6 @@ intel_tv_compute_config(struct intel_encoder *encoder,
        if (!tv_mode)
                return false;
 
-       if (intel_encoder_check_is_cloned(&intel_tv->base))
-               return false;
-
        pipe_config->adjusted_mode.clock = tv_mode->clock;
        DRM_DEBUG_KMS("forcing bpc to 8 for TV\n");
        pipe_config->pipe_bpp = 8*3;