On Thu, May 30, 2013 at 03:04:25PM +0200, Daniel Vetter wrote:
> 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.
>
> I haven't figured out yet why exactly we see this configuration
> request from the setcrtc ioctl, but I can reliably reproduce it by
> unplugging a bunch of connectors and restarting X.
>
> Strangely restarting X again fixes things ...
>
> v2: Kill intel_encoder_check_is_cloned, spotted by Paulo.
>
> v3: Drop the now unused pipe param.
>
> v4: Kill the stray printk Chris spotted.
>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
Patch merged to dinq (imo a bit late for -fixes) with Chris' r-b.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 64 ++++++++++++++----------------------
> drivers/gpu/drm/i915/intel_drv.h | 1 -
> drivers/gpu/drm/i915/intel_lvds.c | 3 --
> drivers/gpu/drm/i915/intel_tv.c | 3 --
> 4 files changed, 24 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 14bb844..ba33a82 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5843,27 +5843,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;
>
> @@ -7523,28 +7505,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)
> {
> @@ -7713,6 +7673,25 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
> pipe_config->pch_pfit.size);
> }
>
> +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,
> @@ -7725,6 +7704,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 3db4b14..a844a05 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -627,7 +627,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 6554860..10c3d56 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 7d11a5a..39debd8 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;
> --
> 1.7.11.7
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html