On Tue, 28 Apr 2020, Imre Deak <imre.deak@xxxxxxxxx> wrote:
> On Tue, Apr 28, 2020 at 10:55:49AM +0300, Jani Nikula wrote:
>> On Thu, 23 Apr 2020, Imre Deak <imre.deak@xxxxxxxxx> 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@xxxxxxxxx>
>> > ---
>> > 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
