Re: [PATCH v2 4/4] drm/i915: Determine DP++ type 1 DVI adaptor presence based on VBT

Ville Syrjälä
2016-05-04T11:29:22Z

On Wed, May 04, 2016 at 03:54:41PM +0530, Sharma, Shashank wrote:
>
>
> On 5/3/2016 12:38 AM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > DP dual mode type 1 DVI adaptors aren't required to implement any
> > registers, so it's a bit hard to detect them. The best way would
> > be to check the state of the CONFIG1 pin, but we have no way to
> > do that. So as a last resort, check the VBT to see if the HDMI
> > port is in fact a dual mode capable DP port.
> >
> > v2: Deal with VBT code reorganization
> > Deal with DRM_DP_DUAL_MODE_UNKNOWN
> > Reduce DEVICE_TYPE_DP_DUAL_MODE_BITS a bit
> > Accept both DP and HDMI dvo_port in VBT as my BSW
> > at least declare its DP port as HDMI :(
> >
> > Cc: stable@vger.kernel.org
> > Cc: Tore Anderson <tore@fud.no>
> > Reported-by: Tore Anderson <tore@fud.no>
> > Fixes: 7a0baa623446 ("Revert "drm/i915: Disable 12bpc hdmi for now"")
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > drivers/gpu/drm/i915/intel_bios.c | 36 +++++++++++++++++++++++++++++++++++
> > drivers/gpu/drm/i915/intel_hdmi.c | 30 +++++++++++++++++++++++++----
> > drivers/gpu/drm/i915/intel_vbt_defs.h | 13 +++++++++++++
> > 4 files changed, 76 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index ff6aaf0c4e1e..abe43922a08f 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -3489,6 +3489,7 @@ bool intel_bios_is_valid_vbt(const void *buf, size_t size);
> > bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
> > bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
> > bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port);
> > +bool intel_bios_is_port_dp_dual_mode(struct drm_i915_private *dev_priv, enum port port);
> > bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port);
> > bool intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
> > enum port port);
> > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> > index 81518116e00d..8b68c4882fba 100644
> > --- a/drivers/gpu/drm/i915/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > @@ -1597,6 +1597,42 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
> > return false;
> > }
> >
> > +bool intel_bios_is_port_dp_dual_mode(struct drm_i915_private *dev_priv, enum port port)
> > +{
> > + static const struct {
> > + u16 dp, hdmi;
> > + } port_mapping[] = {
> > + /*
> > + * Buggy VBTs may declare DP ports as having
> > + * HDMI type dvo_port :( So let's check both.
> > + */
> > + [PORT_B] = { DVO_PORT_DPB, DVO_PORT_HDMIB, },
> > + [PORT_C] = { DVO_PORT_DPC, DVO_PORT_HDMIC, },
> > + [PORT_D] = { DVO_PORT_DPD, DVO_PORT_HDMID, },
> > + [PORT_E] = { DVO_PORT_DPE, DVO_PORT_HDMIE, },
> > + };
> > + int i;
> > +
> > + if (port == PORT_A || port >= ARRAY_SIZE(port_mapping))
> > + return false;
> > +
> > + if (!dev_priv->vbt.child_dev_num)
> > + return false;
> > +
> > + for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> > + const union child_device_config *p_child =
> > + &dev_priv->vbt.child_dev[i];
> > +
> > + if ((p_child->common.dvo_port == port_mapping[port].dp ||
> > + p_child->common.dvo_port == port_mapping[port].hdmi) &&
> > + (p_child->common.device_type & DEVICE_TYPE_DP_DUAL_MODE_BITS) ==
> > + (DEVICE_TYPE_DP_DUAL_MODE & DEVICE_TYPE_DP_DUAL_MODE_BITS))
> > + return true;
> > + }
> > +
> > + return false;
> > +}
> > +
> > /**
> > * intel_bios_is_dsi_present - is DSI present in VBT
> > * @dev_priv: i915 device instance
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index c4d93e6b4bed..6b52c6accf6a 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -1396,16 +1396,38 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
> > }
> >
> > static void
> > -intel_hdmi_dp_dual_mode_detect(struct drm_connector *connector)
> > +intel_hdmi_dp_dual_mode_detect(struct drm_connector *connector, bool has_edid)
> > {
> > struct drm_i915_private *dev_priv = to_i915(connector->dev);
> > struct intel_hdmi *hdmi = intel_attached_hdmi(connector);
> > + enum port port = hdmi_to_dig_port(hdmi)->port;
> > struct i2c_adapter *adapter =
> > intel_gmbus_get_adapter(dev_priv, hdmi->ddc_bus);
> > enum drm_dp_dual_mode_type type = drm_dp_dual_mode_detect(adapter);
> >
> > - if (type == DRM_DP_DUAL_MODE_NONE ||
> > - type == DRM_DP_DUAL_MODE_UNKNOWN)
> > + /*
> > + * Type 1 DVI adaptors are not required to implement any
> > + * registers, so we can't always detect their presence.
> > + * Ideally we should be able to check the state of the
> > + * CONFIG1 pin, but no such luck on our hardware.
> > + *
> > + * The only method left to us is to check the VBT to see
> > + * if the port is a dual mode capable DP port. But let's
> > + * only do that when we sucesfully read the EDID, to avoid
> > + * confusing log messages about DP dual mode adaptors when
> > + * there's nothing connected to the port.
> > + */
> > + if (type == DRM_DP_DUAL_MODE_UNKNOWN) {
> > + if (has_edid &&
> > + intel_bios_is_port_dp_dual_mode(dev_priv, port)) {
> > + DRM_DEBUG_KMS("Assuming DP dual mode adaptor presence based on VBT\n");
> > + type = DRM_DP_DUAL_MODE_TYPE1_DVI;
> > + } else {
> > + type = DRM_DP_DUAL_MODE_NONE;
> > + }
> > + }
> > +
> > + if (type == DRM_DP_DUAL_MODE_NONE)
> > return;
> >
> > hdmi->dp_dual_mode.type = type;
> > @@ -1432,7 +1454,7 @@ intel_hdmi_set_edid(struct drm_connector *connector, bool force)
> > intel_gmbus_get_adapter(dev_priv,
> > intel_hdmi->ddc_bus));
> >
> > - intel_hdmi_dp_dual_mode_detect(connector);
> > + intel_hdmi_dp_dual_mode_detect(connector, edid != NULL);
> >
> > intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
> > }
> > diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h b/drivers/gpu/drm/i915/intel_vbt_defs.h
> > index a4a42f254c35..b1019e01e51d 100644
> > --- a/drivers/gpu/drm/i915/intel_vbt_defs.h
> > +++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
> > @@ -746,6 +746,7 @@ struct bdb_psr {
> > #define DEVICE_TYPE_INT_TV 0x1009
> > #define DEVICE_TYPE_HDMI 0x60D2
> > #define DEVICE_TYPE_DP 0x68C6
> > +#define DEVICE_TYPE_DP_DUAL_MODE 0x60D6
> > #define DEVICE_TYPE_eDP 0x78C6
> >
> > #define DEVICE_TYPE_CLASS_EXTENSION (1 << 15)
> > @@ -780,6 +781,18 @@ struct bdb_psr {
> > DEVICE_TYPE_DISPLAYPORT_OUTPUT | \
> > DEVICE_TYPE_ANALOG_OUTPUT)
> >
> > +#define DEVICE_TYPE_DP_DUAL_MODE_BITS \
> > + (DEVICE_TYPE_INTERNAL_CONNECTOR | \
> > + DEVICE_TYPE_NOT_HDMI_OUTPUT | \
> Why do we need this one ? Shouldn't the others cover all the cases ?
> Is there a special case in your mind ?

Doh. That bit shouldn't be here. It's the only bit I was ignoring in
the previous version, and definitely we should ignore it still. I'll
send out a v3. Good catch there.

>
> Regards
> Shashank
> > + DEVICE_TYPE_MIPI_OUTPUT | \
> > + DEVICE_TYPE_COMPOSITE_OUTPUT | \
> > + DEVICE_TYPE_LVDS_SINGALING | \
> > + DEVICE_TYPE_TMDS_DVI_SIGNALING | \
> > + DEVICE_TYPE_VIDEO_SIGNALING | \
> > + DEVICE_TYPE_DISPLAYPORT_OUTPUT | \
> > + DEVICE_TYPE_DIGITAL_OUTPUT | \
> > + DEVICE_TYPE_ANALOG_OUTPUT)
> > +
> > /* define the DVO port for HDMI output type */
> > #define DVO_B 1
> > #define DVO_C 2
> >

--
Ville Syrjälä
Intel OTC