| Message ID | 20200423181937.25176-1-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/icl+: Prevent using non-TypeC AUX channels on TypeC ports | expand |
On Thu, 23 Apr 2020, Imre Deak <imre.deak@intel.com> wrote: > Using an AUX channel which by default belongs to a non-TypeC PHY won't > work on a TypeC PHY, since - as a side-effect besides providing an AUX > channel - the AUX channel power well affects power manangement specific > to the TypeC subsystem. Using a TypeC AUX channel on a non-TypeC PHY > would probably also cause problems, so for simplicity prevent both. > > This fixes at least an ICL-Y machine in CI, which has a buggy VBT > setting AUX-B as an alternative channel for port C. Is it a production machine? Not happy about adding stuff for pre-pro machines with buggy VBT. It'll bite us later. It always has. Also, hate to see VBT code call into intel_display.c (intel_phy_is_tc). BR, Jani. > > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/display/intel_bios.c | 84 +++++++++++++++-------- > 1 file changed, 57 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c > index 839124647202..10d463723d12 100644 > --- a/drivers/gpu/drm/i915/display/intel_bios.c > +++ b/drivers/gpu/drm/i915/display/intel_bios.c > @@ -1538,11 +1538,38 @@ static enum port get_port_by_aux_ch(struct drm_i915_private *i915, u8 aux_ch) > return PORT_NONE; > } > > +static enum aux_ch > +intel_bios_port_info_aux_ch(const struct ddi_vbt_port_info *info) > +{ > + switch (info->alternate_aux_channel) { > + case DP_AUX_A: > + return AUX_CH_A; > + case DP_AUX_B: > + return AUX_CH_B; > + case DP_AUX_C: > + return AUX_CH_C; > + case DP_AUX_D: > + return AUX_CH_D; > + case DP_AUX_E: > + return AUX_CH_E; > + case DP_AUX_F: > + return AUX_CH_F; > + case DP_AUX_G: > + return AUX_CH_G; > + default: > + MISSING_CASE(info->alternate_aux_channel); > + return AUX_CH_A; > + } > +} > + > static void sanitize_aux_ch(struct drm_i915_private *dev_priv, > enum port port) > { > struct ddi_vbt_port_info *info = &dev_priv->vbt.ddi_port_info[port]; > enum port p; > + enum aux_ch aux_ch; > + bool aux_is_tc; > + bool phy_is_tc; > > if (!info->alternate_aux_channel) > return; > @@ -1571,6 +1598,35 @@ static void sanitize_aux_ch(struct drm_i915_private *dev_priv, > > info->supports_dp = false; > info->alternate_aux_channel = 0; > + > + return; > + } > + > + aux_ch = intel_bios_port_info_aux_ch(info); > + /* The AUX CH -> default port is a 1:1 mapping. */ > + aux_is_tc = intel_phy_is_tc(dev_priv, > + intel_port_to_phy(dev_priv, > + (enum port)aux_ch)); > + phy_is_tc = intel_phy_is_tc(dev_priv, > + intel_port_to_phy(dev_priv, port)); > + if (aux_is_tc != phy_is_tc) { > + /* > + * Using an AUX channel which by default belongs to a TypeC > + * PHY can't be used for non-TypeC PHYs and vice-versa. The > + * reason is that TypeC AUX power wells can only be enabled in > + * the current TypeC mode of the PHY and have an effect on power > + * management specific to the TypeC subsystem. > + */ > + drm_dbg_kms(&dev_priv->drm, > + "Port %c on a %s PHY is trying to use the %s AUX CH %c, " > + "disabling DP support on this port.\n", > + port_name(port), > + phy_is_tc ? "TypeC" : "non-TypeC", > + aux_is_tc ? "TypeC" : "non-TypeC", > + aux_ch_name(aux_ch)); > + > + info->supports_dp = false; > + info->alternate_aux_channel = 0; > } > } > > @@ -2595,33 +2651,7 @@ enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *dev_priv, > return aux_ch; > } > > - switch (info->alternate_aux_channel) { > - case DP_AUX_A: > - aux_ch = AUX_CH_A; > - break; > - case DP_AUX_B: > - aux_ch = AUX_CH_B; > - break; > - case DP_AUX_C: > - aux_ch = AUX_CH_C; > - break; > - case DP_AUX_D: > - aux_ch = AUX_CH_D; > - break; > - case DP_AUX_E: > - aux_ch = AUX_CH_E; > - break; > - case DP_AUX_F: > - aux_ch = AUX_CH_F; > - break; > - case DP_AUX_G: > - aux_ch = AUX_CH_G; > - break; > - default: > - MISSING_CASE(info->alternate_aux_channel); > - aux_ch = AUX_CH_A; > - break; > - } > + aux_ch = intel_bios_port_info_aux_ch(info); > > drm_dbg_kms(&dev_priv->drm, "using AUX %c for port %c (VBT)\n", > aux_ch_name(aux_ch), port_name(port));
On Tue, Apr 28, 2020 at 10:55:49AM +0300, Jani Nikula wrote: > On Thu, 23 Apr 2020, Imre Deak <imre.deak@intel.com> wrote: > > Using an AUX channel which by default belongs to a non-TypeC PHY won't > > work on a TypeC PHY, since - as a side-effect besides providing an AUX > > channel - the AUX channel power well affects power manangement specific > > to the TypeC subsystem. Using a TypeC AUX channel on a non-TypeC PHY > > would probably also cause problems, so for simplicity prevent both. > > > > This fixes at least an ICL-Y machine in CI, which has a buggy VBT > > setting AUX-B as an alternative channel for port C. > > Is it a production machine? Yes. > Not happy about adding stuff for pre-pro machines with buggy VBT. > It'll bite us later. It always has. If there is a buggy VBT with this issue it will cause a problem somewhere down the pipeline which is difficult to track down, power well timeouts, machine hangs etc. I would like to catch this early and avoid having to spend time debugging these other issues. > Also, hate to see VBT code call into intel_display.c (intel_phy_is_tc). That's the way to determine if a port/PHY is TypeC on a platform or not. --Imre > > BR, > Jani. > > > > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_bios.c | 84 +++++++++++++++-------- > > 1 file changed, 57 insertions(+), 27 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c > > index 839124647202..10d463723d12 100644 > > --- a/drivers/gpu/drm/i915/display/intel_bios.c > > +++ b/drivers/gpu/drm/i915/display/intel_bios.c > > @@ -1538,11 +1538,38 @@ static enum port get_port_by_aux_ch(struct drm_i915_private *i915, u8 aux_ch) > > return PORT_NONE; > > } > > > > +static enum aux_ch > > +intel_bios_port_info_aux_ch(const struct ddi_vbt_port_info *info) > > +{ > > + switch (info->alternate_aux_channel) { > > + case DP_AUX_A: > > + return AUX_CH_A; > > + case DP_AUX_B: > > + return AUX_CH_B; > > + case DP_AUX_C: > > + return AUX_CH_C; > > + case DP_AUX_D: > > + return AUX_CH_D; > > + case DP_AUX_E: > > + return AUX_CH_E; > > + case DP_AUX_F: > > + return AUX_CH_F; > > + case DP_AUX_G: > > + return AUX_CH_G; > > + default: > > + MISSING_CASE(info->alternate_aux_channel); > > + return AUX_CH_A; > > + } > > +} > > + > > static void sanitize_aux_ch(struct drm_i915_private *dev_priv, > > enum port port) > > { > > struct ddi_vbt_port_info *info = &dev_priv->vbt.ddi_port_info[port]; > > enum port p; > > + enum aux_ch aux_ch; > > + bool aux_is_tc; > > + bool phy_is_tc; > > > > if (!info->alternate_aux_channel) > > return; > > @@ -1571,6 +1598,35 @@ static void sanitize_aux_ch(struct drm_i915_private *dev_priv, > > > > info->supports_dp = false; > > info->alternate_aux_channel = 0; > > + > > + return; > > + } > > + > > + aux_ch = intel_bios_port_info_aux_ch(info); > > + /* The AUX CH -> default port is a 1:1 mapping. */ > > + aux_is_tc = intel_phy_is_tc(dev_priv, > > + intel_port_to_phy(dev_priv, > > + (enum port)aux_ch)); > > + phy_is_tc = intel_phy_is_tc(dev_priv, > > + intel_port_to_phy(dev_priv, port)); > > + if (aux_is_tc != phy_is_tc) { > > + /* > > + * Using an AUX channel which by default belongs to a TypeC > > + * PHY can't be used for non-TypeC PHYs and vice-versa. The > > + * reason is that TypeC AUX power wells can only be enabled in > > + * the current TypeC mode of the PHY and have an effect on power > > + * management specific to the TypeC subsystem. > > + */ > > + drm_dbg_kms(&dev_priv->drm, > > + "Port %c on a %s PHY is trying to use the %s AUX CH %c, " > > + "disabling DP support on this port.\n", > > + port_name(port), > > + phy_is_tc ? "TypeC" : "non-TypeC", > > + aux_is_tc ? "TypeC" : "non-TypeC", > > + aux_ch_name(aux_ch)); > > + > > + info->supports_dp = false; > > + info->alternate_aux_channel = 0; > > } > > } > > > > @@ -2595,33 +2651,7 @@ enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *dev_priv, > > return aux_ch; > > } > > > > - switch (info->alternate_aux_channel) { > > - case DP_AUX_A: > > - aux_ch = AUX_CH_A; > > - break; > > - case DP_AUX_B: > > - aux_ch = AUX_CH_B; > > - break; > > - case DP_AUX_C: > > - aux_ch = AUX_CH_C; > > - break; > > - case DP_AUX_D: > > - aux_ch = AUX_CH_D; > > - break; > > - case DP_AUX_E: > > - aux_ch = AUX_CH_E; > > - break; > > - case DP_AUX_F: > > - aux_ch = AUX_CH_F; > > - break; > > - case DP_AUX_G: > > - aux_ch = AUX_CH_G; > > - break; > > - default: > > - MISSING_CASE(info->alternate_aux_channel); > > - aux_ch = AUX_CH_A; > > - break; > > - } > > + aux_ch = intel_bios_port_info_aux_ch(info); > > > > drm_dbg_kms(&dev_priv->drm, "using AUX %c for port %c (VBT)\n", > > aux_ch_name(aux_ch), port_name(port)); > > -- > Jani Nikula, Intel Open Source Graphics Center
On Tue, 28 Apr 2020, Imre Deak <imre.deak@intel.com> wrote: > On Tue, Apr 28, 2020 at 10:55:49AM +0300, Jani Nikula wrote: >> On Thu, 23 Apr 2020, Imre Deak <imre.deak@intel.com> wrote: >> > Using an AUX channel which by default belongs to a non-TypeC PHY won't >> > work on a TypeC PHY, since - as a side-effect besides providing an AUX >> > channel - the AUX channel power well affects power manangement specific >> > to the TypeC subsystem. Using a TypeC AUX channel on a non-TypeC PHY >> > would probably also cause problems, so for simplicity prevent both. >> > >> > This fixes at least an ICL-Y machine in CI, which has a buggy VBT >> > setting AUX-B as an alternative channel for port C. >> >> Is it a production machine? > > Yes. *sigh* Yeah I guess that settles it, we'll need this. :/ Ack. BR, Jani. > >> Not happy about adding stuff for pre-pro machines with buggy VBT. >> It'll bite us later. It always has. > > If there is a buggy VBT with this issue it will cause a problem > somewhere down the pipeline which is difficult to track down, power well > timeouts, machine hangs etc. I would like to catch this early and avoid > having to spend time debugging these other issues. > >> Also, hate to see VBT code call into intel_display.c (intel_phy_is_tc). > > That's the way to determine if a port/PHY is TypeC on a platform or not. > > --Imre > >> >> BR, >> Jani. >> >> >> > >> > Signed-off-by: Imre Deak <imre.deak@intel.com> >> > --- >> > drivers/gpu/drm/i915/display/intel_bios.c | 84 +++++++++++++++-------- >> > 1 file changed, 57 insertions(+), 27 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c >> > index 839124647202..10d463723d12 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_bios.c >> > +++ b/drivers/gpu/drm/i915/display/intel_bios.c >> > @@ -1538,11 +1538,38 @@ static enum port get_port_by_aux_ch(struct drm_i915_private *i915, u8 aux_ch) >> > return PORT_NONE; >> > } >> > >> > +static enum aux_ch >> > +intel_bios_port_info_aux_ch(const struct ddi_vbt_port_info *info) >> > +{ >> > + switch (info->alternate_aux_channel) { >> > + case DP_AUX_A: >> > + return AUX_CH_A; >> > + case DP_AUX_B: >> > + return AUX_CH_B; >> > + case DP_AUX_C: >> > + return AUX_CH_C; >> > + case DP_AUX_D: >> > + return AUX_CH_D; >> > + case DP_AUX_E: >> > + return AUX_CH_E; >> > + case DP_AUX_F: >> > + return AUX_CH_F; >> > + case DP_AUX_G: >> > + return AUX_CH_G; >> > + default: >> > + MISSING_CASE(info->alternate_aux_channel); >> > + return AUX_CH_A; >> > + } >> > +} >> > + >> > static void sanitize_aux_ch(struct drm_i915_private *dev_priv, >> > enum port port) >> > { >> > struct ddi_vbt_port_info *info = &dev_priv->vbt.ddi_port_info[port]; >> > enum port p; >> > + enum aux_ch aux_ch; >> > + bool aux_is_tc; >> > + bool phy_is_tc; >> > >> > if (!info->alternate_aux_channel) >> > return; >> > @@ -1571,6 +1598,35 @@ static void sanitize_aux_ch(struct drm_i915_private *dev_priv, >> > >> > info->supports_dp = false; >> > info->alternate_aux_channel = 0; >> > + >> > + return; >> > + } >> > + >> > + aux_ch = intel_bios_port_info_aux_ch(info); >> > + /* The AUX CH -> default port is a 1:1 mapping. */ >> > + aux_is_tc = intel_phy_is_tc(dev_priv, >> > + intel_port_to_phy(dev_priv, >> > + (enum port)aux_ch)); >> > + phy_is_tc = intel_phy_is_tc(dev_priv, >> > + intel_port_to_phy(dev_priv, port)); >> > + if (aux_is_tc != phy_is_tc) { >> > + /* >> > + * Using an AUX channel which by default belongs to a TypeC >> > + * PHY can't be used for non-TypeC PHYs and vice-versa. The >> > + * reason is that TypeC AUX power wells can only be enabled in >> > + * the current TypeC mode of the PHY and have an effect on power >> > + * management specific to the TypeC subsystem. >> > + */ >> > + drm_dbg_kms(&dev_priv->drm, >> > + "Port %c on a %s PHY is trying to use the %s AUX CH %c, " >> > + "disabling DP support on this port.\n", >> > + port_name(port), >> > + phy_is_tc ? "TypeC" : "non-TypeC", >> > + aux_is_tc ? "TypeC" : "non-TypeC", >> > + aux_ch_name(aux_ch)); >> > + >> > + info->supports_dp = false; >> > + info->alternate_aux_channel = 0; >> > } >> > } >> > >> > @@ -2595,33 +2651,7 @@ enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *dev_priv, >> > return aux_ch; >> > } >> > >> > - switch (info->alternate_aux_channel) { >> > - case DP_AUX_A: >> > - aux_ch = AUX_CH_A; >> > - break; >> > - case DP_AUX_B: >> > - aux_ch = AUX_CH_B; >> > - break; >> > - case DP_AUX_C: >> > - aux_ch = AUX_CH_C; >> > - break; >> > - case DP_AUX_D: >> > - aux_ch = AUX_CH_D; >> > - break; >> > - case DP_AUX_E: >> > - aux_ch = AUX_CH_E; >> > - break; >> > - case DP_AUX_F: >> > - aux_ch = AUX_CH_F; >> > - break; >> > - case DP_AUX_G: >> > - aux_ch = AUX_CH_G; >> > - break; >> > - default: >> > - MISSING_CASE(info->alternate_aux_channel); >> > - aux_ch = AUX_CH_A; >> > - break; >> > - } >> > + aux_ch = intel_bios_port_info_aux_ch(info); >> > >> > drm_dbg_kms(&dev_priv->drm, "using AUX %c for port %c (VBT)\n", >> > aux_ch_name(aux_ch), port_name(port)); >> >> -- >> Jani Nikula, Intel Open Source Graphics Center
On Tue, Apr 28, 2020 at 12:05:35PM +0300, Jani Nikula wrote: > On Tue, 28 Apr 2020, Imre Deak <imre.deak@intel.com> wrote: > > On Tue, Apr 28, 2020 at 10:55:49AM +0300, Jani Nikula wrote: > >> On Thu, 23 Apr 2020, Imre Deak <imre.deak@intel.com> wrote: > >> > Using an AUX channel which by default belongs to a non-TypeC PHY won't > >> > work on a TypeC PHY, since - as a side-effect besides providing an AUX > >> > channel - the AUX channel power well affects power manangement specific > >> > to the TypeC subsystem. Using a TypeC AUX channel on a non-TypeC PHY > >> > would probably also cause problems, so for simplicity prevent both. > >> > > >> > This fixes at least an ICL-Y machine in CI, which has a buggy VBT > >> > setting AUX-B as an alternative channel for port C. > >> > >> Is it a production machine? > > > > Yes. Err, I meant to say it's a pre-production machine icl-dsi. However, the problem is not specific to whether the machine is pre-pro or not. A VBT with this problem on a production machine would cause the same problem. > *sigh* > > Yeah I guess that settles it, we'll need this. :/ > > Ack. > > >> Not happy about adding stuff for pre-pro machines with buggy VBT. > >> It'll bite us later. It always has. > > > > If there is a buggy VBT with this issue it will cause a problem > > somewhere down the pipeline which is difficult to track down, power well > > timeouts, machine hangs etc. I would like to catch this early and avoid > > having to spend time debugging these other issues. > > > >> Also, hate to see VBT code call into intel_display.c (intel_phy_is_tc). > > > > That's the way to determine if a port/PHY is TypeC on a platform or not. > > > > --Imre > > > >> > >> BR, > >> Jani. > >> > >> > >> > > >> > Signed-off-by: Imre Deak <imre.deak@intel.com> > >> > --- > >> > drivers/gpu/drm/i915/display/intel_bios.c | 84 +++++++++++++++-------- > >> > 1 file changed, 57 insertions(+), 27 deletions(-) > >> > > >> > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c > >> > index 839124647202..10d463723d12 100644 > >> > --- a/drivers/gpu/drm/i915/display/intel_bios.c > >> > +++ b/drivers/gpu/drm/i915/display/intel_bios.c > >> > @@ -1538,11 +1538,38 @@ static enum port get_port_by_aux_ch(struct drm_i915_private *i915, u8 aux_ch) > >> > return PORT_NONE; > >> > } > >> > > >> > +static enum aux_ch > >> > +intel_bios_port_info_aux_ch(const struct ddi_vbt_port_info *info) > >> > +{ > >> > + switch (info->alternate_aux_channel) { > >> > + case DP_AUX_A: > >> > + return AUX_CH_A; > >> > + case DP_AUX_B: > >> > + return AUX_CH_B; > >> > + case DP_AUX_C: > >> > + return AUX_CH_C; > >> > + case DP_AUX_D: > >> > + return AUX_CH_D; > >> > + case DP_AUX_E: > >> > + return AUX_CH_E; > >> > + case DP_AUX_F: > >> > + return AUX_CH_F; > >> > + case DP_AUX_G: > >> > + return AUX_CH_G; > >> > + default: > >> > + MISSING_CASE(info->alternate_aux_channel); > >> > + return AUX_CH_A; > >> > + } > >> > +} > >> > + > >> > static void sanitize_aux_ch(struct drm_i915_private *dev_priv, > >> > enum port port) > >> > { > >> > struct ddi_vbt_port_info *info = &dev_priv->vbt.ddi_port_info[port]; > >> > enum port p; > >> > + enum aux_ch aux_ch; > >> > + bool aux_is_tc; > >> > + bool phy_is_tc; > >> > > >> > if (!info->alternate_aux_channel) > >> > return; > >> > @@ -1571,6 +1598,35 @@ static void sanitize_aux_ch(struct drm_i915_private *dev_priv, > >> > > >> > info->supports_dp = false; > >> > info->alternate_aux_channel = 0; > >> > + > >> > + return; > >> > + } > >> > + > >> > + aux_ch = intel_bios_port_info_aux_ch(info); > >> > + /* The AUX CH -> default port is a 1:1 mapping. */ > >> > + aux_is_tc = intel_phy_is_tc(dev_priv, > >> > + intel_port_to_phy(dev_priv, > >> > + (enum port)aux_ch)); > >> > + phy_is_tc = intel_phy_is_tc(dev_priv, > >> > + intel_port_to_phy(dev_priv, port)); > >> > + if (aux_is_tc != phy_is_tc) { > >> > + /* > >> > + * Using an AUX channel which by default belongs to a TypeC > >> > + * PHY can't be used for non-TypeC PHYs and vice-versa. The > >> > + * reason is that TypeC AUX power wells can only be enabled in > >> > + * the current TypeC mode of the PHY and have an effect on power > >> > + * management specific to the TypeC subsystem. > >> > + */ > >> > + drm_dbg_kms(&dev_priv->drm, > >> > + "Port %c on a %s PHY is trying to use the %s AUX CH %c, " > >> > + "disabling DP support on this port.\n", > >> > + port_name(port), > >> > + phy_is_tc ? "TypeC" : "non-TypeC", > >> > + aux_is_tc ? "TypeC" : "non-TypeC", > >> > + aux_ch_name(aux_ch)); > >> > + > >> > + info->supports_dp = false; > >> > + info->alternate_aux_channel = 0; > >> > } > >> > } > >> > > >> > @@ -2595,33 +2651,7 @@ enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *dev_priv, > >> > return aux_ch; > >> > } > >> > > >> > - switch (info->alternate_aux_channel) { > >> > - case DP_AUX_A: > >> > - aux_ch = AUX_CH_A; > >> > - break; > >> > - case DP_AUX_B: > >> > - aux_ch = AUX_CH_B; > >> > - break; > >> > - case DP_AUX_C: > >> > - aux_ch = AUX_CH_C; > >> > - break; > >> > - case DP_AUX_D: > >> > - aux_ch = AUX_CH_D; > >> > - break; > >> > - case DP_AUX_E: > >> > - aux_ch = AUX_CH_E; > >> > - break; > >> > - case DP_AUX_F: > >> > - aux_ch = AUX_CH_F; > >> > - break; > >> > - case DP_AUX_G: > >> > - aux_ch = AUX_CH_G; > >> > - break; > >> > - default: > >> > - MISSING_CASE(info->alternate_aux_channel); > >> > - aux_ch = AUX_CH_A; > >> > - break; > >> > - } > >> > + aux_ch = intel_bios_port_info_aux_ch(info); > >> > > >> > drm_dbg_kms(&dev_priv->drm, "using AUX %c for port %c (VBT)\n", > >> > aux_ch_name(aux_ch), port_name(port)); > >> > >> -- > >> Jani Nikula, Intel Open Source Graphics Center > > -- > Jani Nikula, Intel Open Source Graphics Center
diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c index 839124647202..10d463723d12 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.c +++ b/drivers/gpu/drm/i915/display/intel_bios.c @@ -1538,11 +1538,38 @@ static enum port get_port_by_aux_ch(struct drm_i915_private *i915, u8 aux_ch) return PORT_NONE; } +static enum aux_ch +intel_bios_port_info_aux_ch(const struct ddi_vbt_port_info *info) +{ + switch (info->alternate_aux_channel) { + case DP_AUX_A: + return AUX_CH_A; + case DP_AUX_B: + return AUX_CH_B; + case DP_AUX_C: + return AUX_CH_C; + case DP_AUX_D: + return AUX_CH_D; + case DP_AUX_E: + return AUX_CH_E; + case DP_AUX_F: + return AUX_CH_F; + case DP_AUX_G: + return AUX_CH_G; + default: + MISSING_CASE(info->alternate_aux_channel); + return AUX_CH_A; + } +} + static void sanitize_aux_ch(struct drm_i915_private *dev_priv, enum port port) { struct ddi_vbt_port_info *info = &dev_priv->vbt.ddi_port_info[port]; enum port p; + enum aux_ch aux_ch; + bool aux_is_tc; + bool phy_is_tc; if (!info->alternate_aux_channel) return; @@ -1571,6 +1598,35 @@ static void sanitize_aux_ch(struct drm_i915_private *dev_priv, info->supports_dp = false; info->alternate_aux_channel = 0; + + return; + } + + aux_ch = intel_bios_port_info_aux_ch(info); + /* The AUX CH -> default port is a 1:1 mapping. */ + aux_is_tc = intel_phy_is_tc(dev_priv, + intel_port_to_phy(dev_priv, + (enum port)aux_ch)); + phy_is_tc = intel_phy_is_tc(dev_priv, + intel_port_to_phy(dev_priv, port)); + if (aux_is_tc != phy_is_tc) { + /* + * Using an AUX channel which by default belongs to a TypeC + * PHY can't be used for non-TypeC PHYs and vice-versa. The + * reason is that TypeC AUX power wells can only be enabled in + * the current TypeC mode of the PHY and have an effect on power + * management specific to the TypeC subsystem. + */ + drm_dbg_kms(&dev_priv->drm, + "Port %c on a %s PHY is trying to use the %s AUX CH %c, " + "disabling DP support on this port.\n", + port_name(port), + phy_is_tc ? "TypeC" : "non-TypeC", + aux_is_tc ? "TypeC" : "non-TypeC", + aux_ch_name(aux_ch)); + + info->supports_dp = false; + info->alternate_aux_channel = 0; } } @@ -2595,33 +2651,7 @@ enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *dev_priv, return aux_ch; } - switch (info->alternate_aux_channel) { - case DP_AUX_A: - aux_ch = AUX_CH_A; - break; - case DP_AUX_B: - aux_ch = AUX_CH_B; - break; - case DP_AUX_C: - aux_ch = AUX_CH_C; - break; - case DP_AUX_D: - aux_ch = AUX_CH_D; - break; - case DP_AUX_E: - aux_ch = AUX_CH_E; - break; - case DP_AUX_F: - aux_ch = AUX_CH_F; - break; - case DP_AUX_G: - aux_ch = AUX_CH_G; - break; - default: - MISSING_CASE(info->alternate_aux_channel); - aux_ch = AUX_CH_A; - break; - } + aux_ch = intel_bios_port_info_aux_ch(info); drm_dbg_kms(&dev_priv->drm, "using AUX %c for port %c (VBT)\n", aux_ch_name(aux_ch), port_name(port));
Using an AUX channel which by default belongs to a non-TypeC PHY won't work on a TypeC PHY, since - as a side-effect besides providing an AUX channel - the AUX channel power well affects power manangement specific to the TypeC subsystem. Using a TypeC AUX channel on a non-TypeC PHY would probably also cause problems, so for simplicity prevent both. This fixes at least an ICL-Y machine in CI, which has a buggy VBT setting AUX-B as an alternative channel for port C. Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/display/intel_bios.c | 84 +++++++++++++++-------- 1 file changed, 57 insertions(+), 27 deletions(-)