Hi, On Sun, Aug 09, 2015 at 01:12:53PM +0100, Chris Wilson wrote: > We follow the VBT as to whether a DDI port is used for eDP and if so, do > not attach a HDMI encoder to it. However there are machines for which > the VBT eDP flag is a lie (shocking!) and we fail to detect a eDP link. > Furthermore, on those machines the HDMI is connected to that DDI port > but we ignore it. > > If we reorder our port initialisation to try the eDP setup first and > if that fails we can treat it as a normal DP port along with a HDMI > output. To accomplish this, we have to delay registering the DP > connector/encoder until after we establish its final type. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88331 The existing code seems very carefully crafted, taking into account the differences betweem various GPU generations etc, shuffling that around might risk breakage. FWIW, I'm wondering if just adding a quirk for this single Dell workstation might be justified? Best regards, Lukas > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Cc: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_display.c | 27 ++++---- > drivers/gpu/drm/i915/intel_dp.c | 127 +++++++++++++++++------------------ > drivers/gpu/drm/i915/intel_drv.h | 6 +- > 3 files changed, 79 insertions(+), 81 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 0bcd1b1aba4f..aab8dfd1f8a5 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13941,7 +13941,6 @@ static void intel_setup_outputs(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_encoder *encoder; > - bool dpd_is_edp = false; > > intel_lvds_init(dev); > > @@ -13983,31 +13982,33 @@ static void intel_setup_outputs(struct drm_device *dev) > intel_ddi_init(dev, PORT_D); > } else if (HAS_PCH_SPLIT(dev)) { > int found; > - dpd_is_edp = intel_dp_is_edp(dev, PORT_D); > > if (has_edp_a(dev)) > intel_dp_init(dev, DP_A, PORT_A); > > + found = 0; > + /* PCH SDVOB multiplex with HDMIB */ > if (I915_READ(PCH_HDMIB) & SDVO_DETECTED) { > - /* PCH SDVOB multiplex with HDMIB */ > found = intel_sdvo_init(dev, PCH_SDVOB, true); > if (!found) > intel_hdmi_init(dev, PCH_HDMIB, PORT_B); > - if (!found && (I915_READ(PCH_DP_B) & DP_DETECTED)) > - intel_dp_init(dev, PCH_DP_B, PORT_B); > } > + if (!found && I915_READ(PCH_DP_B) & DP_DETECTED) > + intel_dp_init(dev, PCH_DP_B, PORT_B); > > - if (I915_READ(PCH_HDMIC) & SDVO_DETECTED) > - intel_hdmi_init(dev, PCH_HDMIC, PORT_C); > - > - if (!dpd_is_edp && I915_READ(PCH_HDMID) & SDVO_DETECTED) > - intel_hdmi_init(dev, PCH_HDMID, PORT_D); > - > + found = 0; > if (I915_READ(PCH_DP_C) & DP_DETECTED) > - intel_dp_init(dev, PCH_DP_C, PORT_C); > + found = intel_dp_init(dev, PCH_DP_C, PORT_C); > + if (found != DRM_MODE_CONNECTOR_eDP && > + I915_READ(PCH_HDMIC) & SDVO_DETECTED) > + intel_hdmi_init(dev, PCH_HDMIC, PORT_C); > > + found = 0; > if (I915_READ(PCH_DP_D) & DP_DETECTED) > - intel_dp_init(dev, PCH_DP_D, PORT_D); > + found = intel_dp_init(dev, PCH_DP_D, PORT_D); > + if (found != DRM_MODE_CONNECTOR_eDP && > + I915_READ(PCH_HDMID) & SDVO_DETECTED) > + intel_hdmi_init(dev, PCH_HDMID, PORT_D); > } else if (IS_VALLEYVIEW(dev)) { > /* > * The DP_DETECTED bit is the latched state of the DDC > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 1e64a8c1e7cb..8adf500f3989 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1031,14 +1031,13 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > return ret; > } > > -static void > +static int > intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector) > { > struct drm_device *dev = intel_dp_to_dev(intel_dp); > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); > enum port port = intel_dig_port->port; > const char *name = NULL; > - int ret; > > switch (port) { > case PORT_A: > @@ -1080,20 +1079,7 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector) > DRM_DEBUG_KMS("registering %s bus for %s\n", name, > connector->base.kdev->kobj.name); > > - ret = drm_dp_aux_register(&intel_dp->aux); > - if (ret < 0) { > - DRM_ERROR("drm_dp_aux_register() for %s failed (%d)\n", > - name, ret); > - return; > - } > - > - ret = sysfs_create_link(&connector->base.kdev->kobj, > - &intel_dp->aux.ddc.dev.kobj, > - intel_dp->aux.ddc.dev.kobj.name); > - if (ret < 0) { > - DRM_ERROR("sysfs_create_link() for %s failed (%d)\n", name, ret); > - drm_dp_aux_unregister(&intel_dp->aux); > - } > + return drm_dp_aux_register(&intel_dp->aux); > } > > static void > @@ -5019,6 +5005,10 @@ bool intel_dp_is_edp(struct drm_device *dev, enum port port) > [PORT_D] = PORT_IDPD, > }; > > + /* eDP only on port B and/or C on vlv/chv */ > + if (IS_VALLEYVIEW(dev_priv) && !(port == PORT_B || port == PORT_C)) > + return false; > + > if (port == PORT_A) > return true; > > @@ -5670,9 +5660,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > struct edid *edid; > enum pipe pipe = INVALID_PIPE; > > - if (!is_edp(intel_dp)) > - return true; > - > pps_lock(intel_dp); > intel_edp_panel_vdd_sanitize(intel_dp); > pps_unlock(intel_dp); > @@ -5762,7 +5749,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > return true; > } > > -bool > +int > intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > struct intel_connector *intel_connector) > { > @@ -5772,6 +5759,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > struct drm_device *dev = intel_encoder->base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > enum port port = intel_dig_port->port; > + int ret; > int type; > > intel_dp->pps_pipe = INVALID_PIPE; > @@ -5802,34 +5790,14 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > else > type = DRM_MODE_CONNECTOR_DisplayPort; > > - /* > - * For eDP we always set the encoder type to INTEL_OUTPUT_EDP, but > - * for DP the encoder type can be set by the caller to > - * INTEL_OUTPUT_UNKNOWN for DDI, so don't rewrite it. > - */ > - if (type == DRM_MODE_CONNECTOR_eDP) > - intel_encoder->type = INTEL_OUTPUT_EDP; > - > - /* eDP only on port B and/or C on vlv/chv */ > - if (WARN_ON(IS_VALLEYVIEW(dev) && is_edp(intel_dp) && > - port != PORT_B && port != PORT_C)) > - return false; > - > DRM_DEBUG_KMS("Adding %s connector on port %c\n", > type == DRM_MODE_CONNECTOR_eDP ? "eDP" : "DP", > port_name(port)); > > - drm_connector_init(dev, connector, &intel_dp_connector_funcs, type); > - drm_connector_helper_add(connector, &intel_dp_connector_helper_funcs); > - > connector->interlace_allowed = true; > connector->doublescan_allowed = 0; > > - INIT_DELAYED_WORK(&intel_dp->panel_vdd_work, > - edp_panel_vdd_work); > - > - intel_connector_attach_encoder(intel_connector, intel_encoder); > - drm_connector_register(connector); > + INIT_DELAYED_WORK(&intel_dp->panel_vdd_work, edp_panel_vdd_work); > > if (HAS_DDI(dev)) > intel_connector->get_hw_state = intel_ddi_connector_get_hw_state; > @@ -5855,7 +5823,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > BUG(); > } > > - if (is_edp(intel_dp)) { > + if (type == DRM_MODE_CONNECTOR_eDP) { > pps_lock(intel_dp); > intel_dp_init_panel_power_timestamps(intel_dp); > if (IS_VALLEYVIEW(dev)) > @@ -5865,7 +5833,49 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > pps_unlock(intel_dp); > } > > - intel_dp_aux_init(intel_dp, intel_connector); > + ret = intel_dp_aux_init(intel_dp, intel_connector); > + if (ret < 0) { > + DRM_ERROR("registering DP-AUX channel for %s failed (%d)\n", > + intel_dp->aux.name, ret); > + return 0; > + } > + > + if (type == DRM_MODE_CONNECTOR_eDP && > + !intel_edp_init_connector(intel_dp, intel_connector)) { > + drm_dp_aux_unregister(&intel_dp->aux); > + cancel_delayed_work_sync(&intel_dp->panel_vdd_work); > + /* > + * vdd might still be enabled do to the delayed vdd off. > + * Make sure vdd is actually turned off here. > + */ > + pps_lock(intel_dp); > + edp_panel_vdd_off_sync(intel_dp); > + pps_unlock(intel_dp); > + > + /* Downgrade to a normal DP link */ > + type = DRM_MODE_CONNECTOR_DisplayPort; > + } > + > + /* > + * For eDP we always set the encoder type to INTEL_OUTPUT_EDP, but > + * for DP the encoder type can be set by the caller to > + * INTEL_OUTPUT_UNKNOWN for DDI, so don't rewrite it. > + */ > + if (type == DRM_MODE_CONNECTOR_eDP) > + intel_encoder->type = INTEL_OUTPUT_EDP; > + > + drm_connector_init(dev, connector, &intel_dp_connector_funcs, type); > + drm_connector_helper_add(connector, &intel_dp_connector_helper_funcs); > + > + intel_connector_attach_encoder(intel_connector, intel_encoder); > + drm_connector_register(connector); > + > + ret = sysfs_create_link(&connector->kdev->kobj, > + &intel_dp->aux.ddc.dev.kobj, > + intel_dp->aux.ddc.dev.kobj.name); > + if (ret < 0) > + DRM_ERROR("sysfs_create_link() for %s failed (%d)\n", > + intel_dp->aux.name, ret); > > /* init MST on ports that can support it */ > if (HAS_DP_MST(dev) && > @@ -5873,23 +5883,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > intel_dp_mst_encoder_init(intel_dig_port, > intel_connector->base.base.id); > > - if (!intel_edp_init_connector(intel_dp, intel_connector)) { > - drm_dp_aux_unregister(&intel_dp->aux); > - if (is_edp(intel_dp)) { > - cancel_delayed_work_sync(&intel_dp->panel_vdd_work); > - /* > - * vdd might still be enabled do to the delayed vdd off. > - * Make sure vdd is actually turned off here. > - */ > - pps_lock(intel_dp); > - edp_panel_vdd_off_sync(intel_dp); > - pps_unlock(intel_dp); > - } > - drm_connector_unregister(connector); > - drm_connector_cleanup(connector); > - return false; > - } > - > intel_dp_add_properties(intel_dp, connector); > > /* For G4X desktop chip, PEG_BAND_GAP_DATA 3:0 must first be written > @@ -5903,10 +5896,10 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > > i915_debugfs_connector_add(connector); > > - return true; > + return type; > } > > -void > +int > intel_dp_init(struct drm_device *dev, int output_reg, enum port port) > { > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -5914,15 +5907,16 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port) > struct intel_encoder *intel_encoder; > struct drm_encoder *encoder; > struct intel_connector *intel_connector; > + int ret; > > intel_dig_port = kzalloc(sizeof(*intel_dig_port), GFP_KERNEL); > if (!intel_dig_port) > - return; > + return 0; > > intel_connector = intel_connector_alloc(); > if (!intel_connector) { > kfree(intel_dig_port); > - return; > + return 0; > } > > intel_encoder = &intel_dig_port->base; > @@ -5970,11 +5964,14 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port) > intel_dig_port->hpd_pulse = intel_dp_hpd_pulse; > dev_priv->hotplug.irq_port[port] = intel_dig_port; > > - if (!intel_dp_init_connector(intel_dig_port, intel_connector)) { > + ret = intel_dp_init_connector(intel_dig_port, intel_connector); > + if (ret == 0) { > drm_encoder_cleanup(encoder); > kfree(intel_dig_port); > kfree(intel_connector); > } > + > + return ret; > } > > void intel_dp_mst_suspend(struct drm_device *dev) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 18fe6d8b7d93..ade1ce853583 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1179,9 +1179,9 @@ void intel_csr_ucode_fini(struct drm_device *dev); > void assert_csr_loaded(struct drm_i915_private *dev_priv); > > /* intel_dp.c */ > -void intel_dp_init(struct drm_device *dev, int output_reg, enum port port); > -bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > - struct intel_connector *intel_connector); > +int intel_dp_init(struct drm_device *dev, int output_reg, enum port port); > +int intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > + struct intel_connector *intel_connector); > void intel_dp_start_link_train(struct intel_dp *intel_dp); > void intel_dp_complete_link_train(struct intel_dp *intel_dp); > void intel_dp_stop_link_train(struct intel_dp *intel_dp); > -- > 2.5.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
