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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx
> > > >
> > > Cc: Khaled Almahallawy <
> > > khaled.almahallawy@xxxxxxxxx
> > > >
> > > Signed-off-by: José Roberto de Souza <
> > > jose.souza@xxxxxxxxx
> > > >
> > > ---
> > >  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.


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@xxxxxxxxxxxxxxxxxxxxx
> > > 
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > 
> > 
> > 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux