On Mon, Nov 14, 2016 at 08:15:56AM +0100, Daniel Vetter wrote:
> On Fri, Nov 11, 2016 at 07:14:24PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > 
> > My heuristic for detecting type 1 DVI DP++ adaptors based on the VBT
> > port information apparently didn't survive the reality of buggy VBTs.
> > In this particular case we have a machine with a natice HDMI port, but
> > the VBT telsl us it's a DP++ port based on its capabilities.
> > 
> > The dvo_port information in VBT does claim that we're dealing with a
> > HDMI port though, but we have other machines which do the same even
> > when they actually have DP++ ports. So that piece of information alone
> > isn't sufficient to tell the two apart.
> > 
> > After staring at a bunch of VBTs from various machines, I have to
> > conclude that the only other semi-reliable clue we can use is the
> > presence of the AUX channel in the VBT. On this particular machine
> > AUX channel is specified as zero, whereas on some of the other machines
> > which listed the DP++ port as HDMI have a non-zero AUX channel.
> > 
> > I've also seen VBTs which have dvo_port a DP but have a zero AUX
> > channel. I believe those we need to treat as DP ports, so we'll limit
> > the AUX channel check to just the cases where dvo_port is HDMI.
> > 
> > If we encounter any more serious failures with this heuristic I think
> > we'll have to have to throw it out entirely. But that could mean that
> > there is a risk of type 1 DVI dongle users getting greeted by a
> > black screen, so I'd rather not go there unless absolutely necessary.
> > 
> > Cc: Daniel Otero <daniel.ot...@outlook.com>
> > Cc: sta...@vger.kernel.org
> > Tested-by: Daniel Otero <daniel.ot...@outlook.com>
> > Fixes: d61992565bd3 ("drm/i915: Determine DP++ type 1 DVI adaptor presence 
> > based on VBT")
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97994
> > Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_bios.c     | 32 ++++++++++++++++++++++++--------
> >  drivers/gpu/drm/i915/intel_vbt_defs.h |  3 ++-
> >  2 files changed, 26 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_bios.c 
> > b/drivers/gpu/drm/i915/intel_bios.c
> > index 5ab646ef8c9f..33ed05186810 100644
> > --- a/drivers/gpu/drm/i915/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > @@ -1147,7 +1147,7 @@ static void parse_ddi_port(struct drm_i915_private 
> > *dev_priv, enum port port,
> >     if (!child)
> >             return;
> >  
> > -   aux_channel = child->raw[25];
> > +   aux_channel = child->common.aux_channel;
> >     ddc_pin = child->common.ddc_pin;
> >  
> >     is_dvi = child->common.device_type & DEVICE_TYPE_TMDS_DVI_SIGNALING;
> > @@ -1677,7 +1677,8 @@ 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 bool child_dev_is_dp_dual_mode(const union child_device_config 
> > *p_child,
> > +                                 enum port port)
> >  {
> >     static const struct {
> >             u16 dp, hdmi;
> > @@ -1691,22 +1692,37 @@ bool intel_bios_is_port_dp_dual_mode(struct 
> > drm_i915_private *dev_priv, enum por
> >             [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)
> > +   if ((p_child->common.device_type & DEVICE_TYPE_DP_DUAL_MODE_BITS) !=
> > +       (DEVICE_TYPE_DP_DUAL_MODE & DEVICE_TYPE_DP_DUAL_MODE_BITS))
> > +           return false;
> > +
> > +   if (p_child->common.dvo_port == port_mapping[port].dp)
> > +           return true;
> > +
> > +   /* Only accept a HDMI dvo_port as DP++ if it has an AUX channel */
> > +   if (p_child->common.dvo_port == port_mapping[port].hdmi &&
> > +       p_child->common.aux_channel != 0)
> > +           return true;
> > +
> > +   return false;
> > +}
> > +
> > +bool intel_bios_is_port_dp_dual_mode(struct drm_i915_private *dev_priv, 
> > enum port port)
> > +{
> > +   int i;
> > +
> > +   if (port == PORT_A)
> >             return false;
> 
> We check for PORT_A twice now. Otherwise contains what it says on the tin,
> but no idea whether this is the perfect solution either. Also need to
> check vbt docs for the right bit, but assuming that checks out (I'll
> report on irc), and with the above nitpick fixed:
> 
> Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>

11:34 < danvet> vsyrjala, vbt seems to check out too

v2: Remove the duplicate PORT_A check (Daniel)
    Fix some typos in the commit message

Pushed to dinq. Thanks for the review.

> 
> >  
> >     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))
> > +           if (child_dev_is_dp_dual_mode(p_child, port))
> >                     return true;
> >     }
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h 
> > b/drivers/gpu/drm/i915/intel_vbt_defs.h
> > index 68db9621f1f0..8886cab19f98 100644
> > --- a/drivers/gpu/drm/i915/intel_vbt_defs.h
> > +++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
> > @@ -280,7 +280,8 @@ struct common_child_dev_config {
> >     u8 dp_support:1;
> >     u8 tmds_support:1;
> >     u8 support_reserved:5;
> > -   u8 not_common3[12];
> > +   u8 aux_channel;
> > +   u8 not_common3[11];
> >     u8 iboost_level;
> >  } __packed;
> >  
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to