On Wed, Dec 20, 2023 at 02:13:41PM -0800, Radhakrishna Sripada wrote:
> Starting MTL and DG2 if a phy is not marked as USB-typeC or TBT capable
> by vbt we should not consider it as a Legacy type-c phy.
>
> The concept of Legacy-tc existed in platforms from Icelake to Alder lake
> where an external FIA can be routed to one of the phy's thus making the phy
> tc capable without being marked in the vbt.
>
> Discrete cards have native ports and client products post MTL have a 1:1
> mapping with type-C subsystem which is advertised by the bios. So rely on
> the vbt info regarding type-c capability.
>
> Signed-off-by: Radhakrishna Sripada <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
> drivers/gpu/drm/i915/display/intel_display.c | 29 ++++++++++++-------
> .../drm/i915/display/intel_display_device.h | 1 +
> 3 files changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 12a29363e5df..7d5b95f97d5f 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -5100,7 +5100,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv,
> }
>
> if (intel_phy_is_tc(dev_priv, phy)) {
> - bool is_legacy =
> + bool is_legacy = HAS_LEGACY_TC(dev_priv) &&
This doesn't make sense to me. PHYs are either TC or non-TC (aka combo
PHY) and TC PHYs can be configured to work either in legacy (a TC PHY
wired to a native DP connector) or non-legacy mode (a TC PHY wired to a
TC connector). So this would break the functionality on MTL native DP
connectors and all future platforms I looked at which also support this.
> !intel_bios_encoder_supports_typec_usb(devdata) &&
> !intel_bios_encoder_supports_tbt(devdata);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index b10aad15a63d..03006c9af824 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1854,17 +1854,9 @@ bool intel_phy_is_combo(struct drm_i915_private
> *dev_priv, enum phy phy)
> return false;
> }
>
> -bool intel_phy_is_tc(struct drm_i915_private *dev_priv, enum phy phy)
> +static bool intel_phy_is_legacy_tc(struct drm_i915_private *dev_priv, enum
> phy phy)
> {
> - /*
> - * DG2's "TC1", although TC-capable output, doesn't share the same flow
> - * as other platforms on the display engine side and rather rely on the
> - * SNPS PHY, that is programmed separately
> - */
> - if (IS_DG2(dev_priv))
> - return false;
> -
> - if (DISPLAY_VER(dev_priv) >= 13)
> + if (DISPLAY_VER(dev_priv) == 13)
> return phy >= PHY_F && phy <= PHY_I;
> else if (IS_TIGERLAKE(dev_priv))
> return phy >= PHY_D && phy <= PHY_I;
> @@ -1874,6 +1866,23 @@ bool intel_phy_is_tc(struct drm_i915_private
> *dev_priv, enum phy phy)
> return false;
> }
>
> +static bool intel_phy_is_vbt_tc(struct drm_i915_private *dev_priv, enum phy
> phy)
> +{
> + const struct intel_bios_encoder_data *data =
> + intel_bios_encoder_phy_data_lookup(dev_priv, phy);
> +
> + return intel_bios_encoder_supports_typec_usb(data) &&
> + intel_bios_encoder_supports_tbt(data);
Based on the above, this doesn't look correct: a TC PHY can be
configured by the vendor (reflected by the above typec_usb and tbt flags
in VBT) in any of the following ways:
- legacy mode (wired to a native DP connector): typec_usb:false,
tbt:false
- tbt-alt + dp-alt support (wired to a TC connector): typec_usb:true, tbt:true
- tbt-alt only support (wired to a TC connector): typec_usb:false,
tbt:true
- dp-alt only support (wired to a TC connector): typec_usb:true,
tbt:false
For all the above cases intel_phy_is_tc() should return true. So I don't
think this (is a PHY TC or non-TC) can be determined based on the above
VBT flags.
> +}
> +
> +bool intel_phy_is_tc(struct drm_i915_private *dev_priv, enum phy phy)
> +{
> + if (!HAS_LEGACY_TC(dev_priv))
> + return intel_phy_is_vbt_tc(dev_priv, phy);
> + else
> + return intel_phy_is_legacy_tc(dev_priv, phy);
> +}
> +
> bool intel_phy_is_snps(struct drm_i915_private *dev_priv, enum phy phy)
> {
> /*
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h
> b/drivers/gpu/drm/i915/display/intel_display_device.h
> index fe4268813786..9450e263c873 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> @@ -58,6 +58,7 @@ struct drm_printer;
> #define HAS_IPS(i915) (IS_HASWELL_ULT(i915) ||
> IS_BROADWELL(i915))
> #define HAS_LRR(i915) (DISPLAY_VER(i915) >= 12)
> #define HAS_LSPCON(i915) (IS_DISPLAY_VER(i915, 9, 10))
> +#define HAS_LEGACY_TC(i915) (IS_DISPLAY_VER(i915, 11, 13) &&
> !IS_DGFX(i915))
> #define HAS_MBUS_JOINING(i915) (IS_ALDERLAKE_P(i915) ||
> DISPLAY_VER(i915) >= 14)
> #define HAS_MSO(i915) (DISPLAY_VER(i915) >= 12)
> #define HAS_OVERLAY(i915) (DISPLAY_INFO(i915)->has_overlay)
> --
> 2.34.1
>
