On Fri, Oct 20, 2017 at 04:11:27PM -0700, Rodrigo Vivi wrote:
> As we faced in BXT, on CNL DDI_A_4_LANES is not
> set as expected when system is boot with multiple
> monitors connected. This result in wrong lane
> setup impacting the max data rate available and
> consequently blocking modeset on eDP, resulting
> in a blank screen.
>
> Most of CNL SKUs don't support DDI-E.
> The only SKU that supports DDI-E is the same
> that supports the full A/E split called DDI-F.
>
> Also when DDI-F is used DDI-E cannot be used because
> they share Interrupts. So DDI-E is almost useless.
> Anyways let's consider this is possible and rely on
> VBT for that.
>
> Since this become a trend this commit also adds
> an extra commit message so in the future we have
> an extra insight when we are dealing with this
> same case.
>
> This patch was initialy start by Clint, but required
> many changes including full commit message. So
> Credits entirely to Clint for finding this.
>
> Suggested-by: Clint Taylor <clinton.a.tay...@intel.com>
> Cc: Clint Taylor <clinton.a.tay...@intel.com>
> Cc: Mika Kahola <mika.kah...@intel.com>
> Cc: Jani Nikula <jani.nik...@intel.com>
> Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.v...@intel.com>
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index adf51b328844..8acacf800a24 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2803,17 +2803,29 @@ void intel_ddi_init(struct drm_i915_private
> *dev_priv, enum port port)
> }
>
> /*
> - * Bspec says that DDI_A_4_LANES is the only supported configuration
> - * for Broxton. Yet some BIOS fail to set this bit on port A if eDP
> - * wasn't lit up at boot. Force this bit on in our internal
> + * Some BIOS might fail to set this bit on port A if eDP
> + * wasn't lit up at boot. Force this bit when needed on in our internal
> * configuration so that we use the proper lane count for our
> * calculations.
> */
> - if (IS_GEN9_LP(dev_priv) && port == PORT_A) {
> - if (!(intel_dig_port->saved_port_bits & DDI_A_4_LANES)) {
> - DRM_DEBUG_KMS("BXT BIOS forgot to set DDI_A_4_LANES for
> port A; fixing\n");
> + if (port == PORT_A &&
> + !(intel_dig_port->saved_port_bits & DDI_A_4_LANES)) {
> +
> + /* Broxton: Bspec says that DDI_A_4_LANES is the only supported
> + * configuration
> + * Cannonlake: Most of SKUs don't support DDI_E, and the only
> + * one who does also have a full A/E split called
> + * DDI_F what makes DDI_E useless. However for this
> + * case let's trust VBT info.
> + */
> + if (IS_GEN9_LP(dev_priv) ||
> + ((IS_CANNONLAKE(dev_priv)) &&
> + !intel_bios_is_port_present(dev_priv, PORT_E))) {
Can you extract all these messy conditions into some kind of (ideally
less convoluted) helper function? In the end I think it would be cool if
this part of the code ends up looking something like:
if (intel_ddi_a_force_4_lanes(dig_port)) {
DRM_DEBUG_KMS(...);
dig_port->saved_port_bits |= DDI_A_4_LANES;
max_lanes = 4;
}
> + DRM_DEBUG_KMS("Forcing DDI_A_4_LANES for port A\n");
> intel_dig_port->saved_port_bits |= DDI_A_4_LANES;
> max_lanes = 4;
> + } else {
> + DRM_DEBUG_KMS("DDI A configured for 2 lanes\n");
> }
> }
>
> --
> 2.13.5
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
