2014-11-03 8:29 GMT-02:00 Daniel Vetter <daniel@xxxxxxxx>:
> 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.
Why?
> Can't we add a new
> OUTPUT_DDI type, which together with the ddi personality stuff dtrt?
Or we can rename INTEL_OUTPUT_UNKNOWN to INTEL_OUTPUT_DDI_DISCONNECTED
so it smells less hacky to you?
>
> I'm really don't think that sprinkling UNKOWN here is a good idea ...
Why?
>
>> With that: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>
> r-b even without that, just to get things going?
No.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
