Re: [Intel-gfx] [PATCH] drm/i915/cnl: Force DDI_A_4_LANES when needed.
On Tue, Oct 24, 2017 at 09:21:38AM +, Ville Syrjälä wrote: > On Mon, Oct 23, 2017 at 10:39:20AM -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. > > > > This patch was initialy start by Clint, but required > > many changes including full commit message. So > > Credits entirely to Clint for finding this. > > > > v2: Extract all messy conditions into a helper function > > as suggested by Ville. > > Along with simplification I removed the debug > > message on the working case since now all conditions > > are grouped. > > v3: Split the conditions even more as suggested by Ville. > > Get's cleaner and easier to add new cases in the > > future. > > > > Suggested-by: Clint Taylor > > Cc: Clint Taylor > > Cc: Mika Kahola > > Cc: Jani Nikula > > Cc: Ville Syrjälä > > Signed-off-by: Rodrigo Vivi > > Reviewed-by: Ville Syrjälä Thanks for review and all great suggestions. Merged to dinq. > > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 46 > > ++-- > > 1 file changed, 35 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > b/drivers/gpu/drm/i915/intel_ddi.c > > index adf51b328844..38a7088bd39c 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -2694,6 +2694,34 @@ intel_ddi_init_hdmi_connector(struct > > intel_digital_port *intel_dig_port) > > return connector; > > } > > > > +static bool intel_ddi_a_force_4_lanes(struct intel_digital_port *dport) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev); > > + > > + if (dport->port != PORT_A) > > + return false; > > + > > + if (dport->saved_port_bits & DDI_A_4_LANES) > > + return false; > > + > > + /* Broxton/Geminilake: Bspec says that DDI_A_4_LANES is the only > > +* supported configuration > > +*/ > > + if (IS_GEN9_LP(dev_priv)) > > + return true; > > + > > + /* 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_CANNONLAKE(dev_priv) && > > + !intel_bios_is_port_present(dev_priv, PORT_E)) > > + return true; > > + > > + return false; > > +} > > + > > void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port) > > { > > struct intel_digital_port *intel_dig_port; > > @@ -2803,18 +2831,14 @@ 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 > > -* configuration so that we use the proper lane count for our > > -* calculations. > > +* Some BIOS might fail to set this bit on port A if eDP > > +* wasn't lit up at boot. Force this bit set when needed > > +* so 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"); > > - intel_dig_port->saved_port_bits |= DDI_A_4_LANES; > > - max_lanes = 4; > > - } > > + if (intel_ddi_a_force_4_lanes(intel_dig_port)) { > > + 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; > > } > > > > intel_dig_port->max_lanes = max_lanes; > > -- > > 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
Re: [Intel-gfx] [PATCH] drm/i915/cnl: Force DDI_A_4_LANES when needed.
On Mon, Oct 23, 2017 at 10:39:20AM -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. > > This patch was initialy start by Clint, but required > many changes including full commit message. So > Credits entirely to Clint for finding this. > > v2: Extract all messy conditions into a helper function > as suggested by Ville. > Along with simplification I removed the debug > message on the working case since now all conditions > are grouped. > v3: Split the conditions even more as suggested by Ville. > Get's cleaner and easier to add new cases in the > future. > > Suggested-by: Clint Taylor > Cc: Clint Taylor > Cc: Mika Kahola > Cc: Jani Nikula > Cc: Ville Syrjälä > Signed-off-by: Rodrigo Vivi Reviewed-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_ddi.c | 46 > ++-- > 1 file changed, 35 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index adf51b328844..38a7088bd39c 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -2694,6 +2694,34 @@ intel_ddi_init_hdmi_connector(struct > intel_digital_port *intel_dig_port) > return connector; > } > > +static bool intel_ddi_a_force_4_lanes(struct intel_digital_port *dport) > +{ > + struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev); > + > + if (dport->port != PORT_A) > + return false; > + > + if (dport->saved_port_bits & DDI_A_4_LANES) > + return false; > + > + /* Broxton/Geminilake: Bspec says that DDI_A_4_LANES is the only > + * supported configuration > + */ > + if (IS_GEN9_LP(dev_priv)) > + return true; > + > + /* 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_CANNONLAKE(dev_priv) && > + !intel_bios_is_port_present(dev_priv, PORT_E)) > + return true; > + > + return false; > +} > + > void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port) > { > struct intel_digital_port *intel_dig_port; > @@ -2803,18 +2831,14 @@ 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 > - * configuration so that we use the proper lane count for our > - * calculations. > + * Some BIOS might fail to set this bit on port A if eDP > + * wasn't lit up at boot. Force this bit set when needed > + * so 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"); > - intel_dig_port->saved_port_bits |= DDI_A_4_LANES; > - max_lanes = 4; > - } > + if (intel_ddi_a_force_4_lanes(intel_dig_port)) { > + 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; > } > > intel_dig_port->max_lanes = max_lanes; > -- > 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
[Intel-gfx] [PATCH] drm/i915/cnl: Force DDI_A_4_LANES when needed.
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. This patch was initialy start by Clint, but required many changes including full commit message. So Credits entirely to Clint for finding this. v2: Extract all messy conditions into a helper function as suggested by Ville. Along with simplification I removed the debug message on the working case since now all conditions are grouped. v3: Split the conditions even more as suggested by Ville. Get's cleaner and easier to add new cases in the future. Suggested-by: Clint Taylor Cc: Clint Taylor Cc: Mika Kahola Cc: Jani Nikula Cc: Ville Syrjälä Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_ddi.c | 46 ++-- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index adf51b328844..38a7088bd39c 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -2694,6 +2694,34 @@ intel_ddi_init_hdmi_connector(struct intel_digital_port *intel_dig_port) return connector; } +static bool intel_ddi_a_force_4_lanes(struct intel_digital_port *dport) +{ + struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev); + + if (dport->port != PORT_A) + return false; + + if (dport->saved_port_bits & DDI_A_4_LANES) + return false; + + /* Broxton/Geminilake: Bspec says that DDI_A_4_LANES is the only +* supported configuration +*/ + if (IS_GEN9_LP(dev_priv)) + return true; + + /* 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_CANNONLAKE(dev_priv) && + !intel_bios_is_port_present(dev_priv, PORT_E)) + return true; + + return false; +} + void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port) { struct intel_digital_port *intel_dig_port; @@ -2803,18 +2831,14 @@ 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 -* configuration so that we use the proper lane count for our -* calculations. +* Some BIOS might fail to set this bit on port A if eDP +* wasn't lit up at boot. Force this bit set when needed +* so 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"); - intel_dig_port->saved_port_bits |= DDI_A_4_LANES; - max_lanes = 4; - } + if (intel_ddi_a_force_4_lanes(intel_dig_port)) { + 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; } intel_dig_port->max_lanes = max_lanes; -- 2.13.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/cnl: Force DDI_A_4_LANES when needed.
On Mon, Oct 23, 2017 at 10:12:00AM -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. > > This patch was initialy start by Clint, but required > many changes including full commit message. So > Credits entirely to Clint for finding this. > > v2: Extract all messy conditions into a helper function > as suggested by Ville. > Along with simplification I removed the debug > message on the working case since now all conditions > are grouped. > > Suggested-by: Clint Taylor > Cc: Clint Taylor > Cc: Mika Kahola > Cc: Jani Nikula > Cc: Ville Syrjälä > Signed-off-by: Rodrigo Vivi > --- > drivers/gpu/drm/i915/intel_ddi.c | 36 +--- > 1 file changed, 25 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index adf51b328844..5b03c24bae97 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -2694,6 +2694,24 @@ intel_ddi_init_hdmi_connector(struct > intel_digital_port *intel_dig_port) > return connector; > } > > +static bool intel_ddi_a_force_4_lanes(struct intel_digital_port *dport) > +{ > + struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev); > + > + /* 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. > + */ > + return dport->port == PORT_A && > + !(dport->saved_port_bits & DDI_A_4_LANES) && > + (IS_GEN9_LP(dev_priv) || > + ((IS_CANNONLAKE(dev_priv)) && ^ ^ Those don't look necessary. It still took me a while to parse that thing. Maybe it should be split even further? Eg. something like: { if (port != PORT_A) return false; if (saved_port_bits & 4_LANES) return false; /* blah */ if (BXT) return true; /* meh */ if (CNL && !port_e_present) return true; return false; } But your code looks correct as well, and I can live with it if you want to keep it as is. So, with the extra parens removed Reviewed-by: Ville Syrjälä > + !intel_bios_is_port_present(dev_priv, PORT_E))); > +} > + > void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port) > { > struct intel_digital_port *intel_dig_port; > @@ -2803,18 +2821,14 @@ 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 > - * configuration so that we use the proper lane count for our > - * calculations. > + * Some BIOS might fail to set this bit on port A if eDP > + * wasn't lit up at boot. Force this bit set when needed > + * so 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"); > - intel_dig_port->saved_port_bits |= DDI_A_4_LANES; > - max_lanes = 4; > - } > + if (intel_ddi_a_force_4_lanes(intel_dig_port)) { > + 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; > } > > intel_dig_port->max_lanes = max_lanes; > -- > 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
[Intel-gfx] [PATCH] drm/i915/cnl: Force DDI_A_4_LANES when needed.
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. This patch was initialy start by Clint, but required many changes including full commit message. So Credits entirely to Clint for finding this. v2: Extract all messy conditions into a helper function as suggested by Ville. Along with simplification I removed the debug message on the working case since now all conditions are grouped. Suggested-by: Clint Taylor Cc: Clint Taylor Cc: Mika Kahola Cc: Jani Nikula Cc: Ville Syrjälä Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_ddi.c | 36 +--- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index adf51b328844..5b03c24bae97 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -2694,6 +2694,24 @@ intel_ddi_init_hdmi_connector(struct intel_digital_port *intel_dig_port) return connector; } +static bool intel_ddi_a_force_4_lanes(struct intel_digital_port *dport) +{ + struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev); + + /* 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. +*/ + return dport->port == PORT_A && + !(dport->saved_port_bits & DDI_A_4_LANES) && + (IS_GEN9_LP(dev_priv) || +((IS_CANNONLAKE(dev_priv)) && + !intel_bios_is_port_present(dev_priv, PORT_E))); +} + void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port) { struct intel_digital_port *intel_dig_port; @@ -2803,18 +2821,14 @@ 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 -* configuration so that we use the proper lane count for our -* calculations. +* Some BIOS might fail to set this bit on port A if eDP +* wasn't lit up at boot. Force this bit set when needed +* so 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"); - intel_dig_port->saved_port_bits |= DDI_A_4_LANES; - max_lanes = 4; - } + if (intel_ddi_a_force_4_lanes(intel_dig_port)) { + 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; } intel_dig_port->max_lanes = max_lanes; -- 2.13.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/cnl: Force DDI_A_4_LANES when needed.
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 > Cc: Clint Taylor > Cc: Mika Kahola > Cc: Jani Nikula > Cc: Ville Syrjälä > Signed-off-by: Rodrigo Vivi > --- > 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
[Intel-gfx] [PATCH] drm/i915/cnl: Force DDI_A_4_LANES when needed.
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 Cc: Clint Taylor Cc: Mika Kahola Cc: Jani Nikula Cc: Ville Syrjälä Signed-off-by: Rodrigo Vivi --- 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))) { + 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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx