On Mon, Nov 14, 2016 at 08:15:56AM +0100, Daniel Vetter wrote:
> On Fri, Nov 11, 2016 at 07:14:24PM +0200, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > 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.otero@xxxxxxxxxxx>
> > Cc: stable@xxxxxxxxxxxxxxx
> > Tested-by: Daniel Otero <daniel.otero@xxxxxxxxxxx>
> > 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.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > 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.vetter@xxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Ville Syrjälä
Intel OTC
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html