On Fri, Jun 14, 2019 at 11:07:34AM -0700, Matt Roper wrote:
> EHL has a mux on combo PHY A that allows it to be driven either by an
> internal display (using DDI-A or DSI DPHY) or by an external display
> (using DDI-D). This is a motherboard design decision that can not be
> changed on the fly. Let's use the VBT child device settings to try to
> figure out our board's configuration and program the mux accordingly.
>
> Cc: Clint Taylor <Clinton.A.Taylor@xxxxxxxxx>
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> drivers/gpu/drm/i915/intel_bios.c | 24 ++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_bios.h | 1 +
> drivers/gpu/drm/i915/intel_combo_phy.c | 13 +++++++++++++
> 4 files changed, 39 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 368ee717580c..0ae0d85304b5 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -11121,6 +11121,7 @@ enum skl_power_gate {
> #define _ICL_PHY_MISC_B 0x64C04
> #define ICL_PHY_MISC(port) _MMIO_PORT(port, _ICL_PHY_MISC_A, \
> _ICL_PHY_MISC_B)
> +#define ICL_PHY_MISC_MUX_DDID (1 << 28)
> #define ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN (1 << 23)
>
> /* Icelake Display Stream Compression Registers */
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index d3d473934ea4..ac861852e843 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1667,6 +1667,9 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
> if (!child->device_type)
> continue;
>
> + DRM_DEBUG_KMS("Found VBT child device with type 0x%x\n",
> + child->device_type);
> +
> /*
> * Copy as much as we know (sizeof) and is available
> * (child_dev_size) of the child device. Accessing the data must
> @@ -2259,3 +2262,24 @@ enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *dev_priv,
>
> return aux_ch;
> }
> +
> +/**
> + * intel_bios_has_internal_display - are any child devices marked 'internal?'
> + * @dev_priv: i915 device instance
> + *
> + * Returns true if any of the child devices have a device type containing
> + * the %DEVICE_TYPE_INTERNAL_CONNECTOR bit.
> + */
> +bool intel_bios_has_internal_display(struct drm_i915_private *dev_priv)
> +{
> + int i;
> +
> + for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> + struct child_device_config *child = &dev_priv->vbt.child_dev[i];
> +
> + if (child->device_type & DEVICE_TYPE_INTERNAL_CONNECTOR)
> + return true;
> + }
> +
> + return false;
> +}
This feels super fragile. No hw strap for this?
Is the VBT DVO port referring to DDI or PHY?
If it's referring to DDI then I think we could check
for child device being present on DDI D and no child
device on DDI A/DSI. Not sure which way we should go
if there's a conflict.
But if it's referring to the PHY then we're hosed because
we can't tell which DDI is driving DVO port A aka. PHY A.
I guess in that case we should check for an internal/DSI vs.
external connector on DVO port A?
Not sure how many places we have in the code which assumes
1:1 mapping between DDI:PHY. I think we really need to
introduce a new namespace for the PHY so that we aren't
super confused all the time which thing we're talking about.
> diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
> index 4e42cfaf61a7..df822a1efe55 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -240,5 +240,6 @@ bool intel_bios_is_port_hpd_inverted(const struct drm_i915_private *i915,
> bool intel_bios_is_lspcon_present(const struct drm_i915_private *i915,
> enum port port);
> enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *dev_priv, enum port port);
> +bool intel_bios_has_internal_display(struct drm_i915_private *dev_priv);
>
> #endif /* _INTEL_BIOS_H_ */
> diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c b/drivers/gpu/drm/i915/intel_combo_phy.c
> index 841708da5a56..3bf3e41c5296 100644
> --- a/drivers/gpu/drm/i915/intel_combo_phy.c
> +++ b/drivers/gpu/drm/i915/intel_combo_phy.c
> @@ -273,7 +273,20 @@ static void icl_combo_phys_init(struct drm_i915_private *dev_priv)
> continue;
> }
>
> + /*
> + * EHL's combo PHY A can be hooked up to either an external
> + * display (via DDI-D) or an internal display (via DDI-A or
> + * the DSI DPHY). This is a motherboard design decision that
> + * can't be changed on the fly, so initialize the PHY's mux
> + * based on whether our VBT indicates the presence of any
> + * "internal" child devices.
> + */
> val = I915_READ(ICL_PHY_MISC(port));
> + if (!IS_ICELAKE(dev_priv) && port == PORT_A &&
> + !intel_bios_has_internal_display(dev_priv))
> + val |= ICL_PHY_MISC_MUX_DDID;
> + else
> + val &= ~ICL_PHY_MISC_MUX_DDID;
> val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> I915_WRITE(ICL_PHY_MISC(port), val);
>
> --
> 2.14.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
