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 <[email protected]> > Signed-off-by: Matt Roper <[email protected]> > --- > 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 > [email protected] > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
