On Fri, Nov 16, 2012 at 12:04:35PM -0200, Paulo Zanoni wrote: > Hi > > 2012/11/13 Daniel Vetter <[email protected]>: > > After the recent pile of disable-cloning patches, e.g. > > > > commit e3b86d6941c7e5f90be05d986fce1fcb40c68d6b > > Author: Egbert Eich <[email protected]> > > Date: Sat Oct 13 14:30:15 2012 +0200 > > > > DRM/i915: Don't clone SDVO LVDS with analog > > > > and a bug report from Chris Wilson indicating that cloning doesn't > > even work for DVI-SDVO and native VGA, let's just disable cloning on > > sdvo encoders completely. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=29259 > > Tested-by: Chris Wilson <[email protected]> > > Signed-off-by: Daniel Vetter <[email protected]> > > --- > > drivers/gpu/drm/i915/intel_sdvo.c | 12 +++++------- > > 1 file changed, 5 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c > > b/drivers/gpu/drm/i915/intel_sdvo.c > > index d6a5fb9..909d34f 100644 > > --- a/drivers/gpu/drm/i915/intel_sdvo.c > > +++ b/drivers/gpu/drm/i915/intel_sdvo.c > > @@ -2208,7 +2208,6 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, > > int device) > > connector->connector_type = DRM_MODE_CONNECTOR_HDMIA; > > intel_sdvo->is_hdmi = true; > > } > > - intel_sdvo->base.cloneable = true; > > > > intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo); > > if (intel_sdvo->is_hdmi) > > @@ -2239,7 +2238,6 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int > > type) > > > > intel_sdvo->is_tv = true; > > intel_sdvo->base.needs_tv_clock = true; > > - intel_sdvo->base.cloneable = false; > > > > intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo); > > > > @@ -2282,8 +2280,6 @@ intel_sdvo_analog_init(struct intel_sdvo *intel_sdvo, > > int device) > > intel_sdvo_connector->output_flag = SDVO_OUTPUT_RGB1; > > } > > > > - intel_sdvo->base.cloneable = true; > > - > > intel_sdvo_connector_init(intel_sdvo_connector, > > intel_sdvo); > > return true; > > @@ -2314,9 +2310,6 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, > > int device) > > intel_sdvo_connector->output_flag = SDVO_OUTPUT_LVDS1; > > } > > > > - /* SDVO LVDS is not cloneable because the input mode gets adjusted > > by the encoder */ > > - intel_sdvo->base.cloneable = false; > > - > > intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo); > > if (!intel_sdvo_create_enhance_property(intel_sdvo, > > intel_sdvo_connector)) > > goto err; > > @@ -2726,6 +2719,11 @@ bool intel_sdvo_init(struct drm_device *dev, > > uint32_t sdvo_reg, bool is_sdvob) > > goto err_output; > > } > > > > + /* SDVO because we need different clocks for SDVO encoders compared > > to > > + * VGA. And the sdvo encoder is also allowed to adjust the mode. So > > just > > + * give up and disable it. */ > > The code looks good, but the comment above is quite confusing. "SDVO because"?
Oops. What about: "Cloning SDVO with anything is often impossible, since the SDVO encoder can request a special input timing mode. And even if that's not the case we have evidence that cloning a plain unscaled mode with VGA doesn't really work. Furthermore the cloning flags are way to simplistic anyway to express such constraints, so just give up on cloning for SDVO encoders." Is that good enough for a full r-b? Yours, Daniel > > > + intel_sdvo->base.cloneable = false; > > + > > /* Only enable the hotplug irq if we need it, to work around noisy > > * hotplug lines. > > */ > > -- > > 1.7.10.4 > > > > _______________________________________________ > > Intel-gfx mailing list > > [email protected] > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Paulo Zanoni -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
