On Tue, Oct 28, 2014 at 11:20:37AM -0200, Paulo Zanoni wrote:
> 2014-05-19 11:19 GMT-03:00 <ville.syrjala@xxxxxxxxxxxxxxx>:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > On pre-HSW we have two encoders per digital port: one HDMI, one DP.
> > However they are the same physical port in hardware and we can't enable
> > both at the same time. Reject the modeset if the user attempts this.
> >
> > So far we've been saved by the fact that we never see both HDMI and DP
> > connectors as connected. But if the user decides to force a mode anyway,
> > all kinds of funny stuff might happen.
> >
> > Unfortunately we don't seem to have any way to inform userspace that
> > such configurations are invalid except by returning an error from
> > setcrtc. possible_clones only covers real cloning situations, and
> > looking at the connector names doesn't work either since we don't
> > always register both connectors for the same port. I suppose the
> > only way to fix that would be to expose only a single encoder per
> > digital port like we do on HSW+ but that would be a fairly large
> > undertaking for little gain.
> >
> > kms_setmode hits this since it forces modes on non-connected VGA and
> > HDMI connectors. Previosuly it just resulted in weirdness such as
> > failed link training. With this patch it will now get an error back
> > from the kernel and will die with an assert since it thinks that the
> > configuration should be fine.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 44 ++++++++++++++++++++++++++++++++++++
> > 1 file changed, 44 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index a432348..13add38 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9621,6 +9621,45 @@ static bool check_encoder_cloning(struct intel_crtc *crtc)
> > return true;
> > }
> >
> > +static bool check_digital_port_conflicts(struct drm_device *dev)
> > +{
> > + struct intel_connector *connector;
> > + unsigned int used_ports = 0;
> > +
> > + /*
> > + * Walk the connector list instead of the encoder
> > + * list to detect the problem on ddi platforms
> > + * where there's just one encoder per digital port.
> > + */
> > + list_for_each_entry(connector,
> > + &dev->mode_config.connector_list, base.head) {
> > + struct intel_encoder *encoder = connector->new_encoder;
> > +
> > + if (!encoder)
> > + continue;
> > +
> > + WARN_ON(!encoder->new_crtc);
> > +
> > + switch (encoder->type) {
> > + unsigned int port_mask;
> > + case INTEL_OUTPUT_DISPLAYPORT:
> > + case INTEL_OUTPUT_HDMI:
> > + case INTEL_OUTPUT_EDP:
>
> If we're also interested on DDI platforms, we need to check for
> INTEL_OUTPUT_UNKNOWN here too. I guess Daniel could add this while
> applying the patch...
Adding UNKNOWN here smells like a massive hack. Can't we add a new
OUTPUT_DDI type, which together with the ddi personality stuff dtrt?
I'm really don't think that sprinkling UNKOWN here is a good idea ...
> With that: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
r-b even without that, just to get things going?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
