[Intel-gfx] [PATCH] drm/i915: Try to initialize eDP for both ports B and C on VLV
Ville Syrjälä
ville.syrjala at linux.intel.com
Thu Oct 31 20:05:05 CET 2013
On Thu, Oct 31, 2013 at 10:18:00AM -0700, Jesse Barnes wrote:
> On Thu, 31 Oct 2013 18:46:22 +0200
> ville.syrjala at linux.intel.com wrote:
>
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >
> > On VLV both ports B and C can support either DP or eDP. Try to
> > initialize eDP first on each port, and if that fails fall back to
> > regular DP connector.
> >
> > intel_dp_init_typed() is now like the old intel_dp_init, except you pass
> > in the connector type. If you pass DRM_MODE_CONNECTOR_Unknown, the code
> > falls back to the old port/VBT based auto detection to determine the
> > actual type.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=71051
> > Tested-by: Robert Hooker <robert.hooker at canonical.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_ddi.c | 3 +-
> > drivers/gpu/drm/i915/intel_display.c | 11 ++++--
> > drivers/gpu/drm/i915/intel_dp.c | 66 ++++++++++++++++++++++--------------
> > drivers/gpu/drm/i915/intel_drv.h | 5 ++-
> > 4 files changed, 54 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 31f4fe2..dd2660c 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1356,7 +1356,8 @@ intel_ddi_init_dp_connector(struct intel_digital_port *intel_dig_port)
> > return NULL;
> >
> > intel_dig_port->dp.output_reg = DDI_BUF_CTL(port);
> > - if (!intel_dp_init_connector(intel_dig_port, connector)) {
> > + if (!intel_dp_init_connector(intel_dig_port, connector,
> > + DRM_MODE_CONNECTOR_Unknown)) {
> > kfree(connector);
> > return NULL;
> > }
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 8f40ae3..16225eb 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9984,15 +9984,20 @@ static void intel_setup_outputs(struct drm_device *dev)
> > intel_hdmi_init(dev, VLV_DISPLAY_BASE + GEN4_HDMIB,
> > PORT_B);
> > if (I915_READ(VLV_DISPLAY_BASE + DP_B) & DP_DETECTED)
> > - intel_dp_init(dev, VLV_DISPLAY_BASE + DP_B, PORT_B);
> > + if (!intel_dp_init_typed(dev, VLV_DISPLAY_BASE + DP_B,
> > + PORT_B, DRM_MODE_CONNECTOR_eDP))
> > + intel_dp_init_typed(dev, VLV_DISPLAY_BASE + DP_B,
> > + PORT_B, DRM_MODE_CONNECTOR_DisplayPort);
> > }
> >
> > if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIC) & SDVO_DETECTED) {
> > intel_hdmi_init(dev, VLV_DISPLAY_BASE + GEN4_HDMIC,
> > PORT_C);
> > if (I915_READ(VLV_DISPLAY_BASE + DP_C) & DP_DETECTED)
> > - intel_dp_init(dev, VLV_DISPLAY_BASE + DP_C,
> > - PORT_C);
> > + if (!intel_dp_init_typed(dev, VLV_DISPLAY_BASE + DP_C,
> > + PORT_C, DRM_MODE_CONNECTOR_eDP))
> > + intel_dp_init_typed(dev, VLV_DISPLAY_BASE + DP_C,
> > + PORT_C, DRM_MODE_CONNECTOR_DisplayPort);
> > }
> >
> > intel_dsi_init(dev);
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index b3cc333..a97f186 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3540,7 +3540,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> >
> > bool
> > intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> > - struct intel_connector *intel_connector)
> > + struct intel_connector *intel_connector,
> > + int type)
> > {
> > struct drm_connector *connector = &intel_connector->base;
> > struct intel_dp *intel_dp = &intel_dig_port->dp;
> > @@ -3549,33 +3550,32 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > enum port port = intel_dig_port->port;
> > const char *name = NULL;
> > - int type, error;
> > + int error;
> >
> > /* Preserve the current hw state. */
> > intel_dp->DP = I915_READ(intel_dp->output_reg);
> > intel_dp->attached_connector = intel_connector;
> >
> > - type = DRM_MODE_CONNECTOR_DisplayPort;
> > - /*
> > - * FIXME : We need to initialize built-in panels before external panels.
> > - * For X0, DP_C is fixed as eDP. Revisit this as part of VLV eDP cleanup
> > - */
> > - switch (port) {
> > - case PORT_A:
> > - type = DRM_MODE_CONNECTOR_eDP;
> > - break;
> > - case PORT_C:
> > - if (IS_VALLEYVIEW(dev))
> > - type = DRM_MODE_CONNECTOR_eDP;
> > - break;
> > - case PORT_D:
> > - if (HAS_PCH_SPLIT(dev) && intel_dpd_is_edp(dev))
> > + if (type == DRM_MODE_CONNECTOR_Unknown) {
> > + type = DRM_MODE_CONNECTOR_DisplayPort;
> > +
> > + switch (port) {
> > + case PORT_A:
> > type = DRM_MODE_CONNECTOR_eDP;
> > - break;
> > - default: /* silence GCC warning */
> > - break;
> > + break;
> > + case PORT_D:
> > + if (HAS_PCH_SPLIT(dev) && intel_dpd_is_edp(dev))
> > + type = DRM_MODE_CONNECTOR_eDP;
> > + break;
> > + default: /* silence GCC warning */
> > + break;
> > + }
> > }
> >
> > + if (WARN_ON(type != DRM_MODE_CONNECTOR_DisplayPort &&
> > + type != DRM_MODE_CONNECTOR_eDP))
> > + return false;
> > +
> > /*
> > * 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
> > @@ -3598,7 +3598,11 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> > ironlake_panel_vdd_work);
> >
> > intel_connector_attach_encoder(intel_connector, intel_encoder);
> > - drm_sysfs_connector_add(connector);
> > + error = drm_sysfs_connector_add(connector);
> > + if (error) {
> > + drm_connector_cleanup(connector);
> > + return false;
> > + }
> >
> > if (HAS_DDI(dev))
> > intel_connector->get_hw_state = intel_ddi_connector_get_hw_state;
> > @@ -3680,8 +3684,9 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> > return true;
> > }
> >
> > -void
> > -intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
> > +bool
> > +intel_dp_init_typed(struct drm_device *dev, int output_reg,
> > + enum port port, int type)
> > {
> > struct intel_digital_port *intel_dig_port;
> > struct intel_encoder *intel_encoder;
> > @@ -3690,12 +3695,12 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
> >
> > intel_dig_port = kzalloc(sizeof(*intel_dig_port), GFP_KERNEL);
> > if (!intel_dig_port)
> > - return;
> > + return false;
> >
> > intel_connector = kzalloc(sizeof(*intel_connector), GFP_KERNEL);
> > if (!intel_connector) {
> > kfree(intel_dig_port);
> > - return;
> > + return false;
> > }
> >
> > intel_encoder = &intel_dig_port->base;
> > @@ -3727,9 +3732,18 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
> > intel_encoder->cloneable = false;
> > intel_encoder->hot_plug = intel_dp_hot_plug;
> >
> > - if (!intel_dp_init_connector(intel_dig_port, intel_connector)) {
> > + if (!intel_dp_init_connector(intel_dig_port, intel_connector, type)) {
> > drm_encoder_cleanup(encoder);
> > kfree(intel_dig_port);
> > kfree(intel_connector);
> > + return false;
> > }
> > +
> > + return true;
> > +}
> > +
> > +void
> > +intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
> > +{
> > + intel_dp_init_typed(dev, output_reg, port, DRM_MODE_CONNECTOR_Unknown);
> > }
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 9d2624f..ff982d9 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -697,8 +697,11 @@ void intel_display_set_init_power(struct drm_device *dev, bool enable);
> >
> > /* intel_dp.c */
> > void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);
> > +bool intel_dp_init_typed(struct drm_device *dev, int output_reg,
> > + enum port port, int type);
> > bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> > - struct intel_connector *intel_connector);
> > + struct intel_connector *intel_connector,
> > + int type);
> > 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);
>
> We're supposed to get this info from the VBT, but we don't have the new
> spec yet. If we get that, we can parse it out in intel_bios.c and use
> it in intel_dp.c.
>
> In cases where that may be unreliable, your approach might be fine,
> though I suppose the typed init addition should be separate from the
> "try both kinds" addition on VLV.
Yeah this patch is just wrong. I guess I had imagined a better world
where the specs would actually be sane and the panel would always tell
us what type it is.
We could try to tell eDP and DP apart first via the optional eDP DPCD
registers, but if that doesn't confirm that it's eDP, we'd need to do
something silly like trying to talk to the panel w/ vdd off. If we get
an answer only w/ vdd on, then it would eDP.
But I'm going to try the VBT approach now, based on the spec Paulo had.
--
Ville Syrjälä
Intel OTC
More information about the Intel-gfx
mailing list