[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:57 -0700, Matt Roper wrote:
> On Mon, Aug 24, 2020 at 04:45:31PM -0700, Souza, Jose wrote:
> > 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
> 
> In that case should we drop the HBR3 check from the EHL patch?  Because
> on EHL the DP combo PHYs actually can support HBR3.

Oh good catch will fix patch 3.

> 
> 
> Matt
> 
> > 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
> > > > 
> > > > 
> > > 
> > > 
> 
>