From: Daniel Vetter Date: Thu, 30 May 2013 13:04:25 +0000 (+0200) Subject: drm/i915: consolidate and tighten encoder cloning checks X-Git-Url: https://git.karo-electronics.de/?a=commitdiff_plain;h=accfc0c50659c330cfde684017e5c1b58eee2857;p=linux-beck.git drm/i915: consolidate and tighten encoder cloning checks 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 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 Cc: stable@vger.kernel.org Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter --- diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a4b97caf85b0..827d7ca2d9d9 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -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); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index afda71fdb4a6..eae3dbc4e3ee 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -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); diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 655486099b76..10c3d56332f5 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -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; diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c index 7d11a5adc985..39debd80d190 100644 --- a/drivers/gpu/drm/i915/intel_tv.c +++ b/drivers/gpu/drm/i915/intel_tv.c @@ -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;