Message ID | 1369919065-22519-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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@chris-wilson.co.uk> > Cc: stable@vger.kernel.org > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> 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 >
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;
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@chris-wilson.co.uk> Cc: stable@vger.kernel.org Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- 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(-)