Re: [Intel-gfx] [PATCH 1/3] drm/i915/dp: Return the right vswing tables

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Mar 30, 2020 at 07:24:55PM +0000, Souza, Jose wrote:
> On Mon, 2020-03-30 at 17:50 +0300, Ville Syrjälä wrote:
> > On Fri, Mar 27, 2020 at 02:34:11PM -0700, José Roberto de Souza
> > wrote:
> > > DDI ports have its encoders initialized with INTEL_OUTPUT_DDI type
> > > and
> > > later eDP ports that have the type changed to INTEL_OUTPUT_EDP.
> > > But for all other DDI ports it can drive HDMI or DP depending on
> > > what
> > > user connects to the ports.
> > > 
> > > ehl_get_combo_buf_trans() and tgl_get_combo_buf_trans() was
> > > checking
> > > for INTEL_OUTPUT_DP that was never true, causing eDP vswing tables
> > > being used.
> > > 
> > > So here changing the check to INTEL_OUTPUT_DDI, HDMI cases will be
> > > correctly handled as it do not use encoder->type, instead it calls
> > > the
> > > functions with INTEL_OUTPUT_HDMI as type parameter and HDMI don't
> > > have
> > > retraining.
> > > 
> > > Fixes: bd3cf6f7ce20 ("drm/i915/dp/tgl+: Update combo phy vswing
> > > tables")
> > > Cc: Clinton A Taylor <clinton.a.taylor@intel.com>
> > > Cc: Matt Roper <matthew.d.roper@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_ddi.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > index 916a802af788..7af1572d4f1d 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > @@ -947,7 +947,7 @@ static const struct cnl_ddi_buf_trans *
> > >  ehl_get_combo_buf_trans(struct drm_i915_private *dev_priv, int
> > > type, int rate,
> > >  			int *n_entries)
> > >  {
> > > -	if (type == INTEL_OUTPUT_DP && rate > 270000) {
> > > +	if (type == INTEL_OUTPUT_DDI && rate > 270000) {
> > 
> > Please no. I'd rather not see "DDI" here. We want to check which mode
> > we're driving the output in, and "DDI" isn't one of the valid
> > choices.
> > 
> > The fact that we sometimes pass in encoder->type is a bit of shortcut
> > to make the DP vs. EDP distinction. And so far every function knew to
> > only compare the value against EDP/HDMI and neve against DP. Looks
> > like
> > someone broke that (admittedly crappy) convention.
> > 
> > We should probably fix this a bit higher up and make sure we only
> > ever
> > pass in EDP/DP/HDMI, never DDI.
> 
> Okay so for now I will just do the bellow:
> 
> if (type != INTEL_OUTPUT_EDP && type != INTEL_OUTPUT_HDMI && rate >
> 270000) {
> 
> Good enough for now?

Works for me.

> > 
> > >  		*n_entries =
> > > ARRAY_SIZE(ehl_combo_phy_ddi_translations_hbr2_hbr3);
> > >  		return ehl_combo_phy_ddi_translations_hbr2_hbr3;
> > >  	}
> > > @@ -959,7 +959,7 @@ static const struct cnl_ddi_buf_trans *
> > >  tgl_get_combo_buf_trans(struct drm_i915_private *dev_priv, int
> > > type, int rate,
> > >  			int *n_entries)
> > >  {
> > > -	if (type != INTEL_OUTPUT_DP) {
> > > +	if (type != INTEL_OUTPUT_DDI) {
> > >  		return icl_get_combo_buf_trans(dev_priv, type, rate,
> > > n_entries);
> > >  	} else if (rate > 270000) {
> > >  		*n_entries =
> > > ARRAY_SIZE(tgl_combo_phy_ddi_translations_dp_hbr2);
> > > -- 
> > > 2.26.0
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx