| Message ID | 1478884464-14251-1-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Nov 11, 2016 at 07:14:24PM +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@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.otero@outlook.com> > Cc: stable@vger.kernel.org > Tested-by: Daniel Otero <daniel.otero@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.syrjala@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.vetter@ffwll.ch> > > 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
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@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@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.otero@outlook.com> > > Cc: stable@vger.kernel.org > > Tested-by: Daniel Otero <daniel.otero@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.syrjala@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.vetter@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
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; 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;