| Message ID | 20171016234449.8669-1-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 16 Oct 2017, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > From: Clint Taylor <clinton.a.taylor@intel.com> > > DDIA Lane capability control 4 lane bit is not being set by firmware during > clone mode boot. This occurs when multiple monitors are connected during > boot. The driver will configure the port for 2 lane maximum width if this > bit is not set. Please be more specific about what you mean with clone mode. > > Cc: Mika Kahola <mika.kahola@intel.com> > Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index a9c0c16e3838..0ad915d71132 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -2791,9 +2791,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port) > * configuration so that we use the proper lane count for our > * calculations. > */ The comment above needs updating too. Also, it doesn't say anything about cloning. BR, Jani. > - if (IS_GEN9_LP(dev_priv) && port == PORT_A) { > + if ((IS_GEN9_LP(dev_priv) || IS_CANNONLAKE(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"); > + DRM_DEBUG_KMS("BIOS forgot to set DDI_A_4_LANES for port A\n"); > intel_dig_port->saved_port_bits |= DDI_A_4_LANES; > max_lanes = 4; > }
On Mon, Oct 16, 2017 at 04:44:49PM -0700, Rodrigo Vivi wrote: > From: Clint Taylor <clinton.a.taylor@intel.com> > > DDIA Lane capability control 4 lane bit is not being set by firmware during > clone mode boot. This occurs when multiple monitors are connected during > boot. The driver will configure the port for 2 lane maximum width if this > bit is not set. > > Cc: Mika Kahola <mika.kahola@intel.com> > Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index a9c0c16e3838..0ad915d71132 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -2791,9 +2791,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port) > * configuration so that we use the proper lane count for our > * calculations. > */ > - if (IS_GEN9_LP(dev_priv) && port == PORT_A) { > + if ((IS_GEN9_LP(dev_priv) || IS_CANNONLAKE(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"); > + DRM_DEBUG_KMS("BIOS forgot to set DDI_A_4_LANES for port A\n"); > intel_dig_port->saved_port_bits |= DDI_A_4_LANES; > max_lanes = 4; > } CNL has DDI E so this doesn't make sense. If there are CNLs out there that don't actually use DDI E but forget to configure the bifurcation coreectly, then we'll need a fancier way to detect that. Ie. we need to be sure DDI E isn't going to be used before we can force DDI A to use 4 lanes.
On Tue, Oct 17, 2017 at 03:10:16PM +0300, Ville Syrjälä wrote: > On Mon, Oct 16, 2017 at 04:44:49PM -0700, Rodrigo Vivi wrote: > > From: Clint Taylor <clinton.a.taylor@intel.com> > > > > DDIA Lane capability control 4 lane bit is not being set by firmware during > > clone mode boot. This occurs when multiple monitors are connected during > > boot. The driver will configure the port for 2 lane maximum width if this > > bit is not set. > > > > Cc: Mika Kahola <mika.kahola@intel.com> > > Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > > index a9c0c16e3838..0ad915d71132 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -2791,9 +2791,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port) > > * configuration so that we use the proper lane count for our > > * calculations. > > */ > > - if (IS_GEN9_LP(dev_priv) && port == PORT_A) { > > + if ((IS_GEN9_LP(dev_priv) || IS_CANNONLAKE(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"); > > + DRM_DEBUG_KMS("BIOS forgot to set DDI_A_4_LANES for port A\n"); > > intel_dig_port->saved_port_bits |= DDI_A_4_LANES; > > max_lanes = 4; > > } > > CNL has DDI E so this doesn't make sense. If there are CNLs out there > that don't actually use DDI E but forget to configure the bifurcation > coreectly, then we'll need a fancier way to detect that. Ie. we need to > be sure DDI E isn't going to be used before we can force DDI A to use > 4 lanes. Oh, we'd really need some way to make sure all four lanes from DDI A are really connected to the panel. Otherwise link training would surely fail if we try to use four lanes.
On Tue, Oct 17, 2017 at 12:10:16PM +0000, Ville Syrjälä wrote: > On Mon, Oct 16, 2017 at 04:44:49PM -0700, Rodrigo Vivi wrote: > > From: Clint Taylor <clinton.a.taylor@intel.com> > > > > DDIA Lane capability control 4 lane bit is not being set by firmware during > > clone mode boot. This occurs when multiple monitors are connected during > > boot. The driver will configure the port for 2 lane maximum width if this > > bit is not set. > > > > Cc: Mika Kahola <mika.kahola@intel.com> > > Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > > index a9c0c16e3838..0ad915d71132 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -2791,9 +2791,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port) > > * configuration so that we use the proper lane count for our > > * calculations. > > */ > > - if (IS_GEN9_LP(dev_priv) && port == PORT_A) { > > + if ((IS_GEN9_LP(dev_priv) || IS_CANNONLAKE(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"); > > + DRM_DEBUG_KMS("BIOS forgot to set DDI_A_4_LANES for port A\n"); > > intel_dig_port->saved_port_bits |= DDI_A_4_LANES; > > max_lanes = 4; > > } > > CNL has DDI E so this doesn't make sense. If there are CNLs out there > that don't actually use DDI E but forget to configure the bifurcation > coreectly, then we'll need a fancier way to detect that. Ie. we need to > be sure DDI E isn't going to be used before we can force DDI A to use > 4 lanes. We got the confirmation that DDI E is not there for any of the current SKUs what support is currently merged. DDI E is only available on the SKUs that posted yesterday... the one that supports DDI F that is the proper split. Also when DDI F is in use DDI E cannot be used because the interrupts handling. So apparently this is not going to be used at all. But I agree that it would be good if we could have a smarter way to detect it... Any idea? Thanks, Rodrigo. > > -- > Ville Syrjälä > Intel OTC
On Tue, Oct 17, 2017 at 08:04:04AM +0000, Jani Nikula wrote: > On Mon, 16 Oct 2017, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > > From: Clint Taylor <clinton.a.taylor@intel.com> > > > > DDIA Lane capability control 4 lane bit is not being set by firmware during > > clone mode boot. This occurs when multiple monitors are connected during > > boot. The driver will configure the port for 2 lane maximum width if this > > bit is not set. > > Please be more specific about what you mean with clone mode. Oh... yes, we need to change the comment. I didn't see any relation here to the clone mode. If eDP comes back on boot and HDMI comes later everything worked. If HDMI is also present during boot at some point during initialization we have probably a long pulse on edp that hits our check of max_level and change that to 2, because this bit is unset as we were expecting. I will grab more information here on the flow and proper update the commit message. > > > > > Cc: Mika Kahola <mika.kahola@intel.com> > > Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > > index a9c0c16e3838..0ad915d71132 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -2791,9 +2791,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port) > > * configuration so that we use the proper lane count for our > > * calculations. > > */ > > The comment above needs updating too. Also, it doesn't say anything > about cloning. I will update this comment also. But also not related to clone mode I believe. > > BR, > Jani. > > > - if (IS_GEN9_LP(dev_priv) && port == PORT_A) { > > + if ((IS_GEN9_LP(dev_priv) || IS_CANNONLAKE(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"); > > + DRM_DEBUG_KMS("BIOS forgot to set DDI_A_4_LANES for port A\n"); > > intel_dig_port->saved_port_bits |= DDI_A_4_LANES; > > max_lanes = 4; > > } > > -- > Jani Nikula, Intel Open Source Technology Center
On Tue, Oct 17, 2017 at 09:27:39AM -0700, Rodrigo Vivi wrote: > On Tue, Oct 17, 2017 at 12:10:16PM +0000, Ville Syrjälä wrote: > > On Mon, Oct 16, 2017 at 04:44:49PM -0700, Rodrigo Vivi wrote: > > > From: Clint Taylor <clinton.a.taylor@intel.com> > > > > > > DDIA Lane capability control 4 lane bit is not being set by firmware during > > > clone mode boot. This occurs when multiple monitors are connected during > > > boot. The driver will configure the port for 2 lane maximum width if this > > > bit is not set. > > > > > > Cc: Mika Kahola <mika.kahola@intel.com> > > > Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_ddi.c | 5 +++-- > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > > > index a9c0c16e3838..0ad915d71132 100644 > > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > > @@ -2791,9 +2791,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port) > > > * configuration so that we use the proper lane count for our > > > * calculations. > > > */ > > > - if (IS_GEN9_LP(dev_priv) && port == PORT_A) { > > > + if ((IS_GEN9_LP(dev_priv) || IS_CANNONLAKE(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"); > > > + DRM_DEBUG_KMS("BIOS forgot to set DDI_A_4_LANES for port A\n"); > > > intel_dig_port->saved_port_bits |= DDI_A_4_LANES; > > > max_lanes = 4; > > > } > > > > CNL has DDI E so this doesn't make sense. If there are CNLs out there > > that don't actually use DDI E but forget to configure the bifurcation > > coreectly, then we'll need a fancier way to detect that. Ie. we need to > > be sure DDI E isn't going to be used before we can force DDI A to use > > 4 lanes. > > We got the confirmation that DDI E is not there for any of the current SKUs > what support is currently merged. > > DDI E is only available on the SKUs that posted yesterday... the one that > supports DDI F that is the proper split. Also when DDI F is in use DDI E > cannot be used because the interrupts handling. > > So apparently this is not going to be used at all. But I agree that it > would be good if we could have a smarter way to detect it... Any idea? Looks like DDI E doesn't have a hardware strap, so I suppose you could just check vbt.ddi_port_info. That still doesn't protect us against accidentally using 4 lanes when we should have used 2. Not sure there's anything in VBT that could help us there.
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index a9c0c16e3838..0ad915d71132 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -2791,9 +2791,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port) * configuration so that we use the proper lane count for our * calculations. */ - if (IS_GEN9_LP(dev_priv) && port == PORT_A) { + if ((IS_GEN9_LP(dev_priv) || IS_CANNONLAKE(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"); + DRM_DEBUG_KMS("BIOS forgot to set DDI_A_4_LANES for port A\n"); intel_dig_port->saved_port_bits |= DDI_A_4_LANES; max_lanes = 4; }