[Intel-gfx] [PATCH 1/3] drm/i915/display/tgl: Use TGL DP tables for eDP ports without low power support

[Souza, Jose]

On Mon, 2020-08-24 at 16:22 -0700, Matt Roper wrote:
> On Wed, Aug 19, 2020 at 11:51:44AM -0700, José Roberto de Souza wrote:
> > Reusing icl_get_combo_buf_trans() for eDP was causing the wrong table
> > being used when the eDP port don't support low power voltage swing table.
> > 
> > Cc: Lee Shawn C <
> > shawn.c.lee at intel.com
> > >
> > Cc: Khaled Almahallawy <
> > khaled.almahallawy at intel.com
> > >
> > Signed-off-by: José Roberto de Souza <
> > jose.souza at intel.com
> > >
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c | 52 +++++++++++++++---------
> >  1 file changed, 33 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index de5b216561d8..9a035bb7bd06 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -1088,30 +1088,44 @@ tgl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >  
> > -	if (type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.hobl) {
> > -		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > -
> > -		if (!intel_dp->hobl_failed && rate <= 540000) {
> > -			/* Same table applies to TGL, RKL and DG1 */
> > -			*n_entries = ARRAY_SIZE(tgl_combo_phy_ddi_translations_edp_hbr2_hobl);
> > -			return tgl_combo_phy_ddi_translations_edp_hbr2_hobl;
> > +	switch (type) {
> > +	case INTEL_OUTPUT_HDMI:
> > +		*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
> > +		return icl_combo_phy_ddi_translations_hdmi;
> > +	case INTEL_OUTPUT_EDP:
> > +		if (dev_priv->vbt.edp.hobl) {
> > +			struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > +
> > +			if (!intel_dp->hobl_failed && rate <= 540000) {
> > +				/* Same table applies to TGL, RKL and DG1 */
> > +				*n_entries = ARRAY_SIZE(tgl_combo_phy_ddi_translations_edp_hbr2_hobl);
> > +				return tgl_combo_phy_ddi_translations_edp_hbr2_hobl;
> > +			}
> >  		}
> > -	}
> >  
> > -	if (type == INTEL_OUTPUT_HDMI || type == INTEL_OUTPUT_EDP) {
> > -		return icl_get_combo_buf_trans(encoder, type, rate, n_entries);
> > -	} else if (rate > 270000) {
> > -		if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv)) {
> > -			*n_entries = ARRAY_SIZE(tgl_uy_combo_phy_ddi_translations_dp_hbr2);
> > -			return tgl_uy_combo_phy_ddi_translations_dp_hbr2;
> > +		if (rate > 540000) {
> > +			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_hbr3);
> > +			return icl_combo_phy_ddi_translations_edp_hbr3;
> 
> So if we have (HBR3 && !low_vswing) we still want to use the eDP table
> values?  How did you figure that out?  The only relevant comment I see
> in the bspec is
> 
>         eDP panels may support lower power, low voltage, swing values
>         using the "eDP" protocol values from the table or higher power,
>         high voltage, swing values using the "DP" protocol values. 
> 
> which doesn't make any specific mention of HBR3 being a special case.

As combo ports in DP mode don't support HBR3 it is trying to use HBR3 table without even check if supported, in this case if the link training
fails intel_dp_get_link_train_fallback_values() will reduce the rate and lanes until link training succeed or completely fails.

But looks unlikely that a vendor will be ship a product with a HBR3 eDP panel and not set low_vswing in VBT.

> 
> 
> Matt
> 
> > +		} else if (dev_priv->vbt.edp.low_vswing) {
> > +			*n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_hbr2);
> > +			return icl_combo_phy_ddi_translations_edp_hbr2;
> > +		}
> > +		/* fall through */
> > +	default:
> > +		/* All combo DP and eDP ports that do not support low_vswing */
> > +		if (rate > 270000) {
> > +			if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv)) {
> > +				*n_entries = ARRAY_SIZE(tgl_uy_combo_phy_ddi_translations_dp_hbr2);
> > +				return tgl_uy_combo_phy_ddi_translations_dp_hbr2;
> > +			}
> > +
> > +			*n_entries = ARRAY_SIZE(tgl_combo_phy_ddi_translations_dp_hbr2);
> > +			return tgl_combo_phy_ddi_translations_dp_hbr2;
> >  		}
> >  
> > -		*n_entries = ARRAY_SIZE(tgl_combo_phy_ddi_translations_dp_hbr2);
> > -		return tgl_combo_phy_ddi_translations_dp_hbr2;
> > +		*n_entries = ARRAY_SIZE(tgl_combo_phy_ddi_translations_dp_hbr);
> > +		return tgl_combo_phy_ddi_translations_dp_hbr;
> >  	}
> > -
> > -	*n_entries = ARRAY_SIZE(tgl_combo_phy_ddi_translations_dp_hbr);
> > -	return tgl_combo_phy_ddi_translations_dp_hbr;
> >  }
> >  
> >  static const struct tgl_dkl_phy_ddi_buf_trans *
> > -- 
> > 2.28.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > 
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> 
>